forked from ethereum/go-ethereum
-
Notifications
You must be signed in to change notification settings - Fork 13
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
fix: import cycle in conversion refactor #315
Merged
gballet
merged 1 commit into
refactor-transition-post-genesis
from
fix-import-cycle-in-conversion-refactor
Nov 30, 2023
Merged
fix: import cycle in conversion refactor #315
gballet
merged 1 commit into
refactor-transition-post-genesis
from
fix-import-cycle-in-conversion-refactor
Nov 30, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gballet
commented
Nov 30, 2023
Comment on lines
+428
to
437
triedb := state.NewDatabaseWithConfig(db, nil) | ||
triedb.StartVerkleTransition(common.Hash{}, common.Hash{}, config, config.PragueTime, common.Hash{}) | ||
triedb.EndVerkleTransition() | ||
statedb, err := state.New(parent.Root(), triedb, snaps) | ||
if err != nil { | ||
panic(fmt.Sprintf("could not find state for block %d: err=%v, parent root=%x", parent.NumberU64(), err, parent.Root())) | ||
} | ||
statedb.Database().SaveTransitionState(parent.Root()) | ||
for i := 0; i < n; i++ { | ||
triedb := state.NewDatabaseWithConfig(db, nil) | ||
triedb.EndVerkleTransition() | ||
statedb, err := state.New(parent.Root(), triedb, snaps) | ||
if err != nil { | ||
panic(fmt.Sprintf("could not find state for block %d: err=%v, parent root=%x", i, err, parent.Root())) | ||
} | ||
block, receipt := genblock(i, parent, statedb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moving the database initialization outside of the loop. Otherwise, it would start from a fresh database, try to load a missing conversion state (since it would only be stored in a previous version of the database), deduct that the conversion hadn't started, and crash.
gballet
added a commit
that referenced
this pull request
Nov 30, 2023
gballet
added a commit
that referenced
this pull request
Feb 8, 2024
gballet
added a commit
that referenced
this pull request
Feb 28, 2024
gballet
added a commit
that referenced
this pull request
Mar 26, 2024
gballet
added a commit
that referenced
this pull request
Apr 15, 2024
gballet
added a commit
that referenced
this pull request
Apr 29, 2024
quell linter issue support Transition post tree in conversion Refactor transition post genesis (#311) * rewrite per-block conversion pointer management * remove unused method * fix: a branch that can verge at genesis or post genesis (#314) * fix: import cycle in conversion refactor (#315) * fix shadowfork panic in OpenStorageTrie fix OpenStorageTrie: return an error instead of panicking if the account tree not a verkle tree (#321) add a switch to force proof in blocks (#322) * add a switch to force proof in blocks * activate switch * fix switch type add switch to override the stride of the overlay conversion (#323) * add switch to override the stride of the overlay conversion * set a default stride of 10k add a few traces for relaunch add missing step in the chain of setting the override for overlay stride fix: save and load transition state for block processing (#324) * fix: save and load transition state for block processing * log whether the tree is verkle in LoadTransitionState * fix: ensure the transition is marked as started in insertChain * dump saved address * fix nil pointer panic * remove stacktrace that is no longer useful * fix a panic * fix build * check: copy current account address BEFORE it's saved * mandatory panic fix * Remove debug fmt.Println * more cleanup + comments fix: ensure StorageProcessed is properly recovered add traces for gas pool overflow cmd/{geth, utils}: add a command to clear verkle costs (#326) * cmd/{geth, utils}: add a command to clear verkle costs fix: boolean issue fix: load finalization state in FinalizeAndAssemble (#340) * Conversion and TransitionTrie fixes (#346) * fixes Signed-off-by: Ignacio Hagopian <[email protected]> * remove old comment Signed-off-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> * trace cleanup --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> fix: check for nil override in NewBlockChain (#357) fix rebase issue fix a few rebase issues add debug traces fix: assume that having to create a new transaction state mean it hasn't happened. This is a workaround, because it will always be false when restarting a node that has started the transition but not necessarily completed it. add logs to debug shadowfork persist conversion state to db and use an LRU cache for active transition states (#375) * persist conversion state to db * fix: don't recreate LRU when writing state * opt: only write state to db if not already present in LRU * fix: rlp can't encode TransitionState * fix: use gob because binary.Write does not support structs 🤦♂️ * fix: nil pointer panic * add logs to debug shadowfork * no such thing as not enough traces * ditto * fix stupid bug * add a comment for readability * add more traces * Lock the state transition during conversion (#384) * heavy handed approach: lock the state transition during conversion * also lock transition state loading/unloading * reduce logs verbosity * add conversion test to workflow (#386) * add conversion test to workflow * mandatory -f switch fix in rm * forgot & at the end of the geth command * remove incorrect kill * add debug traces * add an overlay stride * fix typo * Apply suggestions from code review eth: add debug_conversionStatus RPC call (#392) * eth: add debug_conversionStatus RPC call * add debug trace4s * Apply suggestions from code review * export started/ended fields fix post-verge sync (#404) * fix post-verge sync * review: fix truncated comment
gballet
added a commit
that referenced
this pull request
Apr 29, 2024
quell linter issue support Transition post tree in conversion Refactor transition post genesis (#311) * rewrite per-block conversion pointer management * remove unused method * fix: a branch that can verge at genesis or post genesis (#314) * fix: import cycle in conversion refactor (#315) * fix shadowfork panic in OpenStorageTrie fix OpenStorageTrie: return an error instead of panicking if the account tree not a verkle tree (#321) add a switch to force proof in blocks (#322) * add a switch to force proof in blocks * activate switch * fix switch type add switch to override the stride of the overlay conversion (#323) * add switch to override the stride of the overlay conversion * set a default stride of 10k add a few traces for relaunch add missing step in the chain of setting the override for overlay stride fix: save and load transition state for block processing (#324) * fix: save and load transition state for block processing * log whether the tree is verkle in LoadTransitionState * fix: ensure the transition is marked as started in insertChain * dump saved address * fix nil pointer panic * remove stacktrace that is no longer useful * fix a panic * fix build * check: copy current account address BEFORE it's saved * mandatory panic fix * Remove debug fmt.Println * more cleanup + comments fix: ensure StorageProcessed is properly recovered add traces for gas pool overflow cmd/{geth, utils}: add a command to clear verkle costs (#326) * cmd/{geth, utils}: add a command to clear verkle costs fix: boolean issue fix: load finalization state in FinalizeAndAssemble (#340) * Conversion and TransitionTrie fixes (#346) * fixes Signed-off-by: Ignacio Hagopian <[email protected]> * remove old comment Signed-off-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> * trace cleanup --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> fix: check for nil override in NewBlockChain (#357) fix rebase issue fix a few rebase issues add debug traces fix: assume that having to create a new transaction state mean it hasn't happened. This is a workaround, because it will always be false when restarting a node that has started the transition but not necessarily completed it. add logs to debug shadowfork persist conversion state to db and use an LRU cache for active transition states (#375) * persist conversion state to db * fix: don't recreate LRU when writing state * opt: only write state to db if not already present in LRU * fix: rlp can't encode TransitionState * fix: use gob because binary.Write does not support structs 🤦♂️ * fix: nil pointer panic * add logs to debug shadowfork * no such thing as not enough traces * ditto * fix stupid bug * add a comment for readability * add more traces * Lock the state transition during conversion (#384) * heavy handed approach: lock the state transition during conversion * also lock transition state loading/unloading * reduce logs verbosity * add conversion test to workflow (#386) * add conversion test to workflow * mandatory -f switch fix in rm * forgot & at the end of the geth command * remove incorrect kill * add debug traces * add an overlay stride * fix typo * Apply suggestions from code review eth: add debug_conversionStatus RPC call (#392) * eth: add debug_conversionStatus RPC call * add debug trace4s * Apply suggestions from code review * export started/ended fields fix post-verge sync (#404) * fix post-verge sync * review: fix truncated comment
Closed
gballet
added a commit
that referenced
this pull request
May 2, 2024
quell linter issue support Transition post tree in conversion Refactor transition post genesis (#311) * rewrite per-block conversion pointer management * remove unused method * fix: a branch that can verge at genesis or post genesis (#314) * fix: import cycle in conversion refactor (#315) * fix shadowfork panic in OpenStorageTrie fix OpenStorageTrie: return an error instead of panicking if the account tree not a verkle tree (#321) add a switch to force proof in blocks (#322) * add a switch to force proof in blocks * activate switch * fix switch type add switch to override the stride of the overlay conversion (#323) * add switch to override the stride of the overlay conversion * set a default stride of 10k add a few traces for relaunch add missing step in the chain of setting the override for overlay stride fix: save and load transition state for block processing (#324) * fix: save and load transition state for block processing * log whether the tree is verkle in LoadTransitionState * fix: ensure the transition is marked as started in insertChain * dump saved address * fix nil pointer panic * remove stacktrace that is no longer useful * fix a panic * fix build * check: copy current account address BEFORE it's saved * mandatory panic fix * Remove debug fmt.Println * more cleanup + comments fix: ensure StorageProcessed is properly recovered add traces for gas pool overflow cmd/{geth, utils}: add a command to clear verkle costs (#326) * cmd/{geth, utils}: add a command to clear verkle costs fix: boolean issue fix: load finalization state in FinalizeAndAssemble (#340) * Conversion and TransitionTrie fixes (#346) * fixes Signed-off-by: Ignacio Hagopian <[email protected]> * remove old comment Signed-off-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]> * trace cleanup --------- Signed-off-by: Ignacio Hagopian <[email protected]> Co-authored-by: Ignacio Hagopian <[email protected]> fix: check for nil override in NewBlockChain (#357) fix rebase issue fix a few rebase issues add debug traces fix: assume that having to create a new transaction state mean it hasn't happened. This is a workaround, because it will always be false when restarting a node that has started the transition but not necessarily completed it. add logs to debug shadowfork persist conversion state to db and use an LRU cache for active transition states (#375) * persist conversion state to db * fix: don't recreate LRU when writing state * opt: only write state to db if not already present in LRU * fix: rlp can't encode TransitionState * fix: use gob because binary.Write does not support structs 🤦♂️ * fix: nil pointer panic * add logs to debug shadowfork * no such thing as not enough traces * ditto * fix stupid bug * add a comment for readability * add more traces * Lock the state transition during conversion (#384) * heavy handed approach: lock the state transition during conversion * also lock transition state loading/unloading * reduce logs verbosity * add conversion test to workflow (#386) * add conversion test to workflow * mandatory -f switch fix in rm * forgot & at the end of the geth command * remove incorrect kill * add debug traces * add an overlay stride * fix typo * Apply suggestions from code review eth: add debug_conversionStatus RPC call (#392) * eth: add debug_conversionStatus RPC call * add debug trace4s * Apply suggestions from code review * export started/ended fields fix post-verge sync (#404) * fix post-verge sync * review: fix truncated comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
move the overlay conversion to its own package so that it can be used from the consensus package without causing an import cycle in tests.
I also noted that there are some issues with the tests, so I moved the database creation outside of the generation loop in
chain_makers.go
in order to ensure that the conversion pointers would be properly saved.