feat(aztec-nr): wire constrained message delivery by vezenovm · Pull Request #23866 · AztecProtocol/aztec-packages

vezenovm · 2026-06-04T13:57:52Z

Fixes F-669
Fixes F-670

The change

Adds constrained-delivery helpers that resolve or bootstrap the app-siloed handshake secret, seed same-tx index reuse, validate index 0 against the standard HandshakeRegistry, and constrain index > 0 through the previous chain nullifier.
Wires constrained private message delivery to derive constrained log tags from the resolved (secret, index) pair and emit the current constrained-message nullifier.
Keeps constrained logs from being squashed with note hashes so recipient discovery can advance the per-secret index chain.
Authorizes the standard HandshakeRegistry utility reads needed by the flow.
Adds TXE, snapshot, and e2e coverage for bootstrap, reuse, index advancement, missing prior-nullifier failure, invalid API combinations, and standard-registry constrained delivery.

Large portion of the diff is tests.

F-741 follow-up for the test skipped in fd07996.

Concurrency / batching constraint (worth a look)

Pinning the e2e tests surfaced a sequencing constraint in constrained delivery that reviewers should sanity-check:

Parallel sends on one (sender, recipient, secret) chain collide. Each send is keyed on an incrementing index, so two sends fired as separate parallel txs read the same index and one tx is rejected. Distinct recipients are distinct chains and parallelize fine. Pinned as it.failing (documents the limit; flips green if parallel sends ever become supported).
Same-chain sends can be batched into one tx, but only onto an already-committed handshake. A later send discharges its predecessor check against a same-tx pending nullifier. Works both via a single contract call (emit_two_events) and client-side BatchCall.
Batching onto a brand-new chain does not reuse, it re-handshakes. The reuse-vs-bootstrap decision in get_or_create_app_siloed_handshake_secret is a utility call reading committed state, so a bootstrap earlier in the same tx is invisible and the later send mints a fresh secret on a separate chain (next index lands at 1, not 2). A new recipient therefore needs one landed tx to establish the chain before sends can be batched onto it. This is why the batched tests seed the handshake first.

All four behaviors are pinned in e2e_constrained_delivery.test.ts; the rationale lives in the module doc and on get_or_create_app_siloed_handshake_secret. Follow-up: allowing utility functions to execute against combined committed + pending transaction state is tracked in F-238.

) Fixes [F-697](https://linear.app/aztec-labs/issue/F-697/aztec-nr-extend-the-onchaindelivery-builder-for-secret-origin) Fixes [F-649](https://linear.app/aztec-labs/issue/F-649/fixaztec-nr-skip-note-log-linkage-for-onchain-constrained-delivery-to) ## Summary - Extends `MessageDelivery` with separate typed builders for unconstrained and constrained on-chain delivery. - Adds internal tag-secret derivation config: - offchain: none - onchain unconstrained: address-pair by default, with optional non-interactive handshake selection - onchain constrained: non-interactive handshake - Introduces `OnchainDeliveryMode` as the typed on-chain mode used by tagging-index helpers and the HandshakeRegistry ABI instead of raw `u8` values. - Keeps invalid delivery/tag-derivation combinations rejected at compile time where possible. - Stops linking constrained-tagged note logs to note-hash squashing, so removing a squashed note does not break the recipient tag index chain. - Re-pins the HandshakeRegistry standard contract and updates the generated standard-contract metadata. Constrained tagging is still not fully wired up: the delivery config is validated, but tag emission remains mocked through the existing wallet-derived unconstrained path behind TODO(#14565). Stack: #23875, then this PR, then #23866, then #23867. --------- Co-authored-by: AztecBot <tech@aztec-labs.com> Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

…et_and_index Also corrects the safety comment on the registry utility call: a duplicate handshake replaces the stored note rather than failing, so the actual argument is that every branch constrains the secret it returns.

…ery-helpers

nchamo

I'm still going over it, but wanted to share some initial feedback

nchamo · 2026-06-17T17:36:54Z

+  return (
+    functionSelector.equals(await STANDARD_HANDSHAKE_REGISTRY_GET_APP_SILOED_SECRET_SELECTOR) &&
+    args.length >= 4 &&
+    // TODO(F-671): will be replaced with `self.msg_sender()` once utility context exposes it.


While it does make sense to have this, since we already have a PR in place (#24062), I would avoid it, simply to avoid merging conflicts

I would avoid it, simply to avoid merging conflicts

Just for clarity sake, you mean this entire check right?

I mean the part about:

args.length >= 4 && args[3].equals(caller.toField())

nchamo · 2026-06-17T18:03:33Z

 /// The fields are private and there is no public constructor: a `MessageDelivery` can only be produced by a
-/// [`MessageDeliveryBuilder`] that enforces valid configurations, so invalid field combinations cannot be
-/// represented to the consumer.
+/// [`MessageDeliveryBuilder`]. The delivery APIs validate the built configuration before consuming it.


I know that we are currently allowing to create "invalid" states, but that is only temporary (for a few PRs), right? The end state will only allow devs to build valid states, right?

Yes that will be the end state.

Then don't specify this. Adding tests/docs that we know we'll soon (in a few days) update doesn't make much sense. It adds more work for both you (coding the update and rollback) and me (reviewing it twice). If you feel the need to clarify it for the review, add a Github comment and I'll take it into account

Or if you feel very strongly that the intermediate state must be clarified (I don't think this change needs it) add a TODO. But don't edit and re-edit

Will leave TODOs for in-flight docs like this 👍

nchamo · 2026-06-17T18:41:05Z

+    )
+}
+
+mod test {


Is it me, or are we missing quite a lot of tests here?

I think we have a lot of behaviors and decision trees on this file that we are not testing. Could we?

Most of the coverage is contained in constrained_delivery_test_contract/src/test.nr but yeah this could do with some unit tests.

Added some unit tests

…src/main.nr Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

…_variables.md Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

…ule, handshake reg selector override constants

…lpers' into mv/f-669-constrained-delivery-helpers

nchamo

Some more comments after the full review

nchamo · 2026-06-19T17:54:22Z

+  return (
+    functionSelector.equals(await STANDARD_HANDSHAKE_REGISTRY_GET_APP_SILOED_SECRET_SELECTOR) &&
+    args.length >= 4 &&
+    // TODO(F-671): will be replaced with `self.msg_sender()` once utility context exposes it.


I mean the part about:

args.length >= 4 && args[3].equals(caller.toField())

nchamo · 2026-06-19T18:00:31Z

 /// The fields are private and there is no public constructor: a `MessageDelivery` can only be produced by a
-/// [`MessageDeliveryBuilder`] that enforces valid configurations, so invalid field combinations cannot be
-/// represented to the consumer.
+/// [`MessageDeliveryBuilder`]. The delivery APIs validate the built configuration before consuming it.


Then don't specify this. Adding tests/docs that we know we'll soon (in a few days) update doesn't make much sense. It adds more work for both you (coding the update and rollback) and me (reviewing it twice). If you feel the need to clarify it for the review, add a Github comment and I'll take it into account

Or if you feel very strongly that the intermediate state must be clarified (I don't think this change needs it) add a TODO. But don't edit and re-edit

nchamo · 2026-06-20T14:15:37Z

+}
+
+#[test]
+unconstrained fn delivery_bootstraps_handshake_and_advances_index() {


Ok, now I get what you were saying in https://github.com/AztecProtocol/aztec-packages/pull/23866/changes#r3430748990. I hadn't looked at this file yet. If you ask me, between the new unit tests there, and the e2e tests, I would get rid of these. Do you see something we are testing here that we are not testing there? I feel like these are a mix of:

Registry unit tests

Constrained delivery unit tests

e2e tests

I think the e2e are good enough as integration tests, but I would like your opinion on it

) ## Summary Addresses the review comments on #23866 (constrained message delivery). Branched off and based on that PR so these follow-up changes can be reviewed in isolation. - **aztec-nr helpers**: tightened the constrained-delivery docs, fixed the misleading `index > 0` nullifier comment, switched secret resolution to `map`/`unwrap_or_else`, and renamed "chain" -> "sequence" across code/tests to avoid colliding with the existing "chain" (blockchain) meaning. - **Noir tests/contracts**: pinned the `get_handshakes` selector in the registry selector-match test, renamed the test helper to `authorizing`, and dropped redundant TXE tests now covered by the e2e and unit suites (plus the `delivery_unconstrained_handshake` compile-failure pin). - **PXE**: removed the temporary `get_app_siloed_secret` caller-arg authorization check (avoids conflicting with #24062), moved the registry-read helpers below the class, and reworked the cross-contract authorization test into `it.each` with shared `beforeEach`/`afterEach`. - Removed a stray `pied!` file that had been accidentally committed. The docs changes from the review are deferred to #24040, which owns the tagging-secret-source framing.

…ery-helpers

…eployment contracts These account, token and benchmarking contracts delivered their notes with MessageDelivery::onchain_constrained(). That mode was effectively a no-op on v5-next, but #23866 (wire constrained message delivery) made it issue a real validate_handshake private call on the first send (index 0) of each (sender, recipient, secret) sequence. That extra app circuit broke the full (merge-queue-only) client_flows step-count benches once v5-next merged into merge-train/fairies-v5, dequeuing #24223: - account_deployments: 10 -> 11 execution steps - amm add_liquidity: 14 -> 17 execution steps and it advanced the per-secret tagging index until bench_build_block exceeded UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN (20), failing every tx commit until the 600s timeout. These flows test app/deployment logic and don't need constrained delivery (constrained delivery has dedicated coverage in constrained_delivery_test_contract), so deliver via onchain_unconstrained instead -- the behavior these benches were written against. No benchmark expectations change: they already encode the unconstrained (no-bootstrap) step counts. The warm-send benches (transfers, bridging) were unaffected either way, and the dedicated constrained_delivery_test_contract is intentionally left as-is.

## Summary Follow-up to #24312: audits where `MessageDelivery::onchain_constrained()` is used and drops it where it buys nothing. Constrained delivery only matters when the message sender isn't incentivized to deliver correctly and differs from the recipient; it also costs an extra nullifier, a handshake bootstrap, and serializes sends per `(sender, recipient, secret)` chain. - **Account constructors** (`schnorr`, `ecdsa_k`, `ecdsa_r`) self-deliver the signing-key note — with `.with_sender(self.address)`, sender and recipient are both the account, so the guarantee is vacuous. Note integrity is already constrained in-circuit by `create_note` before delivery, and discoverability is preserved by the sender override plus the self-note sender scan. Now `onchain_unconstrained().with_sender(self.address)`. - **Simulated account stubs** (`SimulatedSchnorrAccount` / `SimulatedEcdsaAccount`) reverted to their pre-#23866 shape — #23866 both wired constrained delivery into the real accounts and added the matching handshake dummies to these kernelless gas-estimation stubs. - **Test / example contracts** that deliver notes to `self.msg_sender()` (counter, child, static_child, scope_test, no_constructor, nested_utility, test, updated, updatable, private_init_test, and the counter scaffold) downgraded to `onchain_unconstrained()`.

…nt e2e tests (#24337) ## What Splits the token used in e2e tests into two contracts: - **canonical `TokenContract`** (`app/token_contract`) is unchanged and remains the production / public-docs source of truth. Several of its paths (`transfer_to_public`, `transfer_in_private`, `burn_private`, `finalize_transfer_to_private_from_private`, ...) use **constrained** message delivery; plain `transfer` already uses unconstrained delivery. - **`TestTokenContract`** (`contracts/test/test_token_contract`) is its all-**unconstrained-delivery** sibling, generated from canonical Token at build time by `scripts/gen_test_token.sh`. e2e suites where a token is just a unit-of-account vehicle now deploy `TestToken`. The benchmarks hand-enumerate the exact private calls a flow makes (`expectedExecutionSteps`); a constrained-delivery path's first send injects an extra handshake bootstrap that the hand-count doesn't include, which inflated the `client_flows/amm` step count (14 -> 17) once #23866 wired constrained delivery in. With `TestToken` the bench counts model the app flow again, with no `expectedExecutionSteps` changes. Constrained delivery's own cost/behavior stays covered by the dedicated `constrained_delivery_test_contract` + `e2e_constrained_delivery.test.ts`. ## How - `scripts/gen_test_token.sh` copies `app/token_contract`, drops the Noir TXE test module, flips all delivery sites to `onchain_unconstrained`, and renames the contract/package to `TestToken` / `test_token_contract`. The generated source is committed and marked generated. `TestToken` is regenerated from canonical Token automatically, so the two can't drift: | When | Action | Why | |---|---|---| | Local build | regenerate in place (bootstrap build) | an edited canonical Token shows up in `TestToken`-based e2e tests before any commit | | Commit (precommit hook) | regenerate + re-stage | keeps the committed source matching canonical Token (accurate diffs / IDE) | | CI | `--check` only | a stale `TestToken` fails with a clear message | Regeneration is idempotent (an unchanged Token stays a cache hit), which keeps `TestToken` a faithful "canonical Token minus the handshake" -- the premise that makes it a valid bench vehicle. - `token_utils.ts` gains `deployTestToken`; its mint/balance helpers accept either token. `getTokenAllowedSetupFunctions` now keys the FPC allowlist on the deployed token's class (defaulting to canonical Token), so the benches' `TestToken` fee vehicle is allowlisted on the right class. ### Repointed to TestToken (token is a vehicle) `bench/client_flows/*` (the amm bench + its shared base), `e2e_amm`, `forward-compatibility/e2e_amm`, `e2e_orderbook`, `e2e_partial_notes`, `e2e_multiple_accounts_1_enc_key`. In the bridging bench only the FPC fee token is repointed; the bridged L2 asset (deployed by `CrossChainTestHarness`) stays canonical Token, which is harmless because its claim / `mint_to_private` path doesn't use constrained delivery. ### Kept on canonical Token - Subject / discovery: `e2e_token`, the `e2e_2_pxes` cross-PXE section, `e2e_constrained_delivery`. - Gas / step sensitive (pass on constrained today): `e2e_fees/*`, `e2e_kernelless_simulation`. - Docs / deploy-mechanism: `e2e_deploy_contract/*` (the `deploy_token` docs snippet must show canonical Token). Context / rationale: https://gist.github.com/AztecBot/ee4c54b9100e4925cde9b2d17e53cdca and the constrained-delivery wiring in #23866. Follow-ups at the bottom of https://gist.github.com/AztecBot/d3537fa9e1aba8c2e0fe3a9d74698d5f. --------- Co-authored-by: AztecBot <tech@aztec-labs.com>

vezenovm requested review from LeilaWang and nventuro as code owners June 4, 2026 13:57

This was referenced Jun 4, 2026

feat(aztec-nr): wire constrained tagging into message delivery #23867

Closed

feat(aztec-nr): extend OnchainDelivery builder for secret origin #23865

Merged

vezenovm force-pushed the mv/f-697-general-delivery-builder branch from b9f12b8 to befb4ea Compare June 5, 2026 01:34

vezenovm requested review from IlyasRidhuan, MirandaWood, jeanmon and sirasistant as code owners June 5, 2026 01:34

vezenovm force-pushed the mv/f-669-constrained-delivery-helpers branch from 69abd69 to e2a43cd Compare June 5, 2026 01:34

vezenovm removed request for IlyasRidhuan, LeilaWang, MirandaWood, jeanmon and sirasistant June 5, 2026 01:44

Base automatically changed from mv/f-697-general-delivery-builder to merge-train/fairies-v5 June 10, 2026 15:51

feat(aztec-nr): add constrained delivery helper

c552943

vezenovm force-pushed the mv/f-669-constrained-delivery-helpers branch from 6337393 to c552943 Compare June 10, 2026 17:55

vezenovm added 3 commits June 10, 2026 16:39

comments

000162b

fmt

36681e6

.

0271756

vezenovm mentioned this pull request Jun 10, 2026

feat(aztec-nr): Helper to validate the app-siloed secret and index for constrained tagging #23569

Closed

vezenovm changed the title ~~feat(aztec-nr): add calculate_secret_and_index constrained-delivery helper~~ feat(aztec-nr): add resolve_secret_and_index constrained-delivery helper Jun 10, 2026

vezenovm added 2 commits June 10, 2026 18:56

Merge branch 'merge-train/fairies-v5' into mv/f-669-constrained-deliv…

5699c52

…ery-helpers

Merge branch 'merge-train/fairies-v5' into mv/f-669-constrained-deliv…

389493b

…ery-helpers

vezenovm requested a review from nchamo June 11, 2026 14:19

vezenovm added 2 commits June 11, 2026 10:34

some minor simplifications

c334adc

shared unconstrained/constrained tag computaiton

cd2fbea

nchamo reviewed Jun 17, 2026

View reviewed changes

vezenovm and others added 10 commits June 17, 2026 14:55

Update noir-projects/noir-contracts/contracts/test/test_log_contract/…

f6b9c71

…src/main.nr Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

comment updates

05ffc5a

one more comment

45a9a9d

Update docs/docs-developers/docs/aztec-nr/framework-description/state…

b6eb170

…_variables.md Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>

pr review cleanup and comments

f73130b

more snap and test removal

41411a4

constrained_delivery unit tests, re-org get_app_or.. to handshake mod…

73b3c40

…ule, handshake reg selector override constants

Merge remote-tracking branch 'origin/mv/f-669-constrained-delivery-he…

56d84b2

…lpers' into mv/f-669-constrained-delivery-helpers

index >0 comment

833f58e

fix unit test

c98d885

nchamo self-requested a review June 19, 2026 17:56

nchamo reviewed Jun 20, 2026

View reviewed changes

nchamo mentioned this pull request Jun 20, 2026

refactor(aztec-nr): address constrained-delivery review comments #24217

Merged

nchamo and others added 2 commits June 22, 2026 10:09

Merge branch 'merge-train/fairies-v5' into mv/f-669-constrained-deliv…

a22266d

…ery-helpers

nchamo approved these changes Jun 22, 2026

View reviewed changes

vezenovm added 2 commits June 22, 2026 10:55

fix up calls to get_app_siloed_secret

76c29c8

empty

3d1f990

vezenovm merged commit 518d851 into merge-train/fairies-v5 Jun 22, 2026
12 checks passed

vezenovm deleted the mv/f-669-constrained-delivery-helpers branch June 22, 2026 15:31

This was referenced Jun 22, 2026

feat: merge-train/fairies-v5 #24223

Merged

fix(noir-contracts): deliver notes via onchain_unconstrained in app/deployment contracts #24333

Closed

nchamo mentioned this pull request Jun 26, 2026

refactor(noir-contracts): drop unneeded constrained delivery #24336

Merged

vezenovm mentioned this pull request Jun 26, 2026

test(noir-contracts): TestToken with unconstrained delivery and repoint e2e tests #24337

Merged

AztecBot mentioned this pull request Jun 26, 2026

fix(noir-contracts): switch Benchmarking note delivery to onchain_unconstrained #24341

Closed

AztecBot mentioned this pull request Jul 21, 2026

chore: port v5-next to next #24825

Open

Uh oh!

Conversation

vezenovm commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The change

Concurrency / batching constraint (worth a look)

Uh oh!

nchamo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vezenovm Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nchamo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vezenovm commented Jun 4, 2026 •

edited

Loading

vezenovm Jun 17, 2026 •

edited

Loading