Skip to content

[vm] Record txn read sets for hot state promotion#19989

Open
wqfish wants to merge 1 commit into
mainfrom
pr19989
Open

[vm] Record txn read sets for hot state promotion#19989
wqfish wants to merge 1 commit into
mainfrom
pr19989

Conversation

@wqfish

@wqfish wqfish commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Hot state promotions (the to_make_hot set in the block epilogue) are
part of consensus, so their inputs must be deterministic. This reworks how
that set is derived.

Why the old derivation was problematic

Promotions were fed from BlockSTM's read/write summary, which exists for
conflict detection, not for this:

  • Path-dependent. The summary differs between parallel and sequential
    execution (e.g. aggregator v1 reads served by delta resolution are
    dropped in parallel but kept in sequential), so the same block could
    promote different keys depending on how it happened to execute — a
    divergence in a consensus-agreed artifact.
  • Coupled to gas config. It was only populated inside the
    conflict_penalty_window branch, so promotions silently depended on an
    unrelated block-gas knob.
  • Incomplete write exclusion. Its write side misses in-place
    delayed-field rewrites, aggregator v1 writes/deltas and module writes, so
    a key the block writes could still be promoted by the epilogue even
    though the write already makes it hot.

Approach

Record the read set at the VM boundary, where it is a pure function of the
transaction and the pre-state — identical across parallel and sequential
execution and independent of gas config:

  • StorageAdapter records resource, resource-group, table-item, aggregator
    v1 and config reads. Respawned sessions share one recorder so all of a
    transaction's sessions accumulate into a single set.
  • ReadRecordingCodeStorage wraps the code storage and records module
    fetches. It sits above the global module cache, so a module is recorded
    whether served from that cache, the per-block cache or storage.
  • Written keys are enumerated directly from the change set, covering every
    write kind the conflict summary missed.

Worth calling out

The new set intentionally differs from the old one — e.g. exists<T> now
loads the resource and counts as a read — so it ships behind the existing
hotness feature flag, and the mixed-version forge suites keep that flag off
to avoid old/new nodes disagreeing on transaction output. Adds a per-block
promotions histogram and tests covering sequential/parallel parity, each
read and write kind, and discard handling. Follow-ups: read-kind/hotness
tagging and an on-chain, byte-based promotion cap.

Performance

The recording runs unconditionally, so its per-transaction cost has to stay
small. The accumulating sets are keyed by StateKey, which already carries a
precomputed 32-byte hash, so they use a fast (FxHash, via the maintained
rustc-hash) hasher instead of the default SipHash. At extraction the read
set is moved out rather than cloned, and module reads are yielded as an
iterator. In the block hot-state accumulator, to_make_hot is a hash set
ordered once when the promotion cap is applied, so a read no longer pays a
comparison that deserializes the key.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com


Note

Medium Risk
Medium risk: changes consensus-relevant block epilogue inputs and VM execution path (always-on read recording), though behavior is behind the hotness feature flag and mixed-version forge tests disable epilogue hotness.

Overview
Hot state promotion (to_make_hot in the block epilogue) no longer derives from BlockSTM’s conflict read/write summary. The VM now records observed reads and the output carries explicit write keys, and the block executor feeds those into BlockHotStateOpAccumulator in commit order.

Read recording: StorageAdapter accumulates StateKeys for resource, resource-group, table, aggregator v1, and config accesses into a shared ReadRecorder (respawned sessions inherit the parent recorder). ReadRecordingCodeStorage wraps code storage and deduplicates module fetches before turning them into module StateKeys at extraction. Per-txn sets are merged into UnorderedReadSet on AptosTransactionOutput; discarded txns get an empty read set.

Write enumeration: BeforeMaterializationOutput adds storage_keys_read / storage_keys_written; writes include resources, modules, aggregator v1 writes/deltas (not just conflict-summary keys). Hot-state accumulation is gated via is_hot_state_accumulation_enabled and accumulate_hot_state_rw instead of piggybacking on conflict_penalty_window fee accumulation.

Accumulator & ops: to_make_hot uses a hash set with ordering applied once when capping promotions; adds HOT_STATE_PROMOTIONS_PER_BLOCK histogram. Forge compat/framework upgrade suites disable HOTNESS_IN_EPILOGUE for mixed-version safety.

Tests: Large e2e hot_state suite (sequential/parallel parity, read/write kinds, discards) plus unit tests for read recording and shared recorders.

Reviewed by Cursor Bugbot for commit 92c60c6. Bugbot is set up for automated code reviews on this repo. Configure here.

@wqfish wqfish changed the title [execution] VM-boundary hot-state access summary [vm] Record txn read sets for hot state promotion Jun 11, 2026
@@ -91,9 +91,6 @@ impl<T: Transaction> BlockGasLimitProcessor<T> {
} else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: this still enables hot-state accumulation under the existing add_block_limit_outcome_onchain gate even though this PR intentionally changes how to_make_hot is derived. Once hotness_in_epilogue is turned on, mixed-version validators will compute different promotion sets for the same block and serialize different BlockEpiloguePayload::V2 contents, so this rollout is not safe without a separate gate for the new derivation.

Comment thread aptos-move/block-executor/src/executor.rs Outdated
Comment thread aptos-move/block-executor/src/txn_last_input_output.rs Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 2 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 2 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

Comment thread aptos-move/aptos-vm/src/block_executor/mod.rs Outdated
Comment thread aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 2 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 4 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

Comment thread aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs Outdated
@wqfish wqfish added CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR CICD:run-framework-upgrade-test labels Jun 11, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 4 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 4 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Aptos Security Bugbot has reviewed your changes, there are still 4 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aptos Security Bugbot has reviewed your changes, there are still 8 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aptos Security Bugbot has reviewed your changes, there are still 8 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aptos Security Bugbot has reviewed your changes, there are still 8 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hot state promotions (the `to_make_hot` set in the block epilogue) are
part of consensus, so their inputs must be deterministic. This reworks how
that set is derived.

## Why the old derivation was problematic

Promotions were fed from BlockSTM's read/write summary, which exists for
conflict detection, not for this:

- **Path-dependent.** The summary differs between parallel and sequential
  execution (e.g. aggregator v1 reads served by delta resolution are
  dropped in parallel but kept in sequential), so the same block could
  promote different keys depending on how it happened to execute — a
  divergence in a consensus-agreed artifact.
- **Coupled to gas config.** It was only populated inside the
  `conflict_penalty_window` branch, so promotions silently depended on an
  unrelated block-gas knob.
- **Incomplete write exclusion.** Its write side misses in-place
  delayed-field rewrites, aggregator v1 writes/deltas and module writes, so
  a key the block writes could still be promoted by the epilogue even
  though the write already makes it hot.

## Approach

Record the read set at the VM boundary, where it is a pure function of the
transaction and the pre-state — identical across parallel and sequential
execution and independent of gas config:

- `StorageAdapter` records resource, resource-group, table-item, aggregator
  v1 and config reads. Respawned sessions share one recorder so all of a
  transaction's sessions accumulate into a single set.
- `ReadRecordingCodeStorage` wraps the code storage and records module
  fetches. It sits above the global module cache, so a module is recorded
  whether served from that cache, the per-block cache or storage.
- Written keys are enumerated directly from the change set, covering every
  write kind the conflict summary missed.

## Worth calling out

The new set intentionally differs from the old one — e.g. `exists<T>` now
loads the resource and counts as a read — so it ships behind the existing
hotness feature flag, and the mixed-version forge suites keep that flag off
to avoid old/new nodes disagreeing on transaction output. Adds a per-block
promotions histogram and tests covering sequential/parallel parity, each
read and write kind, and discard handling. Follow-ups: read-kind/hotness
tagging and an on-chain, byte-based promotion cap.

## Performance

The recording runs unconditionally, so its per-transaction cost has to stay
small. The accumulating sets are keyed by `StateKey`, which already carries a
precomputed 32-byte hash, so they use a fast (FxHash, via the maintained
`rustc-hash`) hasher instead of the default SipHash. At extraction the read
set is moved out rather than cloned, and module reads are yielded as an
iterator. In the block hot-state accumulator, `to_make_hot` is a hash set
ordered once when the promotion cap is applied, so a read no longer pays a
comparison that deserializes the key.


Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aptos Security Bugbot has reviewed your changes, there are still 8 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aptos Security Bugbot has reviewed your changes and found 1 potential issue.

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

Comment on lines +67 to +87
let code_storage = ReadRecordingCodeStorage::new(view);
match self.vm.execute_single_transaction(
txn,
&resolver,
&code_storage,
&log_context,
auxiliary_info,
) {
Ok((vm_status, vm_output)) => {
if vm_output.status().is_discarded() {
// Discarded transactions commit no state changes, so their reads must not feed
// hot-state promotion. Only carry the read set for outputs that can commit.
let read_set = if vm_output.status().is_discarded() {
speculative_trace!(
&log_context,
format!("Transaction discarded, status: {:?}", vm_status),
);
}
UnorderedReadSet::default()
} else {
let mut keys = resolver.take_recorded_reads();
keys.extend(code_storage.into_recorded_reads());
UnorderedReadSet::new(keys)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: staged init_module execution can still bypass the new module-read recorder.

This wrapper now builds the carried read set from resolver.take_recorded_reads() plus the module keys seen through this outer ReadRecordingCodeStorage. Module-publish transactions later switch to a bare StagingModuleStorage for staged verification and init_module execution (move_vm_ext/session/user_transaction_sessions/user.rs), so any existing dependency module first loaded from that staged storage never reaches either recorder. A publisher can therefore drive init_module through specific dependency modules while keeping them out of to_make_hot, weakening the hot-state signal this feature is trying to persist.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

✅ Forge suite compat success on dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862 ==> 92c60c65d21671c5e941b53b1ec72fedd02ce1d3

Compatibility test results for dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862 ==> 92c60c65d21671c5e941b53b1ec72fedd02ce1d3 (PR)
1. Check liveness of validators at old version: dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862
compatibility::simple-validator-upgrade::liveness-check : committed: 15119.34 txn/s, latency: 2260.44 ms, (p50: 2100 ms, p70: 2300, p90: 3300 ms, p99: 5200 ms), latency samples: 495360
2. Upgrading first Validator to new version: 92c60c65d21671c5e941b53b1ec72fedd02ce1d3
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6241.30 txn/s, latency: 5375.03 ms, (p50: 5900 ms, p70: 6100, p90: 6200 ms, p99: 6500 ms), latency samples: 216820
3. Upgrading rest of first batch to new version: 92c60c65d21671c5e941b53b1ec72fedd02ce1d3
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6018.91 txn/s, latency: 5619.37 ms, (p50: 6200 ms, p70: 6300, p90: 6400 ms, p99: 6500 ms), latency samples: 210180
4. upgrading second batch to new version: 92c60c65d21671c5e941b53b1ec72fedd02ce1d3
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 9747.78 txn/s, latency: 3432.28 ms, (p50: 3700 ms, p70: 3800, p90: 4000 ms, p99: 4300 ms), latency samples: 316560
5. check swarm health
Compatibility test for dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862 ==> 92c60c65d21671c5e941b53b1ec72fedd02ce1d3 passed
Test Ok

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

✅ Forge suite realistic_env_max_load success on 92c60c65d21671c5e941b53b1ec72fedd02ce1d3

two traffics test: inner traffic : committed: 15605.47 txn/s, latency: 1088.85 ms, (p50: 1000 ms, p70: 1100, p90: 1300 ms, p99: 1600 ms), latency samples: 5828040
two traffics test : committed: 100.02 txn/s, latency: 872.21 ms, (p50: 800 ms, p70: 900, p90: 1000 ms, p99: 1700 ms), latency samples: 1620
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 0.301, avg: 0.252", "ConsensusProposalToOrdered: max: 0.114, avg: 0.107", "ConsensusOrderedToCommit: max: 0.153, avg: 0.140", "ConsensusProposalToCommit: max: 0.259, avg: 0.248"]
Max non-epoch-change gap was: 1 rounds at version 47199 (avg 0.00) [limit 4], 0.75s no progress at version 47199 (avg 0.06s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.41s no progress at version 3193126 (avg 0.41s) [limit 16].
Test Ok

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

✅ Forge suite framework_upgrade success on dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862 ==> 92c60c65d21671c5e941b53b1ec72fedd02ce1d3

Compatibility test results for dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862 ==> 92c60c65d21671c5e941b53b1ec72fedd02ce1d3 (PR)
Upgrade the nodes to version: 92c60c65d21671c5e941b53b1ec72fedd02ce1d3
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2379.82 txn/s, submitted: 2385.85 txn/s, failed submission: 6.03 txn/s, expired: 6.03 txn/s, latency: 1216.03 ms, (p50: 1200 ms, p70: 1200, p90: 1700 ms, p99: 2400 ms), latency samples: 213201
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2231.09 txn/s, submitted: 2239.28 txn/s, failed submission: 8.19 txn/s, expired: 8.19 txn/s, latency: 1350.18 ms, (p50: 1100 ms, p70: 1200, p90: 2100 ms, p99: 10600 ms), latency samples: 196080
5. check swarm health
Compatibility test for dfffef35ad9ab37b8eb9d9cc94d8f97911c4d862 ==> 92c60c65d21671c5e941b53b1ec72fedd02ce1d3 passed
Upgrade the remaining nodes to version: 92c60c65d21671c5e941b53b1ec72fedd02ce1d3
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2305.63 txn/s, submitted: 2311.85 txn/s, failed submission: 6.22 txn/s, expired: 6.22 txn/s, latency: 1240.42 ms, (p50: 1200 ms, p70: 1200, p90: 1800 ms, p99: 2600 ms), latency samples: 207720
Test Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR CICD:run-framework-upgrade-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants