test(l2): pin chain_id encoding mismatch in l2_in_message_rolling_hashes public-input commitment#6540
Draft
avilagaston9 wants to merge 1 commit intomainfrom
Draft
test(l2): pin chain_id encoding mismatch in l2_in_message_rolling_hashes public-input commitment#6540avilagaston9 wants to merge 1 commit intomainfrom
avilagaston9 wants to merge 1 commit intomainfrom
Conversation
…Output.
The L1 verifier in `OnChainProposer.sol::_getPublicInputsFromCommitment`
appends, for every entry in `currentBatch.l2InMessageRollingHashes`,
abi.encodePacked(publicInputs, bytes32(rh.chainId), rh.rollingHash)
— 32 bytes for the chain id (uint256 in the struct, left-padded by the
bytes32 cast) plus 32 bytes for the rolling hash, for **64 bytes per
entry**.
The guest-side `ProgramOutput::encode` (used as the SP1/RISC0 public
commitment via `commit_slice(&output.encode())`) instead appends
encoded.extend_from_slice(&chain_id.to_be_bytes());
encoded.extend_from_slice(&hash.to_fixed_bytes());
where `chain_id` is typed as `u64` (`Vec<(u64, H256)>`), so
`u64::to_be_bytes()` returns 8 bytes, not 32. That comes out to **40
bytes per entry**.
As soon as a batch contains any L2-in privileged transactions — i.e.
any L2-to-L2 messaging is exercised — the prover commits to a public
input that is shorter than the one the L1 reconstructs, the two
sha256 hashes diverge, and the proof fails verification on chain.
The other rolling-hash field (`l1_in_messages_rolling_hash`) doesn't
have this issue because it is a single bytes32, and the
`BalanceDiff::chain_id` and top-level `chain_id` fields encode
correctly because both are `U256` and round-trip via
`to_big_endian()` as 32 bytes — the bug is local to the `(u64, H256)`
typing of `l2_in_message_rolling_hashes`.
The new test `l2_in_message_rolling_hashes_chain_id_is_only_8_bytes_not_32`
in `crates/guest-program/src/l2/output.rs` builds an output with one
rolling-hash entry, isolates it past the 256-byte fixed prefix, and
asserts the per-entry contribution is 40 bytes, that the chain_id is
the bare big-endian u64 with no padding, and that the resulting
layout differs from what L1's `abi.encodePacked(bytes32(chainId),
rollingHash)` produces. After the fix — either widening the field to
`Vec<(U256, H256)>` and using `to_big_endian()` like the other
fields, or padding to 32 bytes on encode — the assertions flip
direction, prompting an explicit review of the wire-format change.
Lines of code reportTotal lines added: Detailed view |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The L1 verifier in
OnChainProposer.sol::_getPublicInputsFromCommitmentappends, forevery entry in
currentBatch.l2InMessageRollingHashes,— 64 bytes per entry: 32 bytes for the chain id (
uint256inthe struct, left-padded by the
bytes32cast) plus 32 bytes for therolling hash.
The guest-side
ProgramOutput::encodeincrates/guest-program/src/l2/output.rs, used as the SP1 / RISC0public commitment via
commit_slice(&output.encode()), insteadappends
where
chain_idis typed asu64(Vec<(u64, H256)>), sou64::to_be_bytes()returns 8 bytes, not 32 — yielding 40 bytesper entry.
As soon as a batch contains any L2-in privileged transactions —
i.e. any L2-to-L2 messaging is exercised — the prover commits to a
public input that is shorter than the one the L1 reconstructs, the
two
sha256hashes diverge, and the on-chain proof verificationfails. The committer happily produces commitments and the prover
happily produces proofs; verification just always reverts.
The other rolling-hash field (
l1_in_messages_rolling_hash)doesn't have this issue because it is a single bytes32. The
BalanceDiff::chain_idand top-levelchain_idfields encodecorrectly because both are
U256and round-trip viato_big_endian()as 32 bytes. The bug is local to the(u64, H256)typing ofl2_in_message_rolling_hashesinProgramOutput.Description
Adds a unit test
l2_in_message_rolling_hashes_chain_id_is_only_8_bytes_not_32in
crates/guest-program/src/l2/output.rsthat builds aProgramOutputwith everything zeroed except a singlel2_in_message_rolling_hashesentry, isolates the per-entry bytespast the 256-byte fixed prefix, and asserts:
encoded.len() == PREFIX_LEN + 40(the buggy 8+32 layout).[PREFIX_LEN..PREFIX_LEN+8]are exactlychain_id.to_be_bytes()— confirming it's the bare u64 withno padding.
PREFIX_LEN+8, not atPREFIX_LEN+32.abi.encodePacked(bytes32(chainId), rollingHash)would produce.The fix is intentionally not in this PR. Two reasonable shapes:
l2_in_message_rolling_hashestoVec<(U256, H256)>andencode with
chain_id.to_big_endian(), matching the patternalready used for
BalanceDiff::chain_idand the top-levelchain_id.chain_id.to_be_bytes()so each entry's wire format is 32+32.Either makes the per-entry contribution 64 bytes, matching L1.
Once landed, the
assert_ne!and length assertions in the testflip direction, prompting an explicit review of the wire-format
change.
Reproduction
Passes on
main(the bug is real and reachable).Impact
L2-to-L2 cross-chain messaging is broken at the proof-verification
layer: any batch that includes one or more L2-in privileged
transactions cannot be verified on L1. The committer commits, the
prover proves, the verifier rejects — the L2 stalls. Bridges that
rely on L2↔L2 settlement can't finalize.
Checklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) if the PR includes breaking changes to theStorerequiring a re-sync.