Skip to content

Commit

Permalink
fix: selector name regression (#1800)
Browse files Browse the repository at this point in the history
I introduced a regression in my function [selector type
PR](#1518) which
caused the selector name to be incorrect in circuits.gen.ts. The issue
was with having different names for selector in FunctionData struct in
TS and C++.

This PR fixes it.
  • Loading branch information
benesjan authored and dan-aztec committed Aug 25, 2023
1 parent 2281810 commit e116b27
Show file tree
Hide file tree
Showing 21 changed files with 107 additions and 111 deletions.
8 changes: 4 additions & 4 deletions circuits/cpp/src/aztec3/circuits/abis/.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ TEST(abi_tests, native_read_write_call_context)
TEST(abi_tests, native_read_write_function_data)
{
FunctionData<NT> const function_data = {
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 11,
},
.is_private = false,
Expand All @@ -70,8 +70,8 @@ TEST(abi_tests, native_read_write_function_data)
TEST(abi_tests, native_to_circuit_function_data)
{
FunctionData<NT> const native_function_data = {
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 11,
},
.is_private = false,
Expand Down
10 changes: 5 additions & 5 deletions circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ TEST(abi_tests, hash_tx_request)
EXPECT_EQ(got_hash, tx_request.hash());
}

TEST(abi_tests, compute_function_selector_transfer)
TEST(abi_tests, compute_selector_transfer)
{
const char* function_signature = "transfer(address,uint256)";

Expand Down Expand Up @@ -191,8 +191,8 @@ TEST(abi_tests, compute_function_leaf)
{
// Construct FunctionLeafPreimage with some randomized fields
auto const preimage = FunctionLeafPreimage<NT>{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = engine.get_random_uint32(),
},
.is_private = static_cast<bool>(engine.get_random_uint8() & 1),
Expand Down Expand Up @@ -282,8 +282,8 @@ TEST(abi_tests, compute_function_tree)
TEST(abi_tests, hash_constructor)
{
// Randomize required values
auto const func_data = FunctionData<NT>{ .function_selector =
FunctionSelector<NT>{
auto const func_data = FunctionData<NT>{ .selector =
{
.value = 10,
},
.is_private = true,
Expand Down
16 changes: 8 additions & 8 deletions circuits/cpp/src/aztec3/circuits/abis/function_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@ template <typename NCT> struct FunctionData {
using boolean = typename NCT::boolean;
using fr = typename NCT::fr;

FunctionSelector<NCT> function_selector;
FunctionSelector<NCT> selector;
boolean is_internal = false;
boolean is_private = false;
boolean is_constructor = false;

MSGPACK_FIELDS(function_selector, is_internal, is_private, is_constructor);
MSGPACK_FIELDS(selector, is_internal, is_private, is_constructor);

boolean operator==(FunctionData<NCT> const& other) const
{
return function_selector == other.function_selector && is_internal == other.is_internal &&
is_private == other.is_private && is_constructor == other.is_constructor;
return selector == other.selector && is_internal == other.is_internal && is_private == other.is_private &&
is_constructor == other.is_constructor;
};

template <typename Builder> FunctionData<CircuitTypes<Builder>> to_circuit_type(Builder& builder) const
Expand All @@ -39,7 +39,7 @@ template <typename NCT> struct FunctionData {
auto to_ct = [&](auto& e) { return aztec3::utils::types::to_ct(builder, e); };

FunctionData<CircuitTypes<Builder>> function_data = {
function_selector.to_circuit_type(builder),
selector.to_circuit_type(builder),
to_ct(is_internal),
to_ct(is_private),
to_ct(is_constructor),
Expand All @@ -55,7 +55,7 @@ template <typename NCT> struct FunctionData {
auto to_nt = [&](auto& e) { return aztec3::utils::types::to_nt<Builder>(e); };

FunctionData<NativeTypes> function_data = {
to_native_type(function_selector),
to_native_type(selector),
to_nt(is_internal),
to_nt(is_private),
to_nt(is_constructor),
Expand All @@ -68,7 +68,7 @@ template <typename NCT> struct FunctionData {
{
static_assert(!(std::is_same<NativeTypes, NCT>::value));

function_selector.set_public();
selector.set_public();
fr(is_internal).set_public();
fr(is_private).set_public();
fr(is_constructor).set_public();
Expand All @@ -78,7 +78,7 @@ template <typename NCT> struct FunctionData {
fr hash() const
{
std::vector<fr> const inputs = {
fr(function_selector.value),
fr(selector.value),
fr(is_internal),
fr(is_private),
fr(is_constructor),
Expand Down
22 changes: 9 additions & 13 deletions circuits/cpp/src/aztec3/circuits/abis/function_leaf_preimage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ using std::is_same;
* Templated on NativeTypes/CircuitTypes.
*
* @details A FunctionLeafPreimage contains:
* - `function_selector` keccak hash of function signature truncated to NUM_FUNCTION_SELECTOR_BYTES
* - `selector` keccak hash of function signature truncated to NUM_FUNCTION_SELECTOR_BYTES
* - `is_private` boolean flag
* - `vk_hash` pedersen hash of the function verification key
* - `acir_hash` hash of the function's acir bytecode
Expand All @@ -32,19 +32,19 @@ template <typename NCT> struct FunctionLeafPreimage {
using fr = typename NCT::fr;
using uint32 = typename NCT::uint32;

FunctionSelector<NCT> function_selector = {};
FunctionSelector<NCT> selector = {};
boolean is_internal = false;
boolean is_private = false;
fr vk_hash = 0;
fr acir_hash = 0;

// For serialization, update with new fields
MSGPACK_FIELDS(function_selector, is_internal, is_private, vk_hash, acir_hash);
MSGPACK_FIELDS(selector, is_internal, is_private, vk_hash, acir_hash);

boolean operator==(FunctionLeafPreimage<NCT> const& other) const
{
return function_selector == other.function_selector && is_internal == other.is_internal &&
is_private == other.is_private && vk_hash == other.vk_hash && acir_hash == other.acir_hash;
return selector == other.selector && is_internal == other.is_internal && is_private == other.is_private &&
vk_hash == other.vk_hash && acir_hash == other.acir_hash;
};

template <typename Builder> FunctionLeafPreimage<CircuitTypes<Builder>> to_circuit_type(Builder& builder) const
Expand All @@ -55,11 +55,7 @@ template <typename NCT> struct FunctionLeafPreimage {
auto to_ct = [&](auto& e) { return aztec3::utils::types::to_ct(builder, e); };

FunctionLeafPreimage<CircuitTypes<Builder>> preimage = {
function_selector.to_circuit_type(builder),
to_ct(is_internal),
to_ct(is_private),
to_ct(vk_hash),
to_ct(acir_hash),
selector.to_circuit_type(builder), to_ct(is_internal), to_ct(is_private), to_ct(vk_hash), to_ct(acir_hash),
};

return preimage;
Expand All @@ -72,7 +68,7 @@ template <typename NCT> struct FunctionLeafPreimage {
auto to_nt = [&](auto& e) { return aztec3::utils::types::to_nt<Builder>(e); };

FunctionLeafPreimage<NativeTypes> preimage = {
to_native_type(function_selector), to_nt(is_internal), to_nt(is_private), to_nt(vk_hash), to_nt(acir_hash),
to_native_type(selector), to_nt(is_internal), to_nt(is_private), to_nt(vk_hash), to_nt(acir_hash),
};

return preimage;
Expand All @@ -82,7 +78,7 @@ template <typename NCT> struct FunctionLeafPreimage {
{
static_assert(!(std::is_same<NativeTypes, NCT>::value));

function_selector.set_public();
selector.set_public();
fr(is_internal).set_public();
fr(is_private).set_public();
vk_hash.set_public();
Expand All @@ -92,7 +88,7 @@ template <typename NCT> struct FunctionLeafPreimage {
fr hash() const
{
std::vector<fr> const inputs = {
function_selector.value, fr(is_internal), fr(is_private), vk_hash, acir_hash,
selector.value, fr(is_internal), fr(is_private), vk_hash, acir_hash,
};
return NCT::compress(inputs, GeneratorIndex::FUNCTION_LEAF);
}
Expand Down
8 changes: 4 additions & 4 deletions circuits/cpp/src/aztec3/circuits/abis/function_selector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,23 @@ template <typename NCT> struct FunctionSelector {
// Capture the circuit builder:
auto to_ct = [&](auto& e) { return aztec3::utils::types::to_ct(builder, e); };

FunctionSelector<CircuitTypes<Builder>> function_selector = {
FunctionSelector<CircuitTypes<Builder>> selector = {
to_ct(value),
};

return function_selector;
return selector;
};

template <typename Builder> FunctionSelector<NativeTypes> to_native_type() const
{
static_assert(std::is_same<CircuitTypes<Builder>, NCT>::value);
auto to_nt = [&](auto& e) { return aztec3::utils::types::to_nt<Builder>(e); };

FunctionSelector<NativeTypes> function_selector = {
FunctionSelector<NativeTypes> selector = {
to_nt(value),
};

return function_selector;
return selector;
};

void set_public()
Expand Down
4 changes: 2 additions & 2 deletions circuits/cpp/src/aztec3/circuits/apps/.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ class state_var_tests : public ::testing::Test {
uint256_t(0x01071e9a23e0f7edULL, 0x5d77b35d1830fa3eULL, 0xc6ba3660bb1f0c0bULL, 0x2ef9f7f09867fd6eULL));

FunctionData<NT> const function_data{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 1, // TODO: deduce this from the contract, somehow.
},
.is_private = true,
Expand Down
4 changes: 2 additions & 2 deletions circuits/cpp/src/aztec3/circuits/apps/contract.tpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ template <typename NCT> void Contract<NCT>::set_functions(std::vector<FunctionDe
throw_or_abort("Name already exists");
}
function_datas[function.name] = FunctionData<NCT>{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = static_cast<uint32>(i),
},
.is_private = function.is_private,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ template <typename Builder> class FunctionExecutionContext {

const FunctionData<CT> f_function_data_ct{
// Note: we MUST
.function_selector =
FunctionSelector<CT>{
.selector =
{
.value = f_encoding_ct,
},
.is_private = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class escrow_tests : public ::testing::Test {
uint256_t(0x01071e9a23e0f7edULL, 0x5d77b35d1830fa3eULL, 0xc6ba3660bb1f0c0bULL, 0x2ef9f7f09867fd6eULL));

FunctionData<NT> const function_data{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 1, // TODO: deduce this from the contract, somehow.
},
.is_private = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ TEST(private_to_private_function_call_tests, circuit_private_to_private_function
uint256_t(0x01071e9a23e0f7edULL, 0x5d77b35d1830fa3eULL, 0xc6ba3660bb1f0c0bULL, 0x2ef9f7f09867fd6eULL);

const FunctionData<NT> function_data{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 1, // TODO: deduce this from the contract, somehow.
},
.is_private = true,
Expand Down
6 changes: 3 additions & 3 deletions circuits/cpp/src/aztec3/circuits/hash.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ void check_membership(Builder& builder,
* @brief Calculate the function tree root from the sibling path and leaf preimage.
*
* @tparam NCT (native or circuit)
* @param function_selector in leaf preimage
* @param selector in leaf preimage
* @param is_internal in leaf preimage
* @param is_private in leaf preimage
* @param vk_hash in leaf preimage
Expand All @@ -305,7 +305,7 @@ void check_membership(Builder& builder,
* @return NCT::fr
*/
template <typename NCT> typename NCT::fr function_tree_root_from_siblings(
FunctionSelector<NCT> const& function_selector,
FunctionSelector<NCT> const& selector,
typename NCT::boolean const& is_internal,
typename NCT::boolean const& is_private,
typename NCT::fr const& vk_hash,
Expand All @@ -314,7 +314,7 @@ template <typename NCT> typename NCT::fr function_tree_root_from_siblings(
std::array<typename NCT::fr, FUNCTION_TREE_HEIGHT> const& function_leaf_sibling_path)
{
const auto function_leaf_preimage = FunctionLeafPreimage<NCT>{
.function_selector = function_selector,
.selector = selector,
.is_internal = is_internal,
.is_private = is_private,
.vk_hash = vk_hash,
Expand Down
2 changes: 1 addition & 1 deletion circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ void common_contract_logic(DummyBuilder& builder,

// The logic below ensures that the contract exists in the contracts tree
auto const& computed_function_tree_root =
function_tree_root_from_siblings<NT>(private_call.call_stack_item.function_data.function_selector,
function_tree_root_from_siblings<NT>(private_call.call_stack_item.function_data.selector,
private_call.call_stack_item.function_data.is_internal,
true, // is_private
private_call_vk_hash,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ TEST_F(native_private_kernel_init_tests, contract_deployment_function_data_misma
auto private_inputs = do_private_call_get_kernel_inputs_init(true, constructor, standard_test_args());

// Modify the function selector in function data.
private_inputs.tx_request.function_data.function_selector = FunctionSelector<NT>{
private_inputs.tx_request.function_data.selector = {
.value = numeric::random::get_engine().get_random_uint32(),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ std::pair<PrivateCallData<NT>, ContractDeploymentData<NT>> create_private_call_d
const Point<NT> msg_sender_pub_key = { .x = 123456789, .y = 123456789 };

FunctionData<NT> const function_data{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 1, // TODO: deduce this from the contract, somehow.
},
.is_private = true,
Expand Down Expand Up @@ -203,7 +203,7 @@ std::pair<PrivateCallData<NT>, ContractDeploymentData<NT>> create_private_call_d
// push to array/vector
// use variation of `compute_root_partial_left_tree` to compute the root from leaves
// const auto& function_leaf_preimage = FunctionLeafPreimage<NT>{
// .function_selector = function_data.function_selector,
// .selector = function_data.selector,
// .is_private = function_data.is_private,
// .vk_hash = private_circuit_vk_hash,
// .acir_hash = acir_hash,
Expand All @@ -230,7 +230,7 @@ std::pair<PrivateCallData<NT>, ContractDeploymentData<NT>> create_private_call_d
// update the contract address in the call context now that it is known
call_context.storage_contract_address = contract_address;
} else {
const NT::fr& function_tree_root = function_tree_root_from_siblings<NT>(function_data.function_selector,
const NT::fr& function_tree_root = function_tree_root_from_siblings<NT>(function_data.selector,
function_data.is_internal,
function_data.is_private,
private_circuit_vk_hash,
Expand Down
10 changes: 5 additions & 5 deletions circuits/cpp/src/aztec3/circuits/kernel/public/.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ PublicCallStackItem generate_call_stack_item(NT::fr contract_address,
{
NT::uint32 count = seed + 1;
FunctionData<NT> const function_data{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = count,
},
.is_private = false,
Expand Down Expand Up @@ -267,8 +267,8 @@ PublicKernelInputs<NT> get_kernel_inputs_with_previous_kernel(NT::boolean privat
const NT::address msg_sender = NT::fr(1);

FunctionData<NT> const function_data{
.function_selector =
FunctionSelector<NT>{
.selector =
{
.value = 1,
},
.is_private = false,
Expand Down Expand Up @@ -646,7 +646,7 @@ TEST(public_kernel_tests, function_selector_must_be_valid)
DummyBuilder dummyBuilder = DummyBuilder("public_kernel_tests__function_selector_must_be_valid");
PublicKernelInputs<NT> inputs = get_kernel_inputs_with_previous_kernel(true);

inputs.public_call.call_stack_item.function_data.function_selector = FunctionSelector<NT>{
inputs.public_call.call_stack_item.function_data.selector = {
.value = 0,
};
auto public_inputs = native_public_kernel_circuit_private_previous_kernel(dummyBuilder, inputs);
Expand Down
2 changes: 1 addition & 1 deletion circuits/cpp/src/aztec3/circuits/kernel/public/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ void common_validate_inputs(DummyBuilder& builder, KernelInput const& public_ker
builder.do_assert(this_call_stack_item.contract_address != 0,
"Contract address must be non-zero",
CircuitErrorCode::PUBLIC_KERNEL__CONTRACT_ADDRESS_INVALID);
builder.do_assert(this_call_stack_item.function_data.function_selector.value != 0,
builder.do_assert(this_call_stack_item.function_data.selector.value != 0,
"Function signature must be non-zero",
CircuitErrorCode::PUBLIC_KERNEL__FUNCTION_SIGNATURE_INVALID);
builder.do_assert(this_call_stack_item.function_data.is_constructor == false,
Expand Down
Loading

0 comments on commit e116b27

Please sign in to comment.