Skip to content

Conversation

@BrianBland
Copy link
Collaborator

This updates the overlay root calculation code to compute partial roots for a given trie using an internal thread pool, sharded by first nibble, and then combines these together into a single root.

Uses a fork of alloy-trie with the ability to compute partial roots from a HashBuilder and compute the merged root from an array of HashBuilders.

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Aug 20, 2025

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@and-cb and-cb marked this pull request as ready for review November 13, 2025 17:03
Comment on lines +121 to +127
// SAFETY: OverlayState is thread-safe because:
// - Arc<[...]> provides thread-safe reference counting
// - All fields are immutable after construction
// - No interior mutability is used
// - All operations are read-only or create new instances
unsafe impl Send for OverlayState {}
unsafe impl Sync for OverlayState {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason we can't derive Send+Sync instead of manual impl?

Comment on lines +159 to +160
// Ignore send errors - the channel may be closed during shutdown
sender.send(work_item).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ok if this panics here (from calling unwrap on a send error, even if we want to ignore the send error)?

Comment on lines +102 to +104
let end_idx = if i == thread_count {
// Last thread handles any remaining addresses
addresses.len()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be i == thread_count - 1? Since i is zero-indexed, wouldn't it always miss this case?

@kmchicoine
Copy link
Contributor

General question: what happens if one of the hash builder threads panics? I think add_leaf and add_branch can both panic, but not sure if that's something we need to worry about now

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.

4 participants