feat!: azip 8 public key hashes#23159
Conversation
e55bfae to
8464e00
Compare
8464e00 to
608173e
Compare
cd809b8 to
d145a8a
Compare
Domain separator for hashing individual public keys per AZIP-8.
Value 3452068255 derived from poseidon2("az_dom_sep__single_public_key_hash").
Wired through the constants codegen (constants.in.ts) to TypeScript,
C++, and PIL outputs so the AVM and downstream consumers see it.
Part of AZIP-8 public key hashing migration.
06fe0f9 to
d49756b
Compare
Replaces the four-point PublicKeys struct with one Point (ivpk_m) plus three field-element hashes (npk_m_hash, ovpk_m_hash, tpk_m_hash). Adds the canonical primitive 'hash_public_key(p) = poseidon2(DOM_SEP__SINGLE_PUBLIC_KEY_HASH, [p.x, p.y])' and rewrites PublicKeys::hash to combine the four hashes under DOM_SEP__PUBLIC_KEYS_HASH. Drops NpkM, OvpkM, TpkM wrapper structs (only IvpkM survives because ivpk_m must remain a point for encrypt-to-address). validate_on_curve and validate_non_infinity now apply only to ivpk_m -- the other three keys become PXE-side trust assumptions per AZIP-8 Security Considerations. CONTRACT_INSTANCE_LENGTH drops from 17 to 11 fields (post-AZIP-9 baseline: AZIP-9 had bumped it to 17 by adding `immutables_hash`; AZIP-8 collapses the public-keys block from 12 fields to 6, net 17 - 6 = 11). Propagated via remake-constants. Test golden values regenerated. Downstream consumers (aztec-nr, kernel circuits, contracts) will not compile until subsequent commits migrate them; types crate test suite passes (367 tests).
Migrate aztec-nr (`state_vars`, `macros/notes.nr`, `oracle/keys.nr`) and noir-contracts (`nft_note.nr`, `cards.nr`, scope test) to read `.npk_m_hash` directly from the AZIP-8 reshape. Rewrite `ContractInstancePublished.serialize_non_standard` to 13 fields. The `ContractInstance.VERSION` 1 → 2 bump itself ships upstream via AZIP-9 and is not included here; this commit's contract-side change is the new payload shape, and the comment "Version 2 carries hashes for npk/ovpk/tpk and the affine coordinates of ivpk only (per AZIP-8)" documents the wire layout that pairs with that bump. Also updates the `publish_contract_instance_for_public_execution` helper for the new layout: arg buffer 17 → 11 fields, public-keys loop `0..12 → 0..6`, `universal_deploy` index 16 → 10, and refresh the function selector signature to the new `PublicKeys` shape. Without that helper update, `test_contract::publish_contract_instance` constant-folds an out-of-bounds read into an unsatisfiable AssertZero at compile time. The numbers here are the post-AZIP-9 baseline: AZIP-9 added `immutables_hash` to the arg buffer (bumping it to 17) and grew `serialize_non_standard` to 16 fields, so AZIP-8 takes 17 → 11 and 16 → 13 by collapsing the public-keys block.
Updates the rust example to read .npk_m_hash directly (instead of .npk_m.hash()) and documents that npk/ovpk/tpk are now exposed only as hashes.
- keys.md: rewrite the public_keys_hash pseudocode to show npk/ovpk/tpk as their hashes plus an in-circuit ivpk_m_hash; add an admonition noting the SVG diagram is pending refresh. - custom_notes.md: switch the rust code example to read .npk_m_hash directly (drop the .npk_m.hash() pattern).
Add a `## TBD` migration entry to `docs/.../migration_notes.md` covering contract-author, TS/wallet, indexer, and PXE-security migration steps. Adds `unspendable` to the cspell dictionary -- the word is used in the security note that describes the PXE-side risk if `KeyStore.addAccount`'s on-curve / non-infinity check is bypassed (notes encrypted to a malformed `ivpk_m` can never be decrypted). Common UTXO-domain term, just not in the project's word list yet.
The TS PublicKeys.default() previously hardcoded the three default hashes as Fr literals. If anyone changed DEFAULT_NPK_M_X/Y (or the hashing primitive) in constants.nr without updating those literals, TS and Noir would silently drift on the default-hash values, breaking address derivation symmetry for default-key accounts. Promote them to first-class constants: - DEFAULT_NPK_M_HASH, DEFAULT_OVPK_M_HASH, DEFAULT_TPK_M_HASH in constants.nr; flow through remake-constants to constants.gen.ts so the TS side reads from @aztec/constants. (Not currently propagated to aztec_constants.hpp because no C++ caller references them.) - PublicKeys::default() (Noir) reads them directly. - PublicKeys.default() (TS) reads them from @aztec/constants. - New types-crate test 'default_hashes_match_default_points' asserts hash_public_key(DEFAULT_*_M_X/Y) == DEFAULT_*_M_HASH; this catches drift between the points and the precomputed hashes.
After replacing point.hash() with direct field reads on PublicKeys,
several files still imported the Hash trait or Point type that they
no longer use:
Hash trait (no longer needed since the .hash() resolution moved to a
field access):
- aztec-nr state_vars: private_immutable, private_mutable,
single_private_immutable, single_private_mutable, single_use_claim
- aztec-nr uint-note
- noir-contracts: card_game_contract/cards, nft_contract/nft_note,
test/scope_test_contract
Point type (only ever appeared as the Point literals I replaced with
plain Field values during the KVR reshape):
- private-kernel-lib tests: private_kernel_inner/output_composition_tests,
private_kernel_tail/previous_kernel_validation_tests,
private_kernel_tail_to_public/previous_kernel_validation_tests
- protocol-test-utils/src/fixture_builder
TS:
- pxe/private_kernel/private_kernel_oracle.ts: drop Point (signature
changed to take pkMHash: Fr in fix #4)
- noir-protocol-circuits-types/conversion/client.ts: drop
mapPointFromNoir (only used by the old KVR fromNoir mapping)
All 816 private-kernel-lib tests still pass; both Noir workspaces and
the full TS build are clean.
The `PXE_DATA_SCHEMA_VERSION` bump from 5 to 6 itself is delivered upstream by AZIP-9.
This commit adapts the PXE schema-test fixtures and snapshots so the v6 baseline
captures the AZIP-8 on-disk shape:
- The `ContractStore` schema-test fixture uses the new `PublicKeys` constructor
(`Fr, Point, Fr, Fr`) and `version: 2`.
- `AddressStore.complete_addresses` / `complete_address_index` keys change because
the address derived from a given secret changes (AZIP-8 hashes `public_keys_hash`
over four single-key digests rather than four raw points; AZIP-9 also inserts
`immutables_hash` into `salted_initialization_hash`).
- `KeyStore.key_store` re-keys all per-account material (including the existing
`npk_m_hash` / `ovpk_m_hash` / `tpk_m_hash` / `ivpk_m_hash` entries) under the
new derived address.
- `ContractStore.contracts_instances` carries the new PublicKeys layout.
The three affected per-store snapshots (`AddressStore.json`, `KeyStore.json`,
`ContractStore.json`) are regenerated to the v6 baseline.
This remains BREAKING for end users: the same secret produces a different address, so
existing on-device state cannot be migrated forward at the storage layer.
`DatabaseVersionManager` (using the upstream v6 bump) wipes pre-AZIP-8 PXE DBs on first
open; users re-sync from L1 and operate under the new address.
…a fixture for AZIP-8 The committed `ContractInstancePublishedEventData.hex` fixture was generated by an e2e deploy under the pre-AZIP-8 protocol (4-Point `PublicKeys`, `version: 1`). After AZIP-8 it parses as a malformed v1 event, and `ContractInstancePublishedEvent.toContractInstance` rejects it with "Unexpected contract instance version 1", failing: - archiver `data_store_updater.test.ts` (3 tests) - protocol-contracts `contract_instance_published_event.test.ts` snapshot Regenerated synthetically with `version: 2`, the new `PublicKeys` layout (`npkMHash, ivpkM, ovpkMHash, tpkMHash`), and an `address` actually derived from the instance fields via `computeContractAddressFromInstance` so the archiver's `updateDeployedContractInstances` address-consistency check passes. Snapshot regenerated accordingly. Field values are deterministic so the fixture is reproducible. Strictly an offline regeneration -- the proper end-to-end regenerator (`e2e_deploy_contract/contract_class_registration.test.ts` with `AZTEC_GENERATE_TEST_DATA=1`) is still the canonical producer for "real on-chain payloads" and remains the path to use once that test suite is rerun.
…for AZIP-8
The `Computes contract info for {default,parent,updated}Contract` snapshot tests pin the
derived `address` and `public_keys` values for the three default fixture contracts. Both fields
shift under AZIP-8: `public_keys` shrinks from 8 fields (4 raw Points) to 5 fields
(`npkMHash`, `ivpkM`, `ovpkMHash`, `tpkMHash`), and `address = (preaddress * G + ivpkM).x` with
`preaddress = H(public_keys_hash, partial_address)` -- where `public_keys_hash` is now hashed
over the four single-key digests rather than the four raw points.
The new addresses match the golden values documented in `AZIP_8_IMPLEMENTATION_SUMMARY.md`
("Test golden values that moved"). All other snapshot fields (`artifact_hash`,
`contract_address_salt`, `contract_class_id`, `deployer`, `partial_address`,
`private_functions_root`, `public_bytecode_commitment`, `salted_initialization_hash`)
are unchanged.
…eys layout The AVM testdata fixture used by `avm_minimal.test.ts` (and downstream C++ AVM tests) embeds msgpack-serialized `AvmCircuitInputs`, including hints that carry the old 4-Point `PublicKeys` layout. After AZIP-8 the serialized PublicKeys shape changes (npkMHash + ivpkM + ovpkMHash + tpkMHash), and the buffer fails to match the committed binary. Regenerated with `AZTEC_GENERATE_TEST_DATA=1` on the test itself (the canonical regenerator, per the comment at line 45). File size: 189056 -> 190583 bytes (grows by ~1.5 KB; the AZIP-9 `immutables_hash` addition and the inner-kernel scenario added by commit 18 outweigh the PublicKeys-block shrink from AZIP-8).
… layout
`avm.test.ts` ("serialization sample for avm2") compares the
msgpack-serialized `AvmCircuitInputs` against the committed
`avm_inputs.testdata.bin`. The fixture embeds `PublicKeys` in the
encoded inputs, so the AZIP-8 layout change (4 Points -> 1 hash + 1 Point +
2 hashes) shifts the buffer.
Regenerated via `AZTEC_GENERATE_TEST_DATA=1` on the test itself (the
documented canonical regenerator). File shrinks 2085068 -> 2081088
bytes, consistent with the PublicKeys layout change applied across the
serialized inputs.
…l test data
The `'generates sample Prover.toml files'` test in `e2e_prover/full.test.ts` lists
`'private-kernel-inner'` in the regen array but the test scenario only submits Token
`transfer` calls, which the PXE planner packs into `private_kernel_init_2` — plain
`private_kernel_inner` never fires, `pushTestData('private-kernel-inner', ...)` is never
called, and `getTestData('private-kernel-inner')` returns `[]`. Result:
`private-kernel-inner/Prover.toml` is left untouched by regen and silently goes stale.
After the AZIP-8 / AZIP-9 derivation changes invalidated the formerly-stale fixture's
address hints, this gap surfaced as a `nargo execute` failure on the inner kernel circuit
("computed contract address does not match expected one"). The fix is to seed the test
with a 4-call private chain that the planner splits as `init_3 + inner`:
entrypoint (init_3)
parent.private_nested_static_call (init_3)
parent.private_call (init_3)
child.private_get_value (inner)
Deploys `ParentContract` + `ChildContract`, seeds a note via `child.private_set_value`
(needed because `private_get_value` reads notes and a static call can't write them), then
`proveInteraction`s the nested-call interaction. The existing regen loop now finds data
in `getTestData('private-kernel-inner')` and writes the file.
`proveInteraction` alone is enough to capture the witness — the tx doesn't need to land.
After the AZIP-8 wire-format and address-derivation changes, the committed sample inputs
under `noir-projects/noir-protocol-circuits/crates/*/Prover.toml` were stale on two axes:
1. Schema: the `public_keys` sub-table used the old wrapped-Point layout
(`public_keys.npk_m.inner.{x,y,is_infinite}`, same for ovpk_m / tpk_m), and the
key-validation requests carried `request.pk_m.{x,y,is_infinite}`. The new layout collapses
these to single-Field hashes (`public_keys.npk_m_hash` / `.ovpk_m_hash` / `.tpk_m_hash`,
and `request.pk_m_hash`); `ivpk_m` stays as a Point (it remains a curve point in-circuit
for address derivation).
2. Values: every derived field that depended on `public_keys_hash` had to be recomputed --
addresses, salted_initialization_hash, pre_address, etc. -- because the hash now ingests
the four single-key digests instead of the four raw points.
Regenerated end-to-end against the live prover stack: run
`e2e_prover/full.test.ts` with `AZTEC_GENERATE_TEST_DATA=1 REAL_PROOFS=true`. That test
pushes data via `pushTestData` from the orchestrator/private-kernel code paths and writes
the Prover.tomls at `full.test.ts:250-278`. This is the canonical regenerator -- the values
in the committed files are now self-consistent against the post-AZIP-8 derivation flow.
Closes "Outstanding manual step #5" (Prover.toml regen) -- five private-kernel and seven
rollup circuits in scope.
…nt-disable)
Four lint issues surfaced by `yarn lint`:
- `private_execution.test.ts` -- the `accountHasKey` mock used an `async` arrow with no
`await`. Drop the `async` and wrap returns in `Promise.resolve(...)` so the mock still
satisfies the `Promise<boolean>` signature.
- `utility_execution.test.ts` and `private_kernel/hints/test_utils.ts` -- `Point`
imports are no longer referenced after the AZIP-8 reshape (point-typed master keys are
now hashes); drop the imports.
- `aztec-node/server.ts` -- the `eslint-disable-next-line
aztec-custom/no-non-primitive-in-collections` directive no longer matches a fired rule
("Unused eslint-disable directive"). The lint rule now accepts `CheckpointNumber` in
`Map<...>` keys, so drop the disable and the `TODO(palla)` that marked it.
The `aztec-node` change is pre-existing drift unrelated to AZIP-8; folded in here since it
was reported together with the other findings and is a small fix.
d49756b to
5b8e69a
Compare
…/3 kernel flavors Four new kernel-circuit variants (`private-kernel-init-2`, `private-kernel-init-3`, `private-kernel-inner-2`, `private-kernel-inner-3`) ship upstream; none had committed Prover.toml fixtures. Without them, `nargo execute` on those crates fails the same way the inner kernel did before commit #18 -- input missing. The PXE private-kernel planner batches private apps greedily at N=3, so we need three transactions of different shapes to cover all four init/inner flavors: - 4 apps (entrypoint -> parent.private_nested_static_call -> parent.private_call -> child.private_get_value) -> init_3 + inner - 5 apps (BatchCall: nested chain + one extra leaf call) -> init_3 + inner_2 - 6 apps (BatchCall: nested chain + two extra leaf calls) -> init_3 + inner_3 Extends `e2e_prover/full.test.ts`'s regen array to list the four new circuits and adds the three nested+batched `proveInteraction` calls. Selector and per-shape factories are hoisted so the call sites stay readable. To pass `BatchCall` to `proveInteraction`, widens its `interaction` parameter from `ContractFunctionInteraction | DeployMethod` to also accept `BatchCall` (already exported from `@aztec/aztec.js/contracts`). No aztec.js surface change. Verified: nargo execute --program-dir noir-projects/noir-protocol-circuits/crates/private-kernel-init-2 \ --silence-warnings --skip-brillig-constraints-check (and the analogous commands for init-3, inner-2, inner-3) All four print "Witness saved" with no assertion failures.
| // First we check that the hash of the derived public key matches the requested pk_m_hash. | ||
| let pk_m: Point = derive_public_key(sk_m).into(); | ||
|
|
||
| // Reject the point at infinity. Without this, an attacker could supply sk_m = 0 (which derives |
There was a problem hiding this comment.
@IlyasRidhuan I think we can remove this assertion. The app does not control sk_m and therefore cannot mount such an attack anyway. The security property that is required is that the key sk_m needs to be kept confidential. The specific case with point at infinity is just a special case of confindentiality leakage but it is not per-se an issue (contrary to the standard case of a DH key exchange).
WDYT?
There was a problem hiding this comment.
You're right, i think the comment in the code is implying that an attacker is able to convince a user to deploy a contract where the secret key is "known" (e.g. a buggy/malicious wallet has a bad/manipulated entropy source). Although that isnt reserved to the sk_m = 0, so this check barely protects against that (and we dont expect to protect against that).
On the other hand it's a cheap check that probably eliminates someone deploying a contract key with (0 * G) public key.
I dont have a strong opinion either way
There was a problem hiding this comment.
Ok, I think I will keep the assertion as a "nice safeguard".
There was a problem hiding this comment.
Comment will only mention "Safeguard against using a secret key = 0."
nventuro
left a comment
There was a problem hiding this comment.
Looks good other than some points on comments. In general I'd avoid mentioning the AZIP, which feels a bit odd. There's also a few comments that are slightly incorrect or misleading.
IlyasRidhuan
left a comment
There was a problem hiding this comment.
Changes in cpp and ts look good to me. A couple of things to clarify and can you remove the AZIP-8 mentions from the cpp/ts as well.
Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
I applied all suggestions and removed other instances of AZIP-8 mention in the source code. |
BEGIN_COMMIT_OVERRIDE feat!: add immutables_hash to contract instance (AztecProtocol#23091) feat!: update address derivation (AztecProtocol#23151) feat(avm)!: add immutables_hash member to get contract instance opcode (AztecProtocol#23152) feat!: azip 8 public key hashes (AztecProtocol#23159) feat(avm)!: Remove `is_infinite` flag from AVM ECC & update noir submodule with serialization changes (AztecProtocol#23342) fix(avm): addressing claude review of interaction builders code (AztecProtocol#23431) END_COMMIT_OVERRIDE
Implementation of AZIP-8