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

all: port boring changes from pbss #27176

Merged
merged 7 commits into from
May 9, 2023

Conversation

rjl493456442
Copy link
Member

This PR introduces a wrapper for trie database, so that we can have hash-based and path-based storage scheme respectively implemented in their own package and called by this wrapper.

Additionally, it modifies the unit tests for supporting path-based scheme as well later on.

core/rawdb/schema.go Outdated Show resolved Hide resolved
core/rawdb/schema.go Outdated Show resolved Hide resolved
core/state/iterator_test.go Outdated Show resolved Hide resolved
trie/database_test.go Show resolved Hide resolved
trie/trienode/node.go Outdated Show resolved Hide resolved
@rjl493456442
Copy link
Member Author

rjl493456442 commented Apr 28, 2023

Btw, @holiman @karalabe @fjl I do need the insights from you guys about the package name.

triedb
├── hashdb
│
└── snap

Right now I create a wrapper of triedb and place the original database in hashdb package and pbss stuff in snap directory. These name feels not very good :). Suggestion is appreciated!

@holiman
Copy link
Contributor

holiman commented May 2, 2023

Right now I create a wrapper of triedb and place the original database in hashdb package and pbss stuff in snap directory. These name feels not very good :). Suggestion is appreciated!

How about path, pathbased or pbss instead of snap?

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Big change, can't give any guarantees but it looks good to me

@rjl493456442
Copy link
Member Author

rjl493456442 commented May 4, 2023

We can discuss the package name on triage,

  • pbss and hbss
  • hashdb and pathdb
  • etc

EDIT; hashdb and pathdb are preferred.

Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

SGTM, tried to follow as much of the logic as I could, but some refactors touch too many files for me to properly review it.
Have you run a sync with it?

trie/database_wrap.go Outdated Show resolved Hide resolved
trie/database_wrap.go Outdated Show resolved Hide resolved
Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

Kind of SGTM. Would be nice to merge and put in on a benchmarker for good measure

@karalabe karalabe merged commit 5021d36 into ethereum:master May 9, 2023
@karalabe karalabe added this to the 1.11.7 milestone May 9, 2023
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
* all: port boring changes from pbss

* core, trie: address comments from martin

* trie: minor fixes

* core/rawdb: update comment

* core, eth, tests, trie: address comments

* tests, trie: add extra check when update trie database

* trie/triedb/hashdb: degrade the error to warning
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
* all: port boring changes from pbss

* core, trie: address comments from martin

* trie: minor fixes

* core/rawdb: update comment

* core, eth, tests, trie: address comments

* tests, trie: add extra check when update trie database

* trie/triedb/hashdb: degrade the error to warning
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants