CLAIM note doc comment contradicts the zero-padding check in process_global_index_mainnet/rollup
Nobody has claimed this yet.
Assessment
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Newbie friendliness
- 80/100
- Issue type
- Documentation
- Clarity
- Clearly specified
- Activity status
- Quiet
- Tech stack
- rust
- Domain
- documentation
Research direction
The issue is about a comment in crates/miden-agglayer/asm/agglayer/notes/claim.masm (around line 62) that contradicts the zero-padding check in crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm (functions process_global_index_mainnet and process_global_index_rollup, lines 338-392). Read the comment and the corresponding assertz code to understand the mismatch. The fix is to update the doc comment to state that the top 191 bits must be zero, aligning with the current implementation and the updated bridge contract. Verify by checking the referenced Solidity bridge changes.
Written by the indexing model from the issue text.
Description
The doc comment on the CLAIM note's globalIndex field contradicts what the code actually enforces, and the contradiction looks like it traces back to an upstream change.
claim.masm describes the field as:
Top 191 bits are ignored (not required to be zero), so indexers must decode it exactly like the contract does
But bridge_in.masm's process_global_index_mainnet / process_global_index_rollup do the opposite — they assert the top 5 felts (191 bits) are zero and panic (ERR_LEADING_BITS_NON_ZERO) otherwise.
I don't think the implementation is wrong here. Checked the reference Solidity bridge: older versions of PolygonZkEVMBridgeV2._verifyLeaf carry a comment saying "global index do not assert the unused bits to 0", which is almost certainly where the Miden-side doc comment's wording came from. But agglayer-contracts v11.0.0-rc.3 (PR #478) changelog lists "Checks globalIndex (to assert that all unused bits are 0)" as a change, i.e. the reference contract was updated to require this. Miden's implementation matches the current contract; the doc comment just wasn't updated to match, presumably left over from when the older, more permissive comment was written.
This isn't exploitable as far as I can tell, current behavior looks correct and consistent with what the bridge is meant to enforce today. But the comment is actively misleading for anyone reading this file to understand the security model, especially since it directly contradicts the assertz two lines below it in bridge_in.masm. Worth fixing so a future change doesn't "fix" the check to match the stale comment instead of the other way around.
Location: crates/miden-agglayer/asm/agglayer/notes/claim.masm (doc comment, ~line 62), vs. crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm process_global_index_mainnet/process_global_index_rollup (~lines 338-392).
Happy to send a one-line doc fix if this can be assigned to me.
- Dominant language
- Rust
- Stars
- 133
- Forks
- 168
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 93
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from 0xMiden/protocol
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
objects
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
standards
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
Difficulty 1/5 Under an hour Newbie friendliness 80/100
All issues in 0xMiden/protocol
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Eynzof/Hermes-CN-Desktop#610 ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
gitbutlerapp/gitbutler#15998 · 1 comment ·
-
bug triage:deciding
Difficulty 1/5 Under an hour Newbie friendliness 88/100
open-telemetry/otel-arrow#4132 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100