0xPolygonZero / zk_evm

Apache License 2.0
85 stars 37 forks source link

Do a pass on `mpt_trie` #401

Open 0xaatif opened 4 months ago

0xaatif commented 4 months ago
Nashtare commented 4 months ago

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.

BGluth commented 4 months ago

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:

BGluth commented 4 months ago

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

0xaatif commented 4 months ago

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