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
Bad blocks - potential fix #6212
Conversation
This reverts commit 9be4bd3.
Can you add a description on step 4. Main chain changed to which block? And on step 6, which state root is it building on top off? |
Updated steps + you can check unit test to understand it. |
@@ -665,12 +665,13 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin | |||
// otherwise some nodes would be missing | |||
bool notFoundTheBranchingPointYet = !_blockTree.IsMainChain(branchingPoint.Hash!); | |||
bool notReachedTheReorgBoundary = branchingPoint.Number > (_blockTree.Head?.Header.Number ?? 0); | |||
preMergeFinishBranchingCondition = (notFoundTheBranchingPointYet || notReachedTheReorgBoundary); | |||
bool notInBlockProduction = !options.ContainsFlag(ProcessingOptions.ProducingBlock); |
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.
notInForceProcessing?
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.
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.
notInForceProcessing?
thx, changed.
oh yeah. This code looks unnecessary now, but I am not going to touch it in this PR. Let's analyze it more
* Potential bad blocks fix * Revert "Potential bad blocks fix" This reverts commit 9be4bd3. * Potential fix * Integration tests with iInvalid block and fix * cleanup tests * notInForceProcessing
* Potential bad blocks fix * Revert "Potential bad blocks fix" This reverts commit 9be4bd3. * Potential fix * Integration tests with iInvalid block and fix * cleanup tests * notInForceProcessing
To address invalid block production bug: NethermindEth/nethermind#6212 https://github.com/NethermindEth/nethermind/releases/tag/1.21.1 Signed-off-by: Jakub Sokołowski <jakub@status.im>
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.
Quiero saber bien de esto
Changes
We've identified invalid blocks on the Gnosis network, but the same issue can happen on Ethereum Mainnet. During our debugging session, we uncovered that we were building on an incorrect state root (grandparent) rather than the parent state root.
The
TrySetState
function inBlockProducerBase
ensures that we have the required state, so within the context of block production, we don't need to go through a loop. However, without this change, and in the event of a loop (or "branch") in the blockchainProcessor, we might end up building on top of an incorrect state root. This is because InitBranch sets the state root, and it could lead to the creation of an invalid block. The loop essentially goes backward in an attempt to align with the main chain, so if the main chain undergoes changes during block improvement, we might encounter this issue.The loop scenario can occur as follows:
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?