Skip to content

test: add hand-derived golden tests for eth/71 and snap serializers #12699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benaadams merged 3 commits into from
Aug 7, 2026
Merged

test: add hand-derived golden tests for eth/71 and snap serializers #12699

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1949d27
test(network): pin eth/71 and snap serializer wire encodings
benaadams Aug 5, 2026
c2a3860
test(network): share repeated snap golden fragments
benaadams Aug 5, 2026
46a16da
test(network): address review feedback on snap golden tests
benaadams Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions ...ethermind.Network.Test/P2P/Subprotocols/Eth/V71/BlockAccessListsMessageSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Nethermind.Network.Test.P2P.Subprotocols.Eth.V71;
public class BlockAccessListsMessageSerializerTests
{
[TestCaseSource(nameof(BlockAccessListsRoundtripCases))]
public void Roundtrip(Func<BlockAccessListsMessage> buildMessage, string? expectedData)
public void Roundtrip(Func<BlockAccessListsMessage> buildMessage, string expectedData)
{
BlockAccessListsMessageSerializer serializer = new();
using BlockAccessListsMessage msg = buildMessage();
Expand All @@ -35,11 +35,7 @@ public void Roundtrip(Func<BlockAccessListsMessage> buildMessage, string? expect
buffer.SetReaderIndex(0);
string allHex = buffer.ReadAllHex();
Assert.That(buffer2.ReadAllHex(), Is.EqualTo(allHex), "test zero");

if (expectedData is not null)
{
Assert.That(allHex, Is.EqualTo(expectedData));
}
Assert.That(allHex, Is.EqualTo(expectedData));
}

[TestCaseSource(nameof(BlockAccessListsRejectionCases))]
Expand All @@ -55,7 +51,7 @@ private static IEnumerable<TestCaseData> BlockAccessListsRoundtripCases()
{
yield return new TestCaseData(
new Func<BlockAccessListsMessage>(() => BuildMessage(42)),
null)
"c22ac0")
.SetName("Roundtrip_empty");
yield return new TestCaseData(
new Func<BlockAccessListsMessage>(() => BuildMessage(43, (byte[]?)null)),
Expand All @@ -69,9 +65,10 @@ private static IEnumerable<TestCaseData> BlockAccessListsRoundtripCases()
new Func<BlockAccessListsMessage>(() => BuildMessage(45, [0xc1, 0x80], [0xc2, 0x01, 0x02], null)),
"c82dc6c180c2010280")
.SetName("Roundtrip_multiple_bals");
// A negative request id encodes as its unsigned two's-complement value.
yield return new TestCaseData(
new Func<BlockAccessListsMessage>(() => BuildMessage(-1)),
null)
"ca88ffffffffffffffffc0")
.SetName("Roundtrip_negative_request_id");
}

Expand Down Expand Up @@ -128,31 +125,36 @@ private static void AssertBlockAccessListsMessage(BlockAccessListsMessage actual
public class GetBlockAccessListsMessageSerializerTests
{
[TestCaseSource(nameof(GetBlockAccessListsRoundtripCases))]
public void Roundtrip(Func<GetBlockAccessListsMessage> buildMessage)
public void Roundtrip(Func<GetBlockAccessListsMessage> buildMessage, string expectedData)
{
GetBlockAccessListsMessageSerializer serializer = new();
using GetBlockAccessListsMessage msg = buildMessage();
SerializerTester.TestZero(serializer, msg);
SerializerTester.TestZero(serializer, msg, expectedData);
}

private static IEnumerable<TestCaseData> GetBlockAccessListsRoundtripCases()
{
yield return new TestCaseData(
new Func<GetBlockAccessListsMessage>(() => new GetBlockAccessListsMessage(99, ArrayPoolList<Hash256>.Empty())))
new Func<GetBlockAccessListsMessage>(() => new GetBlockAccessListsMessage(99, ArrayPoolList<Hash256>.Empty())),
"c263c0")
.SetName("Roundtrip_empty_hashes");
// Each hash encodes as 0xa0 + 32 bytes.
yield return new TestCaseData(
new Func<GetBlockAccessListsMessage>(() => new GetBlockAccessListsMessage(100, new ArrayPoolList<Hash256>(1)
{
Keccak.Zero
})))
})),
"e364e1a00000000000000000000000000000000000000000000000000000000000000000")
.SetName("Roundtrip_single_hash");
// The hashes are Keccak.Zero, keccak("A"), and keccak("B").

@claude claude Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — this comment restates the three lines directly below it (Keccak.Zero, TestItem.KeccakA, TestItem.KeccakB), which AGENTS.md asks you to skip ("Comments that merely restate the code are noise"). The other two comments added in this file earn their keep — // Each hash encodes as 0xa0 + 32 bytes. and // A negative request id encodes as its unsigned two's-complement value. both explain a derivation a reader can't get from the code. This one doesn't.

If the intent was to make the two 32-byte literals traceable to their inputs, the useful version says that instead — e.g. // keccak("A") and keccak("B"); the same values #12696 pins in EthSerializerGoldens. — otherwise dropping the line is cleaner.

Fix this →

yield return new TestCaseData(
new Func<GetBlockAccessListsMessage>(() => new GetBlockAccessListsMessage(101, new ArrayPoolList<Hash256>(3)
{
Keccak.Zero,
TestItem.KeccakA,
TestItem.KeccakB
})))
})),
"f86665f863a00000000000000000000000000000000000000000000000000000000000000000a003783fac2efed8fbc9ad443e592ee30e61d65f471140c10ca155e937b435b760a01f675bff07515f5df96737194ea945c36c41e7b4fcef307b7cd4d0e602a69111")
.SetName("Roundtrip_multiple_hashes");
}
}
14 changes: 14 additions & 0 deletions ...hermind.Network.Test/P2P/Subprotocols/Snap/V1/Messages/ByteCodesMessageSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ public void Roundtrip()
{
ArrayPoolList<byte[]> data = new(2) { new byte[] { 0xde, 0xad, 0xc0, 0xde }, new byte[] { 0xfe, 0xed } };

ByteCodesMessage message = new(new ByteArrayListAdapter(data)) { RequestId = 1 };

ByteCodesMessageSerializer serializer = new();

// The message encodes as [requestId, codes].
SerializerTester.TestZero(serializer, message, "ca01c884deadc0de82feed");
}

[Test]
public void Roundtrip_random_request_id()

@claude claude Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — this new fixture member is the one place the PR moves against its own thesis: it adds a fresh non-deterministic test (MessageConstants.Random-assigned id via the constructor) while the rest of the diff is removing random ids. Its marginal coverage over what's already here looks thin — DecodeEncodeDecodeEmpty further down already pins the 8-byte-request-id path with exact bytes (202, 136, ... = ca 88 176a156ae55348b0 c0), so the only thing this adds is "an arbitrary id survives a roundtrip".

It also duplicates the three setup lines of Roundtrip verbatim, which AGENTS.md calls out directly ("When tests differ only by inputs and expected outputs, parameterize a single test with [TestCase(...)]"). Both cases differ only by id and expectation:

        [TestCase(1L, "ca01c884deadc0de82feed")]
        [TestCase(long.MaxValue, "d2887fffffffffffffffc884deadc0de82feed")]
        public void Roundtrip(long requestId, string expectedData)
        {
            ArrayPoolList<byte[]> data = new(2) { new byte[] { 0xde, 0xad, 0xc0, 0xde }, new byte[] { 0xfe, 0xed } };

            ByteCodesMessage message = new(new ByteArrayListAdapter(data)) { RequestId = requestId };

            ByteCodesMessageSerializer serializer = new();

            // The message encodes as [requestId, codes].
            SerializerTester.TestZero(serializer, message, expectedData);
        }

That keeps the multi-byte-id path covered and deterministic. (Second vector derived the same way: 9 + 9 = 18 = 0x12 payload → 0xd2; please re-verify before taking it.) Sibling fixtures also spell case names Roundtrip_Empty / Roundtrip_Many, so Roundtrip_random_request_id is off-convention if it stays.

Fix this →

{
ArrayPoolList<byte[]> data = new(2) { new byte[] { 0xde, 0xad, 0xc0, 0xde }, new byte[] { 0xfe, 0xed } };

// The constructor assigns a random request id; it must roundtrip unchanged.
ByteCodesMessage message = new(new ByteArrayListAdapter(data));

ByteCodesMessageSerializer serializer = new();
Expand Down
24 changes: 18 additions & 6 deletions ...d.Network.Test/P2P/Subprotocols/Snap/V1/Messages/GetAccountRangeMessageSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: LGPL-3.0-only

using Nethermind.Core.Crypto;
using Nethermind.Network.P2P;
using Nethermind.Network.P2P.Subprotocols.Snap.V1.Messages;
using NUnit.Framework;

Expand All @@ -16,8 +15,8 @@ public void Roundtrip()
{
GetAccountRangeMessage msg = new()
{
RequestId = MessageConstants.Random.NextLong(),
AccountRange = new(Keccak.OfAnEmptyString, new Hash256("0x15d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"), new Hash256("0x20d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")),
RequestId = 1111,
AccountRange = new(Keccak.OfAnEmptyString, SnapSerializerGoldens.RangeStart, SnapSerializerGoldens.RangeLimit),
ResponseBytes = 10
};
GetAccountRangeMessageSerializer serializer = new();
Expand All @@ -32,15 +31,22 @@ public void Roundtrip()
Assert.That(deserializedMsg.AccountRange.LimitHash, Is.EqualTo(msg.AccountRange.LimitHash));
Assert.That(deserializedMsg.ResponseBytes, Is.EqualTo(msg.ResponseBytes));

SerializerTester.TestZero(serializer, msg);
// The message encodes as [requestId, rootHash, startingHash, limitHash, responseBytes].
SerializerTester.TestZero(serializer, msg,
"f867" + SnapSerializerGoldens.RequestId1111Rlp +
SnapSerializerGoldens.EmptyStringKeccakRlp +
SnapSerializerGoldens.RangeStartRlp +
SnapSerializerGoldens.RangeLimitRlp +
"0a");
}

[Test]
public void Roundtrip_Defaults()
{
GetAccountRangeMessage msg = new()
{
RequestId = MessageConstants.Random.NextLong(),
// long.MaxValue also pins the eight-byte request-id encoding.
RequestId = long.MaxValue,
AccountRange = new(Keccak.OfAnEmptyString, Keccak.Zero)
};
GetAccountRangeMessageSerializer serializer = new();
Expand All @@ -51,7 +57,13 @@ public void Roundtrip_Defaults()
Assert.That(deserializedMsg.AccountRange.LimitHash, Is.EqualTo(Keccak.MaxValue));
Assert.That(deserializedMsg.ResponseBytes, Is.EqualTo(1000_000));

SerializerTester.TestZero(serializer, msg);
// A null limit hash goes on the wire as Keccak.MaxValue; response bytes 0 as 1000000.
SerializerTester.TestZero(serializer, msg,
"f870" + "887fffffffffffffff" +
SnapSerializerGoldens.EmptyStringKeccakRlp +
"a00000000000000000000000000000000000000000000000000000000000000000" +
"a0ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" +
"830f4240");
}
}
}
5 changes: 3 additions & 2 deletions ...mind.Network.Test/P2P/Subprotocols/Snap/V1/Messages/GetByteCodesMessageSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,15 @@ public void Roundtrip_Empty()
{
GetByteCodesMessage msg = new()
{
RequestId = MessageConstants.Random.NextLong(),
RequestId = 1111,
Hashes = ArrayPoolList<ValueHash256>.Empty(),
Bytes = 10
};

GetByteCodesMessageSerializer serializer = new();

SerializerTester.TestZero(serializer, msg);
// The message encodes as [requestId, hashes, bytes].
SerializerTester.TestZero(serializer, msg, "c5" + SnapSerializerGoldens.RequestId1111Rlp + "c0" + "0a");
}

[Test]
Expand Down
19 changes: 13 additions & 6 deletions ....Network.Test/P2P/Subprotocols/Snap/V1/Messages/GetStorageRangesMessageSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public void Roundtrip_Many()
{
RootHash = TestItem.KeccakA,
Accounts = TestItem.Keccaks.Select(static k => new PathWithAccount(k, null)).ToPooledList(TestItem.Keccaks.Length),
StartingHash = new Hash256("0x15d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"),
LimitHash = new Hash256("0x20d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")
StartingHash = SnapSerializerGoldens.RangeStart,
LimitHash = SnapSerializerGoldens.RangeLimit
},
ResponseBytes = 1000
};
Expand All @@ -44,19 +44,26 @@ public void Roundtrip_Empty()
{
GetStorageRangeMessage msg = new()
{
RequestId = MessageConstants.Random.NextLong(),
RequestId = 1111,
StorageRange = new()
{
RootHash = Keccak.OfAnEmptyString,
Accounts = ArrayPoolList<PathWithAccount>.Empty(),
StartingHash = new Hash256("0x15d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"),
LimitHash = new Hash256("0x20d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")
StartingHash = SnapSerializerGoldens.RangeStart,
LimitHash = SnapSerializerGoldens.RangeLimit
},
ResponseBytes = 1000
};
GetStorageRangesMessageSerializer serializer = new();

SerializerTester.TestZero(serializer, msg);
// The message encodes as [requestId, rootHash, accountPaths, startingHash, limitHash, responseBytes].
SerializerTester.TestZero(serializer, msg,
"f86a" + SnapSerializerGoldens.RequestId1111Rlp +
SnapSerializerGoldens.EmptyStringKeccakRlp +
"c0" +
SnapSerializerGoldens.RangeStartRlp +
SnapSerializerGoldens.RangeLimitRlp +
"8203e8");
}

[Test]
Expand Down
39 changes: 39 additions & 0 deletions ...ermind/Nethermind.Network.Test/P2P/Subprotocols/Snap/V1/Messages/SnapSerializerGoldens.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using Nethermind.Core.Crypto;

namespace Nethermind.Network.Test.P2P.Subprotocols.Snap.V1.Messages;

/// <summary>
/// Shared inputs and hand-derived RLP fragments for the snap serializer goldens.
/// </summary>
/// <remarks>
/// Each golden fragment and its input come from one hex constant, so the
/// expectation cannot drift from the input. The values are verified with an
/// independent encoder (pyrlp + pycryptodome keccak).
/// </remarks>
internal static class SnapSerializerGoldens
{
private const string EmptyStringKeccakHex = "c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470";
private const string RangeStartHex = "15d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470";
private const string RangeLimitHex = "20d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470";

/// <summary>Request id 1111 as an RLP item: 0x82 length prefix + 0x0457.</summary>

@claude claude Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — two nits on the drift-proofing story in this file.

  1. The <remarks> says "Each golden fragment and its input come from one hex constant, so the expectation cannot drift from the input." That holds for RangeStart/RangeLimit, but not for EmptyStringKeccakRlp: its input in every consuming test is Keccak.OfAnEmptyString, not a Hash256 built from EmptyStringKeccakHex. That's arguably the better arrangement (the literal independently pins keccak(""), which the shared-constant fragments deliberately don't), but the doc comment currently claims a property that fragment doesn't have. Worth narrowing the sentence to the range hashes and saying explicitly that the keccak("") literal is independent of the input on purpose.

  2. RequestId1111Rlp is the one fragment whose input isn't co-located: each of the three consuming tests writes RequestId = 1111 as a bare literal while the encoding lives here. Exposing the value alongside the fragment would make it match the pattern the rest of the file establishes:

    /// <summary>The request id the range/bytecode request tests use.</summary>
    public const long RequestId1111 = 1111;

    /// <summary>Request id 1111 as an RLP item: 0x82 length prefix + 0x0457.</summary>
    public const string RequestId1111Rlp = "820457";

Neither is a correctness problem — the values themselves all check out.

Fix this →

public const string RequestId1111Rlp = "820457";

/// <summary>keccak("") as an RLP item: 0xa0 + 32 bytes.</summary>
public const string EmptyStringKeccakRlp = "a0" + EmptyStringKeccakHex;

/// <summary><see cref="RangeStart"/> as an RLP item.</summary>
public const string RangeStartRlp = "a0" + RangeStartHex;

/// <summary><see cref="RangeLimit"/> as an RLP item.</summary>
public const string RangeLimitRlp = "a0" + RangeLimitHex;

/// <summary>The starting hash the range request tests use.</summary>
public static readonly Hash256 RangeStart = new(RangeStartHex);

/// <summary>The limit hash the range request tests use.</summary>
public static readonly Hash256 RangeLimit = new(RangeLimitHex);
}
Loading