Skip to content

feat: add seismic-alloy-trie minimal crate (part of no-trie-fork)#80

Draft
samlaf wants to merge 4 commits intoseismicfrom
feat--no-trie-fork
Draft

feat: add seismic-alloy-trie minimal crate (part of no-trie-fork)#80
samlaf wants to merge 4 commits intoseismicfrom
feat--no-trie-fork

Conversation

@samlaf
Copy link
Copy Markdown
Contributor

@samlaf samlaf commented Mar 5, 2026

DO NOT MERGE

We probably will want to only merge this after the audit is done, unless we decide we want to pull the trigger and want it audited.

This adds a new minimal seismic-alloy-trie wrapper crate around alloy-trie. alloy-trie has functions to compute the storage state root, but they take into<U256> as arguments. We want to be passing FlaggedStorage, so instead of forking alloy-trie like we previously did, I'm adding equivalent functions here that we should use in reth (and foundry?) instead.

See design in https://hackmd.io/ZOGfcUP2SyKmPeOuz0tmvg

Cargo.toml Outdated
Comment on lines +148 to +149
# TODO(samlaf): pointing to rev from https://github.com/SeismicSystems/seismic-alloy-core/pull/58
# Before merging this PR, merge the one from seismic-alloy-core and update the rev here.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +20 to +23
pub fn storage_root_unhashed<T: Into<FlaggedStorage>>(
storage: impl IntoIterator<Item = (B256, T)>,
) -> B256 {
storage_root_unsorted(storage.into_iter().map(|(slot, value)| (keccak256(slot), value)))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Still not 100% sure what's the best / less error-prone design here. The issue with having this is that reth could forget in some place to use this one and instead use the stock alloy-trie's storage_root_unashed, passing it an unshielded FlaggedStorage by mistake.

What we could do is to re-export all of alloy-trie (except for these functions; thereby forcing downstream consumers to use these Into variants) as part of this crate (similar to how the alloy monorepo re-exports alloy-core).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Claude analysis:

That's a reasonable approach. But consider an even simpler option first: can you just shadow the function name?                                          
                  
  If seismic-alloy-trie re-exports a storage_root() that takes FlaggedStorage, and seismic-reth depends on seismic-alloy-trie (which it already does), then
   anyone doing use seismic_alloy_trie::storage_root gets the correct version. The upstream alloy_trie::storage_root still exists but nobody should be
  importing from alloy_trie::root directly — and you can grep for that.                                                                                    
                  
  But if you want a compile-time guarantee, your fork approach works. The minimal change would be:

  1. Keep seismic-trie as a thin fork of alloy-trie
  2. Remove storage_root(), storage_root_unsorted(), storage_root_unhashed() from the ethereum feature
  3. Keep TrieAccount, state_root_*(), ordered_trie_root*() — those are fine and widely used
  4. The seismic-alloy-trie crate provides the replacement storage_root() with FlaggedStorage

  That way, any code that accidentally calls alloy_trie::storage_root() gets a compile error. The fork diff is tiny — just deleting 3 functions — and easy
  to maintain across upstream updates.

  You don't even need a separate seismic feature flag for this. Just delete those 3 functions from the ethereum module. They're convenience helpers, and
  the only correct version now lives in seismic-alloy-trie.

Base automatically changed from veridise-audit-feb-2026 to seismic March 27, 2026 14:34
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.

2 participants