Fix randomly failing receipt deserialization (#5120)

* Fix randomly failing receipt deserialization * Fix test * Addressing comment
NethermindEth · Jan 10, 2023 · 1edfd97 · 1edfd97
1 parent 9ca61f9
commit 1edfd97
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 3 deletions.
diff --git a/...ermind/Nethermind.Network.Test/P2P/Subprotocols/Eth/V63/ReceiptsMessageSerializerTests.cs b/...ermind/Nethermind.Network.Test/P2P/Subprotocols/Eth/V63/ReceiptsMessageSerializerTests.cs
@@ -1,13 +1,15 @@
 // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
 // SPDX-License-Identifier: LGPL-3.0-only
 
+using DotNetty.Buffers;
 using FluentAssertions;
 using Nethermind.Blockchain.Receipts;
 using Nethermind.Core;
 using Nethermind.Core.Extensions;
 using Nethermind.Specs;
 using Nethermind.Core.Test.Builders;
 using Nethermind.Network.P2P.Subprotocols.Eth.V63.Messages;
+using Nethermind.Serialization.Rlp;
 using NUnit.Framework;
 
 namespace Nethermind.Network.Test.P2P.Subprotocols.Eth.V63
@@ -111,6 +113,23 @@ public void Deserialize_empty()
             serializer.Deserialize(new byte[0]).TxReceipts.Should().HaveCount(0);
         }
 
+        [Test]
+        public void Deserialize_non_empty_but_bytebuffer_starts_with_empty()
+        {
+            TxReceipt[][] data = { new[] { Build.A.Receipt.WithAllFieldsFilled.TestObject, Build.A.Receipt.WithAllFieldsFilled.WithBlockNumber(0).TestObject }, new[] { Build.A.Receipt.WithAllFieldsFilled.TestObject, Build.A.Receipt.WithAllFieldsFilled.TestObject } };
+            ReceiptsMessage message = new(data);
+            ReceiptsMessageSerializer serializer = new(RopstenSpecProvider.Instance);
+
+            IByteBuffer buffer = Unpooled.Buffer(serializer.GetLength(message, out int _) + 1);
+            buffer.WriteByte(Rlp.OfEmptySequence[0]);
+            buffer.ReadByte();
+
+            serializer.Serialize(buffer, message);
+            ReceiptsMessage deserialized = serializer.Deserialize(buffer);
+
+            deserialized.TxReceipts.Length.Should().Be(data.Length);
+        }
+
         [Test]
         public void Roundtrip_mainnet_sample()
         {

diff --git a/src/Nethermind/Nethermind.Network/P2P/ProtocolHandlers/ProtocolHandlerBase.cs b/src/Nethermind/Nethermind.Network/P2P/ProtocolHandlers/ProtocolHandlerBase.cs
@@ -59,7 +59,14 @@ protected T Deserialize<T>(IByteBuffer data) where T : P2PMessage
         {
             try
             {
-                return _serializer.Deserialize<T>(data);
+                int originalReaderIndex = data.ReaderIndex;
+                T result = _serializer.Deserialize<T>(data);
+                if (data.IsReadable())
+                {
+                    throw new IncompleteDeserializationException(
+                        $"Incomplete deserialization detected. Buffer is still readable. Read bytes: {data.ReaderIndex - originalReaderIndex}. Readable bytes: {data.ReadableBytes}");
+                }
+                return result;
             }
             catch (RlpException e)
             {
@@ -138,4 +145,11 @@ protected void ReportIn(string messageInfo)
 
         public abstract event EventHandler<ProtocolEventArgs> SubprotocolRequested;
     }
+
+    public class IncompleteDeserializationException : Exception
+    {
+        public IncompleteDeserializationException(string msg) : base(msg)
+        {
+        }
+    }
 }
diff --git a/src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/V63/Messages/ReceiptsMessage.cs b/src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/V63/Messages/ReceiptsMessage.cs
@@ -12,6 +12,9 @@ public class ReceiptsMessage : P2PMessage
         public override int PacketType { get; } = Eth63MessageCode.Receipts;
         public override string Protocol { get; } = "eth";
 
+        private static ReceiptsMessage? _empty;
+        public static ReceiptsMessage Empty => _empty ??= new ReceiptsMessage(null);
+
         public ReceiptsMessage(TxReceipt[][] txReceipts)
         {
             TxReceipts = txReceipts ?? new TxReceipt[0][];

diff --git a/...hermind/Nethermind.Network/P2P/Subprotocols/Eth/V63/Messages/ReceiptsMessageSerializer.cs b/...hermind/Nethermind.Network/P2P/Subprotocols/Eth/V63/Messages/ReceiptsMessageSerializer.cs
@@ -31,7 +31,7 @@ public void Serialize(IByteBuffer byteBuffer, ReceiptsMessage message)
                         b.Select(
                             n => n is null
                                 ? Rlp.OfEmptySequence
-                                // for TxReceipt there is no timestamp, as such, we are using IReceiptSpec. wonder how we can metigate this later if future EIPs affecting this are added. 
+                                // for TxReceipt there is no timestamp, as such, we are using IReceiptSpec. wonder how we can metigate this later if future EIPs affecting this are added.
                                 : _decoder.Encode(n, _specProvider.GetReceiptSpec(n.BlockNumber).IsEip658Enabled ? RlpBehaviors.Eip658Receipts : RlpBehaviors.None)).ToArray())).ToArray());
 
             RlpStream rlpStream = new NettyRlpStream(byteBuffer);
@@ -40,7 +40,16 @@ public void Serialize(IByteBuffer byteBuffer, ReceiptsMessage message)
 
         public ReceiptsMessage Deserialize(IByteBuffer byteBuffer)
         {
-            if (byteBuffer.Array.Length == 0 || byteBuffer.Array.First() == Rlp.OfEmptySequence[0]) return new ReceiptsMessage(null);
+            if (byteBuffer.ReadableBytes == 0)
+            {
+                return ReceiptsMessage.Empty;
+            }
+
+            if (byteBuffer.GetByte(byteBuffer.ReaderIndex) == Rlp.OfEmptySequence[0])
+            {
+                byteBuffer.ReadByte();
+                return ReceiptsMessage.Empty;
+            }
 
             RlpStream rlpStream = new NettyRlpStream(byteBuffer);
             return Deserialize(rlpStream);