Skip to content

blockifier,apollo_batcher: trim OS initial reads to the accessed-key set#14595

Open
yoavGrs wants to merge 1 commit into
yoav/os-initial-reads-3-flow-testsfrom
yoav/thread-os-initial-reads-to-cende
Open

blockifier,apollo_batcher: trim OS initial reads to the accessed-key set#14595
yoavGrs wants to merge 1 commit into
yoav/os-initial-reads-3-flow-testsfrom
yoav/thread-os-initial-reads-to-cende

Conversation

@yoavGrs

@yoavGrs yoavGrs commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Drop initial-reads entries whose keys are not in AccessedKeys (e.g. storage cells read only by a reverted transaction). The OS replays from the read values of the keys it accesses and needs nothing beyond that set, so the extra entries would only bloat the cende blob. Validated by the OS flow tests (113 passing), including all reverted-tx cases.

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

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

yoavGrs commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@yoavGrs yoavGrs marked this pull request as ready for review June 22, 2026 11:57
@cursor

cursor Bot commented Jun 22, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Wrong trimming could break OS block replay; scope is limited to os_input paths and is covered by OS flow tests including reverted transactions.

Overview
Shrinks OS initial_reads so only pre-block values for keys in AccessedKeys are kept—dropping reads that never matter for OS replay (e.g. storage touched only by reverted transactions).

Adds StateMaps::trim_to_accessed_keys in blockifier, filters storage, nonces, class hashes, and compiled class hashes against the accessed-key sets. With os_input, the batcher runs this in decision_reached after write_block_accessed_keys and passes the trimmed map in CentralObjects instead of the full execution cache. OS flow tests apply the same trim before building block input so tests match production.

A small comment in apollo_committer documents the os_input vs non-os_input committer request surface; no behavior change there.

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

@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from baca9cc to c0904c6 Compare June 22, 2026 14:02
@yoavGrs yoavGrs force-pushed the yoav/os-initial-reads-3-flow-tests branch from f4a9965 to 94b61d9 Compare June 22, 2026 14:02
@yoavGrs yoavGrs requested a review from itamar-starkware June 22, 2026 14:41
@yoavGrs yoavGrs self-assigned this Jun 22, 2026
@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from c0904c6 to 28dc2c0 Compare June 23, 2026 07:21
@yoavGrs yoavGrs force-pushed the yoav/os-initial-reads-3-flow-tests branch from 94b61d9 to 4cc1677 Compare June 23, 2026 07:21
@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from 28dc2c0 to 840c728 Compare June 23, 2026 08:56
@yoavGrs yoavGrs force-pushed the yoav/os-initial-reads-3-flow-tests branch from 4cc1677 to 5410333 Compare June 23, 2026 08:56
@itamar-starkware

itamar-starkware commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Suggestion from Claude Code:

Could we add direct unit tests in cached_state_test.rs?

  • trim_to_accessed_keys: build a StateMaps with entries across all four maps plus extras absent from accessed_keys; assert the extras are dropped and the hits retained. Cover empty accessed_keys → all empty, and pin the declared_contracts pass-through (it's not trimmed — intentional only because get_os_initial_reads pre-clears it; worth asserting so a future pub caller doesn't get surprised). The test_state_maps helper (~line 700) has the construction pattern.
  • get_os_initial_reads: assert the three back-fills (class-hash + nonce per accessed contract, compiled-class-hash per declared class) land, and that declared_contracts comes back cleared.

@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from 840c728 to 41b34a9 Compare June 23, 2026 10:41
@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from 41b34a9 to f334583 Compare June 23, 2026 13:31
@yoavGrs yoavGrs force-pushed the yoav/os-initial-reads-3-flow-tests branch from 5410333 to 9d4eabf Compare June 23, 2026 13:31
@yoavGrs yoavGrs force-pushed the yoav/os-initial-reads-3-flow-tests branch from 9d4eabf to d57b5a2 Compare June 23, 2026 14:42
@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from f334583 to c7678fc Compare June 23, 2026 14:42
Drop initial-reads entries whose keys are not in AccessedKeys (e.g. storage cells read only by a reverted transaction). The OS replays from the read values of the keys it accesses and needs nothing beyond that set, so the extra entries would only bloat the cende blob. Validated by the OS flow tests (113 passing), including all reverted-tx cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@yoavGrs yoavGrs force-pushed the yoav/os-initial-reads-3-flow-tests branch from d57b5a2 to 9863839 Compare June 23, 2026 18:32
@yoavGrs yoavGrs force-pushed the yoav/thread-os-initial-reads-to-cende branch from c7678fc to d865f09 Compare June 23, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants