diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index ef095467..a5a652fd 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -476,8 +476,9 @@ impl Transaction { } /// Returns `true` if this transaction has valid inputs for a coinbase - /// transaction, that is, has a single input and it is a coinbase input. - pub fn has_valid_coinbase_transaction_inputs(&self) -> bool { + /// transaction, that is, has a single input and it is a coinbase input + /// (null prevout). + pub fn is_coinbase(&self) -> bool { self.inputs().len() == 1 && matches!( self.inputs().get(0), @@ -485,20 +486,16 @@ impl Transaction { ) } - /// Returns `true` if transaction contains any coinbase inputs. - pub fn has_any_coinbase_inputs(&self) -> bool { - self.inputs() - .iter() - .any(|input| matches!(input, transparent::Input::Coinbase { .. })) - } - - /// Returns `true` if transaction contains any `PrevOut` inputs. + /// Returns `true` if this transaction has valid inputs for a non-coinbase + /// transaction, that is, does not have any coinbase input (non-null prevouts). /// - /// `PrevOut` inputs are also known as `transparent` inputs in the spec. - pub fn contains_prevout_input(&self) -> bool { + /// Note that it's possible for a transaction return false in both + /// [`Transaction::is_coinbase`] and [`Transaction::is_valid_non_coinbase`], + /// though those transactions will be rejected. + pub fn is_valid_non_coinbase(&self) -> bool { self.inputs() .iter() - .any(|input| matches!(input, transparent::Input::PrevOut { .. })) + .all(|input| matches!(input, transparent::Input::PrevOut { .. })) } // sprout diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index 1e86fbd0..c326e9a2 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -430,7 +430,7 @@ impl Transaction { &mut self, outputs: &HashMap, ) -> Result, ValueBalanceError> { - if self.has_valid_coinbase_transaction_inputs() { + if self.is_coinbase() { // TODO: if needed, fixup coinbase: // - miner subsidy // - founders reward or funding streams (hopefully not?) diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index 31fda8ba..49d7edea 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -180,17 +180,13 @@ where let now = Utc::now(); check::time_is_valid_at(&block.header, now, &height, &hash) .map_err(VerifyBlockError::Time)?; - check::coinbase_is_first(&block)?; - let coinbase_tx = block - .transactions - .get(0) - .expect("must have coinbase transaction"); + let coinbase_tx = check::coinbase_is_first(&block)?; check::subsidy_is_valid(&block, network)?; // Now do the slower checks // Check compatibility with ZIP-212 shielded Sapling and Orchard coinbase output decryption - tx::check::coinbase_outputs_are_decryptable(coinbase_tx, network, height)?; + tx::check::coinbase_outputs_are_decryptable(&coinbase_tx, network, height)?; // Send transactions to the transaction verifier to be checked let mut async_checks = FuturesUnordered::new(); diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index 3635211c..ce17abed 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -1,7 +1,7 @@ //! Consensus check functions use chrono::{DateTime, Utc}; -use std::collections::HashSet; +use std::{collections::HashSet, sync::Arc}; use zebra_chain::{ amount::{Amount, Error as AmountError, NonNegative}, @@ -15,28 +15,46 @@ use crate::{error::*, parameters::SLOW_START_INTERVAL}; use super::subsidy; -/// Returns `Ok(())` if there is exactly one coinbase transaction in `Block`, -/// and that coinbase transaction is the first transaction in the block. +/// Checks if there is exactly one coinbase transaction in `Block`, +/// and if that coinbase transaction is the first transaction in the block. +/// Returns the coinbase transaction is successful. /// -/// "The first (and only the first) transaction in a block is a coinbase -/// transaction, which collects and spends any miner subsidy and transaction -/// fees paid by transactions included in this block." [ยง3.10][3.10] +/// > A transaction that has a single transparent input with a null prevout field, +/// > is called a coinbase transaction. Every block has a single coinbase +/// > transaction as the first transaction in the block. /// -/// [3.10]: https://zips.z.cash/protocol/protocol.pdf#coinbasetransactions -pub fn coinbase_is_first(block: &Block) -> Result<(), BlockError> { +/// +pub fn coinbase_is_first(block: &Block) -> Result, BlockError> { + // # Consensus + // + // > A block MUST have at least one transaction + // + // let first = block .transactions .get(0) .ok_or(BlockError::NoTransactions)?; + // > The first transaction in a block MUST be a coinbase transaction, + // > and subsequent transactions MUST NOT be coinbase transactions. + // + // + // + // > A transaction that has a single transparent input with a null prevout + // > field, is called a coinbase transaction. + // + // let mut rest = block.transactions.iter().skip(1); - if !first.has_valid_coinbase_transaction_inputs() { + if !first.is_coinbase() { return Err(TransactionError::CoinbasePosition)?; } - if rest.any(|tx| tx.has_any_coinbase_inputs()) { + // > A transparent input in a non-coinbase transaction MUST NOT have a null prevout + // + // + if !rest.all(|tx| tx.is_valid_non_coinbase()) { return Err(TransactionError::CoinbaseAfterFirst)?; } - Ok(()) + Ok(first.clone()) } /// Returns `Ok(())` if `hash` passes: diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index 5b041fb4..4e08620b 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -42,9 +42,6 @@ pub enum TransactionError { #[error("coinbase input found in non-coinbase transaction")] CoinbaseAfterFirst, - #[error("coinbase transaction MUST NOT have any transparent (PrevOut) inputs")] - CoinbaseHasPrevOutInput, - #[error("coinbase transaction MUST NOT have any JoinSplit descriptions")] CoinbaseHasJoinSplit, @@ -63,6 +60,9 @@ pub enum TransactionError { #[error("coinbase inputs MUST NOT exist in mempool")] CoinbaseInMempool, + #[error("non-coinbase transactions MUST NOT have coinbase inputs")] + NonCoinbaseHasCoinbaseInput, + #[error("transaction is locked until after block height {}", _0.0)] LockedUntilAfterBlockHeight(block::Height), diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 3d536454..1ff6e16e 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -313,15 +313,17 @@ where check::has_inputs_and_outputs(&tx)?; check::has_enough_orchard_flags(&tx)?; - if req.is_mempool() && tx.has_any_coinbase_inputs() { + if req.is_mempool() && tx.is_coinbase() { return Err(TransactionError::CoinbaseInMempool); } - if tx.has_valid_coinbase_transaction_inputs() { + if tx.is_coinbase() { check::coinbase_tx_no_prevout_joinsplit_spend(&tx)?; + } else if !tx.is_valid_non_coinbase() { + return Err(TransactionError::NonCoinbaseHasCoinbaseInput); } // Validate `nExpiryHeight` consensus rules - if tx.has_any_coinbase_inputs() { + if tx.is_coinbase() { check::coinbase_expiry_height(&req.height(), &tx, network)?; } else { check::non_coinbase_expiry_height(&req.height(), &tx)?; @@ -396,7 +398,7 @@ where // Calculate the fee only for non-coinbase transactions. let mut miner_fee = None; - if !tx.has_valid_coinbase_transaction_inputs() { + if !tx.is_coinbase() { // TODO: deduplicate this code with remaining_transaction_value (#TODO: open ticket) miner_fee = match value_balance { Ok(vb) => match vb.remaining_transaction_value() { @@ -678,7 +680,7 @@ where ) -> Result { let transaction = request.transaction(); - if transaction.has_valid_coinbase_transaction_inputs() { + if transaction.is_coinbase() { // The script verifier only verifies PrevOut inputs and their corresponding UTXOs. // Coinbase transactions don't have any PrevOut inputs. Ok(AsyncChecks::new()) diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index ead70e4a..e77c5931 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -111,8 +111,9 @@ pub fn has_enough_orchard_flags(tx: &Transaction) -> Result<(), TransactionError /// /// # Consensus /// -/// > A coinbase transaction MUST NOT have any transparent inputs with non-null prevout fields, -/// > JoinSplit descriptions, or Spend descriptions. +/// > A coinbase transaction MUST NOT have any JoinSplit descriptions. +/// +/// > A coinbase transaction MUST NOT have any Spend descriptions. /// /// > [NU5 onward] In a version 5 coinbase transaction, the enableSpendsOrchard flag MUST be 0. /// @@ -124,10 +125,8 @@ pub fn has_enough_orchard_flags(tx: &Transaction) -> Result<(), TransactionError /// /// pub fn coinbase_tx_no_prevout_joinsplit_spend(tx: &Transaction) -> Result<(), TransactionError> { - if tx.has_valid_coinbase_transaction_inputs() { - if tx.contains_prevout_input() { - return Err(TransactionError::CoinbaseHasPrevOutInput); - } else if tx.joinsplit_count() > 0 { + if tx.is_coinbase() { + if tx.joinsplit_count() > 0 { return Err(TransactionError::CoinbaseHasJoinSplit); } else if tx.sapling_spends_per_anchor().count() > 0 { return Err(TransactionError::CoinbaseHasSpend); diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 462975a5..f4806048 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -210,7 +210,7 @@ fn v5_coinbase_transaction_without_enable_spends_flag_passes_validation() { zebra_test::vectors::MAINNET_BLOCKS.iter(), ) .rev() - .find(|transaction| transaction.has_valid_coinbase_transaction_inputs()) + .find(|transaction| transaction.is_coinbase()) .expect("At least one fake V5 coinbase transaction in the test vectors"); insert_fake_orchard_shielded_data(&mut transaction); @@ -225,7 +225,7 @@ fn v5_coinbase_transaction_with_enable_spends_flag_fails_validation() { zebra_test::vectors::MAINNET_BLOCKS.iter(), ) .rev() - .find(|transaction| transaction.has_valid_coinbase_transaction_inputs()) + .find(|transaction| transaction.is_coinbase()) .expect("At least one fake V5 coinbase transaction in the test vectors"); let shielded_data = insert_fake_orchard_shielded_data(&mut transaction); @@ -1386,8 +1386,7 @@ fn v4_with_signed_sprout_transfer_is_accepted() { let (height, transaction) = test_transactions(network) .rev() .filter(|(_, transaction)| { - !transaction.has_valid_coinbase_transaction_inputs() - && transaction.inputs().is_empty() + !transaction.is_coinbase() && transaction.inputs().is_empty() }) .find(|(_, transaction)| transaction.sprout_groth16_joinsplits().next().is_some()) .expect("No transaction found with Groth16 JoinSplits"); @@ -1456,9 +1455,7 @@ async fn v4_with_joinsplit_is_rejected_for_modification( let (height, mut transaction) = test_transactions(network) .rev() - .filter(|(_, transaction)| { - !transaction.has_valid_coinbase_transaction_inputs() && transaction.inputs().is_empty() - }) + .filter(|(_, transaction)| !transaction.is_coinbase() && transaction.inputs().is_empty()) .find(|(_, transaction)| transaction.sprout_groth16_joinsplits().next().is_some()) .expect("No transaction found with Groth16 JoinSplits"); @@ -1496,8 +1493,7 @@ fn v4_with_sapling_spends() { let (height, transaction) = test_transactions(network) .rev() .filter(|(_, transaction)| { - !transaction.has_valid_coinbase_transaction_inputs() - && transaction.inputs().is_empty() + !transaction.is_coinbase() && transaction.inputs().is_empty() }) .find(|(_, transaction)| transaction.sapling_spends_per_anchor().next().is_some()) .expect("No transaction found with Sapling spends"); @@ -1537,8 +1533,7 @@ fn v4_with_duplicate_sapling_spends() { let (height, mut transaction) = test_transactions(network) .rev() .filter(|(_, transaction)| { - !transaction.has_valid_coinbase_transaction_inputs() - && transaction.inputs().is_empty() + !transaction.is_coinbase() && transaction.inputs().is_empty() }) .find(|(_, transaction)| transaction.sapling_spends_per_anchor().next().is_some()) .expect("No transaction found with Sapling spends"); @@ -1583,8 +1578,7 @@ fn v4_with_sapling_outputs_and_no_spends() { let (height, transaction) = test_transactions(network) .rev() .filter(|(_, transaction)| { - !transaction.has_valid_coinbase_transaction_inputs() - && transaction.inputs().is_empty() + !transaction.is_coinbase() && transaction.inputs().is_empty() }) .find(|(_, transaction)| { transaction.sapling_spends_per_anchor().next().is_none() @@ -1629,10 +1623,7 @@ fn v5_with_sapling_spends() { let transaction = fake_v5_transactions_for_network(network, zebra_test::vectors::MAINNET_BLOCKS.iter()) .rev() - .filter(|transaction| { - !transaction.has_valid_coinbase_transaction_inputs() - && transaction.inputs().is_empty() - }) + .filter(|transaction| !transaction.is_coinbase() && transaction.inputs().is_empty()) .find(|transaction| transaction.sapling_spends_per_anchor().next().is_some()) .expect("No transaction found with Sapling spends"); @@ -1674,10 +1665,7 @@ fn v5_with_duplicate_sapling_spends() { let mut transaction = fake_v5_transactions_for_network(network, zebra_test::vectors::MAINNET_BLOCKS.iter()) .rev() - .filter(|transaction| { - !transaction.has_valid_coinbase_transaction_inputs() - && transaction.inputs().is_empty() - }) + .filter(|transaction| !transaction.is_coinbase() && transaction.inputs().is_empty()) .find(|transaction| transaction.sapling_spends_per_anchor().next().is_some()) .expect("No transaction found with Sapling spends"); diff --git a/zebra-state/src/service/check/utxo.rs b/zebra-state/src/service/check/utxo.rs index 0ebaf9a1..00fe5fbb 100644 --- a/zebra-state/src/service/check/utxo.rs +++ b/zebra-state/src/service/check/utxo.rs @@ -111,6 +111,13 @@ pub fn transparent_spend( /// an attempt to spend the same satoshis twice." /// /// https://developer.bitcoin.org/devguide/block_chain.html#introduction +/// +/// # Consensus +/// +/// > Every non-null prevout MUST point to a unique UTXO in either a preceding block, +/// > or a previous transaction in the same block. +/// +/// fn transparent_spend_chain_order( spend: transparent::OutPoint, spend_tx_index_in_block: usize, @@ -229,7 +236,7 @@ pub fn remaining_transaction_value( ) -> Result<(), ValidateContextError> { for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { // TODO: check coinbase transaction remaining value (#338, #1162) - if transaction.has_valid_coinbase_transaction_inputs() { + if transaction.is_coinbase() { continue; } diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 7c995505..072024b9 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -105,7 +105,7 @@ async fn test_populated_state_responds_correctly( for transaction in &block.transactions { let transaction_hash = transaction.hash(); - let from_coinbase = transaction.has_valid_coinbase_transaction_inputs(); + let from_coinbase = transaction.is_coinbase(); for (index, output) in transaction.outputs().iter().cloned().enumerate() { let outpoint = transparent::OutPoint { hash: transaction_hash, diff --git a/zebrad/src/components/inbound/tests/fake_peer_set.rs b/zebrad/src/components/inbound/tests/fake_peer_set.rs index 3475b65c..db45c1c9 100644 --- a/zebrad/src/components/inbound/tests/fake_peer_set.rs +++ b/zebrad/src/components/inbound/tests/fake_peer_set.rs @@ -217,7 +217,7 @@ async fn mempool_advertise_transaction_ids() -> Result<(), crate::BoxError> { let test_transaction = block .transactions .into_iter() - .find(|tx| !tx.has_any_coinbase_inputs()) + .find(|tx| !tx.is_coinbase()) .expect("at least one non-coinbase transaction"); let test_transaction_id = test_transaction.unmined_id(); let txs = HashSet::from_iter([test_transaction_id]);