Skip to content

test(private-kernel): reject a duplicate nullifier index hidden behind an inactive squashing hint - #25580

Merged
iAmMichaelConnor merged 1 commit into
nextfrom
cb/reset-squash-hidden-duplicate-test
Oct 2, 2026
Merged

test(private-kernel): reject a duplicate nullifier index hidden behind an inactive squashing hint#25580
iAmMichaelConnor merged 1 commit into
nextfrom
cb/reset-squash-hidden-duplicate-test

Conversation

@AztecBot

@AztecBot AztecBot commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Test-only. No circuit change and no VK change: one test and its import in validate_transient_data_squashing_hints.nr.

Refs AztecProtocol/aztec-claude#2313.

What the test covers

validate_transient_data_squashing_hints proves that the active hints' nullifier indices are unique through a sorted side-table of (elem, original_index) tuples. A prover controls those tuples. fails_duplicate_nullifier_index_with_sorted_tuple_from_inactive_hint builds the forgery the last step of that argument exists to stop:

  • two active hints share nullifier index 0;
  • the second sorted tuple points at an inactive hint, so it carries the sentinel index instead of the repeated 0 and the "increasing order" check passes;
  • one extra nullifier_squash_flags entry is set, so the removed-count check balances and a nullifier that no hint names would be dropped.

With a full nullifiers array (claimed length equal to the sentinel), the circuit rejects this at Nullifier index hint exceeds claimed length. The existing duplicate tests use honestly derived tuples and are rejected one step earlier, so nothing covered this path.

Why it is worth having

Mutation check: replacing the range check on the last sorted value with a range check on each active hint's nullifier_index (a plausible refactor that keeps every other test green) makes the circuit accept this witness. All 92 existing reset::transient_data tests still pass under that mutation; this test fails with Test passed when it should have failed.

Checks run locally

Pinned nargo 1.0.0-rc.3: the 9 tests in this file pass and nargo fmt --check is clean. The rest is left to CI.

Created by claudebox · group: slackbot · requested by Mike (@iAmMichaelConnor) · Slack thread

@AztecBot AztecBot added ci-draft Run CI on draft PRs. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure claudebox Owned by claudebox. it can push to this PR. labels Oct 2, 2026
@iAmMichaelConnor
iAmMichaelConnor marked this pull request as ready for review October 2, 2026 12:35
@iAmMichaelConnor
iAmMichaelConnor enabled auto-merge (squash) October 2, 2026 12:36
@iAmMichaelConnor
iAmMichaelConnor merged commit b1b6871 into next Oct 2, 2026
29 of 35 checks passed
@iAmMichaelConnor
iAmMichaelConnor deleted the cb/reset-squash-hidden-duplicate-test branch October 2, 2026 13:20
@AztecBot AztecBot mentioned this pull request Oct 2, 2026
Merged
15 tasks done
AztecBot added a commit that referenced this pull request Oct 4, 2026
@AztecBot
test(private-kernel): reject a duplicate nullifier index hidden behin…
91458ba 
…d an inactive squashing hint (#25580)

Test-only. No circuit change and no VK change: one test and its import
in `validate_transient_data_squashing_hints.nr`.

Refs AztecProtocol/aztec-claude#2313.

## What the test covers

`validate_transient_data_squashing_hints` proves that the active hints'
nullifier indices are unique through a sorted side-table of `(elem,
original_index)` tuples. A prover controls those tuples.
`fails_duplicate_nullifier_index_with_sorted_tuple_from_inactive_hint`
builds the forgery the last step of that argument exists to stop:

- two active hints share nullifier index 0;
- the second sorted tuple points at an inactive hint, so it carries the
sentinel index instead of the repeated 0 and the "increasing order"
check passes;
- one extra `nullifier_squash_flags` entry is set, so the removed-count
check balances and a nullifier that no hint names would be dropped.

With a full nullifiers array (claimed length equal to the sentinel), the
circuit rejects this at `Nullifier index hint exceeds claimed length`.
The existing duplicate tests use honestly derived tuples and are
rejected one step earlier, so nothing covered this path.

## Why it is worth having

Mutation check: replacing the range check on the last sorted value with
a range check on each active hint's `nullifier_index` (a plausible
refactor that keeps every other test green) makes the circuit accept
this witness. All 92 existing `reset::transient_data` tests still pass
under that mutation; this test fails with `Test passed when it should
have failed`.

## Checks run locally

Pinned `nargo` 1.0.0-rc.3: the 9 tests in this file pass and `nargo fmt
--check` is clean. The rest is left to CI.

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/bcd0da92a2cf9af3/jobs/14)
· group: `slackbot` · requested by Mike (@iAmMichaelConnor) · [Slack
thread](https://aztecfoundation.slack.com/archives/D0B2N7W1WJD/p1790886272710929?thread_ts=1790886272.710929&cid=D0B2N7W1WJD)*

(cherry picked from commit b1b6871)
IlyasRidhuan added a commit that referenced this pull request Oct 4, 2026
@IlyasRidhuan
chore: port-to-v6 rolling port (#25575)
8fbd463 
Rolling integration PR for changes labelled `port-to-v6` on `next`. Each
merged source PR is cherry-picked onto `cb/port-to-v6` as its own
commit, keeping `(#N)` in the title.

## Ported

- [x]
[#25536](#25536)
fix(kernels): charge the teardown call's AVM startup gas (pick applied
without conflicts, plus two `fix(port)` commits)
- `fix(port)` 1 drops
`labs-patches/0001-fix-pxe-charge-the-teardown-call-s-AVM-startup-gas-i.patch`,
which the pick brings along. The labs pin on `v6` (`a0d73c0aed`) already
carries that PXE change upstream as aztec-labs-eng/aztec-node#364, so
the patch has nothing to apply to. The Noir side is `gas_meter.nr` and
`validate_gas_used_tests.nr`, both identical to `next`.
- `fix(port)` 2 adds labs patch
`0001-test-pin-testnet-compatibility-to-the-v6-kernel-VK-t.patch`, which
re-pins the VK tree root in
`labs/yarn-project/aztec/src/testnet_compatibility.test.ts`.
- [x]
[#25576](#25576)
fix(p2p): require exact contract class log length in DataTxValidator
(labs patch) (clean pick)
- Adds labs patch
`0010-fix-p2p-require-exact-contract-class-log-length-in-D.patch`. Both
files it touches (`data_validator.ts`, `data_validator.test.ts`) have
the same pre-image blobs on the `v6` labs pin as on `next`'s.
- [x]
[#25577](#25577)
fix(l1): include the Noir compiler in the l1-contracts cache hash (clean
pick)
- `noir/bootstrap.sh hash` is what the other Noir consumers on `v6`
already use (`avm-transpiler`, `noir-protocol-circuits`, `acir_tests`).
- [x]
[#25578](#25578)
fix(circuits): key the VK cache on its real inputs and never cache the
L1 verifier (clean pick)
- With #25577 and this one applied,
`noir-protocol-circuits/bootstrap.sh` and `scripts/circuit_kind.test.sh`
are identical to `next` at b0a7559.
- [x]
[#25579](#25579)
fix(private-kernel): a surviving nullifier link may only resolve to a
non-revertible note hash (pick applied without conflicts, plus one
`fix(port)` commit)
- It changes the `private-kernel-reset-tail-to-public` circuits, so the
VK tree root moves again. The `fix(port)` commit updates labs patch 0001
to the new root.
- [x]
[#23587](#23587)
fix: anchor Grumpkin SRS to pinned chunk hashes on load (clean pick)
- All eight files end up identical to `next` at 08f90e6, as does the
rest of `barretenberg/cpp/src/barretenberg/srs`. No later commit on
`next` touches them.
- [x]
[#25580](#25580)
test(private-kernel): reject a duplicate nullifier index hidden behind
an inactive squashing hint (clean pick)
- Test-only: one test and its import in
`validate_transient_data_squashing_hints.nr`.
- With #25536, #25579 and this one applied, the `private-kernel-lib`
crate is identical to `next` at b1b6871.
- [x]
[#25530](#25530)
chore: move away from auto-close-issues.yml (clean pick)
- Deletes `.github/workflows/auto-close-issues.yml` and
`scripts/auto_close_issues.py`. Nothing else on `v6` invokes either.
- [x]
[#25566](#25566)
docs(types): document that poseidon2_hash_bytes does not commit the byte
length (clean pick)
- A doc comment and three unit tests in `types/src/hash.nr`, which ends
up identical to `next` at 4f4bef8.
- [x]
[#25582](#25582)
fix: Avoid early proof submission prover griefing (clean pick)
- [x]
[#25585](#25585)
test: cover prover registration upgrade paths (clean pick)
- Review follow-up to #25582: forge tests, a test wrapper
simplification, and one corrected comment in `RewardLib.sol`. No
contract logic changes.
- [x]
[#25570](#25570)
refactor(l1): remove the built-in checkpoint reward overrides (clean
pick)
- `v6` carries the override stack this removes (#25312, #25426, #25461)
and the proof-submission optimizations it keeps (#25404, #25406,
#25419).
- Nothing outside `l1-contracts` references what it removes
(`registryRewardOverrides`, `AZTEC_REGISTRY_REWARD_OVERRIDE_*`,
`getRegistryRewardOverrides`, the four `RewardLib__*` errors): no hits
in this repo or in the `v6` labs pin `a0d73c0aed`.
- [x]
[#25571](#25571)
feat(l1): pluggable sequencer reward calculator (AZIP-28) (clean pick)
- Adds `ISequencerRewardCalculator`, `SequencerRewardCalculatorLib`, the
`sequencerRewardCalculator` rollup config field,
`setSequencerRewardCalculator` / `getSequencerRewardCalculator`, and the
optional `AZTEC_SEQUENCER_REWARD_CALCULATOR` deploy variable (zero
address by default).
- Nothing in the `v6` labs pin references the calculator. Labs imports
`network-defaults.json` directly, where the new key is an added default.
- [x]
[#25572](#25572)
test(l1): reference registry reduction sequencer reward calculator
(clean pick)
- Test-tree only (`l1-contracts/test/reward-calculators`, a fork test
and its fixture), plus the `foundry.toml` read permission for the
fixture and regenerated gas reports.
- [x]
[#25573](#25573)
test(l1): reference premium sequencer reward calculator with
provenance-tracking ATPs (clean pick)
- Test-tree only (`l1-contracts/test/reward-calculators/premium`), plus
regenerated gas reports.
- With #25577, #25578, #25582, #25585, #25570, #25571, #25572 and this
one applied, the whole `l1-contracts` directory is identical to `next`
at 551aa41.

## Protocol impact

- #25536 and #25579 both change private kernel circuits, so the VK tree
root that `v6` builds moves:
- from
`0x2d89003cc2dc62b06f07d83d3635c66c63fc43668369d30e7ee516f908ee10e3`
(the testnet deployed from `v6`)
- to
`0x05b95d9ef57c61d1c6626545101db55ae2d1f98ecb618d1c220610c049ee522b`
(taken from this PR's failing CI run on `34e0b24f`), which is what labs
patch 0001 pins
- the intermediate root with #25536 alone was
`0x21f06c3b82320ea5f80646751cdd9fa023dfd61400a42b8c952aa32e8f155793`
(from the failing CI run on `bed8264`)
- After this merges, `v6` no longer matches the currently deployed
testnet until it is redeployed. The re-pin follows the decision recorded
on #25549 (a breaking kernel change ported to `v6` re-pins testnet in
the same PR).
- The protocol contracts hash and the genesis root are unchanged; those
two assertions passed on `34e0b24f`.
- #25570 and #25571 change how the L1 rollup pays sequencers:
- the per-registry checkpoint reward overrides built into the rollup are
gone, along with `RollupConfigInput.registryRewardOverrides`,
`getRegistryRewardOverrides` and the `AZTEC_REGISTRY_REWARD_OVERRIDE_*`
deploy variables
- sequencer reward policy is delegated to an optional governance-set
calculator contract; with none configured every proposer gets the
default sequencer share
- #25572 and #25573 add reference calculators under `l1-contracts/test`
only; no calculator ships in `src` or is configured by default
- #25582 changes the L1 rollup contracts: `RewardLib` reward handling
for early proof submissions, and a new `getHasSubmittedFullEpoch` view
on `IRollup` / `Rollup`.
- Per #25585, the prover node (`prover-node-publisher.ts` in
`aztec-labs-eng/aztec-node`) still skips submission when
`getHasSubmitted` is true. It has to use `getHasSubmittedFullEpoch` for
full-epoch proofs before the fix helps honest provers; that change is
not in this PR or in the `v6` labs pin's patch series.
- The L1 changes take effect only on a rollup deployed from this code.
- #25536 is a repricing: a public tx with a teardown call is charged
20_000 more L2 gas. The PXE on `v6` already estimates that amount.
- #25579 rejects, in tail-to-public only, a nullifier whose note hash
link resolves to a revertible note hash. No honest client produces that
witness; the private-only tail circuit is unchanged.
- #25576 tightens p2p tx validation: a contract class log whose `length`
exceeds its trimmed length is now rejected. Honest producers already
emit the trimmed length.
- #23587 makes bb check a loaded Grumpkin SRS against pinned chunk
hashes. It changes the bb binary, not any circuit.
- #25580 and #25566 add unit tests (and a doc comment) and change no
circuit.
- #25530 removes a GitHub workflow: issues referenced by commits pushed
to `v6` are no longer closed by the script.
- #25577 and #25578 are build-cache only and change no circuit:
  - the l1-contracts cache key now follows the Noir commit
- the VK cache key now covers the circuit kind and the bootstrap script,
so every VK is regenerated once
- the Solidity verifier is written on every build instead of restored
from the cache
- l1-contracts gains a test that `generated/HonkVerifier.sol` equals the
circuits-side verifier

## Testing

- CI3 passed on `fb097a89` (the first eleven ports, through #25585),
including the testnet compatibility test against the re-pinned root and
the l1-contracts forge tests.
- CI3 on `34e0b24f` (the first nine ports, before the second re-pin) ran
about 10,400 tests; the only failure was the VK tree root assertion that
the second re-pin fixes.
- `./labs-patches/bootstrap.sh check`: the series (0001, 0010) applies
cleanly to `a0d73c0aed`. Patch 0001 was produced with `bootstrap.sh
apply` / `export`.
- #25577 / #25578: `bash -n` passes on both bootstrap scripts and
`scripts/circuit_kind.test.sh` passes locally. `v6` has no
`pinned-build.tar.gz`, so the pinned-build path is not exercised.
- No circuits, bb or L1 contracts were built locally (no `nargo`, no bb
build and no pinned `solc` in the port environment). The four AZIP-28
ports (#25570 to #25573) have no CI result yet: the forge suite, the
labs build against the changed rollup ABI and the deploy-script tests
run on this PR's CI.
- The branch is based on `v6` at cf7ea33; `v6` has since gained
#25551 (a notify workflow change), which merges without conflicts.

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/134efca9a11ee14f/jobs/7)
· group: `slackbot` · [Slack
thread](https://aztecprotocol.slack.com/archives/C0AGN2WT3CP/p1790609320989739?thread_ts=1790609320.989739&cid=C0AGN2WT3CP)*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iAmMichaelConnor iAmMichaelConnor iAmMichaelConnor approved these changes
@LeilaWang LeilaWang Awaiting requested review from LeilaWang LeilaWang is a code owner
@iakovenkos iakovenkos Awaiting requested review from iakovenkos iakovenkos is a code owner
@ledwards2225 ledwards2225 Awaiting requested review from ledwards2225

Assignees

No one assigned

Labels

ci-draft Run CI on draft PRs. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure claudebox Owned by claudebox. it can push to this PR. port-to-v6

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AztecBot @iAmMichaelConnor