Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

port: from reth-trie-common to alloy-trie #53

Closed
wants to merge 1 commit into from

Conversation

c0np4nn4
Copy link
Contributor

@c0np4nn4 c0np4nn4 commented Oct 9, 2024

Description

This PR moves all the code from reth/crates/trie/common/ (not just the trie root logic) to alloy-trie, as discussed in issue #11211. The idea is to have all the trie-related functionality in one place within the alloy-trie crate.

The project compiles smoothly with the reth crate added in Cargo.toml, and I haven't run into any circular dependency issues. That said, I want to ensure this aligns with the direction you're heading.

Question

Does this approach fit with what you have in mind? Also, is the plan for reth to start using all this functionality from alloy-trie moving forward?

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this pr should only focus on the two referenced functions

Comment on lines +32 to +34
alloy-consensus = { version = "0.3.6", default-features = false }
alloy-genesis = { version = "0.3.6", default-features = false }

Copy link
Member

Choose a reason for hiding this comment

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

we can't use any of this because this introduces cyclical deps

@rkrasiuk
Copy link
Member

we can introduce ordered_root_* fns initially specified in the issue description, but we shouldn't move any account, storage or state root related stuff, since this crate is a general MPT implementation. feel free to reopen PR introducing the aforementioned methods

@rkrasiuk rkrasiuk closed this Oct 14, 2024
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