[sui-adapter] Resolve Move abort locations to package ID instead of r…

…untime ID (#17884) ## Description Updates Move aborts so that the address in the abort location uses the package ID as opposed to the runtime module ID. Basically, before if you had a package `P` with a module `M` ```move module P::M { public fun aborter() { abort 0 } } ``` published at `0xA` for version 1, `0xB` for version 2, and `0xC` for version 3, then before this change ``` 0xA::aborter().location.address == 0xA 0xB::aborter().location.address == 0xA 0xC::aborter().location.address = 0xA ``` After this change ``` 0xA::aborter().location.address == 0xA 0xB::aborter().location.address == 0xB 0xC::aborter().location.address == 0xC ``` The only meaningful changes are in `sui-execution/latest/sui-adapter/src/error.rs` the other changes are just plumbing/protocol config updates and tests. ## Test plan Added tests to make sure existing behavior is preserved, and that new behavior works as expected. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [X] Protocol: Added new protocol version (47) and enabled resolving Move abort locations to their package ID instead of runtime ID. - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
MystenLabs · May 23, 2024 · be586e7 · be586e7
1 parent 35ea420
commit be586e7
Show file tree

Hide file tree

Showing 11 changed files with 207 additions and 21 deletions.
diff --git a/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution.exp b/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution.exp
@@ -0,0 +1,31 @@
+processed 7 tasks
+
+init:
+A: object(0,0)
+
+task 1 'publish'. lines 6-11:
+created: object(1,0), object(1,1)
+mutated: object(0,0)
+gas summary: computation_cost: 1000000, storage_cost: 5084400,  storage_rebate: 0, non_refundable_storage_fee: 0
+
+task 2 'upgrade'. lines 14-19:
+created: object(2,0)
+mutated: object(0,0), object(1,1)
+gas summary: computation_cost: 1000000, storage_cost: 5084400,  storage_rebate: 2595780, non_refundable_storage_fee: 26220
+
+task 3 'upgrade'. lines 21-26:
+created: object(3,0)
+mutated: object(0,0), object(1,1)
+gas summary: computation_cost: 1000000, storage_cost: 5084400,  storage_rebate: 2595780, non_refundable_storage_fee: 26220
+
+task 4 'run'. lines 28-30:
+Error: Transaction Effects Status: Move Runtime Abort. Location: Test1::M1::f1 (function index 0) at offset 1, Abort Code: 0
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: MoveAbort(MoveLocation { module: ModuleId { address: Test1, name: Identifier("M1") }, function: 0, instruction: 1, function_name: Some("f1") }, 0), source: Some(VMError { major_status: ABORTED, sub_status: Some(0), message: Some("Test1::M1::f1 at offset 1"), exec_state: None, location: Module(ModuleId { address: Test1, name: Identifier("M1") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 1)] }), command: Some(0) } }
+
+task 5 'run'. lines 31-33:
+Error: Transaction Effects Status: Move Runtime Abort. Location: Test2::M1::f1 (function index 0) at offset 1, Abort Code: 0
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: MoveAbort(MoveLocation { module: ModuleId { address: Test2, name: Identifier("M1") }, function: 0, instruction: 1, function_name: Some("f1") }, 0), source: Some(VMError { major_status: ABORTED, sub_status: Some(0), message: Some("Test1::M1::f1 at offset 1"), exec_state: None, location: Module(ModuleId { address: Test1, name: Identifier("M1") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 1)] }), command: Some(0) } }
+
+task 6 'run'. lines 34-34:
+Error: Transaction Effects Status: Move Runtime Abort. Location: Test3::M1::f1 (function index 0) at offset 1, Abort Code: 0
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: MoveAbort(MoveLocation { module: ModuleId { address: Test3, name: Identifier("M1") }, function: 0, instruction: 1, function_name: Some("f1") }, 0), source: Some(VMError { major_status: ABORTED, sub_status: Some(0), message: Some("Test1::M1::f1 at offset 1"), exec_state: None, location: Module(ModuleId { address: Test1, name: Identifier("M1") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 1)] }), command: Some(0) } }
diff --git a/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution.move b/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution.move
@@ -0,0 +1,34 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//# init --addresses Test1=0x0 Test2=0x0 Test3=0x0 --accounts A
+
+//# publish --upgradeable --sender A
+module Test1::M1 {
+    public fun f1() { 
+        abort 0
+    }
+}
+
+
+//# upgrade --package Test1 --upgrade-capability 1,1 --sender A
+module Test2::M1 {
+    public fun f1() { 
+        abort 0
+    }
+}
+
+//# upgrade --package Test2 --upgrade-capability 1,1 --sender A
+module Test3::M1 {
+    public fun f1() { 
+        abort 0
+    }
+}
+
+//# run Test1::M1::f1
+
+// Location will show up as Test2::M1::f1 since the runtime module ID is resolved to the upgraded version
+//# run Test2::M1::f1
+
+// Location will show up as Test3::M1::f1 as the runtime module ID  is resolved to the upgraded version
+//# run Test3::M1::f1
diff --git a/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution_v46.exp b/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution_v46.exp
@@ -0,0 +1,31 @@
+processed 7 tasks
+
+init:
+A: object(0,0)
+
+task 1 'publish'. lines 6-11:
+created: object(1,0), object(1,1)
+mutated: object(0,0)
+gas summary: computation_cost: 1000000, storage_cost: 5084400,  storage_rebate: 0, non_refundable_storage_fee: 0
+
+task 2 'upgrade'. lines 14-19:
+created: object(2,0)
+mutated: object(0,0), object(1,1)
+gas summary: computation_cost: 1000000, storage_cost: 5084400,  storage_rebate: 2595780, non_refundable_storage_fee: 26220
+
+task 3 'upgrade'. lines 21-26:
+created: object(3,0)
+mutated: object(0,0), object(1,1)
+gas summary: computation_cost: 1000000, storage_cost: 5084400,  storage_rebate: 2595780, non_refundable_storage_fee: 26220
+
+task 4 'run'. lines 28-30:
+Error: Transaction Effects Status: Move Runtime Abort. Location: Test1::M1::f1 (function index 0) at offset 1, Abort Code: 0
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: MoveAbort(MoveLocation { module: ModuleId { address: Test1, name: Identifier("M1") }, function: 0, instruction: 1, function_name: Some("f1") }, 0), source: Some(VMError { major_status: ABORTED, sub_status: Some(0), message: Some("Test1::M1::f1 at offset 1"), exec_state: None, location: Module(ModuleId { address: Test1, name: Identifier("M1") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 1)] }), command: Some(0) } }
+
+task 5 'run'. lines 31-33:
+Error: Transaction Effects Status: Move Runtime Abort. Location: Test1::M1::f1 (function index 0) at offset 1, Abort Code: 0
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: MoveAbort(MoveLocation { module: ModuleId { address: Test1, name: Identifier("M1") }, function: 0, instruction: 1, function_name: Some("f1") }, 0), source: Some(VMError { major_status: ABORTED, sub_status: Some(0), message: Some("Test1::M1::f1 at offset 1"), exec_state: None, location: Module(ModuleId { address: Test1, name: Identifier("M1") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 1)] }), command: Some(0) } }
+
+task 6 'run'. lines 34-34:
+Error: Transaction Effects Status: Move Runtime Abort. Location: Test1::M1::f1 (function index 0) at offset 1, Abort Code: 0
+Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: MoveAbort(MoveLocation { module: ModuleId { address: Test1, name: Identifier("M1") }, function: 0, instruction: 1, function_name: Some("f1") }, 0), source: Some(VMError { major_status: ABORTED, sub_status: Some(0), message: Some("Test1::M1::f1 at offset 1"), exec_state: None, location: Module(ModuleId { address: Test1, name: Identifier("M1") }), indices: [], offsets: [(FunctionDefinitionIndex(0), 1)] }), command: Some(0) } }
diff --git a/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution_v46.move b/crates/sui-adapter-transactional-tests/tests/upgrade/abort_code_resolution_v46.move
@@ -0,0 +1,34 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//# init --addresses Test1=0x0 Test2=0x0 Test3=0x0 --accounts A --protocol-version 46
+
+//# publish --upgradeable --sender A
+module Test1::M1 {
+    public fun f1() { 
+        abort 0
+    }
+}
+
+
+//# upgrade --package Test1 --upgrade-capability 1,1 --sender A
+module Test2::M1 {
+    public fun f1() { 
+        abort 0
+    }
+}
+
+//# upgrade --package Test2 --upgrade-capability 1,1 --sender A
+module Test3::M1 {
+    public fun f1() { 
+        abort 0
+    }
+}
+
+//# run Test1::M1::f1
+
+// Location will show up as Test1::M1::f1 since the module ID is not resolved to the upgraded version
+//# run Test2::M1::f1
+
+// Location will show up as Test1::M1::f1 since the module ID is not resolved to the upgraded version
+//# run Test3::M1::f1
diff --git a/crates/sui-open-rpc/spec/openrpc.json b/crates/sui-open-rpc/spec/openrpc.json
@@ -1398,6 +1398,7 @@
                 "recompute_has_public_transfer_in_execution": false,
                 "reject_mutable_random_on_entry_functions": false,
                 "reshare_at_same_initial_version": false,
+                "resolve_abort_locations_to_package_id": false,
                 "scoring_decision_with_validity_cutoff": true,
                 "shared_object_deletion": false,
                 "simple_conservation_checks": false,

diff --git a/crates/sui-protocol-config/src/lib.rs b/crates/sui-protocol-config/src/lib.rs
@@ -130,6 +130,7 @@ const MAX_PROTOCOL_VERSION: u64 = 47;
 // Version 46: Enable native bridge in testnet
 //             Enable resharing at the same initial shared version.
 // Version 47: Use tonic networking for Mysticeti.
+//             Resolve Move abort locations to the package id instead of the runtime module ID.
 
 #[derive(Copy, Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ProtocolVersion(u64);
@@ -437,6 +438,10 @@ struct FeatureFlags {
     // Enable resharing of shared objects using the same initial shared version
     #[serde(skip_serializing_if = "is_false")]
     reshare_at_same_initial_version: bool,
+
+    // Resolve Move abort locations to the package id instead of the runtime module ID.
+    #[serde(skip_serializing_if = "is_false")]
+    resolve_abort_locations_to_package_id: bool,
 }
 
 fn is_false(b: &bool) -> bool {
@@ -1322,6 +1327,10 @@ impl ProtocolConfig {
     pub fn reshare_at_same_initial_version(&self) -> bool {
         self.feature_flags.reshare_at_same_initial_version
     }
+
+    pub fn resolve_abort_locations_to_package_id(&self) -> bool {
+        self.feature_flags.resolve_abort_locations_to_package_id
+    }
 }
 
 #[cfg(not(msim))]
@@ -2207,6 +2216,9 @@ impl ProtocolConfig {
                 47 => {
                     // Use tonic networking for Mysticeti.
                     cfg.feature_flags.consensus_network = ConsensusNetwork::Tonic;
+
+                    // Enable resolving abort code IDs to package ID instead of runtime module ID
+                    cfg.feature_flags.resolve_abort_locations_to_package_id = true;
                 }
                 // Use this template when making changes:
                 //

diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_47.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_47.snap
@@ -46,6 +46,7 @@ feature_flags:
   consensus_network: Tonic
   zklogin_max_epoch_upper_bound_delta: 30
   reshare_at_same_initial_version: true
+  resolve_abort_locations_to_package_id: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_47.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_47.snap
@@ -49,6 +49,7 @@ feature_flags:
   zklogin_max_epoch_upper_bound_delta: 30
   mysticeti_leader_scoring_and_schedule: true
   reshare_at_same_initial_version: true
+  resolve_abort_locations_to_package_id: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_47.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_47.snap
@@ -52,6 +52,7 @@ feature_flags:
   zklogin_max_epoch_upper_bound_delta: 30
   mysticeti_leader_scoring_and_schedule: true
   reshare_at_same_initial_version: true
+  resolve_abort_locations_to_package_id: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

diff --git a/sui-execution/latest/sui-adapter/src/error.rs b/sui-execution/latest/sui-adapter/src/error.rs
@@ -17,6 +17,7 @@ pub(crate) fn convert_vm_error<S: MoveResolver<Err = SuiError>>(
     error: VMError,
     vm: &MoveVM,
     state_view: &S,
+    resolve_abort_location_to_package_id: bool,
 ) -> ExecutionError {
     let kind = match (error.major_status(), error.sub_status(), error.location()) {
         (StatusCode::EXECUTED, _, _) => {
@@ -30,6 +31,11 @@ pub(crate) fn convert_vm_error<S: MoveResolver<Err = SuiError>>(
             ExecutionFailureStatus::VMInvariantViolation
         }
         (StatusCode::ABORTED, Some(code), Location::Module(id)) => {
+            let abort_location_id = if resolve_abort_location_to_package_id {
+                state_view.relocate(id).unwrap_or_else(|_| id.clone())
+            } else {
+                id.clone()
+            };
             let offset = error.offsets().first().copied().map(|(f, i)| (f.0, i));
             debug_assert!(offset.is_some(), "Move should set the location on aborts");
             let (function, instruction) = offset.unwrap_or((0, 0));
@@ -40,7 +46,7 @@ pub(crate) fn convert_vm_error<S: MoveResolver<Err = SuiError>>(
             });
             ExecutionFailureStatus::MoveAbort(
                 MoveLocation {
-                    module: id.clone(),
+                    module: abort_location_id,
                     function,
                     instruction,
                     function_name,

diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/context.rs
@@ -757,15 +757,23 @@ mod checked {
             }
 
             for (id, (recipient, ty, value)) in writes {
-                let abilities = vm
-                    .get_runtime()
-                    .get_type_abilities(&ty)
-                    .map_err(|e| convert_vm_error(e, vm, &linkage_view))?;
+                let abilities = vm.get_runtime().get_type_abilities(&ty).map_err(|e| {
+                    convert_vm_error(
+                        e,
+                        vm,
+                        &linkage_view,
+                        protocol_config.resolve_abort_locations_to_package_id(),
+                    )
+                })?;
                 let has_public_transfer = abilities.has_store();
-                let layout = vm
-                    .get_runtime()
-                    .type_to_type_layout(&ty)
-                    .map_err(|e| convert_vm_error(e, vm, &linkage_view))?;
+                let layout = vm.get_runtime().type_to_type_layout(&ty).map_err(|e| {
+                    convert_vm_error(
+                        e,
+                        vm,
+                        &linkage_view,
+                        protocol_config.resolve_abort_locations_to_package_id(),
+                    )
+                })?;
                 let Some(bytes) = value.simple_serialize(&layout) else {
                     invariant_violation!("Failed to deserialize already serialized Move value");
                 };
@@ -849,7 +857,12 @@ mod checked {
 
         /// Convert a VM Error to an execution one
         pub fn convert_vm_error(&self, error: VMError) -> ExecutionError {
-            crate::error::convert_vm_error(error, self.vm, &self.linkage_view)
+            crate::error::convert_vm_error(
+                error,
+                self.vm,
+                &self.linkage_view,
+                self.protocol_config.resolve_abort_locations_to_package_id(),
+            )
         }
 
         /// Special case errors for type arguments to Move functions
@@ -1154,13 +1167,23 @@ mod checked {
         };
 
         let tag: StructTag = type_.into();
-        let type_ = load_type_from_struct(vm, linkage_view, new_packages, &tag)
-            .map_err(|e| crate::error::convert_vm_error(e, vm, linkage_view))?;
+        let type_ = load_type_from_struct(vm, linkage_view, new_packages, &tag).map_err(|e| {
+            crate::error::convert_vm_error(
+                e,
+                vm,
+                linkage_view,
+                protocol_config.resolve_abort_locations_to_package_id(),
+            )
+        })?;
         let has_public_transfer = if protocol_config.recompute_has_public_transfer_in_execution() {
-            let abilities = vm
-                .get_runtime()
-                .get_type_abilities(&type_)
-                .map_err(|e| crate::error::convert_vm_error(e, vm, linkage_view))?;
+            let abilities = vm.get_runtime().get_type_abilities(&type_).map_err(|e| {
+                crate::error::convert_vm_error(
+                    e,
+                    vm,
+                    linkage_view,
+                    protocol_config.resolve_abort_locations_to_package_id(),
+                )
+            })?;
             abilities.has_store()
         } else {
             has_public_transfer
@@ -1243,7 +1266,14 @@ mod checked {
             let fully_annotated_layout = vm
                 .get_runtime()
                 .type_to_fully_annotated_layout(&obj_value.type_)
-                .map_err(|e| convert_vm_error(e, vm, linkage_view))?;
+                .map_err(|e| {
+                    convert_vm_error(
+                        e,
+                        vm,
+                        linkage_view,
+                        protocol_config.resolve_abort_locations_to_package_id(),
+                    )
+                })?;
             let mut bytes = vec![];
             obj_value.write_bcs_bytes(&mut bytes);
             match get_all_uids(&fully_annotated_layout, &bytes) {
@@ -1401,10 +1431,14 @@ mod checked {
             .get(&id)
             .map(|obj: &LoadedRuntimeObject| obj.version);
 
-        let type_tag = vm
-            .get_runtime()
-            .get_type_tag(&type_)
-            .map_err(|e| crate::error::convert_vm_error(e, vm, linkage_view))?;
+        let type_tag = vm.get_runtime().get_type_tag(&type_).map_err(|e| {
+            crate::error::convert_vm_error(
+                e,
+                vm,
+                linkage_view,
+                protocol_config.resolve_abort_locations_to_package_id(),
+            )
+        })?;
 
         let struct_tag = match type_tag {
             TypeTag::Struct(inner) => *inner,