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

Trie-Refactor #2785

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Trie-Refactor #2785

wants to merge 34 commits into from

Conversation

ScottyPoi
Copy link
Contributor

Continuing from #2662

@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Merging #2785 (133cdcb) into master (a37f51f) will decrease coverage by 5.37%.
The diff coverage is 64.13%.

❗ Current head 133cdcb differs from pull request most recent head 69a0733. Consider uploading reports for the commit 69a0733 to get more accurate results

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 89.56% <ø> (ø)
blockchain 92.85% <ø> (ø)
client ?
common 97.09% <ø> (-0.01%) ⬇️
devp2p 86.58% <ø> (ø)
ethash ∅ <ø> (∅)
evm 66.60% <ø> (-0.03%) ⬇️
statemanager 77.86% <ø> (-8.49%) ⬇️
trie 65.90% <64.13%> (-24.17%) ⬇️
tx 95.87% <ø> (ø)
util 85.31% <ø> (ø)
vm 76.52% <ø> (-0.63%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ScottyPoi ScottyPoi force-pushed the Trie-Refactor branch 2 times, most recently from 1d3397a to 4690352 Compare June 20, 2023 00:46
@ScottyPoi
Copy link
Contributor Author

"How much work is left" has seemed like 1% for a few weeks now...

Besides improving the library in general, I went into this with the intention of changing some fundamental things about the way our Trie was structured that prevented me from using it in the way that I envisioned for a portal state-network clients.

The basic implementation has always worked. Passed all of the ethereum-tests for MPT, as well as all of the basic functionality tests that I could throw at it.

The lightweight (database) version works as well, and can do certain things much faster than the trie currently in use.

Implementing the interface (the thing to be exported as Trie for other packages) was also fairly straightforward at first, and most affected packages accepted the new Trie without any problem.

But for weeks now, I have been stuck in a loop of discovering over and over again new places where the statemanager or vm was designed specifically to anticipate some of the behaviors that I designed away from. Ultimately these contradictions may not be resolvable without also refactoring the other packages to use the trie a little differently.

  1. Clearly this has taken too long
  2. There may be no interest / need for these changes by anyone but @ScottyPoi
  3. At the Portal-Network summit, we concluded that it makes more sense to pursue a verkle based state_network from the start, so even my interest / need for fancy sparse MPT's may be gone.

I still think there are some positive changes that we could keep that aren't related to any of that. But, like @holgerd77 said, these could be smaller singular-focus PR's.

This was referenced Aug 15, 2023
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