Skip to content

Commit

Permalink
[move] Update error behavior on serialization boundaries (#19064)
Browse files Browse the repository at this point in the history
## Description 

Update the semantics around serialization errors when computing type
layouts to follow the underlying layout errors more closely.

## Test plan 

Manually tested.
  • Loading branch information
tzakian authored Aug 21, 2024
1 parent 9a24f66 commit 3182fe6
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 5 deletions.
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 @@ -1341,6 +1341,7 @@
"reject_mutable_random_on_entry_functions": false,
"reshare_at_same_initial_version": false,
"resolve_abort_locations_to_package_id": false,
"rethrow_serialization_type_layout_errors": false,
"scoring_decision_with_validity_cutoff": true,
"shared_object_deletion": false,
"simple_conservation_checks": false,
Expand Down
11 changes: 11 additions & 0 deletions crates/sui-protocol-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ const MAX_PROTOCOL_VERSION: u64 = 55;
// Version 54: Enable random beacon on mainnet.
// Enable soft bundle on mainnet.
// Version 55: Enable enums on mainnet.
// Rethrow serialization type layout errors instead of converting them.

#[derive(Copy, Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct ProtocolVersion(u64);
Expand Down Expand Up @@ -506,6 +507,10 @@ struct FeatureFlags {
// Use AuthorityCapabilitiesV2
#[serde(skip_serializing_if = "is_false")]
authority_capabilities_v2: bool,

// Rethrow type layout errors during serialization instead of trying to convert them.
#[serde(skip_serializing_if = "is_false")]
rethrow_serialization_type_layout_errors: bool,
}

fn is_false(b: &bool) -> bool {
Expand Down Expand Up @@ -1536,6 +1541,10 @@ impl ProtocolConfig {
// 500 is the value used before this field is introduced.
self.consensus_max_num_transactions_in_block.unwrap_or(500)
}

pub fn rethrow_serialization_type_layout_errors(&self) -> bool {
self.feature_flags.rethrow_serialization_type_layout_errors
}
}

#[cfg(not(msim))]
Expand Down Expand Up @@ -2671,6 +2680,8 @@ impl ProtocolConfig {
// Assume 20_000 TPS * 5% max stake per validator / (minimum) 4 blocks per round = 250 transactions per block maximum
// Using a higher limit that is 512, to account for bursty traffic and system transactions.
cfg.consensus_max_num_transactions_in_block = Some(512);

cfg.feature_flags.rethrow_serialization_type_layout_errors = true;
}
// Use this template when making changes:
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ feature_flags:
mysticeti_num_leaders_per_round: 1
soft_bundle: true
enable_coin_deny_list_v2: true
rethrow_serialization_type_layout_errors: true
max_tx_size_bytes: 131072
max_input_objects: 2048
max_size_written_objects: 5000000
Expand Down Expand Up @@ -319,3 +320,4 @@ checkpoint_summary_version_specific_data: 1
max_soft_bundle_size: 5
bridge_should_try_to_finalize_committee: false
max_accumulated_txn_cost_per_object_in_mysticeti_commit: 10

Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ feature_flags:
mysticeti_num_leaders_per_round: 1
soft_bundle: true
enable_coin_deny_list_v2: true
rethrow_serialization_type_layout_errors: true
max_tx_size_bytes: 131072
max_input_objects: 2048
max_size_written_objects: 5000000
Expand Down Expand Up @@ -320,3 +321,4 @@ checkpoint_summary_version_specific_data: 1
max_soft_bundle_size: 5
bridge_should_try_to_finalize_committee: true
max_accumulated_txn_cost_per_object_in_mysticeti_commit: 10

Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ feature_flags:
enable_coin_deny_list_v2: true
passkey_auth: true
authority_capabilities_v2: true
rethrow_serialization_type_layout_errors: true
max_tx_size_bytes: 131072
max_input_objects: 2048
max_size_written_objects: 5000000
Expand Down Expand Up @@ -329,3 +330,4 @@ checkpoint_summary_version_specific_data: 1
max_soft_bundle_size: 5
bridge_should_try_to_finalize_committee: true
max_accumulated_txn_cost_per_object_in_mysticeti_commit: 10

4 changes: 4 additions & 0 deletions external-crates/move/crates/move-vm-config/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ pub struct VMConfig {
pub error_execution_state: bool,
// configuration for binary deserialization (modules)
pub binary_config: BinaryConfig,
// Whether value serialization errors when generating type layouts should be rethrown or
// converted to a different error.
pub rethrow_serialization_type_layout_errors: bool,
}

impl Default for VMConfig {
Expand All @@ -46,6 +49,7 @@ impl Default for VMConfig {
profiler_config: None,
error_execution_state: true,
binary_config: BinaryConfig::with_extraneous_bytes_check(false),
rethrow_serialization_type_layout_errors: false,
}
}
}
Expand Down
19 changes: 14 additions & 5 deletions external-crates/move/crates/move-vm-runtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,20 @@ impl VMRuntime {
_ => (ty, value),
};

let layout = self.loader.type_to_type_layout(ty).map_err(|_err| {
PartialVMError::new(StatusCode::VERIFICATION_ERROR).with_message(
"entry point functions cannot have non-serializable return types".to_string(),
)
})?;
let layout = if self
.loader()
.vm_config()
.rethrow_serialization_type_layout_errors
{
self.loader.type_to_type_layout(ty)?
} else {
self.loader.type_to_type_layout(ty).map_err(|_err| {
PartialVMError::new(StatusCode::VERIFICATION_ERROR).with_message(
"entry point functions cannot have non-serializable return types".to_string(),
)
})?
};

let bytes = value.simple_serialize(&layout).ok_or_else(|| {
PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
.with_message("failed to serialize return values".to_string())
Expand Down
2 changes: 2 additions & 0 deletions sui-execution/latest/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ mod checked {
// Don't augment errors with execution state on-chain
error_execution_state: false,
binary_config: to_binary_config(protocol_config),
rethrow_serialization_type_layout_errors: protocol_config
.rethrow_serialization_type_layout_errors(),
},
)
.map_err(|_| SuiError::ExecutionInvariantViolation)
Expand Down
2 changes: 2 additions & 0 deletions sui-execution/v0/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ mod checked {

profiler_config: vm_profiler_config,
binary_config: to_binary_config(protocol_config),
rethrow_serialization_type_layout_errors: protocol_config
.rethrow_serialization_type_layout_errors(),
},
)
.map_err(|_| SuiError::ExecutionInvariantViolation)
Expand Down
2 changes: 2 additions & 0 deletions sui-execution/v1/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ mod checked {
// Don't augment errors with execution state on-chain
error_execution_state: false,
binary_config: to_binary_config(protocol_config),
rethrow_serialization_type_layout_errors: protocol_config
.rethrow_serialization_type_layout_errors(),
},
)
.map_err(|_| SuiError::ExecutionInvariantViolation)
Expand Down
2 changes: 2 additions & 0 deletions sui-execution/v2/sui-adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ mod checked {
// Don't augment errors with execution state on-chain
error_execution_state: false,
binary_config: to_binary_config(protocol_config),
rethrow_serialization_type_layout_errors: protocol_config
.rethrow_serialization_type_layout_errors(),
},
)
.map_err(|_| SuiError::ExecutionInvariantViolation)
Expand Down

0 comments on commit 3182fe6

Please sign in to comment.