diff --git a/avm-transpiler/src/instructions.rs b/avm-transpiler/src/instructions.rs index 9df6f20551c6..b49753c63579 100644 --- a/avm-transpiler/src/instructions.rs +++ b/avm-transpiler/src/instructions.rs @@ -7,6 +7,7 @@ use crate::opcodes::AvmOpcode; pub const ALL_DIRECT: u8 = 0b00000000; pub const ZEROTH_OPERAND_INDIRECT: u8 = 0b00000001; pub const FIRST_OPERAND_INDIRECT: u8 = 0b00000010; +pub const SECOND_OPERAND_INDIRECT: u8 = 0b00000100; pub const ZEROTH_FIRST_OPERANDS_INDIRECT: u8 = ZEROTH_OPERAND_INDIRECT | FIRST_OPERAND_INDIRECT; /// A simple representation of an AVM instruction for the purpose diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index e4a09137776f..ea6f66f1673c 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -7,7 +7,7 @@ use acvm::brillig_vm::brillig::{ use crate::instructions::{ AvmInstruction, AvmOperand, AvmTypeTag, ALL_DIRECT, FIRST_OPERAND_INDIRECT, - ZEROTH_OPERAND_INDIRECT, + SECOND_OPERAND_INDIRECT, ZEROTH_OPERAND_INDIRECT, }; use crate::opcodes::AvmOpcode; use crate::utils::{dbg_print_avm_program, dbg_print_brillig_program}; @@ -257,8 +257,10 @@ fn handle_foreign_call( "avmOpcodePoseidon" => { handle_single_field_hash_instruction(avm_instrs, function, destinations, inputs) } + "storageWrite" => emit_storage_write(avm_instrs, destinations, inputs), + "storageRead" => emit_storage_read(avm_instrs, destinations, inputs), // Getters. - _ if inputs.len() == 0 && destinations.len() == 1 => { + _ if inputs.is_empty() && destinations.len() == 1 => { handle_getter_instruction(avm_instrs, function, destinations, inputs) } // Anything else. @@ -361,7 +363,7 @@ fn handle_emit_note_hash_or_nullifier( "EMITNOTEHASH" }; - if destinations.len() != 0 || inputs.len() != 1 { + if !destinations.is_empty() || inputs.len() != 1 { panic!( "Transpiler expects ForeignCall::{} to have 0 destinations and 1 input, got {} and {}", function_name, @@ -390,6 +392,87 @@ fn handle_emit_note_hash_or_nullifier( }); } +/// Emit a storage write opcode +/// The current implementation writes an array of values into storage ( contiguous slots in memory ) +fn emit_storage_write( + avm_instrs: &mut Vec, + destinations: &Vec, + inputs: &Vec, +) { + assert!(inputs.len() == 2); + 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"), + }; + + 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"), + }; + + avm_instrs.push(AvmInstruction { + opcode: AvmOpcode::SSTORE, + indirect: Some(ZEROTH_OPERAND_INDIRECT), + operands: vec![ + AvmOperand::U32 { + value: src_offset as u32, + }, + AvmOperand::U32 { + value: src_size as u32, + }, + AvmOperand::U32 { + value: slot_offset as u32, + }, + ], + ..Default::default() + }) +} + +/// Emit a storage read opcode +/// The current implementation reads an array of values from storage ( contiguous slots in memory ) +fn emit_storage_read( + avm_instrs: &mut Vec, + 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!(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"), + }; + + 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"), + }; + + avm_instrs.push(AvmInstruction { + opcode: AvmOpcode::SLOAD, + indirect: Some(SECOND_OPERAND_INDIRECT), + operands: vec![ + AvmOperand::U32 { + value: slot_offset as u32, + }, + AvmOperand::U32 { + value: src_size as u32, + }, + AvmOperand::U32 { + value: dest_offset as u32, + }, + ], + ..Default::default() + }) +} + /// Handle an AVM NULLIFIEREXISTS instruction /// (a nullifierExists brillig foreign call was encountered) /// Adds the new instruction to the avm instructions list. @@ -483,7 +566,7 @@ fn handle_send_l2_to_l1_msg( destinations: &Vec, inputs: &Vec, ) { - if destinations.len() != 0 || inputs.len() != 2 { + if !destinations.is_empty() || inputs.len() != 2 { panic!( "Transpiler expects ForeignCall::SENDL2TOL1MSG to have 0 destinations and 2 inputs, got {} and {}", destinations.len(), diff --git a/noir-projects/aztec-nr/aztec/src/context.nr b/noir-projects/aztec-nr/aztec/src/context.nr index 09162c6cf84b..9f5ae7efb82c 100644 --- a/noir-projects/aztec-nr/aztec/src/context.nr +++ b/noir-projects/aztec-nr/aztec/src/context.nr @@ -14,18 +14,23 @@ use avm::AVMContext; struct Context { private: Option<&mut PrivateContext>, public: Option<&mut PublicContext>, + public_vm: Option<&mut AVMContext>, } impl Context { pub fn private(context: &mut PrivateContext) -> Context { - Context { private: Option::some(context), public: Option::none() } + Context { private: Option::some(context), public: Option::none(), public_vm: Option::none() } } pub fn public(context: &mut PublicContext) -> Context { - Context { public: Option::some(context), private: Option::none() } + Context { public: Option::some(context), private: Option::none(), public_vm: Option::none() } + } + + pub fn public_vm(context: &mut AVMContext) -> Context { + Context { public_vm: Option::some(context), public: Option::none(), private: Option::none() } } pub fn none() -> Context { - Context { public: Option::none(), private: Option::none() } + Context { public: Option::none(), private: Option::none(), public_vm: Option::none() } } } diff --git a/noir-projects/aztec-nr/aztec/src/oracle/storage.nr b/noir-projects/aztec-nr/aztec/src/oracle/storage.nr index 72bec83be3a3..ad9b148f0d07 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/storage.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/storage.nr @@ -12,8 +12,8 @@ pub fn storage_read(storage_slot: Field) -> [Field; N] { } #[oracle(storageWrite)] -fn storage_write_oracle(_storage_slot: Field, _values: [Field; N]) -> [Field; N] {} +fn storage_write_oracle(_storage_slot: Field, _values: [Field; N]) -> Field {} unconstrained pub fn storage_write(storage_slot: Field, fields: [Field; N]) { - let _hash = storage_write_oracle(storage_slot, fields); + let _ = storage_write_oracle(storage_slot, fields); } 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 f5134cb89401..f039bb367e30 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,5 +1,6 @@ contract AvmTest { // Libs + use dep::aztec::state_vars::PublicMutable; use dep::aztec::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH}; use dep::compressed_string::CompressedString; @@ -9,7 +10,21 @@ contract AvmTest { #[aztec(private)] fn constructor() {} - // Public-vm macro will prefix avm to the function name for transpilation + struct Storage { + owner: PublicMutable + } + + #[aztec(public-vm)] + fn setAdmin() { + storage.owner.write(context.sender()); + } + + #[aztec(public-vm)] + fn setAndRead() -> pub AztecAddress { + storage.owner.write(context.sender()); + storage.owner.read() + } + #[aztec(public-vm)] fn addArgsReturn(argA: Field, argB: Field) -> pub Field { argA + argB diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index f9df3f101299..012995d6ed45 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -727,8 +727,14 @@ fn transform_function( /// Transform a function to work with AVM bytecode fn transform_vm_function( func: &mut NoirFunction, - _storage_defined: bool, + storage_defined: bool, ) -> Result<(), AztecMacroError> { + // Create access to storage + if storage_defined { + let storage = abstract_storage("public_vm", true); + func.def.body.0.insert(0, storage); + } + // Push Avm context creation to the beginning of the function let create_context = create_avm_context()?; func.def.body.0.insert(0, create_context); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index f1a8f24ed03f..662dc074d980 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -32,7 +32,7 @@ use num_bigint::BigUint; /// The Brillig VM does not apply a limit to the memory address space, /// As a convention, we take use 64 bits. This means that we assume that /// memory has 2^64 memory slots. -pub(crate) const BRILLIG_MEMORY_ADDRESSING_BIT_SIZE: u32 = 32; +pub(crate) const BRILLIG_MEMORY_ADDRESSING_BIT_SIZE: u32 = 64; // Registers reserved in runtime for special purposes. pub(crate) enum ReservedRegisters { @@ -562,6 +562,7 @@ impl BrilligContext { bit_size: u32, ) { self.debug_show.const_instruction(result, constant); + self.push_opcode(BrilligOpcode::Const { destination: result, value: constant, bit_size }); } diff --git a/noir/noir-repo/test_programs/noir_test_failure/should_fail_suite_with_one_failure/Nargo.toml b/noir/noir-repo/test_programs/noir_test_failure/should_fail_suite_with_one_failure/Nargo.toml new file mode 100644 index 000000000000..3d2cf2c60965 --- /dev/null +++ b/noir/noir-repo/test_programs/noir_test_failure/should_fail_suite_with_one_failure/Nargo.toml @@ -0,0 +1,5 @@ +[package] +name = "should_fail_with_mismatch" +type = "bin" +authors = [""] +[dependencies] diff --git a/yarn-project/simulator/src/acvm/oracle/oracle.ts b/yarn-project/simulator/src/acvm/oracle/oracle.ts index 48824adcae4b..2e1aedfc51da 100644 --- a/yarn-project/simulator/src/acvm/oracle/oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/oracle.ts @@ -253,9 +253,11 @@ export class Oracle { return values.map(toACVMField); } - async storageWrite([startStorageSlot]: ACVMField[], values: ACVMField[]): Promise { - const newValues = await this.typedOracle.storageWrite(fromACVMField(startStorageSlot), values.map(fromACVMField)); - return newValues.map(toACVMField); + storageWrite([startStorageSlot]: ACVMField[], values: ACVMField[]) { + this.typedOracle.storageWrite(fromACVMField(startStorageSlot), values.map(fromACVMField)); + + // We return 0 here as we MUST return something, but the value is not used. + return '0'; } 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..863295aa720a 100644 --- a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts @@ -173,7 +173,7 @@ export abstract class TypedOracle { throw new Error('Not available.'); } - storageWrite(_startStorageSlot: Fr, _values: Fr[]): Promise { + storageWrite(_startStorageSlot: Fr, _values: Fr[]) { throw new Error('Not available.'); } diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index 9b26c3f8b4d2..48e56ed30260 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -255,6 +255,12 @@ export class TaggedMemory { } } + public checkIsValidMemoryOffsetTag(offset: number) { + if (this.getTag(offset) > TypeTag.UINT64) { + throw TagCheckError.forOffset(offset, TypeTag[this.getTag(offset)], 'UINT64'); + } + } + public static checkIsIntegralTag(tag: TypeTag) { if (![TypeTag.UINT8, TypeTag.UINT16, TypeTag.UINT32, TypeTag.UINT64, TypeTag.UINT128].includes(tag)) { throw TagCheckError.forTag(TypeTag[tag], 'integral'); diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 5c2ce65376cc..0f06592d1550 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -111,6 +111,64 @@ 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 context = initContext({ + env: initExecutionEnvironment({ sender, address, storageAddress: address }), + }); + const bytecode = getAvmTestContractBytecode('avm_setAdmin'); + 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 + const worldState = context.persistableState.flush(); + + const storageSlot = worldState.currentStorageValue.get(address.toBigInt())!; + const adminSlotValue = storageSlot.get(slot); + expect(adminSlotValue).toEqual(sender.toField()); + + // Tracing + const storageTrace = worldState.storageWrites.get(address.toBigInt())!; + const slotTrace = storageTrace.get(slot); + expect(slotTrace).toEqual([sender.toField()]); + }); + + it('Should 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)); + + const context = initContext({ + env: initExecutionEnvironment({ sender, address, storageAddress: address }), + }); + const bytecode = getAvmTestContractBytecode('avm_setAndRead'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); + + expect(results.reverted).toBe(false); + + expect(results.output).toEqual([sender.toField()]); + + const worldState = context.persistableState.flush(); + + // Test read trace + const storageReadTrace = worldState.storageReads.get(address.toBigInt())!; + const slotReadTrace = storageReadTrace.get(1n); + expect(slotReadTrace).toEqual([sender.toField()]); + + // Test write trace + const storageWriteTrace = worldState.storageWrites.get(address.toBigInt())!; + const slotWriteTrace = storageWriteTrace.get(1n); + expect(slotWriteTrace).toEqual([sender.toField()]); + }); + }); + describe('Test env getters from noir contract', () => { const testEnvGetter = async (valueName: string, value: any, functionName: string, globalVar: boolean = false) => { // Execute diff --git a/yarn-project/simulator/src/avm/journal/trace.ts b/yarn-project/simulator/src/avm/journal/trace.ts index 620e2e0462ca..c624aa3ed591 100644 --- a/yarn-project/simulator/src/avm/journal/trace.ts +++ b/yarn-project/simulator/src/avm/journal/trace.ts @@ -46,7 +46,7 @@ export class WorldStateAccessTrace { } public tracePublicStorageWrite(storageAddress: Fr, slot: Fr, value: Fr) { - // TODO(4805): check if some threshold is reached for max storage writes + // TODO: check if some threshold is reached for max storage writes // (need access to parent length, or trace needs to be initialized with parent's contents) //const traced: TracedPublicStorageWrite = { // callPointer: Fr.ZERO, @@ -57,6 +57,7 @@ export class WorldStateAccessTrace { // endLifetime: Fr.ZERO, //}; //this.publicStorageWrites.push(traced); + this.journalWrite(storageAddress, slot, value); this.incrementAccessCounter(); } diff --git a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts index 1fb4137205a5..280d15e1adee 100644 --- a/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts +++ b/yarn-project/simulator/src/avm/opcodes/addressing_mode.ts @@ -1,6 +1,6 @@ import { strict as assert } from 'assert'; -import { TaggedMemory, TypeTag } from '../avm_memory_types.js'; +import { TaggedMemory } from '../avm_memory_types.js'; export enum AddressingMode { DIRECT, @@ -51,7 +51,8 @@ export class Addressing { for (const [i, offset] of offsets.entries()) { switch (this.modePerOperand[i]) { case AddressingMode.INDIRECT: - mem.checkTag(TypeTag.UINT32, offset); + // NOTE(reviewer): less than equal is a deviation from the spec - i dont see why this shouldnt be possible! + mem.checkIsValidMemoryOffsetTag(offset); resolved[i] = Number(mem.get(offset).toBigInt()); break; case AddressingMode.DIRECT: 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 e557f3242782..2f14a7a23989 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -70,7 +70,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, /*slotOffset=*/ 0), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*size=*/ 1, /*slotOffset=*/ 0), new Return(/*indirect=*/ 0, /*retOffset=*/ 0, /*size=*/ 2), ]); @@ -159,7 +159,7 @@ describe('External Calls', () => { const otherContextInstructions: Instruction[] = [ new CalldataCopy(/*indirect=*/ 0, /*csOffset=*/ 0, /*copySize=*/ argsSize, /*dstOffset=*/ 0), - new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*slotOffset=*/ 0), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*size=*/ 1, /*slotOffset=*/ 0), ]; const otherContextInstructionsBytecode = encodeToBytecode(otherContextInstructions); diff --git a/yarn-project/simulator/src/avm/opcodes/hashing.test.ts b/yarn-project/simulator/src/avm/opcodes/hashing.test.ts index 452b997a62fa..794abb1468f6 100644 --- a/yarn-project/simulator/src/avm/opcodes/hashing.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/hashing.test.ts @@ -64,6 +64,24 @@ describe('Hashing Opcodes', () => { const result = context.machineState.memory.get(dstOffset); expect(result).toEqual(new Field(expectedHash)); }); + + it('Should hash correctly - indirect pos', async () => { + const args = [new Field(1n), new Field(2n), new Field(3n)]; + const indirect = 1; + const hashOffset = 0; + const realLocation = 4; + + context.machineState.memory.set(hashOffset, new Uint32(realLocation)); + context.machineState.memory.setSlice(realLocation, args); + + const dstOffset = 3; + + const expectedHash = poseidonHash(args.map(field => field.toBuffer())); + await new Poseidon2(indirect, dstOffset, hashOffset, args.length).execute(context); + + const result = context.machineState.memory.get(dstOffset); + expect(result).toEqual(new Field(expectedHash)); + }); }); describe('Keccak', () => { @@ -126,7 +144,6 @@ describe('Hashing Opcodes', () => { expect(combined).toEqual(expectedHash); }); - // TODO: indirect }); describe('Sha256', () => { @@ -245,5 +262,24 @@ describe('Hashing Opcodes', () => { const result = context.machineState.memory.get(dstOffset); expect(result).toEqual(new Field(expectedHash)); }); + + it('Should hash correctly - indirect', async () => { + const args = [new Field(1n), new Field(2n), new Field(3n)]; + const indirect = 1; + const hashOffset = 0; + const realLocation = 4; + + context.machineState.memory.set(hashOffset, new Uint32(realLocation)); + context.machineState.memory.setSlice(realLocation, args); + + const dstOffset = 3; + + const inputBuffer = args.map(field => field.toBuffer()); + const expectedHash = pedersenHash(inputBuffer); + await new Pedersen(indirect, dstOffset, hashOffset, args.length).execute(context); + + const result = context.machineState.memory.get(dstOffset); + expect(result).toEqual(new Field(expectedHash)); + }); }); }); diff --git a/yarn-project/simulator/src/avm/opcodes/storage.test.ts b/yarn-project/simulator/src/avm/opcodes/storage.test.ts index bd53a1d3324d..ab98f3e7140a 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.test.ts @@ -28,9 +28,15 @@ describe('Storage Instructions', () => { SStore.opcode, // opcode 0x01, // indirect ...Buffer.from('12345678', 'hex'), // srcOffset - ...Buffer.from('a2345678', 'hex'), // slotOffset + ...Buffer.from('a2345678', 'hex'), // size + ...Buffer.from('3456789a', 'hex'), // slotOffset ]); - const inst = new SStore(/*indirect=*/ 0x01, /*srcOffset=*/ 0x12345678, /*slotOffset=*/ 0xa2345678); + const inst = new SStore( + /*indirect=*/ 0x01, + /*srcOffset=*/ 0x12345678, + /*size=*/ 0xa2345678, + /*slotOffset=*/ 0x3456789a, + ); expect(SStore.deserialize(buf)).toEqual(inst); expect(inst.serialize()).toEqual(buf); @@ -43,7 +49,7 @@ describe('Storage Instructions', () => { context.machineState.memory.set(0, a); context.machineState.memory.set(1, b); - await new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*slotOffset=*/ 1).execute(context); + await new SStore(/*indirect=*/ 0, /*srcOffset=*/ 1, /*size=*/ 1, /*slotOffset=*/ 0).execute(context); expect(journal.writeStorage).toHaveBeenCalledWith(address, new Fr(a.toBigInt()), new Fr(b.toBigInt())); }); @@ -60,7 +66,8 @@ describe('Storage Instructions', () => { context.machineState.memory.set(0, a); context.machineState.memory.set(1, b); - const instruction = () => new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*slotOffset=*/ 1).execute(context); + const instruction = () => + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*size=*/ 1, /*slotOffset=*/ 1).execute(context); await expect(instruction()).rejects.toThrow(StaticCallStorageAlterError); }); }); @@ -71,9 +78,15 @@ describe('Storage Instructions', () => { SLoad.opcode, // opcode 0x01, // indirect ...Buffer.from('12345678', 'hex'), // slotOffset - ...Buffer.from('a2345678', 'hex'), // dstOffset + ...Buffer.from('a2345678', 'hex'), // size + ...Buffer.from('3456789a', 'hex'), // dstOffset ]); - const inst = new SLoad(/*indirect=*/ 0x01, /*slotOffset=*/ 0x12345678, /*dstOffset=*/ 0xa2345678); + const inst = new SLoad( + /*indirect=*/ 0x01, + /*slotOffset=*/ 0x12345678, + /*size=*/ 0xa2345678, + /*dstOffset=*/ 0x3456789a, + ); expect(SLoad.deserialize(buf)).toEqual(inst); expect(inst.serialize()).toEqual(buf); @@ -90,7 +103,7 @@ describe('Storage Instructions', () => { context.machineState.memory.set(0, a); context.machineState.memory.set(1, b); - await new SLoad(/*indirect=*/ 0, /*slotOffset=*/ 0, /*dstOffset=*/ 1).execute(context); + await new SLoad(/*indirect=*/ 0, /*slotOffset=*/ 0, /*size=*/ 1, /*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 1090d5d35403..3ca3bfa19aaf 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.ts @@ -4,6 +4,7 @@ import type { AvmContext } from '../avm_context.js'; import { Field } from '../avm_memory_types.js'; import { InstructionExecutionError } from '../errors.js'; import { Opcode, OperandType } from '../serialization/instruction_serialization.js'; +import { Addressing } from './addressing_mode.js'; import { Instruction } from './instruction.js'; abstract class BaseStorageInstruction extends Instruction { @@ -13,9 +14,15 @@ abstract class BaseStorageInstruction extends Instruction { OperandType.UINT8, OperandType.UINT32, OperandType.UINT32, + OperandType.UINT32, ]; - constructor(protected indirect: number, protected aOffset: number, protected bOffset: number) { + constructor( + protected indirect: number, + protected aOffset: number, + protected /*temporary*/ size: number, + protected bOffset: number, + ) { super(); } } @@ -24,8 +31,8 @@ export class SStore extends BaseStorageInstruction { static readonly type: string = 'SSTORE'; static readonly opcode = Opcode.SSTORE; - constructor(indirect: number, srcOffset: number, slotOffset: number) { - super(indirect, srcOffset, slotOffset); + constructor(indirect: number, srcOffset: number, /*temporary*/ srcSize: number, slotOffset: number) { + super(indirect, srcOffset, srcSize, slotOffset); } async execute(context: AvmContext): Promise { @@ -33,15 +40,19 @@ export class SStore extends BaseStorageInstruction { throw new StaticCallStorageAlterError(); } - const slot = context.machineState.memory.get(this.aOffset); - const data = context.machineState.memory.get(this.bOffset); - - context.persistableState.writeStorage( - context.environment.storageAddress, - new Fr(slot.toBigInt()), - new Fr(data.toBigInt()), + const [srcOffset, slotOffset] = Addressing.fromWire(this.indirect).resolve( + [this.aOffset, this.bOffset], + context.machineState.memory, ); + const slot = context.machineState.memory.get(slotOffset).toFr(); + const data = context.machineState.memory.getSlice(srcOffset, this.size).map(field => field.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.machineState.incrementPc(); } } @@ -50,19 +61,27 @@ export class SLoad extends BaseStorageInstruction { static readonly type: string = 'SLOAD'; static readonly opcode = Opcode.SLOAD; - constructor(indirect: number, slotOffset: number, dstOffset: number) { - super(indirect, slotOffset, dstOffset); + constructor(indirect: number, slotOffset: number, size: number, dstOffset: number) { + super(indirect, slotOffset, size, dstOffset); } async execute(context: AvmContext): Promise { - const slot = context.machineState.memory.get(this.aOffset); - - const data: Fr = await context.persistableState.readStorage( - context.environment.storageAddress, - new Fr(slot.toBigInt()), + const [aOffset, size, bOffset] = Addressing.fromWire(this.indirect).resolve( + [this.aOffset, this.size, this.bOffset], + context.machineState.memory, ); - context.machineState.memory.set(this.bOffset, new Field(data)); + 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)); + } context.machineState.incrementPc(); } diff --git a/yarn-project/simulator/src/public/public_execution_context.ts b/yarn-project/simulator/src/public/public_execution_context.ts index 3b53fd2490ba..77a6bb7b5508 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -136,7 +136,6 @@ export class PublicExecutionContext extends TypedOracle { * @param values - The values 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]; @@ -144,9 +143,7 @@ export class PublicExecutionContext extends TypedOracle { 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; } /**