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(aztec-noir): align public and private execution patterns #1515

Merged
merged 20 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
7 changes: 7 additions & 0 deletions circuits/cpp/src/aztec3/circuits/abis/c_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,13 @@ WASM_EXPORT const char* abis__test_roundtrip_serialize_private_circuit_public_in
size);
}

WASM_EXPORT const char* abis__test_roundtrip_serialize_public_circuit_public_inputs(
uint8_t const* public_circuits_public_inputs_buf, uint32_t* size)
{
return as_string_output<aztec3::circuits::abis::PublicCircuitPublicInputs<NT>>(public_circuits_public_inputs_buf,
size);
}

WASM_EXPORT const char* abis__test_roundtrip_serialize_function_data(uint8_t const* function_data_buf, uint32_t* size)
{
return as_string_output<aztec3::circuits::abis::FunctionData<NT>>(function_data_buf, size);
Expand Down
1 change: 0 additions & 1 deletion circuits/cpp/src/aztec3/circuits/abis/c_bind.test.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#include "c_bind.h"

#include "function_leaf_preimage.hpp"
#include "tx_request.hpp"

Expand Down
14 changes: 14 additions & 0 deletions circuits/cpp/src/aztec3/circuits/abis/historic_block_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include <barretenberg/barretenberg.hpp>

#include <array>

namespace aztec3::circuits::abis {

using aztec3::utils::types::CircuitTypes;
Expand Down Expand Up @@ -96,6 +98,18 @@ template <typename NCT> struct HistoricBlockData {
global_variables_hash.set_public();
}

std::array<fr, 7> to_array() const
{
return { private_data_tree_root,
nullifier_tree_root,
contract_tree_root,
l1_to_l2_messages_tree_root,
blocks_tree_root, // Note private_kernel_vk_tree_root, is not included yet as
// it is not present in noir,
public_data_tree_root,
global_variables_hash };
}


fr hash()
{
Expand Down
2 changes: 1 addition & 1 deletion circuits/cpp/src/aztec3/circuits/abis/packers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ struct ConstantsPacker {
MAX_NOTES_PER_PAGE,
VIEW_NOTE_ORACLE_RETURN_LENGTH,
CALL_CONTEXT_LENGTH,
CONSTANT_HISTORIC_BLOCK_DATA_LENGTH,
HISTORIC_BLOCK_DATA_LENGTH,
FUNCTION_DATA_LENGTH,
CONTRACT_DEPLOYMENT_DATA_LENGTH,
PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH,
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come the ordering change in here? Was there a mismatch or was this to follow the structure of historic_block_data?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ive made a followup pr that does some reorg to prevent structuring mistakes like this: #1567.

It can be merged into this one or run as a standalone pr. It was a big change so Ive broken it up

Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ template <typename NCT> class PrivateCircuitPublicInputs {
fr historic_contract_tree_root = 0;
fr historic_l1_to_l2_messages_tree_root = 0;
fr historic_blocks_tree_root = 0;
fr historic_global_variables_hash = 0;
fr historic_public_data_tree_root = 0;
fr historic_global_variables_hash = 0;

ContractDeploymentData<NCT> contract_deployment_data{};

Expand All @@ -78,8 +78,8 @@ template <typename NCT> class PrivateCircuitPublicInputs {
historic_contract_tree_root,
historic_l1_to_l2_messages_tree_root,
historic_blocks_tree_root,
historic_global_variables_hash,
historic_public_data_tree_root,
historic_global_variables_hash,
contract_deployment_data,
chain_id,
version);
Expand All @@ -100,8 +100,8 @@ template <typename NCT> class PrivateCircuitPublicInputs {
historic_contract_tree_root == other.historic_contract_tree_root &&
historic_l1_to_l2_messages_tree_root == other.historic_l1_to_l2_messages_tree_root &&
historic_blocks_tree_root == other.historic_blocks_tree_root &&
historic_global_variables_hash == other.historic_global_variables_hash &&
historic_public_data_tree_root == other.historic_public_data_tree_root &&
historic_global_variables_hash == other.historic_global_variables_hash &&
contract_deployment_data == other.contract_deployment_data && chain_id == other.chain_id &&
version == other.version;
};
Expand Down Expand Up @@ -142,8 +142,8 @@ template <typename NCT> class PrivateCircuitPublicInputs {
to_ct(historic_contract_tree_root),
to_ct(historic_l1_to_l2_messages_tree_root),
to_ct(historic_blocks_tree_root),
to_ct(historic_global_variables_hash),
to_ct(historic_public_data_tree_root),
to_ct(historic_global_variables_hash),

to_circuit_type(contract_deployment_data),

Expand Down Expand Up @@ -187,8 +187,8 @@ template <typename NCT> class PrivateCircuitPublicInputs {
to_nt(historic_contract_tree_root),
to_nt(historic_l1_to_l2_messages_tree_root),
to_nt(historic_blocks_tree_root),
to_nt(historic_global_variables_hash),
to_nt(historic_public_data_tree_root),
to_nt(historic_global_variables_hash),

to_native_type(contract_deployment_data),

Expand Down Expand Up @@ -231,8 +231,8 @@ template <typename NCT> class PrivateCircuitPublicInputs {
inputs.push_back(historic_contract_tree_root);
inputs.push_back(historic_l1_to_l2_messages_tree_root);
inputs.push_back(historic_blocks_tree_root);
inputs.push_back(historic_global_variables_hash);
inputs.push_back(historic_public_data_tree_root);
inputs.push_back(historic_global_variables_hash);

inputs.push_back(contract_deployment_data.hash());

Expand Down Expand Up @@ -287,8 +287,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
opt_fr historic_contract_tree_root;
opt_fr historic_l1_to_l2_messages_tree_root;
opt_fr historic_blocks_tree_root;
opt_fr historic_global_variables_hash;
opt_fr historic_public_data_tree_root;
opt_fr historic_global_variables_hash;

std::optional<ContractDeploymentData<NCT>> contract_deployment_data;

Expand All @@ -315,8 +315,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
historic_contract_tree_root,
historic_l1_to_l2_messages_tree_root,
historic_blocks_tree_root,
historic_global_variables_hash,
historic_public_data_tree_root,
historic_global_variables_hash,
contract_deployment_data,
chain_id,
version);
Expand Down Expand Up @@ -350,8 +350,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
opt_fr const& historic_contract_tree_root,
opt_fr const& historic_l1_to_l2_messages_tree_root,
opt_fr const& historic_blocks_tree_root,
opt_fr const& historic_global_variables_hash,
opt_fr const& historic_public_data_tree_root,
opt_fr const& historic_global_variables_hash,

std::optional<ContractDeploymentData<NCT>> const& contract_deployment_data,

Expand All @@ -376,8 +376,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
, historic_contract_tree_root(historic_contract_tree_root)
, historic_l1_to_l2_messages_tree_root(historic_l1_to_l2_messages_tree_root)
, historic_blocks_tree_root(historic_blocks_tree_root)
, historic_global_variables_hash(historic_global_variables_hash)
, historic_public_data_tree_root(historic_public_data_tree_root)
, historic_global_variables_hash(historic_global_variables_hash)
, contract_deployment_data(contract_deployment_data)
, chain_id(chain_id)
, version(version){};
Expand Down Expand Up @@ -414,8 +414,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
new_inputs.historic_contract_tree_root = std::nullopt;
new_inputs.historic_l1_to_l2_messages_tree_root = std::nullopt;
new_inputs.historic_blocks_tree_root = std::nullopt;
new_inputs.historic_global_variables_hash = std::nullopt;
new_inputs.historic_public_data_tree_root = std::nullopt;
new_inputs.historic_global_variables_hash = std::nullopt;

new_inputs.contract_deployment_data = std::nullopt;

Expand Down Expand Up @@ -485,8 +485,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
make_unused_element_zero(builder, historic_contract_tree_root);
make_unused_element_zero(builder, historic_l1_to_l2_messages_tree_root);
make_unused_element_zero(builder, historic_blocks_tree_root);
make_unused_element_zero(builder, historic_global_variables_hash);
make_unused_element_zero(builder, historic_public_data_tree_root);
make_unused_element_zero(builder, historic_global_variables_hash);

make_unused_element_zero(builder, contract_deployment_data);

Expand Down Expand Up @@ -530,8 +530,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
(*historic_contract_tree_root).set_public();
(*historic_l1_to_l2_messages_tree_root).set_public();
(*historic_blocks_tree_root).set_public();
(*historic_global_variables_hash).set_public();
(*historic_public_data_tree_root).set_public();
(*historic_global_variables_hash).set_public();

(*contract_deployment_data).set_public();

Expand Down Expand Up @@ -577,8 +577,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
to_ct(historic_contract_tree_root),
to_ct(historic_l1_to_l2_messages_tree_root),
to_ct(historic_blocks_tree_root),
to_ct(historic_global_variables_hash),
to_ct(historic_public_data_tree_root),
to_ct(historic_global_variables_hash),

to_circuit_type(contract_deployment_data),

Expand Down Expand Up @@ -624,8 +624,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
to_nt(historic_contract_tree_root),
to_nt(historic_l1_to_l2_messages_tree_root),
to_nt(historic_blocks_tree_root),
to_nt(historic_global_variables_hash),
to_nt(historic_public_data_tree_root),
to_nt(historic_global_variables_hash),

to_native_type(contract_deployment_data),

Expand Down Expand Up @@ -672,8 +672,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
inputs.push_back(*historic_contract_tree_root);
inputs.push_back(*historic_l1_to_l2_messages_tree_root);
inputs.push_back(*historic_blocks_tree_root);
inputs.push_back(*historic_global_variables_hash);
inputs.push_back(*historic_public_data_tree_root);
inputs.push_back(*historic_global_variables_hash);

inputs.push_back((*contract_deployment_data).hash());

Expand Down Expand Up @@ -715,8 +715,8 @@ template <typename NCT> class OptionalPrivateCircuitPublicInputs {
.historic_contract_tree_root = historic_contract_tree_root.value(),
.historic_l1_to_l2_messages_tree_root = historic_l1_to_l2_messages_tree_root.value(),
.historic_blocks_tree_root = historic_blocks_tree_root.value(),
.historic_global_variables_hash = historic_global_variables_hash.value(),
.historic_public_data_tree_root = historic_public_data_tree_root.value(),
.historic_global_variables_hash = historic_global_variables_hash.value(),

.contract_deployment_data = contract_deployment_data.value(),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "contract_storage_update_request.hpp"
#include "../../constants.hpp"

#include "aztec3/circuits/abis/historic_block_data.hpp"
#include "aztec3/utils/msgpack_derived_output.hpp"
#include "aztec3/utils/types/circuit_types.hpp"
#include "aztec3/utils/types/native_types.hpp"
Expand Down Expand Up @@ -42,7 +43,7 @@ template <typename NCT> struct PublicCircuitPublicInputs {
// variable-length data.
fr unencrypted_log_preimages_length = 0;

fr historic_public_data_tree_root = 0;
HistoricBlockData<NCT> historic_block_data{};

address prover_address;

Expand All @@ -58,7 +59,7 @@ template <typename NCT> struct PublicCircuitPublicInputs {
new_l2_to_l1_msgs,
unencrypted_logs_hash,
unencrypted_log_preimages_length,
historic_public_data_tree_root,
historic_block_data,
prover_address);

boolean operator==(PublicCircuitPublicInputs<NCT> const& other) const
Expand Down Expand Up @@ -91,7 +92,7 @@ template <typename NCT> struct PublicCircuitPublicInputs {
.unencrypted_logs_hash = to_ct(unencrypted_logs_hash),
.unencrypted_log_preimages_length = to_ct(unencrypted_log_preimages_length),

.historic_public_data_tree_root = to_ct(historic_public_data_tree_root),
.historic_block_data = to_ct(historic_block_data),

.prover_address = to_ct(prover_address),
};
Expand Down Expand Up @@ -121,7 +122,7 @@ template <typename NCT> struct PublicCircuitPublicInputs {
spread_arr_into_vec(unencrypted_logs_hash, inputs);
inputs.push_back(unencrypted_log_preimages_length);

inputs.push_back(historic_public_data_tree_root);
spread_arr_into_vec(historic_block_data.to_array(), inputs);
inputs.push_back(prover_address);

if (inputs.size() != PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH) {
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the cpp using a different formatter? Some of these files have different formats without other stuff changing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, i think these changes came from adams commits where he was using clion as his ide

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the tidy not have been angry about it? 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question, im i suppose it exists within both rulesets

Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,17 @@ template <typename NCT, typename V> struct DefaultPrivateNotePreimage {

// To avoid messy template arguments in the calling code, we use a lambda function with `auto` return type to
// avoid explicitly having to state the circuit type for `V`.
auto circuit_value = [&]() -> auto {
auto circuit_value = [&]() -> auto
{
if constexpr (has_to_circuit_type) {
return value.to_circuit_type();
} else if (has_to_ct) {
return to_ct(value);
} else {
throw_or_abort("Can't convert Value to circuit type");
}
}();
}
();

// When this method is called, this class must be templated over native types. We can avoid templating over the
// circuit types (for the return values) (in order to make the calling syntax cleaner) with the below `decltype`
Expand All @@ -78,15 +80,17 @@ template <typename NCT, typename V> struct DefaultPrivateNotePreimage {
const bool has_to_native_type = requires(V v) { v.to_native_type(); };
const bool has_to_nt = requires(V v) { to_nt(v); };

auto native_value = [&]() -> auto {
auto native_value = [&]() -> auto
{
if constexpr (has_to_native_type) {
return value.to_native_type();
} else if (has_to_nt) {
return to_nt(value);
} else {
throw_or_abort("Can't convert Value to native type");
}
}();
}
();

DefaultPrivateNotePreimage<NativeTypes, typename decltype(native_value)::value_type> preimage = {
native_value, to_nt(owner), to_nt(creator_address), to_nt(memo), to_nt(salt), to_nt(nonce), to_nt(is_dummy),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,17 @@ template <typename NCT, typename V> struct DefaultSingletonPrivateNotePreimage {

// To avoid messy template arguments in the calling code, we use a lambda function with `auto` return type to
// avoid explicitly having to state the circuit type for `V`.
auto circuit_value = [&]() -> auto {
auto circuit_value = [&]() -> auto
{
if constexpr (has_to_circuit_type) {
return value.to_circuit_type();
} else if (has_to_ct) {
return to_ct(value);
} else {
throw_or_abort("Can't convert Value to circuit type");
}
}();
}
();

// When this method is called, this class must be templated over native types. We can avoid templating over the
// circuit types (for the return values) (in order to make the calling syntax cleaner) with the below `decltype`
Expand All @@ -78,15 +80,17 @@ template <typename NCT, typename V> struct DefaultSingletonPrivateNotePreimage {
const bool has_to_native_type = requires(V v) { v.to_native_type(); };
const bool has_to_nt = requires(V v) { to_nt(v); };

auto native_value = [&]() -> auto {
auto native_value = [&]() -> auto
{
if constexpr (has_to_native_type) {
return value.to_native_type();
} else if (has_to_nt) {
return to_nt(value);
} else {
throw_or_abort("Can't convert Value to native type");
}
}();
}
();

DefaultSingletonPrivateNotePreimage<NativeTypes, typename decltype(native_value)::value_type> preimage = {
native_value,
Expand Down
9 changes: 5 additions & 4 deletions circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,11 @@ void common_update_end_values(DummyBuilder& builder,
std::array<NT::fr, MAX_NEW_NULLIFIERS_PER_CALL> siloed_nullified_commitments{};
for (size_t i = 0; i < MAX_NEW_NULLIFIERS_PER_CALL; ++i) {
siloed_nullified_commitments[i] =
nullified_commitments[i] == fr(0) ? fr(0) // don't silo when empty
: nullified_commitments[i] == fr(EMPTY_NULLIFIED_COMMITMENT)
? fr(EMPTY_NULLIFIED_COMMITMENT) // don't silo when empty
: silo_commitment<NT>(storage_contract_address, nullified_commitments[i]);
nullified_commitments[i] == fr(0)
? fr(0) // don't silo when empty
: nullified_commitments[i] == fr(EMPTY_NULLIFIED_COMMITMENT)
? fr(EMPTY_NULLIFIED_COMMITMENT) // don't silo when empty
: silo_commitment<NT>(storage_contract_address, nullified_commitments[i]);
}

push_array_to_array(
Expand Down
13 changes: 11 additions & 2 deletions circuits/cpp/src/aztec3/circuits/kernel/public/.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,16 @@ PublicKernelInputs<NT> get_kernel_inputs_with_previous_kernel(NT::boolean privat
std::array<fr, NUM_FIELDS_PER_SHA256> const unencrypted_logs_hash =
array_of_values<NUM_FIELDS_PER_SHA256>(seed, NUM_FIELDS_PER_SHA256);
fr const unencrypted_log_preimages_length = ++seed;
fr const historic_public_data_tree_root = ++seed;
HistoricBlockData<NT> block_data = {
.private_data_tree_root = ++seed,
.nullifier_tree_root = ++seed,
.contract_tree_root = ++seed,
.l1_to_l2_messages_tree_root = ++seed,
.blocks_tree_root = ++seed,
.private_kernel_vk_tree_root = ++seed,
.public_data_tree_root = ++seed,
.global_variables_hash = ++seed,
};

// create the public circuit public inputs
auto const public_circuit_public_inputs = PublicCircuitPublicInputs<NT>{
Expand All @@ -341,7 +350,7 @@ PublicKernelInputs<NT> get_kernel_inputs_with_previous_kernel(NT::boolean privat
.new_l2_to_l1_msgs = new_l2_to_l1_msgs,
.unencrypted_logs_hash = unencrypted_logs_hash,
.unencrypted_log_preimages_length = unencrypted_log_preimages_length,
.historic_public_data_tree_root = historic_public_data_tree_root,
.historic_block_data = block_data,
};

const PublicCallStackItem call_stack_item{
Expand Down
Loading