diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 9a1c5d9ae91..c4d27cbd63c 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -523,17 +523,13 @@ fn handle_nullifier_exists( destinations: &Vec, inputs: &Vec, ) { - 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()); + 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()); } 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"), @@ -545,9 +541,6 @@ 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 d7a0c5e3dc9..062b21a1ac5 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, unsiloed_nullifier: Field, address: AztecAddress) -> bool { - nullifier_exists(unsiloed_nullifier, address.to_field()) == 1 + fn nullifier_exists(self, nullifier: Field) -> bool { + nullifier_exists(nullifier) == 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, address: Field) -> u8 {} +fn nullifier_exists(nullifier: 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 9e9f38ac614..593c5b16cf3 100644 --- a/noir-projects/aztec-nr/aztec/src/context/interface.nr +++ b/noir-projects/aztec-nr/aztec/src/context/interface.nr @@ -49,6 +49,6 @@ trait PublicContextInterface { contract_address: AztecAddress, function_selector: FunctionSelector, args: [Field; ARGS_COUNT] - ) -> FunctionReturns; - fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool; + ) -> [Field; RETURN_VALUES_LENGTH]; + fn nullifier_exists(self, nullifier: Field) -> 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 deb49d73be3..ae2e831417d 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -15,7 +15,7 @@ use dep::protocol_types::{ public_circuit_public_inputs::PublicCircuitPublicInputs, read_request::ReadRequest, side_effect::{SideEffect, SideEffectLinkedToNoteHash} }, - hash::silo_nullifier, address::{AztecAddress, EthAddress}, + 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, @@ -227,10 +227,8 @@ impl PublicContextInterface for PublicContext { self.inputs.public_global_variables.fee_recipient } - 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 nullifier_exists(self, nullifier: Field) -> bool { + nullifier_exists_oracle(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 f060fc14f1a..f0990a2a4f7 100644 --- a/noir-projects/aztec-nr/aztec/src/initializer.nr +++ b/noir-projects/aztec-nr/aztec/src/initializer.nr @@ -28,13 +28,15 @@ fn mark_as_initialized(context: &mut TContext) where TContext: Context } pub fn assert_is_initialized_public(context: &mut PublicContext) { - let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context.this_address()); - assert(context.nullifier_exists(init_nullifier, context.this_address()), "Not initialized"); + let init_nullifier = compute_contract_initialization_nullifier(context.this_address()); + assert(context.nullifier_exists(init_nullifier), "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, context.this_address()), "Not initialized"); + assert(context.nullifier_exists(init_nullifier), "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 da6e9a1b0f1..8632558bbfe 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 @@ -195,8 +195,10 @@ contract AvmTest { } #[aztec(public)] - fn assert_unsiloed_nullifier_acvm(unsiloed_nullifier: Field) { - assert(context.nullifier_exists(unsiloed_nullifier, context.this_address())); + 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)); } #[aztec(public-vm)] @@ -360,19 +362,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.this_address()) + context.nullifier_exists(nullifier) } #[aztec(public-vm)] fn assert_nullifier_exists(nullifier: Field) { - assert(context.nullifier_exists(nullifier, context.this_address())); + assert(context.nullifier_exists(nullifier)); } // 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, context.this_address()); + let exists = context.nullifier_exists(nullifier); 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 3e1112ff07b..da02fa82a60 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,15 +198,10 @@ 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, context.this_address())); + assert(context.nullifier_exists(nullifier)); } #[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 b82bb8153d9..00da02fb2d9 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,9 +222,10 @@ describe('e2e_inclusion_proofs_contract', () => { }); it('proves existence of a nullifier in public context', async () => { - 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(); + 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(); }); 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 5eac2f364f4..570d15e358a 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts @@ -167,13 +167,11 @@ 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, ); @@ -184,34 +182,30 @@ describe('Accrued Substate', () => { it('Should correctly show false when nullifier does not exist', async () => { const value = new Field(69n); const nullifierOffset = 0; - const addressOffset = 1; - const existsOffset = 2; + const existsOffset = 1; // 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); - context.machineState.memory.set(addressOffset, address); - await new NullifierExists(/*indirect=*/ 0, nullifierOffset, addressOffset, existsOffset).execute(context); + await new NullifierExists(/*indirect=*/ 0, nullifierOffset, 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(), storageAddress: address.toFr(), exists: false }), + expect.objectContaining({ nullifier: value.toFr(), exists: false }), ]); }); it('Should correctly show true when nullifier exists', async () => { const value = new Field(69n); const nullifierOffset = 0; - const addressOffset = 1; - const existsOffset = 2; + const existsOffset = 1; const storedLeafIndex = BigInt(42); // mock host storage this so that persistable state's checkNullifierExists returns true @@ -219,18 +213,16 @@ 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); - context.machineState.memory.set(addressOffset, address); - await new NullifierExists(/*indirect=*/ 0, nullifierOffset, addressOffset, existsOffset).execute(context); + await new NullifierExists(/*indirect=*/ 0, nullifierOffset, 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(), storageAddress: address.toFr(), exists: true }), + expect.objectContaining({ nullifier: value.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 7030fea54f6..c98d65ded62 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts @@ -80,31 +80,19 @@ 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, - OperandType.UINT32, - ]; + static readonly wireFormat = [OperandType.UINT8, OperandType.UINT8, OperandType.UINT32, OperandType.UINT32]; - constructor( - private indirect: number, - private nullifierOffset: number, - private addressOffset: number, - private existsOffset: number, - ) { + constructor(private indirect: number, private nullifierOffset: number, private existsOffset: number) { super(); } public async execute(context: AvmContext): Promise { - const memoryOperations = { reads: 2, writes: 1, indirect: this.indirect }; + const memoryOperations = { reads: 1, 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 address = memory.get(this.addressOffset).toFr(); - const exists = await context.persistableState.checkNullifierExists(address, nullifier); + const exists = await context.persistableState.checkNullifierExists(context.environment.storageAddress, 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 5b0a9ca7752..3d6e3669825 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 = pendingNullifiers.has(M[addressOffset], M[nullifierOffset]) || context.worldState.nullifiers.has( - hash(M[addressOffset], M[nullifierOffset]) +{`exists = context.worldState.nullifiers.has( + hash(context.environment.storageAddress, M[nullifierOffset]) ) M[existsOffset] = exists`} @@ -1464,12 +1464,11 @@ 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 = pendingNullifiers.has(M[addressOffset], M[nullifierOffset]) || context.worldState.nullifiers.has( - hash(M[addressOffset], M[nullifierOffset]) +{`exists = context.worldState.nullifiers.has( + hash(context.environment.storageAddress, M[nullifierOffset]) ) M[existsOffset] = exists`} @@ -1479,7 +1478,6 @@ M[existsOffset] = exists`} TracedNullifierCheck { callPointer: context.environment.callPointer, nullifier: M[nullifierOffset], - storageAddress: M[addressOffset], exists: exists, // defined above counter: ++context.worldStateAccessTrace.accessCounter, } @@ -1487,7 +1485,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**: 120 +- **Bit-size**: 88 ### `EMITNULLIFIER` diff --git a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js index ac5def0355e..5f1202d7079 100644 --- a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js +++ b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js @@ -912,12 +912,11 @@ 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 = pendingNullifiers.has(M[addressOffset], M[nullifierOffset]) || context.worldState.nullifiers.has( - hash(M[addressOffset], M[nullifierOffset]) +exists = context.worldState.nullifiers.has( + hash(context.environment.storageAddress, M[nullifierOffset]) ) M[existsOffset] = exists `, @@ -927,7 +926,6 @@ context.worldStateAccessTrace.nullifierChecks.append( TracedNullifierCheck { callPointer: context.environment.callPointer, nullifier: M[nullifierOffset], - storageAddress: M[addressOffset], exists: exists, // defined above counter: ++context.worldStateAccessTrace.accessCounter, }