0xMiden / 0xMiden/protocol

CLAIM note doc comment contradicts the zero-padding check in process_global_index_mainnet/rollup

未關閉
#3,557 1 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
Rust
星號
132
分支
167
平均合併
1 天 23 小時
30 天內合併 PR
110

描述

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.

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。