-
Notifications
You must be signed in to change notification settings - Fork 719
test: anchor crypto and RLP tests to independent expectations #12712
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.
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
base: master
Are you sure you want to change the base?
Changes from 9 commits
e2e6cf3
dcf731a
fc4ee9f
6040988
4a51eba
b26b434
8eb5104
7204812
9d41c5a
82bda9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,14 +3,12 @@ | |||||||||||||||
|
|
||||||||||||||||
| using System; | ||||||||||||||||
| using System.Collections.Generic; | ||||||||||||||||
| using System.IO; | ||||||||||||||||
| using System.Linq; | ||||||||||||||||
| using Nethermind.Core.Crypto; | ||||||||||||||||
| using Nethermind.Core.Extensions; | ||||||||||||||||
| using Nethermind.Core.Test.Builders; | ||||||||||||||||
| using Nethermind.Crypto; | ||||||||||||||||
| using Nethermind.Int256; | ||||||||||||||||
| using Nethermind.Logging; | ||||||||||||||||
| using Nethermind.Serialization.Rlp; | ||||||||||||||||
| using NUnit.Framework; | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -96,6 +94,21 @@ private static Block[] BuildScenarios() | |||||||||||||||
| .WithBlobGasUsed(ulong.MaxValue) | ||||||||||||||||
| .WithExcessBlobGas(ulong.MaxValue) | ||||||||||||||||
| .WithMixHash(Keccak.EmptyTreeHash) | ||||||||||||||||
| .TestObject, | ||||||||||||||||
| // This scenario sets every optional tail field, with a distinct value per field. This makes the field comparisons discriminating. | ||||||||||||||||
| Build.A.Block.WithNumber(1) | ||||||||||||||||
| .WithBaseFeePerGas(3) | ||||||||||||||||
| .WithTransactions(transactions) | ||||||||||||||||
| .WithUncles(uncles) | ||||||||||||||||
| .WithWithdrawals(8) | ||||||||||||||||
| .WithBloom(new Bloom([Build.A.LogEntry.TestObject])) | ||||||||||||||||
| .WithBlobGasUsed(1) | ||||||||||||||||
| .WithExcessBlobGas(2) | ||||||||||||||||
| .WithParentBeaconBlockRoot(TestItem.KeccakA) | ||||||||||||||||
| .WithRequestsHash(TestItem.KeccakB) | ||||||||||||||||
| .WithBlockAccessListHash(TestItem.KeccakC) | ||||||||||||||||
| .WithSlotNumber(7) | ||||||||||||||||
| .WithMixHash(Keccak.EmptyTreeHash) | ||||||||||||||||
| .TestObject | ||||||||||||||||
| ]; | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -125,7 +138,29 @@ public void Can_do_roundtrip_regression() | |||||||||||||||
| RlpReader ctx = new(bytes); | ||||||||||||||||
| Block? decoded = decoder.Decode(ref ctx); | ||||||||||||||||
| Rlp encoded = decoder.Encode(decoded); | ||||||||||||||||
| Assert.That(encoded.Bytes.ToHexString(), Is.EqualTo(bytes.ToHexString())); | ||||||||||||||||
|
|
||||||||||||||||
| // An independent pyrlp decode of the fixed bytes produced the expected values. | ||||||||||||||||
| // Only this test decodes canonical wire. Only these asserts can find a self-canceling encode/decode error. | ||||||||||||||||
| Assert.That(decoded, Is.Not.Null); | ||||||||||||||||
| Assert.That(decoded!.Transactions, Has.Length.EqualTo(1)); | ||||||||||||||||
| Transaction tx = decoded.Transactions[0]; | ||||||||||||||||
| using (Assert.EnterMultipleScope()) | ||||||||||||||||
| { | ||||||||||||||||
| Assert.That(encoded.Bytes.ToHexString(), Is.EqualTo(bytes.ToHexString())); | ||||||||||||||||
| Assert.That(decoded.Header.Number, Is.EqualTo(5644UL)); | ||||||||||||||||
| Assert.That(decoded.Header.Difficulty, Is.EqualTo(UInt256.One)); | ||||||||||||||||
| Assert.That(decoded.Header.Beneficiary, Is.EqualTo(Address.Zero)); | ||||||||||||||||
| Assert.That(decoded.Header.GasLimit, Is.EqualTo(8_000_000UL)); | ||||||||||||||||
| Assert.That(decoded.Header.GasUsed, Is.EqualTo(21_000UL)); | ||||||||||||||||
| Assert.That(decoded.Header.Timestamp, Is.EqualTo(1549034638UL)); | ||||||||||||||||
| Assert.That(decoded.Header.StateRoot, Is.EqualTo(new Hash256("0xfe77dd4ad7c2a3fa4c11868a00e4d728adcdfef8d2e3c13b256b06cbdbb02ec9"))); | ||||||||||||||||
| Assert.That(tx.Nonce, Is.EqualTo(0UL)); | ||||||||||||||||
| Assert.That(tx.GasPrice, Is.EqualTo((UInt256)1_000_000_000)); | ||||||||||||||||
| Assert.That(tx.GasLimit, Is.EqualTo(21_000UL)); | ||||||||||||||||
| Assert.That(tx.To, Is.EqualTo(new Address("0x22ea9f6b28db76a7162054c05ed812deb2f519cd"))); | ||||||||||||||||
| Assert.That(tx.Value, Is.EqualTo(UInt256.Parse("100000000000000000000000"))); | ||||||||||||||||
| Assert.That(decoded.Uncles, Is.Empty); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| [Test] | ||||||||||||||||
|
|
@@ -137,14 +172,53 @@ public void Can_do_roundtrip_scenarios( | |||||||||||||||
| RlpReader ctx = new(encoded.Bytes); | ||||||||||||||||
| Block? decoded = decoder.Decode(ref ctx); | ||||||||||||||||
| Rlp encoded2 = decoder.Encode(decoded); | ||||||||||||||||
| Assert.That(encoded2.Bytes.ToHexString(), Is.EqualTo(encoded.Bytes.ToHexString())); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| [TestCase("0xf902cef9025ba055870e2f3ef77a9e6163ee5c005dc51d648a2eead382b9044b1a5ad2ee69b0c6a01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347942adc25665018aa1fe0e6bc666dac8fc2697ff9baa0b77e3b74c6c8af85408677375183385a2e55446bd071bf193a4958f7417dc8fba056e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421a056e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421b9010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800188016345785d8a0000800c80a0000000000000000000000000000000000000000000000000000000000000000088000000000000000007a0cc3b10b54dc4e97c01f1df20e8b95874cd5fe83bf6eae64935a16cb08db85fa98080a00000000000000000000000000000000000000000000000000000000000000000a0e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855c0c0f86ce08080946389e7f33ce3b1e94e4325ef02829cd12297ef7188ffffffffffffffffd80180948a0a19589531694250d570040a0c4b74576919b801d8028094000000000000000000000000000000000000100080d8038094a94f5374fce5edbc8e2a8697c15331677e6ebf0b80")] | ||||||||||||||||
| [Ignore("The test is useful for debugging hive - shouldn't be executed on CI")] | ||||||||||||||||
| public void Write_rlp_of_blocks_to_file(string rlp) => | ||||||||||||||||
| // the test is useful for debugging hive | ||||||||||||||||
| File.WriteAllBytes("chains\\block1.rlp".GetApplicationResourcePath(), Bytes.FromHexString(rlp)); | ||||||||||||||||
| // A re-encode comparison cannot find a decode error that the matching encode hides. The field | ||||||||||||||||
| // comparisons below can. A fully self-canceling encode/decode pair passes both; only a test | ||||||||||||||||
| // that decodes canonical wire, like Can_do_roundtrip_regression, can find that class. | ||||||||||||||||
| Assert.That(decoded, Is.Not.Null); | ||||||||||||||||
| Assert.That(decoded!.Transactions, Has.Length.EqualTo(block.Transactions.Length)); | ||||||||||||||||
| using (Assert.EnterMultipleScope()) | ||||||||||||||||
| { | ||||||||||||||||
| Assert.That(encoded2.Bytes.ToHexString(), Is.EqualTo(encoded.Bytes.ToHexString())); | ||||||||||||||||
| // The hash pins the embedded header bytes against the standalone header encoding. It cannot find decode errors. | ||||||||||||||||
| Assert.That(decoded.Hash, Is.EqualTo(block.Hash)); | ||||||||||||||||
|
|
||||||||||||||||
| BlockHeader actual = decoded.Header; | ||||||||||||||||
| BlockHeader expected = block.Header; | ||||||||||||||||
| Assert.That(actual.ParentHash, Is.EqualTo(expected.ParentHash)); | ||||||||||||||||
| Assert.That(actual.UnclesHash, Is.EqualTo(expected.UnclesHash)); | ||||||||||||||||
| Assert.That(actual.Beneficiary, Is.EqualTo(expected.Beneficiary)); | ||||||||||||||||
| Assert.That(actual.StateRoot, Is.EqualTo(expected.StateRoot)); | ||||||||||||||||
| Assert.That(actual.TxRoot, Is.EqualTo(expected.TxRoot)); | ||||||||||||||||
| Assert.That(actual.ReceiptsRoot, Is.EqualTo(expected.ReceiptsRoot)); | ||||||||||||||||
| Assert.That(actual.Bloom, Is.EqualTo(expected.Bloom)); | ||||||||||||||||
| Assert.That(actual.Difficulty, Is.EqualTo(expected.Difficulty)); | ||||||||||||||||
| Assert.That(actual.Number, Is.EqualTo(expected.Number)); | ||||||||||||||||
| Assert.That(actual.GasLimit, Is.EqualTo(expected.GasLimit)); | ||||||||||||||||
| Assert.That(actual.GasUsed, Is.EqualTo(expected.GasUsed)); | ||||||||||||||||
| Assert.That(actual.Timestamp, Is.EqualTo(expected.Timestamp)); | ||||||||||||||||
| Assert.That(actual.ExtraData, Is.EqualTo(expected.ExtraData)); | ||||||||||||||||
| Assert.That(actual.Nonce, Is.EqualTo(expected.Nonce)); | ||||||||||||||||
| Assert.That(actual.BaseFeePerGas, Is.EqualTo(expected.BaseFeePerGas)); | ||||||||||||||||
| Assert.That(actual.WithdrawalsRoot, Is.EqualTo(expected.WithdrawalsRoot)); | ||||||||||||||||
| Assert.That(actual.BlobGasUsed, Is.EqualTo(expected.BlobGasUsed)); | ||||||||||||||||
| Assert.That(actual.ExcessBlobGas, Is.EqualTo(expected.ExcessBlobGas)); | ||||||||||||||||
| Assert.That(actual.MixHash, Is.EqualTo(expected.MixHash)); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium — the field list stops short of the newest header fields.
Suggested change
|
||||||||||||||||
| Assert.That(actual.ParentBeaconBlockRoot, Is.EqualTo(expected.ParentBeaconBlockRoot)); | ||||||||||||||||
| Assert.That(actual.RequestsHash, Is.EqualTo(expected.RequestsHash)); | ||||||||||||||||
| Assert.That(actual.BlockAccessListHash, Is.EqualTo(expected.BlockAccessListHash)); | ||||||||||||||||
| Assert.That(actual.SlotNumber, Is.EqualTo(expected.SlotNumber)); | ||||||||||||||||
|
|
||||||||||||||||
| for (int i = 0; i < block.Transactions.Length; i++) | ||||||||||||||||
| { | ||||||||||||||||
| Assert.That(decoded.Transactions[i].Hash, Is.EqualTo(block.Transactions[i].Hash), $"transaction {i}"); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| Assert.That(decoded.Uncles.Length, Is.EqualTo(block.Uncles.Length)); | ||||||||||||||||
| Assert.That(decoded.Withdrawals?.Length, Is.EqualTo(block.Withdrawals?.Length)); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low (carry-over, optional) — the body is still compared by count only, one level below the now-complete header comparison. Uncles are the cheap half and they're already discriminating: Withdrawals are weaker: Fine to leave for a later chunk in the series — this PR's stated scope is the header and the crypto/RLP anchors. |
||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| [Test] | ||||||||||||||||
| public void Encode_with_pre_encoded_transactions_produces_same_rlp( | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,15 +151,21 @@ public void Compare(string a, string? b, int result) | |
| public void CompareSameInstance() => Assert.That(Keccak.Zero.CompareTo(Keccak.Zero), Is.EqualTo(0)); | ||
|
|
||
| [Test] | ||
| public void Span() | ||
| public void Computes_known_hash_for_span_and_array() | ||
| { | ||
| byte[] byteArray = new byte[1024]; | ||
| for (int i = 0; i < byteArray.Length; i++) | ||
| { | ||
| byteArray[i] = (byte)(i % 256); | ||
| } | ||
|
|
||
| Assert.That(Keccak.Compute(byteArray.AsSpan()), Is.EqualTo(Keccak.Compute(byteArray))); | ||
| // An independent keccak-256 implementation (pycryptodome) produced the expected hash. | ||
| Hash256 expected = new("0x5902e53903be0d0f9656bdbd5b9f0d8c2d815f865645d629eef77f5185f6cd7f"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low (nit) — the test name I couldn't re-derive |
||
| using (Assert.EnterMultipleScope()) | ||
| { | ||
| Assert.That(Keccak.Compute(byteArray.AsSpan()), Is.EqualTo(expected)); | ||
| Assert.That(Keccak.Compute(byteArray), Is.EqualTo(expected)); | ||
| } | ||
| } | ||
|
|
||
| [TestCase("0x", "c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Low — this assert can't fail, and it reads like it can.
HeaderDecoder.cs:54setsHash = Keccak.Compute(headerRlp)from the raw incoming span, and the builder sets the original'sHashviaCalculateHash()over the same encoding (BlockHeaderBuilder.cs:19). So both sides hash the same bytes: no decode defect — symmetric or asymmetric — can move this assert. Its only real content is that the header bytes embedded byBlockDecodermatch what the standalone header encoder produces.The comment two lines up gestures at this ("The hash values also derive from the encoded bytes"), but the assert is then placed first, where a reader will take it as the strongest check in the block. Either drop it or say explicitly that it cross-checks the block-embedded header encoding against the standalone one — otherwise it's the same self-consistency pattern the PR is removing elsewhere.
The same caveat applies to the transaction loop below:
BaseTxDecoder.cs:68setsHashfrom the tx wire bytes, so comparing hashes catches ordering and asymmetric errors (good — that's real coverage) but not a symmetric field swap insideTxDecoder.