Repository navigation
feat(tests): add MPT structural test suite - #3600
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/bogota #3600 +/- ##
=============================================
Coverage 94.99% 94.99%
=============================================
Files 673 673
Lines 40888 40888
Branches 3769 3769
=============================================
Hits 38843 38843
Misses 1428 1428
Partials 617 617
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
52cb9ec to
9057067
Compare
jochem-brouwer
left a comment
There was a problem hiding this comment.
I have not checked the entire PR but I love the structural approach here. However, I would also assume that this behavior is tested in the Frontier/chainstart (legacy) tests somewhere, and we should find out if this is the case and then also how we want to move this forward.
These kind of tests give insight to how the data structure works, and thus also points to cases where we can benchmark these cases (esp. for instance comparing it to PBT)
| tx=_delete_tx(sender, COLLAPSE_SALT), | ||
| post={ | ||
| survivor: Account(balance=1), | ||
| doomed: Account.NONEXISTENT, |
There was a problem hiding this comment.
This does not work on Glamsterdam due to https://eips.ethereum.org/EIPS/eip-8246 (and all related tests using _delete_tx)
From Glamsterdam, any created account cannot be destroyed anymore, if the account existed pre-transaction
There was a problem hiding this comment.
These tests still delete the leaf from Amsterdam on. _delete_tx's init code is SELFDESTRUCT(ORIGIN), so the whole balance leaves the account before finalization; EIP-8246 then clears it to nonce 0, no code and no storage, and with zero balance it is empty, so it is removed.
Every one of these tests asserts the doomed address is Account.NONEXISTENT, and the suite fills on Osaka, Amsterdam and Bogota. With SELFDESTRUCT(ADDRESS) the account would keep its balance and these tests would fail from Amsterdam on, as you describe; 83a4523 states that dependency next to the init code.
3f185a2 to
701a282
Compare
|
|
701a282 to
d91a790
Compare
|
@jochem-brouwer Thanks. I looked for overlap and found none: the legacy consensus tests (ethereum/tests, ethereum/legacytests, and their ports in |
|
cc: @marioevz no rush. Just to signal the PR we discussed in the call about MPT fixtures. This only adds fixtures, so no major EELS refactors nor any complexity. Fixtures are collected from client tests. |
Every docstring now states the node path as `pre:`/`post:` lines in one notation; client symbol citations are gone. Shape helpers move to `trie_shape` (`storage_shape`, `account_shape`, `index_shape`, `covering`, `leaf_rlp_size`). Delete tests are named `test_delete_*`, genesis-only account tests `test_genesis_*`. Account shapes are computed over the full genesis set (pre-alloc plus the fork's system contracts, plus the coinbase after the block) and pin the parent kind around the mined group with `covering`; the previous `all(... if d == 4)` was vacuous when no node sat at depth 4. The storage-root flip test now collapses onto the contract itself, created through the factory with a salt whose address shares four nibbles with the doomed one, so storage root and account re-pathing change in one diff. The three delete-missing tests merge into one parametrized control (the write never reaches trie code), the trie-identical per-transaction variant of the mass delete is dropped, insert-then-delete now covers a leaf split, an extension split and a branch slot, and the index-trie test adds counts 145, 257 and 273. Withdrawal values are encoded as a plain RLP tuple instead of a fork-pinned class.
pre: branch@0 -> {ext(3) -> branch@4 -> {p, q}, leaf(63)}
post: ext(4) -> branch@4 -> {p, q}
Closes the survivor-extension x parent-root cell; the root changes kind
from branch to extension. A client with a dedicated "extension becomes
root" path computes a different root here and nowhere else.
pre: branch@0 -> {ext(3) -> branch@4 -> {p, q}, leaf(63)}
post: the extension split at offset 0, in the middle, at its last
nibble, and the 1-nibble case where it vanishes
Every earlier extension split happened at the root; here the result is
re-parented into an existing branch slot, a different frame in every
node-walking client.
pre: ext(2) -> branch@2 -> {l3, ext(2) -> branch@5 -> {l1, l2}}
post: ext(2) -> branch@2 -> {l3, leaf(61)}
The only extension-parent collapse whose merged leaf does not become
the root: the merge and the update of the parent branch's slot happen
in the same step.
pre: ext(4) -> branch@4 / branch@0 / leaf post: empty Adds root branch -> empty and root leaf -> empty next to the existing root extension case, so every root kind reaches the empty trie root.
pre: ext(4) -> branch@4 -> {c, ext(3) -> branch@8 -> {a, b}} with a,
b inline (31 B) or hashed (33 B) by value size, c always hashed;
and ext(8) -> branch@8 -> {s, branch@9 -> {x, y}} all inline
post: deleted inline / hashed x survivor flips / stays hashed / stays
inline, each restored in a second block
Six cells of the (deleted child, survivor encoding) matrix; the
previous single flip test covered one. Hash-keyed stores decode inline
children from the parent and must re-inline a rebuilt survivor.
pre: empty
post: ext(10) -> branch@10 -> {leaf(53), leaf(53)}, 30 B with a 1-byte
value and exactly 32 B with a 2-byte value
The existing 32-byte case comes from the key side (56 nibbles); a
client computing leaf lengths arithmetically has a separate branch for
a value byte >= 0x80, which only this cell reaches at the threshold.
pre: ext(8) -> branch@8 -> {leaf(55) 31 B, leaf(55) 31 B}
post: the leaf holds a different 1-byte value, still 31 B inline
Next to the existing flip to 33 B: the inline case rewrites the leaf
inside its parent's RLP and never gives it a hash, so dirty tracking
keyed by child hash has nothing to key on.
pre: empty; block 1 builds each geometry, block 2 deletes, block 3
re-inserts
post: the geometry's shape after the round trip
Adds three-to-two, leaf/ext/branch survivors under a branch parent,
both root collapses and the nested-extension leaf to the three-block
test. Genesis lands in a client's base layer; block-1 nodes live in
its diff layer or dirty cache, which is where these cells were still
untested.
pre: branch@0 -> {ext(3) -> branch@4 -> {p, q}, branch@1 -> {1, 14}};
two contracts with ext(4)/ext(5) tries; two contracts holding the
same single-leaf trie
post: both root children collapse, or one collapses while the other
splits; two storage roots change in one diff; one contract's copy
of the shared node is deleted, the other's grown next block
Touched-prefix sets with two entries under one root, parallel storage
roots, and hash-keyed stores that refcount or drop a node still used
by another trie.
pre: ext(2) -> branch@2 -> {l3, ext(2) -> branch@5}; branch@0 with 16
leaves; ext(4) -> branch@4 -> {p, q}
post: the inner extension replaced by a branch in one step; the root
branch filled, reduced to 15 and to 1; q round-trips to its
original value while p really changes
Extension-first-nibble splits so far happened at the root only; root
nodes take dedicated code in several clients; a touched-key applier
must neither skip the changed subtree nor rewrite the unchanged leaf.
pre: .. -> branch@4 -> {survivor, doomed}
post: the survivor credited in the collapse block; the doomed leaf
deleted, re-created by a transfer and deleted again across three
blocks; re-created by a withdrawal, or not when it carries 0 Gwei
The collapse must read the survivor's updated balance; a resurrected
balance-only account is deployable again; withdrawals apply after the
transactions and a zero credit leaves no empty account.
pre: mined siblings at depths 1 and 3 make the parent of the
collapsing branch a controlled branch or extension; three and
sixteen accounts under one branch
post: leaf, extension and branch survivors under a branch parent, a
branch survivor under an extension, 3 -> 2, and 16 -> 1 via
fifteen CREATE2 deletions in one block
Brings the account trie to parity with the storage cells; each cell's
parent kind is pinned over the full genesis set, so an uncontrolled
account sharing the prefix fails the fill instead of changing the cell.
`fund_address(addr, 0)` yields an empty account that the filler omits from the genesis allocation, so EIP-161 clearing of a committed empty leaf cannot be built through the public API. Recorded next to the other inexpressible account cases.
The package docstring now carries the survivor x parent grid and the enumerated classes (splits, root transitions, arity, encoding, one-block sequences, index tries, account bookkeeping) so the "what is covered" statement lives in the repository next to the tests.
From Amsterdam the fork allocates the deterministic factory in genesis, so the filler no longer injects it into the pre-alloc on first use of `deterministic_deploy_contract`, and the `DETERMINISTIC_FACTORY_ADDRESS in pre` check failed every account test that deletes through it. Deploy through the factory only on forks without it, and check its presence in the genesis instead of the pre-alloc.
A state test's own fixture starts from the pre-alloc plus `pre_allocation()`, the blockchain fixtures derived from it from the pre-alloc plus `pre_allocation_blockchain()`, but shapes were asserted only in the second set. Two tests also funded the sender after taking the snapshot, so its leaf was missing from every set they asserted. Assert state-test shapes in both sets and take the snapshot once the pre-alloc is complete. The fixtures are unchanged; 40 of the 92 account fixtures were previously not asserted on the genesis they ship with.
pre: branch@0 -> {ext(1) -> branch@2 -> {d, ext(1) -> ..}, ..}
branch@0 -> {ext(2) -> branch@3 -> {d, branch@4}, ..}
post: branch@0 -> {ext(3) -> branch@4 -> {p, q}, leaf(63)}
Extension and branch survivors had an extension parent only at the
root, where the merged node becomes the new root; the nested case, in
which it is written into the parent branch's slot, existed only for a
leaf survivor. Both cells also run on client-built nodes. They reuse
the slots of the extension-split tests, so no new constants.
…iciary The account tests delete a genesis-funded leaf by CREATE2-ing init code that self-destructs to ORIGIN. From EIP-8246 a self-destructed account keeps its balance and is only removed if that balance is zero, so the deletion depends on the balance leaving the account. A self-destruct to the contract itself would leave a balance-only account on Amsterdam and later forks; state that next to the init code and in the module docstring.
Four fill-time checks did not pin what their docstrings state: - The inline-survivor test accepted 31 or 33 bytes for either deleted leaf; each case now asserts its own size. - The storage-root flip test claimed an extension above the collapsing branch without asserting it. - The withdrawal resurrection test asserted only the genesis shape; it now asserts the two-child branch when the withdrawal re-creates the account and the collapsed leaf when it credits nothing. - The index test compared the root node of two paths of one trie, which are always equal. No fixture changes.
- `SLOT_WRITER`, `DELETE_INITCODE` and `GWEI` live in `constants.py` next to the salts mined against them; both test modules import them and encode writer calldata the same way. - The twelve genesis-trie storage deletes repeated the fourteen cases of the block-built delete test. One `DELETE_CASES` table now drives both tests and pins each case's path before and after the delete, where the block-built test only checked that the shape changed. - The two standalone account collapses are rows of the parametrized collapse test. Its survivor kind is the node below the collapsing branch, and for extension and branch survivors it asserts that the merged extension ends at the survivor's branch. Four blockchain tests share one funding prologue. - The sixteen-to-one storage delete is one transaction, so it is a state test. - Withdrawal leaves are sized from `Withdrawal.to_serializable_list()`; a check of the reference trie's own root hashing is gone. - `_alloc` gives way to the framework's `Storage`, `_shape` to `account_shape`, and the root arity test takes its committed storage as a parameter. Fixture content is unchanged except for two cases: the account extension-merge collapse funds its second address with 1 wei instead of 2, and the sixteen-to-one storage delete gains state fixtures, its blockchain fixtures now derived from them.
The package docstring states the method and defines the shape notation once; the coverage inventory that duplicated the test list is gone. Test docstrings give one sentence for the scenario and its rule, then the `pre:`/`post:` paths, without per-case labels, `Control:` prefixes, claims about client internals or comparisons with other software. Helper docstrings use the imperative mood.
A shared group adds other tests' accounts to the genesis whose shape the tests assert, and the two storage-root flip cases collide on one address.
Since ethereum#3830, `pre` holds the whole genesis of the fixture being filled, so the tests stop rebuilding it from the fork's pre-allocations. Fixtures are unchanged.
CREATE2 deploys code onto the balance-only survivor in the block that deletes its sibling, at the address `SURVIVOR_SALT` already places there.
d91a790 to
a3cf4d9
Compare
|
@marioevz rebased and addressed. LMK if there's anything else needed prior to merging. |
marioevz
left a comment
There was a problem hiding this comment.
LGTM, thanks for the fixes to my comments!
Description
Adds
tests/mpt/: consensus fixtures that exercise the Merkle Patricia Trie as a data structure. Mined slot keys, addresses and CREATE2 salts (constants.py) force each node shape, and every test asserts that shape against the referencepatricializeat fill time (trie_shape.py), so each fixture is known to produce the shape it names. The committed root is the oracle.Each fork from Osaka on (Osaka, Amsterdam, Bogota) gets 116 blockchain, 116 blockchain-engine and 56 state fixtures.
Related Issues or PRs
fillandconsumeissues found while running this suite, fixed by fix(test-fill): fail collection of tests without a spec fixture #3672, fix(test-consume): run and check nethtest block tests in consume direct #3674 and fix(test-consume): skip Besu evmtool's non-JSON summary line in state-test output #3610.Checklist
just static<type>(<area>): <description>, where<type>and<area>come from an appropriateC-<type>, respectivelyA-<area>, label. The title should match the target squash commit message.Cute Animal Picture