From d0dbb5fe6d45f8fddf350c9d2b4f435c0351500e Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Thu, 3 Feb 2022 17:59:08 -0300 Subject: [PATCH 1/4] refactor transaction size consensus rules --- zebra-chain/src/orchard/shielded_data.rs | 16 ++++++++---- zebra-chain/src/sapling/output.rs | 6 ++++- zebra-chain/src/sapling/shielded_data.rs | 8 ++++-- zebra-chain/src/sapling/spend.rs | 6 ++++- zebra-chain/src/transaction.rs | 21 ---------------- zebra-consensus/src/transaction/check.rs | 32 ++++++++++++++++-------- 6 files changed, 48 insertions(+), 41 deletions(-) diff --git a/zebra-chain/src/orchard/shielded_data.rs b/zebra-chain/src/orchard/shielded_data.rs index e09fba508c7..db2f361e54a 100644 --- a/zebra-chain/src/orchard/shielded_data.rs +++ b/zebra-chain/src/orchard/shielded_data.rs @@ -183,8 +183,12 @@ impl TrustedPreallocate for Action { // and the signature is required, // a valid max allocation can never exceed this size const MAX: u64 = (MAX_BLOCK_BYTES - 1) / AUTHORIZED_ACTION_SIZE; + // # Consensus + // // > [NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16. - // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // + // https://zips.z.cash/protocol/protocol.pdf#txnconsensus + // // This acts as nActionsOrchard and is therefore subject to the rule. // The maximum value is actually smaller due to the block size limit, // but we ensure the 2^16 limit with a static assertion. @@ -206,13 +210,15 @@ bitflags! { /// The spend and output flags are passed to the `Halo2Proof` verifier, which verifies /// the relevant note spending and creation consensus rules. /// - /// Consensus rules: + /// # Consensus + /// + /// > [NU5 onward] In a version 5 transaction, the reserved bits 2..7 of the flagsOrchard + /// > field MUST be zero. + /// + /// /// - /// - "In a version 5 transaction, the reserved bits 2..7 of the flagsOrchard field MUST be zero." /// ([`bitflags`](https://docs.rs/bitflags/1.2.1/bitflags/index.html) restricts its values to the /// set of valid flags) - /// - "In a version 5 coinbase transaction, the enableSpendsOrchard flag MUST be 0." - /// (Checked in zebra-consensus) #[derive(Deserialize, Serialize)] pub struct Flags: u8 { /// Enable spending non-zero valued Orchard notes. diff --git a/zebra-chain/src/sapling/output.rs b/zebra-chain/src/sapling/output.rs index 2535a3d3a01..3d9ccb56b6e 100644 --- a/zebra-chain/src/sapling/output.rs +++ b/zebra-chain/src/sapling/output.rs @@ -196,8 +196,12 @@ impl TrustedPreallocate for OutputInTransactionV4 { // Since a serialized Vec uses at least one byte for its length, // the max allocation can never exceed (MAX_BLOCK_BYTES - 1) / OUTPUT_SIZE const MAX: u64 = (MAX_BLOCK_BYTES - 1) / OUTPUT_SIZE; + // # Consensus + // // > [NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16. - // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // + // https://zips.z.cash/protocol/protocol.pdf#txnconsensus + // // This acts as nOutputsSapling and is therefore subject to the rule. // The maximum value is actually smaller due to the block size limit, // but we ensure the 2^16 limit with a static assertion. diff --git a/zebra-chain/src/sapling/shielded_data.rs b/zebra-chain/src/sapling/shielded_data.rs index b35ad0634a9..45b809a6da6 100644 --- a/zebra-chain/src/sapling/shielded_data.rs +++ b/zebra-chain/src/sapling/shielded_data.rs @@ -94,8 +94,12 @@ where /// [`ShieldedData`] validates this [value balance consensus /// rule](https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus): /// - /// "If effectiveVersion = 4 and there are no Spend descriptions or Output - /// descriptions, then valueBalanceSapling MUST be 0." + /// # Consensus + /// + /// > [Sapling onward] If effectiveVersion = 4 and there are no Spend + /// > descriptions or Output descriptions, then valueBalanceSapling MUST be 0. + /// + /// /// /// During deserialization, this rule is checked when there are no spends and /// no outputs. diff --git a/zebra-chain/src/sapling/spend.rs b/zebra-chain/src/sapling/spend.rs index 578619d23dd..0f680457202 100644 --- a/zebra-chain/src/sapling/spend.rs +++ b/zebra-chain/src/sapling/spend.rs @@ -286,8 +286,12 @@ impl TrustedPreallocate for SpendPrefixInTransactionV5 { // and the associated fields are required, // a valid max allocation can never exceed this size const MAX: u64 = (MAX_BLOCK_BYTES - 1) / SHARED_ANCHOR_SPEND_SIZE; + // # Consensus + // // > [NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16. - // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // + // https://zips.z.cash/protocol/protocol.pdf#txnconsensus + // // This acts as nSpendsSapling and is therefore subject to the rule. // The maximum value is actually smaller due to the block size limit, // but we ensure the 2^16 limit with a static assertion. diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index 49457d03ebe..1aeaaa9d3d9 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -223,14 +223,6 @@ impl Transaction { // other properties /// Does this transaction have transparent or shielded inputs? - /// - /// "[Sapling onward] If effectiveVersion < 5, then at least one of tx_in_count, - /// nSpendsSapling, and nJoinSplit MUST be nonzero. - /// - /// [NU5 onward] If effectiveVersion ≥ 5 then this condition MUST hold: - /// tx_in_count > 0 or nSpendsSapling > 0 or (nActionsOrchard > 0 and enableSpendsOrchard = 1)." - /// - /// https://zips.z.cash/protocol/protocol.pdf#txnconsensus pub fn has_transparent_or_shielded_inputs(&self) -> bool { !self.inputs().is_empty() || self.has_shielded_inputs() } @@ -249,14 +241,6 @@ impl Transaction { } /// Does this transaction have transparent or shielded outputs? - /// - /// "[Sapling onward] If effectiveVersion < 5, then at least one of tx_out_count, - /// nOutputsSapling, and nJoinSplit MUST be nonzero. - /// - /// [NU5 onward] If effectiveVersion ≥ 5 then this condition MUST hold: - /// tx_out_count > 0 or nOutputsSapling > 0 or (nActionsOrchard > 0 and enableOutputsOrchard = 1)." - /// - /// https://zips.z.cash/protocol/protocol.pdf#txnconsensus pub fn has_transparent_or_shielded_outputs(&self) -> bool { !self.outputs().is_empty() || self.has_shielded_outputs() } @@ -275,11 +259,6 @@ impl Transaction { } /// Does this transaction has at least one flag when we have at least one orchard action? - /// - /// [NU5 onward] If effectiveVersion >= 5 and nActionsOrchard > 0, then at least one - /// of enableSpendsOrchard and enableOutputsOrchard MUST be 1. - /// - /// https://zips.z.cash/protocol/protocol.pdf#txnconsensus pub fn has_enough_orchard_flags(&self) -> bool { if self.version() < 5 || self.orchard_actions().count() == 0 { return true; diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index c7d1e74625f..0a7d6d26c74 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -60,19 +60,27 @@ pub fn lock_time_has_passed( /// Checks that the transaction has inputs and outputs. /// +/// # Consensus +/// /// For `Transaction::V4`: -/// * At least one of `tx_in_count`, `nSpendsSapling`, and `nJoinSplit` MUST be non-zero. -/// * At least one of `tx_out_count`, `nOutputsSapling`, and `nJoinSplit` MUST be non-zero. +/// +/// > [Sapling onward] If effectiveVersion < 5, then at least one of +/// > tx_in_count, nSpendsSapling, and nJoinSplit MUST be nonzero. +/// +/// > [Sapling onward] If effectiveVersion < 5, then at least one of +/// > tx_out_count, nOutputsSapling, and nJoinSplit MUST be nonzero. /// /// For `Transaction::V5`: -/// * This condition must hold: `tx_in_count` > 0 or `nSpendsSapling` > 0 or -/// (`nActionsOrchard` > 0 and `enableSpendsOrchard` = 1) -/// * This condition must hold: `tx_out_count` > 0 or `nOutputsSapling` > 0 or -/// (`nActionsOrchard` > 0 and `enableOutputsOrchard` = 1) /// -/// This check counts both `Coinbase` and `PrevOut` transparent inputs. +/// > [NU5 onward] If effectiveVersion >= 5 then this condition MUST hold: +/// > tx_in_count > 0 or nSpendsSapling > 0 or (nActionsOrchard > 0 and enableSpendsOrchard = 1). /// -/// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus +/// > [NU5 onward] If effectiveVersion >= 5 then this condition MUST hold: +/// > tx_out_count > 0 or nOutputsSapling > 0 or (nActionsOrchard > 0 and enableOutputsOrchard = 1). +/// +/// +/// +/// This check counts both `Coinbase` and `PrevOut` transparent inputs. pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError> { if !tx.has_transparent_or_shielded_inputs() { Err(TransactionError::NoInputs) @@ -85,11 +93,13 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError> /// Checks that the transaction has enough orchard flags. /// +/// # Consensus +/// /// For `Transaction::V5` only: -/// * If `orchard_actions_count` > 0 then at least one of -/// `ENABLE_SPENDS|ENABLE_OUTPUTS` must be active. /// -/// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus +/// > [NU5 onward] If effectiveVersion >= 5 and nActionsOrchard > 0, then at least one of enableSpendsOrchard and enableOutputsOrchard MUST be 1. +/// +/// pub fn has_enough_orchard_flags(tx: &Transaction) -> Result<(), TransactionError> { if !tx.has_enough_orchard_flags() { return Err(TransactionError::NotEnoughFlags); From 4089e5a73e0637f6a31761a876f69f69d14a3098 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Fri, 4 Feb 2022 09:56:10 -0300 Subject: [PATCH 2/4] quote mssing consensus rule --- zebra-chain/src/transaction/serialize.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 0aa653a1eb3..22955f99827 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -535,6 +535,17 @@ impl ZcashSerialize for Transaction { impl ZcashDeserialize for Transaction { fn zcash_deserialize(reader: R) -> Result { + // # Consensus + // + // > [Pre-Sapling] The encoded size of the transaction MUST be less than or + // > equal to 100000 bytes. + // + // https://zips.z.cash/protocol/protocol.pdf#txnconsensus + // + // Zebra will not verify this rule because we checkpoint up to Canopy blocks, but: + // Since transactions must get mined into a block to be useful, + // we reject transactions that are larger than blocks. + // // If the limit is reached, we'll get an UnexpectedEof error. let mut limited_reader = reader.take(MAX_BLOCK_BYTES); From dcf7bac61b021871bf6441a31e9ac85815fae2ae Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 7 Feb 2022 17:46:07 -0300 Subject: [PATCH 3/4] nit Co-authored-by: teor --- zebra-chain/src/transaction/serialize.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 22955f99827..e91de619ed3 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -542,7 +542,7 @@ impl ZcashDeserialize for Transaction { // // https://zips.z.cash/protocol/protocol.pdf#txnconsensus // - // Zebra will not verify this rule because we checkpoint up to Canopy blocks, but: + // Zebra does not verify this rule because we checkpoint up to Canopy blocks, but: // Since transactions must get mined into a block to be useful, // we reject transactions that are larger than blocks. // From 6fbdc1369cff14f6545c7bfa290bfc851b2cc7db Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 8 Feb 2022 15:12:00 -0300 Subject: [PATCH 4/4] move consensus rule doc --- zebra-chain/src/sapling/shielded_data.rs | 19 ------------------- zebra-chain/src/transaction/serialize.rs | 9 ++++++--- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/zebra-chain/src/sapling/shielded_data.rs b/zebra-chain/src/sapling/shielded_data.rs index 45b809a6da6..f6d4fceb7fa 100644 --- a/zebra-chain/src/sapling/shielded_data.rs +++ b/zebra-chain/src/sapling/shielded_data.rs @@ -90,25 +90,6 @@ where AnchorV: AnchorVariant + Clone, { /// The net value of Sapling spend transfers minus output transfers. - /// - /// [`ShieldedData`] validates this [value balance consensus - /// rule](https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus): - /// - /// # Consensus - /// - /// > [Sapling onward] If effectiveVersion = 4 and there are no Spend - /// > descriptions or Output descriptions, then valueBalanceSapling MUST be 0. - /// - /// - /// - /// During deserialization, this rule is checked when there are no spends and - /// no outputs. - /// - /// During serialization, this rule is structurally validated by [`ShieldedData`]. - /// `value_balance` is a field in [`ShieldedData`], which must have at least - /// one spend or output in its `transfers` field. If [`ShieldedData`] is `None` - /// then there can not possibly be any spends or outputs, and the - /// `value_balance` is always serialized as zero. pub value_balance: Amount, /// A bundle of spends and outputs, containing at least one spend or diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 22955f99827..e9a67c5aca6 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -649,9 +649,12 @@ impl ZcashDeserialize for Transaction { outputs: shielded_outputs.try_into().expect("checked for outputs"), }) } else { - // There are no shielded outputs and no shielded spends, so the value balance - // MUST be zero: - // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // # Consensus + // + // > [Sapling onward] If effectiveVersion = 4 and there are no Spend + // > descriptions or Output descriptions, then valueBalanceSapling MUST be 0. + // + // https://zips.z.cash/protocol/protocol.pdf#txnconsensus if value_balance != 0 { return Err(SerializationError::BadTransactionBalance); }