rpc/jsonrpc: lex-order executionWitness codes and state#21531
Conversation
Witness codes were ordered by codehash and state nodes by trie-traversal order. The canonical stateless-witness format orders both lex-ascending by content. Verified against the EEST zkevm@v0.4.0 corpus: all multi-entry fixtures store codes and state sorted lex by content. State is sorted after verifyWitnessStateless: the stateless verifier's RLPDecode treats the first node as the trie root, so the witness must stay root-first until verification completes, then be reordered for output. Pass rate is unchanged (the suite compares as a multiset); this only aligns serialization order.
| @@ -705,6 +705,10 @@ func (api *DebugAPIImpl) ExecutionWitness(ctx context.Context, blockNrOrHash rpc | |||
| return nil, err | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Could be useful insert a short comment to avoid in future we change the order:
Must sort after verifyWitnessStateless: the verifier expects result.State[0] to be the trie root
There was a problem hiding this comment.
Pull request overview
This PR aligns debug_executionWitness serialization with the canonical (fixture-expected) ordering by sorting both state trie nodes and codes lexicographically by their byte content, while keeping internal stateless verification root-first during decoding.
Changes:
- Sort
ExecutionWitnessResult.Statelexicographically-by-content afterverifyWitnessStatelesscompletes. - Sort the deduplicated pre-state
codeslexicographically-by-content (instead of by codehash).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| out.SortedCodes = append(out.SortedCodes, c) | ||
| } | ||
|
|
||
| preCode := rs.GetPreStateCode() |
| // Sort after verifyWitnessStateless: RLPDecode treats result.State[0] as the trie root. | ||
| slices.SortFunc(result.State, func(a, b hexutil.Bytes) int { | ||
| return bytes.Compare(a, b) | ||
| }) |
yperbasis
left a comment
yperbasis
left a comment
There was a problem hiding this comment.
Approving — the load-bearing invariant is correct: trie.RLPDecode treats State[0] as the root, and the new sort runs after verifyWitnessStateless, so self-verification is untouched. The only consumer of the sorted result is JSON serialization (no internal caller depends on order), and the change is type- and determinism-safe (codes are pairwise-distinct via the hash-keyed map; the multiset is preserved, so the pass rate is unchanged as stated). Nice cleanup dropping codeWithHash.
Findings & suggestions
-
No test locks in the new order (main gap).
TestExecutionWitnessonly asserts non-nil + lengths. An ordering change with no assertion can be silently reverted by the next refactor — and the sort-after-verify invariant is only incidentally guarded today. Suggest pinning it:require.True(t, slices.IsSortedFunc(result.Codes, func(a, b hexutil.Bytes) int { return bytes.Compare(a, b) })) require.True(t, slices.IsSortedFunc(result.State, func(a, b hexutil.Bytes) int { return bytes.Compare(a, b) }))
-
Follow-up (non-blocking): the emitted witness is no longer round-trippable through our own
RLPDecode. It computeskeccak(State[0])as the root, and that lookup never fails (every node's hash is in the map) — so decoding the lex-sorted output wouldn't error, it would silently build a trie rooted at the lexicographically-smallest node. No in-repo path hits this (the only decode is pre-sort), but the positional-root assumption is now inconsistent with what we emit. Worth a tracking issue to locate the root by the known state-root hash instead of by position. -
Minor: the canonical-order claim rests on fixture inspection (
zkevm@v0.4.0) — a link to the format spec in the PR body would make it auditable.
Order
codesandstatelex-ascending by content (was: codes by codehash, state by trie-traversal). Confirmed against zkevm@v0.4.0 fixtures.stateis sorted afterverifyWitnessStateless— itsRLPDecodetreats node 0 as the trie root, so the witness stays root-first through verification.No pass-rate change (multiset compare); serialization alignment only.
Builds on #21518. Refs #20534 (causes 1 & 3).