From f202e1e65b4095ba5bd29a0a3e65bcf68f047459 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 10 Apr 2024 13:37:23 +0000 Subject: [PATCH] feat: change public nullifiers api --- avm-transpiler/src/transpile.rs | 11 ++++++++-- .../aztec-nr/aztec/src/context/avm_context.nr | 6 +++--- .../aztec-nr/aztec/src/context/interface.nr | 2 +- .../aztec/src/context/public_context.nr | 8 +++++--- .../aztec-nr/aztec/src/initializer.nr | 8 +++----- .../contracts/avm_test_contract/src/main.nr | 12 +++++------ .../inclusion_proofs_contract/src/main.nr | 7 ++++++- .../src/e2e_inclusion_proofs_contract.test.ts | 7 +++---- .../src/avm/opcodes/accrued_substate.test.ts | 20 +++++++++++++------ .../src/avm/opcodes/accrued_substate.ts | 20 +++++++++++++++---- .../docs/public-vm/gen/_instruction-set.mdx | 12 ++++++----- .../InstructionSet/InstructionSet.js | 6 ++++-- 12 files changed, 76 insertions(+), 43 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index c4d27cbd63c..9a1c5d9ae91 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -523,13 +523,17 @@ fn handle_nullifier_exists( destinations: &Vec, inputs: &Vec, ) { - if destinations.len() != 1 || inputs.len() != 1 { - panic!("Transpiler expects ForeignCall::CHECKNULLIFIEREXISTS to have 1 destinations and 1 input, got {} and {}", destinations.len(), inputs.len()); + if destinations.len() != 1 || inputs.len() != 2 { + panic!("Transpiler expects ForeignCall::CHECKNULLIFIEREXISTS to have 1 destinations and 2 inputs, got {} and {}", destinations.len(), inputs.len()); } let nullifier_offset_operand = match &inputs[0] { ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32, _ => panic!("Transpiler does not know how to handle ForeignCall::EMITNOTEHASH with HeapArray/Vector inputs"), }; + let address_offset_operand = match &inputs[1] { + ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32, + _ => panic!("Transpiler does not know how to handle ForeignCall::EMITNOTEHASH with HeapArray/Vector inputs"), + }; let exists_offset_operand = match &destinations[0] { ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32, _ => panic!("Transpiler does not know how to handle ForeignCall::EMITNOTEHASH with HeapArray/Vector inputs"), @@ -541,6 +545,9 @@ fn handle_nullifier_exists( AvmOperand::U32 { value: nullifier_offset_operand, }, + AvmOperand::U32 { + value: address_offset_operand, + }, AvmOperand::U32 { value: exists_offset_operand, }, diff --git a/noir-projects/aztec-nr/aztec/src/context/avm_context.nr b/noir-projects/aztec-nr/aztec/src/context/avm_context.nr index 7bb2872e6c0..8231a82519c 100644 --- a/noir-projects/aztec-nr/aztec/src/context/avm_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/avm_context.nr @@ -103,8 +103,8 @@ impl PublicContextInterface for AvmContext { AztecAddress::zero() } - fn nullifier_exists(self, nullifier: Field) -> bool { - nullifier_exists(nullifier) == 1 + fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool { + nullifier_exists(unsiloed_nullifier, address.to_field()) == 1 } fn push_nullifier_read_request(&mut self, nullifier: Field) { @@ -278,7 +278,7 @@ fn note_hash_exists(note_hash: Field, leaf_index: Field) -> u8 {} fn emit_note_hash(note_hash: Field) {} #[oracle(avmOpcodeNullifierExists)] -fn nullifier_exists(nullifier: Field) -> u8 {} +fn nullifier_exists(nullifier: Field, address: Field) -> u8 {} #[oracle(avmOpcodeEmitNullifier)] fn emit_nullifier(nullifier: Field) {} diff --git a/noir-projects/aztec-nr/aztec/src/context/interface.nr b/noir-projects/aztec-nr/aztec/src/context/interface.nr index 1a54cddf4ef..a2a3d2e5f24 100644 --- a/noir-projects/aztec-nr/aztec/src/context/interface.nr +++ b/noir-projects/aztec-nr/aztec/src/context/interface.nr @@ -52,5 +52,5 @@ trait PublicContextInterface { function_selector: FunctionSelector, args: [Field; ARGS_COUNT] ) -> [Field; RETURN_VALUES_LENGTH]; - fn nullifier_exists(self, nullifier: Field) -> bool; + fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool; } diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index c41d38986b3..cd05c1d3b3d 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -14,7 +14,7 @@ use dep::protocol_types::{ public_circuit_public_inputs::PublicCircuitPublicInputs, read_request::ReadRequest, side_effect::{SideEffect, SideEffectLinkedToNoteHash} }, - address::{AztecAddress, EthAddress}, + hash::silo_nullifier, address::{AztecAddress, EthAddress}, constants::{ MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_DATA_READS_PER_CALL, @@ -220,8 +220,10 @@ impl PublicContextInterface for PublicContext { self.inputs.public_global_variables.fee_recipient } - fn nullifier_exists(self, nullifier: Field) -> bool { - nullifier_exists_oracle(nullifier) == 1 + fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool { + // Current public can only check for settled nullifiers, so we always silo. + let siloed_nullifier = silo_nullifier(address, unsiloed_nullifier); + nullifier_exists_oracle(siloed_nullifier) == 1 } fn push_nullifier_read_request(&mut self, nullifier: Field) { diff --git a/noir-projects/aztec-nr/aztec/src/initializer.nr b/noir-projects/aztec-nr/aztec/src/initializer.nr index f0990a2a4f7..f060fc14f1a 100644 --- a/noir-projects/aztec-nr/aztec/src/initializer.nr +++ b/noir-projects/aztec-nr/aztec/src/initializer.nr @@ -28,15 +28,13 @@ fn mark_as_initialized(context: &mut TContext) where TContext: Context } pub fn assert_is_initialized_public(context: &mut PublicContext) { - let init_nullifier = compute_contract_initialization_nullifier(context.this_address()); - assert(context.nullifier_exists(init_nullifier), "Not initialized"); + let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context.this_address()); + assert(context.nullifier_exists(init_nullifier, context.this_address()), "Not initialized"); } pub fn assert_is_initialized_avm(context: &mut AvmContext) { - // WARNING: the AVM always expects UNSILOED nullifiers! - // TODO(fcarreiro@): Change current private/public to take unsiloed nullifiers and an address. let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context.this_address()); - assert(context.nullifier_exists(init_nullifier), "Not initialized"); + assert(context.nullifier_exists(init_nullifier, context.this_address()), "Not initialized"); } pub fn assert_is_initialized_private(context: &mut PrivateContext) { diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index 1645be3c553..c929cff681d 100644 --- a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr @@ -196,10 +196,8 @@ contract AvmTest { } #[aztec(public)] - fn assert_unsiloed_nullifier_acvm(nullifier: Field) { - // ACVM requires siloed nullifier. - let siloed_nullifier = silo_nullifier(context.this_address(), nullifier); - assert(context.nullifier_exists(siloed_nullifier)); + fn assert_unsiloed_nullifier_acvm(unsiloed_nullifier: Field) { + assert(context.nullifier_exists(unsiloed_nullifier, context.this_address())); } #[aztec(public-vm)] @@ -365,19 +363,19 @@ contract AvmTest { // Use the standard context interface to check for a nullifier #[aztec(public-vm)] fn nullifier_exists(nullifier: Field) -> pub bool { - context.nullifier_exists(nullifier) + context.nullifier_exists(nullifier, context.this_address()) } #[aztec(public-vm)] fn assert_nullifier_exists(nullifier: Field) { - assert(context.nullifier_exists(nullifier)); + assert(context.nullifier_exists(nullifier, context.this_address())); } // Use the standard context interface to emit a new nullifier #[aztec(public-vm)] fn emit_nullifier_and_check(nullifier: Field) { context.push_new_nullifier(nullifier, 0); - let exists = context.nullifier_exists(nullifier); + let exists = context.nullifier_exists(nullifier, context.this_address()); assert(exists, "Nullifier was just created, but its existence wasn't detected!"); } diff --git a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr index da02fa82a60..3e1112ff07b 100644 --- a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr @@ -198,10 +198,15 @@ contract InclusionProofs { } } + #[aztec(public)] + fn push_nullifier_public(nullifier: Field) { + context.push_new_nullifier(nullifier, 0); + } + // Proves nullifier existed at latest block #[aztec(public)] fn test_nullifier_inclusion_from_public(nullifier: Field) { - assert(context.nullifier_exists(nullifier)); + assert(context.nullifier_exists(nullifier, context.this_address())); } #[aztec(private)] diff --git a/yarn-project/end-to-end/src/e2e_inclusion_proofs_contract.test.ts b/yarn-project/end-to-end/src/e2e_inclusion_proofs_contract.test.ts index 00da02fb2d9..b82bb8153d9 100644 --- a/yarn-project/end-to-end/src/e2e_inclusion_proofs_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_inclusion_proofs_contract.test.ts @@ -222,10 +222,9 @@ describe('e2e_inclusion_proofs_contract', () => { }); it('proves existence of a nullifier in public context', async () => { - const block = await pxe.getBlock(deploymentBlockNumber); - const nullifier = block?.body.txEffects[0].nullifiers[0]; - - await contract.methods.test_nullifier_inclusion_from_public(nullifier!).send().wait(); + const unsiloedNullifier = new Fr(123456789n); + await contract.methods.push_nullifier_public(unsiloedNullifier).send().wait(); + await contract.methods.test_nullifier_inclusion_from_public(unsiloedNullifier).send().wait(); }); it('nullifier existence failure case', async () => { diff --git a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts index 570d15e358a..5eac2f364f4 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts @@ -167,11 +167,13 @@ describe('Accrued Substate', () => { NullifierExists.opcode, // opcode 0x01, // indirect ...Buffer.from('12345678', 'hex'), // nullifierOffset + ...Buffer.from('02345678', 'hex'), // addressOffset ...Buffer.from('456789AB', 'hex'), // existsOffset ]); const inst = new NullifierExists( /*indirect=*/ 0x01, /*nullifierOffset=*/ 0x12345678, + /*addressOffset=*/ 0x02345678, /*existsOffset=*/ 0x456789ab, ); @@ -182,30 +184,34 @@ describe('Accrued Substate', () => { it('Should correctly show false when nullifier does not exist', async () => { const value = new Field(69n); const nullifierOffset = 0; - const existsOffset = 1; + const addressOffset = 1; + const existsOffset = 2; // mock host storage this so that persistable state's checkNullifierExists returns UNDEFINED const commitmentsDb = mock(); commitmentsDb.getNullifierIndex.mockResolvedValue(Promise.resolve(undefined)); const hostStorage = initHostStorage({ commitmentsDb }); context = initContext({ persistableState: new AvmPersistableStateManager(hostStorage) }); + const address = new Field(context.environment.storageAddress.toField()); context.machineState.memory.set(nullifierOffset, value); - await new NullifierExists(/*indirect=*/ 0, nullifierOffset, existsOffset).execute(context); + context.machineState.memory.set(addressOffset, address); + await new NullifierExists(/*indirect=*/ 0, nullifierOffset, addressOffset, existsOffset).execute(context); const exists = context.machineState.memory.getAs(existsOffset); expect(exists).toEqual(new Uint8(0)); const journalState = context.persistableState.flush(); expect(journalState.nullifierChecks).toEqual([ - expect.objectContaining({ nullifier: value.toFr(), exists: false }), + expect.objectContaining({ nullifier: value.toFr(), storageAddress: address.toFr(), exists: false }), ]); }); it('Should correctly show true when nullifier exists', async () => { const value = new Field(69n); const nullifierOffset = 0; - const existsOffset = 1; + const addressOffset = 1; + const existsOffset = 2; const storedLeafIndex = BigInt(42); // mock host storage this so that persistable state's checkNullifierExists returns true @@ -213,16 +219,18 @@ describe('Accrued Substate', () => { commitmentsDb.getNullifierIndex.mockResolvedValue(Promise.resolve(storedLeafIndex)); const hostStorage = initHostStorage({ commitmentsDb }); context = initContext({ persistableState: new AvmPersistableStateManager(hostStorage) }); + const address = new Field(context.environment.storageAddress.toField()); context.machineState.memory.set(nullifierOffset, value); - await new NullifierExists(/*indirect=*/ 0, nullifierOffset, existsOffset).execute(context); + context.machineState.memory.set(addressOffset, address); + await new NullifierExists(/*indirect=*/ 0, nullifierOffset, addressOffset, existsOffset).execute(context); const exists = context.machineState.memory.getAs(existsOffset); expect(exists).toEqual(new Uint8(1)); const journalState = context.persistableState.flush(); expect(journalState.nullifierChecks).toEqual([ - expect.objectContaining({ nullifier: value.toFr(), exists: true }), + expect.objectContaining({ nullifier: value.toFr(), storageAddress: address.toFr(), exists: true }), ]); }); }); diff --git a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts index c98d65ded62..7030fea54f6 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts @@ -80,19 +80,31 @@ export class NullifierExists extends Instruction { static type: string = 'NULLIFIEREXISTS'; static readonly opcode: Opcode = Opcode.NULLIFIEREXISTS; // Informs (de)serialization. See Instruction.deserialize. - static readonly wireFormat = [OperandType.UINT8, OperandType.UINT8, OperandType.UINT32, OperandType.UINT32]; + static readonly wireFormat = [ + OperandType.UINT8, + OperandType.UINT8, + OperandType.UINT32, + OperandType.UINT32, + OperandType.UINT32, + ]; - constructor(private indirect: number, private nullifierOffset: number, private existsOffset: number) { + constructor( + private indirect: number, + private nullifierOffset: number, + private addressOffset: number, + private existsOffset: number, + ) { super(); } public async execute(context: AvmContext): Promise { - const memoryOperations = { reads: 1, writes: 1, indirect: this.indirect }; + const memoryOperations = { reads: 2, writes: 1, indirect: this.indirect }; const memory = context.machineState.memory.track(this.type); context.machineState.consumeGas(this.gasCost(memoryOperations)); const nullifier = memory.get(this.nullifierOffset).toFr(); - const exists = await context.persistableState.checkNullifierExists(context.environment.storageAddress, nullifier); + const address = memory.get(this.addressOffset).toFr(); + const exists = await context.persistableState.checkNullifierExists(address, nullifier); memory.set(this.existsOffset, exists ? new Uint8(1) : new Uint8(0)); diff --git a/yellow-paper/docs/public-vm/gen/_instruction-set.mdx b/yellow-paper/docs/public-vm/gen/_instruction-set.mdx index 3d6e3669825..5b0a9ca7752 100644 --- a/yellow-paper/docs/public-vm/gen/_instruction-set.mdx +++ b/yellow-paper/docs/public-vm/gen/_instruction-set.mdx @@ -353,8 +353,8 @@ M[existsOffset] = exists`} 0x2f [`NULLIFIEREXISTS`](#isa-section-nullifierexists) Check whether a nullifier exists in the nullifier tree (including nullifiers from earlier in the current transaction or from earlier in the current block) -{`exists = context.worldState.nullifiers.has( - hash(context.environment.storageAddress, M[nullifierOffset]) +{`exists = pendingNullifiers.has(M[addressOffset], M[nullifierOffset]) || context.worldState.nullifiers.has( + hash(M[addressOffset], M[nullifierOffset]) ) M[existsOffset] = exists`} @@ -1464,11 +1464,12 @@ Check whether a nullifier exists in the nullifier tree (including nullifiers fro - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - **Args**: - **nullifierOffset**: memory offset of the unsiloed nullifier + - **addressOffset**: memory offset of the storage address - **existsOffset**: memory offset specifying where to store operation's result (whether the nullifier exists) - **Expression**: -{`exists = context.worldState.nullifiers.has( - hash(context.environment.storageAddress, M[nullifierOffset]) +{`exists = pendingNullifiers.has(M[addressOffset], M[nullifierOffset]) || context.worldState.nullifiers.has( + hash(M[addressOffset], M[nullifierOffset]) ) M[existsOffset] = exists`} @@ -1478,6 +1479,7 @@ M[existsOffset] = exists`} TracedNullifierCheck { callPointer: context.environment.callPointer, nullifier: M[nullifierOffset], + storageAddress: M[addressOffset], exists: exists, // defined above counter: ++context.worldStateAccessTrace.accessCounter, } @@ -1485,7 +1487,7 @@ M[existsOffset] = exists`} - **Triggers downstream circuit operations**: Nullifier siloing (hash with storage contract address), nullifier tree membership check - **Tag updates**: `T[existsOffset] = u8` -- **Bit-size**: 88 +- **Bit-size**: 120 ### `EMITNULLIFIER` diff --git a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js index 5f1202d7079..ac5def0355e 100644 --- a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js +++ b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js @@ -912,11 +912,12 @@ context.worldStateAccessTrace.newNoteHashes.append( ], "Args": [ {"name": "nullifierOffset", "description": "memory offset of the unsiloed nullifier"}, + {"name": "addressOffset", "description": "memory offset of the storage address"}, {"name": "existsOffset", "description": "memory offset specifying where to store operation's result (whether the nullifier exists)"}, ], "Expression": ` -exists = context.worldState.nullifiers.has( - hash(context.environment.storageAddress, M[nullifierOffset]) +exists = pendingNullifiers.has(M[addressOffset], M[nullifierOffset]) || context.worldState.nullifiers.has( + hash(M[addressOffset], M[nullifierOffset]) ) M[existsOffset] = exists `, @@ -926,6 +927,7 @@ context.worldStateAccessTrace.nullifierChecks.append( TracedNullifierCheck { callPointer: context.environment.callPointer, nullifier: M[nullifierOffset], + storageAddress: M[addressOffset], exists: exists, // defined above counter: ++context.worldStateAccessTrace.accessCounter, }