-
Notifications
You must be signed in to change notification settings - Fork 461
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
Beacon reorgs during the sync #4243
Conversation
…-fix' into merge/testing # Conflicts: # src/Nethermind/Nethermind.Merge.Plugin/Synchronization/MergeBlockDownloader.cs
# Conflicts: # src/Nethermind/Nethermind.Runner/configs/ropsten.cfg # src/Nethermind/Nethermind.Runner/configs/sepolia.cfg
# Conflicts: # src/Nethermind/Nethermind.Merge.Plugin/Synchronization/MergeBlockDownloader.cs # src/Nethermind/Nethermind.Runner/configs/ropsten.cfg # src/Nethermind/Nethermind.Runner/configs/sepolia.cfg
# Conflicts: # src/Nethermind/Nethermind.Blockchain/BlockTree.cs # src/Nethermind/Nethermind.Core/BlockInfo.cs # src/Nethermind/Nethermind.Merge.Plugin.Test/BlockTreeTests.ChainLevelHelper.cs # src/Nethermind/Nethermind.Merge.Plugin.Test/BlockTreeTests.cs # src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.Synchronization.cs # src/Nethermind/Nethermind.Merge.Plugin/Synchronization/ChainLevelHelper.cs
limit: | ||
description: 'Limit for hive tests' | ||
required: false | ||
type: string | ||
hive: |
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.
NP: Why move?
bool fillBeaconBlock = options.ContainsFlag(BlockTreeSuggestOptions.FillBeaconBlock); | ||
bool setAsMain = options.ContainsFlag(BlockTreeSuggestOptions.ForceSetAsMain) || | ||
!options.ContainsFlag(BlockTreeSuggestOptions.ForceDontSetAsMain) && !shouldProcess; | ||
|
||
if (_logger.IsTrace) _logger.Trace($"Suggesting a new block. BestSuggestedBlock {BestSuggestedBody}, BestSuggestedBlock TD {BestSuggestedBody?.TotalDifficulty}, Block TD {block?.TotalDifficulty}, Head: {Head}, Head TD: {Head?.TotalDifficulty}, Block {block?.ToString(Block.Format.FullHashAndNumber)}. ShouldProcess: {shouldProcess}, TryProcessKnownBlock: {fillBeaconBlock}, SetAsMain {setAsMain}"); | ||
if (_logger.IsTrace) |
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.
You and @LukaszRozmej seems to keep switching style. Can you sync your IDE or try not to change.
MoveToBeaconMainChain = 32, | ||
|
||
BeaconBlockInsert = TotalDifficultyNotNeeded | BeaconInsert | MoveToBeaconMainChain | NotOnMainChain | ||
BeaconHeaderMetadata = 2, |
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.
np: Can you use a verb for this, like MarkBeaconHeaderMetadata
{ | ||
int processingQueueCount = _processingQueue.Count; | ||
if (!blockInfo.IsBeaconMainChain && blockInfo.IsBeaconInfo) |
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.
To clarify, this mean it have blocks from NP that is not marked as main? What does this mean? Can you add comment?
} | ||
BlockInfo predecessorInfo = _blockTree.GetInfo(predecessor.Number, predecessor.GetOrCalculateHash()).Info; | ||
predecessorInfo.BlockNumber = predecessor.Number; | ||
if (predecessorInfo.IsBeaconMainChain) break; |
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.
What is the difference from IsBeaconMainChain
then just main chain?
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.
BeaconMainChain wasn't validated. It just new payloads from CL clients.
Draft for discussion with @LukaszRozmej