From bdd9b0677089bc54c461beddafc60db95e2456c2 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 29 Aug 2024 16:29:40 +0100 Subject: [PATCH] feat(avm): 1-slot sload/sstore (nr, ts) (#8264) As agreed with Zac, * Changes the AVM opcodes to work 1-slot at a time (this is easier to handle in the circuit). * Bubbles up changes to aztec nr. However, this is internal to the PublicContext only, the exported interface still takes N slots/fields. On the CPP side, I hardcoded sizes to 1. Work needs to be done to simplify things now that we can. --- avm-transpiler/src/transpile.rs | 25 ++-- .../vm/avm/tests/execution.test.cpp | 134 ------------------ .../vm/avm/trace/deserialization.cpp | 4 +- .../barretenberg/vm/avm/trace/execution.cpp | 8 +- .../aztec/src/context/public_context.nr | 22 +-- .../src/avm/opcodes/external_calls.test.ts | 4 +- .../simulator/src/avm/opcodes/storage.test.ts | 23 +-- .../simulator/src/avm/opcodes/storage.ts | 50 ++----- yarn-project/txe/src/oracle/txe_oracle.ts | 28 ++-- .../txe/src/txe_service/txe_service.ts | 10 +- 10 files changed, 69 insertions(+), 239 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index f36e90d9e9b..da3a6eef0a9 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -991,17 +991,16 @@ fn handle_storage_write( }; let src_offset_maybe = inputs[1]; - let (src_offset, 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(src_offset) => src_offset.0, + _ => panic!("ForeignCall address source 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: size as u32 }, AvmOperand::U32 { value: slot_offset as u32 }, ], ..Default::default() @@ -1047,28 +1046,26 @@ 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. The latter is not used by the AVM, but required in the oracle call so that TXE knows how many slots to read. - assert!(destinations.len() == 1); // return values + assert!(inputs.len() == 1); // output + assert!(destinations.len() == 1); // return value 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!("ForeignCall address input should be a single value"), }; let dest_offset_maybe = destinations[0]; - let (dest_offset, 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(dest_offset) => dest_offset.0, + _ => panic!("ForeignCall address destination should be a single value"), }; avm_instrs.push(AvmInstruction { opcode: AvmOpcode::SLOAD, - indirect: Some(FIRST_OPERAND_INDIRECT), + indirect: Some(ALL_DIRECT), operands: vec![ AvmOperand::U32 { value: slot_offset as u32 }, - AvmOperand::U32 { value: size as u32 }, AvmOperand::U32 { value: dest_offset as u32 }, ], ..Default::default() diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp index 43f95355e80..5cd4535a260 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp @@ -1834,7 +1834,6 @@ TEST_F(AvmExecutionTests, kernelOutputStorageLoadOpcodeSimple) + to_hex(OpCode::SLOAD) + // opcode SLOAD "00" // Indirect flag "00000001" // slot offset 1 - "00000001" // slot size 1 "00000002" // write storage value to offset 2 + to_hex(OpCode::RETURN) + // opcode RETURN "00" // Indirect flag @@ -1873,74 +1872,6 @@ TEST_F(AvmExecutionTests, kernelOutputStorageLoadOpcodeSimple) validate_trace(std::move(trace), public_inputs); } -// SLOAD -TEST_F(AvmExecutionTests, kernelOutputStorageLoadOpcodeComplex) -{ - // Sload from a value that has not previously been written to will require a hint to process - std::string bytecode_hex = to_hex(OpCode::SET) + // opcode SET - "00" // Indirect flag - "03" // U32 - "00000009" // value 9 - "00000001" // dst_offset 1 - // Cast set to field - + to_hex(OpCode::CAST) + // opcode CAST - "00" // Indirect flag - "06" // tag field - "00000001" // dst 1 - "00000001" // dst 1 - + to_hex(OpCode::SLOAD) + // opcode SLOAD - "00" // Indirect flag (second operand indirect - dest offset) - "00000001" // slot offset 1 - "00000002" // slot size 2 - "00000002" // write storage value to offset 2 - + to_hex(OpCode::RETURN) + // opcode RETURN - "00" // Indirect flag - "00000000" // ret offset 0 - "00000000"; // ret size 0 - - auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); - - ASSERT_THAT(instructions, SizeIs(4)); - - std::vector calldata = {}; - std::vector returndata = {}; - - // Generate Hint for Sload operation - // side effect counter 0 = value 42 - auto execution_hints = ExecutionHints().with_storage_value_hints({ { 0, 42 }, { 1, 123 } }); - - auto trace = Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec, execution_hints); - - // CHECK SLOAD - // Check output data + side effect counters have been set correctly - auto sload_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_sload == 1; }); - EXPECT_EQ(sload_row->main_ia, 42); // Read value - EXPECT_EQ(sload_row->main_ib, 9); // Storage slot - EXPECT_EQ(sload_row->main_side_effect_counter, 0); - sload_row++; - EXPECT_EQ(sload_row->main_ia, 123); // Read value - EXPECT_EQ(sload_row->main_ib, 10); // Storage slot - EXPECT_EQ(sload_row->main_side_effect_counter, 1); - - // Get the row of the first read storage read out - uint32_t sload_out_offset = START_SLOAD_WRITE_OFFSET; - auto sload_kernel_out_row = - std::ranges::find_if(trace.begin(), trace.end(), [&](Row r) { return r.main_clk == sload_out_offset; }); - EXPECT_EQ(sload_kernel_out_row->main_kernel_value_out, 42); // value - EXPECT_EQ(sload_kernel_out_row->main_kernel_side_effect_out, 0); - EXPECT_EQ(sload_kernel_out_row->main_kernel_metadata_out, 9); // slot - sload_kernel_out_row++; - EXPECT_EQ(sload_kernel_out_row->main_kernel_value_out, 123); // value - EXPECT_EQ(sload_kernel_out_row->main_kernel_side_effect_out, 1); - EXPECT_EQ(sload_kernel_out_row->main_kernel_metadata_out, 10); // slot - - feed_output(sload_out_offset, 42, 0, 9); - feed_output(sload_out_offset + 1, 123, 1, 10); - - validate_trace(std::move(trace), public_inputs); -} - // SSTORE TEST_F(AvmExecutionTests, kernelOutputStorageStoreOpcodeSimple) { @@ -1954,7 +1885,6 @@ TEST_F(AvmExecutionTests, kernelOutputStorageStoreOpcodeSimple) + to_hex(OpCode::SSTORE) + // opcode SSTORE "00" // Indirect flag "00000001" // src offset - "00000001" // size offset 1 "00000003" // slot offset + to_hex(OpCode::RETURN) + // opcode RETURN "00" // Indirect flag @@ -1991,68 +1921,6 @@ TEST_F(AvmExecutionTests, kernelOutputStorageStoreOpcodeSimple) validate_trace(std::move(trace), public_inputs, calldata); } -// SSTORE -TEST_F(AvmExecutionTests, kernelOutputStorageStoreOpcodeComplex) -{ - // SSTORE, write 2 elements of calldata to dstOffset 1 and 2. - std::vector calldata = { 42, 123, 9, 10 }; - std::string bytecode_hex = to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY - "00" // Indirect flag - "00000000" // cd_offset - "00000004" // copy_size - "00000001" // dst_offset, (i.e. where we store the addr) - + to_hex(OpCode::SET) + // opcode SET (inidirect SSTORE) - "00" - "03" - "00000001" // Value - "00000010" + // Dest val - to_hex(OpCode::SSTORE) + // opcode SSTORE - "01" // Indirect flag - "00000010" // src offset - "00000002" // size offset 1 - "00000003" // slot offset - + to_hex(OpCode::RETURN) + // opcode RETURN - "00" // Indirect flag - "00000000" // ret offset 0 - "00000000"; // ret size 0 - - auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); - - ASSERT_THAT(instructions, SizeIs(4)); - - std::vector returndata = {}; - - auto trace = Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec); - // CHECK SSTORE - auto sstore_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_sstore == 1; }); - EXPECT_EQ(sstore_row->main_ia, 42); // Read value - EXPECT_EQ(sstore_row->main_ib, 9); // Storage slot - EXPECT_EQ(sstore_row->main_side_effect_counter, 0); - sstore_row++; - - EXPECT_EQ(sstore_row->main_ia, 123); // Read value - EXPECT_EQ(sstore_row->main_ib, 10); // Storage slot - EXPECT_EQ(sstore_row->main_side_effect_counter, 1); - - // Get the row of the first storage write out - uint32_t sstore_out_offset = START_SSTORE_WRITE_OFFSET; - auto sstore_kernel_out_row = - std::ranges::find_if(trace.begin(), trace.end(), [&](Row r) { return r.main_clk == sstore_out_offset; }); - EXPECT_EQ(sstore_kernel_out_row->main_kernel_value_out, 42); // value - EXPECT_EQ(sstore_kernel_out_row->main_kernel_side_effect_out, 0); - EXPECT_EQ(sstore_kernel_out_row->main_kernel_metadata_out, 9); // slot - sstore_kernel_out_row++; - EXPECT_EQ(sstore_kernel_out_row->main_kernel_value_out, 123); // value - EXPECT_EQ(sstore_kernel_out_row->main_kernel_side_effect_out, 1); - EXPECT_EQ(sstore_kernel_out_row->main_kernel_metadata_out, 10); // slot - - feed_output(sstore_out_offset, 42, 0, 9); - feed_output(sstore_out_offset + 1, 123, 1, 10); - - validate_trace(std::move(trace), public_inputs, calldata); -} - // SLOAD and SSTORE TEST_F(AvmExecutionTests, kernelOutputStorageOpcodes) { @@ -2071,12 +1939,10 @@ TEST_F(AvmExecutionTests, kernelOutputStorageOpcodes) + to_hex(OpCode::SLOAD) + // opcode SLOAD "00" // Indirect flag "00000001" // slot offset 1 - "00000001" // size is 1 "00000002" // write storage value to offset 2 + to_hex(OpCode::SSTORE) + // opcode SSTORE "00" // Indirect flag "00000002" // src offset 2 (since the sload writes to 2) - "00000001" // size is 1 "00000001" // slot offset is 1 + to_hex(OpCode::RETURN) + // opcode RETURN "00" // Indirect flag diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index 3c9d8bbf49d..bade655927c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -96,8 +96,8 @@ const std::unordered_map> OPCODE_WIRE_FORMAT = { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32 } }, // Side Effects - Public Storage - { OpCode::SLOAD, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32 } }, - { OpCode::SSTORE, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32 } }, + { OpCode::SLOAD, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } }, + { OpCode::SSTORE, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } }, // Side Effects - Notes, Nullfiers, Logs, Messages { OpCode::NOTEHASHEXISTS, { OperandType::INDIRECT, diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 36e0aba65a3..fc53843c31d 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -638,14 +638,14 @@ std::vector Execution::gen_trace(std::vector const& instructio case OpCode::SLOAD: trace_builder.op_sload(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), - std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + 1, + std::get(inst.operands.at(2))); break; case OpCode::SSTORE: trace_builder.op_sstore(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), - std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + 1, + std::get(inst.operands.at(2))); break; case OpCode::NOTEHASHEXISTS: trace_builder.op_note_hash_exists(std::get(inst.operands.at(0)), diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index e53749bb3a8..57a57c05935 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -164,7 +164,11 @@ impl PublicContext { } fn raw_storage_read(_self: Self, storage_slot: Field) -> [Field; N] { - storage_read(storage_slot) + let mut out = [0; N]; + for i in 0..N { + out[i] = storage_read(storage_slot + i as Field); + } + out } fn storage_read(self, storage_slot: Field) -> T where T: Deserialize { @@ -172,7 +176,9 @@ impl PublicContext { } fn raw_storage_write(_self: Self, storage_slot: Field, values: [Field; N]) { - storage_write(storage_slot, values); + for i in 0..N { + storage_write(storage_slot + i as Field, values[i]); + } } fn storage_write(self, storage_slot: Field, value: T) where T: Serialize { @@ -272,12 +278,12 @@ unconstrained fn call_static( call_static_opcode(gas, address, args, function_selector) } -unconstrained fn storage_read(storage_slot: Field) -> [Field; N] { - storage_read_opcode(storage_slot, N as Field) +unconstrained fn storage_read(storage_slot: Field) -> Field { + storage_read_opcode(storage_slot) } -unconstrained fn storage_write(storage_slot: Field, values: [Field; N]) { - storage_write_opcode(storage_slot, values); +unconstrained fn storage_write(storage_slot: Field, value: Field) { + storage_write_opcode(storage_slot, value); } impl Empty for PublicContext { @@ -371,10 +377,10 @@ unconstrained fn call_static_opcode( // ^ return data ^ success #[oracle(avmOpcodeStorageRead)] -unconstrained fn storage_read_opcode(storage_slot: Field, length: Field) -> [Field; N] {} +unconstrained fn storage_read_opcode(storage_slot: Field) -> Field {} #[oracle(avmOpcodeStorageWrite)] -unconstrained fn storage_write_opcode(storage_slot: Field, values: [Field; N]) {} +unconstrained fn storage_write_opcode(storage_slot: Field, value: Field) {} struct FunctionReturns { values: [Field; N] 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 b6ce36a0cd8..663b079a438 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -89,7 +89,7 @@ describe('External Calls', () => { /*copySize=*/ argsSize, /*dstOffset=*/ 0, ), - new SStore(/*indirect=*/ 0, /*srcOffset=*/ valueOffset, /*size=*/ 1, /*slotOffset=*/ slotOffset), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ valueOffset, /*slotOffset=*/ slotOffset), new Return(/*indirect=*/ 0, /*retOffset=*/ 0, /*size=*/ 2), ]), ); @@ -230,7 +230,7 @@ describe('External Calls', () => { context.machineState.memory.setSlice(argsOffset, args); const otherContextInstructions: Instruction[] = [ - new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*size=*/ 0, /*slotOffset=*/ 0), + new SStore(/*indirect=*/ 0, /*srcOffset=*/ 0, /*slotOffset=*/ 0), ]; const otherContextInstructionsBytecode = markBytecodeAsAvm(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 7ddaa9cb5bb..fd8cc1e484e 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.test.ts @@ -29,15 +29,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); @@ -50,7 +44,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(persistableState.writeStorage).toHaveBeenCalledWith(address, new Fr(a.toBigInt()), new Fr(b.toBigInt())); }); @@ -67,8 +61,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(StaticCallAlterationError); }); }); @@ -79,15 +72,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); @@ -104,7 +91,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(persistableState.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 0e5cc6cdf2a..731dbaec852 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, TypeTag } from '../avm_memory_types.js'; import { StaticCallAlterationError } 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*/ size: number, slotOffset: number) { - super(indirect, srcOffset, size, slotOffset); + constructor(indirect: number, srcOffset: number, slotOffset: number) { + super(indirect, srcOffset, slotOffset); } public async execute(context: AvmContext): Promise { @@ -40,21 +32,17 @@ export class SStore extends BaseStorageInstruction { throw new StaticCallAlterationError(); } - const memoryOperations = { reads: this.size + 1, indirect: this.indirect }; + const memoryOperations = { reads: 2, indirect: this.indirect }; const memory = context.machineState.memory.track(this.type); - context.machineState.consumeGas(this.gasCost({ ...memoryOperations, dynMultiplier: this.size })); + context.machineState.consumeGas(this.gasCost({ ...memoryOperations })); const [srcOffset, slotOffset] = Addressing.fromWire(this.indirect).resolve([this.aOffset, this.bOffset], memory); memory.checkTag(TypeTag.FIELD, slotOffset); - memory.checkTagsRange(TypeTag.FIELD, srcOffset, this.size); + memory.checkTag(TypeTag.FIELD, srcOffset); const slot = memory.get(slotOffset).toFr(); - const data = 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); - } + const value = memory.get(srcOffset).toFr(); + context.persistableState.writeStorage(context.environment.storageAddress, slot, value); memory.assert(memoryOperations); context.machineState.incrementPc(); @@ -65,29 +53,21 @@ 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); } public async execute(context: AvmContext): Promise { - const memoryOperations = { writes: this.size, reads: 1, indirect: this.indirect }; + const memoryOperations = { writes: 1, reads: 1, indirect: this.indirect }; const memory = context.machineState.memory.track(this.type); - context.machineState.consumeGas(this.gasCost({ ...memoryOperations, dynMultiplier: this.size })); + context.machineState.consumeGas(this.gasCost({ ...memoryOperations })); const [slotOffset, dstOffset] = Addressing.fromWire(this.indirect).resolve([this.aOffset, this.bOffset], memory); memory.checkTag(TypeTag.FIELD, slotOffset); - const slot = memory.get(slotOffset); - - // Write each read value from storage into memory - for (let i = 0; i < this.size; i++) { - const data: Fr = await context.persistableState.readStorage( - context.environment.storageAddress, - new Fr(slot.toBigInt() + BigInt(i)), - ); - - memory.set(dstOffset + i, new Field(data)); - } + const slot = memory.get(slotOffset).toFr(); + const value = await context.persistableState.readStorage(context.environment.storageAddress, slot); + memory.set(dstOffset, new Field(value)); context.machineState.incrementPc(); memory.assert(memoryOperations); diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 96ff0160f37..3d649b5e267 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -464,28 +464,22 @@ export class TXE implements TypedOracle { throw new Error('Method not implemented.'); } - async avmOpcodeStorageRead(slot: Fr, length: Fr) { + async avmOpcodeStorageRead(slot: Fr) { const db = this.trees.asLatest(); - const result = []; + const leafSlot = computePublicDataTreeLeafSlot(this.contractAddress, slot); - for (let i = 0; i < length.toNumber(); i++) { - const leafSlot = computePublicDataTreeLeafSlot(this.contractAddress, slot.add(new Fr(i))).toBigInt(); - - const lowLeafResult = await db.getPreviousValueIndex(MerkleTreeId.PUBLIC_DATA_TREE, leafSlot); - if (!lowLeafResult || !lowLeafResult.alreadyPresent) { - result.push(Fr.ZERO); - continue; - } + const lowLeafResult = await db.getPreviousValueIndex(MerkleTreeId.PUBLIC_DATA_TREE, leafSlot.toBigInt()); + if (!lowLeafResult || !lowLeafResult.alreadyPresent) { + return Fr.ZERO; + } - const preimage = (await db.getLeafPreimage( - MerkleTreeId.PUBLIC_DATA_TREE, - lowLeafResult.index, - )) as PublicDataTreeLeafPreimage; + const preimage = (await db.getLeafPreimage( + MerkleTreeId.PUBLIC_DATA_TREE, + lowLeafResult.index, + )) as PublicDataTreeLeafPreimage; - result.push(preimage.value); - } - return result; + return preimage.value; } async storageRead( diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index a1699e7ac98..36a429e0fda 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -562,13 +562,13 @@ export class TXEService { return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); } - async avmOpcodeStorageRead(slot: ForeignCallSingle, length: ForeignCallSingle) { - const values = await (this.typedOracle as TXE).avmOpcodeStorageRead(fromSingle(slot), fromSingle(length)); - return toForeignCallResult([toArray(values)]); + async avmOpcodeStorageRead(slot: ForeignCallSingle) { + const value = await (this.typedOracle as TXE).avmOpcodeStorageRead(fromSingle(slot)); + return toForeignCallResult([toSingle(value)]); } - async avmOpcodeStorageWrite(startStorageSlot: ForeignCallSingle, values: ForeignCallArray) { - await this.typedOracle.storageWrite(fromSingle(startStorageSlot), fromArray(values)); + async avmOpcodeStorageWrite(slot: ForeignCallSingle, value: ForeignCallSingle) { + await this.typedOracle.storageWrite(fromSingle(slot), [fromSingle(value)]); return toForeignCallResult([]); }