Make withdrawals nullable and check that

NethermindEth · Oct 26, 2022 · 9e6ee93 · 9e6ee93
1 parent 37ebce6
commit 9e6ee93
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 19 deletions.
diff --git a/src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs b/src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs
@@ -31,7 +31,7 @@ public class PayloadAttributes
 
         public Address SuggestedFeeRecipient { get; set; }
 
-        public IList<Withdrawal> Withdrawals { get; set; }
+        public IList<Withdrawal>? Withdrawals { get; set; }
 
         /// <summary>
         /// GasLimit

diff --git a/src/Nethermind/Nethermind.Core/Block.cs b/src/Nethermind/Nethermind.Core/Block.cs
@@ -37,7 +37,7 @@ public Block(
             BlockHeader blockHeader,
             IEnumerable<Transaction> transactions,
             IEnumerable<BlockHeader> uncles,
-            IEnumerable<Withdrawal>? withdrawals = null) // TODO Remove withdrawals default value
+            IEnumerable<Withdrawal>? withdrawals = null)
         {
             Header = blockHeader;
             Body = new BlockBody(transactions.ToArray(), uncles.ToArray(), withdrawals?.ToArray());
@@ -61,7 +61,7 @@ public Block(BlockHeader blockHeader) : this(blockHeader, BlockBody.Empty) { }
 
         public BlockHeader[] Uncles => Body.Uncles; // do not add setter here
 
-        public Withdrawal[] Withdrawals => Body.Withdrawals;
+        public Withdrawal[]? Withdrawals => Body.Withdrawals;
 
         public Keccak? Hash => Header.Hash; // do not add setter here
 

diff --git a/src/Nethermind/Nethermind.Core/BlockBody.cs b/src/Nethermind/Nethermind.Core/BlockBody.cs
@@ -20,12 +20,11 @@ namespace Nethermind.Core
 {
     public class BlockBody
     {
-        // TODO Remove withdrawals default value
         public BlockBody(Transaction[]? transactions, BlockHeader[]? uncles, Withdrawal[]? withdrawals = null)
         {
             Transactions = transactions ?? Array.Empty<Transaction>();
             Uncles = uncles ?? Array.Empty<BlockHeader>();
-            Withdrawals = withdrawals ?? Array.Empty<Withdrawal>();
+            Withdrawals = withdrawals;
         }
 
         public BlockBody() : this(null, null, null) { }
@@ -34,18 +33,18 @@ public BlockBody() : this(null, null, null) { }
 
         public BlockBody WithChangedUncles(BlockHeader[] uncles) => new(Transactions, uncles, Withdrawals);
 
-        public BlockBody WithChangedWithdrawals(Withdrawal[] withdrawals) => new(Transactions, Uncles, withdrawals);
+        public BlockBody WithChangedWithdrawals(Withdrawal[]? withdrawals) => new(Transactions, Uncles, withdrawals);
 
         public static BlockBody WithOneTransactionOnly(Transaction tx) => new(new[] { tx }, null, null);
 
         public Transaction[] Transactions { get; internal set; }
 
         public BlockHeader[] Uncles { get; }
 
-        public Withdrawal[] Withdrawals { get; internal set; }
+        public Withdrawal[]? Withdrawals { get; internal set; }
 
         public static BlockBody Empty { get; } = new();
 
-        public bool IsEmpty => Transactions.Length == 0 && Uncles.Length == 0 && Withdrawals.Length == 0;
+        public bool IsEmpty => Transactions.Length == 0 && Uncles.Length == 0 && (Withdrawals?.Length ?? 0) == 0;
     }
 }
diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.Setup.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.Setup.cs
@@ -104,6 +104,7 @@ private IEngineRpcModule CreateEngineModule(MergeTestBlockchain chain, ISyncConf
                     chain.BeaconSync,
                     chain.BeaconPivot,
                     peerRefresher,
+                    chain.SpecProvider,
                     chain.LogManager),
                 new ExecutionStatusHandler(chain.BlockTree),
                 new GetPayloadBodiesV1Handler(chain.BlockTree, chain.LogManager),

diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/V1/ExecutionPayloadV1.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/V1/ExecutionPayloadV1.cs
@@ -114,7 +114,7 @@ public virtual bool TryGetBlock(out Block? block, UInt256? totalDifficulty = nul
         /// Gets or sets an RLP-encoded collection of <see cref="Withdrawal"/> as defined in
         /// <see href="https://eips.ethereum.org/EIPS/eip-4895">EIP-4895</see>.
         /// </summary>
-        public IEnumerable<byte[]> Withdrawals { get; set; } = Enumerable.Empty<byte[]>();
+        public IEnumerable<byte[]>? Withdrawals { get; set; }
 
         public override string ToString() => $"{BlockNumber} ({BlockHash})";
 
@@ -130,7 +130,7 @@ public Transaction[] GetTransactions() => Transactions
         /// Decodes the <see cref="Withdrawals"/> and returns a collection of <see cref="Withdrawal"/>.
         /// </summary>
         /// <returns>An RLP-decoded collection of <see cref="Withdrawal"/>.</returns>
-        public IEnumerable<Withdrawal> DecodedWithdrawals() => Withdrawals
+        public IEnumerable<Withdrawal>? DecodedWithdrawals() => Withdrawals?
             .Select(w => Rlp.Decode<Withdrawal>(w, RlpBehaviors.None));
     }
 }
diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/ForkchoiceUpdatedV1Handler.cs b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/ForkchoiceUpdatedV1Handler.cs
@@ -25,13 +25,15 @@
 using Nethermind.Consensus.Producers;
 using Nethermind.Core;
 using Nethermind.Core.Crypto;
+using Nethermind.Core.Specs;
 using Nethermind.Crypto;
 using Nethermind.JsonRpc;
 using Nethermind.Logging;
 using Nethermind.Merge.Plugin.BlockProduction;
 using Nethermind.Merge.Plugin.Data.V1;
 using Nethermind.Merge.Plugin.InvalidChainTracker;
 using Nethermind.Merge.Plugin.Synchronization;
+using Org.BouncyCastle.Asn1.Cms;
 
 namespace Nethermind.Merge.Plugin.Handlers.V1
 {
@@ -52,6 +54,7 @@ public class ForkchoiceUpdatedV1Handler : IForkchoiceUpdatedV1Handler
         private readonly IBeaconPivot _beaconPivot;
         private readonly ILogger _logger;
         private readonly IPeerRefresher _peerRefresher;
+        private readonly ISpecProvider _specProvider;
 
         public ForkchoiceUpdatedV1Handler(
             IBlockTree blockTree,
@@ -64,6 +67,7 @@ public ForkchoiceUpdatedV1Handler(
             IMergeSyncController mergeSyncController,
             IBeaconPivot beaconPivot,
             IPeerRefresher peerRefresher,
+            ISpecProvider specProvider,
             ILogManager logManager)
         {
             _blockTree = blockTree ?? throw new ArgumentNullException(nameof(blockTree));
@@ -76,6 +80,7 @@ public ForkchoiceUpdatedV1Handler(
             _mergeSyncController = mergeSyncController;
             _beaconPivot = beaconPivot;
             _peerRefresher = peerRefresher;
+            _specProvider = specProvider;
             _logger = logManager.GetClassLogger();
         }
 
@@ -106,6 +111,28 @@ public Task<ResultWrapper<ForkchoiceUpdatedV1Result>> Handle(ForkchoiceStateV1 f
                 }
                 return ForkchoiceUpdatedV1Result.Syncing;
             }
+            else if (payloadAttributes is not null)
+            {
+                var spec = _specProvider.GetSpec(newHeadBlock.Number);
+
+                if (spec.IsEip4895Enabled && payloadAttributes.Withdrawals is null)
+                {
+                    var error = "Withdrawals are null with EIP-4895 activated.";
+
+                    if (_logger.IsInfo) _logger.Info($"Invalid payload attributes: {error}");
+
+                    return ForkchoiceUpdatedV1Result.Error(error, MergeErrorCodes.InvalidPayloadAttributes);
+                }
+
+                if (!spec.IsEip4895Enabled && payloadAttributes.Withdrawals is not null)
+                {
+                    var error = "Withdrawals are not null with EIP-4895 not activated.";
+
+                    if (_logger.IsInfo) _logger.Info($"Invalid payload attributes: {error}");
+
+                    return ForkchoiceUpdatedV1Result.Error(error, MergeErrorCodes.InvalidPayloadAttributes);
+                }
+            }
 
             BlockInfo? blockInfo = _blockTree.GetInfo(newHeadBlock.Number, newHeadBlock.GetOrCalculateHash()).Info;
             if (blockInfo == null)

diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/NewPayloadV1Handler.cs b/src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/NewPayloadV1Handler.cs
@@ -122,6 +122,26 @@ public async Task<ResultWrapper<PayloadStatusV1>> HandleAsync(ExecutionPayloadV1
                 return NewPayloadV1Result.Invalid(lastValidHash, $"Block {request} is known to be a part of an invalid chain.");
             }
 
+            var spec = _specProvider.GetSpec(block.Number);
+
+            if (spec.IsEip4895Enabled && request.Withdrawals is null)
+            {
+                var error = $"Withdrawals are null in block {request.BlockHash} with EIP-4895 activated.";
+
+                if (_logger.IsInfo) _logger.Info($"Invalid: {error}");
+
+                return NewPayloadV1Result.Invalid(lastValidHash, error);
+            }
+
+            if (!spec.IsEip4895Enabled && request.Withdrawals is not null)
+            {
+                var error = $"Withdrawals are not null in block {request.BlockHash} with EIP-4895 not activated.";
+
+                if (_logger.IsInfo) _logger.Info($"Invalid: {error}");
+
+                return NewPayloadV1Result.Invalid(lastValidHash, error);
+            }
+
             if (block.Header.Number <= _syncConfig.PivotNumberParsed)
             {
                 if (_logger.IsInfo) _logger.Info($"Pre-pivot block, ignored and returned Syncing. Result of {requestStr}.");

diff --git a/src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs b/src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs
@@ -350,6 +350,7 @@ public Task InitRpcModules()
                         _beaconSync,
                         _beaconPivot,
                         _peerRefresher,
+                        _api.SpecProvider,
                         _api.LogManager),
                     new ExecutionStatusHandler(_api.BlockTree),
                     new GetPayloadBodiesV1Handler(_api.BlockTree, _api.LogManager),

diff --git a/src/Nethermind/Nethermind.Serialization.Rlp/BlockDecoder.cs b/src/Nethermind/Nethermind.Serialization.Rlp/BlockDecoder.cs
@@ -83,7 +83,7 @@ public class BlockDecoder : IRlpValueDecoder<Block>, IRlpStreamDecoder<Block>
             return new Block(header, transactions, uncleHeaders, withdrawals);
         }
 
-        private (int Total, int Txs, int Uncles, int Withdrawals) GetContentLength(Block item, RlpBehaviors rlpBehaviors)
+        private (int Total, int Txs, int Uncles, int? Withdrawals) GetContentLength(Block item, RlpBehaviors rlpBehaviors)
         {
             int contentLength = _headerDecoder.GetLength(item.Header, rlpBehaviors);
 
@@ -93,8 +93,10 @@ public class BlockDecoder : IRlpValueDecoder<Block>, IRlpStreamDecoder<Block>
             int unclesLength = GetUnclesLength(item, rlpBehaviors);
             contentLength += Rlp.LengthOfSequence(unclesLength);
 
-            int withdrawalsLength = GetWithdrawalsLength(item, rlpBehaviors);
-            contentLength += Rlp.LengthOfSequence(withdrawalsLength);
+            int? withdrawalsLength = GetWithdrawalsLength(item, rlpBehaviors);
+
+            if (withdrawalsLength.HasValue)
+                contentLength += Rlp.LengthOfSequence(withdrawalsLength.Value);
 
             return (contentLength, txLength, unclesLength, withdrawalsLength);
         }
@@ -121,8 +123,8 @@ private int GetTxLength(Block item, RlpBehaviors rlpBehaviors)
             return txLength;
         }
 
-        private int GetWithdrawalsLength(Block item, RlpBehaviors rlpBehaviors) =>
-            item.Withdrawals.Sum(w => _withdrawalDecoder.GetLength(w, rlpBehaviors));
+        private int? GetWithdrawalsLength(Block item, RlpBehaviors rlpBehaviors) =>
+            item.Withdrawals?.Sum(w => _withdrawalDecoder.GetLength(w, rlpBehaviors));
 
         public int GetLength(Block? item, RlpBehaviors rlpBehaviors)
         {
@@ -206,7 +208,7 @@ public void Encode(RlpStream stream, Block? item, RlpBehaviors rlpBehaviors = Rl
                 return;
             }
 
-            (int contentLength, int txsLength, int unclesLength, int withdrawalsLength) = GetContentLength(item, rlpBehaviors);
+            (int contentLength, int txsLength, int unclesLength, int? withdrawalsLength) = GetContentLength(item, rlpBehaviors);
             stream.StartSequence(contentLength);
             stream.Encode(item.Header);
             stream.StartSequence(txsLength);
@@ -221,10 +223,14 @@ public void Encode(RlpStream stream, Block? item, RlpBehaviors rlpBehaviors = Rl
                 stream.Encode(item.Uncles[i]);
             }
 
-            stream.StartSequence(withdrawalsLength);
-            for (int i = 0; i < item.Withdrawals.Length; i++)
+            if (withdrawalsLength.HasValue)
             {
-                stream.Encode(item.Withdrawals[i]);
+                stream.StartSequence(withdrawalsLength.Value);
+
+                for (int i = 0; i < item.Withdrawals.Length; i++)
+                {
+                    stream.Encode(item.Withdrawals[i]);
+                }
             }
         }
     }