Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(avm): 1-slot sload/sstore (nr, ts) #8264

Merged
merged 1 commit into from
Aug 29, 2024
Merged

feat(avm): 1-slot sload/sstore (nr, ts) #8264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions avm-transpiler/src/transpile.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading