Closed
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4528 +/- ##
==========================================
- Coverage 32.67% 31.86% -0.81%
==========================================
Files 497 497
Lines 58849 58899 +50
==========================================
- Hits 19227 18767 -460
- Misses 36245 36746 +501
- Partials 3377 3386 +9 |
Contributor
❌ 22 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
KolbyML
force-pushed
the
kolby-nit-4548-block-reexecution-pathdb-support-2
branch
from
1d2fd45 to
8397886
Compare
pmikolajczyk41
approved these changes
Mar 19, 2026
Comment on lines
+281
to
+283
| if start < requestedStart { | ||
| log.Info("BlocksReExecutor fell back to an earlier available anchor state", "requestedAnchor", requestedStart, "stateAt", start, "endBlock", currentBlock) | ||
| } |
|
|
||
| func (s *BlocksReExecutor) dereferenceRoot(root common.Hash) { | ||
| if s.db == nil { | ||
| return |
Member
There was a problem hiding this comment.
maybe we should log about it (?)
Comment on lines
+346
to
+347
| value := reflect.ValueOf(executor).Elem() | ||
| room := value.FieldByName("room").Int() |
Member
There was a problem hiding this comment.
maybe we could add some getter?
| f.String(prefix+".mode", DefaultConfig.Mode, "mode to run the blocks-reexecutor on. Valid modes full and random. full - execute all the blocks in the given range. random - execute a random sample range of blocks with in a given range") | ||
| f.String(prefix+".blocks", DefaultConfig.Blocks, "json encoded list of block ranges in the form of start and end block numbers in a list of size 2") | ||
| f.Bool(prefix+".commit-state-to-disk", DefaultConfig.CommitStateToDisk, "if set, blocks-reexecutor not only re-executes blocks but it also commits their state to triedb") | ||
| f.Int(prefix+".room", DefaultConfig.Room, "number of threads to parallelize blocks re-execution") |
Member
There was a problem hiding this comment.
maybe we could add a note that this doesn't apply for pathdb?
bragaigor
reviewed
Mar 19, 2026
Comment on lines
+209
to
+213
| if err == nil { | ||
| _ = blocksReExecutor.db.TrieDB().Reference(header.Root, common.Hash{}) // Will be dereferenced later in advanceStateUpToBlock | ||
| return sdb, func() { blocksReExecutor.dereferenceRoot(header.Root) }, nil | ||
| } | ||
| return sdb, arbitrum.NoopStateRelease, err |
Contributor
There was a problem hiding this comment.
nitpick: I'm so used to read err != nil that took me a while to notice err == nil so wondering if it would improve code readability and consistency with other parts to reverse and use err != nil?
magicxyyz
requested changes
Mar 19, 2026
| if header == nil { | ||
| return nil, arbitrum.NoopStateRelease, errors.New("start header not found") | ||
| } | ||
| statedb, err := blockchain.StateAt(header.Root) |
Contributor
There was a problem hiding this comment.
I think we should also add fallback to bc.HistoricState as in StateAndHeaderFromHeader:
if bc.TrieDB().Scheme() == rawdb.PathScheme {
statedb, err := bc.StateAt(header.Root)
if err != nil {
statedb, err = bc.HistoricState(header.Root)
if err != nil {
return nil, nil, err
}
}
return statedb, header, nil
}
| chainEnd := blockchain.CurrentBlock().Number.Uint64() | ||
| minBlocksPerThread := uint64(10000) | ||
| room := c.Room | ||
| if scheme == rawdb.PathScheme && room > 1 { |
Contributor
There was a problem hiding this comment.
Is there a reason why we need to limit re-execution to single thread?
concurrent historical state access is already supported, eg. eth_call RPC calls are executed concurrently.
Did you get any errors when trying to run it in parallel?
Member
Author
|
Closing this |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
fixes NIT-4548