Open
Conversation
…feat/execution-recorder-interface
…ither invalid or empty
… that block up to current block's parent header
use case example: Submit retryable tx with empty calldata and that returns early
hudem1
requested review from
AnkushinDaniil,
damian-orzechowski,
svlachakis and
wurdum
as code owners
There was a problem hiding this comment.
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #753 +/- ##
==========================================
- Coverage 77.24% 77.22% -0.03%
==========================================
Files 185 187 +2
Lines 11889 12153 +264
Branches 1615 1658 +43
==========================================
+ Hits 9184 9385 +201
- Misses 2113 2159 +46
- Partials 592 609 +17
🚀 New features to boost your workflow:
|
hudem1
commented
Mar 13, 2026
Comment on lines
+162
to
+187
| // Flush the trie dirty cache to disk on shutdown so that the latest processed blocks' | ||
| // state is persisted. Without this, Nethermind only persists up to head - PruningBoundary, | ||
| // while Nitro persists up to head. On restart, the mismatch causes Nitro to detect an error. | ||
| // _api.ProcessExit!.Token.Register(() => | ||
| // { | ||
| // ILogger logger = _api.LogManager.GetClassLogger<ArbitrumPlugin>(); | ||
| // if (logger.IsInfo) | ||
| // logger.Info("Flushing trie cache to disk before shutdown..."); | ||
|
|
||
| // _api.WorldStateManager?.FlushCache(CancellationToken.None); | ||
|
|
||
| // // FlushCache persists state but does not update BestPersistedState because | ||
| // // TrieStore.AnnounceReorgBoundaries() requires LatestCommittedBlockNumber >= LastPersistedBlockNumber + maxDepth, | ||
| // // which is never met when persisting up to the head block itself. | ||
| // // Set it directly so that on restart the node head matches the last processed block. | ||
| // long? headNumber = _api.BlockTree!.Head?.Number; | ||
| // if (headNumber.HasValue) | ||
| // { | ||
| // if (logger.IsInfo) | ||
| // logger.Info($"Setting BestPersistedState to {headNumber.Value}."); | ||
| // _api.BlockTree!.BestPersistedState = headNumber.Value; | ||
| // } | ||
|
|
||
| // if (logger.IsInfo) | ||
| // logger.Info("Trie cache flushed."); | ||
| // }); |
Collaborator
Author
There was a problem hiding this comment.
When running non-archive node, such as when using memory pruning, the node does not persist every block. Therefore, it does not persist the last processed block. On restart, this causes some mismatch issue where nitro picks up from last processed block and nethermind starts from last processed - PruningBoundary.
To solve that issue, i suggest 2 possible solutions storing the last processed block on shutdown:
- either only uncomment this code:
- pros: no modif to base NMC.
- cons: will store the whole dirty nodes cache: i currently set
MaxUnpersistedBlockCountto 1000 in this PR for the .json config files, meaning 1000 blocks' state will be persisted on shutdown only for to have the last processed block stored.
- or use the code in this base NMC PR
- pros: will persist only the last processed block
- cons: requires adding those modifications to base NMC (which would then be configurable for nodes other than Arbitrum), so, not sure that's a good solution.
16 tasks
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
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.
Dependent on this NMC PR : NethermindEth/nethermind#10701
This method taking a [start, end] makes sure the state trie exists for each block within that range (reconstructing it if needed), so that when the corresponding RecordBlockCreation calls are made, the state already exists and the witness generation can be made without latency (which would be due to on the fly state reconstruction).
Reconstructed state goes into some memDB that gets pruned gradually when the trie nodes are considered useless (not referenced anymore), otherwise it'd grow indefinitely.
This follows nitro which uses this Referencing/Dereferencing mechanism to keep track of needed trie nodes.
And just like nitro,
ReconstructedStateTrieStorewrites to this memDB overlay and is backed by the main trie store (as read only) for reading trie nodes from disk (when not present in memDB). When checking for state root existence, it ignores the dirty nodes cache as not reliable as memory pruning could evict nodes there and only part of the state trie could get persisted. So, it checks only the memDB or disk.