Skip to content

Commit 9512e7d

Browse files
authored
[3/6] Generalize accumulator version into a system object versions map (#27104)
## Description Stack of 6 (builds on [2/6]). Generalizes `AssignedVersions.accumulator_version` (a single `Option<SequenceNumber>`) into a `system_object_versions: BTreeMap<ObjectID, SequenceNumber>` map, so consensus version assignment can pin arbitrary system objects — not just the accumulator root — to the version a transaction is sequenced against, and wires the authority to pass that map straight into execution (replacing the accumulator-root-only map built inline in [1/6]). An `accumulator_version()` accessor keeps existing callers working. Pure refactor: the map still holds at most the accumulator root version today, so behavior is unchanged. ## Test plan Covered by the existing `shared_object_version_manager` unit tests, updated to construct `AssignedVersions` through the new API, and by the address-funds e2e tests, which assign the accumulator version for object-funds withdrawals and exercise the `accumulator_version()` accessor end to end. ## Release notes Check each box that your change affects. If none of the boxes relate to your changes, release notes are not required. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
1 parent 561996d commit 9512e7d

9 files changed

Lines changed: 139 additions & 128 deletions

File tree

crates/sui-core/src/accumulators/object_funds_checker/integration_tests.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,10 @@ impl TestEnv {
175175

176176
fn execution_env(&self) -> ExecutionEnv {
177177
let accumulator_version = self.oref(&SUI_ACCUMULATOR_ROOT_OBJECT_ID).1;
178-
ExecutionEnv::new()
179-
.with_assigned_versions(AssignedVersions::new(vec![], Some(accumulator_version)))
178+
ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
179+
vec![],
180+
Some(accumulator_version),
181+
))
180182
}
181183

182184
/// Executes at the current accumulator version, expecting success.
@@ -236,8 +238,10 @@ async fn test_object_withdraw_basic_flow() {
236238
.authority
237239
.try_execute_immediately(
238240
&cert,
239-
ExecutionEnv::new()
240-
.with_assigned_versions(AssignedVersions::new(vec![], Some(accumulator_version))),
241+
ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
242+
vec![],
243+
Some(accumulator_version),
244+
)),
241245
&env.epoch_store,
242246
)
243247
.unwrap()
@@ -270,7 +274,7 @@ async fn test_object_withdraw_multiple_withdraws() {
270274
// Fastpath execution
271275
.try_execute_immediately(
272276
&cert,
273-
ExecutionEnv::new().with_assigned_versions(AssignedVersions::new(
277+
ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
274278
vec![],
275279
Some(accumulator_version),
276280
)),
@@ -309,7 +313,7 @@ async fn test_object_withdraw_multiple_withdraws() {
309313
// Fastpath execution
310314
.try_execute_immediately(
311315
&cert,
312-
ExecutionEnv::new().with_assigned_versions(AssignedVersions::new(
316+
ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
313317
vec![],
314318
Some(accumulator_version),
315319
)),

crates/sui-core/src/accumulators/object_funds_checker/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ impl ObjectFundsChecker {
124124
// for the epoch, and every production path that produces such a tx also
125125
// assigns an accumulator version. The `None` paths (accumulator-disabled
126126
// epoch, end-of-epoch tx) never produce withdraws and so never reach here.
127-
let Some(accumulator_version) = execution_env.assigned_versions.accumulator_version else {
127+
let Some(accumulator_version) = execution_env.assigned_versions.accumulator_version()
128+
else {
128129
debug_fatal!("accumulator_version must be set for a tx with object withdraws");
129130
return false;
130131
};

crates/sui-core/src/accumulators/object_funds_checker/unit_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ async fn test_should_commit_early_exits() {
399399
&tx,
400400
&TestEffectsBuilder::new(tx.data()).build(),
401401
&withdraws,
402-
&ExecutionEnv::new().with_assigned_versions(AssignedVersions::new(
402+
&ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
403403
vec![],
404404
Some(SequenceNumber::from_u64(0))
405405
)),
@@ -419,7 +419,7 @@ async fn test_should_commit_early_exits() {
419419
)))
420420
.build(),
421421
&withdraws,
422-
&ExecutionEnv::new().with_assigned_versions(AssignedVersions::new(
422+
&ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
423423
vec![],
424424
Some(SequenceNumber::from_u64(0))
425425
)),
@@ -482,7 +482,7 @@ async fn test_should_commit_ignores_zero_net_withdraws() {
482482
&tx,
483483
&effects,
484484
&running_max_withdraws,
485-
&ExecutionEnv::new().with_assigned_versions(AssignedVersions::new(
485+
&ExecutionEnv::new().with_assigned_versions(AssignedVersions::new_for_testing(
486486
vec![],
487487
Some(SequenceNumber::from_u64(0))
488488
)),

crates/sui-core/src/authority.rs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,7 +1541,7 @@ impl AuthorityState {
15411541
return ExecutionOutput::EpochEnded;
15421542
}
15431543

1544-
let accumulator_version = execution_env.assigned_versions.accumulator_version;
1544+
let accumulator_version = execution_env.assigned_versions.accumulator_version();
15451545

15461546
let (transaction_outputs, timings, execution_error_opt) = match self.process_certificate(
15471547
&tx_guard,
@@ -2060,15 +2060,14 @@ impl AuthorityState {
20602060
self.config.certificate_deny_config.certificate_deny_set(),
20612061
&execution_env.funds_withdraw_status,
20622062
);
2063-
let accumulator_version = execution_env.assigned_versions.accumulator_version;
2064-
// The accumulator root is the only system object a transaction is sequenced against today.
2065-
// Pin its version so execution can gate reads on it and record a retry if this node has not
2066-
// caught up; see `TemporaryStore::check_system_object_available`. A later PR generalizes this
2067-
// to an arbitrary set of system objects.
2068-
let system_object_versions: BTreeMap<ObjectID, SequenceNumber> = accumulator_version
2069-
.map(|v| (SUI_ACCUMULATOR_ROOT_OBJECT_ID, v))
2070-
.into_iter()
2071-
.collect();
2063+
// Versions of system objects this transaction may read during execution, each at the version
2064+
// it was sequenced against. Execution gates reads on these (and records a retry if an object
2065+
// has not caught up); see `TemporaryStore::check_system_object_available`.
2066+
let system_object_versions = execution_env
2067+
.assigned_versions
2068+
.system_object_versions
2069+
.clone();
2070+
let accumulator_version = execution_env.assigned_versions.accumulator_version();
20722071
let execution_params = match early_execution_error {
20732072
None => ExecutionOrEarlyError::ok(accumulator_version),
20742073
Some(errors) => ExecutionOrEarlyError::failed(errors, accumulator_version),
@@ -2082,7 +2081,7 @@ impl AuthorityState {
20822081
&*self.coin_reservation_resolver,
20832082
sender,
20842083
&mut kind,
2085-
execution_env.assigned_versions.accumulator_version,
2084+
execution_env.assigned_versions.accumulator_version(),
20862085
)
20872086
.expect("rewriting must succeed for a certified transaction")
20882087
} else {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ pub async fn submit_to_consensus(
9999
.into_map()
100100
.get(&executable.key())
101101
.cloned()
102-
.unwrap_or_else(|| AssignedVersions::new(vec![], None));
102+
.unwrap_or_default();
103103

104104
Ok((executable, versions))
105105
}
@@ -182,9 +182,9 @@ pub async fn submit_and_execute_with_error(
182182
.into_map()
183183
.get(&executable.key())
184184
.cloned()
185-
.unwrap_or_else(|| AssignedVersions::new(vec![], None))
185+
.unwrap_or_default()
186186
} else {
187-
AssignedVersions::new(vec![], None)
187+
AssignedVersions::default()
188188
};
189189

190190
// State accumulator for validation
@@ -460,7 +460,7 @@ pub async fn assign_versions_and_schedule(
460460
.into_map()
461461
.get(&executable.key())
462462
.cloned()
463-
.unwrap_or_else(|| AssignedVersions::new(vec![], None));
463+
.unwrap_or_default();
464464

465465
let env = ExecutionEnv::new().with_assigned_versions(versions.clone());
466466
authority.execution_scheduler().enqueue_transactions(
@@ -489,5 +489,5 @@ pub async fn assign_shared_object_versions(
489489
.into_map()
490490
.get(&executable.key())
491491
.cloned()
492-
.unwrap_or_else(|| AssignedVersions::new(vec![], None))
492+
.unwrap_or_default()
493493
}

0 commit comments

Comments
 (0)