From f81431f4c0a4a4e4626c3556be0ed175ebcc32e4 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 7 Jun 2023 11:35:50 +0200 Subject: [PATCH 1/4] Added ExecutionPayload._transactions to avoid multiple RLP decoding --- .../Nethermind.Merge.Plugin/Data/ExecutionPayload.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index f5ea4cc7f2f1..5cea6490f9e4 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -139,11 +139,13 @@ public virtual bool TryGetBlock(out Block? block, UInt256? totalDifficulty = nul } } + private Transaction[]? _transactions = null; + /// /// Decodes and returns an array of from . /// /// An RLP-decoded array of . - public Transaction[] GetTransactions() => Transactions + public Transaction[] GetTransactions() => _transactions ??= Transactions .Select(t => Rlp.Decode(t, RlpBehaviors.SkipTypedWrapping)) .ToArray(); @@ -151,7 +153,7 @@ public Transaction[] GetTransactions() => Transactions /// RLP-encodes and sets the transactions specified to . /// /// An array of transactions to encode. - public void SetTransactions(params Transaction[] transactions) => Transactions = transactions + public void SetTransactions(params Transaction[] transactions) => Transactions = (_transactions = transactions) .Select(t => Rlp.Encode(t, RlpBehaviors.SkipTypedWrapping).Bytes) .ToArray(); From 240e58728442a5191d2c66ce96fd5f7baa245a56 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 7 Jun 2023 12:00:57 +0200 Subject: [PATCH 2/4] Simplify and refactor validation --- .../Nethermind.Core/Extensions/Bytes.cs | 14 +++++ .../EngineRpcModule.Cancun.cs | 60 +++++-------------- 2 files changed, 28 insertions(+), 46 deletions(-) diff --git a/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs b/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs index ffcd75a0153b..d9d04f10fa47 100644 --- a/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs +++ b/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs @@ -24,6 +24,7 @@ namespace Nethermind.Core.Extensions public static unsafe partial class Bytes { public static readonly IEqualityComparer EqualityComparer = new BytesEqualityComparer(); + public static readonly IEqualityComparer NullableEqualityComparer = new NullableBytesEqualityComparer(); public static readonly ISpanEqualityComparer SpanEqualityComparer = new SpanBytesEqualityComparer(); public static readonly BytesComparer Comparer = new(); @@ -40,6 +41,19 @@ public override int GetHashCode(byte[] obj) } } + private class NullableBytesEqualityComparer : EqualityComparer + { + public override bool Equals(byte[]? x, byte[]? y) + { + return AreEqual(x, y); + } + + public override int GetHashCode(byte[]? obj) + { + return obj?.GetSimplifiedHashCode() ?? 0; + } + } + private class SpanBytesEqualityComparer : ISpanEqualityComparer { public bool Equals(ReadOnlySpan x, ReadOnlySpan y) => AreEqual(x, y); diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs index ba91eb877291..0e99f729adc2 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs @@ -1,9 +1,11 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Nethermind.Core; +using Nethermind.Core.Extensions; using Nethermind.JsonRpc; using Nethermind.Merge.Plugin.Data; using Nethermind.Merge.Plugin.Handlers; @@ -14,11 +16,13 @@ public partial class EngineRpcModule : IEngineRpcModule { private readonly IAsyncHandler _getPayloadHandlerV3; - public Task> engine_newPayloadV3(ExecutionPayload executionPayload, byte[][]? blobVersionedHashes = null) + public Task> engine_newPayloadV3(ExecutionPayload executionPayload, byte[]?[]? blobVersionedHashes = null) => + Validate(executionPayload, blobVersionedHashes) ?? NewPayload(executionPayload, 3); + + private ResultWrapper? Validate(ExecutionPayload executionPayload, byte[]?[]? blobVersionedHashes) { - if (blobVersionedHashes is null) + ResultWrapper ErrorResult(string error) { - string error = "Blob versioned hashes must be set"; if (_logger.IsWarn) _logger.Warn(error); return ResultWrapper.Success( new PayloadStatusV1 @@ -29,50 +33,14 @@ public Task> engine_newPayloadV3(ExecutionPayload }); } - int index = 0; - - foreach (Transaction tx in executionPayload.GetTransactions()) - { - if (!tx.SupportsBlobs || tx.BlobVersionedHashes is null) - { - continue; - } - - foreach (byte[]? blobVersionedHash in tx.BlobVersionedHashes) - { - if (index == blobVersionedHashes.Length - || blobVersionedHash is null - || blobVersionedHashes[index] is null - || !blobVersionedHash.SequenceEqual(blobVersionedHashes[index])) - { - string error = "Blob versioned hashes do not match"; - if (_logger.IsWarn) _logger.Warn(error); - return ResultWrapper.Success( - new PayloadStatusV1 - { - Status = PayloadStatus.Invalid, - LatestValidHash = null, - ValidationError = error - }); - } - index++; - } - } - - if (index != blobVersionedHashes.Length) - { - string error = "Blob versioned hashes do not match"; - if (_logger.IsWarn) _logger.Warn(error); - return ResultWrapper.Success( - new PayloadStatusV1 - { - Status = PayloadStatus.Invalid, - LatestValidHash = null, - ValidationError = error - }); - } + static IEnumerable FlattenHashesFromTransactions(ExecutionPayload payload) => + payload.GetTransactions() + .Where(t => t.BlobVersionedHashes is not null) + .SelectMany(t => t.BlobVersionedHashes!); - return NewPayload(executionPayload, 3); + return blobVersionedHashes is null ? ErrorResult("Blob versioned hashes must be set") + : !FlattenHashesFromTransactions(executionPayload).SequenceEqual(blobVersionedHashes, Bytes.NullableEqualityComparer) ? ErrorResult("Blob versioned hashes do not match") + : null; } public async Task> engine_getPayloadV3(byte[] payloadId) => From bf62734aa3a3179a58623b9079c19a33c2549afe Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 7 Jun 2023 12:10:17 +0200 Subject: [PATCH 3/4] Add test --- src/Nethermind/Nethermind.Core.Test/BytesTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Nethermind/Nethermind.Core.Test/BytesTests.cs b/src/Nethermind/Nethermind.Core.Test/BytesTests.cs index 1c961f842a8a..98f00b09c9f6 100644 --- a/src/Nethermind/Nethermind.Core.Test/BytesTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/BytesTests.cs @@ -359,5 +359,11 @@ public void Or(byte[] first, byte[] second, byte[] expected) first.AsSpan().Or(second); first.Should().Equal(expected); } + + [Test] + public void NullableComparision() + { + Bytes.NullableEqualityComparer.Equals(null, null).Should().BeTrue(); + } } } From 4e56ab9205c01bb13520027ffefb9cd0504d8737 Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Fri, 9 Jun 2023 10:24:39 +0300 Subject: [PATCH 4/4] Uglify to comply --- .../Data/ExecutionPayload.cs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index 5cea6490f9e4..c541f003b926 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -70,12 +70,22 @@ public ExecutionPayload(Block block) public ulong Timestamp { get; set; } + private byte[][] _encodedTransactions = Array.Empty(); + /// /// Gets or sets an array of RLP-encoded transaction where each item is a byte list (data) /// representing TransactionType || TransactionPayload or LegacyTransaction as defined in /// EIP-2718. /// - public byte[][] Transactions { get; set; } = Array.Empty(); + public byte[][] Transactions + { + get { return _encodedTransactions; } + set + { + _encodedTransactions = value; + _transactions = null; + } + } /// /// Gets or sets a collection of as defined in @@ -153,9 +163,13 @@ public Transaction[] GetTransactions() => _transactions ??= Transactions /// RLP-encodes and sets the transactions specified to . /// /// An array of transactions to encode. - public void SetTransactions(params Transaction[] transactions) => Transactions = (_transactions = transactions) - .Select(t => Rlp.Encode(t, RlpBehaviors.SkipTypedWrapping).Bytes) - .ToArray(); + public void SetTransactions(params Transaction[] transactions) + { + Transactions = transactions + .Select(t => Rlp.Encode(t, RlpBehaviors.SkipTypedWrapping).Bytes) + .ToArray(); + _transactions = transactions; + } public override string ToString() => $"{BlockNumber} ({BlockHash})"; }