From efd88a2c47b8e0f5da35d42ac5d20b7cc378741f Mon Sep 17 00:00:00 2001 From: Anders Holmbjerg Kristiansen Date: Thu, 30 Nov 2023 15:18:44 +0100 Subject: [PATCH 1/5] Also check withdrawals before insertion --- .../Validators/BlockValidator.cs | 13 +++++++++---- .../FastBlocks/BodiesSyncFeed.cs | 4 +++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs index 971a1c42a72..4c6c00520ca 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs @@ -320,14 +320,19 @@ public static bool ValidateUnclesHashMatches(Block block, out Hash256 unclesHash } public static bool ValidateWithdrawalsHashMatches(Block block, out Hash256? withdrawalsRoot) + { + return ValidateWithdrawalsHashMatches(block.Header, block.Body, out withdrawalsRoot); + } + + public static bool ValidateWithdrawalsHashMatches(BlockHeader header, BlockBody body, out Hash256? withdrawalsRoot) { withdrawalsRoot = null; - if (block.Withdrawals == null) - return block.Header.WithdrawalsRoot == null; + if (body.Withdrawals == null) + return header.WithdrawalsRoot == null; - withdrawalsRoot = new WithdrawalTrie(block.Withdrawals).RootHash; + withdrawalsRoot = new WithdrawalTrie(body.Withdrawals).RootHash; - return block.Header.WithdrawalsRoot == withdrawalsRoot; + return header.WithdrawalsRoot == withdrawalsRoot; } private static string Invalid(Block block) => diff --git a/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs b/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs index cb06445c567..461c8eed96c 100644 --- a/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs +++ b/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Nethermind.Blockchain; using Nethermind.Blockchain.Synchronization; +using Nethermind.Consensus.Validators; using Nethermind.Core; using Nethermind.Core.Crypto; using Nethermind.Db; @@ -190,7 +191,8 @@ private bool TryPrepareBlock(BlockInfo blockInfo, BlockBody blockBody, out Block Hash256 rootHash = TxTrie.CalculateRoot(blockBody.Transactions); bool txRootIsValid = rootHash == header.TxRoot; bool unclesHashIsValid = UnclesHash.Calculate(blockBody.Uncles) == header.UnclesHash; - if (txRootIsValid && unclesHashIsValid) + bool withdrawalsRootIsValid = BlockValidator.ValidateWithdrawalsHashMatches(header, blockBody, out _); + if (txRootIsValid && unclesHashIsValid && withdrawalsRootIsValid) { block = new Block(header, blockBody); } From 57624bbb5d2fc53a55d1f6c5d2ec76a2c307f82e Mon Sep 17 00:00:00 2001 From: Anders Holmbjerg Kristiansen Date: Thu, 30 Nov 2023 17:53:42 +0100 Subject: [PATCH 2/5] refactor --- .../Validators/BlockValidator.cs | 24 +++++++++++++++---- .../FastBlocks/BodiesSyncFeed.cs | 2 +- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs index 4c6c00520ca..83e03979cd2 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs @@ -306,17 +306,33 @@ private bool ValidateEip4844Fields(Block block, IReleaseSpec spec, out string? e return true; } + public static bool ValidateBodyForHeader(BlockHeader header, BlockBody toBeValidated) + { + return ValidateTxRootMatchesTxs(header, toBeValidated, out _) && + ValidateUnclesHashMatches(header, toBeValidated, out _) && + ValidateWithdrawalsHashMatches(header, toBeValidated, out _); + } + public static bool ValidateTxRootMatchesTxs(Block block, out Hash256 txRoot) { - txRoot = new TxTrie(block.Transactions).RootHash; - return txRoot == block.Header.TxRoot; + return ValidateTxRootMatchesTxs(block.Header, block.Body, out txRoot); + } + public static bool ValidateTxRootMatchesTxs(BlockHeader header, BlockBody body, out Hash256 txRoot) + { + txRoot = new TxTrie(body.Transactions).RootHash; + return txRoot == header.TxRoot; } public static bool ValidateUnclesHashMatches(Block block, out Hash256 unclesHash) { - unclesHash = UnclesHash.Calculate(block); + return ValidateUnclesHashMatches(block.Header, block.Body, out unclesHash); + } + + public static bool ValidateUnclesHashMatches(BlockHeader header, BlockBody body, out Hash256 unclesHash) + { + unclesHash = UnclesHash.Calculate(body.Uncles); - return block.Header.UnclesHash == unclesHash; + return header.UnclesHash == unclesHash; } public static bool ValidateWithdrawalsHashMatches(Block block, out Hash256? withdrawalsRoot) diff --git a/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs b/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs index 461c8eed96c..69d4a2a7935 100644 --- a/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs +++ b/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs @@ -192,7 +192,7 @@ private bool TryPrepareBlock(BlockInfo blockInfo, BlockBody blockBody, out Block bool txRootIsValid = rootHash == header.TxRoot; bool unclesHashIsValid = UnclesHash.Calculate(blockBody.Uncles) == header.UnclesHash; bool withdrawalsRootIsValid = BlockValidator.ValidateWithdrawalsHashMatches(header, blockBody, out _); - if (txRootIsValid && unclesHashIsValid && withdrawalsRootIsValid) + if (BlockValidator.ValidateBodyForHeader(header, blockBody)) { block = new Block(header, blockBody); } From 62b49c874ec9d2d6c7d55d7d8964daba64b0f5d6 Mon Sep 17 00:00:00 2001 From: ak88 Date: Sat, 2 Dec 2023 14:59:07 +0100 Subject: [PATCH 3/5] refactor --- .../Nethermind.Consensus/Validators/BlockValidator.cs | 6 ++---- .../Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs | 6 +----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs index 83e03979cd2..db786c1b547 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs @@ -306,12 +306,10 @@ private bool ValidateEip4844Fields(Block block, IReleaseSpec spec, out string? e return true; } - public static bool ValidateBodyForHeader(BlockHeader header, BlockBody toBeValidated) - { - return ValidateTxRootMatchesTxs(header, toBeValidated, out _) && + public static bool ValidateBodyAgainstHeader(BlockHeader header, BlockBody toBeValidated) => + ValidateTxRootMatchesTxs(header, toBeValidated, out _) && ValidateUnclesHashMatches(header, toBeValidated, out _) && ValidateWithdrawalsHashMatches(header, toBeValidated, out _); - } public static bool ValidateTxRootMatchesTxs(Block block, out Hash256 txRoot) { diff --git a/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs b/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs index 69d4a2a7935..a42985a66c3 100644 --- a/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs +++ b/src/Nethermind/Nethermind.Synchronization/FastBlocks/BodiesSyncFeed.cs @@ -188,11 +188,7 @@ public override SyncResponseHandlingResult HandleResponse(BodiesSyncBatch? batch private bool TryPrepareBlock(BlockInfo blockInfo, BlockBody blockBody, out Block? block) { BlockHeader header = _blockTree.FindHeader(blockInfo.BlockHash, blockNumber: blockInfo.BlockNumber); - Hash256 rootHash = TxTrie.CalculateRoot(blockBody.Transactions); - bool txRootIsValid = rootHash == header.TxRoot; - bool unclesHashIsValid = UnclesHash.Calculate(blockBody.Uncles) == header.UnclesHash; - bool withdrawalsRootIsValid = BlockValidator.ValidateWithdrawalsHashMatches(header, blockBody, out _); - if (BlockValidator.ValidateBodyForHeader(header, blockBody)) + if (BlockValidator.ValidateBodyAgainstHeader(header, blockBody)) { block = new Block(header, blockBody); } From 5149c939f6670c00c5b225a7d9b89040f74bc2a7 Mon Sep 17 00:00:00 2001 From: ak88 Date: Fri, 8 Dec 2023 11:59:51 +0100 Subject: [PATCH 4/5] unit tests --- .../Validators/BlockValidatorTests.cs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs index 1993a8fce0f..a1bef4e40a3 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs @@ -3,12 +3,15 @@ using Nethermind.Consensus.Validators; using Nethermind.Core; +using Nethermind.Core.Crypto; using Nethermind.Core.Specs; using Nethermind.Core.Test.Builders; using Nethermind.Logging; using Nethermind.Specs; using Nethermind.Specs.Test; +using NSubstitute; using NUnit.Framework; +using Org.BouncyCastle.Asn1.BC; namespace Nethermind.Blockchain.Test.Validators { @@ -30,5 +33,61 @@ public void When_more_uncles_than_allowed_returns_false() bool result = blockValidator.ValidateSuggestedBlock(Build.A.Block.WithUncles(Build.A.BlockHeader.TestObject).TestObject); Assert.False(result); } + + [Test] + public void ValidateBodyAgainstHeader_BlockIsValid_ReturnsTrue() + { + Block block = Build.A.Block + .WithTransactions(1, Substitute.For()) + .WithWithdrawals(1) + .TestObject; + + Assert.That( + BlockValidator.ValidateBodyAgainstHeader(block.Header, block.Body), + Is.True); + } + + [Test] + public void ValidateBodyAgainstHeader_BlockHasInvalidTxRoot_ReturnsFalse() + { + Block block = Build.A.Block + .WithTransactions(1, Substitute.For()) + .WithWithdrawals(1) + .TestObject; + block.Header.TxRoot = Keccak.OfAnEmptyString; + + Assert.That( + BlockValidator.ValidateBodyAgainstHeader(block.Header, block.Body), + Is.False); + } + + + [Test] + public void ValidateBodyAgainstHeader_BlockHasInvalidUnclesRoot_ReturnsFalse() + { + Block block = Build.A.Block + .WithTransactions(1, Substitute.For()) + .WithWithdrawals(1) + .TestObject; + block.Header.UnclesHash = Keccak.OfAnEmptyString; + + Assert.That( + BlockValidator.ValidateBodyAgainstHeader(block.Header, block.Body), + Is.False); + } + + [Test] + public void ValidateBodyAgainstHeader_BlockHasInvalidWithdrawalsRoot_ReturnsFalse() + { + Block block = Build.A.Block + .WithTransactions(1, Substitute.For()) + .WithWithdrawals(1) + .TestObject; + block.Header.WithdrawalsRoot = Keccak.OfAnEmptyString; + + Assert.That( + BlockValidator.ValidateBodyAgainstHeader(block.Header, block.Body), + Is.False); + } } } From 339a58e1b10544d8d6f1c87829bcfcf2d4775b32 Mon Sep 17 00:00:00 2001 From: ak88 Date: Fri, 8 Dec 2023 12:13:37 +0100 Subject: [PATCH 5/5] unnecessary using --- .../Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs index a1bef4e40a3..c38d5c264f2 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/BlockValidatorTests.cs @@ -11,7 +11,6 @@ using Nethermind.Specs.Test; using NSubstitute; using NUnit.Framework; -using Org.BouncyCastle.Asn1.BC; namespace Nethermind.Blockchain.Test.Validators {