Skip to content

Commit

Permalink
feat: Kernel fix & test improvements/refactoring (#239)
Browse files Browse the repository at this point in the history
* fix bug in native kernel - contract logic was being skipped

* zero-initialize all abi and kernel structs. helper function to zero-init arrays of frs

* better struct initialization in app circuits files

* dont need to do as much in base utils since structs are sanely zero-initialized

* use fill to initialize array of fields to zero

* fix native private kernel, refactor private kernel tests, add real logic in test setup for computing function/contract tree roots/siblings

* new hash functions
  • Loading branch information
dbanks12 authored Apr 13, 2023
1 parent d155b90 commit 5ae96a0
Show file tree
Hide file tree
Showing 9 changed files with 509 additions and 942 deletions.
8 changes: 7 additions & 1 deletion cpp/cmake/module.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ function(barretenberg_module MODULE_NAME)
WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
else()
# Currently haven't found a way to easily wrap the calls in wasmtime when run from ctest.
gtest_discover_tests(${MODULE_NAME}_tests WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
# Needed to add `TEST_DISCOVERY_TIMEOUT` to work around:
# ```
# Error running test executable.
# ...
# Result: Process terminated due to timeout
# ```
gtest_discover_tests(${MODULE_NAME}_tests WORKING_DIRECTORY ${CMAKE_BINARY_DIR} PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
endif()
endif()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ namespace aztec3::circuits::apps::test_apps::basic_contract_deployment {
using aztec3::circuits::abis::OptionalPrivateCircuitPublicInputs;

OptionalPrivateCircuitPublicInputs<NT> constructor(FunctionExecutionContext& exec_ctx,
NT::fr const& _arg0,
NT::fr const& _arg1,
NT::fr const& _arg2)
std::array<NT::fr, ARGS_LENGTH> const& args)
{
/****************************************************************
* PREAMBLE
Expand All @@ -24,9 +22,9 @@ OptionalPrivateCircuitPublicInputs<NT> constructor(FunctionExecutionContext& exe
// Convert params into circuit types:
auto& composer = exec_ctx.composer;

CT::fr arg0 = to_ct(composer, _arg0);
CT::fr arg1 = to_ct(composer, _arg1);
CT::fr arg2 = to_ct(composer, _arg2);
CT::fr arg0 = to_ct(composer, args[0]);
CT::fr arg1 = to_ct(composer, args[1]);
CT::fr arg2 = to_ct(composer, args[2]);

auto& oracle = exec_ctx.oracle;
const CT::address msg_sender = oracle.get_msg_sender();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ namespace aztec3::circuits::apps::test_apps::basic_contract_deployment {
using aztec3::circuits::abis::OptionalPrivateCircuitPublicInputs;

OptionalPrivateCircuitPublicInputs<NT> constructor(FunctionExecutionContext& exec_ctx,
NT::fr const& _arg0,
NT::fr const& _arg1,
NT::fr const& _arg2);
std::array<NT::fr, ARGS_LENGTH> const& args);

} // namespace aztec3::circuits::apps::test_apps::basic_contract_deployment
2 changes: 1 addition & 1 deletion cpp/src/aztec3/circuits/apps/test_apps/escrow/.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ TEST_F(escrow_tests, test_deposit)
auto asset_id = NT::fr(1);
auto memo = NT::fr(999);

auto result = deposit(exec_ctx, amount, asset_id, memo);
auto result = deposit(exec_ctx, { amount, asset_id, memo });
info("result: ", result);

info("computed witness: ", composer.computed_witness);
Expand Down
10 changes: 4 additions & 6 deletions cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ namespace aztec3::circuits::apps::test_apps::escrow {
using aztec3::circuits::abis::OptionalPrivateCircuitPublicInputs;

OptionalPrivateCircuitPublicInputs<NT> deposit(FunctionExecutionContext& exec_ctx,
NT::fr const& _amount,
NT::fr const& _asset_id,
NT::fr const& _memo)
std::array<NT::fr, ARGS_LENGTH> const& args)
{
/****************************************************************
* PREAMBLE
Expand All @@ -25,9 +23,9 @@ OptionalPrivateCircuitPublicInputs<NT> deposit(FunctionExecutionContext& exec_ct
// Convert params into circuit types:
auto& composer = exec_ctx.composer;

CT::fr amount = to_ct(composer, _amount);
CT::fr asset_id = to_ct(composer, _asset_id);
CT::fr memo = to_ct(composer, _memo);
CT::fr amount = to_ct(composer, args[0]);
CT::fr asset_id = to_ct(composer, args[1]);
CT::fr memo = to_ct(composer, args[2]);

auto& oracle = exec_ctx.oracle;
const CT::address msg_sender = oracle.get_msg_sender();
Expand Down
4 changes: 1 addition & 3 deletions cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ namespace aztec3::circuits::apps::test_apps::escrow {
using aztec3::circuits::abis::OptionalPrivateCircuitPublicInputs;

OptionalPrivateCircuitPublicInputs<NT> deposit(FunctionExecutionContext& exec_ctx,
NT::fr const& _amount,
NT::fr const& _asset_id,
NT::fr const& _memo);
std::array<NT::fr, ARGS_LENGTH> const& args);

} // namespace aztec3::circuits::apps::test_apps::escrow
107 changes: 100 additions & 7 deletions cpp/src/aztec3/circuits/hash.hpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
#include <array>
#include <aztec3/circuits/abis/function_data.hpp>
#include "aztec3/circuits/abis/function_leaf_preimage.hpp"
#include <aztec3/circuits/abis/private_kernel/new_contract_data.hpp>
#include <aztec3/constants.hpp>

namespace aztec3::circuits {

using abis::FunctionData;
using aztec3::circuits::abis::FunctionLeafPreimage;
using aztec3::circuits::abis::private_kernel::ContractLeafPreimage;

template <typename NCT> typename NCT::fr compute_args_hash(std::array<typename NCT::fr, ARGS_LENGTH> args)
{
Expand Down Expand Up @@ -87,20 +92,108 @@ typename NCT::fr add_contract_address_to_nullifier(typename NCT::address contrac
* @param siblingPath The nodes representing the merkle siblings of the leaf, its parent,
* the next parent, etc up to the sibling below the root
* @return The computed Merkle tree root.
*
* TODO need to use conditional assigns instead of `ifs` for circuit version.
* see membership.hpp:check_subtree_membership (left/right/conditional_assign, etc)
*/
template <typename NCT, size_t N>
typename NCT::fr root_from_sibling_path(typename NCT::fr leaf,
typename NCT::uint32 leafIndex,
std::array<typename NCT::fr, N> siblingPath)
typename NCT::fr root_from_sibling_path(typename NCT::fr const& leaf,
typename NCT::uint32 const& leafIndex,
std::array<typename NCT::fr, N> const& siblingPath)
{
for (size_t i = 0; i < siblingPath.size(); i++) {
auto node = leaf;
for (size_t i = 0; i < N; i++) {
if (leafIndex & (1 << i)) {
leaf = NCT::merkle_hash(siblingPath[i], leaf);
node = NCT::merkle_hash(siblingPath[i], node);
} else {
leaf = NCT::merkle_hash(leaf, siblingPath[i]);
node = NCT::merkle_hash(node, siblingPath[i]);
}
}
return leaf; // root
return node; // root
}

/**
* @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 is_private in leaf preimage
* @param vk_hash in leaf preimage
* @param acir_hash in leaf preimage
* @param function_leaf_index leaf index in the function tree
* @param function_leaf_sibling_path
* @return NCT::fr
*/
template <typename NCT>
typename NCT::fr function_tree_root_from_siblings(
typename NCT::fr const& function_selector,
typename NCT::boolean const& is_private,
typename NCT::fr const& vk_hash,
typename NCT::fr const& acir_hash,
typename NCT::uint32 const& function_leaf_index,
std::array<typename NCT::fr, FUNCTION_TREE_HEIGHT> const& function_leaf_sibling_path)
{
const auto function_leaf_preimage = FunctionLeafPreimage<NCT>{
.function_selector = function_selector,
.is_private = is_private,
.vk_hash = vk_hash,
.acir_hash = acir_hash,
};

const auto function_leaf = function_leaf_preimage.hash();

auto function_tree_root =
root_from_sibling_path<NCT>(function_leaf, function_leaf_index, function_leaf_sibling_path);
return function_tree_root;
}

/**
* @brief Calculate the contract tree root from the sibling path and leaf preimage.
*
* @tparam NCT (native or circuit)
* @param function_tree_root in leaf preimage
* @param storage_contract_address in leaf preimage
* @param portal_contract_address in leaf preimage
* @param contract_leaf_index leaf index in the function tree
* @param contract_leaf_sibling_path
* @return NCT::fr
*/
template <typename NCT>
typename NCT::fr contract_tree_root_from_siblings(
typename NCT::fr const& function_tree_root,
typename NCT::address const& storage_contract_address,
typename NCT::address const& portal_contract_address,
typename NCT::uint32 const& contract_leaf_index,
std::array<typename NCT::fr, CONTRACT_TREE_HEIGHT> const& contract_leaf_sibling_path)
{
const ContractLeafPreimage<NCT> contract_leaf_preimage{ storage_contract_address,
portal_contract_address,
function_tree_root };

const auto contract_leaf = contract_leaf_preimage.hash();

const auto computed_contract_tree_root =
root_from_sibling_path<NCT>(contract_leaf, contract_leaf_index, contract_leaf_sibling_path);
return computed_contract_tree_root;
}

/**
* @brief Compute sibling path for an empty tree.
*
* @tparam NCT (native or circuit)
* @tparam TREE_HEIGHT
* @param zero_leaf the leaf value that corresponds to a zero preimage
* @return std::array<typename NCT::fr, TREE_HEIGHT>
*/
template <typename NCT, size_t TREE_HEIGHT>
std::array<typename NCT::fr, TREE_HEIGHT> compute_empty_sibling_path(typename NCT::fr const& zero_leaf)
{
std::array<typename NCT::fr, TREE_HEIGHT> sibling_path = { zero_leaf };
for (size_t i = 1; i < TREE_HEIGHT; i++) {
// hash previous sibling with itself to get node above
sibling_path[i] = NCT::merkle_hash(sibling_path[i - 1], sibling_path[i - 1]);
}
return sibling_path;
}

} // namespace aztec3::circuits
Loading

0 comments on commit 5ae96a0

Please sign in to comment.