Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sui-adapter] Resolve Move abort locations to package ID instead of runtime ID #17884

Merged
merged 2 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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) } }
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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) } }
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions crates/sui-open-rpc/spec/openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions crates/sui-protocol-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))]
Expand Down Expand Up @@ -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:
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion sui-execution/latest/sui-adapter/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, _, _) => {
Expand All @@ -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));
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
Loading