diff --git a/cspell.json b/cspell.json index 082a14997a23..dc2183f8f52e 100644 --- a/cspell.json +++ b/cspell.json @@ -374,6 +374,7 @@ "unshift", "unshifted", "unsiloed", + "unsquashable", "unsynched", "unvalidated", "unzipit", diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator.nr index 38ddd7de1d74..f199988405eb 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator.nr @@ -1,5 +1,5 @@ mod validate_accumulated_items; -mod validate_no_transient_data; +mod validate_unsquashable_note_hash_nullifier_ordering; mod validate_siloed_values; use types::{ @@ -8,8 +8,8 @@ use types::{ traits::Empty, }; use validate_accumulated_items::validate_accumulated_items; -use validate_no_transient_data::validate_no_transient_data; use validate_siloed_values::validate_siloed_values; +use validate_unsquashable_note_hash_nullifier_ordering::validate_unsquashable_note_hash_nullifier_ordering; /// Validate the previous kernel for a private tail, either to_rollup or to_public. /// `is_for_public` should be known at compile time. @@ -31,10 +31,14 @@ pub fn validate_previous_kernel_for_tail( "min_revertible_side_effect_counter must not be 0 for tail_to_public", ); } else { - // The `min_revertible_side_effect_counter` could be anything for a pure private tx, as it's not used in the - // tail circuit or any circuits before the tail. + // For a private-only tx, `min_revertible_side_effect_counter` can be 0 or non-zero - it doesn't + // affect correctness. The reset circuit uses 0 as the split counter for private-only txs, so the + // revertible/non-revertible boundary is never applied regardless of this value. } + // Here, we finally reconcile that the "claimed" revertible counter (as hinted by all app circuits + // and kernel iterations of this tx) actually reconciles with the min_revertible_side_effect counter + // that was set by one of the app circuits of this tx. assert_eq( previous_kernel_public_inputs.claimed_revertible_counter, min_revertible_side_effect_counter, @@ -64,7 +68,7 @@ pub fn validate_previous_kernel_for_tail( // --- fee_payer --- let fee_payer = previous_kernel_public_inputs.fee_payer; - // TODO: use assert_not_empty after Noir #9002. + assert(!fee_payer.is_empty(), "Fee payer can't be empty"); // --- expiration_timestamp --- @@ -91,6 +95,8 @@ pub fn validate_previous_kernel_for_tail( let first_nullifier = previous_kernel_public_inputs.end.nullifiers.array[0]; assert_eq( claimed_first_nullifier, + // Note: we assert that this first_nullifier is siloed within `validate_siloed_values` below, + // by asserting that the contract_address is 0. first_nullifier.innermost().value, "First nullifier claim was not satisfied", ); @@ -117,9 +123,10 @@ pub fn validate_previous_kernel_for_tail( validate_siloed_values(note_hashes, "note hashes"); validate_siloed_values(nullifiers, "nullifiers"); validate_siloed_values(private_logs, "private logs"); + l2_to_l1_msgs.for_each(|msg| msg.innermost().recipient.validate()); - validate_no_transient_data(previous_kernel_public_inputs); + validate_unsquashable_note_hash_nullifier_ordering(previous_kernel_public_inputs); for private_log in private_logs.array { assert( diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_accumulated_items.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_accumulated_items.nr index cc0e6e09f4da..6a82eb2c1d4f 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_accumulated_items.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_accumulated_items.nr @@ -56,8 +56,9 @@ pub fn validate_accumulated_items( // We require the entire contract class log hash to be empty because the contract address is exposed to the public. assert_trimmed_array(contract_class_logs_hashes, |log_hash| log_hash.is_empty()); - // For `public_call_requests`, we only need to ensure that the trimmed items are nullish. Items emitted from private - // functions are guaranteed to be non-empty - their `msg_sender` is either the caller's contract address (which + // For `public_call_requests`, we only need to ensure that the trimmed items are nullish (i.e. + // we don't need to ensure denseness here). Items emitted from private functions are _guaranteed_ + // to be non-empty because their `msg_sender` is either the caller's contract address (which // cannot be 0) or `NULL_MSG_SENDER_CONTRACT_ADDRESS`. // See `private_call_data_validator.nr > validate_public_call_request` for details. if is_for_public { diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_siloed_values.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_siloed_values.nr index 5dc6dbe2f3c4..860908da314e 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_siloed_values.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_siloed_values.nr @@ -1,9 +1,10 @@ use types::{address::AztecAddress, side_effect::Scoped, utils::arrays::ClaimedLengthArray}; /// Ensure that the data has been properly siloed in the reset circuit. -/// Since the siloing is always performed from index 0, we only need to check the last item (within the claimed length). +/// Since the siloing is always performed from index 0 within the reset circuit, we only need to +/// check the last item (within the claimed length). /// If the last item is siloed, then the entire array is siloed. -/// A siloed item has a zero contract address, and unsiloed item must not have a zero contract address. +/// A siloed item has a zero contract address, and an unsiloed item must not have a zero contract address. /// See `reset_output_validator.nr` for how siloing is performed. pub fn validate_siloed_values( array: ClaimedLengthArray, N>, diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_no_transient_data.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_unsquashable_note_hash_nullifier_ordering.nr similarity index 52% rename from noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_no_transient_data.nr rename to noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_unsquashable_note_hash_nullifier_ordering.nr index 0d407accd1a3..505b55ac46b4 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_no_transient_data.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/previous_kernel_for_tail_validator/validate_unsquashable_note_hash_nullifier_ordering.nr @@ -3,24 +3,27 @@ use types::{ constants::MAX_NULLIFIERS_PER_TX, side_effect::Ordered, utils::arrays::find_first_index, }; -// This is mainly for ensuring that for any nullifier that links to a note hash, -// it is created _after_ the note hash. -// This is enforced for transient data when they are squashed in the reset circuit. -// But if a pair is not transient, their counters will be checked here. -// Why would we have a (nullifier, pending note) pair that is non-transient? -// When a pending note hash is non-revertible and its nullifier is revertible, we can't -// squash them, but we still need to perform this check on their counters. -// A nice side effect of this check is that it also makes sure all the transient data is squashed: -// In aztec-nr, if a contract is emitting a nullifier for a non-revertible note -// hash, or if it doesn't want to squash the note hash at all (to keep a full record -// of what had happened, for example), it could set the nullifier.note_hash to be -// the _siloed_ note hash (or not set it at all). -// When we run this function (in the tail), because the non-squashed note hashes -// are already siloed in the reset circuit, the nullifiers that map to non-transient -// note hashes will match up with those _siloed_ note hashes. But for nullifiers -// that should already have been squashed against a transient (not siloed) note -// hash, they won't be able to find a match. -pub fn validate_no_transient_data(previous_kernel_public_inputs: PrivateKernelCircuitPublicInputs) { +/// This is mainly for ensuring that for any nullifier that links to a note hash, +/// it is created _after_ the note hash. +/// This is enforced for transient data when they are squashed in the reset circuit. +/// But if a pair is not transient, their counters will be checked here. +/// Why would we have a (nullifier, pending note) pair that is non-transient? +/// When a pending note hash is non-revertible and its nullifier is revertible, we can't +/// squash them, but we still need to perform this check on their counters. +/// +/// A nice side effect of this check is that it also makes sure all the transient data is squashed: +/// In aztec-nr, if a contract is emitting a nullifier for a non-revertible note +/// hash, or if it doesn't want to squash the note hash at all (to keep a full record +/// of what had happened, for example), it could set the nullifier.note_hash to be +/// the _siloed_ note hash (or not set it at all). +/// When we run this function (in the tail), because the non-squashed note hashes +/// are already siloed in the reset circuit, the nullifiers that map to non-transient +/// note hashes will match up with those _siloed_ note hashes. But for nullifiers +/// that should already have been squashed against a transient (not siloed) note +/// hash, they won't be able to find a match. +pub fn validate_unsquashable_note_hash_nullifier_ordering( + previous_kernel_public_inputs: PrivateKernelCircuitPublicInputs, +) { // Safety: the below hints are constrained by the following methods. See private_kernel_inner for use. let note_hash_index_for_each_nullifier = unsafe { get_note_hash_index_for_each_nullifier(previous_kernel_public_inputs) }; @@ -41,7 +44,18 @@ pub fn validate_no_transient_data(previous_kernel_public_inputs: PrivateKernelCi note_hash.counter() < nullifier.counter(), "Cannot link a note hash emitted after a nullifier", ); - // No need to verify logs linked to a note hash are squashed. + // Note: we don't need to assert that the contract_addresses of the note_hash and + // nullifier match, because by this point (after the final reset circuit) they are all + // siloed, and hence their contract_addresses will be 0. + + // Note: A nullifier's note_hash field could reference a siloed note_hash from a + // different contract. This is harmless: the note_hash field is discarded in + // expose_to_public() and never reaches the rollup. Cross-contract squashing is + // prevented by the reset circuit's contract_address check in + // validate_squashable_note_hash_nullifier_pair. The only effect of the link here + // is a counter-ordering check, which has no security implication. + + // Note: No need to verify logs linked to a note hash are squashed. // When a note hash is squashed, all associated logs are guaranteed to be removed. // See reset/transient_data/transient_data_validator.nr for details. } @@ -56,14 +70,13 @@ pub fn validate_no_transient_data(previous_kernel_public_inputs: PrivateKernelCi // For each nullifier, this array gives the index of that note_hash in the new note_hashes array. // // note_hashes: [ C0, C1, C2, C3, C4, C5, C6, C7] -// nullifiers: [N(C5), N, N(C3), N, N, 0, 0, 0] <-- Notes C5 and C3 are transient notes. +// nullifiers: [N(C5), N, N(C3), N, N, 0, 0, 0] <-- E.g. N(C5) is the nullifier pointing to C5. // -// this: [ 5, _, 3, _, _, _, _, _] <-- the index of the transient note for each nullifier +// this: [ 5, _, 3, _, _, _, _, _] <-- the index of the linked note for each nullifier // unconstrained fn get_note_hash_index_for_each_nullifier( previous_kernel: PrivateKernelCircuitPublicInputs, ) -> [u32; MAX_NULLIFIERS_PER_TX] { - // Q: there's no "null" value, so how do we distinguish between "null" and transient note_hash index 0? let mut note_hash_index_for_each_nullifier = [0; MAX_NULLIFIERS_PER_TX]; let note_hashes = previous_kernel.end.note_hashes; let nullifiers = previous_kernel.end.nullifiers; diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator.nr index 77e21a72d1b6..38d6a3bb138b 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator.nr @@ -117,7 +117,7 @@ impl TailOutputValidator { } fn validate_gas_used(self) { - let gas_used = meter_gas_used(self.previous_kernel, false /* is_for_public */); + let gas_used = meter_gas_used(self.previous_kernel, false); assert_eq(self.output.gas_used, gas_used, "incorrect metered gas used"); let limits = self.previous_kernel.constants.tx_context.gas_settings.gas_limits; diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator/validate_expiration_timestamp.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator/validate_expiration_timestamp.nr index 2fad0f599b8d..0b511fed1d09 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator/validate_expiration_timestamp.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_output_validator/validate_expiration_timestamp.nr @@ -1,5 +1,8 @@ use types::{abis::global_variables::GlobalVariables, constants::MAX_TX_LIFETIME}; +/// We enforce in the tail circuit that the `include_by_timestamp` is at most MAX_TX_LIFETIME after the +/// tx's anchor timestamp. This restriction was motivated by wanting to prune network nodes' +/// mempools of all txs that are longer than a day old. pub fn validate_expiration_timestamp(expiration_timestamp: u64, global_variables: GlobalVariables) { let max_timestamp = global_variables.timestamp + MAX_TX_LIFETIME; assert(expiration_timestamp <= max_timestamp, "expiration_timestamp exceeds max tx lifetime"); diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr index e6e275822045..95afd0649479 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr @@ -58,14 +58,11 @@ pub fn execute( ) -> PrivateToRollupKernelCircuitPublicInputs { // Validate inputs. if !::std::runtime::is_unconstrained() { - inputs.previous_kernel.verify(true /* is_last_kernel */); + inputs.previous_kernel.verify(true); inputs.previous_kernel.validate_vk_in_vk_tree(ALLOWED_PREVIOUS_CIRCUITS); } - validate_previous_kernel_for_tail( - inputs.previous_kernel.public_inputs, - false, /* is_for_public */ - ); + validate_previous_kernel_for_tail(inputs.previous_kernel.public_inputs, false); // Generate output. // Safety: The output is validated below by TailOutputValidator. diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 2f4953ffdf22..d7f68a1f6a15 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -163,7 +163,7 @@ pub global INITIAL_L2_BLOCK_NUM: Field = 1; pub global FIELDS_PER_BLOB: u32 = 4096; pub global BLOBS_PER_CHECKPOINT: u32 = 6; pub global MAX_CHECKPOINTS_PER_EPOCH: u32 = 32; -pub global MAX_TX_LIFETIME: u64 = 86400; // 1 day +pub global MAX_TX_LIFETIME: u64 = 86400; // 1 day. Arbitrarily chosen. // The genesis values are taken from world_state.test.cpp > WorldStateTest.GetInitialTreeInfoForAllTrees pub global GENESIS_BLOCK_HEADER_HASH: Field = 0x2ff681dd7730c7b9e5650c70afa57ee81377792dfc95d98c11817b8c761ff965;