feat(pxe): constrained tag sync optimization and recipient logs sync benchmarks - #24275
Conversation
Constrained delivery emits a gapless tagging-index stream, so the recipient can stop at the first missing tag instead of always probing a full UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN (20) window per secret. The initial probe is INITIAL_CONSTRAINED_PROBE_LEN tags and grows to the full window only when every probed index has a log; probe advancement is decoupled from finalized-cursor persistence so unfinalized logs at the top of the run are still fetched. Adds a unit micro-benchmark for the per-sync tag-query cost.
The mocked node makes sync-time an unfaithful end-to-end latency; real full-sync timing already lives in the client_flows /sync e2e bench (ProvingTimings.sync). Keeps the metrics the micro-bench is for: tag-queries, round-trips, and blocking-time.
Add a realistic mixed-population row (999 idle + 1 deep straggler at K=100) and run deep catch-up at 1000 secrets as well as 100, so every value in the PR-description tables is produced by the committed bench. Generalize scenario seeding and assertions to a per-secret new-log distribution and compute firstMissOptimum from it.
The constrained tag-sync source restated "gapless stream stops at the first missing tag" ~5x and the doubling-probe mechanics ~4x. Keep one canonical home for each (the module JSDoc for the why, the nextProbeLen comment for the mechanics) and reduce the rest to one-line local facts; also correct boundEnd's comment to note it is an exclusive bound and probeLen's to note the queried span can be smaller when boundEnd caps it. Drop the tag-sync micro-bench from bench_cmds: it existed to find the probe-growth numbers locally, which is done, so we no longer track it on the benchmark dashboard. It stays runnable as a local tool, and the doubling behavior remains covered in CI by the unit-test file.
…' into mv/optimize-constrained-tag-sync
Revert the in-place strengthening of the existing unconstrained and mixed tests and re-express the constrained-specific coverage as three new tests in the constrained-secrets block: multi-secret batching, halting at the first all-miss batch (sentinel proof), and doubling-probe re-anchoring past the cold-start bound.
The constrained-delivery optimization rewrote the old "continues when all indexes in the batch have logs" test (which scanned the full window every round) into the constrained doubling-probe test, leaving the unconstrained full-window-scan drain of a run longer than one window without direct coverage. Add an unconstrained test pinning that a contiguous run filling and exceeding the cold-start window drains fully: all logs returned, both cursors advanced to the last index, and per-round query sizes of [WINDOW_LEN + 1, WINDOW_LEN] showing full-window re-anchoring rather than a doubling probe.
The constrained-sync micro-benchmark is skipped in CI (test_cmds skips *.bench.test.ts and it is not in bench_cmds), so bring its coverage into sync_tagged_private_logs.test.ts, which runs in CI: - log-then-linear round-count law (K=1,3,20,50,100 -> 2,3,5,6,9 rounds) - batched straggler: a single deep straggler among idle secrets sets the round count while idle secrets each cost one probe and drop - cold-start constrained-vs-unconstrained scan-shape contrast - tighten the deep-cap test to pin the linear-past-cap regime Qualify the overstated ~log2(K) catch-up claim in constants.ts, sync_tagged_private_logs.ts, and the bench: catch-up is logarithmic only while the probe doubles, then linear at ~K/WINDOW_LEN once it saturates the cap. No behavior change.
nchamo
left a comment
There was a problem hiding this comment.
I'm out of power at home, but I managed to review part of the PR
| // WINDOW_LEN probe into a single tag. A secret K logs behind catches up in ~log2(K) round-trips while the probe is still | ||
| // doubling (1, 2, 4, 8, 16), but once it saturates the cap and advances WINDOW_LEN tags per round, deeper catch-up is | ||
| // linear at ~K/WINDOW_LEN rounds. Either way it beats both the full window every round and one round per log. | ||
| export const INITIAL_CONSTRAINED_PROBE_LEN = 1; |
There was a problem hiding this comment.
I'm curious if you considered the possibility of starting at 2 vs 1. It would scale faster for catch-ups, and the overhead isn't that big. Not sure if it's the best option, but I wanted to understand if you had looked into it
There was a problem hiding this comment.
Good thought, I had not looked at that.
I added columns to the PR desc for an initial probe starting at 2 and 4. For an initial=2, it does look like it saves one round-trip in the catch-up and mixed cases. It doubles the number of tag-queries in the steady state case which seems like the dominant situation for syncing. However, even saving one round-trip in the catch-up cases seems like it it is worth it as both probes have the same amount of round trips in the steady state scenario.
For an initial=4 the round trips reduce again by 1 from initial=2. However, in these benches I mock the blocking time with a set 5ms, I imagine in a real scenario this would be greater as the node will actually have to do the work to fetch the tags. We see at initial=4 that the total blocking time starts to increase over lower initial probes in the deep catch up scenarios. initial=2 feels like the sweet spot as we remove an extra round-trip while only the per-secret cost only grows by one tag for steady state and deep catch up scenarios.
I have switched the initial probe to 2. Let me know if you agree.
There was a problem hiding this comment.
Let's start with 2 and we can change it back to 1 if we see a need in the future
Move the duplicated extractTags and computeSiloedTagForIndex helpers into a shared tagging/testing module, consumed by the recipient sync test, the bench, and the sender sync test.
Constrained-only probe state (boundEnd, probeLen) now lives on the constrained variant instead of riding along as optional fields on every pending secret. Callers narrow on the kind discriminant before touching that state.
Use foundation's sum() instead of hand-rolled reduce, and run the independent per-secret seed writes and hit-tag computation concurrently.
The doubling schedule and ~log2(K) then ~K/WINDOW_LEN complexity analysis now lives in one canonical block on INITIAL_CONSTRAINED_PROBE_LEN. The recipient sync module docstring, the inline probe comment, and the bench docstring link to it instead of restating it.
Replace the expectedConstrainedProbeRanges helper, which re-derived the expected probe schedule from the same logic as the SUT, with hardcoded golden probe sizes and round-trip counts. A future drift in the scan now fails the test instead of being mirrored by the oracle.
…' into mv/optimize-constrained-tag-sync
|
❌ Failed to cherry-pick to |
Promotes `v5-next` onto `v5` for the **v5.2.0** release. Frozen at `ee5d2d367e` — the `v5-next` tip at cut time. Both `merge-train/spartan-v5` and `merge-train/fairies-v5` are drained to that same commit, so nothing staged is left behind. ### Testnet validation This branch was cut at `a4db216abf`, which is byte-identical to the `v5.2.0-nightly.20260815` tag — same commit, same tree (`0b22572eace2419ede6bbab173514d6965d4e0d0`). That nightly's CI3 run is green, its artifacts are published to npm and Docker Hub, and it has been **running healthily on testnet since 2026-08-15**: | Component | Image / reported version | |---|---| | validators, prover node, prover broker, prover agents (`testnet` ns) | `aztecprotocol/aztec:5.2.0-nightly.20260815` | | `v5.testnet.rpc.aztec-labs.com` | `nodeVersion=5.2.0-nightly.20260815` | | `canonical.testnet.rpc.aztec-labs.com` | `nodeVersion=5.2.0-nightly.20260815` | Chain advancing normally, no prunes or reorgs, no WARN/ERROR across node pods since rollout. The branch has since been fast-forwarded to `ee5d2d367e` to pick up #25242 (configurable RPC server HTTP timeouts and CORS allowed-headers). That is the only delta from the soaked tree — 8 files, +131/−10, no nightly covers it yet. Its config defaults were checked against Node's built-ins (`keepAliveTimeout` 5000 ms, `headersTimeout` 60000 ms) and match exactly, and the CORS default path resolves to the same `cors()` call as before, so a node that sets none of the new env vars behaves identically. It touches no circuits, protocol contracts, or generated constants. ### Manifest `.release-please-manifest.json` reads `5.2.0` on this branch, which is the released version — correct as-is, no change needed in this PR. `v5-next` moves to `5.3.0` separately in #25240; this branch is frozen and cannot pick that up, so the two can merge in either order. `v5` is a strict ancestor of `v5-next` this cycle, so there was no manifest conflict to pre-resolve. ### Scope 139 commits (86 non-merge, 23 PR-level) spanning 2026-07-14 to 2026-08-17. | Area | Theme | |---|---| | `prover-node` / `prover-client` | Epoch-proving robustness: retry-to-converge with failure declared only at submission-window expiry, ticker-driven expiry, per-checkpoint post-mortem upload, checkpoint-only re-proving, prune-induced fault handling (#24678, #24982, #24983, #24990, #25027) | | `pxe` | Sync performance: hash-pinned node read cache (#24969), anchor-bounded tag log caching (#25074), note/event validation from cached tx data (#25076), constrained tag sync (#24275), sender tagging finalization from log blocks (#25045) | | `p2p` / `validator-client` | Gossip tx validation no longer stalls behind tx-pool finalization (#25148); startup fails when p2p fails to start (#25177); slashing/proposing/health/`sendTx` gated on p2p connectivity (#25185); duplicate time-sensitive proposal validation removed (#25207); `ValidatedProposal` branding (#25222) | | `ethereum` / `aztec-node` | L1 watchers poll `getLogs` instead of `eth_newFilter` (#25176); block stream sync no longer resolves against an earlier pass (#25206) | | `stdlib` / `foundation` | Deserialization bounds hardening (#25026, #25028, #25029, #25109); checkpoint block-shape and block-count validation (#25229); JSON-RPC cookies (#25231) | | `archiver` | Removed-block cleanup and ownership-checked tx-effect deletes (#24765); L2→L1 witness from a single store snapshot (#24754) | | `aztec` | Declarative deployment framework at `@aztec/aztec/deploy` (#24685) | | `slasher` | Own-validator slash-target warnings and metrics (#25058) | | telemetry | JSON-RPC metrics (#25159) | | JSON-RPC server | Configurable HTTP keep-alive / headers timeouts and CORS allowed-headers, defaults preserving current behaviour (#25242) | | toolchain | Noir bumped to `v1.0.0-beta.25` (#24907) | **One breaking change**, inherited from the Noir bump: note types declared directly inside a `contract` module must now be `pub`. Everything else is additive or internal. ### Protocol constants Built from source and compared against `v5` — a cache-free rebuild of every protocol circuit with each ref's own nargo and bb, then regeneration of `vk_tree.ts` and `protocol_contract_data.ts`: - `vkTreeRoot` = `0x2b3b6ea4412b9c8f6457a37f91a2870306f8641e07e16a49b68bda6f8bc02892` — unchanged from v5.1.0 - `protocolContractsHash` = `0x2c075866eafc88a1f6f9addc7e337c6e64e45d1cb7fd7c0d612ebcec72aab2ca` — unchanged from v5.1.0 The Noir beta.24 → beta.25 bump does not reach the circuits: the release build consumes the committed `pinned-build.tar.gz`, which is bit-identical between v5.1.0 and this commit (blob `3bedcb1fd1…`), so the protocol-circuit bytecode is frozen rather than recompiled. The 47 verification keys were recomputed locally from that pinned bytecode with the cache disabled, and all 47 `check_pinned_vk` checks passed. `protocolContractsHash` is likewise backed by the new `pinned-protocol-contracts.tar.gz`, whose three artifacts were byte-compared against the build. Both values were also confirmed inside the published `@aztec/protocol-contracts` and `@aztec/noir-protocol-circuits-types` packages for `5.2.0-nightly.20260815`. That makes v5.2.0 a drop-in upgrade against the current rollup rather than a coordinated one. ✅ **Gate re-run against `ee5d2d367e` (the current head) and passed** — both values reproduced exactly from a build at this commit, with `vkTreeRoot` numerically evaluated rather than inferred. `29556326ce..ee5d2d3` touches no `noir-projects/**`, `l1-contracts/src/**`, `ConstantsGen.sol`, `constants.gen.ts` or `constants.nr`. Scope of the check: it verifies that the pinned circuit bytecode plus locally recomputed VKs agree with the pin — not that a from-source recompile reproduces v5.1.0's bytecode. <details> <summary>Commits added after the original constants check at 2955632</summary> `#25207` `#25222` `#25206` `#25185` `#25176` `#25163` `#25229` `#25159` `#25162` `#25231` `#25224` `#25228` `#25230` `#25242` plus their merge commits. </details> ### Known gap Migration notes carry entries under `## TBD` that arrived with the docs baseline backport (#25017) and describe `next`-line changes not present on the v5 line — protocol contracts removed from `@aztec/noir-contracts.js`, and the `at(wallet)` → `withWallet(wallet)` deprecation. Neither exists on this branch. There is also no `## 5.1.0` heading, and the entries that ship in v5.2.0 sit under `## TBD` rather than a version heading. This ships in the release docs, so it is worth correcting on `v5-next` and backporting to `v5` before the tag, rather than after.
Fixes F-704
Summary
Constrained delivery emits a gapless tagging-index stream, so PXE can stop scanning at the first missing constrained tag instead of probing the full unfinalized window on every sync.
This PR:
findHighestIndexesunchanged.Benchmarks
Benchmarks compare doubling against fixed-size probes. The takeaway is that doubling preserves the steady-state tag-query floor of
P=1while avoidingP=1's one-round-trip-per-log catch-up behavior.A sync runs in rounds: each round computes the next batch of tags, sends them to the node, and blocks on the result before deciding the next round. All counts are per sync.
Scenarios seed a recipient that already synced prior messages, then measure the next single sync.
secrets = Nmeans N independent sender streams synced together in one batched pass.K=100) at 1,000 secrets.Cost per sync: doubling vs fixed-P alternatives
doubling is the shipped policy. The fixed-P columns are the selection comparison that motivated it:
P=84is the current full-window behavior, andP=1..5sweep the constant-step alternative.Unconstrained is not shown as its own row: its windowed scan cannot first-miss, so it is invariant to P and reproduces the
P=84column.Tag-queries (thousands) (exact count = value x 1,000; bold = fewest among the fixed-P columns;
doubling (init=1)is the shipped policy,doubling (init=2)/doubling (init=4)start the probe at 2/4 instead of 1):Round-trips (bold = fewest among the fixed-P columns;
doubling (init=1)is the shipped policy,doubling (init=2)/doubling (init=4)start the probe at 2/4 instead of 1):Blocking wall-clock (ms) (reported only, noisy; the three
doublingcolumns are from one paired re-run, the fixed-P columns from the earlier comparison sweep, so not every column is from a single run):Takeaway
P=1is the tag-query floor, but it pays one round-trip per new log during catch-up. The full window (P=84) minimizes catch-up round-trips, but it charges every idle secret the full-window tag cost on every sync.Doubling is the middle ground: it matches
P=1at steady state, stays close toP=1on tag queries during catch-up, and collapses deep catch-up round-trips geometrically. In the mixed scenario, doubling uses 1,126 tag queries vs 1,100 forP=1, but needs 7 round-trips instead of 101; compared withP=84, it avoids the 84,084-tag idle tax while staying within 5 round-trips of the full-window catch-up path.Testing