Merge/processing through queue #4171
Conversation
…ueEmpty # Conflicts: # src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs # src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V1.cs
This reverts commit 69ff59c.
Combine Suggest setAsMain into BlockTreeSuggestOptions
…oughQueue # Conflicts: # src/Nethermind/Nethermind.State/IReadOnlyStateProvider.cs
| 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}"); | ||
| bool setAsMainSet = (options & BlockTreeSuggestOptions.SetAsMain) != 0; |
There was a problem hiding this comment.
it is weird that we have SetAsMain and DontSetAsMain option
There was a problem hiding this comment.
yeah, but before we had bool? setAsMain = null. We could force to set or not set, or derive that from !ShouldProcess flag. Left it the same here. Otherwise, we can change it for ShouldProcess contain flag NotSetAsMain and SetAsMain being the default? It's riskier in my opinion.
There was a problem hiding this comment.
Shoudve kept as bool or make the option a proper struct imo.
There was a problem hiding this comment.
Bit manipulation is overly complicated for something that could just use a standard struct, hard to read. There is no noticable performance nor size advantage for this use case.
There was a problem hiding this comment.
Made it bit more readable and added more docs. Yes it wasn't about performance, wanted to make it more consistent API.
src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs
Outdated
Show resolved
Hide resolved
LukaszRozmej
force-pushed
the
merge/processingThroughQueue
branch
from
feb0e08
to
0c4c074
Compare
|
Closing in favor of #4224 |
Changes:
Moves processing payloads to be always through queue
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests??