EIP-4444: Add RPC errors for pruned blocks#8501
Conversation
There was a problem hiding this comment.
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
src/Nethermind/Nethermind.JsonRpc/Modules/BlockFinderExtensions.cs:48
- The condition contains a duplicate check for blockParameter.BlockHash < blockFinder.EarliestHash. Consider removing the redundant check or revising the condition if a different condition was intended.
if (blockParameter is not null && (blockParameter.BlockHash < blockFinder.EarliestHash || blockParameter.BlockHash < blockFinder.EarliestHash))
src/Nethermind/Nethermind.JsonRpc/ErrorCodes.cs:121
- The XML comment for 'PrunedHistoryUnavailable' is misleading since the constant is used for pruned history errors. Consider updating the comment to accurately describe the error.
/// Too many blocks for simulation
|
|
||
| EarliestHash = GenesisHash; | ||
| if (_syncConfig.AncientReceiptsBarrierCalc > 0 && _syncConfig.AncientBodiesBarrierCalc > 0) | ||
| { | ||
| var lowestBlock = _syncConfig.AncientReceiptsBarrierCalc < _syncConfig.AncientBodiesBarrierCalc ? _syncConfig.AncientReceiptsBarrierCalc : _syncConfig.AncientBodiesBarrierCalc; | ||
| EarliestHash = FindBlockHash(lowestBlock); | ||
| } | ||
|
|
There was a problem hiding this comment.
That's probably very naive, and won't work. It should be called after we process/sync up to ancient barriers. Not sure when/where it happens tho.
|
I was considering passing I've decided to go with the EearliestHash approach, as it seems the most straightforward, and easily accessible from within RPC context. But there is a potential issue with current implementation - it probably wont be initialized on the empty DB. As an alternative (hacky) solution we could store EarliestBlockNumber, which would not require any queries in the constructor. |
|
|
||
| public static SearchResult<Block> SearchForBlock(this IBlockFinder blockFinder, BlockParameter? blockParameter, bool allowNulls = false) | ||
| { | ||
| if (blockParameter is not null && (blockParameter.BlockHash < blockFinder.EarliestHash || blockParameter.BlockHash < blockFinder.EarliestHash)) |
There was a problem hiding this comment.
Same condition twice?
How do you want to compare hashes?
There was a problem hiding this comment.
🤦 forgot about the fact that there were hashes. Updated the code to fetch the block number instead
|
superseded by #8521 |
Fixes Closes Resolves #8493
Please choose one of the keywords above to refer to the issue this PR solves followed by the issue number (e.g. Fixes #000). If no issue number, remove the line. Also, remove everything marked optional that is not applicable. Remove this note after reading.
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Optional. Remove if not applicable.
Documentation
Requires documentation update
If yes, link the PR to the docs update or the issue with the details labeled
docs. Remove if not applicable.Requires explanation in Release Notes
If yes, fill in the details here. Remove if not applicable.
Remarks
Optional. Remove if not applicable.