[upgrade] Upgrade::upgrade_reconfig needs to increment epoch (#945)

* scaffold upgrade logic emit reconfig which doesn't increment epoch.

* move compiles

* patch e2e test

* validator node file should have path to genesis

* testing epoch reconfig

* need to increment epoch otherwise safty rules get lost.

* patch incrementing of epoch

* make upgrade prints always appear

* comments on possible rust implementation

* stashing vm triggering reconfiguration until v5.0.11

* changelog

config/management/genesis/src/ol_node_files.rs

@@ -350,7 +350,7 @@ fn make_validator_cfg(output_dir: PathBuf, namespace: &str) -> Result<NodeConfig
         WaypointConfig::FromStorage(SecureBackend::OnDiskStorage(disk_storage.clone()));
 
     c.execution.backend = SecureBackend::OnDiskStorage(disk_storage.clone());
-    // c.execution.genesis_file_location = output_dir.clone().join("genesis.blob");
+    c.execution.genesis_file_location = output_dir.clone().join("genesis.blob");
 
     c.consensus.safety_rules.service = SafetyRulesService::Thread;
     c.consensus.safety_rules.backend = SecureBackend::OnDiskStorage(disk_storage.clone());

language/diem-framework/modules/0L/Upgrade.move

@@ -43,9 +43,22 @@ module Upgrade {
         temp.payload = payload;
     }
 
+    use 0x1::Epoch;
+    use 0x1::DiemConfig;
+
+    // private. Can only be called by the VM
+    fun upgrade_reconfig(vm: &signer) acquires UpgradePayload {
+        CoreAddresses::assert_vm(vm);
+        reset_payload(vm);
+        let new_epoch_height = Epoch::get_timer_height_start(vm) + 2; // This is janky, but there's no other way to get the current block height, unless the prologue gives it to us. The upgrade reconfigure happens on round 2, so we'll increment the new start by 2 from previous.
+        Epoch::reset_timer(vm, new_epoch_height);
+        DiemConfig::upgrade_reconfig(vm);
+
+    }
+
         // Function code: 03
-    public fun reset_payload(account: &signer) acquires UpgradePayload {
-        assert(Signer::address_of(account) == CoreAddresses::DIEM_ROOT_ADDRESS(), Errors::requires_role(210003)); 
+    fun reset_payload(vm: &signer) acquires UpgradePayload {
+        CoreAddresses::assert_vm(vm);
         assert(exists<UpgradePayload>(CoreAddresses::DIEM_ROOT_ADDRESS()), Errors::not_published(210003)); 
         let temp = borrow_global_mut<UpgradePayload>(CoreAddresses::DIEM_ROOT_ADDRESS());
         temp.payload = Vector::empty<u8>();

language/diem-framework/modules/DiemConfig.move

@@ -4,6 +4,8 @@ address 0x1 {
 /// to synchronize configuration changes for the validators.
 
 module DiemConfig {
+    friend 0x1::Upgrade;
+
     use 0x1::CoreAddresses;
     use 0x1::Errors;
     use 0x1::Event;
@@ -385,13 +387,13 @@ module DiemConfig {
 
     /// Emit a `NewEpochEvent` event but DO NOT increment the EPOCH.
     /// this is used only in upgrade scenarios.
-    fun upgrade_reconfig() acquires Configuration {
-
+    public(friend) fun upgrade_reconfig(vm: &signer) acquires Configuration {
+        CoreAddresses::assert_vm(vm);
         assert(exists<Configuration>(CoreAddresses::DIEM_ROOT_ADDRESS()), Errors::not_published(ECONFIGURATION));
         let config_ref = borrow_global_mut<Configuration>(CoreAddresses::DIEM_ROOT_ADDRESS());
 
-        // Don't increment
-        // config_ref.epoch = 1;
+        // Must increment otherwise the diem-nodes lose track due to safety-rules.
+        config_ref.epoch = config_ref.epoch + 1;
 
         Event::emit_event<NewEpochEvent>(
             &mut config_ref.events,

language/diem-vm/src/diem_vm.rs

@@ -41,7 +41,7 @@ use move_vm_runtime::{
     move_vm::MoveVM,
     session::Session,
 };
-use move_vm_types::{gas_schedule::{calculate_intrinsic_gas, GasStatus}, data_store::DataStore};
+use move_vm_types::gas_schedule::{calculate_intrinsic_gas, GasStatus};
 use std::{convert::TryFrom, sync::Arc};
 use diem_framework_releases::import_stdlib;
 
@@ -478,7 +478,7 @@ impl DiemVMImpl {
         gas_status: &mut GasStatus,
         log_context: &impl LogContext,
     ) -> Result<(), VMStatus> {
-        info!("0L ==== stdlib upgrade: checking for stdlib upgrade");
+        println!("0L ==== stdlib upgrade: checking for stdlib upgrade");
         // tick Oracle::check_upgrade
         let args = vec![
             MoveValue::Signer(txn_data.sender),
@@ -510,7 +510,7 @@ impl DiemVMImpl {
         if round==2 {
             let payload = get_upgrade_payload(remote_cache)?.payload;
             if payload.len() > 0 {
-                info!("0L ==== stdlib upgrade: upgrade payload elected in previous epoch");
+                println!("0L ==== stdlib upgrade: upgrade payload elected in previous epoch");
 
                 // publish the agreed stdlib
                 let new_stdlib = import_stdlib(&payload);
@@ -528,9 +528,10 @@ impl DiemVMImpl {
                     ).expect("Failed to publish module");
                     counter += 1;
                 }
-                info!("0L ==== stdlib upgrade: published {} modules", counter);
 
-                // reset the UpgradePayload
+                println!("0L ==== stdlib upgrade: published {} modules", counter);
+
+                // TODO: This will be deprecated in v5.0.11, see below.
                 let args = vec![
                     MoveValue::Signer(txn_data.sender),
                 ];
@@ -542,21 +543,28 @@ impl DiemVMImpl {
                     // txn_data.sender(),
                     gas_status,
                     log_context,
-                ).expect("Couldn't reset upgrade payload");
-
-                session.execute_function(
-                    &DIEMCONFIG_MODULE,
-                    &UPGRADE_RECONFIG,
-                    vec![],
-                    serialize_values(&vec![]),
-                    // txn_data.sender(),
-                    gas_status,
-                    log_context,
-                ).expect("Couldn't emit reconfig event");
-
-                // session.data_cache.emit_event(guid, seq_num, ty, val)
-
+                ).expect("Couldn't reset payload");
                 info!("==== stdlib upgrade: end upgrade at time: {} ====", timestamp);
+
+                ///////////////////////////////////////////
+
+                // ENABLE THIS CODE ON V5.0.11
+                // trigger a reconfiguration of type Upgrade
+                // let args = vec![
+                //     MoveValue::Signer(txn_data.sender),
+                // ];
+                // session.execute_function(
+                //     &UPGRADE_MODULE,
+                //     &UPGRADE_RECONFIG,
+                //     vec![],
+                //     serialize_values(&args),
+                //     gas_status,
+                //     log_context,
+                // ).expect("Couldn't trigger upgrade reconfig event");
+
+                ///////////////////////////////////////////
+
+                println!("==== stdlib upgrade: end upgrade at time: {} ====", timestamp);
             }
         }
 

language/diem-vm/src/system_module_names.rs

@@ -31,13 +31,6 @@ pub static UPGRADE_MODULE: Lazy<ModuleId> = Lazy::new(|| {
     )
 });
 
-pub static DIEMCONFIG_MODULE: Lazy<ModuleId> = Lazy::new(|| {
-    ModuleId::new(
-        account_config::CORE_CODE_ADDRESS,
-        DIEMCONFIG_MODULE_NAME.clone(),
-    )
-});
-
 // Oracles
 static ORACLE_MODULE_NAME: Lazy<Identifier> =
     Lazy::new(|| Identifier::new("Oracle").unwrap());
@@ -46,9 +39,7 @@ pub static CHECK_UPGRADE: Lazy<Identifier> =
 static UPGRADE_MODULE_NAME: Lazy<Identifier> =
     Lazy::new(|| Identifier::new("Upgrade").unwrap());
 
-static DIEMCONFIG_MODULE_NAME: Lazy<Identifier> =
-    Lazy::new(|| Identifier::new("DiemConfig").unwrap());
-
+// TODO: this will be deprecated in 5.0.11
 pub static RESET_PAYLOAD: Lazy<Identifier> =
     Lazy::new(|| Identifier::new("reset_payload").unwrap());
 

ol/changelog/5_0_10.md

@@ -0,0 +1,67 @@
+## 5.0.10
+
+Resolving the Move Cache issue (https://github.com/OLSF/libra/issues/941) will require a two step upgrade process. 
+
+1. Initially Move changes will be propagated (on chain Hot Upgrade) then, 
+2. later Rust changes will be propagated (Node Upgrade). Read about upgrade types here: ol/documentation/network-upgrades/upgrades.md
+
+This version `v5.0.10` is only the first fase; updates the Move stdlib. DO NOT UPDATE NODE BINARIES. You are only required to vote on the new stdlib file.
+
+Changes to the upgrade logic must be in place on the chain, BEFORE any diem-node changes can be deployed. So for safety and clarity, this version 5.0.10 will NOT implement any interfaces to the Move changes.
+
+
+### TL;DR Deployment
+
+You are only required to submit a vote for the new stdlib file.
+
+The correct way is to build the Move files from source and submit a tx with the file.
+
+```
+cd ~/libra
+git fetch
+git checkout v5.0.10 -f
+make stdlib
+
+# now vote on the upgrade with the file compiled above
+
+txs upgrade-oracle -v -f <source>/language/diem-framework/staged/stdlib.mv
+```
+
+The lazy way is just to vote with the hash of the stdlib which was proposed.
+```
+txs upgrade-oracle -v -h <hash>
+```
+
+### Summary
+
+This upgrade corrects behavior of vfn.node.yaml config creator, and provides tooling for updating on-chain node discovery information (update IP addresses).
+
+### Changes
+
+##### Move Changes
+From issue: https://github.com/OLSF/libra/issues/941
+
+When a "hot upgrade" is taking place the upgrade payload is submitted as a writeset on round 2 of consensus.
+
+If one or more transactions are submitted on this block (which is likely, since at the epoch boundary there is downtime and transactions will accumulate), may be using different mempool caches than what is newly on chain. The solution is to patch the cache.
+
+(https://github.com/diem/diem/blob/main/diem-move/diem-framework/core/sources/DiemConfig.move#L321) is the Move code for emitting the magic reconfiguration event, and here is the Rust code that picks it up. I'm less clear about the exact mechanism for spreading the news of a reconfiguration outward from the latter code, but I believe that it's a mix of the caller looking for these events and ReconfigNotificationListener (depending on the component)
+
+
+PRs:
+https://github.com/OLSF/libra/pull/942
+
+https://github.com/OLSF/libra/pull/945
+
+##### Rust Changes
+
+None 
+
+##### Tests
+
+- All continuous integration tests passed.
+- QA was performed on Rex Devnet.
+
+##### Compatibility
+The Move stdlib and framework changes are backwards compatible with `diem-node` from v5.0.1
+