From 5c01eb94896c76e3dbb967132b0c7799fe90112f Mon Sep 17 00:00:00 2001 From: Oleg Jakushkin Date: Tue, 3 Jan 2023 08:19:03 +0000 Subject: [PATCH 1/5] iterative to fixed test change O(N) to O(1) in case of processing requests, also reduced storage space requirements for alternative approach --- .../Processing/BlockchainProcessor.cs | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs index d873ac28730..3598bf5decc 100644 --- a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs +++ b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs @@ -339,12 +339,36 @@ private void FireProcessingQueueEmpty() if (!shouldProcess) { - if (_logger.IsDebug) _logger.Debug($"Skipped processing of {suggestedBlock.ToString(Block.Format.FullHashAndNumber)}, Head = {_blockTree.Head?.Header?.ToString(BlockHeader.Format.Short)}, total diff = {totalDifficulty}, head total diff = {_blockTree.Head?.TotalDifficulty}"); + if (_logger.IsDebug) + _logger.Debug( + $"Skipped processing of {suggestedBlock.ToString(Block.Format.FullHashAndNumber)}, Head = {_blockTree.Head?.Header?.ToString(BlockHeader.Format.Short)}, total diff = {totalDifficulty}, head total diff = {_blockTree.Head?.TotalDifficulty}"); return null; } - ProcessingBranch processingBranch = PrepareProcessingBranch(suggestedBlock, options); - PrepareBlocksToProcess(suggestedBlock, options, processingBranch); + Keccak stateRoot = null; + + if (!suggestedBlock.IsGenesis) + { + var branchingPoint = _blockTree.FindParentHeader(suggestedBlock.Header, + BlockTreeLookupOptions.TotalDifficultyNotNeeded); + branchingPoint ??= suggestedBlock.Header; // Case it was root + stateRoot = branchingPoint?.StateRoot; + } + + ProcessingBranch processingBranch = new ProcessingBranch(stateRoot, new List(){}); + + //TODO: test unit tests for no PrepareProcessingBranch in case of ForceProcessing flag! + if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) + { + processingBranch.Blocks.Clear(); + processingBranch.BlocksToProcess.Add(suggestedBlock); + _recoveryStep.RecoverData(suggestedBlock); + } + else + { + processingBranch = PrepareProcessingBranch(suggestedBlock, options); + PrepareBlocksToProcess(suggestedBlock, options, processingBranch); + } _stopwatch.Restart(); Block[]? processedBlocks = ProcessBranch(processingBranch, options, tracer); @@ -497,9 +521,9 @@ private void PrepareBlocksToProcess(Block suggestedBlock, ProcessingOptions opti ProcessingBranch processingBranch) { List blocksToProcess = processingBranch.BlocksToProcess; - if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) + if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) { - processingBranch.Blocks.Clear(); + processingBranch.Blocks.Clear(); // TODO: investigate why if we clear it all we need to collect and iterate on all the blocks in PrepareProcessingBranch? blocksToProcess.Add(suggestedBlock); } else @@ -537,9 +561,15 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin bool suggestedBlockIsPostMerge = suggestedBlock.IsPostMerge; Block toBeProcessed = suggestedBlock; + long iterations = 0; do { - blocksToBeAddedToMain.Add(toBeProcessed); + iterations++; + if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) + { + blocksToBeAddedToMain.Add(toBeProcessed); + } + if (_logger.IsTrace) _logger.Trace( $"To be processed (of {suggestedBlock.ToString(Block.Format.Short)}) is {toBeProcessed?.ToString(Block.Format.Short)}"); @@ -570,6 +600,8 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin bool toBeProcessedIsNotBlockOne = toBeProcessed.Number > 1; if (_logger.IsTrace) _logger.Trace($"Finding parent of {toBeProcessed.ToString(Block.Format.Short)}"); + + //Tx to filter on trace here? toBeProcessed = _blockTree.FindParent(toBeProcessed.Header, BlockTreeLookupOptions.None); if (_logger.IsTrace) _logger.Trace($"Found parent {toBeProcessed?.ToString(Block.Format.Short)}"); bool isFastSyncTransition = headIsGenesis && toBeProcessedIsNotBlockOne; @@ -586,7 +618,7 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin // If we hit this condition, it means that something is wrong in MultiSyncModeSelector. // MultiSyncModeSelector switched to full sync when it shouldn't // In this case, it is better to stop searching for more blocks and failed during the processing than trying to build a branch up to the genesis point - if (blocksToBeAddedToMain.Count > MaxBlocksDuringFastSyncTransition) + if (iterations > MaxBlocksDuringFastSyncTransition) { if (_logger.IsWarn) _logger.Warn($"Too long branch to be processed during fast sync transition. Current block to be processed {toBeProcessed}, StateRoot: {toBeProcessed?.StateRoot}"); break; @@ -615,6 +647,7 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin if (_logger.IsTrace) _logger.Trace( $" Current branching point: {branchingPoint.Number}, {branchingPoint.Hash} TD: {branchingPoint.TotalDifficulty} Processing conditions notFoundTheBranchingPointYet {notFoundTheBranchingPointYet}, notReachedTheReorgBoundary: {notReachedTheReorgBoundary}, suggestedBlockIsPostMerge {suggestedBlockIsPostMerge}"); + } while (preMergeFinishBranchingCondition); if (branchingPoint is not null && branchingPoint.Hash != _blockTree.Head?.Hash) From 5ed60abbba9a16dcb9e04d307ebc02f0e65c3c55 Mon Sep 17 00:00:00 2001 From: Oleg Jakushkin Date: Tue, 3 Jan 2023 12:19:55 +0000 Subject: [PATCH 2/5] minor fixes --- .../Processing/BlockchainProcessor.cs | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs index 3598bf5decc..7fd91b94cf7 100644 --- a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs +++ b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs @@ -347,28 +347,28 @@ private void FireProcessingQueueEmpty() Keccak stateRoot = null; - if (!suggestedBlock.IsGenesis) - { - var branchingPoint = _blockTree.FindParentHeader(suggestedBlock.Header, - BlockTreeLookupOptions.TotalDifficultyNotNeeded); - branchingPoint ??= suggestedBlock.Header; // Case it was root - stateRoot = branchingPoint?.StateRoot; - } + //if (!suggestedBlock.IsGenesis) + //{ + // var branchingPoint = _blockTree.FindParentHeader(suggestedBlock.Header, + // BlockTreeLookupOptions.TotalDifficultyNotNeeded); + // branchingPoint ??= suggestedBlock.Header; // Case it was root + // stateRoot = branchingPoint?.StateRoot; + //} ProcessingBranch processingBranch = new ProcessingBranch(stateRoot, new List(){}); //TODO: test unit tests for no PrepareProcessingBranch in case of ForceProcessing flag! - if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) - { - processingBranch.Blocks.Clear(); - processingBranch.BlocksToProcess.Add(suggestedBlock); - _recoveryStep.RecoverData(suggestedBlock); - } - else - { + //if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) + //{ + // processingBranch.Blocks.Clear(); + // processingBranch.BlocksToProcess.Add(suggestedBlock); + // _recoveryStep.RecoverData(suggestedBlock); + //} + //else + //{ processingBranch = PrepareProcessingBranch(suggestedBlock, options); PrepareBlocksToProcess(suggestedBlock, options, processingBranch); - } + // } _stopwatch.Restart(); Block[]? processedBlocks = ProcessBranch(processingBranch, options, tracer); @@ -565,7 +565,7 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin do { iterations++; - if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) + if (!options.ContainsFlag(ProcessingOptions.ForceProcessing)) { blocksToBeAddedToMain.Add(toBeProcessed); } @@ -642,7 +642,8 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin // we need to dig deeper to go all the way to the false (reorg boundary) head // otherwise some nodes would be missing bool notFoundTheBranchingPointYet = !_blockTree.IsMainChain(branchingPoint.Hash!); - bool notReachedTheReorgBoundary = branchingPoint.Number > (_blockTree.Head?.Header.Number ?? 0); + bool notReachedTheReorgBoundary = (options & ProcessingOptions.Trace) == ProcessingOptions.Trace + && ( branchingPoint.Number > (_blockTree.Head?.Header.Number ?? 0)); preMergeFinishBranchingCondition = (notFoundTheBranchingPointYet || notReachedTheReorgBoundary); if (_logger.IsTrace) _logger.Trace( From 651dfe1f915f098b8dc365c7edeab1cd3ad169af Mon Sep 17 00:00:00 2001 From: Oleg Jakushkin Date: Wed, 4 Jan 2023 11:30:47 +0000 Subject: [PATCH 3/5] Clean up --- .../Processing/BlockchainProcessor.cs | 28 ++----------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs index 7fd91b94cf7..a43c27803ba 100644 --- a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs +++ b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs @@ -344,31 +344,9 @@ private void FireProcessingQueueEmpty() $"Skipped processing of {suggestedBlock.ToString(Block.Format.FullHashAndNumber)}, Head = {_blockTree.Head?.Header?.ToString(BlockHeader.Format.Short)}, total diff = {totalDifficulty}, head total diff = {_blockTree.Head?.TotalDifficulty}"); return null; } - - Keccak stateRoot = null; - - //if (!suggestedBlock.IsGenesis) - //{ - // var branchingPoint = _blockTree.FindParentHeader(suggestedBlock.Header, - // BlockTreeLookupOptions.TotalDifficultyNotNeeded); - // branchingPoint ??= suggestedBlock.Header; // Case it was root - // stateRoot = branchingPoint?.StateRoot; - //} - - ProcessingBranch processingBranch = new ProcessingBranch(stateRoot, new List(){}); - - //TODO: test unit tests for no PrepareProcessingBranch in case of ForceProcessing flag! - //if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) - //{ - // processingBranch.Blocks.Clear(); - // processingBranch.BlocksToProcess.Add(suggestedBlock); - // _recoveryStep.RecoverData(suggestedBlock); - //} - //else - //{ - processingBranch = PrepareProcessingBranch(suggestedBlock, options); - PrepareBlocksToProcess(suggestedBlock, options, processingBranch); - // } + + ProcessingBranch processingBranch = PrepareProcessingBranch(suggestedBlock, options); + PrepareBlocksToProcess(suggestedBlock, options, processingBranch); _stopwatch.Restart(); Block[]? processedBlocks = ProcessBranch(processingBranch, options, tracer); From eec31732d08dcf938c87b521bfadd0aaa3d34963 Mon Sep 17 00:00:00 2001 From: Oleg Jakushkin Date: Wed, 4 Jan 2023 14:43:05 +0000 Subject: [PATCH 4/5] Cleanup complience --- .../Nethermind.Consensus/Processing/BlockchainProcessor.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs index a43c27803ba..7cf605f3a0f 100644 --- a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs +++ b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs @@ -344,7 +344,7 @@ private void FireProcessingQueueEmpty() $"Skipped processing of {suggestedBlock.ToString(Block.Format.FullHashAndNumber)}, Head = {_blockTree.Head?.Header?.ToString(BlockHeader.Format.Short)}, total diff = {totalDifficulty}, head total diff = {_blockTree.Head?.TotalDifficulty}"); return null; } - + ProcessingBranch processingBranch = PrepareProcessingBranch(suggestedBlock, options); PrepareBlocksToProcess(suggestedBlock, options, processingBranch); @@ -578,8 +578,6 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin bool toBeProcessedIsNotBlockOne = toBeProcessed.Number > 1; if (_logger.IsTrace) _logger.Trace($"Finding parent of {toBeProcessed.ToString(Block.Format.Short)}"); - - //Tx to filter on trace here? toBeProcessed = _blockTree.FindParent(toBeProcessed.Header, BlockTreeLookupOptions.None); if (_logger.IsTrace) _logger.Trace($"Found parent {toBeProcessed?.ToString(Block.Format.Short)}"); bool isFastSyncTransition = headIsGenesis && toBeProcessedIsNotBlockOne; From 3dfc556bbadd7d050e36c0938b4378b0ad273da0 Mon Sep 17 00:00:00 2001 From: Oleg Jakushkin Date: Wed, 4 Jan 2023 14:43:48 +0000 Subject: [PATCH 5/5] Automated cleanup compliance --- .../Nethermind.Consensus/Processing/BlockchainProcessor.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs index 7cf605f3a0f..19b94863a68 100644 --- a/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs +++ b/src/Nethermind/Nethermind.Consensus/Processing/BlockchainProcessor.cs @@ -499,7 +499,7 @@ private void PrepareBlocksToProcess(Block suggestedBlock, ProcessingOptions opti ProcessingBranch processingBranch) { List blocksToProcess = processingBranch.BlocksToProcess; - if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) + if (options.ContainsFlag(ProcessingOptions.ForceProcessing)) { processingBranch.Blocks.Clear(); // TODO: investigate why if we clear it all we need to collect and iterate on all the blocks in PrepareProcessingBranch? blocksToProcess.Add(suggestedBlock); @@ -619,12 +619,12 @@ private ProcessingBranch PrepareProcessingBranch(Block suggestedBlock, Processin // otherwise some nodes would be missing bool notFoundTheBranchingPointYet = !_blockTree.IsMainChain(branchingPoint.Hash!); bool notReachedTheReorgBoundary = (options & ProcessingOptions.Trace) == ProcessingOptions.Trace - && ( branchingPoint.Number > (_blockTree.Head?.Header.Number ?? 0)); + && (branchingPoint.Number > (_blockTree.Head?.Header.Number ?? 0)); preMergeFinishBranchingCondition = (notFoundTheBranchingPointYet || notReachedTheReorgBoundary); if (_logger.IsTrace) _logger.Trace( $" Current branching point: {branchingPoint.Number}, {branchingPoint.Hash} TD: {branchingPoint.TotalDifficulty} Processing conditions notFoundTheBranchingPointYet {notFoundTheBranchingPointYet}, notReachedTheReorgBoundary: {notReachedTheReorgBoundary}, suggestedBlockIsPostMerge {suggestedBlockIsPostMerge}"); - + } while (preMergeFinishBranchingCondition); if (branchingPoint is not null && branchingPoint.Hash != _blockTree.Head?.Hash)