blockifier: add predicted_alias_storage_entries#14102
Conversation
PR SummaryLow Risk Overview Refactors the compression decision logic into shared Reviewed by Cursor Bugbot for commit b5f856e. Bugbot is set up for automated code reviews on this repo. Configure here. |
ArielElp
left a comment
There was a problem hiding this comment.
@ArielElp reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on yoavGrs).
b002b93 to
851e1d7
Compare
a0228f5 to
066d324
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on yoavGrs).
crates/blockifier/src/state/stateful_compression.rs line 85 at r1 (raw file):
/// read (and may write) for the given `state_diff`, without running the allocation. Mirrors the /// access pattern of `AliasUpdater::new` (counter read) plus each `AliasUpdater::insert_alias` /// call (one read per qualifying key).
kind of annoying that you need this function. can you see if you can reuse / generalize allocate_aliases_in_storage for this?
in your test it seems that they seem to have the same result...
I would go on to say the allocate_aliases_in_storage should be an associated function of AliasUpdater, and the new predicted_alias_storage_entries should also be an associated function of AliasUpdater.
(it bugs me because there are duplicated ifs and scanning logic, I prefer these to be in one place..)
Code quote:
/// Predicts the storage entries on the alias contract that `allocate_aliases_in_storage` will
/// read (and may write) for the given `state_diff`, without running the allocation. Mirrors the
/// access pattern of `AliasUpdater::new` (counter read) plus each `AliasUpdater::insert_alias`
/// call (one read per qualifying key).Predicts the alias-contract storage entries that allocate_aliases_in_storage would touch for a given state_diff, without running the allocation. Used by the batcher to assemble the accessed-keys set written for OS replay. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
851e1d7 to
b5f856e
Compare
066d324 to
1bec947
Compare
yoavGrs
left a comment
There was a problem hiding this comment.
@yoavGrs made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware).
crates/blockifier/src/state/stateful_compression.rs line 85 at r1 (raw file):
Previously, dorimedini-starkware wrote…
kind of annoying that you need this function. can you see if you can reuse / generalize
allocate_aliases_in_storagefor this?
in your test it seems that they seem to have the same result...I would go on to say the
allocate_aliases_in_storageshould be an associated function ofAliasUpdater, and the newpredicted_alias_storage_entriesshould also be an associated function ofAliasUpdater.(it bugs me because there are duplicated
ifs and scanning logic, I prefer these to be in one place..)
Done.
|
Artifacts upload workflows: |
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on yoavGrs).

Predicts the alias-contract storage entries that allocate_aliases_in_storage
would touch for a given state_diff, without running the allocation. Used by
the batcher to assemble the accessed-keys set written for OS replay.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com