diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index c1123799dba9..7692615f73c8 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -890,38 +890,35 @@ fn handle_black_box_function(avm_instrs: &mut Vec, operation: &B ), } } + /// Emit a storage write opcode -/// The current implementation writes an array of values into storage ( contiguous slots in memory ) fn handle_storage_write( avm_instrs: &mut Vec, destinations: &Vec, inputs: &Vec, ) { assert!(inputs.len() == 2); - assert!(destinations.len() == 1); + assert!(destinations.is_empty()); let slot_offset_maybe = inputs[0]; let slot_offset = match slot_offset_maybe { ValueOrArray::MemoryAddress(slot_offset) => slot_offset.0, - _ => panic!("ForeignCall address destination should be a single value"), + _ => panic!("Storage write address destination should be a single value"), }; let src_offset_maybe = inputs[1]; - let (src_offset, src_size) = match src_offset_maybe { - ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size), - _ => panic!("Storage write address inputs should be an array of values"), + let src_offset = match src_offset_maybe { + ValueOrArray::MemoryAddress(offset) => offset.0, + _ => panic!("Storage write address inputs should be a single value"), }; avm_instrs.push(AvmInstruction { opcode: AvmOpcode::SSTORE, - indirect: Some(ZEROTH_OPERAND_INDIRECT), + indirect: Some(ALL_DIRECT), operands: vec![ AvmOperand::U32 { value: src_offset as u32, }, - AvmOperand::U32 { - value: src_size as u32, - }, AvmOperand::U32 { value: slot_offset as u32, }, @@ -937,32 +934,28 @@ fn handle_storage_read( destinations: &Vec, inputs: &Vec, ) { - // For the foreign calls we want to handle, we do not want inputs, as they are getters - assert!(inputs.len() == 2); // output, len - but we dont use this len - its for the oracle + assert!(inputs.len() == 1); assert!(destinations.len() == 1); let slot_offset_maybe = inputs[0]; let slot_offset = match slot_offset_maybe { ValueOrArray::MemoryAddress(slot_offset) => slot_offset.0, - _ => panic!("ForeignCall address destination should be a single value"), + _ => panic!("Storage read address destination should be a single value"), }; let dest_offset_maybe = destinations[0]; - let (dest_offset, src_size) = match dest_offset_maybe { - ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size), - _ => panic!("Storage write address inputs should be an array of values"), + let dest_offset = match dest_offset_maybe { + ValueOrArray::MemoryAddress(slot_offset) => slot_offset.0, + _ => panic!("Storage read address inputs should be a single value"), }; avm_instrs.push(AvmInstruction { opcode: AvmOpcode::SLOAD, - indirect: Some(SECOND_OPERAND_INDIRECT), + indirect: Some(ALL_DIRECT), operands: vec![ AvmOperand::U32 { value: slot_offset as u32, }, - AvmOperand::U32 { - value: src_size as u32, - }, AvmOperand::U32 { value: dest_offset as u32, }, diff --git a/noir-projects/aztec-nr/aztec/src/oracle/storage.nr b/noir-projects/aztec-nr/aztec/src/oracle/storage.nr index 72bec83be3a3..7655887a0ccc 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/storage.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/storage.nr @@ -1,19 +1,29 @@ use dep::protocol_types::traits::{Deserialize, Serialize}; #[oracle(storageRead)] -fn storage_read_oracle(_storage_slot: Field, _number_of_elements: Field) -> [Field; N] {} +fn storage_read_oracle(_storage_slot: Field) -> Field {} -unconstrained fn storage_read_oracle_wrapper(_storage_slot: Field) -> [Field; N] { - storage_read_oracle(_storage_slot, N) +unconstrained fn storage_read_oracle_wrapper(storage_slot: Field) -> Field { + storage_read_oracle(storage_slot) } pub fn storage_read(storage_slot: Field) -> [Field; N] { - storage_read_oracle_wrapper(storage_slot) + let mut ret: [Field; N] = [0; N]; + for i in 0..N { + ret[i] = storage_read_oracle_wrapper(storage_slot + (i as Field)); + } + ret } #[oracle(storageWrite)] -fn storage_write_oracle(_storage_slot: Field, _values: [Field; N]) -> [Field; N] {} +fn storage_write_oracle(_storage_slot: Field, _value: Field) {} -unconstrained pub fn storage_write(storage_slot: Field, fields: [Field; N]) { - let _hash = storage_write_oracle(storage_slot, fields); +unconstrained fn storage_write_oracle_wrapper(storage_slot: Field, value: Field) { + storage_write_oracle(storage_slot, value); +} + +pub fn storage_write(storage_slot: Field, fields: [Field; N]) { + for i in 0..N { + storage_write_oracle_wrapper(storage_slot + (i as Field), fields[i]); + } } 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 eb1feef85896..4997332f52aa 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 @@ -1,4 +1,39 @@ +use dep::aztec::protocol_types::traits::{Serialize, Deserialize}; + +// struct TokenNote { +// amount: Field, +// owner: AztecAddress, +// } + +// impl Serialize for TokenNote { +// fn serialize(&self) -> Vec { +// let mut serialized = Vec::new(); +// serialized.push(self.amount); +// serialized.push(self.owner.serialize()); +// serialized +// } +// } + +struct Note { + a: Field, + b: Field, +} + +impl Serialize<2> for Note { + fn serialize(self) -> [Field; 2] { + [self.a, self.b] + } +} + +impl Deserialize<2> for Note { + fn deserialize(wire: [Field; 2]) -> Note { + Note {a: wire[0], b: wire[1]} + } +} + contract AvmTest { + use crate::Note; + // Libs use dep::aztec::state_vars::PublicMutable; use dep::aztec::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH}; @@ -12,18 +47,35 @@ contract AvmTest { fn constructor() {} struct Storage { - owner: PublicMutable + single: PublicMutable, + list: PublicMutable, + } + + #[aztec(public-vm)] + fn setStorageSingle() { + storage.single.write(context.sender()); + } + + #[aztec(public-vm)] + fn readStorageSingle() -> pub AztecAddress { + storage.single.read() + } + + #[aztec(public-vm)] + fn setReadStorageSingle() -> pub AztecAddress { + storage.single.write(context.sender()); + storage.single.read() } #[aztec(public-vm)] - fn setAdmin() { - storage.owner.write(context.sender()); + fn setStorageList(a: Field, b: Field) { + storage.list.write(Note { a, b }); } #[aztec(public-vm)] - fn setAndRead() -> pub AztecAddress { - storage.owner.write(context.sender()); - storage.owner.read() + fn readStorageList() -> pub [Field; 2] { + let note: Note = storage.list.read(); + [note.a, note.b] } #[aztec(public-vm)] diff --git a/yarn-project/simulator/src/acvm/oracle/oracle.ts b/yarn-project/simulator/src/acvm/oracle/oracle.ts index 48824adcae4b..326b373219d8 100644 --- a/yarn-project/simulator/src/acvm/oracle/oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/oracle.ts @@ -248,11 +248,13 @@ export class Oracle { return toACVMField(portalContactAddress); } + // FIXME: change to match new oracle format async storageRead([startStorageSlot]: ACVMField[], [numberOfElements]: ACVMField[]): Promise { const values = await this.typedOracle.storageRead(fromACVMField(startStorageSlot), +numberOfElements); return values.map(toACVMField); } + // FIXME: change to match new oracle format async storageWrite([startStorageSlot]: ACVMField[], values: ACVMField[]): Promise { const newValues = await this.typedOracle.storageWrite(fromACVMField(startStorageSlot), values.map(fromACVMField)); return newValues.map(toACVMField); diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 2f57c0ff5145..95471bb6fd07 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -439,7 +439,7 @@ describe('AVM simulator', () => { it(`Should execute contract function that makes a nested static call which modifies storage`, async () => { const callBytecode = getAvmTestContractBytecode('avm_raw_nested_static_call_to_set_admin'); - const nestedBytecode = getAvmTestContractBytecode('avm_setAdmin'); + const nestedBytecode = getAvmTestContractBytecode('avm_setStorageSingle'); const context = initContext(); jest .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode') @@ -468,7 +468,7 @@ describe('AVM simulator', () => { it(`Should execute contract function that makes a nested static call which modifies storage (old interface)`, async () => { const callBytecode = getAvmTestContractBytecode('avm_nested_static_call_to_set_admin'); - const nestedBytecode = getAvmTestContractBytecode('avm_setAdmin'); + const nestedBytecode = getAvmTestContractBytecode('avm_setStorageSingle'); const context = initContext(); jest .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode') @@ -490,7 +490,7 @@ describe('AVM simulator', () => { const context = initContext({ env: initExecutionEnvironment({ sender, address, storageAddress: address }), }); - const bytecode = getAvmTestContractBytecode('avm_setAdmin'); + const bytecode = getAvmTestContractBytecode('avm_setStorageSingle'); const results = await new AvmSimulator(context).executeBytecode(bytecode); // Get contract function artifact @@ -509,7 +509,7 @@ describe('AVM simulator', () => { expect(slotTrace).toEqual([sender.toField()]); }); - it('Should read a value from storage', async () => { + it('Should set and read a value from storage', async () => { // We are setting the owner const sender = AztecAddress.fromField(new Fr(1)); const address = AztecAddress.fromField(new Fr(420)); @@ -517,7 +517,7 @@ describe('AVM simulator', () => { const context = initContext({ env: initExecutionEnvironment({ sender, address, storageAddress: address }), }); - const bytecode = getAvmTestContractBytecode('avm_setAndRead'); + const bytecode = getAvmTestContractBytecode('avm_setReadStorageSingle'); const results = await new AvmSimulator(context).executeBytecode(bytecode); expect(results.reverted).toBe(false); @@ -536,6 +536,31 @@ describe('AVM simulator', () => { const slotWriteTrace = storageWriteTrace.get(1n); expect(slotWriteTrace).toEqual([sender.toField()]); }); + + it('Should set multiple values in storage', async () => { + const slot = 2n; + const sender = AztecAddress.fromField(new Fr(1)); + const address = AztecAddress.fromField(new Fr(420)); + const calldata = [new Fr(1), new Fr(2)]; + + const context = initContext({ + env: initExecutionEnvironment({ sender, address, calldata, storageAddress: address }), + }); + const bytecode = getAvmTestContractBytecode('avm_setStorageList'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + + const worldState = context.persistableState.flush(); + const storageSlot = worldState.currentStorageValue.get(address.toBigInt())!; + expect(storageSlot.get(slot)).toEqual(calldata[0]); + expect(storageSlot.get(slot + 1n)).toEqual(calldata[1]); + + // Tracing + const storageTrace = worldState.storageWrites.get(address.toBigInt())!; + expect(storageTrace.get(slot)).toEqual([calldata[0]]); + expect(storageTrace.get(slot + 1n)).toEqual([calldata[1]]); + }); }); }); }); diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts index 1df77f9d0a68..1851002ae81b 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -71,7 +71,7 @@ describe('External Calls', () => { const successOffset = 7; const otherContextInstructionsBytecode = encodeToBytecode([ new CalldataCopy(/*indirect=*/ 0, /*csOffset=*/ 0, /*copySize=*/ argsSize, /*dstOffset=*/ 0), - new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*size=*/ 1, /*slotOffset=*/ 0), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*slotOffset=*/ 0), new Return(/*indirect=*/ 0, /*retOffset=*/ 0, /*size=*/ 2), ]); @@ -163,7 +163,7 @@ describe('External Calls', () => { const otherContextInstructions: Instruction[] = [ new CalldataCopy(/*indirect=*/ 0, /*csOffset=*/ 0, /*copySize=*/ argsSize, /*dstOffset=*/ 0), - new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*size=*/ 1, /*slotOffset=*/ 0), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*slotOffset=*/ 0), ]; const otherContextInstructionsBytecode = encodeToBytecode(otherContextInstructions); diff --git a/yarn-project/simulator/src/avm/opcodes/storage.test.ts b/yarn-project/simulator/src/avm/opcodes/storage.test.ts index ab98f3e7140a..e9eb0743d203 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.test.ts @@ -28,15 +28,9 @@ describe('Storage Instructions', () => { SStore.opcode, // opcode 0x01, // indirect ...Buffer.from('12345678', 'hex'), // srcOffset - ...Buffer.from('a2345678', 'hex'), // size ...Buffer.from('3456789a', 'hex'), // slotOffset ]); - const inst = new SStore( - /*indirect=*/ 0x01, - /*srcOffset=*/ 0x12345678, - /*size=*/ 0xa2345678, - /*slotOffset=*/ 0x3456789a, - ); + const inst = new SStore(/*indirect=*/ 0x01, /*srcOffset=*/ 0x12345678, /*slotOffset=*/ 0x3456789a); expect(SStore.deserialize(buf)).toEqual(inst); expect(inst.serialize()).toEqual(buf); @@ -49,7 +43,7 @@ describe('Storage Instructions', () => { context.machineState.memory.set(0, a); context.machineState.memory.set(1, b); - await new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*size=*/ 1, /*slotOffset=*/ 0).execute(context); + await new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*slotOffset=*/ 0).execute(context); expect(journal.writeStorage).toHaveBeenCalledWith(address, new Fr(a.toBigInt()), new Fr(b.toBigInt())); }); @@ -66,8 +60,7 @@ describe('Storage Instructions', () => { context.machineState.memory.set(0, a); context.machineState.memory.set(1, b); - const instruction = () => - new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*size=*/ 1, /*slotOffset=*/ 1).execute(context); + const instruction = () => new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*slotOffset=*/ 1).execute(context); await expect(instruction()).rejects.toThrow(StaticCallStorageAlterError); }); }); @@ -78,15 +71,9 @@ describe('Storage Instructions', () => { SLoad.opcode, // opcode 0x01, // indirect ...Buffer.from('12345678', 'hex'), // slotOffset - ...Buffer.from('a2345678', 'hex'), // size ...Buffer.from('3456789a', 'hex'), // dstOffset ]); - const inst = new SLoad( - /*indirect=*/ 0x01, - /*slotOffset=*/ 0x12345678, - /*size=*/ 0xa2345678, - /*dstOffset=*/ 0x3456789a, - ); + const inst = new SLoad(/*indirect=*/ 0x01, /*slotOffset=*/ 0x12345678, /*dstOffset=*/ 0x3456789a); expect(SLoad.deserialize(buf)).toEqual(inst); expect(inst.serialize()).toEqual(buf); @@ -103,7 +90,7 @@ describe('Storage Instructions', () => { context.machineState.memory.set(0, a); context.machineState.memory.set(1, b); - await new SLoad(/*indirect=*/ 0, /*slotOffset=*/ 0, /*size=*/ 1, /*dstOffset=*/ 1).execute(context); + await new SLoad(/*indirect=*/ 0, /*slotOffset=*/ 0, /*dstOffset=*/ 1).execute(context); expect(journal.readStorage).toHaveBeenCalledWith(address, new Fr(a.toBigInt())); diff --git a/yarn-project/simulator/src/avm/opcodes/storage.ts b/yarn-project/simulator/src/avm/opcodes/storage.ts index 3ca3bfa19aaf..a232bc8883a8 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.ts @@ -1,5 +1,3 @@ -import { Fr } from '@aztec/foundation/fields'; - import type { AvmContext } from '../avm_context.js'; import { Field } from '../avm_memory_types.js'; import { InstructionExecutionError } from '../errors.js'; @@ -14,15 +12,9 @@ abstract class BaseStorageInstruction extends Instruction { OperandType.UINT8, OperandType.UINT32, OperandType.UINT32, - OperandType.UINT32, ]; - constructor( - protected indirect: number, - protected aOffset: number, - protected /*temporary*/ size: number, - protected bOffset: number, - ) { + constructor(protected indirect: number, protected aOffset: number, protected bOffset: number) { super(); } } @@ -31,8 +23,8 @@ export class SStore extends BaseStorageInstruction { static readonly type: string = 'SSTORE'; static readonly opcode = Opcode.SSTORE; - constructor(indirect: number, srcOffset: number, /*temporary*/ srcSize: number, slotOffset: number) { - super(indirect, srcOffset, srcSize, slotOffset); + constructor(indirect: number, srcOffset: number, slotOffset: number) { + super(indirect, srcOffset, slotOffset); } async execute(context: AvmContext): Promise { @@ -46,12 +38,9 @@ export class SStore extends BaseStorageInstruction { ); const slot = context.machineState.memory.get(slotOffset).toFr(); - const data = context.machineState.memory.getSlice(srcOffset, this.size).map(field => field.toFr()); + const data = context.machineState.memory.get(srcOffset).toFr(); - for (const [index, value] of Object.entries(data)) { - const adjustedSlot = slot.add(new Fr(BigInt(index))); - context.persistableState.writeStorage(context.environment.storageAddress, adjustedSlot, value); - } + context.persistableState.writeStorage(context.environment.storageAddress, slot, data); context.machineState.incrementPc(); } @@ -61,27 +50,19 @@ export class SLoad extends BaseStorageInstruction { static readonly type: string = 'SLOAD'; static readonly opcode = Opcode.SLOAD; - constructor(indirect: number, slotOffset: number, size: number, dstOffset: number) { - super(indirect, slotOffset, size, dstOffset); + constructor(indirect: number, slotOffset: number, dstOffset: number) { + super(indirect, slotOffset, dstOffset); } async execute(context: AvmContext): Promise { - const [aOffset, size, bOffset] = Addressing.fromWire(this.indirect).resolve( - [this.aOffset, this.size, this.bOffset], + const [aOffset, bOffset] = Addressing.fromWire(this.indirect).resolve( + [this.aOffset, this.bOffset], context.machineState.memory, ); - const slot = context.machineState.memory.get(aOffset); - - // Write each read value from storage into memory - for (let i = 0; i < size; i++) { - const data: Fr = await context.persistableState.readStorage( - context.environment.storageAddress, - new Fr(slot.toBigInt() + BigInt(i)), - ); - - context.machineState.memory.set(bOffset + i, new Field(data)); - } + const slot = context.machineState.memory.get(aOffset).toFr(); + const data = await context.persistableState.readStorage(context.environment.storageAddress, slot); + context.machineState.memory.set(bOffset, new Field(data)); context.machineState.incrementPc(); }