From f045e2308b7fefe10cbed821d17c4c1d8274cc8f Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 13:23:43 -0300 Subject: [PATCH 01/25] Add default interface implementation - Reduce burden on implementators --- src/Nethermind/Nethermind.TxPool/ITxValidator.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Nethermind/Nethermind.TxPool/ITxValidator.cs b/src/Nethermind/Nethermind.TxPool/ITxValidator.cs index 1510c9a165a..bec156f83b1 100644 --- a/src/Nethermind/Nethermind.TxPool/ITxValidator.cs +++ b/src/Nethermind/Nethermind.TxPool/ITxValidator.cs @@ -9,6 +9,7 @@ namespace Nethermind.TxPool { public interface ITxValidator { + bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => IsWellFormed(transaction, releaseSpec, out _); bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error); } } From 6eeec8168e9f1753be310cd0c703445a2f40b190 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 15:30:15 -0300 Subject: [PATCH 02/25] Separate all validators - Construct per TxType validator --- .../Validators/TxValidator.cs | 343 ++++------------ .../Validators/TxValidators.cs | 380 ++++++++++++++++++ 2 files changed, 450 insertions(+), 273 deletions(-) create mode 100644 src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 409f7c32a6d..157488de837 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -1,295 +1,92 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only -using System; +using System.Collections.Generic; using Nethermind.Consensus.Messages; using Nethermind.Core; -using Nethermind.Core.Crypto; -using Nethermind.Core.Extensions; using Nethermind.Core.Specs; -using Nethermind.Crypto; -using Nethermind.Evm; -using Nethermind.Int256; using Nethermind.TxPool; -namespace Nethermind.Consensus.Validators -{ - public class TxValidator : ITxValidator - { - private readonly ulong _chainIdValue; - - public TxValidator(ulong chainId) - { - _chainIdValue = chainId; - } - - /* Full and correct validation is only possible in the context of a specific block - as we cannot generalize correctness of the transaction without knowing the EIPs implemented - and the world state (account nonce in particular ). - Even without protocol change the tx can become invalid if another tx - from the same account with the same nonce got included on the chain. - As such we can decide whether tx is well formed but we also have to validate nonce - just before the execution of the block / tx. */ - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) - { - return IsWellFormed(transaction, releaseSpec, out _); - } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? error) - { - error = null; +namespace Nethermind.Consensus.Validators; - // validate type before calculating intrinsic gas to avoid exception - return ValidateTxType(transaction, releaseSpec, ref error) - // This is unnecessarily calculated twice - at validation and execution times. - && ValidateWithError(transaction.GasLimit >= IntrinsicGasCalculator.Calculate(transaction, releaseSpec), TxErrorMessages.IntrinsicGasTooLow, ref error) - // if it is a call or a transfer then we require the 'To' field to have a value while for an init it will be empty - && ValidateWithError(ValidateSignature(transaction, releaseSpec), TxErrorMessages.InvalidTxSignature, ref error) - && ValidateChainId(transaction, ref error) - && ValidateWithError(Validate1559GasFields(transaction, releaseSpec), TxErrorMessages.InvalidMaxPriorityFeePerGas, ref error) - && ValidateWithError(Validate3860Rules(transaction, releaseSpec), TxErrorMessages.ContractSizeTooBig, ref error) - && Validate4844Fields(transaction, ref error); - } - - private static bool Validate3860Rules(Transaction transaction, IReleaseSpec releaseSpec) => - !transaction.IsAboveInitCode(releaseSpec); +public sealed class TxValidator : ITxValidator +{ + private readonly Dictionary _validators; - private static bool ValidateTxType(Transaction transaction, IReleaseSpec releaseSpec, ref string error) + public TxValidator(ulong chainId, Dictionary? validators = null) + { + _validators = new() { - bool result = transaction.Type switch - { - TxType.Legacy => true, - TxType.AccessList => releaseSpec.UseTxAccessLists, - TxType.EIP1559 => releaseSpec.IsEip1559Enabled, - TxType.Blob => releaseSpec.IsEip4844Enabled, - _ => false - }; - - if (!result) { - error = TxErrorMessages.InvalidTxType(releaseSpec.Name); - return false; - } - - return true; - } - - - private static bool Validate1559GasFields(Transaction transaction, IReleaseSpec releaseSpec) + TxType.Legacy, new AllTxValidator([ + new IntrinsicGasTxValidator(), + new LegacySignatureTxValidator(chainId), + new ContractSizeTxValidator(), + new NonBlobFieldsTxValidator(), + ]) + }, + { + TxType.AccessList, new AllTxValidator([ + new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), + new IntrinsicGasTxValidator(), + new SignatureTxValidator(), + new ExpectedChainIdTxValidator(chainId), + new ContractSizeTxValidator(), + new NonBlobFieldsTxValidator(), + ]) + }, + { + TxType.EIP1559, new AllTxValidator([ + new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), + new IntrinsicGasTxValidator(), + new SignatureTxValidator(), + new ExpectedChainIdTxValidator(chainId), + new GasFieldsTxValidator(), + new ContractSizeTxValidator(), + new NonBlobFieldsTxValidator(), + ]) + }, + { + TxType.Blob, new AllTxValidator([ + new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), + new IntrinsicGasTxValidator(), + new SignatureTxValidator(), + new ExpectedChainIdTxValidator(chainId), + new GasFieldsTxValidator(), + new ContractSizeTxValidator(), + new BlobFieldsTxValidator(), + new MempoolBlobTxValidator() + ]) + }, + }; + if (validators is not null) { - if (!releaseSpec.IsEip1559Enabled || !transaction.Supports1559) - return true; - - return transaction.MaxFeePerGas >= transaction.MaxPriorityFeePerGas; - } - - private bool ValidateChainId(Transaction transaction, ref string? error) - { - return transaction.Type switch - { - TxType.Legacy => true, - _ => ValidateChainIdNonLegacy(transaction.ChainId, ref error) - }; - - bool ValidateChainIdNonLegacy(ulong? chainId, ref string? error) + foreach (var (key, value) in validators) { - bool result = chainId == _chainIdValue; - if (!result) - { - error = TxErrorMessages.InvalidTxChainId(_chainIdValue, transaction.ChainId); - return false; - } - - return true; + _validators[key] = value; } } + } - private bool ValidateWithError(bool validation, string errorMessage, ref string? error) + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => IsWellFormed(transaction, releaseSpec, out _); + + /// + /// Full and correct validation is only possible in the context of a specific block + /// as we cannot generalize correctness of the transaction without knowing the EIPs implemented + /// and the world state(account nonce in particular). + /// Even without protocol change, the tx can become invalid if another tx + /// from the same account with the same nonce got included on the chain. + /// As such, we can decide whether tx is well formed as long as we also validate nonce + /// just before the execution of the block / tx. + /// + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? error) + { + if (!_validators.TryGetValue(transaction.Type, out ITxValidator? validator)) { - if (!validation) - { - error = errorMessage; - return false; - } - - return true; + error = TxErrorMessages.InvalidTxType(releaseSpec.Name); + return false; } - private bool ValidateSignature(Transaction tx, IReleaseSpec spec) - { - Signature? signature = tx.Signature; - - if (signature is null) - { - return false; - } - - UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); - UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); - - if (sValue.IsZero || sValue >= (spec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) - { - return false; - } - - if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) - { - return false; - } - - if (signature.V is 27 or 28) - { - return true; - } - - if (tx.Type == TxType.Legacy && spec.IsEip155Enabled && (signature.V == _chainIdValue * 2 + 35ul || signature.V == _chainIdValue * 2 + 36ul)) - { - return true; - } - - return !spec.ValidateChainId; - } - - private static bool Validate4844Fields(Transaction transaction, ref string? error) - { - // Execution-payload version verification - if (!transaction.SupportsBlobs) - { - if (transaction.MaxFeePerBlobGas is not null) - { - error = TxErrorMessages.NotAllowedMaxFeePerBlobGas; - return false; - } - - if (transaction.BlobVersionedHashes is not null) - { - error = TxErrorMessages.NotAllowedBlobVersionedHashes; - return false; - } - - if (transaction is { NetworkWrapper: ShardBlobNetworkWrapper }) - { - //This must be an internal issue? - error = TxErrorMessages.InvalidTransaction; - return false; - } - - return true; - } - - if (transaction.To is null) - { - error = TxErrorMessages.TxMissingTo; - return false; - } - - if (transaction.MaxFeePerBlobGas is null) - { - error = TxErrorMessages.BlobTxMissingMaxFeePerBlobGas; - return false; - } - - if (transaction.BlobVersionedHashes is null) - { - error = TxErrorMessages.BlobTxMissingBlobVersionedHashes; - return false; - } - - int blobCount = transaction.BlobVersionedHashes.Length; - ulong totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount); - if (totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction) - { - error = TxErrorMessages.BlobTxGasLimitExceeded; - return false; - } - - if (blobCount < Eip4844Constants.MinBlobsPerTransaction) - { - error = TxErrorMessages.BlobTxMissingBlobs; - return false; - } - - for (int i = 0; i < blobCount; i++) - { - if (transaction.BlobVersionedHashes[i] is null) - { - error = TxErrorMessages.MissingBlobVersionedHash; - return false; - } - - if (transaction.BlobVersionedHashes[i].Length != KzgPolynomialCommitments.BytesPerBlobVersionedHash) - { - error = TxErrorMessages.InvalidBlobVersionedHashSize; - return false; - } - - if (transaction.BlobVersionedHashes[i][0] != KzgPolynomialCommitments.KzgBlobHashVersionV1) - { - error = TxErrorMessages.InvalidBlobVersionedHashVersion; - return false; - } - } - - // Mempool version verification if presents - if (transaction.NetworkWrapper is ShardBlobNetworkWrapper wrapper) - { - if (wrapper.Blobs.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - if (wrapper.Commitments.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - if (wrapper.Proofs.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - for (int i = 0; i < blobCount; i++) - { - if (wrapper.Blobs[i].Length != Ckzg.Ckzg.BytesPerBlob) - { - error = TxErrorMessages.ExceededBlobSize; - return false; - } - if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) - { - error = TxErrorMessages.ExceededBlobCommitmentSize; - return false; - } - if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) - { - error = TxErrorMessages.InvalidBlobProofSize; - return false; - } - } - - Span hash = stackalloc byte[32]; - for (int i = 0; i < blobCount; i++) - { - if (!KzgPolynomialCommitments.TryComputeCommitmentHashV1( - wrapper.Commitments[i].AsSpan(), hash) || - !hash.SequenceEqual(transaction.BlobVersionedHashes[i])) - { - error = TxErrorMessages.InvalidBlobCommitmentHash; - return false; - } - } - - if (!KzgPolynomialCommitments.AreProofsValid(wrapper.Blobs, wrapper.Commitments, wrapper.Proofs)) - { - error = TxErrorMessages.InvalidBlobProof; - return false; - } - - } - - return true; - } + return validator.IsWellFormed(transaction, releaseSpec, out error); } } diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs new file mode 100644 index 00000000000..3ea8b2257f3 --- /dev/null +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -0,0 +1,380 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Nethermind.Consensus.Messages; +using Nethermind.Core; +using Nethermind.Core.Crypto; +using Nethermind.Core.Specs; +using Nethermind.Crypto; +using Nethermind.Evm; +using Nethermind.Int256; +using Nethermind.TxPool; + +namespace Nethermind.Consensus.Validators; + +public sealed class AllTxValidator(List validators) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + foreach (ITxValidator validator in validators) + { + if (!validator.IsWellFormed(transaction, releaseSpec, out error)) + { + return false; + } + } + + return true; + } +} + +public sealed class IntrinsicGasTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + // This is unnecessarily calculated twice - at validation and execution times. + var intrinsicGas = IntrinsicGasCalculator.Calculate(transaction, releaseSpec); + if (transaction.GasLimit < intrinsicGas) + { + error = TxErrorMessages.IntrinsicGasTooLow; + return false; + } + + return true; + } +} + +public sealed class ReleaseSpecTxValidator(Func validate) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (!validate(releaseSpec)) + { + error = TxErrorMessages.InvalidTxType(releaseSpec.Name); + return false; + } + + return true; + } +} + +public sealed class ExpectedChainIdTxValidator(ulong chainId) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (transaction.ChainId != chainId) + { + error = TxErrorMessages.InvalidTxChainId(chainId, transaction.ChainId); + return false; + } + + return true; + } +} + +public sealed class GasFieldsTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + if (!releaseSpec.IsEip1559Enabled) + { + return true; + } + + if (transaction.MaxFeePerGas < transaction.MaxPriorityFeePerGas) + { + error = TxErrorMessages.InvalidMaxPriorityFeePerGas; + return false; + } + + return true; + } +} + +public sealed class ContractSizeTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + if (transaction.IsAboveInitCode(releaseSpec)) + { + error = TxErrorMessages.ContractSizeTooBig; + return false; + } + + return true; + } +} + +/// +/// Ensure that non Blob transactions do not contain Blob specific fields. +/// This validator will be deprecated once we have a proper Transaction type hierarchy. +/// +public sealed class NonBlobFieldsTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + // Execution-payload version verification + if (transaction.MaxFeePerBlobGas is not null) + { + error = TxErrorMessages.NotAllowedMaxFeePerBlobGas; + return false; + } + + if (transaction.BlobVersionedHashes is not null) + { + error = TxErrorMessages.NotAllowedBlobVersionedHashes; + return false; + } + + if (transaction is { NetworkWrapper: ShardBlobNetworkWrapper }) + { + // NOTE: This must be an internal issue + error = TxErrorMessages.InvalidTransaction; + return false; + } + + return true; + } +} + +public sealed class BlobFieldsTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (transaction.To is null) + { + error = TxErrorMessages.TxMissingTo; + return false; + } + + if (transaction.MaxFeePerBlobGas is null) + { + error = TxErrorMessages.BlobTxMissingMaxFeePerBlobGas; + return false; + } + + if (transaction.BlobVersionedHashes is null) + { + error = TxErrorMessages.BlobTxMissingBlobVersionedHashes; + return false; + } + + var blobCount = transaction.BlobVersionedHashes.Length; + var totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount); + if (totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction) + { + error = TxErrorMessages.BlobTxGasLimitExceeded; + return false; + } + + if (blobCount < Eip4844Constants.MinBlobsPerTransaction) + { + error = TxErrorMessages.BlobTxMissingBlobs; + return false; + } + + for (int i = 0; i < blobCount; i++) + { + if (transaction.BlobVersionedHashes[i] is null) + { + error = TxErrorMessages.MissingBlobVersionedHash; + return false; + } + + if (transaction.BlobVersionedHashes[i].Length != KzgPolynomialCommitments.BytesPerBlobVersionedHash) + { + error = TxErrorMessages.InvalidBlobVersionedHashSize; + return false; + } + + if (transaction.BlobVersionedHashes[i][0] != KzgPolynomialCommitments.KzgBlobHashVersionV1) + { + error = TxErrorMessages.InvalidBlobVersionedHashVersion; + return false; + } + } + + return true; + } +} + +/// +/// Validate Blob transactions in mempool version. +/// +public sealed class MempoolBlobTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (transaction.NetworkWrapper is not ShardBlobNetworkWrapper wrapper) + { + return true; + } + + int blobCount = transaction.BlobVersionedHashes.Length; + if (wrapper.Blobs.Length != blobCount) + { + error = TxErrorMessages.InvalidBlobData; + return false; + } + + if (wrapper.Commitments.Length != blobCount) + { + error = TxErrorMessages.InvalidBlobData; + return false; + } + + if (wrapper.Proofs.Length != blobCount) + { + error = TxErrorMessages.InvalidBlobData; + return false; + } + + for (int i = 0; i < blobCount; i++) + { + if (wrapper.Blobs[i].Length != Ckzg.Ckzg.BytesPerBlob) + { + error = TxErrorMessages.ExceededBlobSize; + return false; + } + if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) + { + error = TxErrorMessages.ExceededBlobCommitmentSize; + return false; + } + if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) + { + error = TxErrorMessages.InvalidBlobProofSize; + return false; + } + } + + Span hash = stackalloc byte[32]; + for (int i = 0; i < blobCount; i++) + { + if (!KzgPolynomialCommitments.TryComputeCommitmentHashV1(wrapper.Commitments[i].AsSpan(), hash) || + !hash.SequenceEqual(transaction.BlobVersionedHashes[i])) + { + error = TxErrorMessages.InvalidBlobCommitmentHash; + return false; + } + } + + if (!KzgPolynomialCommitments.AreProofsValid(wrapper.Blobs, wrapper.Commitments, wrapper.Proofs)) + { + error = TxErrorMessages.InvalidBlobProof; + return false; + } + + return true; + } +} + +public sealed class LegacySignatureTxValidator(ulong chainId) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + Signature? signature = transaction.Signature; + if (signature is null) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); + UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); + + if (sValue.IsZero || sValue >= (releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + if (signature.V is 27 or 28) + { + return true; + } + + if (releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul)) + { + return true; + } + + if (releaseSpec.ValidateChainId) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + return true; + } +} + +public sealed class SignatureTxValidator : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + Signature? signature = transaction.Signature; + if (signature is null) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); + UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); + + if (sValue.IsZero || sValue >= (releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + if (signature.V is 27 or 28) + { + return true; + } + + if (releaseSpec.ValidateChainId) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + return true; + } +} From 6cef6ef03855fadbe3d8f02306a8451c493c645a Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 15:53:47 -0300 Subject: [PATCH 03/25] Merge signature validation - Avoid code duplication through inheritance --- .../Validators/TxValidators.cs | 52 +++++-------------- 1 file changed, 12 insertions(+), 40 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index 3ea8b2257f3..d87d2de7095 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -255,11 +255,13 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not error = TxErrorMessages.ExceededBlobSize; return false; } + if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) { error = TxErrorMessages.ExceededBlobCommitmentSize; return false; } + if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) { error = TxErrorMessages.InvalidBlobProofSize; @@ -288,8 +290,10 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not } } -public sealed class LegacySignatureTxValidator(ulong chainId) : ITxValidator +public abstract class BaseSignatureTxValidator : ITxValidator { + protected virtual bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) => false; + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; @@ -321,7 +325,7 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not return true; } - if (releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul)) + if (AdditionalCheck(transaction, releaseSpec)) { return true; } @@ -336,45 +340,13 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not } } -public sealed class SignatureTxValidator : ITxValidator +public sealed class LegacySignatureTxValidator(ulong chainId) : BaseSignatureTxValidator { - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + protected override bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) { - error = null; - - Signature? signature = transaction.Signature; - if (signature is null) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); - UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); - - if (sValue.IsZero || sValue >= (releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - if (signature.V is 27 or 28) - { - return true; - } - - if (releaseSpec.ValidateChainId) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - return true; + Signature signature = transaction.Signature; + return releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul); } } + +public sealed class SignatureTxValidator : BaseSignatureTxValidator; From a5758b84893b990dc62902415fe864bd9b8cafb7 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:26:55 -0300 Subject: [PATCH 04/25] Move to primary constructor --- .../Validators/TestTransactionValidator.cs | 40 --------- .../Validators/TxValidator.cs | 90 +++++++++---------- 2 files changed, 43 insertions(+), 87 deletions(-) delete mode 100644 src/Nethermind/Nethermind.Blockchain.Test/Validators/TestTransactionValidator.cs diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestTransactionValidator.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestTransactionValidator.cs deleted file mode 100644 index 430e5bb2e6e..00000000000 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TestTransactionValidator.cs +++ /dev/null @@ -1,40 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using Nethermind.Core; -using Nethermind.Core.Specs; -using Nethermind.TxPool; - -namespace Nethermind.Blockchain.Test.Validators -{ - public class TestTxValidator : ITxValidator - { - public static TestTxValidator AlwaysValid = new(true); - public static TestTxValidator NeverValid = new(false); - - private readonly Queue _validationResults = new(); - private readonly bool? _alwaysSameResult; - - public TestTxValidator(Queue validationResults) - { - _validationResults = validationResults; - } - - public TestTxValidator(bool validationResult) - { - _alwaysSameResult = validationResult; - } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) - { - return _alwaysSameResult ?? _validationResults.Dequeue(); - } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? errorMessage) - { - errorMessage = null; - return _alwaysSameResult ?? _validationResults.Dequeue(); - } - } -} diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 157488de837..3accda937c8 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -9,56 +9,52 @@ namespace Nethermind.Consensus.Validators; -public sealed class TxValidator : ITxValidator +public sealed class TxValidator(ulong chainId) : ITxValidator { - private readonly Dictionary _validators; - - public TxValidator(ulong chainId, Dictionary? validators = null) + private readonly Dictionary _validators = new() { - _validators = new() { - { - TxType.Legacy, new AllTxValidator([ - new IntrinsicGasTxValidator(), - new LegacySignatureTxValidator(chainId), - new ContractSizeTxValidator(), - new NonBlobFieldsTxValidator(), - ]) - }, - { - TxType.AccessList, new AllTxValidator([ - new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), - new IntrinsicGasTxValidator(), - new SignatureTxValidator(), - new ExpectedChainIdTxValidator(chainId), - new ContractSizeTxValidator(), - new NonBlobFieldsTxValidator(), - ]) - }, - { - TxType.EIP1559, new AllTxValidator([ - new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), - new IntrinsicGasTxValidator(), - new SignatureTxValidator(), - new ExpectedChainIdTxValidator(chainId), - new GasFieldsTxValidator(), - new ContractSizeTxValidator(), - new NonBlobFieldsTxValidator(), - ]) - }, - { - TxType.Blob, new AllTxValidator([ - new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), - new IntrinsicGasTxValidator(), - new SignatureTxValidator(), - new ExpectedChainIdTxValidator(chainId), - new GasFieldsTxValidator(), - new ContractSizeTxValidator(), - new BlobFieldsTxValidator(), - new MempoolBlobTxValidator() - ]) - }, - }; + TxType.Legacy, new AllTxValidator([ + new IntrinsicGasTxValidator(), + new LegacySignatureTxValidator(chainId), + new ContractSizeTxValidator(), + new NonBlobFieldsTxValidator(), + ]) + }, + { + TxType.AccessList, new AllTxValidator([ + new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), + new IntrinsicGasTxValidator(), + new SignatureTxValidator(), + new ExpectedChainIdTxValidator(chainId), + new ContractSizeTxValidator(), + new NonBlobFieldsTxValidator(), + ]) + }, + { + TxType.EIP1559, new AllTxValidator([ + new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), + new IntrinsicGasTxValidator(), + new SignatureTxValidator(), + new ExpectedChainIdTxValidator(chainId), + new GasFieldsTxValidator(), + new ContractSizeTxValidator(), + new NonBlobFieldsTxValidator(), + ]) + }, + { + TxType.Blob, new AllTxValidator([ + new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), + new IntrinsicGasTxValidator(), + new SignatureTxValidator(), + new ExpectedChainIdTxValidator(chainId), + new GasFieldsTxValidator(), + new ContractSizeTxValidator(), + new BlobFieldsTxValidator(), + new MempoolBlobTxValidator() + ]) + }, + }; if (validators is not null) { foreach (var (key, value) in validators) From cb523bdc5d47934983b002e7222614a4105091d9 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:28:05 -0300 Subject: [PATCH 05/25] Update `OptimismTxValidator` - By default this validator does not check anything (existing behavior) --- .../Nethermind.Api/IApiWithBlockchain.cs | 2 +- src/Nethermind/Nethermind.Api/NethermindApi.cs | 2 +- .../Validators/TxValidator.cs | 12 +++++------- .../InitializeBlockchainOptimism.cs | 4 ++-- .../Nethermind.Optimism/OptimismTxValidator.cs | 16 +++------------- 5 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs b/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs index 496f107e7ea..4f6a49f2375 100644 --- a/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs +++ b/src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs @@ -74,7 +74,7 @@ public interface IApiWithBlockchain : IApiWithStores, IBlockchainBridgeFactory IHealthHintService? HealthHintService { get; set; } IRpcCapabilitiesProvider? RpcCapabilitiesProvider { get; set; } ITransactionComparerProvider? TransactionComparerProvider { get; set; } - ITxValidator? TxValidator { get; set; } + TxValidator? TxValidator { get; set; } /// /// Manager of block finalization diff --git a/src/Nethermind/Nethermind.Api/NethermindApi.cs b/src/Nethermind/Nethermind.Api/NethermindApi.cs index e07aa13ba9e..e9f7588ea36 100644 --- a/src/Nethermind/Nethermind.Api/NethermindApi.cs +++ b/src/Nethermind/Nethermind.Api/NethermindApi.cs @@ -208,7 +208,7 @@ public ISealEngine SealEngine public ITxPoolInfoProvider? TxPoolInfoProvider { get; set; } public IHealthHintService? HealthHintService { get; set; } public IRpcCapabilitiesProvider? RpcCapabilitiesProvider { get; set; } - public ITxValidator? TxValidator { get; set; } + public TxValidator? TxValidator { get; set; } public IBlockFinalizationManager? FinalizationManager { get; set; } public IGasLimitCalculator? GasLimitCalculator { get; set; } diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 3accda937c8..5824360bd1d 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -55,13 +55,11 @@ public sealed class TxValidator(ulong chainId) : ITxValidator ]) }, }; - if (validators is not null) - { - foreach (var (key, value) in validators) - { - _validators[key] = value; - } - } + + public TxValidator AddValidator(TxType type, ITxValidator validator) + { + _validators[type] = validator; + return this; } public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => IsWellFormed(transaction, releaseSpec, out _); diff --git a/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs b/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs index 6c42963e032..8f7f128bbd9 100644 --- a/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs +++ b/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs @@ -10,6 +10,7 @@ using Nethermind.Consensus.Producers; using Nethermind.Consensus.Validators; using Nethermind.Consensus.Withdrawals; +using Nethermind.Core; using Nethermind.Evm; using Nethermind.Evm.TransactionProcessing; using Nethermind.Init.Steps; @@ -72,9 +73,8 @@ protected override IBlockValidator CreateBlockValidator() if (_api.InvalidChainTracker is null) throw new StepDependencyException(nameof(_api.InvalidChainTracker)); if (_api.TxValidator is null) throw new StepDependencyException(nameof(_api.TxValidator)); - OptimismTxValidator txValidator = new(_api.TxValidator); BlockValidator blockValidator = new( - txValidator, + _api.TxValidator.AddValidator(TxType.DepositTx, new OptimismTxValidator()), _api.HeaderValidator, _api.UnclesValidator, _api.SpecProvider, diff --git a/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs b/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs index c0653b7aa70..501cd4fd19d 100644 --- a/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs @@ -1,28 +1,18 @@ // SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System.Diagnostics.CodeAnalysis; using Nethermind.Core; using Nethermind.Core.Specs; using Nethermind.TxPool; -using System.Diagnostics.CodeAnalysis; namespace Nethermind.Optimism; -public class OptimismTxValidator : ITxValidator +public sealed class OptimismTxValidator : ITxValidator { - private readonly ITxValidator _txValidator; - - public OptimismTxValidator(ITxValidator txValidator) - { - _txValidator = txValidator; - } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => - IsWellFormed(transaction, releaseSpec, out _); public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; - return transaction.Type == TxType.DepositTx || _txValidator.IsWellFormed(transaction, releaseSpec, out error); + return true; } - } From f6f0c922cdd02e97891cd24ec60f7e69996071d3 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:32:23 -0300 Subject: [PATCH 06/25] Reorder imports (avoid diff) --- src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs b/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs index 501cd4fd19d..9a6b38d87cb 100644 --- a/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs @@ -1,10 +1,10 @@ // SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only -using System.Diagnostics.CodeAnalysis; using Nethermind.Core; using Nethermind.Core.Specs; using Nethermind.TxPool; +using System.Diagnostics.CodeAnalysis; namespace Nethermind.Optimism; From 7d85f1e3cbc819cea026715b9ae5c4e3013128de Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:39:14 -0300 Subject: [PATCH 07/25] `IntrinsicGasTxValidator`: Use singleton - Avoid allocations --- .../Nethermind.Consensus/Validators/TxValidator.cs | 8 ++++---- .../Nethermind.Consensus/Validators/TxValidators.cs | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 5824360bd1d..075b21fb3fc 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -15,7 +15,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator { { TxType.Legacy, new AllTxValidator([ - new IntrinsicGasTxValidator(), + IntrinsicGasTxValidator.Instance, new LegacySignatureTxValidator(chainId), new ContractSizeTxValidator(), new NonBlobFieldsTxValidator(), @@ -24,7 +24,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator { TxType.AccessList, new AllTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), - new IntrinsicGasTxValidator(), + IntrinsicGasTxValidator.Instance, new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), new ContractSizeTxValidator(), @@ -34,7 +34,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator { TxType.EIP1559, new AllTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), - new IntrinsicGasTxValidator(), + IntrinsicGasTxValidator.Instance, new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), @@ -45,7 +45,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator { TxType.Blob, new AllTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), - new IntrinsicGasTxValidator(), + IntrinsicGasTxValidator.Instance, new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index d87d2de7095..4ad92baee7c 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -35,6 +35,9 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not public sealed class IntrinsicGasTxValidator : ITxValidator { + public static readonly IntrinsicGasTxValidator Instance = new(); + private IntrinsicGasTxValidator() { } + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; From 17ff03f39a7769be05b1ed97a7f396f2c2798a07 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:40:32 -0300 Subject: [PATCH 08/25] `ContractSizeTxValidator`: Use singleton - Avoid allocations --- .../Nethermind.Consensus/Validators/TxValidator.cs | 8 ++++---- .../Nethermind.Consensus/Validators/TxValidators.cs | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 075b21fb3fc..ab2b540b0c1 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -17,7 +17,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator TxType.Legacy, new AllTxValidator([ IntrinsicGasTxValidator.Instance, new LegacySignatureTxValidator(chainId), - new ContractSizeTxValidator(), + ContractSizeTxValidator.Instance, new NonBlobFieldsTxValidator(), ]) }, @@ -27,7 +27,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator IntrinsicGasTxValidator.Instance, new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), - new ContractSizeTxValidator(), + ContractSizeTxValidator.Instance, new NonBlobFieldsTxValidator(), ]) }, @@ -38,7 +38,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), - new ContractSizeTxValidator(), + ContractSizeTxValidator.Instance, new NonBlobFieldsTxValidator(), ]) }, @@ -49,7 +49,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), - new ContractSizeTxValidator(), + ContractSizeTxValidator.Instance, new BlobFieldsTxValidator(), new MempoolBlobTxValidator() ]) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index 4ad92baee7c..fd8acd422e3 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -108,6 +108,9 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not public sealed class ContractSizeTxValidator : ITxValidator { + public static readonly ContractSizeTxValidator Instance = new(); + private ContractSizeTxValidator() { } + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; From 37f626d82c208572e91b14e9d29e781a5594aeef Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:41:13 -0300 Subject: [PATCH 09/25] `NonBlobFieldsTxValidator`: Use singleton - Avoid allocations --- .../Nethermind.Consensus/Validators/TxValidator.cs | 6 +++--- .../Nethermind.Consensus/Validators/TxValidators.cs | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index ab2b540b0c1..0f15395319d 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -18,7 +18,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator IntrinsicGasTxValidator.Instance, new LegacySignatureTxValidator(chainId), ContractSizeTxValidator.Instance, - new NonBlobFieldsTxValidator(), + NonBlobFieldsTxValidator.Instance, ]) }, { @@ -28,7 +28,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator new SignatureTxValidator(), new ExpectedChainIdTxValidator(chainId), ContractSizeTxValidator.Instance, - new NonBlobFieldsTxValidator(), + NonBlobFieldsTxValidator.Instance, ]) }, { @@ -39,7 +39,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), ContractSizeTxValidator.Instance, - new NonBlobFieldsTxValidator(), + NonBlobFieldsTxValidator.Instance, ]) }, { diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index fd8acd422e3..adc32c11050 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -130,6 +130,9 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not /// public sealed class NonBlobFieldsTxValidator : ITxValidator { + public static readonly NonBlobFieldsTxValidator Instance = new(); + private NonBlobFieldsTxValidator() { } + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; From 53f184cad882228146f34450e85f48a6b4304cc3 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:41:59 -0300 Subject: [PATCH 10/25] `SignatureTxValidator`: Use singleton - Avoid allocations --- .../Nethermind.Consensus/Validators/TxValidator.cs | 6 +++--- .../Nethermind.Consensus/Validators/TxValidators.cs | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 0f15395319d..8e5aebd9837 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -25,7 +25,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator TxType.AccessList, new AllTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), IntrinsicGasTxValidator.Instance, - new SignatureTxValidator(), + SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), ContractSizeTxValidator.Instance, NonBlobFieldsTxValidator.Instance, @@ -35,7 +35,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator TxType.EIP1559, new AllTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), IntrinsicGasTxValidator.Instance, - new SignatureTxValidator(), + SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), ContractSizeTxValidator.Instance, @@ -46,7 +46,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator TxType.Blob, new AllTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), IntrinsicGasTxValidator.Instance, - new SignatureTxValidator(), + SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), new GasFieldsTxValidator(), ContractSizeTxValidator.Instance, diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index adc32c11050..1e45123967c 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -358,4 +358,8 @@ protected override bool AdditionalCheck(Transaction transaction, IReleaseSpec re } } -public sealed class SignatureTxValidator : BaseSignatureTxValidator; +public sealed class SignatureTxValidator : BaseSignatureTxValidator +{ + public static readonly SignatureTxValidator Instance = new(); + private SignatureTxValidator() { } +} From 5f5dce289409e78fab7ea1f4e25494db484ee372 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:42:58 -0300 Subject: [PATCH 11/25] `GasFieldsTxValidator`: Use singleton - Avoid allocations --- src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs | 4 ++-- .../Nethermind.Consensus/Validators/TxValidators.cs | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 8e5aebd9837..0d3fd2b3911 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -37,7 +37,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), - new GasFieldsTxValidator(), + GasFieldsTxValidator.Instance, ContractSizeTxValidator.Instance, NonBlobFieldsTxValidator.Instance, ]) @@ -48,7 +48,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), - new GasFieldsTxValidator(), + GasFieldsTxValidator.Instance, ContractSizeTxValidator.Instance, new BlobFieldsTxValidator(), new MempoolBlobTxValidator() diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index 1e45123967c..8d3110c848f 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -88,6 +88,9 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not public sealed class GasFieldsTxValidator : ITxValidator { + public static readonly GasFieldsTxValidator Instance = new(); + private GasFieldsTxValidator() { } + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; From dd15a569db1d6436edee71f74167a1a828549e33 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Tue, 3 Sep 2024 16:44:10 -0300 Subject: [PATCH 12/25] Use singleton for BlobTxValidator internals --- .../Nethermind.Consensus/Validators/TxValidator.cs | 4 ++-- .../Nethermind.Consensus/Validators/TxValidators.cs | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 0d3fd2b3911..54802c12236 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -50,8 +50,8 @@ public sealed class TxValidator(ulong chainId) : ITxValidator new ExpectedChainIdTxValidator(chainId), GasFieldsTxValidator.Instance, ContractSizeTxValidator.Instance, - new BlobFieldsTxValidator(), - new MempoolBlobTxValidator() + BlobFieldsTxValidator.Instance, + MempoolBlobTxValidator.Instance ]) }, }; diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs index 8d3110c848f..8ea899fe448 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs @@ -166,6 +166,9 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not public sealed class BlobFieldsTxValidator : ITxValidator { + public static readonly BlobFieldsTxValidator Instance = new(); + private BlobFieldsTxValidator() { } + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; @@ -232,6 +235,9 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not /// public sealed class MempoolBlobTxValidator : ITxValidator { + public static readonly MempoolBlobTxValidator Instance = new(); + private MempoolBlobTxValidator() { } + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { error = null; From 1a95ca617dd63e2769ef6605e4f8e699c4f00393 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Wed, 4 Sep 2024 11:48:00 -0300 Subject: [PATCH 13/25] Move everything to the same file - No need to separate things --- .../Validators/TxValidator.cs | 365 +++++++++++++++++ .../Validators/TxValidators.cs | 374 ------------------ 2 files changed, 365 insertions(+), 374 deletions(-) delete mode 100644 src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 54802c12236..26a1bc6f611 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -1,11 +1,17 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System; +using System.Diagnostics.CodeAnalysis; using System.Collections.Generic; using Nethermind.Consensus.Messages; using Nethermind.Core; using Nethermind.Core.Specs; using Nethermind.TxPool; +using Nethermind.Core.Crypto; +using Nethermind.Crypto; +using Nethermind.Evm; +using Nethermind.Int256; namespace Nethermind.Consensus.Validators; @@ -84,3 +90,362 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out return validator.IsWellFormed(transaction, releaseSpec, out error); } } + +public sealed class AllTxValidator(List validators) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + foreach (ITxValidator validator in validators) + { + if (!validator.IsWellFormed(transaction, releaseSpec, out error)) + { + return false; + } + } + + return true; + } +} + +public sealed class IntrinsicGasTxValidator : ITxValidator +{ + public static readonly IntrinsicGasTxValidator Instance = new(); + private IntrinsicGasTxValidator() { } + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + // This is unnecessarily calculated twice - at validation and execution times. + var intrinsicGas = IntrinsicGasCalculator.Calculate(transaction, releaseSpec); + if (transaction.GasLimit < intrinsicGas) + { + error = TxErrorMessages.IntrinsicGasTooLow; + return false; + } + + return true; + } +} + +public sealed class ReleaseSpecTxValidator(Func validate) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (!validate(releaseSpec)) + { + error = TxErrorMessages.InvalidTxType(releaseSpec.Name); + return false; + } + + return true; + } +} + +public sealed class ExpectedChainIdTxValidator(ulong chainId) : ITxValidator +{ + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (transaction.ChainId != chainId) + { + error = TxErrorMessages.InvalidTxChainId(chainId, transaction.ChainId); + return false; + } + + return true; + } +} + +public sealed class GasFieldsTxValidator : ITxValidator +{ + public static readonly GasFieldsTxValidator Instance = new(); + private GasFieldsTxValidator() { } + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (!releaseSpec.IsEip1559Enabled) + { + return true; + } + + if (transaction.MaxFeePerGas < transaction.MaxPriorityFeePerGas) + { + error = TxErrorMessages.InvalidMaxPriorityFeePerGas; + return false; + } + + return true; + } +} + +public sealed class ContractSizeTxValidator : ITxValidator +{ + public static readonly ContractSizeTxValidator Instance = new(); + private ContractSizeTxValidator() { } + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + if (transaction.IsAboveInitCode(releaseSpec)) + { + error = TxErrorMessages.ContractSizeTooBig; + return false; + } + + return true; + } +} + +/// +/// Ensure that non Blob transactions do not contain Blob specific fields. +/// This validator will be deprecated once we have a proper Transaction type hierarchy. +/// +public sealed class NonBlobFieldsTxValidator : ITxValidator +{ + public static readonly NonBlobFieldsTxValidator Instance = new(); + private NonBlobFieldsTxValidator() { } + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + // Execution-payload version verification + if (transaction.MaxFeePerBlobGas is not null) + { + error = TxErrorMessages.NotAllowedMaxFeePerBlobGas; + return false; + } + + if (transaction.BlobVersionedHashes is not null) + { + error = TxErrorMessages.NotAllowedBlobVersionedHashes; + return false; + } + + if (transaction is { NetworkWrapper: ShardBlobNetworkWrapper }) + { + // NOTE: This must be an internal issue + error = TxErrorMessages.InvalidTransaction; + return false; + } + + return true; + } +} + +public sealed class BlobFieldsTxValidator : ITxValidator +{ + public static readonly BlobFieldsTxValidator Instance = new(); + private BlobFieldsTxValidator() { } + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (transaction.To is null) + { + error = TxErrorMessages.TxMissingTo; + return false; + } + + if (transaction.MaxFeePerBlobGas is null) + { + error = TxErrorMessages.BlobTxMissingMaxFeePerBlobGas; + return false; + } + + if (transaction.BlobVersionedHashes is null) + { + error = TxErrorMessages.BlobTxMissingBlobVersionedHashes; + return false; + } + + var blobCount = transaction.BlobVersionedHashes.Length; + var totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount); + if (totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction) + { + error = TxErrorMessages.BlobTxGasLimitExceeded; + return false; + } + + if (blobCount < Eip4844Constants.MinBlobsPerTransaction) + { + error = TxErrorMessages.BlobTxMissingBlobs; + return false; + } + + for (int i = 0; i < blobCount; i++) + { + if (transaction.BlobVersionedHashes[i] is null) + { + error = TxErrorMessages.MissingBlobVersionedHash; + return false; + } + + if (transaction.BlobVersionedHashes[i].Length != KzgPolynomialCommitments.BytesPerBlobVersionedHash) + { + error = TxErrorMessages.InvalidBlobVersionedHashSize; + return false; + } + + if (transaction.BlobVersionedHashes[i][0] != KzgPolynomialCommitments.KzgBlobHashVersionV1) + { + error = TxErrorMessages.InvalidBlobVersionedHashVersion; + return false; + } + } + + return true; + } +} + +/// +/// Validate Blob transactions in mempool version. +/// +public sealed class MempoolBlobTxValidator : ITxValidator +{ + public static readonly MempoolBlobTxValidator Instance = new(); + private MempoolBlobTxValidator() { } + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + if (transaction.NetworkWrapper is not ShardBlobNetworkWrapper wrapper) + { + return true; + } + + int blobCount = transaction.BlobVersionedHashes.Length; + if (wrapper.Blobs.Length != blobCount) + { + error = TxErrorMessages.InvalidBlobData; + return false; + } + + if (wrapper.Commitments.Length != blobCount) + { + error = TxErrorMessages.InvalidBlobData; + return false; + } + + if (wrapper.Proofs.Length != blobCount) + { + error = TxErrorMessages.InvalidBlobData; + return false; + } + + for (int i = 0; i < blobCount; i++) + { + if (wrapper.Blobs[i].Length != Ckzg.Ckzg.BytesPerBlob) + { + error = TxErrorMessages.ExceededBlobSize; + return false; + } + + if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) + { + error = TxErrorMessages.ExceededBlobCommitmentSize; + return false; + } + + if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) + { + error = TxErrorMessages.InvalidBlobProofSize; + return false; + } + } + + Span hash = stackalloc byte[32]; + for (int i = 0; i < blobCount; i++) + { + if (!KzgPolynomialCommitments.TryComputeCommitmentHashV1(wrapper.Commitments[i].AsSpan(), hash) || + !hash.SequenceEqual(transaction.BlobVersionedHashes[i])) + { + error = TxErrorMessages.InvalidBlobCommitmentHash; + return false; + } + } + + if (!KzgPolynomialCommitments.AreProofsValid(wrapper.Blobs, wrapper.Commitments, wrapper.Proofs)) + { + error = TxErrorMessages.InvalidBlobProof; + return false; + } + + return true; + } +} + +public abstract class BaseSignatureTxValidator : ITxValidator +{ + protected virtual bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) => false; + + public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + { + error = null; + + Signature? signature = transaction.Signature; + if (signature is null) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); + UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); + + if (sValue.IsZero || sValue >= (releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + if (signature.V is 27 or 28) + { + return true; + } + + if (AdditionalCheck(transaction, releaseSpec)) + { + return true; + } + + if (releaseSpec.ValidateChainId) + { + error = TxErrorMessages.InvalidTxSignature; + return false; + } + + return true; + } +} + +public sealed class LegacySignatureTxValidator(ulong chainId) : BaseSignatureTxValidator +{ + protected override bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) + { + Signature signature = transaction.Signature; + return releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul); + } +} + +public sealed class SignatureTxValidator : BaseSignatureTxValidator +{ + public static readonly SignatureTxValidator Instance = new(); + private SignatureTxValidator() { } +} diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs deleted file mode 100644 index 8ea899fe448..00000000000 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidators.cs +++ /dev/null @@ -1,374 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using Nethermind.Consensus.Messages; -using Nethermind.Core; -using Nethermind.Core.Crypto; -using Nethermind.Core.Specs; -using Nethermind.Crypto; -using Nethermind.Evm; -using Nethermind.Int256; -using Nethermind.TxPool; - -namespace Nethermind.Consensus.Validators; - -public sealed class AllTxValidator(List validators) : ITxValidator -{ - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - foreach (ITxValidator validator in validators) - { - if (!validator.IsWellFormed(transaction, releaseSpec, out error)) - { - return false; - } - } - - return true; - } -} - -public sealed class IntrinsicGasTxValidator : ITxValidator -{ - public static readonly IntrinsicGasTxValidator Instance = new(); - private IntrinsicGasTxValidator() { } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - // This is unnecessarily calculated twice - at validation and execution times. - var intrinsicGas = IntrinsicGasCalculator.Calculate(transaction, releaseSpec); - if (transaction.GasLimit < intrinsicGas) - { - error = TxErrorMessages.IntrinsicGasTooLow; - return false; - } - - return true; - } -} - -public sealed class ReleaseSpecTxValidator(Func validate) : ITxValidator -{ - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (!validate(releaseSpec)) - { - error = TxErrorMessages.InvalidTxType(releaseSpec.Name); - return false; - } - - return true; - } -} - -public sealed class ExpectedChainIdTxValidator(ulong chainId) : ITxValidator -{ - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (transaction.ChainId != chainId) - { - error = TxErrorMessages.InvalidTxChainId(chainId, transaction.ChainId); - return false; - } - - return true; - } -} - -public sealed class GasFieldsTxValidator : ITxValidator -{ - public static readonly GasFieldsTxValidator Instance = new(); - private GasFieldsTxValidator() { } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - if (!releaseSpec.IsEip1559Enabled) - { - return true; - } - - if (transaction.MaxFeePerGas < transaction.MaxPriorityFeePerGas) - { - error = TxErrorMessages.InvalidMaxPriorityFeePerGas; - return false; - } - - return true; - } -} - -public sealed class ContractSizeTxValidator : ITxValidator -{ - public static readonly ContractSizeTxValidator Instance = new(); - private ContractSizeTxValidator() { } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - if (transaction.IsAboveInitCode(releaseSpec)) - { - error = TxErrorMessages.ContractSizeTooBig; - return false; - } - - return true; - } -} - -/// -/// Ensure that non Blob transactions do not contain Blob specific fields. -/// This validator will be deprecated once we have a proper Transaction type hierarchy. -/// -public sealed class NonBlobFieldsTxValidator : ITxValidator -{ - public static readonly NonBlobFieldsTxValidator Instance = new(); - private NonBlobFieldsTxValidator() { } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - // Execution-payload version verification - if (transaction.MaxFeePerBlobGas is not null) - { - error = TxErrorMessages.NotAllowedMaxFeePerBlobGas; - return false; - } - - if (transaction.BlobVersionedHashes is not null) - { - error = TxErrorMessages.NotAllowedBlobVersionedHashes; - return false; - } - - if (transaction is { NetworkWrapper: ShardBlobNetworkWrapper }) - { - // NOTE: This must be an internal issue - error = TxErrorMessages.InvalidTransaction; - return false; - } - - return true; - } -} - -public sealed class BlobFieldsTxValidator : ITxValidator -{ - public static readonly BlobFieldsTxValidator Instance = new(); - private BlobFieldsTxValidator() { } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (transaction.To is null) - { - error = TxErrorMessages.TxMissingTo; - return false; - } - - if (transaction.MaxFeePerBlobGas is null) - { - error = TxErrorMessages.BlobTxMissingMaxFeePerBlobGas; - return false; - } - - if (transaction.BlobVersionedHashes is null) - { - error = TxErrorMessages.BlobTxMissingBlobVersionedHashes; - return false; - } - - var blobCount = transaction.BlobVersionedHashes.Length; - var totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount); - if (totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction) - { - error = TxErrorMessages.BlobTxGasLimitExceeded; - return false; - } - - if (blobCount < Eip4844Constants.MinBlobsPerTransaction) - { - error = TxErrorMessages.BlobTxMissingBlobs; - return false; - } - - for (int i = 0; i < blobCount; i++) - { - if (transaction.BlobVersionedHashes[i] is null) - { - error = TxErrorMessages.MissingBlobVersionedHash; - return false; - } - - if (transaction.BlobVersionedHashes[i].Length != KzgPolynomialCommitments.BytesPerBlobVersionedHash) - { - error = TxErrorMessages.InvalidBlobVersionedHashSize; - return false; - } - - if (transaction.BlobVersionedHashes[i][0] != KzgPolynomialCommitments.KzgBlobHashVersionV1) - { - error = TxErrorMessages.InvalidBlobVersionedHashVersion; - return false; - } - } - - return true; - } -} - -/// -/// Validate Blob transactions in mempool version. -/// -public sealed class MempoolBlobTxValidator : ITxValidator -{ - public static readonly MempoolBlobTxValidator Instance = new(); - private MempoolBlobTxValidator() { } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (transaction.NetworkWrapper is not ShardBlobNetworkWrapper wrapper) - { - return true; - } - - int blobCount = transaction.BlobVersionedHashes.Length; - if (wrapper.Blobs.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - if (wrapper.Commitments.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - if (wrapper.Proofs.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - for (int i = 0; i < blobCount; i++) - { - if (wrapper.Blobs[i].Length != Ckzg.Ckzg.BytesPerBlob) - { - error = TxErrorMessages.ExceededBlobSize; - return false; - } - - if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) - { - error = TxErrorMessages.ExceededBlobCommitmentSize; - return false; - } - - if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) - { - error = TxErrorMessages.InvalidBlobProofSize; - return false; - } - } - - Span hash = stackalloc byte[32]; - for (int i = 0; i < blobCount; i++) - { - if (!KzgPolynomialCommitments.TryComputeCommitmentHashV1(wrapper.Commitments[i].AsSpan(), hash) || - !hash.SequenceEqual(transaction.BlobVersionedHashes[i])) - { - error = TxErrorMessages.InvalidBlobCommitmentHash; - return false; - } - } - - if (!KzgPolynomialCommitments.AreProofsValid(wrapper.Blobs, wrapper.Commitments, wrapper.Proofs)) - { - error = TxErrorMessages.InvalidBlobProof; - return false; - } - - return true; - } -} - -public abstract class BaseSignatureTxValidator : ITxValidator -{ - protected virtual bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) => false; - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - Signature? signature = transaction.Signature; - if (signature is null) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); - UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); - - if (sValue.IsZero || sValue >= (releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - if (signature.V is 27 or 28) - { - return true; - } - - if (AdditionalCheck(transaction, releaseSpec)) - { - return true; - } - - if (releaseSpec.ValidateChainId) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - return true; - } -} - -public sealed class LegacySignatureTxValidator(ulong chainId) : BaseSignatureTxValidator -{ - protected override bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) - { - Signature signature = transaction.Signature; - return releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul); - } -} - -public sealed class SignatureTxValidator : BaseSignatureTxValidator -{ - public static readonly SignatureTxValidator Instance = new(); - private SignatureTxValidator() { } -} From 1430dc53821d5e8fd4f804b5b12a366e9cde5fd1 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 11:48:08 -0300 Subject: [PATCH 14/25] Rename `AddValidator` to `WithValidator` --- src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs | 2 +- .../Nethermind.Optimism/InitializeBlockchainOptimism.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 26a1bc6f611..b2de6f5c580 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -62,7 +62,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator }, }; - public TxValidator AddValidator(TxType type, ITxValidator validator) + public TxValidator WithValidator(TxType type, ITxValidator validator) { _validators[type] = validator; return this; diff --git a/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs b/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs index 8f7f128bbd9..7c90d1db0e7 100644 --- a/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs +++ b/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs @@ -74,7 +74,7 @@ protected override IBlockValidator CreateBlockValidator() if (_api.TxValidator is null) throw new StepDependencyException(nameof(_api.TxValidator)); BlockValidator blockValidator = new( - _api.TxValidator.AddValidator(TxType.DepositTx, new OptimismTxValidator()), + _api.TxValidator.WithValidator(TxType.DepositTx, new OptimismTxValidator()), _api.HeaderValidator, _api.UnclesValidator, _api.SpecProvider, From 750e3faa247786b0d9184ee86dd9cc41e3bd5bad Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 11:51:10 -0300 Subject: [PATCH 15/25] Use `` instead of `` --- .../Nethermind.Consensus/Validators/TxValidator.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index b2de6f5c580..9a6bb5222a2 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -20,7 +20,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator private readonly Dictionary _validators = new() { { - TxType.Legacy, new AllTxValidator([ + TxType.Legacy, new CompositeTxValidator([ IntrinsicGasTxValidator.Instance, new LegacySignatureTxValidator(chainId), ContractSizeTxValidator.Instance, @@ -28,7 +28,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator ]) }, { - TxType.AccessList, new AllTxValidator([ + TxType.AccessList, new CompositeTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, @@ -38,7 +38,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator ]) }, { - TxType.EIP1559, new AllTxValidator([ + TxType.EIP1559, new CompositeTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, @@ -49,7 +49,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator ]) }, { - TxType.Blob, new AllTxValidator([ + TxType.Blob, new CompositeTxValidator([ new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, @@ -70,7 +70,7 @@ public TxValidator WithValidator(TxType type, ITxValidator validator) public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => IsWellFormed(transaction, releaseSpec, out _); - /// + /// /// Full and correct validation is only possible in the context of a specific block /// as we cannot generalize correctness of the transaction without knowing the EIPs implemented /// and the world state(account nonce in particular). @@ -78,7 +78,7 @@ public TxValidator WithValidator(TxType type, ITxValidator validator) /// from the same account with the same nonce got included on the chain. /// As such, we can decide whether tx is well formed as long as we also validate nonce /// just before the execution of the block / tx. - /// + /// public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? error) { if (!_validators.TryGetValue(transaction.Type, out ITxValidator? validator)) @@ -91,7 +91,7 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out } } -public sealed class AllTxValidator(List validators) : ITxValidator +public sealed class CompositeTxValidator(List validators) : ITxValidator { public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { From 84dce880fa6da13ae7df039ce015b06e5a1b965a Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 11:51:47 -0300 Subject: [PATCH 16/25] Remove unnecessary nullability annotation --- src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 9a6bb5222a2..395284ab18a 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -81,7 +81,7 @@ public TxValidator WithValidator(TxType type, ITxValidator validator) /// public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? error) { - if (!_validators.TryGetValue(transaction.Type, out ITxValidator? validator)) + if (!_validators.TryGetValue(transaction.Type, out ITxValidator validator)) { error = TxErrorMessages.InvalidTxType(releaseSpec.Name); return false; From 86bc383b4e578ada60a30c4db14647bc64730be1 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 11:55:45 -0300 Subject: [PATCH 17/25] Rename `AdditonalCheck` to `ValidateChainId` --- .../Nethermind.Consensus/Validators/TxValidator.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 395284ab18a..9a450580c2c 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -387,7 +387,7 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not public abstract class BaseSignatureTxValidator : ITxValidator { - protected virtual bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) => false; + protected virtual bool ValidateChainId(Transaction transaction, IReleaseSpec releaseSpec) => false; public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) { @@ -420,7 +420,7 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not return true; } - if (AdditionalCheck(transaction, releaseSpec)) + if (ValidateChainId(transaction, releaseSpec)) { return true; } @@ -437,7 +437,7 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not public sealed class LegacySignatureTxValidator(ulong chainId) : BaseSignatureTxValidator { - protected override bool AdditionalCheck(Transaction transaction, IReleaseSpec releaseSpec) + protected override bool ValidateChainId(Transaction transaction, IReleaseSpec releaseSpec) { Signature signature = transaction.Signature; return releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul); From 1691e20c3ad09dc4be5f01bc798e31db6f965881 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 11:59:47 -0300 Subject: [PATCH 18/25] Use `static` modifier in lambdas when possible --- .../Nethermind.Consensus/Validators/TxValidator.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 9a450580c2c..40ecec7708a 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -29,7 +29,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator }, { TxType.AccessList, new CompositeTxValidator([ - new ReleaseSpecTxValidator(spec => spec.IsEip2930Enabled), + new ReleaseSpecTxValidator(static spec => spec.IsEip2930Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), @@ -39,7 +39,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator }, { TxType.EIP1559, new CompositeTxValidator([ - new ReleaseSpecTxValidator(spec => spec.IsEip1559Enabled), + new ReleaseSpecTxValidator(static spec => spec.IsEip1559Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), @@ -50,7 +50,7 @@ public sealed class TxValidator(ulong chainId) : ITxValidator }, { TxType.Blob, new CompositeTxValidator([ - new ReleaseSpecTxValidator(spec => spec.IsEip4844Enabled), + new ReleaseSpecTxValidator(static spec => spec.IsEip4844Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), From 644ac9ac917f3fe13de92dfb7e9676e5e18258ae Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 12:02:19 -0300 Subject: [PATCH 19/25] Remove `IsWellFormed` with no error message --- src/Nethermind/Nethermind.TxPool/ITxValidator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Nethermind/Nethermind.TxPool/ITxValidator.cs b/src/Nethermind/Nethermind.TxPool/ITxValidator.cs index bec156f83b1..1510c9a165a 100644 --- a/src/Nethermind/Nethermind.TxPool/ITxValidator.cs +++ b/src/Nethermind/Nethermind.TxPool/ITxValidator.cs @@ -9,7 +9,6 @@ namespace Nethermind.TxPool { public interface ITxValidator { - bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => IsWellFormed(transaction, releaseSpec, out _); bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error); } } From d4091c95891e202dc021ba72e5034a9afd58e31e Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 12:29:27 -0300 Subject: [PATCH 20/25] Add Blob EIP4844 no EIP1559 validation test --- .../Validators/TxValidatorTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs index 8e91f13773b..51548180cff 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs @@ -531,6 +531,25 @@ public void IsWellFormed_BlobTxHasProofOverTheSizeLimit_ReturnFalse() Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); } + [Test] + public void BlobTransactions_are_valid_with_eip4844_no_eip1559() + { + Transaction tx = Build.A.Transaction + .WithType(TxType.Blob) + .WithTimestamp(ulong.MaxValue) + .WithTo(TestItem.AddressA) + .WithMaxFeePerGas(1) + .WithMaxFeePerBlobGas(1) + .WithBlobVersionedHashes(1) + .WithChainId(TestBlockchainIds.ChainId) + .SignedAndResolved().TestObject; + + TxValidator txValidator = new(TestBlockchainIds.ChainId); + IReleaseSpec releaseSpec = new ReleaseSpec() { IsEip4844Enabled = true, IsEip1559Enabled = false }; + + txValidator.IsWellFormed(tx, releaseSpec).Should().BeTrue(); + } + private static byte[] MakeArray(int count, params byte[] elements) => elements.Take(Math.Min(count, elements.Length)) .Concat(new byte[Math.Max(0, count - elements.Length)]) From fff2f90163c8e778e70f6a5a4c0f252f5f4f2f2f Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 12:35:19 -0300 Subject: [PATCH 21/25] Change `` to `` --- src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 40ecec7708a..6503bc8cc22 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -204,10 +204,10 @@ public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [Not } } -/// +/// /// Ensure that non Blob transactions do not contain Blob specific fields. /// This validator will be deprecated once we have a proper Transaction type hierarchy. -/// +/// public sealed class NonBlobFieldsTxValidator : ITxValidator { public static readonly NonBlobFieldsTxValidator Instance = new(); From 5e14c7155fff7a825622dafd9d3420f6fe429ecf Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 12:57:11 -0300 Subject: [PATCH 22/25] Add `bad gas fields` test with EIP1559 disabled --- .../Validators/TxValidatorTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs index 51548180cff..c29d6cf7b62 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs @@ -550,6 +550,26 @@ public void BlobTransactions_are_valid_with_eip4844_no_eip1559() txValidator.IsWellFormed(tx, releaseSpec).Should().BeTrue(); } + [Test] + public void BlobTransactions_bad_gas_fields_is_valid_when_no_eip1559() + { + Transaction tx = Build.A.Transaction + .WithType(TxType.Blob) + .WithTimestamp(ulong.MaxValue) + .WithTo(TestItem.AddressA) + .WithMaxFeePerGas(1) + .WithMaxPriorityFeePerGas(2) + .WithMaxFeePerBlobGas(1) + .WithBlobVersionedHashes(1) + .WithChainId(TestBlockchainIds.ChainId) + .SignedAndResolved().TestObject; + + TxValidator txValidator = new(TestBlockchainIds.ChainId); + IReleaseSpec releaseSpec = new ReleaseSpec() { IsEip4844Enabled = true, IsEip1559Enabled = false }; + + txValidator.IsWellFormed(tx, releaseSpec).Should().BeTrue(); + } + private static byte[] MakeArray(int count, params byte[] elements) => elements.Take(Math.Min(count, elements.Length)) .Concat(new byte[Math.Max(0, count - elements.Length)]) From 04d7fffbb6d917aa895d4a8687d907223d7f6f43 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 13:25:36 -0300 Subject: [PATCH 23/25] Add remarks to tests --- .../Validators/TxValidatorTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs index c29d6cf7b62..5255076f1b3 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs @@ -531,6 +531,10 @@ public void IsWellFormed_BlobTxHasProofOverTheSizeLimit_ReturnFalse() Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); } + /// + /// According to https://github.com/ethereum/EIPs/blob/e2c094cd0f50303eddcfb87f36899e00545dcaaf/EIPS/eip-4844.md?plain=1#L11 + /// EIP 4844 implies 1559, thus the ReleaseSpec we're constructing here should not be possible. + /// [Test] public void BlobTransactions_are_valid_with_eip4844_no_eip1559() { @@ -550,6 +554,7 @@ public void BlobTransactions_are_valid_with_eip4844_no_eip1559() txValidator.IsWellFormed(tx, releaseSpec).Should().BeTrue(); } + /// Same as [Test] public void BlobTransactions_bad_gas_fields_is_valid_when_no_eip1559() { From 840d3a31b0935e4f4b68a6b7089cfddf2fad2b49 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel Date: Fri, 6 Sep 2024 14:56:09 -0300 Subject: [PATCH 24/25] Use small array instead of `Dictionary` --- .../Validators/TxValidator.cs | 88 +++++++++---------- 1 file changed, 42 insertions(+), 46 deletions(-) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index 6503bc8cc22..d478875d929 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -15,56 +15,51 @@ namespace Nethermind.Consensus.Validators; -public sealed class TxValidator(ulong chainId) : ITxValidator +public sealed class TxValidator : ITxValidator { - private readonly Dictionary _validators = new() + private readonly ITxValidator[] _validators; + + public TxValidator(ulong chainId) { - { - TxType.Legacy, new CompositeTxValidator([ - IntrinsicGasTxValidator.Instance, - new LegacySignatureTxValidator(chainId), - ContractSizeTxValidator.Instance, - NonBlobFieldsTxValidator.Instance, - ]) - }, - { - TxType.AccessList, new CompositeTxValidator([ - new ReleaseSpecTxValidator(static spec => spec.IsEip2930Enabled), - IntrinsicGasTxValidator.Instance, - SignatureTxValidator.Instance, - new ExpectedChainIdTxValidator(chainId), - ContractSizeTxValidator.Instance, - NonBlobFieldsTxValidator.Instance, - ]) - }, - { - TxType.EIP1559, new CompositeTxValidator([ - new ReleaseSpecTxValidator(static spec => spec.IsEip1559Enabled), - IntrinsicGasTxValidator.Instance, - SignatureTxValidator.Instance, - new ExpectedChainIdTxValidator(chainId), - GasFieldsTxValidator.Instance, - ContractSizeTxValidator.Instance, - NonBlobFieldsTxValidator.Instance, - ]) - }, - { - TxType.Blob, new CompositeTxValidator([ - new ReleaseSpecTxValidator(static spec => spec.IsEip4844Enabled), - IntrinsicGasTxValidator.Instance, - SignatureTxValidator.Instance, - new ExpectedChainIdTxValidator(chainId), - GasFieldsTxValidator.Instance, - ContractSizeTxValidator.Instance, - BlobFieldsTxValidator.Instance, - MempoolBlobTxValidator.Instance - ]) - }, - }; + _validators = new ITxValidator[byte.MaxValue + 1]; + _validators[(byte)TxType.Legacy] = new CompositeTxValidator([ + IntrinsicGasTxValidator.Instance, + new LegacySignatureTxValidator(chainId), + ContractSizeTxValidator.Instance, + NonBlobFieldsTxValidator.Instance, + ]); + _validators[(byte)TxType.AccessList] = new CompositeTxValidator([ + new ReleaseSpecTxValidator(static spec => spec.IsEip2930Enabled), + IntrinsicGasTxValidator.Instance, + SignatureTxValidator.Instance, + new ExpectedChainIdTxValidator(chainId), + ContractSizeTxValidator.Instance, + NonBlobFieldsTxValidator.Instance, + ]); + _validators[(byte)TxType.EIP1559] = new CompositeTxValidator([ + new ReleaseSpecTxValidator(static spec => spec.IsEip1559Enabled), + IntrinsicGasTxValidator.Instance, + SignatureTxValidator.Instance, + new ExpectedChainIdTxValidator(chainId), + GasFieldsTxValidator.Instance, + ContractSizeTxValidator.Instance, + NonBlobFieldsTxValidator.Instance, + ]); + _validators[(byte)TxType.Blob] = new CompositeTxValidator([ + new ReleaseSpecTxValidator(static spec => spec.IsEip4844Enabled), + IntrinsicGasTxValidator.Instance, + SignatureTxValidator.Instance, + new ExpectedChainIdTxValidator(chainId), + GasFieldsTxValidator.Instance, + ContractSizeTxValidator.Instance, + BlobFieldsTxValidator.Instance, + MempoolBlobTxValidator.Instance + ]); + } public TxValidator WithValidator(TxType type, ITxValidator validator) { - _validators[type] = validator; + _validators[(byte)type] = validator; return this; } @@ -81,7 +76,8 @@ public TxValidator WithValidator(TxType type, ITxValidator validator) /// public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? error) { - if (!_validators.TryGetValue(transaction.Type, out ITxValidator validator)) + ITxValidator? validator = _validators[(byte)transaction.Type]; + if (validator is null) { error = TxErrorMessages.InvalidTxType(releaseSpec.Name); return false; From ec0d5a27cf129fb516c99418fe0dc5a3d6dd51f3 Mon Sep 17 00:00:00 2001 From: Lukasz Rozmej Date: Mon, 9 Sep 2024 19:46:56 +0200 Subject: [PATCH 25/25] Refactor 'TxValidator' further (#7402) Co-authored-by: Jorge Mederos <46798594+jmederosalvarado@users.noreply.github.com> --- .../Nethermind.Api/INethermindApi.cs | 16 + .../Validators/TxValidatorTests.cs | 86 +--- .../Validators/AlwaysValid.cs | 12 +- .../Validators/BlockValidator.cs | 6 +- .../Validators/TxValidator.cs | 377 +++++------------- .../Builders/TransactionValidatorBuilder.cs | 11 +- .../Nethermind.Core/TransactionExtensions.cs | 27 +- .../Nethermind.Core/ValidationResult.cs | 13 + .../Steps/StepInitializationException.cs | 4 +- .../InvalidBlockInterceptor.cs | 52 +-- .../InitializeBlockchainOptimism.cs | 94 ++--- .../OptimismHeaderValidator.cs | 11 +- .../OptimismTxDecoder.cs | 4 +- .../OptimismTxValidator.cs | 18 - .../Nethermind.Serialization.Rlp/TxDecoder.cs | 19 +- .../TxDecoders/ITxDecoder.cs | 2 +- .../Nethermind.Specs/ReleaseSpec.cs | 9 +- .../Filters/MalformedTxFilter.cs | 23 +- .../Nethermind.TxPool/ITxValidator.cs | 3 +- 19 files changed, 284 insertions(+), 503 deletions(-) create mode 100644 src/Nethermind/Nethermind.Core/ValidationResult.cs rename src/Nethermind/{Nethermind.Serialization.Rlp/TxDecoders => Nethermind.Optimism}/OptimismTxDecoder.cs (96%) delete mode 100644 src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs diff --git a/src/Nethermind/Nethermind.Api/INethermindApi.cs b/src/Nethermind/Nethermind.Api/INethermindApi.cs index 8d864b2e9f8..fff51021393 100644 --- a/src/Nethermind/Nethermind.Api/INethermindApi.cs +++ b/src/Nethermind/Nethermind.Api/INethermindApi.cs @@ -2,7 +2,12 @@ // SPDX-License-Identifier: LGPL-3.0-only #nullable enable +using System; using Nethermind.Config; +using Nethermind.Core; +using Nethermind.Serialization.Rlp; +using Nethermind.Serialization.Rlp.TxDecoders; +using Nethermind.TxPool; namespace Nethermind.Api { @@ -15,4 +20,15 @@ public T Config() where T : IConfig (IApiWithNetwork GetFromApi, INethermindApi SetInApi) ForRpc => (this, this); } + + public static class NethermindApiExtensions + { + public static void RegisterTxType(this INethermindApi api, TxType type, ITxDecoder decoder, ITxValidator validator) + { + ArgumentNullException.ThrowIfNull(api.TxValidator); + + api.TxValidator.RegisterValidator(type, validator); + TxDecoder.Instance.RegisterDecoder(decoder); + } + } } diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs index 5255076f1b3..7d3bb9bc00b 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs @@ -56,7 +56,7 @@ public void Zero_r_is_not_valid() Transaction tx = Build.A.Transaction.WithSignature(signature).TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - txValidator.IsWellFormed(tx, MuirGlacier.Instance).Should().BeFalse(); + txValidator.IsWellFormed(tx, MuirGlacier.Instance).AsBool().Should().BeFalse(); } private static byte CalculateV() => (byte)EthereumEcdsa.CalculateV(TestBlockchainIds.ChainId); @@ -72,7 +72,7 @@ public void Zero_s_is_not_valid() Transaction tx = Build.A.Transaction.WithSignature(signature).TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - txValidator.IsWellFormed(tx, MuirGlacier.Instance).Should().BeFalse(); + txValidator.IsWellFormed(tx, MuirGlacier.Instance).AsBool().Should().BeFalse(); } [Test, Timeout(Timeout.MaxTestTime)] @@ -86,7 +86,7 @@ public void Bad_chain_id_is_not_valid() Transaction tx = Build.A.Transaction.WithSignature(signature).TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - txValidator.IsWellFormed(tx, MuirGlacier.Instance).Should().BeFalse(); + txValidator.IsWellFormed(tx, MuirGlacier.Instance).AsBool().Should().BeFalse(); } [Test, Timeout(Timeout.MaxTestTime)] @@ -100,7 +100,7 @@ public void No_chain_id_legacy_tx_is_valid() Transaction tx = Build.A.Transaction.WithSignature(signature).TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - txValidator.IsWellFormed(tx, MuirGlacier.Instance).Should().BeTrue(); + txValidator.IsWellFormed(tx, MuirGlacier.Instance).AsBool().Should().BeTrue(); } [Test, Timeout(Timeout.MaxTestTime)] @@ -114,7 +114,7 @@ public void Is_valid_with_valid_chain_id() Transaction tx = Build.A.Transaction.WithSignature(signature).TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - txValidator.IsWellFormed(tx, MuirGlacier.Instance).Should().BeTrue(); + txValidator.IsWellFormed(tx, MuirGlacier.Instance).AsBool().Should().BeTrue(); } [Timeout(Timeout.MaxTestTime)] @@ -134,7 +134,7 @@ public void Before_eip_155_has_to_have_valid_chain_id_unless_overridden(bool val releaseSpec.ValidateChainId.Returns(validateChainId); TxValidator txValidator = new(TestBlockchainIds.ChainId); - txValidator.IsWellFormed(tx, releaseSpec).Should().Be(!validateChainId); + txValidator.IsWellFormed(tx, releaseSpec).AsBool().Should().Be(!validateChainId); } [Timeout(Timeout.MaxTestTime)] @@ -274,7 +274,7 @@ public void Transaction_with_init_code_above_max_value_is_rejected_when_eip3860E .WithData(initCode).TestObject; TxValidator txValidator = new(1); - txValidator.IsWellFormed(tx, releaseSpec).Should().Be(expectedResult); + txValidator.IsWellFormed(tx, releaseSpec).AsBool().Should().Be(expectedResult); } //leading zeros in AccessList - expected to pass (real mainnet tx) @@ -345,8 +345,8 @@ public void ShardBlobTransactions_should_have_destination_set() .WithChainId(TestBlockchainIds.ChainId) .SignedAndResolved().TestObject; - Assert.That(txValidator.IsWellFormed(txWithoutTo, Cancun.Instance), Is.False); - Assert.That(txValidator.IsWellFormed(txWithTo, Cancun.Instance)); + Assert.That(txValidator.IsWellFormed(txWithoutTo, Cancun.Instance).AsBool(), Is.False); + Assert.That(txValidator.IsWellFormed(txWithTo, Cancun.Instance).AsBool()); } [Timeout(Timeout.MaxTestTime)] @@ -404,9 +404,8 @@ public void IsWellFormed_NotBlobTxButMaxFeePerBlobGasIsSet_ReturnFalse() Transaction tx = txBuilder.TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] @@ -419,9 +418,8 @@ public void IsWellFormed_NotBlobTxButBlobVersionedHashesIsSet_ReturnFalse() Transaction tx = txBuilder.TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] @@ -437,9 +435,8 @@ public void IsWellFormed_BlobTxToIsNull_ReturnFalse() Transaction tx = txBuilder.TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] public void IsWellFormed_BlobTxHasMoreDataGasThanAllowed_ReturnFalse() @@ -454,9 +451,8 @@ public void IsWellFormed_BlobTxHasMoreDataGasThanAllowed_ReturnFalse() Transaction tx = txBuilder.TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] @@ -472,9 +468,8 @@ public void IsWellFormed_BlobTxHasNoBlobs_ReturnFalse() Transaction tx = txBuilder.TestObject; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] @@ -490,9 +485,8 @@ public void IsWellFormed_BlobTxHasBlobOverTheSizeLimit_ReturnFalse() Transaction tx = txBuilder.TestObject; ((ShardBlobNetworkWrapper)tx.NetworkWrapper!).Blobs[0] = new byte[Ckzg.Ckzg.BytesPerBlob + 1]; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] @@ -508,9 +502,8 @@ public void IsWellFormed_BlobTxHasCommitmentOverTheSizeLimit_ReturnFalse() Transaction tx = txBuilder.TestObject; ((ShardBlobNetworkWrapper)tx.NetworkWrapper!).Commitments[0] = new byte[Ckzg.Ckzg.BytesPerCommitment + 1]; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } [Test] @@ -526,53 +519,8 @@ public void IsWellFormed_BlobTxHasProofOverTheSizeLimit_ReturnFalse() Transaction tx = txBuilder.TestObject; ((ShardBlobNetworkWrapper)tx.NetworkWrapper!).Proofs[0] = new byte[Ckzg.Ckzg.BytesPerProof + 1]; TxValidator txValidator = new(TestBlockchainIds.ChainId); - string? error; - Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance, out error), Is.False); - } - - /// - /// According to https://github.com/ethereum/EIPs/blob/e2c094cd0f50303eddcfb87f36899e00545dcaaf/EIPS/eip-4844.md?plain=1#L11 - /// EIP 4844 implies 1559, thus the ReleaseSpec we're constructing here should not be possible. - /// - [Test] - public void BlobTransactions_are_valid_with_eip4844_no_eip1559() - { - Transaction tx = Build.A.Transaction - .WithType(TxType.Blob) - .WithTimestamp(ulong.MaxValue) - .WithTo(TestItem.AddressA) - .WithMaxFeePerGas(1) - .WithMaxFeePerBlobGas(1) - .WithBlobVersionedHashes(1) - .WithChainId(TestBlockchainIds.ChainId) - .SignedAndResolved().TestObject; - - TxValidator txValidator = new(TestBlockchainIds.ChainId); - IReleaseSpec releaseSpec = new ReleaseSpec() { IsEip4844Enabled = true, IsEip1559Enabled = false }; - - txValidator.IsWellFormed(tx, releaseSpec).Should().BeTrue(); - } - - /// Same as - [Test] - public void BlobTransactions_bad_gas_fields_is_valid_when_no_eip1559() - { - Transaction tx = Build.A.Transaction - .WithType(TxType.Blob) - .WithTimestamp(ulong.MaxValue) - .WithTo(TestItem.AddressA) - .WithMaxFeePerGas(1) - .WithMaxPriorityFeePerGas(2) - .WithMaxFeePerBlobGas(1) - .WithBlobVersionedHashes(1) - .WithChainId(TestBlockchainIds.ChainId) - .SignedAndResolved().TestObject; - - TxValidator txValidator = new(TestBlockchainIds.ChainId); - IReleaseSpec releaseSpec = new ReleaseSpec() { IsEip4844Enabled = true, IsEip1559Enabled = false }; - - txValidator.IsWellFormed(tx, releaseSpec).Should().BeTrue(); + Assert.That(txValidator.IsWellFormed(tx, Cancun.Instance).AsBool(), Is.False); } private static byte[] MakeArray(int count, params byte[] elements) => diff --git a/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs b/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs index 92ec5b749bf..0550ac08ddb 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/AlwaysValid.cs @@ -11,9 +11,11 @@ namespace Nethermind.Consensus.Validators; public class Always : IBlockValidator, ISealValidator, IUnclesValidator, ITxValidator { private readonly bool _result; + private readonly ValidationResult _validationResult; private Always(bool result) { + _validationResult = result ? ValidationResult.Success : "Always invalid."; _result = result; } @@ -79,16 +81,10 @@ public bool Validate(BlockHeader header, BlockHeader[] uncles) return _result; } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) { - return _result; + return _validationResult; } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? errorMessage) - { - errorMessage = null; - return _result; - } - public bool ValidateWithdrawals(Block block, out string? error) { error = null; diff --git a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs index 04e0ca08f68..e720c5c44f9 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/BlockValidator.cs @@ -292,9 +292,11 @@ private bool ValidateTransactions(Block block, IReleaseSpec spec, out string? er { Transaction transaction = transactions[txIndex]; - if (!_txValidator.IsWellFormed(transaction, spec, out errorMessage)) + ValidationResult isWellFormed = _txValidator.IsWellFormed(transaction, spec); + if (!isWellFormed) { - if (_logger.IsDebug) _logger.Debug($"{Invalid(block)} Invalid transaction: {errorMessage}"); + if (_logger.IsDebug) _logger.Debug($"{Invalid(block)} Invalid transaction: {isWellFormed}"); + errorMessage = isWellFormed.Error; return false; } } diff --git a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs index d478875d929..5207135b109 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: LGPL-3.0-only using System; -using System.Diagnostics.CodeAnalysis; using System.Collections.Generic; using Nethermind.Consensus.Messages; using Nethermind.Core; @@ -17,26 +16,25 @@ namespace Nethermind.Consensus.Validators; public sealed class TxValidator : ITxValidator { - private readonly ITxValidator[] _validators; + private readonly ITxValidator?[] _validators = new ITxValidator?[Transaction.MaxTxType + 1]; public TxValidator(ulong chainId) { - _validators = new ITxValidator[byte.MaxValue + 1]; - _validators[(byte)TxType.Legacy] = new CompositeTxValidator([ + RegisterValidator(TxType.Legacy, new CompositeTxValidator([ IntrinsicGasTxValidator.Instance, new LegacySignatureTxValidator(chainId), ContractSizeTxValidator.Instance, NonBlobFieldsTxValidator.Instance, - ]); - _validators[(byte)TxType.AccessList] = new CompositeTxValidator([ + ])); + RegisterValidator(TxType.AccessList, new CompositeTxValidator([ new ReleaseSpecTxValidator(static spec => spec.IsEip2930Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, new ExpectedChainIdTxValidator(chainId), ContractSizeTxValidator.Instance, NonBlobFieldsTxValidator.Instance, - ]); - _validators[(byte)TxType.EIP1559] = new CompositeTxValidator([ + ])); + RegisterValidator(TxType.EIP1559, new CompositeTxValidator([ new ReleaseSpecTxValidator(static spec => spec.IsEip1559Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, @@ -44,8 +42,8 @@ public TxValidator(ulong chainId) GasFieldsTxValidator.Instance, ContractSizeTxValidator.Instance, NonBlobFieldsTxValidator.Instance, - ]); - _validators[(byte)TxType.Blob] = new CompositeTxValidator([ + ])); + RegisterValidator(TxType.Blob, new CompositeTxValidator([ new ReleaseSpecTxValidator(static spec => spec.IsEip4844Enabled), IntrinsicGasTxValidator.Instance, SignatureTxValidator.Instance, @@ -54,16 +52,10 @@ public TxValidator(ulong chainId) ContractSizeTxValidator.Instance, BlobFieldsTxValidator.Instance, MempoolBlobTxValidator.Instance - ]); + ])); } - public TxValidator WithValidator(TxType type, ITxValidator validator) - { - _validators[(byte)type] = validator; - return this; - } - - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => IsWellFormed(transaction, releaseSpec, out _); + public void RegisterValidator(TxType type, ITxValidator validator) => _validators[(byte)type] = validator; /// /// Full and correct validation is only possible in the context of a specific block @@ -74,34 +66,26 @@ public TxValidator WithValidator(TxType type, ITxValidator validator) /// As such, we can decide whether tx is well formed as long as we also validate nonce /// just before the execution of the block / tx. /// - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, out string? error) - { - ITxValidator? validator = _validators[(byte)transaction.Type]; - if (validator is null) - { - error = TxErrorMessages.InvalidTxType(releaseSpec.Name); - return false; - } - - return validator.IsWellFormed(transaction, releaseSpec, out error); - } + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => + _validators.TryGetByTxType(transaction.Type, out ITxValidator validator) + ? validator.IsWellFormed(transaction, releaseSpec) + : TxErrorMessages.InvalidTxType(releaseSpec.Name); } public sealed class CompositeTxValidator(List validators) : ITxValidator { - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) { - error = null; - foreach (ITxValidator validator in validators) { - if (!validator.IsWellFormed(transaction, releaseSpec, out error)) + ValidationResult isWellFormed = validator.IsWellFormed(transaction, releaseSpec); + if (!isWellFormed) { - return false; + return isWellFormed; } } - return true; + return ValidationResult.Success; } } @@ -110,52 +94,23 @@ public sealed class IntrinsicGasTxValidator : ITxValidator public static readonly IntrinsicGasTxValidator Instance = new(); private IntrinsicGasTxValidator() { } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => // This is unnecessarily calculated twice - at validation and execution times. - var intrinsicGas = IntrinsicGasCalculator.Calculate(transaction, releaseSpec); - if (transaction.GasLimit < intrinsicGas) - { - error = TxErrorMessages.IntrinsicGasTooLow; - return false; - } - - return true; - } + transaction.GasLimit < IntrinsicGasCalculator.Calculate(transaction, releaseSpec) + ? TxErrorMessages.IntrinsicGasTooLow + : ValidationResult.Success; } public sealed class ReleaseSpecTxValidator(Func validate) : ITxValidator { - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (!validate(releaseSpec)) - { - error = TxErrorMessages.InvalidTxType(releaseSpec.Name); - return false; - } - - return true; - } + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => + !validate(releaseSpec) ? TxErrorMessages.InvalidTxType(releaseSpec.Name) : ValidationResult.Success; } public sealed class ExpectedChainIdTxValidator(ulong chainId) : ITxValidator { - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (transaction.ChainId != chainId) - { - error = TxErrorMessages.InvalidTxChainId(chainId, transaction.ChainId); - return false; - } - - return true; - } + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => + transaction.ChainId != chainId ? TxErrorMessages.InvalidTxChainId(chainId, transaction.ChainId) : ValidationResult.Success; } public sealed class GasFieldsTxValidator : ITxValidator @@ -163,23 +118,8 @@ public sealed class GasFieldsTxValidator : ITxValidator public static readonly GasFieldsTxValidator Instance = new(); private GasFieldsTxValidator() { } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (!releaseSpec.IsEip1559Enabled) - { - return true; - } - - if (transaction.MaxFeePerGas < transaction.MaxPriorityFeePerGas) - { - error = TxErrorMessages.InvalidMaxPriorityFeePerGas; - return false; - } - - return true; - } + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => + transaction.MaxFeePerGas < transaction.MaxPriorityFeePerGas ? TxErrorMessages.InvalidMaxPriorityFeePerGas : ValidationResult.Success; } public sealed class ContractSizeTxValidator : ITxValidator @@ -187,17 +127,8 @@ public sealed class ContractSizeTxValidator : ITxValidator public static readonly ContractSizeTxValidator Instance = new(); private ContractSizeTxValidator() { } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - if (transaction.IsAboveInitCode(releaseSpec)) - { - error = TxErrorMessages.ContractSizeTooBig; - return false; - } - - return true; - } + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => + transaction.IsAboveInitCode(releaseSpec) ? TxErrorMessages.ContractSizeTooBig : ValidationResult.Success; } /// @@ -209,32 +140,14 @@ public sealed class NonBlobFieldsTxValidator : ITxValidator public static readonly NonBlobFieldsTxValidator Instance = new(); private NonBlobFieldsTxValidator() { } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => transaction switch { - error = null; - // Execution-payload version verification - if (transaction.MaxFeePerBlobGas is not null) - { - error = TxErrorMessages.NotAllowedMaxFeePerBlobGas; - return false; - } - - if (transaction.BlobVersionedHashes is not null) - { - error = TxErrorMessages.NotAllowedBlobVersionedHashes; - return false; - } - - if (transaction is { NetworkWrapper: ShardBlobNetworkWrapper }) - { - // NOTE: This must be an internal issue - error = TxErrorMessages.InvalidTransaction; - return false; - } - - return true; - } + { MaxFeePerBlobGas: not null } => TxErrorMessages.NotAllowedMaxFeePerBlobGas, + { BlobVersionedHashes: not null } => TxErrorMessages.NotAllowedBlobVersionedHashes, + { NetworkWrapper: ShardBlobNetworkWrapper } => TxErrorMessages.InvalidTransaction, + _ => ValidationResult.Success + }; } public sealed class BlobFieldsTxValidator : ITxValidator @@ -242,64 +155,37 @@ public sealed class BlobFieldsTxValidator : ITxValidator public static readonly BlobFieldsTxValidator Instance = new(); private BlobFieldsTxValidator() { } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - - if (transaction.To is null) + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) => + transaction switch { - error = TxErrorMessages.TxMissingTo; - return false; - } + { To: null } => TxErrorMessages.TxMissingTo, + { MaxFeePerBlobGas: null } => TxErrorMessages.BlobTxMissingMaxFeePerBlobGas, + { BlobVersionedHashes: null } => TxErrorMessages.BlobTxMissingBlobVersionedHashes, + _ => ValidateBlobFields(transaction) + }; - if (transaction.MaxFeePerBlobGas is null) - { - error = TxErrorMessages.BlobTxMissingMaxFeePerBlobGas; - return false; - } - - if (transaction.BlobVersionedHashes is null) - { - error = TxErrorMessages.BlobTxMissingBlobVersionedHashes; - return false; - } - - var blobCount = transaction.BlobVersionedHashes.Length; - var totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount); - if (totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction) - { - error = TxErrorMessages.BlobTxGasLimitExceeded; - return false; - } - - if (blobCount < Eip4844Constants.MinBlobsPerTransaction) - { - error = TxErrorMessages.BlobTxMissingBlobs; - return false; - } + private ValidationResult ValidateBlobFields(Transaction transaction) + { + int blobCount = transaction.BlobVersionedHashes!.Length; + ulong totalDataGas = BlobGasCalculator.CalculateBlobGas(blobCount); + return totalDataGas > Eip4844Constants.MaxBlobGasPerTransaction ? TxErrorMessages.BlobTxGasLimitExceeded + : blobCount < Eip4844Constants.MinBlobsPerTransaction ? TxErrorMessages.BlobTxMissingBlobs + : ValidateBlobVersionedHashes(); - for (int i = 0; i < blobCount; i++) + ValidationResult ValidateBlobVersionedHashes() { - if (transaction.BlobVersionedHashes[i] is null) + for (int i = 0; i < blobCount; i++) { - error = TxErrorMessages.MissingBlobVersionedHash; - return false; + switch (transaction.BlobVersionedHashes[i]) + { + case null: return TxErrorMessages.MissingBlobVersionedHash; + case { Length: not KzgPolynomialCommitments.BytesPerBlobVersionedHash }: return TxErrorMessages.InvalidBlobVersionedHashSize; + case { Length: KzgPolynomialCommitments.BytesPerBlobVersionedHash } when transaction.BlobVersionedHashes[i][0] != KzgPolynomialCommitments.KzgBlobHashVersionV1: return TxErrorMessages.InvalidBlobVersionedHashVersion; + } } - if (transaction.BlobVersionedHashes[i].Length != KzgPolynomialCommitments.BytesPerBlobVersionedHash) - { - error = TxErrorMessages.InvalidBlobVersionedHashSize; - return false; - } - - if (transaction.BlobVersionedHashes[i][0] != KzgPolynomialCommitments.KzgBlobHashVersionV1) - { - error = TxErrorMessages.InvalidBlobVersionedHashVersion; - return false; - } + return ValidationResult.Success; } - - return true; } } @@ -311,132 +197,83 @@ public sealed class MempoolBlobTxValidator : ITxValidator public static readonly MempoolBlobTxValidator Instance = new(); private MempoolBlobTxValidator() { } - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) { - error = null; - - if (transaction.NetworkWrapper is not ShardBlobNetworkWrapper wrapper) - { - return true; - } - - int blobCount = transaction.BlobVersionedHashes.Length; - if (wrapper.Blobs.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - if (wrapper.Commitments.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - if (wrapper.Proofs.Length != blobCount) - { - error = TxErrorMessages.InvalidBlobData; - return false; - } - - for (int i = 0; i < blobCount; i++) + int blobCount = transaction.BlobVersionedHashes!.Length; + return transaction.NetworkWrapper is not ShardBlobNetworkWrapper wrapper ? ValidationResult.Success + : wrapper.Blobs.Length != blobCount ? TxErrorMessages.InvalidBlobData + : wrapper.Commitments.Length != blobCount ? TxErrorMessages.InvalidBlobData + : wrapper.Proofs.Length != blobCount ? TxErrorMessages.InvalidBlobData + : ValidateBlobs(); + + ValidationResult ValidateBlobs() { - if (wrapper.Blobs[i].Length != Ckzg.Ckzg.BytesPerBlob) - { - error = TxErrorMessages.ExceededBlobSize; - return false; - } - - if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) - { - error = TxErrorMessages.ExceededBlobCommitmentSize; - return false; - } - - if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) + for (int i = 0; i < blobCount; i++) { - error = TxErrorMessages.InvalidBlobProofSize; - return false; + if (wrapper.Blobs[i].Length != Ckzg.Ckzg.BytesPerBlob) + { + return TxErrorMessages.ExceededBlobSize; + } + + if (wrapper.Commitments[i].Length != Ckzg.Ckzg.BytesPerCommitment) + { + return TxErrorMessages.ExceededBlobCommitmentSize; + } + + if (wrapper.Proofs[i].Length != Ckzg.Ckzg.BytesPerProof) + { + return TxErrorMessages.InvalidBlobProofSize; + } } - } - Span hash = stackalloc byte[32]; - for (int i = 0; i < blobCount; i++) - { - if (!KzgPolynomialCommitments.TryComputeCommitmentHashV1(wrapper.Commitments[i].AsSpan(), hash) || - !hash.SequenceEqual(transaction.BlobVersionedHashes[i])) + Span hash = stackalloc byte[32]; + for (int i = 0; i < blobCount; i++) { - error = TxErrorMessages.InvalidBlobCommitmentHash; - return false; + if (!KzgPolynomialCommitments.TryComputeCommitmentHashV1(wrapper.Commitments[i].AsSpan(), hash) || !hash.SequenceEqual(transaction.BlobVersionedHashes[i])) + { + return TxErrorMessages.InvalidBlobCommitmentHash; + } } - } - if (!KzgPolynomialCommitments.AreProofsValid(wrapper.Blobs, wrapper.Commitments, wrapper.Proofs)) - { - error = TxErrorMessages.InvalidBlobProof; - return false; + return !KzgPolynomialCommitments.AreProofsValid(wrapper.Blobs, wrapper.Commitments, wrapper.Proofs) + ? TxErrorMessages.InvalidBlobProof + : ValidationResult.Success; } - - return true; } } public abstract class BaseSignatureTxValidator : ITxValidator { - protected virtual bool ValidateChainId(Transaction transaction, IReleaseSpec releaseSpec) => false; + protected virtual ValidationResult ValidateChainId(Transaction transaction, IReleaseSpec releaseSpec) => + releaseSpec.ValidateChainId ? TxErrorMessages.InvalidTxSignature : ValidationResult.Success; - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec) { - error = null; - Signature? signature = transaction.Signature; if (signature is null) { - error = TxErrorMessages.InvalidTxSignature; - return false; + return TxErrorMessages.InvalidTxSignature; } UInt256 sValue = new(signature.SAsSpan, isBigEndian: true); UInt256 rValue = new(signature.RAsSpan, isBigEndian: true); - if (sValue.IsZero || sValue >= (releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N)) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - if (rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - if (signature.V is 27 or 28) - { - return true; - } - - if (ValidateChainId(transaction, releaseSpec)) - { - return true; - } - - if (releaseSpec.ValidateChainId) - { - error = TxErrorMessages.InvalidTxSignature; - return false; - } - - return true; + UInt256 sMax = releaseSpec.IsEip2Enabled ? Secp256K1Curve.HalfNPlusOne : Secp256K1Curve.N; + return sValue.IsZero || sValue >= sMax ? TxErrorMessages.InvalidTxSignature + : rValue.IsZero || rValue >= Secp256K1Curve.NMinusOne ? TxErrorMessages.InvalidTxSignature + : signature.V is 27 or 28 ? ValidationResult.Success + : ValidateChainId(transaction, releaseSpec); } } public sealed class LegacySignatureTxValidator(ulong chainId) : BaseSignatureTxValidator { - protected override bool ValidateChainId(Transaction transaction, IReleaseSpec releaseSpec) + protected override ValidationResult ValidateChainId(Transaction transaction, IReleaseSpec releaseSpec) { - Signature signature = transaction.Signature; - return releaseSpec.IsEip155Enabled && (signature.V == chainId * 2 + 35ul || signature.V == chainId * 2 + 36ul); + ulong v = transaction.Signature!.V; + return releaseSpec.IsEip155Enabled && (v == chainId * 2 + 35ul || v == chainId * 2 + 36ul) + ? ValidationResult.Success + : base.ValidateChainId(transaction, releaseSpec); } } diff --git a/src/Nethermind/Nethermind.Core.Test/Builders/TransactionValidatorBuilder.cs b/src/Nethermind/Nethermind.Core.Test/Builders/TransactionValidatorBuilder.cs index f945817a461..fe41081f60e 100644 --- a/src/Nethermind/Nethermind.Core.Test/Builders/TransactionValidatorBuilder.cs +++ b/src/Nethermind/Nethermind.Core.Test/Builders/TransactionValidatorBuilder.cs @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using Nethermind.Consensus.Validators; using Nethermind.Core.Specs; using Nethermind.TxPool; using NSubstitute; @@ -9,7 +10,7 @@ namespace Nethermind.Core.Test.Builders { public class TransactionValidatorBuilder : BuilderBase { - private bool _alwaysTrue; + private ValidationResult _always; public TransactionValidatorBuilder() { @@ -20,7 +21,7 @@ public TransactionValidatorBuilder ThatAlwaysReturnsFalse { get { - _alwaysTrue = false; + _always = "Invalid"; return this; } } @@ -29,15 +30,15 @@ public TransactionValidatorBuilder ThatAlwaysReturnsTrue { get { - _alwaysTrue = true; + _always = ValidationResult.Success; return this; } } protected override void BeforeReturn() { - TestObjectInternal.IsWellFormed(Arg.Any(), Arg.Any(), out _).Returns(_alwaysTrue); - TestObjectInternal.IsWellFormed(Arg.Any(), Arg.Any(), out _).Returns(_alwaysTrue); + TestObjectInternal.IsWellFormed(Arg.Any(), Arg.Any()).Returns(_always); + TestObjectInternal.IsWellFormed(Arg.Any(), Arg.Any()).Returns(_always); base.BeforeReturn(); } } diff --git a/src/Nethermind/Nethermind.Core/TransactionExtensions.cs b/src/Nethermind/Nethermind.Core/TransactionExtensions.cs index 412bce0e3c8..7f54a6a18ef 100644 --- a/src/Nethermind/Nethermind.Core/TransactionExtensions.cs +++ b/src/Nethermind/Nethermind.Core/TransactionExtensions.cs @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System.Diagnostics.CodeAnalysis; using Nethermind.Core.Specs; using Nethermind.Int256; @@ -33,7 +34,9 @@ public static UInt256 CalculateTransactionPotentialCost(this Transaction tx, boo { UInt256 effectiveGasPrice = tx.CalculateEffectiveGasPrice(eip1559Enabled, baseFee); if (tx.IsServiceTransaction) + { effectiveGasPrice = UInt256.Zero; + } return effectiveGasPrice * (ulong)tx.GasLimit + tx.Value; } @@ -57,15 +60,23 @@ public static UInt256 CalculateEffectiveGasPrice(this Transaction tx, bool eip15 return effectiveGasPrice; } - public static UInt256 CalculateMaxPriorityFeePerGas(this Transaction tx, bool eip1559Enabled, in UInt256 baseFee) - { - return eip1559Enabled ? UInt256.Min(tx.MaxPriorityFeePerGas, tx.MaxFeePerGas > baseFee ? tx.MaxFeePerGas - baseFee : 0) : tx.MaxPriorityFeePerGas; - } - public static bool IsAboveInitCode(this Transaction tx, IReleaseSpec spec) - { - return tx.IsContractCreation && spec.IsEip3860Enabled && (tx.DataLength) > spec.MaxInitCodeSize; - } + public static UInt256 CalculateMaxPriorityFeePerGas(this Transaction tx, bool eip1559Enabled, in UInt256 baseFee) => + eip1559Enabled ? UInt256.Min(tx.MaxPriorityFeePerGas, tx.MaxFeePerGas > baseFee ? tx.MaxFeePerGas - baseFee : 0) : tx.MaxPriorityFeePerGas; + public static bool IsAboveInitCode(this Transaction tx, IReleaseSpec spec) => + tx.IsContractCreation && spec.IsEip3860Enabled && tx.DataLength > spec.MaxInitCodeSize; + public static bool TryGetByTxType(this T?[] array, TxType txType, [NotNullWhen(true)] out T? item) + { + var type = (byte)txType; + if (type > Transaction.MaxTxType) + { + item = default; + return false; + } + + item = array[type]; + return item != null; + } } } diff --git a/src/Nethermind/Nethermind.Core/ValidationResult.cs b/src/Nethermind/Nethermind.Core/ValidationResult.cs new file mode 100644 index 00000000000..739d7e5296e --- /dev/null +++ b/src/Nethermind/Nethermind.Core/ValidationResult.cs @@ -0,0 +1,13 @@ +// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +namespace Nethermind.Core; + +public record struct ValidationResult(string? Error) +{ + public static ValidationResult Success => new(null); + public static implicit operator bool(ValidationResult result) => result.AsBool(); + public static implicit operator ValidationResult(string error) => new(error); + public override string ToString() => Error ?? "Success"; + public bool AsBool() => Error is null; +} diff --git a/src/Nethermind/Nethermind.Init/Steps/StepInitializationException.cs b/src/Nethermind/Nethermind.Init/Steps/StepInitializationException.cs index 8fe0e62a4f3..767a9daf54d 100644 --- a/src/Nethermind/Nethermind.Init/Steps/StepInitializationException.cs +++ b/src/Nethermind/Nethermind.Init/Steps/StepInitializationException.cs @@ -20,9 +20,7 @@ public StepDependencyException(string message) public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null) { - if (argument is not null) - return; - throw new StepDependencyException(paramName ?? ""); + if (argument is null) throw new StepDependencyException(paramName ?? ""); } } } diff --git a/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs b/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs index 87cbb58f650..3b32e476975 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/InvalidChainTracker/InvalidBlockInterceptor.cs @@ -8,23 +8,15 @@ namespace Nethermind.Merge.Plugin.InvalidChainTracker; -public class InvalidBlockInterceptor : IBlockValidator +public class InvalidBlockInterceptor( + IBlockValidator headerValidator, + IInvalidChainTracker invalidChainTracker, + ILogManager logManager) + : IBlockValidator { - private readonly IBlockValidator _baseValidator; - private readonly IInvalidChainTracker _invalidChainTracker; - private readonly ILogger _logger; - - public InvalidBlockInterceptor( - IBlockValidator headerValidator, - IInvalidChainTracker invalidChainTracker, - ILogManager logManager) - { - _baseValidator = headerValidator; - _invalidChainTracker = invalidChainTracker; - _logger = logManager.GetClassLogger(typeof(InvalidBlockInterceptor)); - } + private readonly ILogger _logger = logManager.GetClassLogger(typeof(InvalidBlockInterceptor)); - public bool ValidateOrphanedBlock(Block block, [NotNullWhen(false)] out string? error) => _baseValidator.ValidateOrphanedBlock(block, out error); + public bool ValidateOrphanedBlock(Block block, [NotNullWhen(false)] out string? error) => headerValidator.ValidateOrphanedBlock(block, out error); public bool Validate(BlockHeader header, BlockHeader? parent, bool isUncle = false) { @@ -32,7 +24,7 @@ public bool Validate(BlockHeader header, BlockHeader? parent, bool isUncle = fal } public bool Validate(BlockHeader header, BlockHeader? parent, bool isUncle, [NotNullWhen(false)] out string? error) { - bool result = _baseValidator.Validate(header, parent, isUncle, out error); + bool result = headerValidator.Validate(header, parent, isUncle, out error); if (!result) { if (_logger.IsTrace) _logger.Trace($"Intercepted a bad header {header}"); @@ -41,9 +33,9 @@ public bool Validate(BlockHeader header, BlockHeader? parent, bool isUncle, [Not if (_logger.IsDebug) _logger.Debug($"Header invalidation should not be tracked"); return result; } - _invalidChainTracker.OnInvalidBlock(header.Hash!, header.ParentHash); + invalidChainTracker.OnInvalidBlock(header.Hash!, header.ParentHash); } - _invalidChainTracker.SetChildParent(header.Hash!, header.ParentHash!); + invalidChainTracker.SetChildParent(header.Hash!, header.ParentHash!); return result; } @@ -54,7 +46,7 @@ public bool Validate(BlockHeader header, bool isUncle = false) public bool Validate(BlockHeader header, bool isUncle, [NotNullWhen(false)] out string? error) { - bool result = _baseValidator.Validate(header, isUncle, out error); + bool result = headerValidator.Validate(header, isUncle, out error); if (!result) { if (_logger.IsTrace) _logger.Trace($"Intercepted a bad header {header}"); @@ -63,15 +55,15 @@ public bool Validate(BlockHeader header, bool isUncle, [NotNullWhen(false)] out if (_logger.IsDebug) _logger.Debug($"Header invalidation should not be tracked"); return result; } - _invalidChainTracker.OnInvalidBlock(header.Hash!, header.ParentHash); + invalidChainTracker.OnInvalidBlock(header.Hash!, header.ParentHash); } - _invalidChainTracker.SetChildParent(header.Hash!, header.ParentHash!); + invalidChainTracker.SetChildParent(header.Hash!, header.ParentHash!); return result; } public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? error, bool validateHashes = true) { - bool result = _baseValidator.ValidateSuggestedBlock(block, out error, validateHashes); + bool result = headerValidator.ValidateSuggestedBlock(block, out error, validateHashes); if (!result) { if (_logger.IsTrace) _logger.Trace($"Intercepted a bad block {block}"); @@ -80,9 +72,9 @@ public bool ValidateSuggestedBlock(Block block, [NotNullWhen(false)] out string? if (_logger.IsDebug) _logger.Debug($"Block invalidation should not be tracked"); return result; } - _invalidChainTracker.OnInvalidBlock(block.Hash!, block.ParentHash); + invalidChainTracker.OnInvalidBlock(block.Hash!, block.ParentHash); } - _invalidChainTracker.SetChildParent(block.Hash!, block.ParentHash!); + invalidChainTracker.SetChildParent(block.Hash!, block.ParentHash!); return result; } @@ -92,7 +84,7 @@ public bool ValidateProcessedBlock(Block block, TxReceipt[] receipts, Block sugg } public bool ValidateProcessedBlock(Block processedBlock, TxReceipt[] receipts, Block suggestedBlock, [NotNullWhen(false)] out string? error) { - bool result = _baseValidator.ValidateProcessedBlock(processedBlock, receipts, suggestedBlock, out error); + bool result = headerValidator.ValidateProcessedBlock(processedBlock, receipts, suggestedBlock, out error); if (!result) { if (_logger.IsTrace) _logger.Trace($"Intercepted a bad block {processedBlock}"); @@ -101,9 +93,9 @@ public bool ValidateProcessedBlock(Block processedBlock, TxReceipt[] receipts, B if (_logger.IsDebug) _logger.Debug($"Block invalidation should not be tracked"); return result; } - _invalidChainTracker.OnInvalidBlock(suggestedBlock.Hash!, suggestedBlock.ParentHash); + invalidChainTracker.OnInvalidBlock(suggestedBlock.Hash!, suggestedBlock.ParentHash); } - _invalidChainTracker.SetChildParent(suggestedBlock.Hash!, suggestedBlock.ParentHash!); + invalidChainTracker.SetChildParent(suggestedBlock.Hash!, suggestedBlock.ParentHash!); return result; } @@ -115,7 +107,7 @@ private static bool ShouldNotTrackInvalidation(BlockHeader header) public bool ValidateWithdrawals(Block block, out string? error) { - bool result = _baseValidator.ValidateWithdrawals(block, out error); + bool result = headerValidator.ValidateWithdrawals(block, out error); if (!result) { @@ -128,10 +120,10 @@ public bool ValidateWithdrawals(Block block, out string? error) return false; } - _invalidChainTracker.OnInvalidBlock(block.Hash!, block.ParentHash); + invalidChainTracker.OnInvalidBlock(block.Hash!, block.ParentHash); } - _invalidChainTracker.SetChildParent(block.Hash!, block.ParentHash!); + invalidChainTracker.SetChildParent(block.Hash!, block.ParentHash!); return result; } diff --git a/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs b/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs index 7c90d1db0e7..432673ee005 100644 --- a/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs +++ b/src/Nethermind/Nethermind.Optimism/InitializeBlockchainOptimism.cs @@ -18,94 +18,80 @@ namespace Nethermind.Optimism; -public class InitializeBlockchainOptimism : InitializeBlockchain +public class InitializeBlockchainOptimism(OptimismNethermindApi api) : InitializeBlockchain(api) { - private readonly OptimismNethermindApi _api; - private readonly IBlocksConfig _blocksConfig; - - public InitializeBlockchainOptimism(OptimismNethermindApi api) : base(api) - { - _api = api; - _blocksConfig = api.Config(); - } + private readonly IBlocksConfig _blocksConfig = api.Config(); protected override Task InitBlockchain() { - _api.SpecHelper = new(_api.ChainSpec.Optimism); - _api.L1CostHelper = new(_api.SpecHelper, _api.ChainSpec.Optimism.L1BlockAddress); + api.RegisterTxType(TxType.DepositTx, new OptimismTxDecoder(), Always.Valid); + + api.SpecHelper = new(api.ChainSpec.Optimism); + api.L1CostHelper = new(api.SpecHelper, api.ChainSpec.Optimism.L1BlockAddress); return base.InitBlockchain(); } protected override ITransactionProcessor CreateTransactionProcessor(CodeInfoRepository codeInfoRepository, VirtualMachine virtualMachine) { - if (_api.SpecProvider is null) throw new StepDependencyException(nameof(_api.SpecProvider)); - if (_api.SpecHelper is null) throw new StepDependencyException(nameof(_api.SpecHelper)); - if (_api.L1CostHelper is null) throw new StepDependencyException(nameof(_api.L1CostHelper)); - if (_api.WorldState is null) throw new StepDependencyException(nameof(_api.WorldState)); + if (api.SpecProvider is null) throw new StepDependencyException(nameof(api.SpecProvider)); + if (api.SpecHelper is null) throw new StepDependencyException(nameof(api.SpecHelper)); + if (api.L1CostHelper is null) throw new StepDependencyException(nameof(api.L1CostHelper)); + if (api.WorldState is null) throw new StepDependencyException(nameof(api.WorldState)); return new OptimismTransactionProcessor( - _api.SpecProvider, - _api.WorldState, + api.SpecProvider, + api.WorldState, virtualMachine, - _api.LogManager, - _api.L1CostHelper, - _api.SpecHelper, + api.LogManager, + api.L1CostHelper, + api.SpecHelper, codeInfoRepository ); } protected override IHeaderValidator CreateHeaderValidator() { - if (_api.InvalidChainTracker is null) throw new StepDependencyException(nameof(_api.InvalidChainTracker)); + if (api.InvalidChainTracker is null) throw new StepDependencyException(nameof(api.InvalidChainTracker)); OptimismHeaderValidator opHeaderValidator = new( - _api.BlockTree, - _api.SealValidator, - _api.SpecProvider, - _api.LogManager); + api.BlockTree, + api.SealValidator, + api.SpecProvider, + api.LogManager); - return new InvalidHeaderInterceptor(opHeaderValidator, _api.InvalidChainTracker, _api.LogManager); + return new InvalidHeaderInterceptor(opHeaderValidator, api.InvalidChainTracker, api.LogManager); } protected override IBlockValidator CreateBlockValidator() { - if (_api.InvalidChainTracker is null) throw new StepDependencyException(nameof(_api.InvalidChainTracker)); - if (_api.TxValidator is null) throw new StepDependencyException(nameof(_api.TxValidator)); - - BlockValidator blockValidator = new( - _api.TxValidator.WithValidator(TxType.DepositTx, new OptimismTxValidator()), - _api.HeaderValidator, - _api.UnclesValidator, - _api.SpecProvider, - _api.LogManager); - - return new InvalidBlockInterceptor(blockValidator, _api.InvalidChainTracker, _api.LogManager); + if (api.InvalidChainTracker is null) throw new StepDependencyException(nameof(api.InvalidChainTracker)); + return new InvalidBlockInterceptor(base.CreateBlockValidator(), api.InvalidChainTracker, api.LogManager); } protected override BlockProcessor CreateBlockProcessor(BlockCachePreWarmer? preWarmer) { - if (_api.DbProvider is null) throw new StepDependencyException(nameof(_api.DbProvider)); - if (_api.RewardCalculatorSource is null) throw new StepDependencyException(nameof(_api.RewardCalculatorSource)); - if (_api.TransactionProcessor is null) throw new StepDependencyException(nameof(_api.TransactionProcessor)); - if (_api.SpecHelper is null) throw new StepDependencyException(nameof(_api.SpecHelper)); - if (_api.SpecProvider is null) throw new StepDependencyException(nameof(_api.SpecProvider)); - if (_api.BlockTree is null) throw new StepDependencyException(nameof(_api.BlockTree)); - if (_api.WorldState is null) throw new StepDependencyException(nameof(_api.WorldState)); + if (api.DbProvider is null) throw new StepDependencyException(nameof(api.DbProvider)); + if (api.RewardCalculatorSource is null) throw new StepDependencyException(nameof(api.RewardCalculatorSource)); + if (api.TransactionProcessor is null) throw new StepDependencyException(nameof(api.TransactionProcessor)); + if (api.SpecHelper is null) throw new StepDependencyException(nameof(api.SpecHelper)); + if (api.SpecProvider is null) throw new StepDependencyException(nameof(api.SpecProvider)); + if (api.BlockTree is null) throw new StepDependencyException(nameof(api.BlockTree)); + if (api.WorldState is null) throw new StepDependencyException(nameof(api.WorldState)); Create2DeployerContractRewriter contractRewriter = - new(_api.SpecHelper, _api.SpecProvider, _api.BlockTree); + new(api.SpecHelper, api.SpecProvider, api.BlockTree); return new OptimismBlockProcessor( - _api.SpecProvider, - _api.BlockValidator, - _api.RewardCalculatorSource.Get(_api.TransactionProcessor!), - new BlockProcessor.BlockValidationTransactionsExecutor(_api.TransactionProcessor, _api.WorldState), - _api.WorldState, - _api.ReceiptStorage, - new BlockhashStore(_api.SpecProvider, _api.WorldState), - _api.LogManager, - _api.SpecHelper, + api.SpecProvider, + api.BlockValidator, + api.RewardCalculatorSource.Get(api.TransactionProcessor!), + new BlockProcessor.BlockValidationTransactionsExecutor(api.TransactionProcessor, api.WorldState), + api.WorldState, + api.ReceiptStorage, + new BlockhashStore(api.SpecProvider, api.WorldState), + api.LogManager, + api.SpecHelper, contractRewriter, new BlockProductionWithdrawalProcessor(new NullWithdrawalProcessor()), preWarmer: preWarmer); diff --git a/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs b/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs index e163c84f792..32db335b572 100644 --- a/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs @@ -10,11 +10,12 @@ namespace Nethermind.Optimism; -public class OptimismHeaderValidator : HeaderValidator +public class OptimismHeaderValidator( + IBlockTree? blockTree, + ISealValidator? sealValidator, + ISpecProvider? specProvider, + ILogManager? logManager) + : HeaderValidator(blockTree, sealValidator, specProvider, logManager) { - public OptimismHeaderValidator(IBlockTree? blockTree, ISealValidator? sealValidator, ISpecProvider? specProvider, ILogManager? logManager) : base(blockTree, sealValidator, specProvider, logManager) - { - } - protected override bool ValidateGasLimitRange(BlockHeader header, BlockHeader parent, IReleaseSpec spec, ref string? error) => true; } diff --git a/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/OptimismTxDecoder.cs b/src/Nethermind/Nethermind.Optimism/OptimismTxDecoder.cs similarity index 96% rename from src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/OptimismTxDecoder.cs rename to src/Nethermind/Nethermind.Optimism/OptimismTxDecoder.cs index 18173c5094d..9bb2e77fcdd 100644 --- a/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/OptimismTxDecoder.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismTxDecoder.cs @@ -4,8 +4,10 @@ using System; using Nethermind.Core; using Nethermind.Core.Crypto; +using Nethermind.Serialization.Rlp; +using Nethermind.Serialization.Rlp.TxDecoders; -namespace Nethermind.Serialization.Rlp.TxDecoders; +namespace Nethermind.Optimism; public sealed class OptimismTxDecoder(Func? transactionFactory = null) : BaseEIP1559TxDecoder(TxType.DepositTx, transactionFactory) where T : Transaction, new() diff --git a/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs b/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs deleted file mode 100644 index 9a6b38d87cb..00000000000 --- a/src/Nethermind/Nethermind.Optimism/OptimismTxValidator.cs +++ /dev/null @@ -1,18 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using Nethermind.Core; -using Nethermind.Core.Specs; -using Nethermind.TxPool; -using System.Diagnostics.CodeAnalysis; - -namespace Nethermind.Optimism; - -public sealed class OptimismTxValidator : ITxValidator -{ - public bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error) - { - error = null; - return true; - } -} diff --git a/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoder.cs b/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoder.cs index 0ef1eb96806..6e62bb887cc 100644 --- a/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoder.cs +++ b/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoder.cs @@ -2,8 +2,6 @@ // SPDX-License-Identifier: LGPL-3.0-only using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.Extensions.ObjectPool; using Nethermind.Core; @@ -30,20 +28,19 @@ public sealed class GeneratedTxDecoder : TxDecoder; public class TxDecoder : IRlpStreamDecoder, IRlpValueDecoder where T : Transaction, new() { - private readonly Dictionary _decoders; + private readonly ITxDecoder?[] _decoders = new ITxDecoder?[Transaction.MaxTxType + 1]; protected TxDecoder(Func? transactionFactory = null) { Func factory = transactionFactory ?? (() => new T()); - _decoders = new() { - { TxType.Legacy, new LegacyTxDecoder(factory) }, - { TxType.AccessList, new AccessListTxDecoder(factory) }, - { TxType.EIP1559, new EIP1559TxDecoder(factory) }, - { TxType.Blob, new BlobTxDecoder(factory) }, - { TxType.DepositTx, new OptimismTxDecoder(factory) } - }; + RegisterDecoder(new LegacyTxDecoder(factory)); + RegisterDecoder(new AccessListTxDecoder(factory)); + RegisterDecoder(new EIP1559TxDecoder(factory)); + RegisterDecoder(new BlobTxDecoder(factory)); } + public void RegisterDecoder(ITxDecoder decoder) => _decoders[(int)decoder.Type] = decoder; + public T? Decode(RlpStream rlpStream, RlpBehaviors rlpBehaviors = RlpBehaviors.None) { void ThrowIfLegacy(TxType txType1) @@ -83,7 +80,7 @@ void ThrowIfLegacy(TxType txType1) } private ITxDecoder GetDecoder(TxType txType) => - _decoders.TryGetValue(txType, out ITxDecoder decoder) + _decoders.TryGetByTxType(txType, out ITxDecoder decoder) ? decoder : throw new RlpException($"Unknown transaction type {txType}") { Data = { { "txType", txType } } }; diff --git a/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/ITxDecoder.cs b/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/ITxDecoder.cs index 30cdcec6a4c..07a16c24951 100644 --- a/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/ITxDecoder.cs +++ b/src/Nethermind/Nethermind.Serialization.Rlp/TxDecoders/ITxDecoder.cs @@ -6,7 +6,7 @@ namespace Nethermind.Serialization.Rlp.TxDecoders; -interface ITxDecoder +public interface ITxDecoder { public TxType Type { get; } diff --git a/src/Nethermind/Nethermind.Specs/ReleaseSpec.cs b/src/Nethermind/Nethermind.Specs/ReleaseSpec.cs index 249fbd2a200..a156d095eb3 100644 --- a/src/Nethermind/Nethermind.Specs/ReleaseSpec.cs +++ b/src/Nethermind/Nethermind.Specs/ReleaseSpec.cs @@ -59,7 +59,12 @@ public ReleaseSpec Clone() return (ReleaseSpec)MemberwiseClone(); } - public bool IsEip1559Enabled { get; set; } + public bool IsEip1559Enabled + { + get => _isEip1559Enabled || IsEip4844Enabled; + set => _isEip1559Enabled = value; + } + public bool IsEip3198Enabled { get; set; } public bool IsEip3529Enabled { get; set; } public bool IsEip3607Enabled { get; set; } @@ -96,6 +101,8 @@ public Address Eip4788ContractAddress public bool IsEip7709Enabled { get; set; } private Address _eip2935ContractAddress; + private bool _isEip1559Enabled; + public Address Eip2935ContractAddress { get => IsEip2935Enabled ? _eip2935ContractAddress : null; diff --git a/src/Nethermind/Nethermind.TxPool/Filters/MalformedTxFilter.cs b/src/Nethermind/Nethermind.TxPool/Filters/MalformedTxFilter.cs index 2de0143dd37..52f0753471d 100644 --- a/src/Nethermind/Nethermind.TxPool/Filters/MalformedTxFilter.cs +++ b/src/Nethermind/Nethermind.TxPool/Filters/MalformedTxFilter.cs @@ -10,27 +10,20 @@ namespace Nethermind.TxPool.Filters /// /// Filters out transactions that are not well formed (not conforming with the yellowpaper and EIPs) /// - internal sealed class MalformedTxFilter : IIncomingTxFilter + internal sealed class MalformedTxFilter( + IChainHeadSpecProvider specProvider, + ITxValidator txValidator, + ILogger logger) + : IIncomingTxFilter { - private readonly ITxValidator _txValidator; - private readonly IChainHeadSpecProvider _specProvider; - private readonly ILogger _logger; - - public MalformedTxFilter(IChainHeadSpecProvider specProvider, ITxValidator txValidator, ILogger logger) - { - _txValidator = txValidator; - _specProvider = specProvider; - _logger = logger; - } - public AcceptTxResult Accept(Transaction tx, ref TxFilteringState state, TxHandlingOptions txHandlingOptions) { - IReleaseSpec spec = _specProvider.GetCurrentHeadSpec(); - if (!_txValidator.IsWellFormed(tx, spec, out _)) + IReleaseSpec spec = specProvider.GetCurrentHeadSpec(); + if (!txValidator.IsWellFormed(tx, spec)) { Metrics.PendingTransactionsMalformed++; // It may happen that other nodes send us transactions that were signed for another chain or don't have enough gas. - if (_logger.IsTrace) _logger.Trace($"Skipped adding transaction {tx.ToString(" ")}, invalid transaction."); + if (logger.IsTrace) logger.Trace($"Skipped adding transaction {tx.ToString(" ")}, invalid transaction."); return AcceptTxResult.Invalid; } diff --git a/src/Nethermind/Nethermind.TxPool/ITxValidator.cs b/src/Nethermind/Nethermind.TxPool/ITxValidator.cs index 1510c9a165a..64c2075715f 100644 --- a/src/Nethermind/Nethermind.TxPool/ITxValidator.cs +++ b/src/Nethermind/Nethermind.TxPool/ITxValidator.cs @@ -3,12 +3,11 @@ using Nethermind.Core; using Nethermind.Core.Specs; -using System.Diagnostics.CodeAnalysis; namespace Nethermind.TxPool { public interface ITxValidator { - bool IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec, [NotNullWhen(false)] out string? error); + public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec); } }