Skip to content

feat: integrate decided blocks into storage adapter#3282

Merged
CHr15F0x merged 9 commits intomainfrom
chris/decided-into-storage-adapter
Apr 13, 2026
Merged

feat: integrate decided blocks into storage adapter#3282
CHr15F0x merged 9 commits intomainfrom
chris/decided-into-storage-adapter

Conversation

@CHr15F0x
Copy link
Copy Markdown
Contributor

Fixes: #3248

@CHr15F0x CHr15F0x force-pushed the chris/decided-into-storage-adapter branch 2 times, most recently from c92ce6f to 0eb4587 Compare March 23, 2026 11:01
@CHr15F0x CHr15F0x force-pushed the chris/decided-into-storage-adapter branch from 0eb4587 to 715ee5f Compare April 2, 2026 09:01
@CHr15F0x CHr15F0x force-pushed the chris/decided-into-storage-adapter branch from ce9d67a to 8af1c4e Compare April 3, 2026 22:28
@CHr15F0x CHr15F0x changed the base branch from main to chris/rm-dead-code April 3, 2026 22:29
@CHr15F0x CHr15F0x changed the title [WIP] feat: integrate decided blocks into storage adapter feat: integrate decided blocks into storage adapter Apr 3, 2026
@CHr15F0x CHr15F0x marked this pull request as ready for review April 3, 2026 22:29
@CHr15F0x CHr15F0x requested a review from a team as a code owner April 3, 2026 22:29
Base automatically changed from chris/rm-dead-code to main April 7, 2026 06:44
Copy link
Copy Markdown
Contributor

@zvolin zvolin left a comment

Choose a reason for hiding this comment

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

a first pass, will get back to this

Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
@CHr15F0x CHr15F0x marked this pull request as draft April 8, 2026 09:54
@CHr15F0x CHr15F0x force-pushed the chris/decided-into-storage-adapter branch 2 times, most recently from 54420db to 37d0617 Compare April 8, 2026 21:16
@CHr15F0x CHr15F0x force-pushed the chris/decided-into-storage-adapter branch from 9555e33 to f35d970 Compare April 9, 2026 16:59
@CHr15F0x CHr15F0x marked this pull request as ready for review April 9, 2026 17:05
@CHr15F0x CHr15F0x requested a review from a team as a code owner April 9, 2026 17:05
@CHr15F0x
Copy link
Copy Markdown
Contributor Author

CHr15F0x commented Apr 9, 2026

BTW: serialized casm definition as well as serialized sierra definition should be newtyped so that we don't pass around naked Vec<u8> in both cases. But this will be a different PR.

Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
Copy link
Copy Markdown
Contributor

@zvolin zvolin left a comment

Choose a reason for hiding this comment

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

in general looks good, left some idea, curious what you think.

but maan, I must say those decided:: helpers are veeryy hard to understand, my brain unfolds when I read them 😅 almost a rust riddles

I was thinking if we could make them easier to digest, and in general there are two patterns.

First the match on blockid, each function basically has the same code in latest and number arms, maybe we could just prepare the correct iterator beforehand?

fn blocks_for_id(decided_blocks, id) -> impl Iterator<...> {
    // or just match and Boxes
    blocks.iter().rev().filter(move |(bn, _)| match block_id {
        BlockId::Number(n) => **bn <= n,
        BlockId::Latest => true,
        // decided blocks have no hash
        BlockId::Hash(_) => false,
    })
    // maybe also map out the number at this point?
}

doesn't look too good, but should remove half of the code :p

The other pattern is searchig for classes / contract updates. Another helpers for that could maybe also make it more readable?

fn find_class(block, class_hash) -> Option<&DeclaredClass> {
    block
        .declared_classes
        .iter()
        .find(|c| c.sierra_hash == SierraHash(class_hash.0))
}
  • something similar for state updates, should get rid of one round of find_map + then_some

Comment thread crates/pathfinder/src/validator.rs Outdated
Comment thread crates/common/src/l2.rs
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs
Comment thread crates/executor/src/state_reader/storage_adapter/concurrent.rs Outdated
Copy link
Copy Markdown
Contributor

@zvolin zvolin left a comment

Choose a reason for hiding this comment

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

🚢

@CHr15F0x CHr15F0x merged commit 40b7d6e into main Apr 13, 2026
10 checks passed
@CHr15F0x CHr15F0x deleted the chris/decided-into-storage-adapter branch April 13, 2026 10:29
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.

execution in on_finalized_block_decided sometimes causes commitment mismatch

3 participants