Skip to content

Commit

Permalink
feat(avm): 1-slot sload/sstore (nr, ts) (#8264)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@fcarreiro
fcarreiro authored Aug 29, 2024
1 parent 51d6699 commit bdd9b06
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 239 deletions.
25 changes: 11 additions & 14 deletions avm-transpiler/src/transpile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -1047,28 +1046,26 @@ fn handle_storage_read(
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
// 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()
Expand Down
134 changes: 0 additions & 134 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<FF> calldata = {};
std::vector<FF> 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)
{
Expand All @@ -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
Expand Down Expand Up @@ -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<FF> 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<FF> 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)
{
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ const std::unordered_map<OpCode, std::vector<OperandType>> 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,
Expand Down
8 changes: 4 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,14 +638,14 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
case OpCode::SLOAD:
trace_builder.op_sload(std::get<uint8_t>(inst.operands.at(0)),
std::get<uint32_t>(inst.operands.at(1)),
std::get<uint32_t>(inst.operands.at(2)),
std::get<uint32_t>(inst.operands.at(3)));
1,
std::get<uint32_t>(inst.operands.at(2)));
break;
case OpCode::SSTORE:
trace_builder.op_sstore(std::get<uint8_t>(inst.operands.at(0)),
std::get<uint32_t>(inst.operands.at(1)),
std::get<uint32_t>(inst.operands.at(2)),
std::get<uint32_t>(inst.operands.at(3)));
1,
std::get<uint32_t>(inst.operands.at(2)));
break;
case OpCode::NOTEHASHEXISTS:
trace_builder.op_note_hash_exists(std::get<uint8_t>(inst.operands.at(0)),
Expand Down
22 changes: 14 additions & 8 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,21 @@ impl PublicContext {
}

fn raw_storage_read<let N: u32>(_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<T, let N: u32>(self, storage_slot: Field) -> T where T: Deserialize<N> {
T::deserialize(self.raw_storage_read(storage_slot))
}

fn raw_storage_write<let N: u32>(_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<T, let N: u32>(self, storage_slot: Field, value: T) where T: Serialize<N> {
Expand Down Expand Up @@ -272,12 +278,12 @@ unconstrained fn call_static<let RET_SIZE: u32>(
call_static_opcode(gas, address, args, function_selector)
}

unconstrained fn storage_read<let N: u32>(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<let N: u32>(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 {
Expand Down Expand Up @@ -371,10 +377,10 @@ unconstrained fn call_static_opcode<let RET_SIZE: u32>(
// ^ return data ^ success

#[oracle(avmOpcodeStorageRead)]
unconstrained fn storage_read_opcode<let N: u32>(storage_slot: Field, length: Field) -> [Field; N] {}
unconstrained fn storage_read_opcode(storage_slot: Field) -> Field {}

#[oracle(avmOpcodeStorageWrite)]
unconstrained fn storage_write_opcode<let N: u32>(storage_slot: Field, values: [Field; N]) {}
unconstrained fn storage_write_opcode(storage_slot: Field, value: Field) {}

struct FunctionReturns<let N: u32> {
values: [Field; N]
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]),
);
Expand Down Expand Up @@ -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));
Expand Down
23 changes: 5 additions & 18 deletions yarn-project/simulator/src/avm/opcodes/storage.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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()));
});
Expand All @@ -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);
});
});
Expand All @@ -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);
Expand All @@ -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()));

Expand Down
Loading

0 comments on commit bdd9b06

Please sign in to comment.