[Compiler-v2][VM] Add compilation metadata and mainnet check (#12814)

* check metadata * add tests * add metadata test * handle review comments * add feature flag * update release.yaml * minor
aptos-labs · Apr 15, 2024 · afcf9fd · afcf9fd
1 parent 94a139f
commit afcf9fd
Show file tree

Hide file tree

Showing 15 changed files with 335 additions and 14 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/aptos-move/aptos-release-builder/data/release.yaml b/aptos-move/aptos-release-builder/data/release.yaml
@@ -24,3 +24,13 @@ proposals:
           git_hash: ~
       - DefaultGasWithOverride:
           feature_version: 13
+  - name: step_3_enable_checking_unstable_bytecode
+    metadata:
+      title: "Enable the check to reject unstable bytecode to be published to mainnet"
+      description: "Reject unstable bytecode to be published to mainnet"
+      discussion_url: "https://github.com/aptos-labs/aptos-core/pull/12814"
+    execution_mode: MultiStep
+    update_sequence:
+      - FeatureFlag:
+          enabled:
+            - reject_unstable_bytecode
diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs
@@ -108,6 +108,7 @@ pub enum FeatureFlag {
     MultisigV2Enhancement,
     DelegationPoolAllowlisting,
     ModuleEventMigration,
+    RejectUnstableBytecode,
 }
 
 fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
@@ -272,6 +273,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
                 AptosFeatureFlag::DELEGATION_POOL_ALLOWLISTING
             },
             FeatureFlag::ModuleEventMigration => AptosFeatureFlag::MODULE_EVENT_MIGRATION,
+            FeatureFlag::RejectUnstableBytecode => AptosFeatureFlag::REJECT_UNSTABLE_BYTECODE,
         }
     }
 }
@@ -365,6 +367,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
                 FeatureFlag::DelegationPoolAllowlisting
             },
             AptosFeatureFlag::MODULE_EVENT_MIGRATION => FeatureFlag::ModuleEventMigration,
+            AptosFeatureFlag::REJECT_UNSTABLE_BYTECODE => FeatureFlag::RejectUnstableBytecode,
         }
     }
 }

diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs
@@ -1444,6 +1444,12 @@ impl AptosVM {
         mut expected_modules: BTreeSet<String>,
         allowed_deps: Option<BTreeMap<AccountAddress, BTreeSet<String>>>,
     ) -> VMResult<()> {
+        if self
+            .features()
+            .is_enabled(FeatureFlag::REJECT_UNSTABLE_BYTECODE)
+        {
+            self.reject_unstable_bytecode(modules)?;
+        }
         for m in modules {
             if !expected_modules.remove(m.self_id().name().as_str()) {
                 return Err(Self::metadata_validation_error(&format!(
@@ -1486,6 +1492,26 @@ impl AptosVM {
         Ok(())
     }
 
+    /// Check whether the bytecode can be published to mainnet based on the unstable tag in the metadata
+    fn reject_unstable_bytecode(&self, modules: &[CompiledModule]) -> VMResult<()> {
+        if self.move_vm.chain_id().is_mainnet() {
+            for module in modules {
+                if let Some(metadata) =
+                    aptos_framework::get_compilation_metadata_from_compiled_module(module)
+                {
+                    if metadata.unstable {
+                        return Err(PartialVMError::new(StatusCode::UNSTABLE_BYTECODE_REJECTED)
+                            .with_message(
+                                "code marked unstable is not published on mainnet".to_string(),
+                            )
+                            .finish(Location::Undefined));
+                    }
+                }
+            }
+        }
+        Ok(())
+    }
+
     fn metadata_validation_error(msg: &str) -> VMError {
         PartialVMError::new(StatusCode::CONSTRAINT_NOT_SATISFIED)
             .with_message(format!("metadata and code bundle mismatch: {}", msg))

diff --git a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs
@@ -15,7 +15,10 @@ use aptos_gas_algebra::DynamicExpression;
 use aptos_gas_schedule::{MiscGasParameters, NativeGasParameters};
 use aptos_native_interface::SafeNativeBuilder;
 use aptos_table_natives::NativeTableContext;
-use aptos_types::on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, TimedFeatures};
+use aptos_types::{
+    chain_id::ChainId,
+    on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, TimedFeatures},
+};
 use move_binary_format::{
     deserializer::DeserializerConfig,
     errors::VMResult,
@@ -226,6 +229,10 @@ impl MoveVmExt {
     pub(crate) fn features(&self) -> &Features {
         &self.features
     }
+
+    pub fn chain_id(&self) -> ChainId {
+        ChainId::new(self.chain_id)
+    }
 }
 
 impl Deref for MoveVmExt {

diff --git a/aptos-move/e2e-move-tests/src/harness.rs b/aptos-move/e2e-move-tests/src/harness.rs
@@ -9,13 +9,14 @@ use aptos_gas_schedule::{
     AptosGasParameters, FromOnChainGasSchedule, InitialGasSchedule, ToOnChainGasSchedule,
 };
 use aptos_language_e2e_tests::{
-    account::{Account, AccountData},
+    account::{Account, AccountData, TransactionBuilder},
     executor::FakeExecutor,
 };
 use aptos_types::{
     access_path::AccessPath,
     account_address::AccountAddress,
     account_config::{AccountResource, CoinStoreResource, CORE_CODE_ADDRESS},
+    chain_id::ChainId,
     contract_event::ContractEvent,
     move_utils::MemberId,
     on_chain_config::{FeatureFlag, GasScheduleV2, OnChainConfig},
@@ -237,12 +238,12 @@ impl MoveHarness {
         result
     }
 
-    /// Creates a transaction, based on provided payload.
-    pub fn create_transaction_payload(
+    /// Creates a transaction without signing it
+    pub fn create_transaction_without_sign(
         &mut self,
         account: &Account,
         payload: TransactionPayload,
-    ) -> SignedTransaction {
+    ) -> TransactionBuilder {
         let on_chain_seq_no = self.sequence_number(account.address());
         let seq_no_ref = self.txn_seq_no.get_mut(account.address()).unwrap();
         let seq_no = std::cmp::max(on_chain_seq_no, *seq_no_ref);
@@ -253,6 +254,27 @@ impl MoveHarness {
             .max_gas_amount(self.max_gas_per_txn)
             .gas_unit_price(self.default_gas_unit_price)
             .payload(payload)
+    }
+
+    /// Creates a transaction, based on provided payload.
+    /// The chain_id is by default for test
+    pub fn create_transaction_payload(
+        &mut self,
+        account: &Account,
+        payload: TransactionPayload,
+    ) -> SignedTransaction {
+        self.create_transaction_without_sign(account, payload)
+            .sign()
+    }
+
+    /// Creates a transaction to be sent to mainnet
+    pub fn create_transaction_payload_mainnet(
+        &mut self,
+        account: &Account,
+        payload: TransactionPayload,
+    ) -> SignedTransaction {
+        self.create_transaction_without_sign(account, payload)
+            .chain_id(ChainId::mainnet())
             .sign()
     }
 
@@ -267,6 +289,17 @@ impl MoveHarness {
         self.run(txn)
     }
 
+    /// Runs a transaction sent to mainnet
+    pub fn run_transaction_payload_mainnet(
+        &mut self,
+        account: &Account,
+        payload: TransactionPayload,
+    ) -> TransactionStatus {
+        let txn = self.create_transaction_payload_mainnet(account, payload);
+        assert!(self.chain_id_is_mainnet(&CORE_CODE_ADDRESS));
+        self.run(txn)
+    }
+
     /// Runs a transaction and return gas used.
     pub fn evaluate_gas(&mut self, account: &Account, payload: TransactionPayload) -> u64 {
         let txn = self.create_transaction_payload(account, payload);
@@ -795,6 +828,12 @@ impl MoveHarness {
             .sequence_number()
     }
 
+    fn chain_id_is_mainnet(&self, addr: &AccountAddress) -> bool {
+        self.read_resource::<ChainId>(addr, ChainId::struct_tag())
+            .unwrap()
+            .is_mainnet()
+    }
+
     pub fn modify_gas_schedule_raw(&mut self, modify: impl FnOnce(&mut GasScheduleV2)) {
         let mut gas_schedule = self.get_gas_schedule();
         modify(&mut gas_schedule);

diff --git a/aptos-move/e2e-move-tests/src/lib.rs b/aptos-move/e2e-move-tests/src/lib.rs
@@ -52,3 +52,14 @@ pub(crate) fn build_package(
     }
     BuiltPackage::build(package_path.to_owned(), options)
 }
+
+#[cfg(test)]
+pub(crate) fn build_package_with_compiler_version(
+    package_path: PathBuf,
+    options: BuildOptions,
+    compiler_version: CompilerVersion,
+) -> anyhow::Result<BuiltPackage> {
+    let mut options = options;
+    options.compiler_version = Some(compiler_version);
+    BuiltPackage::build(package_path.to_owned(), options)
+}
diff --git a/aptos-move/e2e-move-tests/src/tests/dependencies.rs b/aptos-move/e2e-move-tests/src/tests/dependencies.rs
@@ -68,7 +68,7 @@ fn exceeding_max_dependency_size() {
     let mut h = MoveHarness::new();
 
     h.modify_gas_schedule(|gas_params| {
-        gas_params.vm.txn.max_total_dependency_size = 220.into();
+        gas_params.vm.txn.max_total_dependency_size = 260.into();
     });
 
     let acc = h.new_account_at(AccountAddress::from_hex_literal("0xcafe").unwrap());