Introduce ExecutionPayloadParams concept to be able to better compose… (

#5932)

* Introduce ExecutionPayloadParams concept to be able to better compose and validate complex method signatures

* whitespace

* fix CreateBlockRequest

* Revert "fix CreateBlockRequest"

This reverts commit 21c72e4.

* fix

* fixes

* fix

* fix whitespace

* support V1 error codes

src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs

@@ -18,7 +18,7 @@ namespace Nethermind.Merge.Plugin.Data;
 /// <summary>
 /// Represents an object mapping the <c>ExecutionPayload</c> structure of the beacon chain spec.
 /// </summary>
-public class ExecutionPayload : IForkValidator
+public class ExecutionPayload : IForkValidator, IExecutionPayloadParams
 {
     public ExecutionPayload() { } // Needed for tests
 
@@ -165,14 +165,16 @@ public void SetTransactions(params Transaction[] transactions)
 
     public override string ToString() => $"{BlockNumber} ({BlockHash.ToShortString()})";
 
-    public virtual bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error)
+    ExecutionPayload IExecutionPayloadParams.ExecutionPayload => this;
+
+    public virtual ValidationResult ValidateParams(IReleaseSpec spec, int version, out string? error)
     {
         int GetVersion() => Withdrawals is null ? 1 : 2;
 
         if (spec.IsEip4844Enabled)
         {
             error = "ExecutionPayloadV3 expected";
-            return false;
+            return ValidationResult.Fail;
         }
 
         int actualVersion = GetVersion();
@@ -184,12 +186,9 @@ public virtual bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(
             _ => actualVersion > version ? $"ExecutionPayloadV{version} expected" : null
         };
 
-        return error is null;
+        return error is null ? ValidationResult.Success : ValidationResult.Fail;
     }
 
     public virtual bool ValidateFork(ISpecProvider specProvider) =>
         !specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled;
-
-    public bool ValidateParams(ISpecProvider specProvider, int version, [NotNullWhen(false)] out string? error) =>
-            ValidateParams(specProvider.GetSpec(BlockNumber, Timestamp), version, out error);
 }

src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs

@@ -47,13 +47,6 @@ public override bool TryGetBlock(out Block? block, UInt256? totalDifficulty = nu
         return true;
     }
 
-    public override bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error)
-    {
-        // handled by `JsonObject` attribute of the class
-        error = null;
-        return true;
-    }
-
     public override bool ValidateFork(ISpecProvider specProvider) =>
         specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled;
 }

src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs

@@ -0,0 +1,49 @@
+// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited
+// SPDX-License-Identifier: LGPL-3.0-only
+
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using Nethermind.Core.Extensions;
+using Nethermind.Core.Specs;
+
+namespace Nethermind.Merge.Plugin.Data;
+
+public interface IExecutionPayloadParams
+{
+    ExecutionPayload ExecutionPayload { get; }
+    ValidationResult ValidateParams(IReleaseSpec spec, int version, out string? error);
+}
+
+public enum ValidationResult : byte { Success, Fail, Invalid };
+
+public class ExecutionPayloadV3Params : IExecutionPayloadParams
+{
+    private readonly ExecutionPayloadV3 _executionPayload;
+    private readonly byte[]?[] _blobVersionedHashes;
+
+    public ExecutionPayloadV3Params(ExecutionPayloadV3 executionPayload, byte[]?[] blobVersionedHashes)
+    {
+        _executionPayload = executionPayload;
+        _blobVersionedHashes = blobVersionedHashes;
+    }
+
+    public ExecutionPayload ExecutionPayload => _executionPayload;
+
+    public ValidationResult ValidateParams(IReleaseSpec spec, int version, out string? error)
+    {
+        static IEnumerable<byte[]?> FlattenHashesFromTransactions(ExecutionPayloadV3 payload) =>
+            payload.GetTransactions()
+                .Where(t => t.BlobVersionedHashes is not null)
+                .SelectMany(t => t.BlobVersionedHashes!);
+
+        if (FlattenHashesFromTransactions(_executionPayload).SequenceEqual(_blobVersionedHashes, Bytes.NullableEqualityComparer))
+        {
+            error = null;
+            return ValidationResult.Success;
+        }
+
+        error = "Blob versioned hashes do not match";
+        return ValidationResult.Invalid;
+    }
+}

src/Nethermind/Nethermind.Merge.Plugin/Data/PayloadStatusV1.cs

@@ -17,10 +17,11 @@ public class PayloadStatusV1
 
         public static readonly PayloadStatusV1 Accepted = new() { Status = PayloadStatus.Accepted };
 
-        public static PayloadStatusV1 Invalid(Keccak? latestValidHash) => new()
+        public static PayloadStatusV1 Invalid(Keccak? latestValidHash, string? validationError = null) => new()
         {
             Status = PayloadStatus.Invalid,
-            LatestValidHash = latestValidHash
+            LatestValidHash = latestValidHash,
+            ValidationError = validationError
         };
 
         /// <summary>

src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs

@@ -17,44 +17,7 @@ public partial class EngineRpcModule : IEngineRpcModule
     private readonly IAsyncHandler<byte[], GetPayloadV3Result?> _getPayloadHandlerV3;
 
     public Task<ResultWrapper<PayloadStatusV1>> engine_newPayloadV3(ExecutionPayloadV3 executionPayload, byte[]?[] blobVersionedHashes) =>
-        ValidateFork(executionPayload) ?? PreValidatePayload(executionPayload, blobVersionedHashes) ?? NewPayload(executionPayload, 3);
-
-    private ResultWrapper<PayloadStatusV1>? PreValidatePayload(ExecutionPayloadV3 executionPayload, byte[]?[] blobVersionedHashes)
-    {
-        ResultWrapper<PayloadStatusV1> ErrorResult(string error)
-        {
-            if (_logger.IsWarn) _logger.Warn(error);
-            return ResultWrapper<PayloadStatusV1>.Success(
-                new PayloadStatusV1
-                {
-                    Status = PayloadStatus.Invalid,
-                    LatestValidHash = null,
-                    ValidationError = error
-                });
-        }
-
-        static IEnumerable<byte[]?> FlattenHashesFromTransactions(ExecutionPayloadV3 payload) =>
-            payload.GetTransactions()
-                .Where(t => t.BlobVersionedHashes is not null)
-                .SelectMany(t => t.BlobVersionedHashes!);
-
-        return !FlattenHashesFromTransactions(executionPayload).SequenceEqual(blobVersionedHashes, Bytes.NullableEqualityComparer)
-            ? ErrorResult("Blob versioned hashes do not match")
-            : null;
-    }
-
-    private ResultWrapper<PayloadStatusV1>? ValidateFork(ExecutionPayload executionPayload)
-    {
-        if (executionPayload.ValidateFork(_specProvider))
-        {
-            return null;
-        }
-        else
-        {
-            if (_logger.IsWarn) _logger.Warn($"The payload is not supported by the current fork");
-            return ResultWrapper<PayloadStatusV1>.Fail("unsupported fork", ErrorCodes.UnsupportedFork);
-        }
-    }
+        NewPayload(new ExecutionPayloadV3Params(executionPayload, blobVersionedHashes), 3);
 
     public async Task<ResultWrapper<GetPayloadV3Result?>> engine_getPayloadV3(byte[] payloadId) =>
         await _getPayloadHandlerV3.HandleAsync(payloadId);

src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs

@@ -6,6 +6,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Nethermind.Consensus.Producers;
+using Nethermind.Core.Specs;
 using Nethermind.JsonRpc;
 using Nethermind.Merge.Plugin.Data;
 using Nethermind.Merge.Plugin.GC;
@@ -64,12 +65,24 @@ private async Task<ResultWrapper<ForkchoiceUpdatedV1Result>> ForkchoiceUpdated(F
         }
     }
 
-    private async Task<ResultWrapper<PayloadStatusV1>> NewPayload(ExecutionPayload executionPayload, int version)
+    private async Task<ResultWrapper<PayloadStatusV1>> NewPayload(IExecutionPayloadParams executionPayloadParams, int version)
     {
-        if (!executionPayload.ValidateParams(_specProvider, version, out string? error))
+        ExecutionPayload executionPayload = executionPayloadParams.ExecutionPayload;
+
+        if (!executionPayload.ValidateFork(_specProvider))
+        {
+            if (_logger.IsWarn) _logger.Warn($"The payload is not supported by the current fork");
+            return ResultWrapper<PayloadStatusV1>.Fail("unsupported fork", version < 2 ? ErrorCodes.InvalidParams : ErrorCodes.UnsupportedFork);
+        }
+
+        IReleaseSpec releaseSpec = _specProvider.GetSpec(executionPayload.BlockNumber, executionPayload.Timestamp);
+        ValidationResult validationResult = executionPayloadParams.ValidateParams(releaseSpec, version, out string? error);
+        if (validationResult != ValidationResult.Success)
         {
             if (_logger.IsWarn) _logger.Warn(error);
-            return ResultWrapper<PayloadStatusV1>.Fail(error, ErrorCodes.InvalidParams);
+            return validationResult == ValidationResult.Fail
+                ? ResultWrapper<PayloadStatusV1>.Fail(error!, ErrorCodes.InvalidParams)
+                : ResultWrapper<PayloadStatusV1>.Success(PayloadStatusV1.Invalid(null, error));
         }
 
         if (await _locker.WaitAsync(_timeout))

src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Shanghai.cs

@@ -30,5 +30,5 @@ public Task<ResultWrapper<ForkchoiceUpdatedV1Result>> engine_forkchoiceUpdatedV2
         => _executionGetPayloadBodiesByRangeV1Handler.Handle(start, count);
 
     public Task<ResultWrapper<PayloadStatusV1>> engine_newPayloadV2(ExecutionPayload executionPayload)
-        => ValidateFork(executionPayload) ?? NewPayload(executionPayload, 2);
+        => NewPayload(executionPayload, 2);
 }