[fix] epoch trigger feature flag (#168)

docs/core_devs/governance_fixtures.md

@@ -11,13 +11,13 @@ To generate these fixtures we use the `libra-framework cli`.
 
 ```
 cd libra-framework
-cargo r governance --output-dir ../tools/txs/tests/fixtures --framework-local-dir ./libra-framework --only-make-template
+cargo r governance --script-dir ../tools/txs/tests/fixtures --framework-local-dir ./libra-framework --only-make-template
 ```
 
 2. Don't make changes to this file. It should compile as-is. Run the cli tool again.
 ```
 cd libra-framework
-cargo r governance --output-dir ../tools/txs/tests/fixtures --framework-local-dir ./libra-framework
+cargo r governance --script-dir ../tools/txs/tests/fixtures/governance_script_template --framework-local-dir ./libra-framework
 ```
 
 3. check the files are as expected

framework/cached-packages/build.rs

@@ -1,4 +1,4 @@
-use libra_framework::builder::release::ReleaseTarget;
+use libra_framework::release::ReleaseTarget;
 use std::{env::current_dir, path::PathBuf};
 
 fn main() {
@@ -20,7 +20,7 @@ fn main() {
 
         ReleaseTarget::Head
             .create_release(
-                true,
+                false,
                 Some(
                     PathBuf::from(std::env::var("OUT_DIR").expect("OUT_DIR defined"))
                         .join("head.mrb"),

framework/libra-framework/sources/block.move

@@ -3,14 +3,16 @@ module diem_framework::block {
     use std::error;
     use std::vector;
     use std::option;
+    use std::features;
+    use std::string;
     use diem_framework::account;
     use diem_framework::event::{Self, EventHandle};
     use diem_framework::reconfiguration;
     use diem_framework::stake;
     use diem_framework::state_storage;
     use diem_framework::system_addresses;
     use diem_framework::timestamp;
-    // use diem_std::debug::print;
+    use diem_std::debug::print;
     use ol_framework::testnet;
 
     //////// 0L ////////
@@ -153,21 +155,10 @@ module diem_framework::block {
         stake::update_performance_statistics(proposer_index, failed_proposer_indices);
         state_storage::on_new_block(reconfiguration::current_epoch());
 
-        if (timestamp - reconfiguration::last_reconfiguration_time() >=
-        block_metadata_ref.epoch_interval) {
-            // if we are in test mode, have the VM do the reconfiguration
-            if (testnet::is_not_mainnet()) {
-              epoch_boundary::epoch_boundary(
-                &vm,
-                reconfiguration::get_current_epoch(),
-                round
-              );
-            } else {
-              // flip the epoch bit, so that the epoch
-              // boundary can be called by any transaction
-              epoch_boundary::enable_epoch_trigger(&vm, reconfiguration::get_current_epoch());
-            }
-        };
+
+        // seperate logic for epoch advancment to allow for testing
+        maybe_advance_epoch(&vm, timestamp, block_metadata_ref, round);
+
     }
 
     #[view]
@@ -229,11 +220,47 @@ module diem_framework::block {
         );
     }
 
+    fun maybe_advance_epoch(
+        vm: &signer,
+        timestamp: u64,
+        block_metadata_ref: &mut BlockResource,
+        round: u64
+        ) {
+
+            // Check to prevent underflow, return quietly if the check fails
+            if (timestamp < reconfiguration::last_reconfiguration_time()) {
+                print(&string::utf8(b"Timestamp is less than the last reconfiguration time, returning early to prevent underflow. Check block prologue in block.move"));
+                return
+            };
+
+            if (timestamp - reconfiguration::last_reconfiguration_time() >= block_metadata_ref.epoch_interval) {
+                if (!features::epoch_trigger_enabled() || testnet::is_not_mainnet()) {
+                    epoch_boundary::epoch_boundary(
+                        vm,
+                        reconfiguration::get_current_epoch(),
+                        round
+                    );
+                } else {
+                    epoch_boundary::enable_epoch_trigger(vm, reconfiguration::get_current_epoch());
+                }
+            }
+    }
+
     #[test_only]
     public fun initialize_for_test(account: &signer, epoch_interval_microsecs: u64) {
         initialize(account, epoch_interval_microsecs);
     }
 
+    #[test_only]
+    public fun test_maybe_advance_epoch(
+        vm: &signer,
+        timestamp: u64,
+        round: u64,
+    ) acquires BlockResource {
+        let block_metadata_ref = borrow_global_mut<BlockResource>(@diem_framework);
+        maybe_advance_epoch(vm, timestamp, block_metadata_ref, round);
+    }
+
     #[test(diem_framework = @diem_framework)]
     public entry fun test_update_epoch_interval(diem_framework: signer) acquires BlockResource {
         account::create_account_for_test(@diem_framework);

framework/libra-framework/sources/ol_sources/epoch_boundary.move

@@ -190,7 +190,8 @@ module diem_framework::epoch_boundary {
         }
     }
 
-
+    ///TODO: epoch trigger is currently disabled and requires further testing.
+    /// refer to block.move and std::features
     /// flip the bit to allow the epoch to be reconfigured on any transaction
     public(friend) fun enable_epoch_trigger(vm_signer: &signer, closing_epoch:
     u64) acquires BoundaryBit {

framework/libra-framework/sources/ol_sources/tests/boundary.test.move

@@ -2,6 +2,7 @@
 #[test_only]
 module ol_framework::test_boundary {
   use std::vector;
+  use std::features;
   use diem_std::bls12381;
   use ol_framework::mock;
   use ol_framework::proof_of_fee;
@@ -10,13 +11,14 @@ module ol_framework::test_boundary {
   use ol_framework::vouch;
   use ol_framework::testnet;
   use ol_framework::validator_universe;
-  use diem_framework::stake;
   use ol_framework::epoch_boundary;
+  use ol_framework::block;  
   use ol_framework::ol_account;
+  use diem_framework::stake;
   use diem_framework::reconfiguration;
   use diem_framework::timestamp;
   use diem_framework::diem_governance;
-
+  
   // use diem_std::debug::print;
 
   const Alice: address = @0x1000a;
@@ -210,4 +212,57 @@ module ol_framework::test_boundary {
     // aborts
   }
 
+
+  #[test(root = @ol_framework, alice = @0x1000a, marlon = @0x12345)]
+  fun test_epoch_trigger_disabled(root: &signer) {
+    common_test_setup(root);
+    // testing mainnet, so change the chainid
+    testnet::unset(root);
+
+    //verify trigger is not enabled 
+    assert!(!features::epoch_trigger_enabled(), 101);
+
+    // test setup advances to epoch #2
+    let epoch = reconfiguration::get_current_epoch();
+    assert!(epoch == 2, 7357001);
+    epoch_boundary::test_set_boundary_ready(root, epoch);
+
+
+    // case: trigger not set and flipped
+    timestamp::fast_forward_seconds(1); // needed for reconfig
+    block::test_maybe_advance_epoch(root, 602000001, 602000000);
+
+    // test epoch advances
+    let epoch = reconfiguration::get_current_epoch();
+    assert!(epoch == 3, 7357002);
+
+  }
+
+  #[test(root = @ol_framework, alice = @0x1000a, marlon = @0x12345)]
+  fun test_epoch_trigger_enabled(root: &signer) {
+    common_test_setup(root);
+    // testing mainnet, so change the chainid
+    testnet::unset(root);
+    // test setup advances to epoch #2
+    let epoch = reconfiguration::get_current_epoch();
+    assert!(epoch == 2, 7357001);
+    epoch_boundary::test_set_boundary_ready(root, epoch);
+
+    // case: epoch trigger set
+    features::change_feature_flags(root, vector[features::get_epoch_trigger()], vector[]);
+    timestamp::fast_forward_seconds(1); // needed for reconfig
+    block::test_maybe_advance_epoch(root, 603000001, 602000000);
+
+    // test epoch did not advance and needs to be triggered
+    let epoch = reconfiguration::get_current_epoch();
+    assert!(epoch == 2, 7357002);
+
+    // test epoch can be triggered and advances
+    epoch_boundary::test_trigger(root);
+    let epoch = reconfiguration::get_current_epoch();
+    assert!(epoch == 3, 7357002);
+  }
+
+
+
 }

framework/move-stdlib/sources/configs/features.move

@@ -209,6 +209,14 @@ module std::features {
         is_enabled(DIEM_UNIQUE_IDENTIFIERS)
     }
 
+    /// Whether the new epoch trigger logic is enabled.
+    /// Lifetime: transient
+    const EPOCH_TRIGGER_ENABLED: u64 = 24;
+    public fun get_epoch_trigger(): u64 { EPOCH_TRIGGER_ENABLED }
+    public fun epoch_trigger_enabled(): bool acquires Features {
+        is_enabled(EPOCH_TRIGGER_ENABLED)
+    }
+
     // ============================================================================================
     // Feature Flag Implementation
 

framework/src/builder/framework_generate_upgrade_proposal.rs

@@ -1,10 +1,7 @@
 //! generate framework upgrade proposal scripts
 //! see vendor diem-move/framework/src/release_bundle.rs
 
-use crate::{
-    builder::{framework_release_bundle::libra_author_script_file, release::ol_release_default},
-    BYTECODE_VERSION,
-};
+use crate::{builder::framework_release_bundle::libra_author_script_file, BYTECODE_VERSION};
 use anyhow::{ensure, Context, Result};
 use diem_crypto::HashValue;
 use diem_framework::{BuildOptions, BuiltPackage, ReleasePackage};
@@ -59,16 +56,15 @@ pub fn make_framework_upgrade_artifacts(
 
         // We first need to compile and build each CORE MODULE we are upgrading (e.g. MoveStdlib, LibraFramework)
 
-        // let options = BuildOptions {
-        //     with_srcs: true,         // this will store the source bytes on chain, as in genesis
-        //     with_abis: false,        // NOTE: this is set to false in vendor
-        //     with_source_maps: false, // NOTE: this is set to false in vendor
-        //     with_error_map: true,
-        //     skip_fetch_latest_git_deps: true,
-        //     bytecode_version: Some(BYTECODE_VERSION),
-        //     ..BuildOptions::default()
-        // };
-        let options = ol_release_default();
+        let options = BuildOptions {
+            with_srcs: true, // this will store the source bytes on chain, as in genesis
+            with_abis: false,
+            with_source_maps: false,
+            with_error_map: true,
+            skip_fetch_latest_git_deps: true,
+            bytecode_version: Some(BYTECODE_VERSION),
+            ..BuildOptions::default()
+        };
 
         ensure!(
             core_module_dir.exists(),

framework/src/builder/mod.rs

@@ -10,5 +10,3 @@
 // pub mod release_config_ext; // a trait to extend the release config struct see diem-move/diem-release-builder/src/components/mod.rs
 pub mod framework_generate_upgrade_proposal; // see diem-move/diem-release-builder/src/components/framework.rs
 pub mod framework_release_bundle; // note this lives in a different module in vendor. see diem-move/framework/src/release_bundle.rs
-pub mod named_addresses; // OL uses different names for addresses
-pub mod release; // OL uses different names for addresses

framework/src/framework_cli.rs

@@ -1,7 +1,7 @@
 //! framework cli entry points
 
-use crate::builder::{
-    framework_generate_upgrade_proposal::{
+use crate::{
+    builder::framework_generate_upgrade_proposal::{
         init_move_dir_wrapper, libra_compile_script, make_framework_upgrade_artifacts, save_build,
     },
     release::ReleaseTarget,

framework/src/lib.rs

@@ -1,12 +1,12 @@
 pub mod builder;
 pub mod framework_cli;
-// pub mod release;
+pub mod release;
 pub mod upgrade_fixtures;
 
 //////// 0L ///////
 /// Returns the release bundle for the current code.
 pub fn head_release_bundle() -> diem_framework::ReleaseBundle {
-    builder::release::ReleaseTarget::Head
+    release::ReleaseTarget::Head
         .load_bundle()
         .expect("release build failed")
 }