Skip to content

Commit ee98574

Browse files
tzakiantzakian
authored
[cherry-pick] Bump to protocol v113 and clean up some execution paths (#25585) (#25588)
Add protocol version 113 with refactored execution check ordering, expanded node config options, and updated consensus validation. Includes snapshot regeneration and new test coverage. CI + some new tests --- 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: Add a new protocol version to support some refactoring in execution. - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] Indexing Framework:
1 parent 69fd218 commit ee98574

18 files changed

+1967
-30
lines changed

‎crates/sui-config/src/node.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ pub struct NodeConfig {
154154
#[serde(default)]
155155
pub transaction_deny_config: TransactionDenyConfig,
156156

157+
/// Whether dev-inspect transaction execution is disabled on this node.
158+
#[serde(default)]
159+
pub dev_inspect_disabled: bool,
160+
157161
#[serde(default)]
158162
pub certificate_deny_config: CertificateDenyConfig,
159163

‎crates/sui-core/src/authority.rs‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,6 +2422,14 @@ impl AuthorityState {
24222422
.into());
24232423
}
24242424

2425+
let dev_inspect = checks.disabled();
2426+
if dev_inspect && self.config.dev_inspect_disabled {
2427+
return Err(SuiErrorKind::UnsupportedFeatureError {
2428+
error: "simulate with checks disabled is not allowed on this node".to_string(),
2429+
}
2430+
.into());
2431+
}
2432+
24252433
// Cheap validity checks for a transaction, including input size limits.
24262434
transaction.validity_check_no_gas_check(epoch_store.protocol_config())?;
24272435

@@ -2466,7 +2474,15 @@ impl AuthorityState {
24662474

24672475
let protocol_config = epoch_store.protocol_config();
24682476

2469-
let (gas_status, checked_input_objects) = if checks.enabled() {
2477+
let (gas_status, checked_input_objects) = if dev_inspect {
2478+
sui_transaction_checks::check_dev_inspect_input(
2479+
protocol_config,
2480+
&transaction,
2481+
input_objects,
2482+
receiving_objects,
2483+
epoch_store.reference_gas_price(),
2484+
)?
2485+
} else {
24702486
sui_transaction_checks::check_transaction_input(
24712487
epoch_store.protocol_config(),
24722488
epoch_store.reference_gas_price(),
@@ -2476,14 +2492,6 @@ impl AuthorityState {
24762492
&self.metrics.bytecode_verifier_metrics,
24772493
&self.config.verifier_signing_config,
24782494
)?
2479-
} else {
2480-
sui_transaction_checks::check_dev_inspect_input(
2481-
protocol_config,
2482-
&transaction,
2483-
input_objects,
2484-
receiving_objects,
2485-
epoch_store.reference_gas_price(),
2486-
)?
24872495
};
24882496

24892497
// TODO see if we can spin up a VM once and reuse it
@@ -2525,7 +2533,7 @@ impl AuthorityState {
25252533
kind,
25262534
signer,
25272535
transaction.digest(),
2528-
checks.disabled(),
2536+
dev_inspect,
25292537
);
25302538

25312539
let loaded_runtime_objects = tracking_store.into_read_objects();
@@ -2610,8 +2618,15 @@ impl AuthorityState {
26102618
.into());
26112619
}
26122620

2613-
let show_raw_txn_data_and_effects = show_raw_txn_data_and_effects.unwrap_or(false);
26142621
let skip_checks = skip_checks.unwrap_or(true);
2622+
if skip_checks && self.config.dev_inspect_disabled {
2623+
return Err(SuiErrorKind::UnsupportedFeatureError {
2624+
error: "dev-inspect with skip_checks is not allowed on this node".to_string(),
2625+
}
2626+
.into());
2627+
}
2628+
2629+
let show_raw_txn_data_and_effects = show_raw_txn_data_and_effects.unwrap_or(false);
26152630
let reference_gas_price = epoch_store.reference_gas_price();
26162631
let protocol_config = epoch_store.protocol_config();
26172632
let max_tx_gas = protocol_config.max_tx_gas();
@@ -2746,6 +2761,7 @@ impl AuthorityState {
27462761
Some(error) => ExecutionOrEarlyError::Err(error),
27472762
None => ExecutionOrEarlyError::Ok(()),
27482763
};
2764+
27492765
let (inner_temp_store, _, effects, execution_result) = executor.dev_inspect_transaction(
27502766
self.get_backing_store().as_ref(),
27512767
protocol_config,

‎crates/sui-core/src/authority/authority_per_epoch_store.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,6 +1496,7 @@ impl AuthorityPerEpochStore {
14961496
config: &self.protocol_config,
14971497
epoch: self.epoch(),
14981498
chain_identifier: self.get_chain_identifier(),
1499+
reference_gas_price: self.reference_gas_price(),
14991500
}
15001501
}
15011502

‎crates/sui-core/src/authority/snapshots/sui_core__authority__execution_time_estimator__tests__execution_time_observer_v113.snap‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
---
2+
source: crates/sui-core/src/authority/execution_time_estimator.rs
3+
expression: snapshot_data
4+
---
5+
protocol_version: 113
6+
consensus_observations:
7+
- - MakeMoveVec
8+
- observations:
9+
- - 2
10+
- secs: 0
11+
nanos: 27000000
12+
- - 7
13+
- secs: 0
14+
nanos: 18000000
15+
- - 10
16+
- secs: 0
17+
nanos: 20000000
18+
- - 1
19+
- secs: 0
20+
nanos: 25000000
21+
stake_weighted_median:
22+
secs: 0
23+
nanos: 25000000
24+
- - MergeCoins
25+
- observations:
26+
- - 9
27+
- secs: 0
28+
nanos: 64000000
29+
- - 2
30+
- secs: 0
31+
nanos: 62000000
32+
- - 2
33+
- secs: 0
34+
nanos: 32000000
35+
- - 7
36+
- secs: 0
37+
nanos: 0
38+
stake_weighted_median:
39+
secs: 0
40+
nanos: 62000000
41+
- - SplitCoins
42+
- observations:
43+
- - 6
44+
- secs: 0
45+
nanos: 42000000
46+
- - 3
47+
- secs: 0
48+
nanos: 25000000
49+
- - 8
50+
- secs: 0
51+
nanos: 67000000
52+
- - 3
53+
- secs: 0
54+
nanos: 61000000
55+
stake_weighted_median:
56+
secs: 0
57+
nanos: 61000000
58+
- - TransferObjects
59+
- observations:
60+
- - 1
61+
- secs: 0
62+
nanos: 13000000
63+
- - 10
64+
- secs: 0
65+
nanos: 80000000
66+
- - 0
67+
- ~
68+
- - 9
69+
- secs: 0
70+
nanos: 39000000
71+
stake_weighted_median:
72+
secs: 0
73+
nanos: 39000000
74+
- - Upgrade
75+
- observations:
76+
- - 0
77+
- ~
78+
- - 8
79+
- secs: 0
80+
nanos: 389000000
81+
- - 10
82+
- secs: 0
83+
nanos: 274000000
84+
- - 4
85+
- secs: 0
86+
nanos: 587000000
87+
stake_weighted_median:
88+
secs: 0
89+
nanos: 389000000
90+
- - MoveEntryPoint:
91+
package: "0x0000000000000000000000000000000000000000000000000000000000000001"
92+
module: coin
93+
function: transfer
94+
type_arguments: []
95+
- observations:
96+
- - 7
97+
- secs: 0
98+
nanos: 404000000
99+
- - 6
100+
- secs: 0
101+
nanos: 162000000
102+
- - 9
103+
- secs: 0
104+
nanos: 268000000
105+
- - 3
106+
- secs: 0
107+
nanos: 254000000
108+
stake_weighted_median:
109+
secs: 0
110+
nanos: 268000000
111+
- - MoveEntryPoint:
112+
package: "0x0000000000000000000000000000000000000000000000000000000000000002"
113+
module: nft
114+
function: mint
115+
type_arguments: []
116+
- observations:
117+
- - 0
118+
- ~
119+
- - 0
120+
- ~
121+
- - 2
122+
- secs: 0
123+
nanos: 142000000
124+
- - 8
125+
- secs: 0
126+
nanos: 499000000
127+
stake_weighted_median:
128+
secs: 0
129+
nanos: 499000000
130+
transaction_estimates:
131+
- - coin_transfer_call
132+
- secs: 0
133+
nanos: 268000000
134+
- - mixed_move_calls
135+
- secs: 0
136+
nanos: 767000000
137+
- - native_commands_with_observations
138+
- secs: 0
139+
nanos: 374000000
140+
- - transfer_objects_2_items
141+
- secs: 0
142+
nanos: 117000000
143+
- - split_coins_3_amounts
144+
- secs: 0
145+
nanos: 244000000
146+
- - merge_coins_3_sources
147+
- secs: 0
148+
nanos: 248000000
149+
- - make_move_vec_4_elements
150+
- secs: 0
151+
nanos: 125000000
152+
- - mixed_commands
153+
- secs: 0
154+
nanos: 240000000
155+
- - upgrade_package
156+
- secs: 0
157+
nanos: 389000000

‎crates/sui-core/src/authority/test_authority_builder.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ pub struct TestAuthorityBuilder<'a> {
6868
authority_overload_config: Option<AuthorityOverloadConfig>,
6969
cache_config: Option<ExecutionCacheConfig>,
7070
chain_override: Option<Chain>,
71+
dev_inspect_disabled: bool,
7172
}
7273

7374
impl<'a> TestAuthorityBuilder<'a> {
@@ -95,6 +96,11 @@ impl<'a> TestAuthorityBuilder<'a> {
9596
self
9697
}
9798

99+
pub fn with_dev_inspect_disabled(mut self) -> Self {
100+
self.dev_inspect_disabled = true;
101+
self
102+
}
103+
98104
pub fn with_certificate_deny_config(mut self, config: CertificateDenyConfig) -> Self {
99105
assert!(self.certificate_deny_config.replace(config).is_none());
100106
self
@@ -349,6 +355,7 @@ impl<'a> TestAuthorityBuilder<'a> {
349355
config.certificate_deny_config = certificate_deny_config;
350356
config.authority_overload_config = authority_overload_config;
351357
config.authority_store_pruning_config = pruning_config;
358+
config.dev_inspect_disabled = self.dev_inspect_disabled;
352359

353360
let chain_identifier = ChainIdentifier::from(*genesis.checkpoint().digest());
354361
let policy_config = config.policy_config.clone();

‎crates/sui-core/src/checkpoints/mod.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,6 +1855,12 @@ impl CheckpointBuilder {
18551855
// added as dependencies, so that those transactions can be waited on using
18561856
// `consensus_messages_processed_notify()`. System transactions (such as
18571857
// settlements) are exempt from this already.
1858+
//
1859+
// However, we DO need to add them to `effects_in_current_checkpoint` so that
1860+
// `complete_checkpoint_effects` won't pull them in again as dependencies when
1861+
// processing later pending checkpoints in the same batch.
1862+
effects_in_current_checkpoint
1863+
.extend(settlement_effects.iter().map(|e| *e.transaction_digest()));
18581864
sorted.extend(settlement_effects);
18591865
}
18601866

‎crates/sui-core/src/consensus_validator.rs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,18 @@ impl SuiTxValidator {
150150
.into());
151151
}
152152
}
153+
154+
if let Some(aliases) = tx.aliases() {
155+
let num_sigs = tx.tx().tx_signatures().len();
156+
for (sig_idx, _) in aliases.iter() {
157+
if (*sig_idx as usize) >= num_sigs {
158+
return Err(SuiErrorKind::UnexpectedMessage(format!(
159+
"UserTransactionV2 alias contains out-of-bounds signature index {sig_idx} (transaction has {num_sigs} signatures)",
160+
)).into());
161+
}
162+
}
163+
}
164+
153165
// TODO(fastpath): move deterministic verifications of user transactions here.
154166
}
155167

@@ -1079,4 +1091,57 @@ mod tests {
10791091
// The executed transaction should NOT be rejected.
10801092
assert!(rejected_transactions.is_empty());
10811093
}
1094+
1095+
#[tokio::test]
1096+
async fn test_reject_invalid_alias_signature_index() {
1097+
let (sender, keypair) = deterministic_random_account_key();
1098+
1099+
let gas_object = Object::with_id_owner_for_testing(ObjectID::random(), sender);
1100+
let owned_object = Object::with_id_owner_for_testing(ObjectID::random(), sender);
1101+
1102+
let network_config =
1103+
sui_swarm_config::network_config_builder::ConfigBuilder::new_with_temp_dir()
1104+
.committee_size(NonZeroUsize::new(1).unwrap())
1105+
.with_objects(vec![gas_object.clone(), owned_object.clone()])
1106+
.build();
1107+
1108+
let state = TestAuthorityBuilder::new()
1109+
.with_network_config(&network_config, 0)
1110+
.build()
1111+
.await;
1112+
1113+
let transaction = test_user_transaction(
1114+
&state,
1115+
sender,
1116+
&keypair,
1117+
gas_object.clone(),
1118+
vec![owned_object.clone()],
1119+
)
1120+
.await;
1121+
1122+
// Extract the inner transaction and construct a PlainTransactionWithClaims
1123+
// with a bogus alias where sig_idx = 255 (far exceeding the 1 signature).
1124+
let inner_tx: Transaction = transaction.into_tx().into();
1125+
let bogus_aliases = nonempty::nonempty![(255u8, None)];
1126+
let tx_with_bogus_alias = PlainTransactionWithClaims::from_aliases(inner_tx, bogus_aliases);
1127+
1128+
let serialized_tx = bcs::to_bytes(&ConsensusTransaction::new_user_transaction_v2_message(
1129+
&state.name,
1130+
tx_with_bogus_alias,
1131+
))
1132+
.unwrap();
1133+
1134+
let validator = SuiTxValidator::new(
1135+
state.clone(),
1136+
state.epoch_store_for_testing().clone(),
1137+
Arc::new(CheckpointServiceNoop {}),
1138+
SuiTxValidatorMetrics::new(&Default::default()),
1139+
);
1140+
1141+
let res = validator.verify_batch(&[&serialized_tx]);
1142+
assert!(
1143+
res.is_err(),
1144+
"Should reject transaction with out-of-bounds alias signature index"
1145+
);
1146+
}
10821147
}

0 commit comments

Comments
 (0)