From e190e598c6e998d4cfe26805c53bbaef2d58f0b3 Mon Sep 17 00:00:00 2001 From: ludamad Date: Wed, 6 Mar 2024 21:07:18 -0500 Subject: [PATCH] Revert "feat(avm): storage (#4673)" This reverts commit bfdbf2e0cb6e5daff4178aca4c5a9b5b87f8b57d. --- avm-transpiler/src/instructions.rs | 1 - avm-transpiler/src/transpile.rs | 91 +------------------ noir-projects/aztec-nr/aztec/src/context.nr | 11 +-- .../aztec-nr/aztec/src/oracle/storage.nr | 4 +- .../contracts/avm_test_contract/src/main.nr | 17 +--- noir/noir-repo/aztec_macros/src/lib.rs | 8 +- .../noirc_evaluator/src/brillig/brillig_ir.rs | 3 +- .../Nargo.toml | 5 - .../simulator/src/acvm/oracle/oracle.ts | 8 +- .../simulator/src/acvm/oracle/typed_oracle.ts | 2 +- .../simulator/src/avm/avm_memory_types.ts | 6 -- .../simulator/src/avm/avm_simulator.test.ts | 58 ------------ .../simulator/src/avm/journal/trace.ts | 3 +- .../src/avm/opcodes/addressing_mode.ts | 5 +- .../src/avm/opcodes/external_calls.test.ts | 4 +- .../simulator/src/avm/opcodes/hashing.test.ts | 38 +------- .../simulator/src/avm/opcodes/storage.test.ts | 27 ++---- .../simulator/src/avm/opcodes/storage.ts | 55 ++++------- .../src/public/public_execution_context.ts | 3 + 19 files changed, 50 insertions(+), 299 deletions(-) delete mode 100644 noir/noir-repo/test_programs/noir_test_failure/should_fail_suite_with_one_failure/Nargo.toml diff --git a/avm-transpiler/src/instructions.rs b/avm-transpiler/src/instructions.rs index b49753c6357..9df6f20551c 100644 --- a/avm-transpiler/src/instructions.rs +++ b/avm-transpiler/src/instructions.rs @@ -7,7 +7,6 @@ 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 ea6f66f1673..e4a09137776 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, - SECOND_OPERAND_INDIRECT, ZEROTH_OPERAND_INDIRECT, + ZEROTH_OPERAND_INDIRECT, }; use crate::opcodes::AvmOpcode; use crate::utils::{dbg_print_avm_program, dbg_print_brillig_program}; @@ -257,10 +257,8 @@ 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.is_empty() && destinations.len() == 1 => { + _ if inputs.len() == 0 && destinations.len() == 1 => { handle_getter_instruction(avm_instrs, function, destinations, inputs) } // Anything else. @@ -363,7 +361,7 @@ fn handle_emit_note_hash_or_nullifier( "EMITNOTEHASH" }; - if !destinations.is_empty() || inputs.len() != 1 { + if destinations.len() != 0 || inputs.len() != 1 { panic!( "Transpiler expects ForeignCall::{} to have 0 destinations and 1 input, got {} and {}", function_name, @@ -392,87 +390,6 @@ 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. @@ -566,7 +483,7 @@ fn handle_send_l2_to_l1_msg( destinations: &Vec, inputs: &Vec, ) { - if !destinations.is_empty() || inputs.len() != 2 { + if destinations.len() != 0 || 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 9f5ae7efb82..09162c6cf84 100644 --- a/noir-projects/aztec-nr/aztec/src/context.nr +++ b/noir-projects/aztec-nr/aztec/src/context.nr @@ -14,23 +14,18 @@ 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(), public_vm: Option::none() } + Context { private: Option::some(context), public: Option::none() } } pub fn public(context: &mut PublicContext) -> Context { - 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() } + Context { public: Option::some(context), private: Option::none() } } pub fn none() -> Context { - Context { public: Option::none(), private: Option::none(), public_vm: Option::none() } + Context { public: Option::none(), private: 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 ad9b148f0d0..72bec83be3a 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 {} +fn storage_write_oracle(_storage_slot: Field, _values: [Field; N]) -> [Field; N] {} unconstrained pub fn storage_write(storage_slot: Field, fields: [Field; N]) { - let _ = storage_write_oracle(storage_slot, fields); + let _hash = 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 f039bb367e3..f5134cb8940 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,6 +1,5 @@ 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; @@ -10,21 +9,7 @@ contract AvmTest { #[aztec(private)] fn constructor() {} - 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() - } - + // Public-vm macro will prefix avm to the function name for transpilation #[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 012995d6ed4..f9df3f10129 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -727,14 +727,8 @@ 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 662dc074d98..f1a8f24ed03 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 = 64; +pub(crate) const BRILLIG_MEMORY_ADDRESSING_BIT_SIZE: u32 = 32; // Registers reserved in runtime for special purposes. pub(crate) enum ReservedRegisters { @@ -562,7 +562,6 @@ 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 deleted file mode 100644 index 3d2cf2c6096..00000000000 --- a/noir/noir-repo/test_programs/noir_test_failure/should_fail_suite_with_one_failure/Nargo.toml +++ /dev/null @@ -1,5 +0,0 @@ -[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 2e1aedfc51d..48824adcae4 100644 --- a/yarn-project/simulator/src/acvm/oracle/oracle.ts +++ b/yarn-project/simulator/src/acvm/oracle/oracle.ts @@ -253,11 +253,9 @@ export class Oracle { return values.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'; + async storageWrite([startStorageSlot]: ACVMField[], values: ACVMField[]): Promise { + const newValues = await this.typedOracle.storageWrite(fromACVMField(startStorageSlot), values.map(fromACVMField)); + return newValues.map(toACVMField); } emitEncryptedLog( diff --git a/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts b/yarn-project/simulator/src/acvm/oracle/typed_oracle.ts index 863295aa720..22eaf2f2938 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[]) { + storageWrite(_startStorageSlot: Fr, _values: Fr[]): Promise { 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 48e56ed3026..9b26c3f8b4d 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -255,12 +255,6 @@ 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 0f06592d155..5c2ce65376c 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -111,64 +111,6 @@ 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 c624aa3ed59..620e2e0462c 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: check if some threshold is reached for max storage writes + // TODO(4805): 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,7 +57,6 @@ 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 280d15e1ade..1fb4137205a 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 } from '../avm_memory_types.js'; +import { TaggedMemory, TypeTag } from '../avm_memory_types.js'; export enum AddressingMode { DIRECT, @@ -51,8 +51,7 @@ export class Addressing { for (const [i, offset] of offsets.entries()) { switch (this.modePerOperand[i]) { case AddressingMode.INDIRECT: - // NOTE(reviewer): less than equal is a deviation from the spec - i dont see why this shouldnt be possible! - mem.checkIsValidMemoryOffsetTag(offset); + mem.checkTag(TypeTag.UINT32, 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 2f14a7a2398..e557f324278 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, /*size=*/ 1, /*slotOffset=*/ 0), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*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, /*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/hashing.test.ts b/yarn-project/simulator/src/avm/opcodes/hashing.test.ts index 794abb1468f..452b997a62f 100644 --- a/yarn-project/simulator/src/avm/opcodes/hashing.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/hashing.test.ts @@ -64,24 +64,6 @@ 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', () => { @@ -144,6 +126,7 @@ describe('Hashing Opcodes', () => { expect(combined).toEqual(expectedHash); }); + // TODO: indirect }); describe('Sha256', () => { @@ -262,24 +245,5 @@ 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 ab98f3e7140..bd53a1d3324 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 + ...Buffer.from('a2345678', 'hex'), // slotOffset ]); - const inst = new SStore( - /*indirect=*/ 0x01, - /*srcOffset=*/ 0x12345678, - /*size=*/ 0xa2345678, - /*slotOffset=*/ 0x3456789a, - ); + const inst = new SStore(/*indirect=*/ 0x01, /*srcOffset=*/ 0x12345678, /*slotOffset=*/ 0xa2345678); 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=*/ 0, /*slotOffset=*/ 1).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 + ...Buffer.from('a2345678', 'hex'), // dstOffset ]); - const inst = new SLoad( - /*indirect=*/ 0x01, - /*slotOffset=*/ 0x12345678, - /*size=*/ 0xa2345678, - /*dstOffset=*/ 0x3456789a, - ); + const inst = new SLoad(/*indirect=*/ 0x01, /*slotOffset=*/ 0x12345678, /*dstOffset=*/ 0xa2345678); 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 3ca3bfa19aa..1090d5d3540 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.ts @@ -4,7 +4,6 @@ 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 { @@ -14,15 +13,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 +24,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 { @@ -40,18 +33,14 @@ export class SStore extends BaseStorageInstruction { throw new StaticCallStorageAlterError(); } - 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()); + const slot = context.machineState.memory.get(this.aOffset); + const data = context.machineState.memory.get(this.bOffset); - 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, + new Fr(slot.toBigInt()), + new Fr(data.toBigInt()), + ); 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], - context.machineState.memory, - ); - - const slot = context.machineState.memory.get(aOffset); + const slot = context.machineState.memory.get(this.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)), - ); + const data: Fr = await context.persistableState.readStorage( + context.environment.storageAddress, + new Fr(slot.toBigInt()), + ); - context.machineState.memory.set(bOffset + i, new Field(data)); - } + context.machineState.memory.set(this.bOffset, 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 77a6bb7b550..3b53fd2490b 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -136,6 +136,7 @@ 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]; @@ -143,7 +144,9 @@ 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; } /**