diff --git a/zebra-chain/src/amount.rs b/zebra-chain/src/amount.rs index eb3110ae..e18e39e2 100644 --- a/zebra-chain/src/amount.rs +++ b/zebra-chain/src/amount.rs @@ -227,6 +227,18 @@ impl PartialEq> for Amount { } } +impl PartialEq for Amount { + fn eq(&self, other: &i64) -> bool { + self.0.eq(other) + } +} + +impl PartialEq> for i64 { + fn eq(&self, other: &Amount) -> bool { + self.eq(&other.0) + } +} + impl Eq for Amount {} impl Eq for Amount {} diff --git a/zebra-chain/src/sapling/shielded_data.rs b/zebra-chain/src/sapling/shielded_data.rs index f3ddbc16..d418f50e 100644 --- a/zebra-chain/src/sapling/shielded_data.rs +++ b/zebra-chain/src/sapling/shielded_data.rs @@ -86,6 +86,21 @@ 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): + /// + /// "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/serialization/error.rs b/zebra-chain/src/serialization/error.rs index 2c08d970..e545901c 100644 --- a/zebra-chain/src/serialization/error.rs +++ b/zebra-chain/src/serialization/error.rs @@ -23,4 +23,10 @@ pub enum SerializationError { #[from] source: crate::amount::Error, }, + /// Invalid transaction with a non-zero balance and no Sapling shielded spends or outputs. + /// + /// Transaction does not conform to the Sapling [consensus + /// rule](https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus). + #[error("transaction balance is non-zero but doesn't have Sapling shielded spends or outputs")] + BadTransactionBalance, } diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index 3ebf1c1b..d81deab1 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -608,6 +608,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 + if value_balance != 0 { + return Err(SerializationError::BadTransactionBalance); + } None }; diff --git a/zebra-chain/src/transaction/tests/vectors.rs b/zebra-chain/src/transaction/tests/vectors.rs index 3bc94e91..c9e0d088 100644 --- a/zebra-chain/src/transaction/tests/vectors.rs +++ b/zebra-chain/src/transaction/tests/vectors.rs @@ -3,7 +3,7 @@ use super::super::*; use crate::{ block::{Block, Height, MAX_BLOCK_BYTES}, parameters::{Network, NetworkUpgrade}, - serialization::{ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize}, + serialization::{SerializationError, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize}, }; use std::convert::TryInto; @@ -37,6 +37,37 @@ fn librustzcash_tx_hash() { assert_eq!(hash, expected); } +#[test] +fn doesnt_deserialize_transaction_with_invalid_value_balance() { + zebra_test::init(); + + let dummy_transaction = Transaction::V4 { + inputs: vec![], + outputs: vec![], + lock_time: LockTime::Height(Height(1)), + expiry_height: Height(10), + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let mut input_bytes = Vec::new(); + dummy_transaction + .zcash_serialize(&mut input_bytes) + .expect("dummy transaction should serialize"); + // Set value balance to non-zero + // There are 4 * 4 byte fields and 2 * 1 byte compact sizes = 18 bytes before the 8 byte amount + // (Zcash is little-endian unless otherwise specified: + // https://zips.z.cash/protocol/nu5.pdf#endian) + input_bytes[18] = 1; + + let result = Transaction::zcash_deserialize(&input_bytes[..]); + + assert!(matches!( + result, + Err(SerializationError::BadTransactionBalance) + )); +} + #[test] fn zip143_deserialize_and_round_trip() { zebra_test::init(); diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index cd3a1b02..d81b7b96 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -215,8 +215,6 @@ where } if let Some(sapling_shielded_data) = sapling_shielded_data { - check::sapling_balances_match(&sapling_shielded_data)?; - for spend in sapling_shielded_data.spends_per_anchor() { // Consensus rule: cv and rk MUST NOT be of small // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index aa5727b9..4ee98a07 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -4,7 +4,7 @@ use zebra_chain::{ orchard::Flags, - sapling::{AnchorVariant, Output, PerSpendAnchor, ShieldedData, Spend}, + sapling::{Output, PerSpendAnchor, Spend}, transaction::Transaction, }; @@ -40,31 +40,6 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError> } } -/// Check that if there are no Spends or Outputs, the Sapling valueBalance is also 0. -/// -/// If effectiveVersion = 4 and there are no Spend descriptions or Output descriptions, -/// then valueBalanceSapling MUST be 0. -/// -/// This check is redundant for `Transaction::V5`, because the transaction format -/// omits `valueBalanceSapling` when there are no spends and no outputs. But it's -/// simpler to just do the redundant check anyway. -/// -/// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus -pub fn sapling_balances_match( - sapling_shielded_data: &ShieldedData, -) -> Result<(), TransactionError> -where - AnchorV: AnchorVariant + Clone, -{ - if (sapling_shielded_data.spends().count() + sapling_shielded_data.outputs().count() != 0) - || i64::from(sapling_shielded_data.value_balance) == 0 - { - Ok(()) - } else { - Err(TransactionError::BadBalance) - } -} - /// Check that a coinbase transaction has no PrevOut inputs, JoinSplits, or spends. /// /// A coinbase transaction MUST NOT have any transparent inputs, JoinSplit descriptions, diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index f55f443b..650135a6 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -35,8 +35,6 @@ fn v5_fake_transactions() -> Result<(), Report> { .. } => { if let Some(s) = sapling_shielded_data { - check::sapling_balances_match(&s)?; - for spend in s.spends_per_anchor() { check::spend_cv_rk_not_small_order(&spend)? }