Skip to content

Commit

Permalink
chore: rework nonces (#1210) (#1331)
Browse files Browse the repository at this point in the history 
# Description

The way nonces work now, there can be inconsistencies in nonce
assignment in the simulator vs the private kernel. Furthermore, you
cannot know during function execution what the full set of commitments
will be for the whole TX as some new commitments may be nullified and
squashed. But we still want the ability to determine nonces and
therefore uniqueNoteHashes from L1 calldata alone. I am sure I am not
explaining all of the issues well enough, but it was determined that the
current nonce paradigm will not work and therefore we must rework it.

Rework nonces so that siloing by contract address happens first and
uniqueness comes later. For now, nonces are injeced by the private
ordering circuit (vs suggestion which was base rollup circuit). Pending
notes and their reads have no nonces when processed in kernel. The
public kernel (and therefore all commitments created in public
functions) does not use nonces.

Here was Mike's proposal for the rework:

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/7b20c886-1e92-452c-a886-c3da5ed64e17)

Why not just use leaf index as nonce?

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/e6337107-ac93-4a3b-b83c-27213cb5133d)

## Followup tasks
* #1029
* #1194
* #1329
* #1407
* #1408
* #1409
* #1410
* Future enhancement: The root rollup circuit could insert all messages
at the very beginning of the root rollup circuit, so that txs within the
rollup can refer to that state root and read L1>L2 messages immediately.
* #1383
* #1386
* We should implement subscription / polling methods for Aztec logs
* We should maybe write rpc functions which allow calldata to be
subscribed-to, keyed by tx_hash.
* If a dapp wants to write a note from a public function, a lot of honus
will be on a dapp developer to retain preimage information, query the
blockchain, and derive the nonce. We should provide some examples to
demonstrate this pattern.
  • Loading branch information
@dbanks12
dbanks12 authored Aug 3, 2023
1 parent bb38c7a commit 665cb75
Show file tree
Hide file tree
Showing 47 changed files with 710 additions and 461 deletions.
4 changes: 2 additions & 2 deletions circuits/cpp/src/aztec3/circuits/abis/packers.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ struct GeneratorIndexPacker {
int COMMITMENT = GeneratorIndex::COMMITMENT;
int COMMITMENT_NONCE = GeneratorIndex::COMMITMENT_NONCE;
int UNIQUE_COMMITMENT = GeneratorIndex::UNIQUE_COMMITMENT;
int OUTER_COMMITMENT = GeneratorIndex::OUTER_COMMITMENT;
int SILOED_COMMITMENT = GeneratorIndex::SILOED_COMMITMENT;
int NULLIFIER = GeneratorIndex::NULLIFIER;
int INITIALISATION_NULLIFIER = GeneratorIndex::INITIALISATION_NULLIFIER;
int OUTER_NULLIFIER = GeneratorIndex::OUTER_NULLIFIER;
Expand Down Expand Up @@ -141,7 +141,7 @@ struct GeneratorIndexPacker {
pack(NVP(COMMITMENT,
COMMITMENT_NONCE,
UNIQUE_COMMITMENT,
OUTER_COMMITMENT,
SILOED_COMMITMENT,
NULLIFIER,
INITIALISATION_NULLIFIER,
OUTER_NULLIFIER,
Expand Down
14 changes: 7 additions & 7 deletions circuits/cpp/src/aztec3/circuits/hash.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,29 +100,29 @@ typename NCT::fr compute_commitment_nonce(typename NCT::fr first_nullifier, type
}

template <typename NCT>
typename NCT::fr compute_unique_commitment(typename NCT::fr nonce, typename NCT::fr inner_commitment)
typename NCT::fr silo_commitment(typename NCT::address contract_address, typename NCT::fr inner_commitment)
{
using fr = typename NCT::fr;

std::vector<fr> const inputs = {
nonce,
contract_address.to_field(),
inner_commitment,
};

return NCT::hash(inputs, aztec3::GeneratorIndex::UNIQUE_COMMITMENT);
return NCT::hash(inputs, aztec3::GeneratorIndex::SILOED_COMMITMENT);
}

template <typename NCT>
typename NCT::fr silo_commitment(typename NCT::address contract_address, typename NCT::fr commitment)
typename NCT::fr compute_unique_commitment(typename NCT::fr nonce, typename NCT::fr siloed_commitment)
{
using fr = typename NCT::fr;

std::vector<fr> const inputs = {
contract_address.to_field(),
commitment,
nonce,
siloed_commitment,
};

return NCT::hash(inputs, aztec3::GeneratorIndex::OUTER_COMMITMENT);
return NCT::hash(inputs, aztec3::GeneratorIndex::UNIQUE_COMMITMENT);
}

template <typename NCT>
Expand Down
8 changes: 4 additions & 4 deletions circuits/cpp/src/aztec3/circuits/kernel/private/c_bind.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
namespace {
using Builder = UltraCircuitBuilder;
using NT = aztec3::utils::types::NativeTypes;
using DummyBuilder = aztec3::utils::DummyCircuitBuilder;
using DummyCircuitBuilder = aztec3::utils::DummyCircuitBuilder;
using aztec3::circuits::abis::PreviousKernelData;
using aztec3::circuits::abis::TxRequest;
using aztec3::circuits::abis::private_kernel::PrivateCallData;
Expand Down Expand Up @@ -67,7 +67,7 @@ WASM_EXPORT uint8_t* private_kernel__sim_init(uint8_t const* tx_request_buf,
size_t* private_kernel_public_inputs_size_out,
uint8_t const** private_kernel_public_inputs_buf)
{
DummyBuilder builder = DummyBuilder("private_kernel__sim_init");
DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_init");

PrivateCallData<NT> private_call_data;
read(private_call_buf, private_call_data);
Expand Down Expand Up @@ -98,7 +98,7 @@ WASM_EXPORT uint8_t* private_kernel__sim_inner(uint8_t const* previous_kernel_bu
size_t* private_kernel_public_inputs_size_out,
uint8_t const** private_kernel_public_inputs_buf)
{
DummyBuilder builder = DummyBuilder("private_kernel__sim_inner");
DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_inner");
PrivateCallData<NT> private_call_data;
read(private_call_buf, private_call_data);

Expand All @@ -124,7 +124,7 @@ WASM_EXPORT uint8_t* private_kernel__sim_inner(uint8_t const* previous_kernel_bu
}

CBIND(private_kernel__sim_ordering, [](PreviousKernelData<NT> previous_kernel) {
DummyBuilder builder = DummyBuilder("private_kernel__sim_ordering");
DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_ordering");
auto const& public_inputs = native_private_kernel_circuit_ordering(builder, previous_kernel);
return builder.result_or_error(public_inputs);
});
92 changes: 29 additions & 63 deletions circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "aztec3/circuits/abis/function_data.hpp"
#include "aztec3/circuits/abis/kernel_circuit_public_inputs.hpp"
#include "aztec3/circuits/abis/new_contract_data.hpp"
#include "aztec3/circuits/abis/previous_kernel_data.hpp"
#include "aztec3/circuits/abis/private_kernel/private_call_data.hpp"
#include "aztec3/circuits/abis/read_request_membership_witness.hpp"
#include "aztec3/circuits/hash.hpp"
Expand All @@ -21,7 +20,6 @@ using aztec3::circuits::abis::ContractLeafPreimage;
using aztec3::circuits::abis::FunctionData;
using aztec3::circuits::abis::KernelCircuitPublicInputs;
using aztec3::circuits::abis::NewContractData;
using aztec3::circuits::abis::PreviousKernelData;
using aztec3::circuits::abis::ReadRequestMembershipWitness;

using aztec3::utils::array_length;
Expand Down Expand Up @@ -61,15 +59,14 @@ void common_validate_call_stack(DummyBuilder& builder, PrivateCallData<NT> const
* - https://discourse.aztec.network/t/spending-notes-which-havent-yet-been-inserted/180
*
* @param builder
* @param storage_contract_address Contract address to use when siloing read requests
* @param historic_private_data_tree_root This is a reference to the historic root which all
* read requests are checked against here.
* @param read_requests the commitments being read by this private call
* @param read_requests the commitments being read by this private call - 'pending note reads' here are
* `inner_note_hashes` (not yet siloed, not unique), but 'pre-existing note reads' are `unique_siloed_note_hashes`
* @param read_request_membership_witnesses used to compute the private data root
* for a given request which is essentially a membership check
*/
void common_validate_read_requests(DummyBuilder& builder,
NT::fr const& storage_contract_address,
NT::fr const& historic_private_data_tree_root,
std::array<fr, MAX_READ_REQUESTS_PER_CALL> const& read_requests,
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>,
Expand All @@ -89,41 +86,40 @@ void common_validate_read_requests(DummyBuilder& builder,
// for every request in all kernel iterations
for (size_t rr_idx = 0; rr_idx < aztec3::MAX_READ_REQUESTS_PER_CALL; rr_idx++) {
const auto& read_request = read_requests[rr_idx];
// the read request comes un-siloed from the app circuit so we must silo it here
// so that it matches the private data tree leaf that we are membership checking
const auto leaf = silo_commitment<NT>(storage_contract_address, read_request);
const auto& witness = read_request_membership_witnesses[rr_idx];

// A pending commitment is the one that is not yet added to private data tree
// A transient read is when we try to "read" a pending commitment
// We determine if it is a transient read depending on the leaf index from the membership witness
// Note that the Merkle membership proof would be null and void in case of an transient read
// but we use the leaf index as a placeholder to detect a transient read.
// but we use the leaf index as a placeholder to detect a 'pending note read'.
if (read_request != 0 && !witness.is_transient) {
const auto& root_for_read_request =
root_from_sibling_path<NT>(leaf, witness.leaf_index, witness.sibling_path);
builder.do_assert(root_for_read_request == historic_private_data_tree_root,
format("private data tree root mismatch at read_request[",
rr_idx,
"]",
"\n\texpected root: ",
historic_private_data_tree_root,
"\n\tbut got root*: ",
root_for_read_request,
"\n\tread_request: ",
read_request,
"\n\tsiloed-rr (leaf): ",
leaf,
"\n\tleaf_index: ",
witness.leaf_index,
"\n\tis_transient: ",
witness.is_transient,
"\n\thint_to_commitment: ",
witness.hint_to_commitment,
"\n\t* got root by siloing read_request (compressing with "
"storage_contract_address to get leaf) "
"and merkle-hashing to a root using membership witness"),
CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
root_from_sibling_path<NT>(read_request, witness.leaf_index, witness.sibling_path);
builder.do_assert(
root_for_read_request == historic_private_data_tree_root,
format("private data tree root mismatch at read_request[",
rr_idx,
"]",
"\n\texpected root: ",
historic_private_data_tree_root,
"\n\tbut got root*: ",
root_for_read_request,
"\n\tread_request**: ",
read_request,
"\n\tleaf_index: ",
witness.leaf_index,
"\n\tis_transient: ",
witness.is_transient,
"\n\thint_to_commitment: ",
witness.hint_to_commitment,
"\n\t* got root by treating the read_request as a leaf in the private data tree "
"and merkle-hashing to a root using the membership witness"
"\n\t** for 'pre-existing note reads', the read_request is the unique_siloed_note_hash "
"(it has been hashed with contract address and then a nonce)"),
CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/1354): do we need to enforce
// that a non-transient read_request was derived from the proper/current contract address?
}
}
}
Expand Down Expand Up @@ -226,13 +222,9 @@ void common_update_end_values(DummyBuilder& builder,

// commitments
std::array<NT::fr, MAX_NEW_COMMITMENTS_PER_CALL> siloed_new_commitments{};
const auto& first_nullifier = public_inputs.end.new_nullifiers[0];
const auto index_start = array_length(public_inputs.end.new_commitments);
for (size_t i = 0; i < new_commitments.size(); ++i) {
const auto nonce = compute_commitment_nonce<NT>(first_nullifier, index_start + i);
const auto unique_commitment = compute_unique_commitment<NT>(nonce, new_commitments[i]);
siloed_new_commitments[i] =
new_commitments[i] == 0 ? 0 : silo_commitment<NT>(storage_contract_address, unique_commitment);
new_commitments[i] == 0 ? 0 : silo_commitment<NT>(storage_contract_address, new_commitments[i]);
}
push_array_to_array(
builder,
Expand Down Expand Up @@ -406,30 +398,4 @@ void common_contract_logic(DummyBuilder& builder,
}
}

void common_inner_ordering_initialise_end_values(PreviousKernelData<NT> const& previous_kernel,
KernelCircuitPublicInputs<NT>& public_inputs)
{
public_inputs.constants = previous_kernel.public_inputs.constants;

// Ensure the arrays are the same as previously, before we start pushing more data onto them in other
// functions within this circuit:
auto& end = public_inputs.end;
const auto& start = previous_kernel.public_inputs.end;

end.new_commitments = start.new_commitments;
end.new_nullifiers = start.new_nullifiers;

end.private_call_stack = start.private_call_stack;
end.public_call_stack = start.public_call_stack;
end.new_l2_to_l1_msgs = start.new_l2_to_l1_msgs;

end.encrypted_logs_hash = start.encrypted_logs_hash;
end.unencrypted_logs_hash = start.unencrypted_logs_hash;

end.encrypted_log_preimages_length = start.encrypted_log_preimages_length;
end.unencrypted_log_preimages_length = start.unencrypted_log_preimages_length;

end.optionally_revealed_data = start.optionally_revealed_data;
}

} // namespace aztec3::circuits::kernel::private_kernel
6 changes: 0 additions & 6 deletions circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "aztec3/circuits/abis/contract_deployment_data.hpp"
#include "aztec3/circuits/abis/function_data.hpp"
#include "aztec3/circuits/abis/kernel_circuit_public_inputs.hpp"
#include "aztec3/circuits/abis/previous_kernel_data.hpp"
#include "aztec3/circuits/abis/private_kernel/private_call_data.hpp"
#include "aztec3/circuits/abis/read_request_membership_witness.hpp"
#include "aztec3/utils/dummy_circuit_builder.hpp"
Expand All @@ -17,7 +16,6 @@ using aztec3::circuits::abis::ContractDeploymentData;
using DummyBuilder = aztec3::utils::DummyCircuitBuilder;
using aztec3::circuits::abis::FunctionData;
using aztec3::circuits::abis::KernelCircuitPublicInputs;
using aztec3::circuits::abis::PreviousKernelData;
using aztec3::circuits::abis::ReadRequestMembershipWitness;
using aztec3::circuits::abis::private_kernel::PrivateCallData;

Expand All @@ -26,7 +24,6 @@ using aztec3::circuits::abis::private_kernel::PrivateCallData;
void common_validate_call_stack(DummyBuilder& builder, PrivateCallData<NT> const& private_call);

void common_validate_read_requests(DummyBuilder& builder,
NT::fr const& storage_contract_address,
NT::fr const& historic_private_data_tree_root,
std::array<fr, MAX_READ_REQUESTS_PER_CALL> const& read_requests,
std::array<ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>,
Expand All @@ -48,7 +45,4 @@ void common_contract_logic(DummyBuilder& builder,
ContractDeploymentData<NT> const& contract_dep_data,
FunctionData<NT> const& function_data);

void common_inner_ordering_initialise_end_values(PreviousKernelData<NT> const& previous_kernel,
KernelCircuitPublicInputs<NT>& public_inputs);

} // namespace aztec3::circuits::kernel::private_kernel
16 changes: 4 additions & 12 deletions circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,8 @@ TEST_F(native_private_kernel_tests, native_accumulate_transient_read_requests)
{
auto private_inputs_init = do_private_call_get_kernel_inputs_init(false, deposit, standard_test_args());

auto first_nullifier = private_inputs_init.tx_request.hash();

private_inputs_init.private_call.call_stack_item.public_inputs.new_commitments[0] = fr(12);
private_inputs_init.private_call.call_stack_item.public_inputs.read_requests[0] =
compute_unique_commitment<NT>(compute_commitment_nonce<NT>(first_nullifier, 1), fr(23));
private_inputs_init.private_call.call_stack_item.public_inputs.read_requests[0] = fr(23);
private_inputs_init.private_call.read_request_membership_witnesses[0].is_transient = true;

DummyBuilder builder = DummyBuilder("native_private_kernel_tests__native_accumulate_transient_read_requests");
Expand All @@ -64,8 +61,7 @@ TEST_F(native_private_kernel_tests, native_accumulate_transient_read_requests)
auto private_inputs_inner = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());

private_inputs_inner.private_call.call_stack_item.public_inputs.new_commitments[0] = fr(23);
private_inputs_inner.private_call.call_stack_item.public_inputs.read_requests[0] =
compute_unique_commitment<NT>(compute_commitment_nonce<NT>(first_nullifier, 0), fr(12));
private_inputs_inner.private_call.call_stack_item.public_inputs.read_requests[0] = fr(12);
private_inputs_inner.private_call.read_request_membership_witnesses[0].is_transient = true;

// The original call is not multi-iterative (call stack depth == 1) and we re-feed the same private call stack
Expand Down Expand Up @@ -101,11 +97,8 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match)
{
auto private_inputs_init = do_private_call_get_kernel_inputs_init(false, deposit, standard_test_args());

auto first_nullifier = private_inputs_init.tx_request.hash();

private_inputs_init.private_call.call_stack_item.public_inputs.new_commitments[0] = fr(10);
private_inputs_init.private_call.call_stack_item.public_inputs.read_requests[0] =
compute_unique_commitment<NT>(compute_commitment_nonce<NT>(first_nullifier, 1), fr(23));
private_inputs_init.private_call.call_stack_item.public_inputs.read_requests[0] = fr(23);
private_inputs_init.private_call.read_request_membership_witnesses[0].is_transient = true;

DummyBuilder builder = DummyBuilder("native_private_kernel_tests__native_transient_read_requests_no_match");
Expand All @@ -120,8 +113,7 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match)
auto private_inputs_inner = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());

private_inputs_inner.private_call.call_stack_item.public_inputs.new_commitments[0] = fr(23);
private_inputs_inner.private_call.call_stack_item.public_inputs.read_requests[0] =
compute_unique_commitment<NT>(compute_commitment_nonce<NT>(first_nullifier, 0), fr(12));
private_inputs_inner.private_call.call_stack_item.public_inputs.read_requests[0] = fr(12);
private_inputs_inner.private_call.read_request_membership_witnesses[0].is_transient = true;

// The original call is not multi-iterative (call stack depth == 1) and we re-feed the same private call stack
Expand Down
Loading

0 comments on commit 665cb75

Please sign in to comment.