feat!: include init_hash in private initialization nullifier to prevent privacy leak#21427
Conversation
nchamo
added
the
ci-no-fail-fast
|
|
||
| const mockContractInstance = async (artifact: ContractArtifact, address: AztecAddress) => { | ||
| contracts[address.toString()] = artifact; | ||
| const mockContractInstance = async (artifact: ContractArtifact) => { |
There was a problem hiding this comment.
Now assert_is_initialized_private (in Noir) calls get_contract_instance(address) to fetch the contract's initialization_hash. That oracle validates that instance.to_address() == address, meaning the instance fields must actually hash to the expected address. Previously, the tests used AztecAddress.random() with incomplete/fake contract instances, which broke under this new validation.
So we made a few changes to fix this inconsistency and now, instead of using a random address, we use the actual instance's address
| contractAddress: parentAddress, | ||
| anchorBlockHeader, | ||
| artifact: ParentContractArtifact, | ||
| artifact: parentContractArtifact, |
There was a problem hiding this comment.
This was a bug, we were supposed to use the clone here
|
|
||
| ```rust | ||
| use dep::aztec::history::deployment::assert_contract_was_initialized_by; | ||
| use dep::aztec::oracle::get_contract_instance::get_contract_instance; |
There was a problem hiding this comment.
Just updating some docs
|
|
||
| The private initialization nullifier is no longer derived from just the contract address. It is now computed as a Poseidon2 hash of `[address, init_hash]` using a dedicated domain separator. This prevents observers from determining whether a fully private contract has been initialized by simply knowing its address. | ||
|
|
||
| Note that `Wallet.getContractMetadata` now returns `isContractInitialized: false` when the wallet does not have the contract instance registered, since `init_hash` is needed to compute the nullifier. Previously, this check worked for any address. |
There was a problem hiding this comment.
Shouldn't it instead fail, as it can't know?
There was a problem hiding this comment.
I think that we shouldn't fail, getContractMetadata can already be called when the walled doesn't have the contract instance registered, and it says so in the return type (by returning instance: undefined).
I think we should change it so that now we have this too isContractInitialized : boolean | undefined, and explain correctly on the js docs. Or maybe we could use something different like isContractInitialized: boolean | 'unknown' to be clearer. I'm don't like any of my suggestions, but I don't think we should fail
noir-projects/aztec-nr/aztec/src/macros/functions/initialization_utils.nr
Outdated
Show resolved
Hide resolved
| const parentAddress = await mockContractInstance(ParentContractArtifact); | ||
| const childAddress = await mockContractInstance(ChildContractArtifact); |
There was a problem hiding this comment.
In this test, the parent's entry point does a nested call to Child. Both contracts have initializers, so they will both call assert_is_initialized_private during private execution. And the thing is that that the oracle validates instance.to_address() == address, which fails with random addresses
So we create mocks, and use their addresses instead
|
Feel free to request my review again once you address Nico's feedback 👍 |
…rivate-init-nullifier-privacy-fix # Conflicts: # docs/docs-developers/docs/resources/migration_notes.md # noir-projects/noir-contracts/contracts/test/test_contract/src/test/deployment_proofs.nr
…rivate-init-nullifier-privacy-fix # Conflicts: # yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts
|
❌ Failed to cherry-pick to |
BEGIN_COMMIT_OVERRIDE fix(stdlib): accept null return_type for void Noir functions (#21647) feat!: make AES128 decrypt oracle return Option (#21696) fix(aztec-nr): fix OOB index with nonzero offset (#21613) feat!: include init_hash in private initialization nullifier to prevent privacy leak (#21427) END_COMMIT_OVERRIDE
Resolved conflicts in: - migration_notes.md: kept only init_hash migration note (dropped unrelated next-only notes) - deployment_proofs.nr: included new wrong-init-hash test and updated error message - constants_tests.nr: added DOM_SEP__PRIVATE_INITIALIZATION_NULLIFIER import, updated tester size - private_execution.test.ts: merged imports from both sides
Resolved conflicts in: - migration_notes.md: kept only init_hash migration note - constants_tests.nr: added DOM_SEP__PRIVATE_INITIALIZATION_NULLIFIER import, tester size <51, 45> - private_execution.test.ts: merged imports from both sides
Resolved conflicts in: - migration_notes.md: kept only init_hash migration note - constants_tests.nr: added DOM_SEP__PRIVATE_INITIALIZATION_NULLIFIER import, tester size <51, 45> - private_execution.test.ts: merged imports from both sides
4 participants
Problem
The private initialization nullifier was computed as just
address.to_field(). Anyone who knows a contract's address can compute this nullifier and check for its existence in the nullifier tree, revealing whether the contract has been initialized. This is a privacy leak for fully private contracts.Fix
The private initialization nullifier is now computed as
poseidon2_hash(address, init_hash)with a dedicated domain separator (DOM_SEP__PRIVATE_INITIALIZATION_NULLIFIER). Sinceinit_hashis not publicly available for fully private contracts, address knowledge alone is no longer sufficient to determine initialization status.Fixes F-194
Fixes #17128