Skip to content

refactor: add stack comments to claim_batch_pipe_double_words#2620

Open
partylikeits1983 wants to merge 1 commit intoagglayerfrom
ajl-minor-refactor-batch_pipe_double_words
Open

refactor: add stack comments to claim_batch_pipe_double_words#2620
partylikeits1983 wants to merge 1 commit intoagglayerfrom
ajl-minor-refactor-batch_pipe_double_words

Conversation

@partylikeits1983
Copy link
Contributor

@partylikeits1983 partylikeits1983 commented Mar 17, 2026

This PR adds stack comments to claim_batch_pipe_double_words, and was an opportunity to research if the call to mem::pipe_double_words_preimage_to_memory for PROOF_DATA_KEY is necessary in the claim_batch_pipe_double_words procedure.

The purpose of this PR is to close #2237 specifically point 5:

  1. Refactor & clean up batch_pipe_double_words context: Add Agglayer CLAIM note & bridging in functionality #2188 (comment). Refactor & clean up batch_pipe_double_words context: Add Agglayer CLAIM note & bridging in functionality #2188 (comment)

@bobbinth left a comment here regarding the old state of the claim_batch_pipe_double_words procedure. In the comment, it is pointed out that it is not necessary to call exec.mem::pipe_double_words_preimage_to_memory for the PROOF_DATA_KEY, which is technically true.

This is the current state of the claim_batch_pipe_double_words procedure:

# Inputs: [PROOF_DATA_KEY, LEAF_DATA_KEY]
# Outputs: []
proc claim_batch_pipe_double_words
    # 1) Verify PROOF_DATA_KEY
    mem_storew_be.CLAIM_PROOF_DATA_KEY_MEM_ADDR
    adv.push_mapval
    # => [PROOF_DATA_KEY]

    push.CLAIM_PROOF_DATA_START_PTR push.CLAIM_PROOF_DATA_WORD_LEN
    exec.mem::pipe_double_words_preimage_to_memory drop
    # => []

    # 2) Verify LEAF_DATA_KEY
    mem_storew_be.CLAIM_LEAF_DATA_KEY_MEM_ADDR
    adv.push_mapval
    # => [LEAF_DATA_KEY]

    push.CLAIM_LEAF_DATA_START_PTR push.CLAIM_LEAF_DATA_WORD_LEN
    exec.mem::pipe_double_words_preimage_to_memory drop
    # => []
end

It is possible to remove the call to mem::pipe_double_words_preimage_to_memory for the PROOF_DATA_KEY, however, I think we should keep this call.

Why? Because the bridge should not have to trust that the CLAIM note (or the note calling the claim procedure), correctly inserted PROOF_DATA under PROOF_DATA_KEY in the AdviceMap. This call to mem::pipe_double_words_preimage_to_memory is in effect a sanity check. If we removed it, it's unlikely to open a possible vulnerability, however, I think we should keep it, unless there is a strong reason to remove it. Removing it would save around ~257 cycles.

Essentially this call to mem::pipe_double_words_preimage_to_memory makes it so that the bridge_in component does not have to trust that the note which called the claim procedure correctly inserted PROOF_DATA under PROOF_DATA_KEY in the AdviceMap.


Closes #2237

@partylikeits1983 partylikeits1983 changed the title refactor: add stack comments to claim_batch_pipe_double_words refactor: add stack comments to claim_batch_pipe_double_words Mar 17, 2026
@partylikeits1983 partylikeits1983 self-assigned this Mar 17, 2026
@partylikeits1983 partylikeits1983 added no changelog This PR does not require an entry in the `CHANGELOG.md` file agglayer PRs or issues related to AggLayer bridging integration pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority labels Mar 17, 2026
@partylikeits1983 partylikeits1983 marked this pull request as ready for review March 17, 2026 14:47
Copy link
Collaborator

@mmagician mmagician left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not remove the current invocation of claim_batch_pipe_double_words, but leaf data is piped to memory twice now:

  1. claim -> claim_batch_pipe_double_words (piped via pipe_double_words_preimage_to_memory)
  2. claim -> verify_leaf_bridge -> get_leaf_value -> (piped via pipe_preimage_to_memory)

Since both happen within the AggLayerBridge account context, the latter is not necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agglayer PRs or issues related to AggLayer bridging integration no changelog This PR does not require an entry in the `CHANGELOG.md` file pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants