Skip to content

Commit

Permalink
feat!: getcontractinstance instruction returns only a specified member (
Browse files Browse the repository at this point in the history 
#9300)

`GETCONTRACTINSTANCE` now takes member enum as immediate operand and
writes/returns a single field from the contract instance. Also
writes/returns a u1/bool for "exists".

Changed the trace to accept (separately) address, exists,
contractInstance since the trace generally operates on lower-level
types, not structs.

Noir has a different oracle for each enum value (similar to the `GETENV`
variations).
  • Loading branch information
@dbanks12
dbanks12 authored Oct 27, 2024
1 parent 493a3f3 commit 29b692f
Show file tree
Hide file tree
Showing 30 changed files with 844 additions and 475 deletions.
44 changes: 34 additions & 10 deletions avm-transpiler/src/transpile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,6 @@ fn handle_foreign_call(
"avmOpcodeNullifierExists" => handle_nullifier_exists(avm_instrs, destinations, inputs),
"avmOpcodeL1ToL2MsgExists" => handle_l1_to_l2_msg_exists(avm_instrs, destinations, inputs),
"avmOpcodeSendL2ToL1Msg" => handle_send_l2_to_l1_msg(avm_instrs, destinations, inputs),
"avmOpcodeGetContractInstance" => {
handle_get_contract_instance(avm_instrs, destinations, inputs);
}
"avmOpcodeCalldataCopy" => handle_calldata_copy(avm_instrs, destinations, inputs),
"avmOpcodeReturn" => handle_return(avm_instrs, destinations, inputs),
"avmOpcodeRevert" => handle_revert(avm_instrs, destinations, inputs),
Expand All @@ -408,6 +405,10 @@ fn handle_foreign_call(
_ if inputs.is_empty() && destinations.len() == 1 => {
handle_getter_instruction(avm_instrs, function, destinations, inputs);
}
// Get contract instance variations.
_ if function.starts_with("avmOpcodeGetContractInstance") => {
handle_get_contract_instance(avm_instrs, function, destinations, inputs);
}
// Anything else.
_ => panic!("Transpiler doesn't know how to process ForeignCall function {}", function),
}
Expand Down Expand Up @@ -1304,35 +1305,58 @@ fn handle_storage_write(
/// Emit a GETCONTRACTINSTANCE opcode
fn handle_get_contract_instance(
avm_instrs: &mut Vec<AvmInstruction>,
function: &str,
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
enum ContractInstanceMember {
DEPLOYER,
CLASS_ID,
INIT_HASH,
}

assert!(inputs.len() == 1);
assert!(destinations.len() == 1);
assert!(destinations.len() == 2);

let member_idx = match function {
"avmOpcodeGetContractInstanceDeployer" => ContractInstanceMember::DEPLOYER,
"avmOpcodeGetContractInstanceClassId" => ContractInstanceMember::CLASS_ID,
"avmOpcodeGetContractInstanceInitializationHash" => ContractInstanceMember::INIT_HASH,
_ => panic!("Transpiler doesn't know how to process function {:?}", function),
};

let address_offset_maybe = inputs[0];
let address_offset = match address_offset_maybe {
ValueOrArray::MemoryAddress(slot_offset) => slot_offset,
ValueOrArray::MemoryAddress(offset) => offset,
_ => panic!("GETCONTRACTINSTANCE address should be a single value"),
};

let dest_offset_maybe = destinations[0];
let dest_offset = match dest_offset_maybe {
ValueOrArray::HeapArray(HeapArray { pointer, .. }) => pointer,
_ => panic!("GETCONTRACTINSTANCE destination should be an array"),
ValueOrArray::MemoryAddress(offset) => offset,
_ => panic!("GETCONTRACTINSTANCE dst destination should be a single value"),
};

let exists_offset_maybe = destinations[1];
let exists_offset = match exists_offset_maybe {
ValueOrArray::MemoryAddress(offset) => offset,
_ => panic!("GETCONTRACTINSTANCE exists destination should be a single value"),
};

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::GETCONTRACTINSTANCE,
indirect: Some(
AddressingModeBuilder::default()
.direct_operand(&address_offset)
.indirect_operand(&dest_offset)
.direct_operand(&dest_offset)
.direct_operand(&exists_offset)
.build(),
),
operands: vec![
AvmOperand::U32 { value: address_offset.to_usize() as u32 },
AvmOperand::U32 { value: dest_offset.to_usize() as u32 },
AvmOperand::U8 { value: member_idx as u8 },
AvmOperand::U16 { value: address_offset.to_usize() as u16 },
AvmOperand::U16 { value: dest_offset.to_usize() as u16 },
AvmOperand::U16 { value: exists_offset.to_usize() as u16 },
],
..Default::default()
});
Expand Down
3 changes: 1 addition & 2 deletions barretenberg/cpp/pil/avm/main.pil
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,8 @@ namespace main(256);
// op_err * (sel_op_fdiv + sel_op_XXX + ... - 1) == 0
// Note that the above is even a stronger constraint, as it shows
// that exactly one sel_op_XXX must be true.
// At this time, we have only division producing an error.
#[SUBOP_ERROR_RELEVANT_OP]
op_err * ((sel_op_fdiv + sel_op_div) - 1) = 0;
op_err * ((sel_op_fdiv + sel_op_div + sel_op_get_contract_instance) - 1) = 0;

// TODO: constraint that we stop execution at the first error (tag_err or op_err)
// An error can only happen at the last sub-operation row.
Expand Down
4 changes: 3 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/main.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,9 @@ template <typename FF_> class mainImpl {
}
{
using Accumulator = typename std::tuple_element_t<79, ContainerOverSubrelations>;
auto tmp = (new_term.main_op_err * ((new_term.main_sel_op_fdiv + new_term.main_sel_op_div) - FF(1)));
auto tmp = (new_term.main_op_err * (((new_term.main_sel_op_fdiv + new_term.main_sel_op_div) +
new_term.main_sel_op_get_contract_instance) -
FF(1)));
tmp *= scaling_factor;
std::get<79>(evals) += typename Accumulator::View(tmp);
}
Expand Down
155 changes: 115 additions & 40 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 @@ -1245,7 +1245,7 @@ TEST_F(AvmExecutionTests, msmOpCode)
}

// Positive test for Kernel Input opcodes
TEST_F(AvmExecutionTests, kernelInputOpcodes)
TEST_F(AvmExecutionTests, getEnvOpcode)
{
std::string bytecode_hex =
to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
Expand Down Expand Up @@ -1497,6 +1497,32 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
validate_trace(std::move(trace), convert_public_inputs(public_inputs_vec), calldata, returndata);
}

// TODO(9395): allow this intruction to raise error flag in main.pil
// TEST_F(AvmExecutionTests, getEnvOpcodeBadEnum)
//{
// std::string bytecode_hex =
// to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
// "00" // Indirect flag
// + to_hex(static_cast<uint8_t>(EnvironmentVariable::MAX_ENV_VAR)) + // envvar ADDRESS
// "0001"; // dst_offset
//
// auto bytecode = hex_to_bytes(bytecode_hex);
// auto instructions = Deserialization::parse(bytecode);
//
// // Public inputs for the circuit
// std::vector<FF> calldata;
// std::vector<FF> returndata;
// ExecutionHints execution_hints;
// auto trace = gen_trace(bytecode, calldata, public_inputs_vec, returndata, execution_hints);
//
// // Bad enum should raise error flag
// auto address_row =
// std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_address == 1; });
// EXPECT_EQ(address_row->main_op_err, FF(1));
//
// validate_trace(std::move(trace), convert_public_inputs(public_inputs_vec), calldata, returndata);
//}

// Positive test for L2GASLEFT opcode
TEST_F(AvmExecutionTests, l2GasLeft)
{
Expand Down Expand Up @@ -2110,43 +2136,10 @@ TEST_F(AvmExecutionTests, opCallOpcodes)
validate_trace(std::move(trace), public_inputs, calldata, returndata);
}

TEST_F(AvmExecutionTests, opGetContractInstanceOpcodes)
TEST_F(AvmExecutionTests, opGetContractInstanceOpcode)
{
std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U32) +
"00" // val
"00" // dst_offset
+ to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U32) +
"01" // val
"01" +
to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY for addr
"00" // Indirect flag
"0000" // cd_offset
"0001" // copy_size
"0001" // dst_offset, (i.e. where we store the addr)
+ to_hex(OpCode::SET_8) + // opcode SET for the indirect dst offset
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U32) +
"03" // val i
"02" + // dst_offset 2
to_hex(OpCode::GETCONTRACTINSTANCE) + // opcode CALL
"02" // Indirect flag
"00000001" // address offset
"00000002" // dst offset
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"0003" // ret offset 3
"0006"; // ret size 6

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

FF address = 10;
std::vector<FF> calldata = { address };
std::vector<FF> returndata = {};
const uint8_t address_byte = 0x42;
const FF address(address_byte);

// Generate Hint for call operation
// Note: opcode does not write 'address' into memory
Expand All @@ -2158,14 +2151,96 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcodes)
grumpkin::g1::affine_element::random_element(),
grumpkin::g1::affine_element::random_element(),
};
auto execution_hints =
ExecutionHints().with_contract_instance_hints({ { address, { address, 1, 2, 3, 4, 5, public_keys_hints } } });
const ContractInstanceHint instance = ContractInstanceHint{
.address = address,
.exists = true,
.salt = 2,
.deployer_addr = 42,
.contract_class_id = 66,
.initialisation_hash = 99,
.public_keys = public_keys_hints,
};
auto execution_hints = ExecutionHints().with_contract_instance_hints({ { address, instance } });

std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U8) + to_hex(address_byte) + // val
"01" // dst_offset 0
+ to_hex(OpCode::GETCONTRACTINSTANCE) + // opcode GETCONTRACTINSTANCE
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(ContractInstanceMember::DEPLOYER)) + // member enum
"0001" // address offset
"0010" // dst offset
"0011" // exists offset
+ to_hex(OpCode::GETCONTRACTINSTANCE) + // opcode GETCONTRACTINSTANCE
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(ContractInstanceMember::CLASS_ID)) + // member enum
"0001" // address offset
"0012" // dst offset
"0013" // exists offset
+ to_hex(OpCode::GETCONTRACTINSTANCE) + // opcode GETCONTRACTINSTANCE
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(ContractInstanceMember::INIT_HASH)) + // member enum
"0001" // address offset
"0014" // dst offset
"0015" // exists offset
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"0010" // ret offset 1
"0006"; // ret size 6 (dst & exists for all 3)

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

ASSERT_THAT(instructions, SizeIs(5));

std::vector<FF> const calldata{};
// alternating member value, exists bool
std::vector<FF> const expected_returndata = {
instance.deployer_addr, 1, instance.contract_class_id, 1, instance.initialisation_hash, 1,
};

std::vector<FF> returndata{};
auto trace = gen_trace(bytecode, calldata, public_inputs_vec, returndata, execution_hints);
EXPECT_EQ(returndata, std::vector<FF>({ 1, 2, 3, 4, 5, returned_point.x })); // The first one represents true

validate_trace(std::move(trace), public_inputs, calldata, returndata);

// Validate returndata
EXPECT_EQ(returndata, expected_returndata);
}

TEST_F(AvmExecutionTests, opGetContractInstanceOpcodeBadEnum)
{
const uint8_t address_byte = 0x42;
const FF address(address_byte);

std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
+ to_hex(AvmMemoryTag::U8) + to_hex(address_byte) + // val
"01" // dst_offset 0
+ to_hex(OpCode::GETCONTRACTINSTANCE) + // opcode GETCONTRACTINSTANCE
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(ContractInstanceMember::MAX_MEMBER)) + // member enum
"0001" // address offset
"0010" // dst offset
"0011"; // exists offset

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

std::vector<FF> calldata;
std::vector<FF> returndata;
ExecutionHints execution_hints;
auto trace = gen_trace(bytecode, calldata, public_inputs_vec, returndata, execution_hints);

// Bad enum should raise error flag
auto address_row = std::ranges::find_if(
trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_get_contract_instance == 1; });
EXPECT_EQ(address_row->main_op_err, FF(1));

validate_trace(std::move(trace), public_inputs, calldata, returndata);
}

// Negative test detecting an invalid opcode byte.
TEST_F(AvmExecutionTests, invalidOpcode)
{
Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
OperandType::UINT16,
/*TODO: leafIndexOffset is not constrained*/ OperandType::UINT16,
OperandType::UINT16 } },
{ OpCode::GETCONTRACTINSTANCE, { OperandType::INDIRECT8, OperandType::UINT32, OperandType::UINT32 } },
{ OpCode::GETCONTRACTINSTANCE,
{ OperandType::INDIRECT8, OperandType::UINT8, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16 } },
{ OpCode::EMITUNENCRYPTEDLOG,
{
OperandType::INDIRECT8,
Expand Down
6 changes: 4 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,8 +604,10 @@ std::vector<Row> Execution::gen_trace(std::vector<FF> const& calldata,
break;
case OpCode::GETCONTRACTINSTANCE:
trace_builder.op_get_contract_instance(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<uint8_t>(inst.operands.at(1)),
std::get<uint16_t>(inst.operands.at(2)),
std::get<uint16_t>(inst.operands.at(3)),
std::get<uint16_t>(inst.operands.at(4)));
break;

// Accrued Substate
Expand Down
8 changes: 8 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ enum class EnvironmentVariable {
MAX_ENV_VAR
};

enum class ContractInstanceMember {
DEPLOYER,
CLASS_ID,
INIT_HASH,
// sentinel
MAX_MEMBER,
};

class Bytecode {
public:
static bool is_valid(uint8_t byte);
Expand Down
Loading

0 comments on commit 29b692f

Please sign in to comment.