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.

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!: getcontractinstance instruction returns only a specified member #9300

Merged
merged 5 commits into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 34 additions & 10 deletions avm-transpiler/src/transpile.rs
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
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
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
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
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
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
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
Loading