Conversation
nchamo
added
ci-no-fail-fast
…strained-delivery-to-a-non-interactive
nchamo
commented
Jun 29, 2026
vezenovm
approved these changes
Jun 30, 2026
vezenovm
left a comment
vezenovm
left a comment
Contributor
There was a problem hiding this comment.
just some nits, looks good!
…strained-delivery-to-a-non-interactive
nchamo
enabled auto-merge (squash)
July 1, 2026 01:06
nchamo
deleted the
nchamo/f-770-default-unconstrained-delivery-to-a-non-interactive
branch
July 1, 2026 02:46
This was referenced Jul 1, 2026
AztecBot
added a commit
that referenced
this pull request
Jul 1, 2026
The bench measures steady-state app cost. #24387 defaulted unconstrained delivery to a non-interactive handshake, which injects two extra private app executions and a nullifier per cold (sender, recipient) chain — reflected in the +APPS_PER_ONCHAIN_HANDSHAKE_DELIVERY constant this commit removes. Wire a resolveTaggingSecretStrategy hook on the bench's user wallet that returns address-derived for unconstrained delivery (falling back to a non-interactive handshake for constrained, since Noir rejects address-derived there). Matches the pre-#24387 behavior for unconstrained sends without adding a pre-bootstrap tx or its associated nullifier.
vezenovm
pushed a commit
that referenced
this pull request
Jul 1, 2026
…bench; widen the PXE unfinalized tagging window (#24429) ## Summary - widen the PXE `UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN` to `MAX_PRIVATE_LOGS_PER_TX` (aligning the window with the per-tx private-log cap, which is the real bound the store is gated on) - wire a `resolveTaggingSecretStrategy` hook on the client-flows bench user wallet that returns `address-derived` for unconstrained delivery, restoring the pre-#24387 tagging secret path for these benches ## Investigation The merge-train PR #24388 was removed from the merge queue on 2026-07-01 at 13:05 UTC after CI3 failed in `bench all` on merge-group SHA `84262a7ee42e1c391c00b55b377910ddb4308230`, in run `28515530242`. The concrete failures: - `client_flows/transfers`: expected 7 execution steps but got 10 for the sponsored-FPC transfer case, matching 2 extra app executions plus one extra kernel batch after #24387 (`feat(pxe): default unconstrained delivery to a handshake`). - `bench_build_block`: timed out after repeated `Highest used index 21 is further than window length from the highest finalized index 0. Tagging window length 20 is configured too low` errors. Both failures share a root cause: #24387 defaulted unconstrained delivery to a non-interactive handshake for external recipients, and the benches don't pre-establish a chain, so every timed transfer inflates a cold-chain handshake bootstrap into the measured numbers. ## Why the hook, not just wider fudges The bench's subject is steady-state app cost. Baking `+2 app executions per cold handshake` into `expectedExecutionSteps` measures a first-time-only cost every run, and re-tunes with every future handshake-bootstrap change. Instead, the user wallet now wires a `resolveTaggingSecretStrategy` hook that returns `{ type: 'address-derived' }` for unconstrained delivery — the same tagging secret path this bench got before #24387. The hook only affects the client-flows bench's user wallet, not production defaults. Constrained delivery is unaffected: the Noir circuit rejects `address-derived` there, so the hook falls back to `non-interactive-handshake` for the constrained branch. ## Why the window bump stands independently The previous doc-comment on `UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN` was self-contradictory: the header text said the value had to be larger than `MAX_PRIVATE_LOGS_PER_TX` "because there could be more than MAX_PRIVATE_LOGS_PER_TX indexes consumed in case the logs are squashed," but the constant was set to `20` (well below `MAX_PRIVATE_LOGS_PER_TX = 64`). The real limit is one full private-tx's worth of unfinalized tag indexes per chain; aligning the constant with that bound makes the semantic clearer. Refs #24388, #24387. ## Testing - Diff verified: `client_flows/transfers.test.ts` now runs with the pre-#24387 `expectedExecutionSteps(1 + apps + 1 + recursions)` counts. - Local unit tests / `bench all` will re-run under CI3 on this branch's push.
vezenovm
added a commit
that referenced
this pull request
Jul 6, 2026
…E_LOGS_PER_TX + headroom, not exactly MAX_PRIVATE_LOGS_PER_TX (#24437) ## Summary Follow-up to #24429. That PR widened `UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN` from `20` to exactly `MAX_PRIVATE_LOGS_PER_TX` (64). This PR argues that's still not the right value for the scenario #24429 was trying to fix, and proposes `MAX_PRIVATE_LOGS_PER_TX + 20`. **Scope note (see "Known limitation" below):** this fixes a real, CI-confirmed problem; multiple ordinary pending txs to the same counterparty stacking up before the first is mined. There is a separate, harder problem captured in [F-783](https://linear.app/aztec-labs/issue/F-783/pxe-unfinalized-tagging-indexes-window-len-cannot-bound-a-squash-heavy). Additional separate breakage follow-up on the sender-side when pending logs exceed the window [F-784](https://linear.app/aztec-labs/issue/F-784/pxe-typed-error-and-resync-recheck-when-pending-tagging-indexes-exceed). ## Investigation `MAX_PRIVATE_LOGS_PER_TX` is a genuine, protocol-enforced bound for ordinary transaction patterns: `noir-protocol-circuits` `constants.nr` defines `MAX_PRIVATE_LOGS_PER_TX` directly as `MAX_NOTE_HASHES_PER_TX`. A fresh secret's very first ordinary tx can legitimately consume that many indexes before anything is finalized. But exactly `MAX_PRIVATE_LOGS_PER_TX` leaves zero headroom for a second unconfirmed tx to the same counterparty once the first one is already near the max. `finalized` only advances once the sender's own node observes a tx as mined (see `syncSenderTaggingIndexes`), so any burst of sends to one counterparty before the first is mined can exhaust the window. This is exactly what happened in #24429's `bench_build_block` failure: `#24387`'s handshake default added extra per-cold-chain overhead, and the bench builds up a backlog of unmined txs by design, so `used` ran past `finalized` (which stayed at `0`) before window `20` could absorb it. ## Confirmed live: reproduced on this branch at bare `20` To settle whether #24429's bump was actually necessary, we reset this branch's constant back to bare `20` and let `bench_build_block` run to completion in CI rather than relying on the reasoning above alone. It failed the same way (see http://ci.aztec-labs.com/4ed074af4b8ab633 from [a4feea1](a4feea1), on the current `merge-train/fairies-v5` tip: ``` ERROR: kv-store:lmdb-v2 pxe-0 Failed to commit transaction: Error: Highest used index 21 is further than window length from the highest finalized index 0. Tagging window length 20 is configured too low. Contact the Aztec team to increase it! ``` So this isn't a synthetic worst case: it's a real failure on real CI infrastructure, at index `21`, i.e. just one past the window. Not even a large burst of txs. ## Red/green evidence Added `sender_tagging_store.test.ts`: "allows an ordinary pending tx to stack on a fresh secret already at the `MAX_PRIVATE_LOGS_PER_TX` floor" — one tx uses the full worst-case 64-log burst from a fresh secret, then a second, ordinary-sized pending tx to the same secret must still be storable before either is mined. - **Red at bare `20`:** the *first* call in that test already throws, with zero other txs involved: ``` Highest used index 63 is further than window length from the highest finalized index 0. Tagging window length 20 is configured too low. ``` - **Green at `MAX_PRIVATE_LOGS_PER_TX + 20`:** same scenario passes. This is the key point for review: the `bench_build_block` failure isn't just "risky under bursty load".`20` is mathematically incapable of tolerating even one legitimate worst-case single ordinary tx, independent of what CI on this branch shows either way. CI passing at `20` only means our current test/bench suite doesn't happen to construct a single tx tagging more than 20 logs to one recipient; it was never testing the general safety of the constant, just this specific workload's exposure to it. The live CI failure above is the same underlying gap, just surfaced through a different, real workload instead of a targeted unit test. We also added tests to pin the window's stacking semantics: a run of window+1 one-log pending txs to one secret fits, while the next tx throws. ## Why `+ 20`, not `* 2` or another additive constant - The floor (`MAX_PRIVATE_LOGS_PER_TX`) covers one worst-case *ordinary* tx. The failure mode above is about *additional, ordinary-sized* pending txs to the same counterparty stacking up, not another maxed-out 64-log tx. - It's the value the team had already judged sufficient for realistic pending-tx headroom before #24429 (it was just misapplied as the *entire* budget instead of a margin on top of the floor). --------- Co-authored-by: Martin Verzilli <martin@aztec-labs.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
#24312 made constrained delivery default to a non-interactive handshake when no handshake covers
(sender, recipient). This does the same for unconstrained delivery, branching on the recipient: an external recipient defaults to a non-interactive handshake so it discovers the message without having registered the sender, while a self-send (the recipient is one of the wallet's own accounts) uses an address-derived secret, which needs no handshake and leaves no onchain trace.The change
resolveTaggingStrategy(PXE) defaults both delivery modes to a non-interactive handshake, then always falls back to an address-derived secret for an unconstrained self-send.Fixes F-770