From b04a6b026267b39284923829f869fe067aaae36c Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 26 Apr 2024 12:25:08 +0200 Subject: [PATCH] Skip hash validations when block comes from NewPayload and we just calculated those hashes to begin with. --- .../Validators/TestBlockValidator.cs | 2 +- .../Validators/AlwaysValid.cs | 2 +- .../Validators/BlockValidator.cs | 27 ++++++++++++------- .../Validators/IBlockValidator.cs | 2 +- .../Validators/NeverValidBlockValidator.cs | 2 +- .../InvalidBlockInterceptorTest.cs | 8 +++--- .../Handlers/NewPayloadHandler.cs | 4 +-- .../InvalidBlockInterceptor.cs | 8 ++---- .../BlockDownloaderTests.cs | 2 +- 9 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestBlockValidator.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestBlockValidator.cs index a9bfd109efb..23a4fb9dd26 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestBlockValidator.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestBlockValidator.cs @@ -56,7 +56,7 @@ public bool ValidateSuggestedBlock(Block block) { return _alwaysSameResultForSuggested ?? _suggestedValidationResults.Dequeue(); } - public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error) + public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error, bool validateHashes = true) { error = null; return _alwaysSameResultForSuggested ?? _suggestedValidationResults.Dequeue(); diff --git a/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs b/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs index 85abab721b0..92ec5b749bf 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs @@ -102,7 +102,7 @@ public bool ValidateOrphanedBlock(Block block, out string? error) return _result; } - public bool ValidateSuggestedBlock(Block block, out string? error) + public bool ValidateSuggestedBlock(Block block, out string? error, bool validateHashes = true) { error = null; return _result; diff --git a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs index 902bf829380..04e0ca08f68 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs @@ -88,15 +88,17 @@ public bool ValidateSuggestedBlock(Block block) { return ValidateSuggestedBlock(block, out _); } + /// /// Suggested block validation runs basic checks that can be executed before going through the expensive EVM processing. /// /// A block to validate /// Message detailing a validation failure. + /// /// /// true if the is valid; otherwise, false. /// - public bool ValidateSuggestedBlock(Block block, out string? errorMessage) + public bool ValidateSuggestedBlock(Block block, out string? errorMessage, bool validateHashes = true) { IReleaseSpec spec = _specProvider.GetSpec(block.Header); @@ -113,14 +115,14 @@ public bool ValidateSuggestedBlock(Block block, out string? errorMessage) return false; } - if (!ValidateUnclesHashMatches(block, out Hash256 unclesHash)) + if (validateHashes && !ValidateUnclesHashMatches(block, out Hash256 unclesHash)) { if (_logger.IsDebug) _logger.Debug($"{Invalid(block)} Uncles hash mismatch: expected {block.Header.UnclesHash}, got {unclesHash}"); errorMessage = BlockErrorMessages.InvalidUnclesHash; return false; } - if (!_unclesValidator.Validate(block.Header, block.Uncles)) + if (block.Uncles.Length > 0 && !_unclesValidator.Validate(block.Header, block.Uncles)) { if (_logger.IsDebug) _logger.Debug($"{Invalid(block)} Invalid uncles"); errorMessage = BlockErrorMessages.InvalidUncle; @@ -134,15 +136,20 @@ public bool ValidateSuggestedBlock(Block block, out string? errorMessage) return false; } - if (!ValidateTxRootMatchesTxs(block, out Hash256 txRoot)) + if (validateHashes) { - if (_logger.IsDebug) _logger.Debug($"{Invalid(block)} Transaction root hash mismatch: expected {block.Header.TxRoot}, got {txRoot}"); - errorMessage = BlockErrorMessages.InvalidTxRoot(block.Header.TxRoot, txRoot); - return false; - } + if (!ValidateTxRootMatchesTxs(block, out Hash256 txRoot)) + { + if (_logger.IsDebug) _logger.Debug($"{Invalid(block)} Transaction root hash mismatch: expected {block.Header.TxRoot}, got {txRoot}"); + errorMessage = BlockErrorMessages.InvalidTxRoot(block.Header.TxRoot!, txRoot); + return false; + } - if (!ValidateWithdrawals(block, spec, out errorMessage)) - return false; + if (!ValidateWithdrawals(block, spec, out errorMessage)) + { + return false; + } + } return true; } diff --git a/src/Nethermind/Nethermind.Consensus/Validators/IBlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/IBlockValidator.cs index efc9d858277..e31754c8870 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/IBlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/IBlockValidator.cs @@ -9,6 +9,6 @@ namespace Nethermind.Consensus.Validators; public interface IBlockValidator : IHeaderValidator, IWithdrawalValidator { bool ValidateOrphanedBlock(Block block, [NotNullWhen(false)] out string? error); - bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error); + bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error, bool validateHashes = true); bool ValidateProcessedBlock(Block processedBlock, TxReceipt[] receipts, Block suggestedBlock, [NotNullWhen(false)] out string? error); } diff --git a/src/Nethermind/Nethermind.Consensus/Validators/NeverValidBlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/NeverValidBlockValidator.cs index 9982dea2e4c..de1a3fa77bf 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/NeverValidBlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/NeverValidBlockValidator.cs @@ -55,7 +55,7 @@ public bool ValidateOrphanedBlock(Block block, out string? error) return false; } - public bool ValidateSuggestedBlock(Block block, out string? error) + public bool ValidateSuggestedBlock(Block block, out string? error, bool validateHashes = true) { error = null; return false; diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/InvalidChainTracker/InvalidBlockInterceptorTest.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/InvalidChainTracker/InvalidBlockInterceptorTest.cs index 7483159ddbd..2d7f6226b01 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/InvalidChainTracker/InvalidBlockInterceptorTest.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/InvalidChainTracker/InvalidBlockInterceptorTest.cs @@ -43,8 +43,8 @@ public void Setup() public void TestValidateSuggestedBlock(bool baseReturnValue, bool isInvalidBlockReported) { Block block = Build.A.Block.TestObject; - _baseValidator.ValidateSuggestedBlock(block, out string? error).Returns(baseReturnValue); - _invalidBlockInterceptor.ValidateSuggestedBlock(block); + _baseValidator.ValidateSuggestedBlock(block, out _).Returns(baseReturnValue); + _invalidBlockInterceptor.ValidateSuggestedBlock(block, out _); _tracker.Received().SetChildParent(block.GetOrCalculateHash(), block.ParentHash!); if (isInvalidBlockReported) @@ -103,7 +103,7 @@ public void TestBlockWithNotMatchingTxShouldNotGetTracked() )); _baseValidator.ValidateSuggestedBlock(block, out _).Returns(false); - _invalidBlockInterceptor.ValidateSuggestedBlock(block); + _invalidBlockInterceptor.ValidateSuggestedBlock(block, out _); _tracker.DidNotReceive().SetChildParent(block.GetOrCalculateHash(), block.ParentHash!); _tracker.DidNotReceive().OnInvalidBlock(block.GetOrCalculateHash(), block.ParentHash); @@ -121,7 +121,7 @@ public void TestBlockWithIncorrectWithdrawalsShouldNotGetTracked() )); _baseValidator.ValidateSuggestedBlock(block, out _).Returns(false); - _invalidBlockInterceptor.ValidateSuggestedBlock(block); + _invalidBlockInterceptor.ValidateSuggestedBlock(block, out _); _tracker.DidNotReceive().SetChildParent(block.GetOrCalculateHash(), block.ParentHash!); _tracker.DidNotReceive().OnInvalidBlock(block.GetOrCalculateHash(), block.ParentHash); diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/NewPayloadHandler.cs b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/NewPayloadHandler.cs index 7e7d9f1b17b..d418492d139 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/NewPayloadHandler.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/NewPayloadHandler.cs @@ -154,7 +154,7 @@ public async Task> HandleAsync(ExecutionPayload r if (!ShouldProcessBlock(block, parentHeader, out ProcessingOptions processingOptions)) // we shouldn't process block { - if (!_blockValidator.ValidateSuggestedBlock(block, out string? error)) + if (!_blockValidator.ValidateSuggestedBlock(block, out string? error, validateHashes: false)) { if (_logger.IsInfo) _logger.Info($"Rejecting invalid block received during the sync, block: {block}"); return NewPayloadV1Result.Invalid(error); @@ -394,7 +394,7 @@ private bool ValidateWithBlockValidator(Block block, BlockHeader parent, out str { block.Header.TotalDifficulty ??= parent.TotalDifficulty + block.Difficulty; block.Header.IsPostMerge = true; // I think we don't need to set it again here. - bool isValid = _blockValidator.ValidateSuggestedBlock(block, out error); + bool isValid = _blockValidator.ValidateSuggestedBlock(block, out error, validateHashes: false); if (!isValid && _logger.IsWarn) _logger.Warn($"Block validator rejected the block {block.ToString(Block.Format.FullHashAndNumber)}."); return isValid; } diff --git a/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs b/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs index 2a3c9dd350e..87cbb58f650 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs @@ -69,13 +69,9 @@ public bool Validate(BlockHeader header, bool isUncle, [NotNullWhen(false)] out return result; } - public bool ValidateSuggestedBlock(Block block) + public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error, bool validateHashes = true) { - return ValidateSuggestedBlock(block, out _); - } - public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error) - { - bool result = _baseValidator.ValidateSuggestedBlock(block, out error); + bool result = _baseValidator.ValidateSuggestedBlock(block, out error, validateHashes); if (!result) { if (_logger.IsTrace) _logger.Trace($"Intercepted a bad block {block}"); diff --git a/src/Nethermind/Nethermind.Synchronization.Test/BlockDownloaderTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/BlockDownloaderTests.cs index a903d3137a2..709607c9a41 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/BlockDownloaderTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/BlockDownloaderTests.cs @@ -489,7 +489,7 @@ public bool ValidateOrphanedBlock(Block block, [NotNullWhen(false)] out string? return true; } - public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error) + public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error, bool validateHashes = true) { Thread.Sleep(1000); error = null;