Skip to content

Commit

Permalink
feat(avm)!: returndatasize + returndatacopy (#9475)
Browse files Browse the repository at this point in the history
This PR
* Introduces RETURNDATASIZE and RETURNDATACOPY (also copies revert data if reverted)
* Fixes a bug in CALL in witgen
* Changes the public context to return slices when calling public functions. This was partly done because templated functions would always be inlined and I wanted to avoid that blowup

Note that the rethrowing hack is still present in the simulator, so the rethrowing branch in the public context is still not used. I will make this change once @sirasistant finishes the string encoding changes.

In a later PR we can remove the returndata from CALL.

Part of #9061.
  • Loading branch information
fcarreiro authored Oct 29, 2024
1 parent a34d4aa commit 8f71006
Show file tree
Hide file tree
Showing 41 changed files with 1,301 additions and 868 deletions.
4 changes: 4 additions & 0 deletions avm-transpiler/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub enum AvmOpcode {
// Execution environment
GETENVVAR_16,
CALLDATACOPY,
RETURNDATASIZE,
RETURNDATACOPY,
// Control flow
JUMP_16,
JUMPI_16,
Expand Down Expand Up @@ -123,6 +125,8 @@ impl AvmOpcode {
AvmOpcode::GETENVVAR_16 => "GETENVVAR_16",
// Execution Environment - Calldata
AvmOpcode::CALLDATACOPY => "CALLDATACOPY",
AvmOpcode::RETURNDATASIZE => "RETURNDATASIZE",
AvmOpcode::RETURNDATACOPY => "RETURNDATACOPY",

// Machine State
// Machine State - Internal Control Flow
Expand Down
91 changes: 90 additions & 1 deletion avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ fn handle_foreign_call(
"avmOpcodeL1ToL2MsgExists" => handle_l1_to_l2_msg_exists(avm_instrs, destinations, inputs),
"avmOpcodeSendL2ToL1Msg" => handle_send_l2_to_l1_msg(avm_instrs, destinations, inputs),
"avmOpcodeCalldataCopy" => handle_calldata_copy(avm_instrs, destinations, inputs),
"avmOpcodeReturndataSize" => handle_returndata_size(avm_instrs, destinations, inputs),
"avmOpcodeReturndataCopy" => handle_returndata_copy(avm_instrs, destinations, inputs),
"avmOpcodeReturn" => handle_return(avm_instrs, destinations, inputs),
"avmOpcodeRevert" => handle_revert(avm_instrs, destinations, inputs),
"avmOpcodeStorageRead" => handle_storage_read(avm_instrs, destinations, inputs),
Expand Down Expand Up @@ -1217,6 +1219,87 @@ fn handle_calldata_copy(
});
}

// #[oracle(avmOpcodeReturndataSize)]
// unconstrained fn returndata_size_opcode() -> u32 {}
fn handle_returndata_size(
avm_instrs: &mut Vec<AvmInstruction>,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
assert!(inputs.len() == 0);
assert!(destinations.len() == 1);

let dest_offset = match destinations[0] {
ValueOrArray::MemoryAddress(address) => address,
_ => panic!("ReturndataSize destination should be a memory location"),
};

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::RETURNDATASIZE,
indirect: Some(AddressingModeBuilder::default().direct_operand(&dest_offset).build()),
operands: vec![AvmOperand::U16 { value: dest_offset.to_usize() as u16 }],
..Default::default()
});
}

// #[oracle(avmOpcodeReturndataCopy)]
// unconstrained fn returndata_copy_opcode(rdoffset: u32, copy_size: u32) -> [Field] {}
fn handle_returndata_copy(
avm_instrs: &mut Vec<AvmInstruction>,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
assert!(inputs.len() == 2);
assert!(destinations.len() == 2);

let cd_offset = match inputs[0] {
ValueOrArray::MemoryAddress(address) => address,
_ => panic!("ReturndataCopy offset should be a memory address"),
};

let copy_size_offset = match inputs[1] {
ValueOrArray::MemoryAddress(address) => address,
_ => panic!("ReturndataCopy size should be a memory address"),
};

// We skip the first destination, which is the size of the slice.
let (dest_offset, write_size_here_offset) = match destinations[1] {
ValueOrArray::HeapVector(HeapVector { pointer, size }) => (pointer, size),
_ => panic!("ReturndataCopy destination should be a vector (slice)"),
};

avm_instrs.extend([
// First we write the return data.
AvmInstruction {
opcode: AvmOpcode::RETURNDATACOPY,
indirect: Some(
AddressingModeBuilder::default()
.direct_operand(&cd_offset)
.direct_operand(&copy_size_offset)
.indirect_operand(&dest_offset)
.build(),
),
operands: vec![
AvmOperand::U16 { value: cd_offset.to_usize() as u16 },
AvmOperand::U16 { value: copy_size_offset.to_usize() as u16 },
AvmOperand::U16 { value: dest_offset.to_usize() as u16 },
],
..Default::default()
},
// Then we set the size of the slice, using the input size.
generate_mov_instruction(
Some(
AddressingModeBuilder::default()
.direct_operand(&copy_size_offset)
.direct_operand(&write_size_here_offset)
.build(),
),
copy_size_offset.to_usize() as u32,
write_size_here_offset.to_usize() as u32,
),
]);
}

// #[oracle(avmOpcodeReturn)]
// unconstrained fn return_opcode<let N: u32>(returndata: [Field; N]) {}
fn handle_return(
Expand Down Expand Up @@ -1309,6 +1392,7 @@ fn handle_get_contract_instance(
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
#[allow(non_camel_case_types)]
enum ContractInstanceMember {
DEPLOYER,
CLASS_ID,
Expand Down Expand Up @@ -1444,7 +1528,7 @@ pub fn patch_assert_message_pcs(
/// This must be done before transpiling to properly transpile jump destinations.
/// This is necessary for two reasons:
/// 1. The transpiler injects `initial_offset` instructions at the beginning of the program.
/// 2. Some brillig instructions (_e.g._ Stop, or certain ForeignCalls) map to multiple AVM instructions
/// 2. Some brillig instructions map to multiple AVM instructions
/// args:
/// initial_offset: how many AVM instructions were inserted at the start of the program
/// brillig: the Brillig program
Expand All @@ -1456,6 +1540,11 @@ pub fn map_brillig_pcs_to_avm_pcs(brillig_bytecode: &[BrilligOpcode<FieldElement
pc_map[0] = 0; // first PC is always 0 as there are no instructions inserted by AVM at start
for i in 0..brillig_bytecode.len() - 1 {
let num_avm_instrs_for_this_brillig_instr = match &brillig_bytecode[i] {
BrilligOpcode::ForeignCall { function, .. }
if function == "avmOpcodeReturndataCopy" =>
{
2
}
_ => 1,
};
// next Brillig pc will map to an AVM pc offset by the
Expand Down
8 changes: 7 additions & 1 deletion barretenberg/cpp/pil/avm/main.pil
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ namespace main(256);

//===== Memory Slice Gadget Selectors =========================================
pol commit sel_op_calldata_copy;
pol commit sel_op_returndata_size;
pol commit sel_op_returndata_copy;
pol commit sel_op_external_return;
pol commit sel_op_external_revert;

Expand Down Expand Up @@ -275,6 +277,8 @@ namespace main(256);
sel_op_static_call * (1 - sel_op_static_call) = 0;

sel_op_calldata_copy * (1 - sel_op_calldata_copy) = 0;
sel_op_returndata_size * (1 - sel_op_returndata_size) = 0;
sel_op_returndata_copy * (1 - sel_op_returndata_copy) = 0;
sel_op_external_return * (1 - sel_op_external_return) = 0;
sel_op_external_revert * (1 - sel_op_external_revert) = 0;

Expand Down Expand Up @@ -409,7 +413,8 @@ namespace main(256);

//===== CONTROL_FLOW_CONSISTENCY ============================================
pol SEL_ALL_CTRL_FLOW = sel_op_jump + sel_op_jumpi + sel_op_internal_call
+ sel_op_internal_return + sel_op_external_call + sel_op_static_call + sel_op_external_return;
+ sel_op_internal_return + sel_op_external_call + sel_op_static_call
+ sel_op_external_return + sel_op_external_revert;
pol SEL_ALU_R_TAG = sel_op_add + sel_op_sub + sel_op_mul + sel_op_div + sel_op_not + sel_op_eq
+ sel_op_lt + sel_op_lte + sel_op_shr + sel_op_shl;
pol SEL_ALU_W_TAG = sel_op_cast;
Expand All @@ -420,6 +425,7 @@ namespace main(256);
+ sel_op_ecadd + sel_op_msm;
pol SEL_ALL_MEMORY = sel_op_mov + sel_op_set;
pol OPCODE_SELECTORS = sel_op_fdiv + sel_op_calldata_copy + sel_op_get_contract_instance
+ sel_op_returndata_size + sel_op_returndata_copy
+ SEL_ALL_ALU + SEL_ALL_BINARY + SEL_ALL_MEMORY + SEL_ALL_GADGET
+ KERNEL_INPUT_SELECTORS + KERNEL_OUTPUT_SELECTORS + SEL_ALL_LEFTGAS
+ SEL_ALL_CTRL_FLOW;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ AvmCircuitBuilder::ProverPolynomials AvmCircuitBuilder::compute_polynomials() co
polys.main_sel_op_or.set_if_valid_index(i, rows[i].main_sel_op_or);
polys.main_sel_op_poseidon2.set_if_valid_index(i, rows[i].main_sel_op_poseidon2);
polys.main_sel_op_radix_be.set_if_valid_index(i, rows[i].main_sel_op_radix_be);
polys.main_sel_op_returndata_copy.set_if_valid_index(i, rows[i].main_sel_op_returndata_copy);
polys.main_sel_op_returndata_size.set_if_valid_index(i, rows[i].main_sel_op_returndata_size);
polys.main_sel_op_sender.set_if_valid_index(i, rows[i].main_sel_op_sender);
polys.main_sel_op_set.set_if_valid_index(i, rows[i].main_sel_op_set);
polys.main_sel_op_sha256.set_if_valid_index(i, rows[i].main_sel_op_sha256);
Expand Down
Loading

0 comments on commit 8f71006

Please sign in to comment.