[feature] create platform features module for seperation of vendor an…

…d platform features (#177)

Co-authored-by: 0o-de-lally <[email protected]>

framework/libra-framework/sources/block.move

@@ -3,7 +3,6 @@ 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};
@@ -17,6 +16,7 @@ module diem_framework::block {
 
     //////// 0L ////////
     use ol_framework::epoch_boundary;
+    use ol_framework::ol_features;
 
     friend diem_framework::genesis;
 
@@ -238,7 +238,7 @@ module diem_framework::block {
                 // do automatic epochs in testnet.
                 // in main or stage, check if the feature flag is enabled for
                 // manual epochs
-                if (!features::epoch_trigger_enabled() || testnet::is_testnet()) {
+                if (!ol_features::epoch_trigger_enabled() || testnet::is_testnet()) {
                     epoch_boundary::epoch_boundary(
                         vm,
                         reconfiguration::get_current_epoch(),

framework/libra-framework/sources/multisig_account.move

@@ -456,7 +456,7 @@ module diem_framework::multisig_account {
         metadata_keys: vector<String>,
         metadata_values: vector<vector<u8>>,
     ) acquires MultisigAccount {
-        assert!(features::multisig_accounts_enabled(), error::unavailable(EMULTISIG_ACCOUNTS_NOT_ENABLED_YET));
+        // commit note: multisig accounts enabled permanently
         assert!(
             num_signatures_required > 0 && num_signatures_required <= vector::length(&owners),
             error::invalid_argument(EINVALID_SIGNATURES_REQUIRED),
@@ -957,6 +957,7 @@ module diem_framework::multisig_account {
     use diem_std::multi_ed25519;
     #[test_only]
     use std::string::utf8;
+    #[test_only]
     use std::features;
 
     #[test_only]
@@ -1096,13 +1097,7 @@ module diem_framework::multisig_account {
             vector[]);
     }
 
-    #[test(owner = @0x123)]
-    #[expected_failure(abort_code = 0xD000E, location = Self)]
-    public entry fun test_create_with_without_feature_flag_enabled_should_fail(
-        owner: &signer) acquires MultisigAccount {
-        create_account(address_of(owner));
-        create(owner, 2, vector[], vector[]);
-    }
+    // commit note: not a relevant test, now that it is enabled permanently
 
     #[test(owner_1 = @0x123, owner_2 = @0x124, owner_3 = @0x125)]
     #[expected_failure(abort_code = 0x10001, location = Self)]

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

@@ -0,0 +1,25 @@
+
+// Vendor was silly and mixed the platform implementation's feature definitions
+// together with the feature flag module.
+// So we are moving it to the libra_framework:: module, outside of std::
+
+module ol_framework::ol_features {
+    use std::features;
+
+    // Helper
+    public fun change_feature_flags(framework: &signer, enable: vector<u64>,
+    disable: vector<u64>) {
+      features::change_feature_flags(framework, enable, disable);
+    }
+
+    // NOTE: OL features id begin numbering with 50
+
+    /// Whether the new epoch trigger logic is enabled.
+    /// Lifetime: transient
+    const EPOCH_TRIGGER_ENABLED: u64 = 50;
+    public fun get_epoch_trigger(): u64 { EPOCH_TRIGGER_ENABLED }
+    public fun epoch_trigger_enabled(): bool {
+        features::is_enabled(EPOCH_TRIGGER_ENABLED)
+    }
+
+}

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

@@ -2,7 +2,6 @@
 #[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;
@@ -18,9 +17,11 @@ module ol_framework::test_boundary {
   use diem_framework::reconfiguration;
   use diem_framework::timestamp;
   use diem_framework::diem_governance;
+  use ol_framework::ol_features;
 
   // use diem_std::debug::print;
 
+  // TODO: let's make these consts all caps
   const Alice: address = @0x1000a;
   const Bob: address = @0x1000b;
   const Carol: address = @0x1000c;
@@ -220,11 +221,11 @@ module ol_framework::test_boundary {
     testnet::unset(root);
 
     //verify trigger is not enabled
-    assert!(!features::epoch_trigger_enabled(), 101);
+    assert!(!ol_features::epoch_trigger_enabled(), 7357001);
 
     // test setup advances to epoch #2
     let epoch = reconfiguration::get_current_epoch();
-    assert!(epoch == 2, 7357001);
+    assert!(epoch == 2, 7357002);
     epoch_boundary::test_set_boundary_ready(root, epoch);
 
 
@@ -234,7 +235,7 @@ module ol_framework::test_boundary {
 
     // test epoch advances
     let epoch = reconfiguration::get_current_epoch();
-    assert!(epoch == 3, 7357002);
+    assert!(epoch == 3, 7357003);
 
   }
 
@@ -249,7 +250,7 @@ module ol_framework::test_boundary {
     epoch_boundary::test_set_boundary_ready(root, epoch);
 
     // case: epoch trigger set
-    features::change_feature_flags(root, vector[features::get_epoch_trigger()], vector[]);
+    ol_features::change_feature_flags(root, vector[ol_features::get_epoch_trigger()], vector[]);
     timestamp::fast_forward_seconds(1); // needed for reconfig
     block::test_maybe_advance_epoch(root, 603000001, 602000000);
 

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

@@ -209,14 +209,6 @@ 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
 
@@ -244,8 +236,10 @@ module std::features {
         });
     }
 
+    // commit note: breaking change to ABI because of visibility
+    #[view]
     /// Check whether the feature is enabled.
-    fun is_enabled(feature: u64): bool acquires Features {
+    public fun is_enabled(feature: u64): bool acquires Features {
         exists<Features>(@std) &&
         contains(&borrow_global<Features>(@std).features, feature)
     }

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

@@ -37,27 +37,27 @@ spec std::features {
 
     spec fun spec_is_enabled(feature: u64): bool;
 
-    spec fun spec_periodical_reward_rate_decrease_enabled(): bool {
-        spec_is_enabled(PERIODICAL_REWARD_RATE_DECREASE)
-    }
-
-    spec fun spec_fee_payer_enabled(): bool {
-        spec_is_enabled(FEE_PAYER_ENABLED)
-    }
-
-    spec periodical_reward_rate_decrease_enabled {
-        pragma opaque;
-        aborts_if [abstract] false;
-        ensures [abstract] result == spec_periodical_reward_rate_decrease_enabled();
-    }
-
-    spec fun spec_partial_governance_voting_enabled(): bool {
-        spec_is_enabled(PARTIAL_GOVERNANCE_VOTING)
-    }
-
-    spec partial_governance_voting_enabled {
-        pragma opaque;
-        aborts_if [abstract] false;
-        ensures [abstract] result == spec_partial_governance_voting_enabled();
-    }
+    // spec fun spec_periodical_reward_rate_decrease_enabled(): bool {
+    //     spec_is_enabled(PERIODICAL_REWARD_RATE_DECREASE)
+    // }
+
+    // spec fun spec_fee_payer_enabled(): bool {
+    //     spec_is_enabled(FEE_PAYER_ENABLED)
+    // }
+
+    // spec periodical_reward_rate_decrease_enabled {
+    //     pragma opaque;
+    //     aborts_if [abstract] false;
+    //     ensures [abstract] result == spec_periodical_reward_rate_decrease_enabled();
+    // }
+
+    // spec fun spec_partial_governance_voting_enabled(): bool {
+    //     spec_is_enabled(PARTIAL_GOVERNANCE_VOTING)
+    // }
+
+    // spec partial_governance_voting_enabled {
+    //     pragma opaque;
+    //     aborts_if [abstract] false;
+    //     ensures [abstract] result == spec_partial_governance_voting_enabled();
+    // }
 }

framework/src/upgrade_fixtures/fixtures/upgrade-multi-lib/1-move-stdlib/script_sha3

@@ -1 +1 @@
-614568d14fabba0e8833d4bb974f57e6871b948ebd997082542afbd5b47c0bae
+cf51bda815dcf76e610e85586adb1119b456cdc647c367ac108934101e442552