From 9c5ab338c36af9c7b2a6e539a6843e9d2d515c13 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Mon, 11 Mar 2024 15:33:40 +0000 Subject: [PATCH] wip --- avm-transpiler/src/transpile.rs | 33 ++--- .../aztec-nr/aztec/src/oracle/storage.nr | 24 +++- .../contracts/avm_test_contract/src/main.nr | 50 +++++++- .../__snapshots__/index.test.ts.snap | 10 +- yarn-project/simulator/src/acvm/acvm.ts | 5 +- .../simulator/src/acvm/oracle/oracle.ts | 11 +- .../simulator/src/acvm/oracle/typed_oracle.ts | 4 +- .../simulator/src/avm/avm_simulator.test.ts | 120 ++++++++++++++---- .../src/avm/opcodes/external_calls.test.ts | 4 +- .../simulator/src/avm/opcodes/storage.test.ts | 23 +--- .../simulator/src/avm/opcodes/storage.ts | 43 ++----- .../src/client/client_execution_context.ts | 24 ++-- .../simulator/src/client/view_data_oracle.ts | 17 +-- .../src/public/public_execution_context.ts | 41 ++---- 14 files changed, 234 insertions(+), 175 deletions(-) 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..76ac08e2e335 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,25 @@ +use dep::aztec::protocol_types::traits::{Serialize, Deserialize}; + +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 +33,35 @@ contract AvmTest { fn constructor() {} struct Storage { - owner: PublicMutable + single: PublicMutable, + list: PublicMutable, + } + + #[aztec(public-vm)] + fn setStorageSingle(a: Field) { + storage.single.write(a); + } + + #[aztec(public-vm)] + fn readStorageSingle() -> pub Field { + storage.single.read() + } + + #[aztec(public-vm)] + fn setReadStorageSingle(a: Field) -> pub Field { + storage.single.write(a); + 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.serialize() } #[aztec(public-vm)] diff --git a/yarn-project/protocol-contracts/src/gas-token/__snapshots__/index.test.ts.snap b/yarn-project/protocol-contracts/src/gas-token/__snapshots__/index.test.ts.snap index 387a8dcf251b..2d96da587669 100644 --- a/yarn-project/protocol-contracts/src/gas-token/__snapshots__/index.test.ts.snap +++ b/yarn-project/protocol-contracts/src/gas-token/__snapshots__/index.test.ts.snap @@ -2,10 +2,10 @@ exports[`GasToken returns canonical protocol contract 1`] = ` { - "address": AztecAddress<0x01ffec73ac535628f98088b70f766f47989801a0dfc754cf4996f505cfd8f082>, + "address": AztecAddress<0x0b990cd3da37d6d83cd1dc2cca06ee3cae22877ec9040b817c9ce3ae47634683>, "instance": { - "address": AztecAddress<0x01ffec73ac535628f98088b70f766f47989801a0dfc754cf4996f505cfd8f082>, - "contractClassId": Fr<0x2c32fd0ebccda2e20057f37fa2e6085c07d9a1236a72a54f58c724418f7b4438>, + "address": AztecAddress<0x0b990cd3da37d6d83cd1dc2cca06ee3cae22877ec9040b817c9ce3ae47634683>, + "contractClassId": Fr<0x136acc115d2b9c3acc18236bb2b72077c0cd855963de8c24f27c3c69120385c9>, "initializationHash": Fr<0x0bf6e812f14bb029f7cb9c8da8367dd97c068e788d4f21007fd97014eba8cf9f>, "portalContractAddress": EthAddress<0x0000000000000000000000000000000000000000>, "publicKeysHash": Fr<0x27b1d0839a5b23baf12a8d195b18ac288fcf401afb2f70b8a4b529ede5fa9fed>, @@ -18,7 +18,7 @@ exports[`GasToken returns canonical protocol contract 1`] = ` exports[`GasToken returns canonical protocol contract 2`] = ` { "artifactHash": Fr<0x076fb6d7493b075a880eeed90fec7c4c01e0a24d442522449e4d56c26357205f>, - "id": Fr<0x2c32fd0ebccda2e20057f37fa2e6085c07d9a1236a72a54f58c724418f7b4438>, + "id": Fr<0x136acc115d2b9c3acc18236bb2b72077c0cd855963de8c24f27c3c69120385c9>, "privateFunctions": [ { "isInternal": false, @@ -27,7 +27,7 @@ exports[`GasToken returns canonical protocol contract 2`] = ` }, ], "privateFunctionsRoot": Fr<0x13b29c3f4a96eb14d5d3539a6308ff9736ad5d67e3f61ffbb7da908e14980828>, - "publicBytecodeCommitment": Fr<0x1c5e1c199e3affad8f3d3ec7db3e3b40639b3c0ef82351506ceb25cde3b04924>, + "publicBytecodeCommitment": Fr<0x1db9d13c128a3c6f7d736161ca3cfeb78b19e5018c3765e90c760aedc7959e26>, "version": 1, } `; diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index ffa53b6ad051..3d67407ef6c7 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -19,7 +19,7 @@ import { ORACLE_NAMES } from './oracle/index.js'; */ type ACIRCallback = Record< ORACLE_NAMES, - (...args: ForeignCallInput[]) => ForeignCallOutput | Promise + (...args: ForeignCallInput[]) => ForeignCallOutput | Promise | Promise >; /** @@ -102,7 +102,8 @@ export async function acvm( } const result = await oracleFunction.call(callback, ...args); - return [result]; + // void functions return undefined, which is mapped to an empty array. + return result === undefined ? [] : [result]; } catch (err) { let typedError: Error; if (err instanceof Error) { diff --git a/yarn-project/simulator/src/acvm/oracle/oracle.ts b/yarn-project/simulator/src/acvm/oracle/oracle.ts index 48824adcae4b..e9ba60e7415b 100644 --- a/yarn-project/simulator/src/acvm/oracle/oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/oracle.ts @@ -248,14 +248,13 @@ export class Oracle { return toACVMField(portalContactAddress); } - async storageRead([startStorageSlot]: ACVMField[], [numberOfElements]: ACVMField[]): Promise { - const values = await this.typedOracle.storageRead(fromACVMField(startStorageSlot), +numberOfElements); - return values.map(toACVMField); + async storageRead([storageSlot]: ACVMField[]): Promise { + const value = await this.typedOracle.storageRead(fromACVMField(storageSlot)); + return toACVMField(value); } - async storageWrite([startStorageSlot]: ACVMField[], values: ACVMField[]): Promise { - const newValues = await this.typedOracle.storageWrite(fromACVMField(startStorageSlot), values.map(fromACVMField)); - return newValues.map(toACVMField); + async storageWrite([storageSlot]: ACVMField[], [value]: ACVMField[]): Promise { + await this.typedOracle.storageWrite(fromACVMField(storageSlot), fromACVMField(value)); } emitEncryptedLog( diff --git a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts index 22eaf2f29386..8ce28dd3aee4 100644 --- a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts @@ -169,11 +169,11 @@ export abstract class TypedOracle { throw new Error('Not available.'); } - storageRead(_startStorageSlot: Fr, _numberOfElements: number): Promise { + storageRead(_storageSlot: Fr): Promise { throw new Error('Not available.'); } - storageWrite(_startStorageSlot: Fr, _values: Fr[]): Promise { + storageWrite(_storageSlot: Fr, _value: Fr): Promise { throw new Error('Not available.'); } diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 2f57c0ff5145..e6cdb37e7d4d 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') @@ -482,59 +482,135 @@ describe('AVM simulator', () => { describe('Storage accesses', () => { it('Should set a single value in storage', async () => { - // We are setting the owner const slot = 1n; - const sender = AztecAddress.fromField(new Fr(1)); const address = AztecAddress.fromField(new Fr(420)); + const value = new Fr(88); + const calldata = [value]; const context = initContext({ - env: initExecutionEnvironment({ sender, address, storageAddress: address }), + env: initExecutionEnvironment({ calldata, 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 expect(results.reverted).toBe(false); - // Contract 420 - Storage slot 1 should contain the value 1 + // World state const worldState = context.persistableState.flush(); - const storageSlot = worldState.currentStorageValue.get(address.toBigInt())!; const adminSlotValue = storageSlot.get(slot); - expect(adminSlotValue).toEqual(sender.toField()); + expect(adminSlotValue).toEqual(value); // Tracing const storageTrace = worldState.storageWrites.get(address.toBigInt())!; const slotTrace = storageTrace.get(slot); - expect(slotTrace).toEqual([sender.toField()]); + expect(slotTrace).toEqual([value]); }); - it('Should read a value from storage', async () => { - // We are setting the owner - const sender = AztecAddress.fromField(new Fr(1)); + it('Should read a single value in storage', async () => { + const slot = 1n; + const value = new Fr(12345); const address = AztecAddress.fromField(new Fr(420)); + const storage = new Map([[slot, value]]); const context = initContext({ - env: initExecutionEnvironment({ sender, address, storageAddress: address }), + env: initExecutionEnvironment({ storageAddress: address }), }); - const bytecode = getAvmTestContractBytecode('avm_setAndRead'); + jest + .spyOn(context.persistableState.hostStorage.publicStateDb, 'storageRead') + .mockImplementation((_address, slot) => Promise.resolve(storage.get(slot.toBigInt())!)); + const bytecode = getAvmTestContractBytecode('avm_readStorageSingle'); const results = await new AvmSimulator(context).executeBytecode(bytecode); + // Get contract function artifact expect(results.reverted).toBe(false); + expect(results.output).toEqual([value]); - expect(results.output).toEqual([sender.toField()]); - + // Tracing const worldState = context.persistableState.flush(); + const storageTrace = worldState.storageReads.get(address.toBigInt())!; + const slotTrace = storageTrace.get(slot); + expect(slotTrace).toEqual([value]); + }); + + it('Should set and read a value from storage', async () => { + const slot = 1n; + const value = new Fr(12345); + const address = AztecAddress.fromField(new Fr(420)); + const calldata = [value]; + + const context = initContext({ + env: initExecutionEnvironment({ calldata, address, storageAddress: address }), + }); + const bytecode = getAvmTestContractBytecode('avm_setReadStorageSingle'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + expect(results.output).toEqual([value]); // Test read trace + const worldState = context.persistableState.flush(); const storageReadTrace = worldState.storageReads.get(address.toBigInt())!; - const slotReadTrace = storageReadTrace.get(1n); - expect(slotReadTrace).toEqual([sender.toField()]); + const slotReadTrace = storageReadTrace.get(slot); + expect(slotReadTrace).toEqual([value]); // Test write trace const storageWriteTrace = worldState.storageWrites.get(address.toBigInt())!; - const slotWriteTrace = storageWriteTrace.get(1n); - expect(slotWriteTrace).toEqual([sender.toField()]); + const slotWriteTrace = storageWriteTrace.get(slot); + expect(slotWriteTrace).toEqual([value]); + }); + + 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]]); + }); + + it('Should read multiple values in storage', async () => { + const slot = 2n; + const address = AztecAddress.fromField(new Fr(420)); + const values = [new Fr(1), new Fr(2)]; + const storage = new Map([ + [slot, values[0]], + [slot + 1n, values[1]], + ]); + + const context = initContext({ + env: initExecutionEnvironment({ address, storageAddress: address }), + }); + jest + .spyOn(context.persistableState.hostStorage.publicStateDb, 'storageRead') + .mockImplementation((_address, slot) => Promise.resolve(storage.get(slot.toBigInt())!)); + const bytecode = getAvmTestContractBytecode('avm_readStorageList'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + expect(results.output).toEqual(values); + + // Tracing + const worldState = context.persistableState.flush(); + const storageTrace = worldState.storageReads.get(address.toBigInt())!; + expect(storageTrace.get(slot)).toEqual([values[0]]); + expect(storageTrace.get(slot + 1n)).toEqual([values[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(); } diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index c0f90ab27583..d13909acb987 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -465,26 +465,20 @@ export class ClientExecutionContext extends ViewDataOracle { /** * Read the public storage data. - * @param startStorageSlot - The starting storage slot. - * @param numberOfElements - Number of elements to read from the starting storage slot. + * @param storageSlot - The storage slot to read from. */ - public async storageRead(startStorageSlot: Fr, numberOfElements: number): Promise { + public async storageRead(storageSlot: Fr): Promise { // TODO(#4320): This is a hack to work around not having directly access to the public data tree but // still having access to the witnesses const bn = await this.db.getBlockNumber(); - const values = []; - for (let i = 0n; i < numberOfElements; i++) { - const storageSlot = new Fr(startStorageSlot.value + i); - const leafSlot = computePublicDataTreeLeafSlot(this.callContext.storageContractAddress, storageSlot); - const witness = await this.db.getPublicDataTreeWitness(bn, leafSlot); - if (!witness) { - throw new Error(`No witness for slot ${storageSlot.toString()}`); - } - const value = witness.leafPreimage.value; - this.log(`Oracle storage read: slot=${storageSlot.toString()} value=${value}`); - values.push(value); + const leafSlot = computePublicDataTreeLeafSlot(this.callContext.storageContractAddress, storageSlot); + const witness = await this.db.getPublicDataTreeWitness(bn, leafSlot); + if (!witness) { + throw new Error(`No witness for slot ${storageSlot.toString()}`); } - return values; + const value = witness.leafPreimage.value; + this.log(`Oracle storage read: slot=${storageSlot.toString()} value=${value}`); + return value; } } diff --git a/yarn-project/simulator/src/client/view_data_oracle.ts b/yarn-project/simulator/src/client/view_data_oracle.ts index 4c953463ef52..b5219d6dc2f0 100644 --- a/yarn-project/simulator/src/client/view_data_oracle.ts +++ b/yarn-project/simulator/src/client/view_data_oracle.ts @@ -236,18 +236,11 @@ export class ViewDataOracle extends TypedOracle { /** * Read the public storage data. - * @param startStorageSlot - The starting storage slot. - * @param numberOfElements - Number of elements to read from the starting storage slot. + * @param storageSlot - The storage slot to read from. */ - public async storageRead(startStorageSlot: Fr, numberOfElements: number) { - const values = []; - for (let i = 0n; i < numberOfElements; i++) { - const storageSlot = new Fr(startStorageSlot.value + i); - const value = await this.aztecNode.getPublicStorageAt(this.contractAddress, storageSlot); - - this.log(`Oracle storage read: slot=${storageSlot.toString()} value=${value}`); - values.push(value); - } - return values; + public async storageRead(storageSlot: Fr) { + const value = await this.aztecNode.getPublicStorageAt(this.contractAddress, storageSlot); + this.log(`Oracle storage read: slot=${storageSlot.toString()} value=${value}`); + return value; } } diff --git a/yarn-project/simulator/src/public/public_execution_context.ts b/yarn-project/simulator/src/public/public_execution_context.ts index 8d332e8edb72..a7ed82de8729 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -115,38 +115,25 @@ export class PublicExecutionContext extends TypedOracle { /** * Read the public storage data. - * @param startStorageSlot - The starting storage slot. - * @param numberOfElements - Number of elements to read from the starting storage slot. + * @param storageSlot - The storage slot to read from. */ - public async storageRead(startStorageSlot: Fr, numberOfElements: number) { - const values = []; - for (let i = 0; i < Number(numberOfElements); i++) { - const storageSlot = new Fr(startStorageSlot.value + BigInt(i)); - const sideEffectCounter = this.sideEffectCounter.count(); - const value = await this.storageActions.read(storageSlot, sideEffectCounter); - this.log(`Oracle storage read: slot=${storageSlot.toString()} value=${value.toString()}`); - values.push(value); - } - return values; + public async storageRead(storageSlot: Fr) { + const sideEffectCounter = this.sideEffectCounter.count(); + const value = await this.storageActions.read(storageSlot, sideEffectCounter); + this.log(`Oracle storage read: slot=${storageSlot.toString()} value=${value.toString()}`); + return value; } /** - * Write some values to the public storage. - * @param startStorageSlot - The starting storage slot. - * @param values - The values to be written. + * Write a value to the public storage. + * @param storageSlot - The storage slot to write to. + * @param newValue - The value to be written. */ - public async storageWrite(startStorageSlot: Fr, values: Fr[]) { - const newValues = []; - for (let i = 0; i < values.length; i++) { - const storageSlot = new Fr(startStorageSlot.toBigInt() + BigInt(i)); - const newValue = values[i]; - const sideEffectCounter = this.sideEffectCounter.count(); - this.storageActions.write(storageSlot, newValue, sideEffectCounter); - await this.stateDb.storageWrite(this.execution.callContext.storageContractAddress, storageSlot, newValue); - this.log(`Oracle storage write: slot=${storageSlot.toString()} value=${newValue.toString()}`); - newValues.push(newValue); - } - return newValues; + public async storageWrite(storageSlot: Fr, newValue: Fr) { + const sideEffectCounter = this.sideEffectCounter.count(); + this.storageActions.write(storageSlot, newValue, sideEffectCounter); + await this.stateDb.storageWrite(this.execution.callContext.storageContractAddress, storageSlot, newValue); + this.log(`Oracle storage write: slot=${storageSlot.toString()} value=${newValue.toString()}`); } /**