From 4af7e513e2c683c6f5b069702e27ed9d8c0461d0 Mon Sep 17 00:00:00 2001 From: iAmMichaelConnor Date: Sat, 20 Dec 2025 15:03:31 +0000 Subject: [PATCH] doc: misc extra protocol circuit comments --- cspell.json | 1 + .../crates/blob/src/blob_batching.nr | 13 +++--- .../crates/blob/src/utils/mod.nr | 2 +- .../crates/blob/src/utils/validate_point.nr | 8 ++-- .../validate_consecutive_block_rollups.nr | 8 +++- .../block_rollup_public_inputs_composer.nr | 5 +++ ...heckpoint_rollup_public_inputs_composer.nr | 24 +++++++++-- .../checkpoint_root_inputs_validator.nr | 6 ++- .../src/root/root_rollup_private_inputs.nr | 2 +- .../components/private_tail_validator.nr | 24 +++++++---- .../components/private_tx_effect_builder.nr | 8 ++-- .../public_tx_base_inputs_validator.nr | 2 +- .../components/public_tx_effect_builder.nr | 31 +++++++++++-- .../tx_base_public_inputs_composer.nr | 10 +++-- .../public_tx_base_rollup_private_inputs.nr | 8 ++-- .../types/src/blob_data/tx_blob_data.nr | 21 +++++++++ .../crates/types/src/constants.nr | 10 +++-- .../crates/types/src/utils/arrays.nr | 43 ------------------- .../scripts/unravel_favourite_structs.sh | 3 ++ yarn-project/stdlib/src/tx/tx.ts | 11 ++++- 20 files changed, 149 insertions(+), 91 deletions(-) diff --git a/cspell.json b/cspell.json index 0bdfba1d6e33..37f20889cd09 100644 --- a/cspell.json +++ b/cspell.json @@ -157,6 +157,7 @@ "ierc", "IGSE", "incentivized", + "incrementation", "indexeddb", "interruptible", "IPFS", diff --git a/noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching.nr b/noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching.nr index a5cbf8a39f7c..fcc830956f6e 100644 --- a/noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching.nr +++ b/noir-projects/noir-protocol-circuits/crates/blob/src/blob_batching.nr @@ -4,7 +4,7 @@ use crate::{ FinalBlobBatchingChallenges, }, blob::barycentric_evaluate_blob_at_z, - utils::{compress_to_blob_commitment, validate_canonical_point_at_infinity}, + utils::{compress_to_blob_commitment, validate_canonical_representation_if_infinity}, }; use bigcurve::BigCurve; use bignum::BLS12_381_Fr; @@ -42,7 +42,7 @@ fn compute_blob_challenge( challenge } -/// Evaluates each blob required for an L1 block: +/// Evaluates each blob required for an L1 checkpoint: /// - Compresses each of the blob's injected commitments. /// - Evaluates each blob individually to find its challenge `z_i` and evaluation `y_i`. /// - Updates the batched blob accumulator. @@ -54,13 +54,15 @@ pub fn evaluate_blobs_and_batch( final_blob_challenges: FinalBlobBatchingChallenges, start_accumulator: BlobAccumulator, ) -> BlobAccumulator { - // The rationale behind this is that the start accumulator is hinted, and in merge we just assert that left.end.eq(right.start) + // The rationale behind this line is that the start accumulator is hinted, and in merge we just assert that left.end.eq(right.start) // And on root we check that left.start.eq(empty()). // However, currently the equals method of big curve points doesn't check for equality of the limbs on infinity points. // This means that infinity points can be swapped with non canonical versions without the eq realizing it. // I don't think there is a way to take advantage of that, since we should never have infinity points in c_acc, // but as good measure we'll validate that it's canonical here. - validate_canonical_point_at_infinity(start_accumulator.c_acc); + // Also, the Root Rollup circuit technically validates that the start accumulator of the checkpoint is Empty, + // and the Empty impl uses the canonical representation of infinity. + validate_canonical_representation_if_infinity(start_accumulator.c_acc); let mut end_accumulator = start_accumulator; for i in 0..NumBlobs { @@ -68,7 +70,8 @@ pub fn evaluate_blobs_and_batch( let commitment_point = kzg_commitments_points[i]; // We need to validate that the point is on the curve and canonical (0,0 if infinity), since it's hinted in this verification scheme commitment_point.validate_on_curve(); - validate_canonical_point_at_infinity(commitment_point); + // Strangely, `validate_on_curve` doesn't validate the point at infinity, so we check that here: + validate_canonical_representation_if_infinity(commitment_point); let c_i = compress_to_blob_commitment(commitment_point); // Note that with multiple blobs per block, each blob uses the same blob_fields_hash in z_i. diff --git a/noir-projects/noir-protocol-circuits/crates/blob/src/utils/mod.nr b/noir-projects/noir-protocol-circuits/crates/blob/src/utils/mod.nr index 005300d6548a..920250edbcda 100644 --- a/noir-projects/noir-protocol-circuits/crates/blob/src/utils/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/blob/src/utils/mod.nr @@ -4,4 +4,4 @@ mod validate_point; pub use compress_to_blob_commitment::compress_to_blob_commitment; pub use validate_final_blob_batching_challenges::validate_final_blob_batching_challenges; -pub use validate_point::validate_canonical_point_at_infinity; +pub use validate_point::validate_canonical_representation_if_infinity; diff --git a/noir-projects/noir-protocol-circuits/crates/blob/src/utils/validate_point.nr b/noir-projects/noir-protocol-circuits/crates/blob/src/utils/validate_point.nr index ed70c98615ec..bf184909bb10 100644 --- a/noir-projects/noir-protocol-circuits/crates/blob/src/utils/validate_point.nr +++ b/noir-projects/noir-protocol-circuits/crates/blob/src/utils/validate_point.nr @@ -3,7 +3,7 @@ use bigcurve::BigCurve; use bignum::{BigNum, fields::bls12_381Fq::BLS12_381_Fq}; /// Validates that if the point is at infinity, it has the canonical x and y coordinates. -pub fn validate_canonical_point_at_infinity(point: BLSPoint) { +pub fn validate_canonical_representation_if_infinity(point: BLSPoint) { if point.is_infinity() { let canonical_point_at_infinity = BLSPoint::point_at_infinity(); assert_eq(point.x, canonical_point_at_infinity.x, "Non canonical x coordinate at infinity"); @@ -14,7 +14,7 @@ pub fn validate_canonical_point_at_infinity(point: BLSPoint) { #[test] fn should_accept_canonical_point_at_infinity() { let point = BLSPoint { x: BLS12_381_Fq::zero(), y: BLS12_381_Fq::zero(), is_infinity: true }; - validate_canonical_point_at_infinity(point); + validate_canonical_representation_if_infinity(point); } #[test(should_fail_with = "Non canonical x coordinate at infinity")] @@ -22,7 +22,7 @@ fn should_fail_if_non_canonical_x_coordinate_point_at_infinity() { let x = BLS12_381_Fq::one(); let y = BLS12_381_Fq::zero(); let point = BLSPoint { x, y, is_infinity: true }; - validate_canonical_point_at_infinity(point); + validate_canonical_representation_if_infinity(point); } #[test(should_fail_with = "Non canonical y coordinate at infinity")] @@ -30,5 +30,5 @@ fn should_fail_if_non_canonical_y_coordinate_point_at_infinity() { let x = BLS12_381_Fq::zero(); let y = BLS12_381_Fq::one(); let point = BLSPoint { x, y, is_infinity: true }; - validate_canonical_point_at_infinity(point); + validate_canonical_representation_if_infinity(point); } diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_merge/utils/validate_consecutive_block_rollups.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_merge/utils/validate_consecutive_block_rollups.nr index 6409f4be5bdf..84cd1b08ba66 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_merge/utils/validate_consecutive_block_rollups.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_merge/utils/validate_consecutive_block_rollups.nr @@ -35,12 +35,16 @@ fn assert_prev_block_rollups_follow_on_from_each_other( "Mismatched sponge blobs: expected right.start_sponge_blob to match left.end_sponge_blob", ); + // Notice: consecutive blocks within a checkpoint can have the same timestamp. + // I.e. the `<=` is _intentional_. assert( left.end_timestamp <= right.start_timestamp, "Rollup block timestamps do not follow on from each other", ); - // Non-empty `in_hash` originates from the first block root and must propagate through all merge steps via the left - // rollup only. Which means the right rollup must not carry or propagate the `in_hash`. + // Non-empty `in_hash` originates from the first block root and is propagated through all merge + // steps via the left rollup only (see merge_block_rollups.nr). + // We prevent the right rollup from propagating a nonzero `in_hash`, so it's impossible for any + // block but the first to propagate a nonzero `in_hash` up to the checkpoint root. assert_eq(right.in_hash, 0, "Right rollup must not carry in_hash"); } diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_root/components/block_rollup_public_inputs_composer.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_root/components/block_rollup_public_inputs_composer.nr index c7fe5769688f..f81d660c3022 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_root/components/block_rollup_public_inputs_composer.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_root/components/block_rollup_public_inputs_composer.nr @@ -203,6 +203,11 @@ impl BlockRollupPublicInputsComposer { // Absorb data for this block into the end sponge blob. let mut block_end_sponge_blob = self.end_sponge_blob; // `in_hash` is not 0 if and only if it's the first block in the checkpoint. + // Note: the `in_hash` of the first block in the checkpoint is asserted to be nonzero + // by the Checkpoint Root circuit. + // Note: the `in_hash` of subsequent blocks in the checkpoint is asserted to be 0 + // within `validate_consecutive_block_rollups.nr`, by asserting that the `in_hash` of all + // "right" rollups is `0`. let is_first_block_in_checkpoint = self.in_hash != 0; block_end_sponge_blob.absorb_block_end_data( global_variables, diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_rollup_public_inputs_composer.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_rollup_public_inputs_composer.nr index cb297fffe60a..8423b1fc5341 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_rollup_public_inputs_composer.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_rollup_public_inputs_composer.nr @@ -20,6 +20,7 @@ use types::{ pub struct CheckpointRollupPublicInputsComposer { merged_rollup: BlockRollupPublicInputs, + // The below are all hints: start_blob_accumulator: BlobAccumulator, final_blob_challenges: FinalBlobBatchingChallenges, blobs_fields: [Field; FIELDS_PER_BLOB * NumBlobs], @@ -30,6 +31,7 @@ pub struct CheckpointRollupPublicInputsComposer { impl CheckpointRollupPublicInputsComposer { pub fn new( previous_rollups: [BlockRollupPublicInputs; NumPreviousRollups], + // The below args are all hints: start_blob_accumulator: BlobAccumulator, final_blob_challenges: FinalBlobBatchingChallenges, blobs_fields: [Field; NumBlobs * FIELDS_PER_BLOB], @@ -114,12 +116,26 @@ impl CheckpointRollupPublicInputsComposer { end_sponge_blob.absorb_checkpoint_end_marker(); let num_blob_fields = end_sponge_blob.num_absorbed_fields; - // Check that the first `num_blob_fields` of the given fields do match what's been absorbed into the sponge. - let mut expected_end_sponge = SpongeBlob::init(); - expected_end_sponge.absorb(self.blobs_fields, num_blob_fields); + + let mut expected_end_sponge_blob = SpongeBlob::init(); + + // Check that the first `num_blob_fields` of the hinted `self.blobs_fields` do match what's + // been absorbed into the sponge. + // Elaboration: + // We have an `end_sponge_blob`, which is the result of incrementally absorbing data to the sponge + // over many circuits (Tx Base circuits, Block Root circuits, and this Checkpoint Root circuit). + // We need to give this Checkpoint Root circuit access to _all_ of that underlying data, flattened, + // because later in this circuit we'll be interpolating that data into polynomials (one polynomial + // per blob that we've used-up). + // The flattened, underlying data is hinted to this circuit as `self.blobs_fields`. + // To ensure `self.blobs_fields` actually matches the data that all of the previous circuits saw, + // we repeat the entire sponge computation -- the one that was previously split across many + // circuits -- entirely within this circuit in the line below. + // We hope that `expected_end_sponge_blob == end_sponge_blob`. + expected_end_sponge_blob.absorb(self.blobs_fields, num_blob_fields); assert_eq( end_sponge_blob, - expected_end_sponge, + expected_end_sponge_blob, "Provided blob fields do not match the fields that were absorbed", ); // Check that all fields after `num_blob_fields` are zero. diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_root_inputs_validator.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_root_inputs_validator.nr index ea9d075019c0..489ad32a4577 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_root_inputs_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_root/components/checkpoint_root_inputs_validator.nr @@ -78,6 +78,9 @@ impl CheckpointRootInputsVal "The start state of the checkpoint does not match the state in the previous block header", ); + // Notice: unlike blocks within the same checkpoint (which can share the same timestamp), + // the blocks of _different_ checkpoints must have strictly increasing timestamps. + // I.e. the `>` here is _intentional_. assert( first_rollup.start_timestamp > self.previous_block_header.global_variables.timestamp, "The start timestamp must be after the previous block's timestamp", @@ -87,6 +90,7 @@ impl CheckpointRootInputsVal // In `validate_consecutive_block_rollups`, it ensures that the hash only propagates through all merge steps // exclusively via the left rollup. The value in the left rollup must not be 0 when it reaches the checkpoint // root (this point). + // Q: what if the L1 subtree being copied to L2 is empty? What is the value in this case? assert( first_rollup.in_hash != 0, "in_hash must be set via the first block root in the checkpoint", @@ -118,7 +122,6 @@ impl CheckpointRootInputsVal fn validate_end_states(self) { let end_sponge_blob = self.previous_rollups[NumPreviousRollups - 1].public_inputs.end_sponge_blob; - // Check that the number of absorbed blob fields is not larger than the maximum number of fields allowed. // Note: It must not equal the max allowed number because an additional checkpoint end marker will be absorbed // in `checkpoint_rollup_public_inputs_composer`. @@ -126,7 +129,6 @@ impl CheckpointRootInputsVal end_sponge_blob.num_absorbed_fields < FIELDS_PER_BLOB * BLOBS_PER_CHECKPOINT, "Attempted to overfill blobs", ); - // The `end_sponge_blob` is validated against the injected blob fields in // `checkpoint_rollup_public_inputs_composer.nr`, after the checkpoint end marker is absorbed. } diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root/root_rollup_private_inputs.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root/root_rollup_private_inputs.nr index 76b0ed8b26cb..93c207ddcfaf 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root/root_rollup_private_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root/root_rollup_private_inputs.nr @@ -76,7 +76,7 @@ impl RootRollupPrivateInputs { // Below we check: // 1. The first accumulator of the entire epoch (left.start_blob_accumulator) is empty. merged.start_blob_accumulator.assert_empty("Epoch did not start with empty blob state."); - // 2. The `final_blob_challenges` matches the final state of the accumulator. + // 2. The claimed `final_blob_challenges` match the final state of the accumulator. validate_final_blob_batching_challenges( merged.end_blob_accumulator, merged.final_blob_challenges, diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tail_validator.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tail_validator.nr index f4ff1e5228c3..586afa9d2a7f 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tail_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tail_validator.nr @@ -18,13 +18,14 @@ use dep::types::{ utils::arrays::array_padded_with, }; -/// Validate the tx constants against the block constants. +/// Validate some of the tx constants that were propagated from private-land (from the private_tail's public inputs) +/// against the corresponding items of this circuit's block constants. /// - For private-only txs, the block constants are provided via private inputs. -/// - For public-inclusive txs, some values are copied over from the AVM proof's public inputs. This function indirectly -/// checks that the values in `private_tail` match those from the AVM. +/// - For public-inclusive txs, some values are copied over from the AVM proof's public inputs. This function +/// checks that the relevant values in `private_tail` match those from the AVM. pub fn validate_tx_constant_data( tx_constants: TxConstantData, - block_constants: BlockConstantData, + block_constants: BlockConstantData, // where applicable, these constants have been copied-over from the avm's public inputs. anchor_block_archive_sibling_path: [Field; ARCHIVE_HEIGHT], ) { // Values from `tx_constants` to be checked in this function: @@ -35,11 +36,13 @@ pub fn validate_tx_constant_data( let tx_vk_tree_root = tx_constants.vk_tree_root; let tx_protocol_contracts_hash = tx_constants.protocol_contracts_hash; - // Values from `block_constants` to be checked against those from `tx_constants`: + // Values from `block_constants` to be checked against those from `tx_constants`. + // (When executing this fn as part of the public tx base, we comment when an item has been copied-over + // from the avm's public inputs). let block_last_archive_root = block_constants.last_archive.root; - let block_chain_id = block_constants.global_variables.chain_id; - let block_version = block_constants.global_variables.version; - let block_gas_fees = block_constants.global_variables.gas_fees; + let block_chain_id = block_constants.global_variables.chain_id; // from avm + let block_version = block_constants.global_variables.version; // from avm + let block_gas_fees = block_constants.global_variables.gas_fees; // from avm let block_vk_tree_root = block_constants.vk_tree_root; let block_protocol_contracts_hash = block_constants.protocol_contracts_hash; @@ -75,6 +78,8 @@ pub fn validate_tx_constant_data( "Mismatched vk_tree_root between kernel and rollup", ); + // This assertion is not strictly necessary for public tx base, since the value in `block_constants` is copied over + // from `tx_constants`. However, using this single function for both private and public tx bases is cleaner. assert_eq( tx_protocol_contracts_hash, block_protocol_contracts_hash, @@ -82,7 +87,8 @@ pub fn validate_tx_constant_data( ); // Ensure that the `max_fees_per_gas` specified by the tx is greater than or equal to the `gas_fees` for the block. - // Note that for private only txs, the gas against the limit is checked on tail, since it doesn't have public execution. + // Note that for private-only txs, the gas against the limit is checked in the private-kernel-tail, + // since it doesn't have public execution. assert( tx_gas_settings.max_fees_per_gas.fee_per_da_gas >= block_gas_fees.fee_per_da_gas, "da gas is higher than the maximum specified by the tx", diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tx_effect_builder.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tx_effect_builder.nr index 576cbde0335e..8275938c4767 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tx_effect_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/private_tx_effect_builder.nr @@ -24,7 +24,9 @@ pub fn build_tx_effect( ) -> (TxEffect, TxEffectArrayLengths) { let accumulated_data = private_to_rollup.end; - // Silo L2 to L1 messages. + // Compute siloed L2-to-L1 message hashes (siloed with the contract address of the function that + // emitted the message). We do this sequencer-side, because the hashes are computed with sha256 + // for EVM compatibility. let l2_to_l1_msgs = accumulated_data.l2_to_l1_msgs.map(|message| silo_l2_to_l1_message( message, private_to_rollup.constants.tx_context.version, @@ -48,7 +50,7 @@ pub fn build_tx_effect( [PublicDataWrite::empty(); MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX]; public_data_writes[0] = fee_payer_balance_write; - // Construct contract class logs from the hashes from kernel circuits and the log fields given via private inputs. + // Construct ContractClassLogs from the hashes from kernel circuits and the log fields given via private inputs. let contract_class_logs = accumulated_data.contract_class_logs_hashes.mapi(|i, log_hash| { ContractClassLog { log: Log::new(contract_class_log_fields[i], log_hash.inner.length), @@ -83,7 +85,7 @@ pub fn build_tx_effect( note_hashes: array_length(accumulated_data.note_hashes), nullifiers: array_length(accumulated_data.nullifiers), l2_to_l1_msgs: array_length(l2_to_l1_msgs), - public_data_writes: 1, + public_data_writes: 1, // No public functions were called. This is the fee_payer's FeeJuice balance decrementation. private_logs: private_logs_array_length, contract_class_logs: contract_class_logs_array_length, }; diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_base_inputs_validator.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_base_inputs_validator.nr index b90d61b6eff8..7a29effe3951 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_base_inputs_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_base_inputs_validator.nr @@ -84,7 +84,7 @@ impl PublicTxBaseInputsValidator { private_tail_validator::validate_tx_constant_data( private_tail.constants, - self.constants, + self.constants, // where applicable, these constants have been copied-over from the avm's public inputs. self.anchor_block_archive_sibling_path, ); diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_effect_builder.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_effect_builder.nr index f4f1f995ac72..5f37ef1edff1 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_effect_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/public_tx_effect_builder.nr @@ -20,12 +20,29 @@ pub fn build_tx_effect( let revert_code = if reverted { 1 } else { 0 }; let global_variables = avm.global_variables; - // Silo L2 to L1 messages. + // Compute siloed L2-to-L1 message hashes (siloed with the contract address of the function that + // emitted the message). We do this sequencer-side, because the hashes are computed with sha256. let siloed_l2_to_l1_msgs = avm.accumulated_data.l2_to_l1_msgs.map(|message| { silo_l2_to_l1_message(message, global_variables.version, global_variables.chain_id) }); - // Construct private logs. + // If any of the public functions reverted within the `avm`, then the tx is considered to have reverted, + // and none of the tx's _revertible_ side-effects (be they from private or public functions) will be broadcast + // nor inserted into trees. But, the protocol will still broadcast/insert the _non-revertible_ side-effects + // of the tx. + // + // The non-revertible side-effects exist as a consequence of fee abstraction: the `fee_payer` (whomever + // has "put themselves on the hook" to pay fees on the user's behalf) must have a guarantee that + // certain notes/nullifiers/public-state-updates/logs _will_ be included. These "non-revertible + // side-effects" will typically be the side-effects which represent payment of a fee (through some abstract + // fee payment mechanism) to the `fee_payer`. + // + // Note: the other revertible side-effects (notes, nullifiers, public data updates, l2-to-l1 msgs) + // are conditionally discarded by the avm circuit. Private logs and contract class logs are + // different, in that they're only emitted by private functions and so the avm doesn't care about them + // or "see" them. + + // Conditionally discard the revertible private logs. let private_logs = if reverted { private_to_public.non_revertible_accumulated_data.private_logs } else { @@ -35,7 +52,7 @@ pub fn build_tx_effect( ) }; - // Construct contract class logs. + // Conditionally discard the revertible contract class logs. let contract_class_log_hashes = if reverted { private_to_public.non_revertible_accumulated_data.contract_class_logs_hashes } else { @@ -46,11 +63,17 @@ pub fn build_tx_effect( }; // Validate the given log fields against the log hashes from kernel circuits. + // Notice: this validation check is being done here (instead of by this circuit's 'validator', + // and inconsistently vs the tx-private-base circuit), because in the case of an avm revert, + // we only wish to reconcile against the contract class log hashes of _non-revertible_ contract + // class logs -- and the conditional discarding of the revertible logs has only just happened + // (because we try to avoid mutation within a circuit's 'validator'). private_tail_validator::validate_contract_class_logs( contract_class_log_hashes, contract_class_log_fields, ); - // Assign the log fields with the correct length and contract address. + + // Construct ContractClassLogs from the hashes from kernel circuits and the log fields given via private inputs. let contract_class_logs = contract_class_log_hashes.mapi(|i, log_hash| { ContractClassLog { log: Log::new(contract_class_log_fields[i], log_hash.inner.length), diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/tx_base_public_inputs_composer.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/tx_base_public_inputs_composer.nr index 1eca58afb9cd..5c8d564eb0f1 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/tx_base_public_inputs_composer.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/components/tx_base_public_inputs_composer.nr @@ -79,15 +79,19 @@ impl TxBasePublicInputsComposer { ) -> Self { let private_to_public = public_chonk_verifier.private_tail; - // Note: Some values below are copied from `avm` rather than `private_to_public` and this must remain unchanged. - // The constants are checked in `private_tail_validator` to ensure they match the values in `private_to_public`. + // Note: Some values below are copied from the avm's public inputs rather than the private tail's public inputs, + // and this MUST remain unchanged. + // Then in `private_tail_validator.nr`, these avm constants are checked to ensure they match those of the private tail's + // public inputs (`private_to_public`). This ensures that the values match between the avm and the private tail. + // If the values in constants were instead copied over from private tail's public inputs, then the validations in + // `private_tail_validator.nr` become pointless. let constants = BlockConstantData { last_archive, l1_to_l2_tree_snapshot: avm.start_tree_snapshots.l1_to_l2_message_tree, global_variables: avm.global_variables, vk_tree_root: private_to_public.constants.vk_tree_root, protocol_contracts_hash: private_to_public.constants.protocol_contracts_hash, - // We take the prover id of the public chonk verifier, but we validate that it matches the prover id of the avm in validate_public_chonk_verifier_against_avm + // We take the prover id of the public chonk verifier, but we validate that it matches the prover id of the avm in validate_public_chonk_verifier_against_avm. That seems inconsistent and confusing. prover_id: public_chonk_verifier.prover_id, }; diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/public_tx_base_rollup_private_inputs.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/public_tx_base_rollup_private_inputs.nr index a704bc4348bf..733b0c169769 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/public_tx_base_rollup_private_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/tx_base/public_tx_base_rollup_private_inputs.nr @@ -34,8 +34,10 @@ pub struct PublicTxBaseRollupPrivateInputs { pub(crate) anchor_block_archive_sibling_path: [Field; ARCHIVE_HEIGHT], // Fields of the contract class logs. - // These fields are validated against the hashes committed to and propagated from the private tail kernel. The - // fields themselves are added to the blobs, instead of the hashes. + // These fields are validated against the contract class log hashes that are committed to and propagated + // from the private tail kernel. The fields themselves are added to the blobs, instead of the hashes. + // Note: if the avm reverted, only the _non-revertible_ contract class logs will be fed into this circuit, and + // only the _non-revertible_ contract class log hashes will be reconciled. pub(crate) contract_class_log_fields: [[Field; CONTRACT_CLASS_LOG_SIZE_IN_FIELDS]; MAX_CONTRACT_CLASS_LOGS_PER_TX], } @@ -52,7 +54,7 @@ impl PublicTxBaseRollupPrivateInputs { PublicTxBaseInputsValidator::new( self.public_chonk_verifier_proof_data, self.avm_proof_data, - composer.constants, + composer.constants, // where applicable, these constants have been copied-over from the avm's public inputs. self.anchor_block_archive_sibling_path, ) .validate(); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/blob_data/tx_blob_data.nr b/noir-projects/noir-protocol-circuits/crates/types/src/blob_data/tx_blob_data.nr index d340c036d74a..585f5e01aeab 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/blob_data/tx_blob_data.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/blob_data/tx_blob_data.nr @@ -4,10 +4,12 @@ use crate::{ MAX_CONTRACT_CLASS_LOGS_PER_TX, MAX_L2_TO_L1_MSGS_PER_TX, MAX_NOTE_HASHES_PER_TX, MAX_NULLIFIERS_PER_TX, MAX_PRIVATE_LOGS_PER_TX, MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, PRIVATE_LOG_SIZE_IN_FIELDS, TX_START_PREFIX, + TX_START_PREFIX_BIT_SIZE, }, traits::ToField, }; use super::tx_effect::TxEffect; +use std::static_assert; pub global MAX_TX_BLOB_DATA_SIZE_IN_FIELDS: u32 = 1 // tx start marker + 1 // tx hash @@ -64,6 +66,22 @@ pub fn create_tx_start_marker( contract_class_log_length: u32, // Assumed to be a u16, checked on tail to be less than CONTRACT_CLASS_LOG_SIZE_IN_FIELDS revert_code: u8, ) -> Field { + static_assert( + NUM_BLOB_FIELDS_BIT_SIZE + + REVERT_CODE_BIT_SIZE + + CONTRACT_CLASS_LOG_LENGTH_BIT_SIZE + + PUBLIC_LOGS_LENGTH_BIT_SIZE + + PRIVATE_LOGS_LENGTH_BIT_SIZE + + NUM_PRIVATE_LOG_BIT_SIZE + + NUM_PUBLIC_DATA_WRITE_BIT_SIZE + + NUM_L2_TO_L1_MSG_BIT_SIZE + + NUM_NULLIFIER_BIT_SIZE + + NUM_NOTE_HASH_BIT_SIZE + + TX_START_PREFIX_BIT_SIZE + <= 253, + "These bit-sizes don't fit within a field anymore. Have you changed a constant?", + ); + let num_blob_fields_field = num_blob_fields as Field; num_blob_fields_field.assert_max_bit_size::(); let mut marker = num_blob_fields_field; @@ -86,6 +104,9 @@ pub fn create_tx_start_marker( marker += public_logs_length_field * shift; shift *= (1 << PUBLIC_LOGS_LENGTH_BIT_SIZE as u64) as Field; + // This is the length of all private logs (concatenated) that will be inserted into the blob. + // Each private_log conveys its own `length` field; all such `length` fields are included in this + // private_logs_length value. let private_logs_length_field = private_logs_length as Field; private_logs_length_field.assert_max_bit_size::(); marker += private_logs_length_field * shift; 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 3630c923edcb..75ab23598daa 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -161,7 +161,7 @@ pub global INITIAL_CHECKPOINT_NUMBER: Field = 1; pub global INITIAL_L2_BLOCK_NUM: Field = 1; pub global FIELDS_PER_BLOB: u32 = 4096; pub global BLOBS_PER_CHECKPOINT: u32 = 6; -pub global AZTEC_MAX_EPOCH_DURATION: u32 = 48; +pub global AZTEC_MAX_EPOCH_DURATION: u32 = 48; // Is this the max number of checkpoints per epoch? If so, we should rename it. pub global MAX_INCLUDE_BY_TIMESTAMP_DURATION: u64 = 86400; // 1 day // The genesis values are taken from world_state.test.cpp > WorldStateTest.GetInitialTreeInfoForAllTrees pub global GENESIS_BLOCK_HEADER_HASH: Field = @@ -1136,9 +1136,11 @@ pub global AVM_SSTORE_DYN_DA_GAS: u32 = 2 * DA_BYTES_PER_FIELD * DA_GAS_PER_BYTE // BLOB PREFIXES // Used when decoding blobs of tightly packed effects // TODO: Maybe hash instead of directly encoding to field for NUMS -pub global TX_START_PREFIX: Field = 0x74785f7374617274; // = "tx_start".to_field() in nr -pub global BLOCK_END_PREFIX: Field = 0x626c6f636b5f656e64; // = "block_end".to_field() in nr -pub global CHECKPOINT_END_PREFIX: Field = 0x636865636b706f696e745f656e64; // = "checkpoint_end".to_field() in nr +// TODO: we need noir tests that demonstrate that these derivations are correct. +pub global TX_START_PREFIX: Field = 0x74785f7374617274; // = "tx_start".to_field() in nr (8 bytes) +pub global TX_START_PREFIX_BIT_SIZE: u32 = 64; +pub global BLOCK_END_PREFIX: Field = 0x626c6f636b5f656e64; // = "block_end".to_field() in nr (9 bytes) +pub global CHECKPOINT_END_PREFIX: Field = 0x636865636b706f696e745f656e64; // = "checkpoint_end".to_field() in nr (14 bytes) // Constants related to proof type of a recursive proof verification. // Keep following constants in sync with the enum acir_format::PROOF_TYPE in recursion_constraint.hpp diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr index 6e52875f120e..3e0c9ffbbe57 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr @@ -214,49 +214,6 @@ where // ARRAY WRAPPERS //********************************************************************************** -/* - * - * - * - * |-----------------------------------------|------------------------------| - * | LHS | RHS | - * |-----------------------------------------|------------------------------| - * ClaimedLengthArray | Interspersed 0s possible. | Unvalidated. | - * | Possibly not fully trimmed. | Nonempty elements possible. | - * |-----------------------------------------|------------------------------| - * EmptyRHSArray | Interspersed 0s possible. | All 0s (validated). | - * | Possibly not fully trimmed. | | - * |-----------------------------------------|------------------------------| - * TrimmedArray | Interspersed 0s possible. | All 0s (validated) | - * | Last lhs element validated as nonempty. | | - * | (I.e. fully trimmed) | | - * |-----------------------------------------|------------------------------| - * DenseTrimmedArray | Dense (validated). | All 0s (validated) | - * |-----------------------------------------|------------------------------| - * - * - * | What guarantees do we have? | - * |--------|--------|--------------------------------| - * | Dense? | RHS | Length vs Fully Trimmed Length | - * |--------|--------|--------------------------------| - * ClaimedLengthArray | ? | ? | ? | - * | | | | - * |--------|--------|--------------------------------| - * EmptyRHSArray | ? | All 0s | Length >= Fully Trimmed Length | - * | | | | - * |--------|--------|--------------------------------| - * TrimmedArray | ? | All 0s | Length == Fully Trimmed Length | - * | | | | - * | | | | - * |--------|--------|--------------------------------| - * DenseTrimmedArray | Yes | All 0s | Length == Fully Trimmed Length | - * |--------|--------|--------------------------------| - * - * - * An ClaimedLengthArray is distinct from a regular array [T; N], because it carries a length. - * - */ - /// ClaimedLengthArray - An array interpreted by Kernel circuits. /// Its `length` is merely a claim that must eventually be validated. /// Validation must include: diff --git a/noir-projects/noir-protocol-circuits/scripts/unravel_favourite_structs.sh b/noir-projects/noir-protocol-circuits/scripts/unravel_favourite_structs.sh index 79c0f2375330..47409bbb48b6 100755 --- a/noir-projects/noir-protocol-circuits/scripts/unravel_favourite_structs.sh +++ b/noir-projects/noir-protocol-circuits/scripts/unravel_favourite_structs.sh @@ -27,6 +27,9 @@ node scripts/unravel_struct.js target/private_kernel_reset_4_4_4_4_4_4_4_4_4.jso append_line_break "PRIVATE KERNEL TAIL" node scripts/unravel_struct.js target/private_kernel_tail.json --all >> "$FILE" +append_line_break "PRIVATE KERNEL TAIL TO PUBLIC" +node scripts/unravel_struct.js target/private_kernel_tail_to_public.json --all >> "$FILE" + append_line_break "PRIVATE TX BASE" node scripts/unravel_struct.js target/rollup_tx_base_private.json PrivateTxBaseRollupPrivateInputs >> "$FILE" diff --git a/yarn-project/stdlib/src/tx/tx.ts b/yarn-project/stdlib/src/tx/tx.ts index 63294ff18a18..285b83d402db 100644 --- a/yarn-project/stdlib/src/tx/tx.ts +++ b/yarn-project/stdlib/src/tx/tx.ts @@ -30,7 +30,11 @@ export class Tx extends Gossipable { private calldataMap: Map | undefined; constructor( - /** Identifier of the tx */ + /** + * Identifier of the tx. + * It's a hash of the public inputs of the tx's proof. + * This claimed hash is reconciled against the tx's public inputs (`this.data`) in data_validator.ts. + */ public readonly txHash: TxHash, /** * Output of the private kernel circuit for this tx. @@ -43,11 +47,14 @@ export class Tx extends Gossipable { /** * Contract class log fields emitted from the tx. * Their order should match the order of the log hashes returned from `this.data.getNonEmptyContractClassLogsHashes`. - * It's checked in data_validator.ts + * This claimed data is reconciled against a hash of this data (that is contained within + * the tx's public inputs (`this.data`)), in data_validator.ts. */ public readonly contractClassLogFields: ContractClassLogFields[], /** * An array of calldata for the enqueued public function calls and the teardown function call. + * This claimed data is reconciled against hashes of this data (that are contained within + * the tx's public inputs (`this.data`)), in data_validator.ts. */ public readonly publicFunctionCalldata: HashedValues[], ) {