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

Do a pass on mpt_trie #401

Open
6 tasks
Tracked by #270
0xaatif opened this issue Jul 16, 2024 · 4 comments
Open
6 tasks
Tracked by #270

Do a pass on mpt_trie #401

0xaatif opened this issue Jul 16, 2024 · 4 comments
Labels
crate: mpt_trie Anything related to the mpt_trie crate.

Comments

@0xaatif
Copy link
Contributor

0xaatif commented Jul 16, 2024

@github-project-automation github-project-automation bot moved this to Backlog in Zero EVM Jul 16, 2024
@0xaatif 0xaatif changed the title Remove mpt_trie Refactor MPT trie Jul 16, 2024
@0xaatif 0xaatif changed the title Refactor MPT trie Factor out MPT trie Jul 16, 2024
@Nashtare
Copy link
Collaborator

Remove the stats and debug modules - I don't think we use them

This is not accurate. We heavily use them when debugging trie discrepancies yielding KernelPanic on the prover side.
We leverage the tools defined here that rely on these modules.

@Nashtare Nashtare added this to the System strengthening milestone Jul 16, 2024
@BGluth
Copy link
Contributor

BGluth commented Jul 16, 2024

I'm skeptical of the Arc<Node>s everywhere - I'd prefer they were boxed, but we get a perf penalty in constructing dummy tries for our tests (I don't think this is a good reason)

I'm trying to remember the exact reason why I ended up adding the Arc<_>s. It's obviously not a race condition within the library, but I remember convincing myself at the time that it was needed. You might be able to replace it with an Rc, but we really want to make sure that we keep that CoW setup for performance reasons.

Use alloy-trie's logic for calculating hashes efficiently.

Yeah give it a look, but two things:

  • Make sure their logic works with our Hash node types.
  • Check that it lazily caches & calculates node hashes (very likely does).

@BGluth
Copy link
Contributor

BGluth commented Jul 16, 2024

Also just to clarify from the issue title, are you thinking about removing mpt_trie entirely?

@0xaatif
Copy link
Contributor Author

0xaatif commented Jul 16, 2024

Also just to clarify from the issue title, are you thinking about removing mpt_trie entirely?

No, I think our trie library offers something unique that other libraries don't :) I just want to make it simpler if possible, c.f #400

@Nashtare Nashtare added the crate: mpt_trie Anything related to the mpt_trie crate. label Jul 17, 2024
@0xaatif 0xaatif changed the title Factor out MPT trie Do a pass on mpt_trie Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: mpt_trie Anything related to the mpt_trie crate.
Projects
Status: Backlog
Development

No branches or pull requests

3 participants