Skip to content

feat: implement Clone for TransactionContextBuilder - #2979

Merged
PhilippGackstatter merged 2 commits into
nextfrom
pgackst-clone-for-tx-context-builder
May 27, 2026
Merged

feat: implement Clone for TransactionContextBuilder#2979
PhilippGackstatter merged 2 commits into
nextfrom
pgackst-clone-for-tx-context-builder

Conversation

@PhilippGackstatter

Copy link
Copy Markdown
Contributor

Implements Clone for TransactionContextBuilder so we can simplify multisig test code that rebuilds transaction contexts twice: once to get the tx summary, once to execute.

Also adds a helper for unwrapping a TransactionExecutorError::Unauthorized variant.

Motivated by reducing line count in general but in particular for #2973.

@PhilippGackstatter PhilippGackstatter added 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 labels May 26, 2026
@PhilippGackstatter
PhilippGackstatter force-pushed the pgackst-clone-for-tx-context-builder branch from 3f5b81b to c8e701f Compare May 26, 2026 09:07
@PhilippGackstatter
PhilippGackstatter marked this pull request as ready for review May 26, 2026 09:29

@partylikeits1983 partylikeits1983 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@PhilippGackstatter
PhilippGackstatter added this pull request to the merge queue May 27, 2026
Merged via the queue into next with commit 85caa18 May 27, 2026
20 checks passed
@PhilippGackstatter
PhilippGackstatter deleted the pgackst-clone-for-tx-context-builder branch May 27, 2026 08:14
mmagician pushed a commit that referenced this pull request May 29, 2026
* feat: implement Clone for TransactionContextBuilder

* chore: add unwrap_unauthorized_err
partylikeits1983 added a commit that referenced this pull request Jun 5, 2026
Brings the v0.15.x line on main into next via a full merge (supersedes the selective backport on this PR). Includes the network-account tx-script allowlist (#3028, #3049), circular note-dependency rejection (#2993), agglayer GER/B2AGG fixes (#2983, #2988/#3005), BURN note id uniqueness (#2987), TransactionContextBuilder Clone (#2979), and version bumps.

Conflicts resolved in favor of next: Cargo.toml/Cargo.lock keep 0.16.0; batch/mod.rs keeps the kernel/output modules; proposed_batch.rs tests keep the 4-arg new_unverified API; kernel_tests/batch/mod.rs keeps batch_verifier; test_active_note.rs keeps the get_metadata test; bridge_out.rs keeps the MintPolicy import.
mmagician added a commit that referenced this pull request Jul 20, 2026
* feat: implement `Clone` for `TransactionContextBuilder` (#2979)

* feat: implement Clone for TransactionContextBuilder

* chore: add unwrap_unauthorized_err

* feat(agglayer): reject duplicate GER insertions (#2983)

* feat(agglayer): reject duplicate GER insertions

Adds a duplicate-detection guard to `bridge_config::update_ger`: after
calling `set_map_item`, the OLD_VALUE returned is compared against
EMPTY_WORD; if it is not empty the GER was already registered and the
transaction panics with ERR_GER_ALREADY_REGISTERED.

Note on Solidity divergence: the Solidity GlobalExitRootManager treats
duplicate GER insertions as idempotent no-ops. Miden intentionally
rejects them because a permanently-unconsumable network note is a worse
failure mode than an explicit transaction revert. This divergence is
documented in SPEC.md.

Docs updated on `update_ger`, the `update_ger.masm` note script, and
SPEC.md (Panics row + Section 2.3 prose). A new integration test
(`update_ger_rejects_duplicate`) verifies that a second UPDATE_GER note
carrying the same GER value is rejected with the expected error code.

Closes #2708

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: add CHANGELOG entry for #2983

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: combine padw+assert_eqw and use assert_transaction_executor_error!

- bridge_config.masm: collapse the `padw` / `assert_eqw` pair onto a single
  line as suggested in review.
- tests: replace the manual `is_err()` + `error_msg.contains(...)` block
  with the standard `assert_transaction_executor_error!` macro.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: use word::eqz + assert instead of padw + assert_eqw

Slightly cheaper than the `padw`-then-`assert_eqw` pair: `word::eqz`
consumes the 4-felt `OLD_VALUE` and pushes a single 0/1 flag, then a
plain `assert.err=...` verifies it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: remove MASM format & style guide (#2994)

The MASM doc-comment and formatting conventions previously captured in
masm_doc_comment_fmt.md are now covered by the .claude/skills/masm-*
skill files plus the MASM formatting tool. Maintaining a separate
hand-written guide is redundant and prone to drift, so remove it.

Closes #2021

Co-authored-by: Claude (Opus) <noreply@anthropic.com>

* docs: discourage multiple attachments with the same scheme (#2992)

The protocol does not prevent adding multiple attachments with the same
scheme to a note (e.g. two NetworkAccountTarget attachments). Document
that doing so is discouraged as it brings no additional benefit and only
increases public on-chain data and fees, and clarify in the rustdoc that
the first matching attachment is treated as the canonical one.

Closes #2873

Co-authored-by: Claude (Opus) <noreply@anthropic.com>

* Make `BURN` note ID unique (#2987)

* refactor: include leaves number in BURN note ID computation

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

---------

Co-authored-by: Marti <marti@miden.team>
Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* Extend execution-error assertion macros with patter and any arms (#2897)

* Extend execution-error assertion macros with patter and any arms

* Fix formating

* Apply PR review feedback

* fix: Reject circular note dependencies at transaction, batch and block level (#2993)

* feat: reject consuming and creating same note in a tx

* feat: erase notes on the fly

* chore: remove unused `NoteCommitmentMismatch` error variants

* chore: add changelog

* chore: rename input/output note tracker functions

* feat: reintroduce `NoteTracker` for readability

* chore: improve clarity of comments

* chore: format changelog

* chore: add second cross-tx circular dep test

* fix(agglayer): enforce NoteType::Public for B2AGG bridge-out notes (#2988) (#3005)

* feat(protocol): add active_note::is_public / is_private (#2988)

Cherry-pick the reusable protocol-level building blocks from #2988,
which was merged to the agglayer branch as an agglayer-specific
B2AGG bridge-out hardening:

  - active_note::is_public
  - active_note::is_private

Both procs read the active note's metadata and decode its note
type via the existing note::metadata_into_note_type primitive
(from #2738), then compare against NOTE_TYPE_PUBLIC /
NOTE_TYPE_PRIVATE from miden::protocol::util::note. A parameterized
kernel test covers the Public and Private cases.

The agglayer-only files from #2988 (bridge_out.masm, B2AGG.masm,
tests/agglayer/bridge_out.rs) and the constant-value flip the PR
applied on agglayer's NoteType encoding are intentionally not
brought along: next already encodes Private=0, Public=1, and the
existing metadata_into_note_type is tailored to next's metadata
bit layout.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>

* fix(agglayer): enforce NoteType::Public for B2AGG bridge-out notes (#2988)

Cherry-pick the bridge-side half of #2988 to next. The B2AGG note
type lives in NoteMetadata, not in the recipient commitment, so an
attacker could submit a note with an identical recipient,
attachment, and asset but NoteType::Private. consensus would accept
it, the leaf would be folded into the on-chain Local Exit Tree, but
aggkit could never recover the pre-image off-chain — the LET mirror
would permanently desync and the bridge-out path would brick.

Add an active_note::is_public assert at the top of bridge_out,
extend the bridge_out and b2agg.masm doc comments, and parameterize
the existing destination-network-is-Miden test into
test_bridge_out_rejects_invalid_b2agg_note covering the
recipient-identical private-note case too.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>

* chore: fmt files

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>
Co-authored-by: Marti <marti@miden.team>
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>

* chore: bump version to v0.15.1

* feat: allow root-allowlisted tx scripts on network accounts (#3028)

feat: allow root-allowlisted tx scripts on network accounts (#3028)

AuthNetworkAccount previously banned every transaction script outright. This generalizes that ban into a tx-script-root allowlist that mirrors the existing note-script allowlist: a transaction that runs no tx script is allowed, and any tx script that does run must have its MAST root in an allowlist fixed at account creation. This lets network accounts run owner-approved scripts (e.g. setting the expiration delta, needed by the ntx-builder) while preserving the invariant that only root-pinned, pre-approved code can run against a signature-less account.

Changes:
- Add `TransactionScriptRoot` newtype mirroring `NoteScriptRoot`; `TransactionScript::root()` now returns it.
- Add `tx_script_allowlist.masm` (`assert_tx_script_allowed`) and switch the network auth component to it; remove the now-unused `assert_no_tx_script`.
- Add `NetworkAccountTxScriptAllowlist` storage (`allowed_tx_scripts` slot); an empty allowlist permits no tx scripts (the prior behavior).
- Wire through `AuthNetworkAccount::with_allowed_tx_scripts` and `AuthMethod::NetworkAccount.allowed_tx_script_roots`.

A root pins a script's code but not its `TX_SCRIPT_ARGS`/advice inputs, which an arbitrary submitter controls, so only scripts whose behavior is fixed regardless of those inputs should be allowlisted.

Breaking: the component gains a second storage slot (existing network accounts must be recreated), and `TransactionScript::root()` returns `TransactionScriptRoot` instead of `Word`.

Closes #3027.

Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>

* refactor: rename network-account note allowlist API (#3049)

* refactor: rename network-account note allowlist API

* chore: increment crate versions to v0.15.2 (#3053)

* chore: add RUSTSEC-2026-0173 to deny.toml

* feat(agglayer): make bridge network ID a deployment setting (#3062)

* feat(agglayer): make bridge network ID a deployment setting

The bridge's AggLayer network ID was hardcoded as the MASM constant
MIDEN_NETWORK_ID and compiled directly into the bridge contract, baking
it into the bridge code commitment. This forced testnet/mainnet, which
may use different network IDs, to ship different bridge binaries.

Store the network ID in the bridge account instead:

- Add a `network_id` value storage slot (`agglayer::bridge::network_id`),
  written once at account creation.
- `bridge_in`/`bridge_out` read it via a new `bridge_config::load_network_id`
  helper instead of `push.MIDEN_NETWORK_ID`, so the bridge code commitment
  is identical across all networks.
- Require `network_id` in `AggLayerBridge::new` and
  `create_(existing_)bridge_account`; add an `AggLayerBridge::network_id`
  reader and `network_id_slot_name` accessor.
- Remove the hardcoded `MIDEN_NETWORK_ID` constant entirely (MASM
  `constants.masm` and its build.rs parsing). Tests keep a fixture-only
  constant matching the bundled Solidity claim vectors.

The ID is never mutated by any bridge procedure, so it is effectively
immutable for the life of the account. Adds bridge-in and bridge-out tests
proving a bridge configured with a non-default network ID enforces it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* fix: remove unused AggLayerBridge import in bridge_in test

The top-level import is shadowed by a function-local `use` in
test_bridge_in_claim_to_p2id, so clippy flags it as unused with
-D warnings.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>

* docs(agglayer): sync spec, diagram & comments after faucet metadata move (#2980)

* docs(agglayer): update spec/diagram/comments after #2771

PR #2771 moved faucet metadata onto the bridge (faucet_metadata_map),
removed the faucet's asset_to_origin_asset / get_metadata_hash / get_scale
procs, added the native lock/unlock path, and grew CONFIG_AGG_BRIDGE to 18
felts -- but left several docs referring to the old FPI-based design.

- SPEC.md 3.1: drop FPI references in bridge_out/bridge_in, fix
  register_faucet inputs and the [1, is_native, 0, 0] registry value, add
  store_faucet_metadata_hash + internal metadata-reader proc docs, add the
  faucet_metadata_map storage row, document the native lock/unlock branch.
- SPEC.md 3.2: faucet component now re-exports only mint_and_send +
  receive_and_burn; delete the removed proc subsections and the dead faucet
  storage slots.
- SPEC.md 4.3: CONFIG_AGG_BRIDGE is 18 felts; fix the layout table and
  consumption (register_faucet + store_faucet_metadata_hash).
- bridge_out.masm / bridge_out.rs: drop 'via FPI' wording.
- bridge-out diagram: 'FPI: get amount/metadata' -> bridge-storage reads;
  regenerate PNG.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* Update crates/miden-agglayer/SPEC.md

* fix: rustfmt formatting in bridge_out.rs doc comment

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>

* chore: increment crate versions to v0.15.3 (#3076)

* feat(agglayer): make bridge network ID a deployment setting (#3062)

* feat(agglayer): make bridge network ID a deployment setting

The bridge's AggLayer network ID was hardcoded as the MASM constant
MIDEN_NETWORK_ID and compiled directly into the bridge contract, baking
it into the bridge code commitment. This forced testnet/mainnet, which
may use different network IDs, to ship different bridge binaries.

Store the network ID in the bridge account instead:

- Add a `network_id` value storage slot (`agglayer::bridge::network_id`),
  written once at account creation.
- `bridge_in`/`bridge_out` read it via a new `bridge_config::load_network_id`
  helper instead of `push.MIDEN_NETWORK_ID`, so the bridge code commitment
  is identical across all networks.
- Require `network_id` in `AggLayerBridge::new` and
  `create_(existing_)bridge_account`; add an `AggLayerBridge::network_id`
  reader and `network_id_slot_name` accessor.
- Remove the hardcoded `MIDEN_NETWORK_ID` constant entirely (MASM
  `constants.masm` and its build.rs parsing). Tests keep a fixture-only
  constant matching the bundled Solidity claim vectors.

The ID is never mutated by any bridge procedure, so it is effectively
immutable for the life of the account. Adds bridge-in and bridge-out tests
proving a bridge configured with a non-default network ID enforces it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* fix: remove unused AggLayerBridge import in bridge_in test

The top-level import is shadowed by a function-local `use` in
test_bridge_in_claim_to_p2id, so clippy flags it as unused with
-D warnings.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>
(cherry picked from commit 5572686)

* docs(agglayer): sync spec, diagram & comments after faucet metadata move (#2980)

* docs(agglayer): update spec/diagram/comments after #2771

PR #2771 moved faucet metadata onto the bridge (faucet_metadata_map),
removed the faucet's asset_to_origin_asset / get_metadata_hash / get_scale
procs, added the native lock/unlock path, and grew CONFIG_AGG_BRIDGE to 18
felts -- but left several docs referring to the old FPI-based design.

- SPEC.md 3.1: drop FPI references in bridge_out/bridge_in, fix
  register_faucet inputs and the [1, is_native, 0, 0] registry value, add
  store_faucet_metadata_hash + internal metadata-reader proc docs, add the
  faucet_metadata_map storage row, document the native lock/unlock branch.
- SPEC.md 3.2: faucet component now re-exports only mint_and_send +
  receive_and_burn; delete the removed proc subsections and the dead faucet
  storage slots.
- SPEC.md 4.3: CONFIG_AGG_BRIDGE is 18 felts; fix the layout table and
  consumption (register_faucet + store_faucet_metadata_hash).
- bridge_out.masm / bridge_out.rs: drop 'via FPI' wording.
- bridge-out diagram: 'FPI: get amount/metadata' -> bridge-storage reads;
  regenerate PNG.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* Apply suggestions from code review

Co-authored-by: Marti <marcin.gorny.94@protonmail.com>

* Update crates/miden-agglayer/SPEC.md

* fix: rustfmt formatting in bridge_out.rs doc comment

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>
(cherry picked from commit eeafd71)

* docs: align v0.15.x changelog history with main

The cherry-pick-based syncs from main left next's CHANGELOG missing the
v0.15.1 and v0.15.3 release sections and mislabeled the v0.15.1 section
as a duplicated v0.15.0 heading. Restore the release history as recorded
on main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test: assert the bridge network ID via the Rust storage reader

Exercises AggLayerBridge::network_id, which previously had no caller,
against the value seeded at account creation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: fix changelog

---------

Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
Co-authored-by: Andrey Khmuro <andrey@polygon.technology>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Marija Mijailović <mijailovicmarija@hotmail.com>
Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>
Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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