Skip to content

refactor(mpt): give digest node references a fixed-size type - #680

Draft
Qumeric wants to merge 1 commit into
valery/mpt-put-digestfrom
valery/mpt-digest-type
Draft

refactor(mpt): give digest node references a fixed-size type#680
Qumeric wants to merge 1 commit into
valery/mpt-put-digestfrom
valery/mpt-digest-type

Conversation

@Qumeric

@Qumeric Qumeric commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

digest_eq and put_digest (both now on develop-v2.1.0) each re-establish a fact the code already
knows: a digest node reference is 32 bytes. They have to, because NodeRef::Digest and
NodeData::Digest document that in a comment while holding a &[u8] — so the length is thrown away
at every use, and the compiler falls back to memcmp/memcpy calls for operations that should be a
handful of word moves.

Carrying &Digest (&[u8; 32]) states it in the type instead. Both helpers disappear: comparing is
== and copying is put_slice, each already inline. The conversions happen once, where witness bytes
enter the trie, through a single digest_from. The diff removes more than it adds.

Why this is worth landing even though it is performance-neutral

Measured at +0.053% instructions — neutral, as expected, because digest_eq/put_digest already
achieve the same codegen by hand. The reason to land it is that it turns two unchecked assumptions
into ordinary errors:

  • The root reference (decode_trie) treated any non-list node of at least 32 bytes as a digest
    without checking its length. It now reports a malformed witness.
  • from_rlp_slice asserted slice.len() < 32 for anything that was not 33 bytes — a debug_assert,
    so release builds dropped the check and silently built a reference that violated the invariant.
    A reference of an unexpected length is now simply a Bytes reference, which fails the comparison
    against its parent and surfaces as NodeRefMismatch.

The guest runs in release. Encoding the invariant in the type is what makes the second one
unrepresentable rather than merely asserted.

bytes_eq stays for the one remaining caller, comparing references shorter than 32 bytes, where the
byte loop is still cheaper than a call. Its doc comment now explains why digests do not need it.

One footgun worth knowing about for reviewers: DIGEST_LEN is used in match arms. Without the
import in scope an uppercase identifier in a pattern becomes a fresh binding that matches everything,
silently. rustc does catch it — unreachable pattern, unused variable, and non-snake-case warnings all
fire — and I verified that by removing the import and watching all three appear.

Draft: this needs both #684 and #668 first

Its base branch predates the current queue. The change touches the digest sites that #684 reorganizes
into rlp.rs and the node-hashing site that #668 converts to the sponge, so a meaningful base has to
contain both — no single open PR does. Rebasing it onto #684 alone leaves conflicts that are only
resolvable by reintroducing sponge code, and onto develop-v2.1.0 alone it would have to carry both
PRs' changes in its own diff.

Staying draft until #684 and #668 land, then rebasing onto develop-v2.1.0 for a clean two-file diff.

`NodeRef::Digest` and `NodeData::Digest` documented their payload as always 32 bytes but held a
`&[u8]`, so the length was lost at every use and had to be re-established by hand to keep
comparisons and copies inline. Carrying `&Digest` instead states the invariant in the type: the
two helpers that re-derived it are gone, and the conversions now happen once, where witness bytes
enter the trie.

The only site that never checked the length is the root reference, where a non-list node of at
least 32 bytes was assumed to be a digest; it now reports a malformed witness instead. Dropping
the `debug_assert` in `from_rlp_slice` likewise turns a debug-only panic on a reference of an
unexpected length into an ordinary reference mismatch.
@Qumeric
Qumeric force-pushed the valery/mpt-digest-type branch from 5a70c6e to bd39d15 Compare July 25, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant