Validate transaction lock times (#3060)

* Create a `LockTime::unlocked` helper constructor

Returns a `LockTime` that is unlocked at the genesis block.

* Return `Option<LockTime>` from `lock_time` method

Prepare to return `None` for when a transaction has its lock time
disabled.

* Return `None` instead of zero `LockTime`

Because a zero lock time means that the transaction was unlocked at the
genesis block, so it was never actually locked.

* Rephrase zero lock time check comment

Clarify that the check is not redundant, and is necessary for the
genesis transaction.

Co-authored-by: teor <teor@riseup.net>

* Add a `transparent::Input::sequence` getter method

Retrieve a transparent input's sequence number.

* Check if lock time is enabled by a sequence number

Validate the consensus rule that the lock time is only enabled if at
least one transparent input has a value different from `u32::MAX` as its
sequence number.

* Add more Zcash specific details to comment

Explain the Zcash specific lock time behaviors.

Co-authored-by: teor <teor@riseup.net>

* Add `time` field to `Request::Block` variant

The block time to use to check if the transaction was unlocked and
allowed to be included in the block.

* Add `Request::block_time` getter

Returns the block time for the block that owns the transaction being
validated or the current time plus a tolerance for mempool transactions.

* Validate transaction lock times

If they are enabled by a transaction's transparent input sequence
numbers, make sure that they are in the past.

* Add comments with consensus rule parts

Make it easier to map what part of the consensus rule each match arm is
responsible for.

Co-authored-by: teor <teor@riseup.net>

zebra-chain/src/transaction.rs

@@ -318,13 +318,49 @@ impl Transaction {
     }
 
     /// Get this transaction's lock time.
-    pub fn lock_time(&self) -> LockTime {
+    pub fn lock_time(&self) -> Option<LockTime> {
-        match self {
+        let lock_time = match self {
-            Transaction::V1 { lock_time, .. } => *lock_time,
+            Transaction::V1 { lock_time, .. }
-            Transaction::V2 { lock_time, .. } => *lock_time,
+            | Transaction::V2 { lock_time, .. }
-            Transaction::V3 { lock_time, .. } => *lock_time,
+            | Transaction::V3 { lock_time, .. }
-            Transaction::V4 { lock_time, .. } => *lock_time,
+            | Transaction::V4 { lock_time, .. }
-            Transaction::V5 { lock_time, .. } => *lock_time,
+            | Transaction::V5 { lock_time, .. } => *lock_time,
+        };
+
+        // `zcashd` checks that the block height is greater than the lock height.
+        // This check allows the genesis block transaction, which would otherwise be invalid.
+        // (Or have to use a lock time.)
+        //
+        // It matches the `zcashd` check here:
+        // https://github.com/zcash/zcash/blob/1a7c2a3b04bcad6549be6d571bfdff8af9a2c814/src/main.cpp#L720
+        if lock_time == LockTime::unlocked() {
+            return None;
+        }
+
+        // Consensus rule:
+        //
+        // > The transaction must be finalized: either its locktime must be in the past (or less
+        // > than or equal to the current block height), or all of its sequence numbers must be
+        // > 0xffffffff.
+        //
+        // In `zcashd`, this rule applies to both coinbase and prevout input sequence numbers.
+        //
+        // Unlike Bitcoin, Zcash allows transactions with no transparent inputs. These transactions
+        // only have shielded inputs. Surprisingly, the `zcashd` implementation ignores the lock
+        // time in these transactions. `zcashd` only checks the lock time when it finds a
+        // transparent input sequence number that is not `u32::MAX`.
+        //
+        // https://developer.bitcoin.org/devguide/transactions.html#non-standard-transactions
+        let has_sequence_number_enabling_lock_time = self
+            .inputs()
+            .iter()
+            .map(transparent::Input::sequence)
+            .any(|sequence_number| sequence_number != u32::MAX);
+
+        if has_sequence_number_enabling_lock_time {
+            Some(lock_time)
+        } else {
+            None
         }
     }
 

zebra-chain/src/transaction/lock_time.rs

@@ -37,6 +37,13 @@ impl LockTime {
     /// LockTime is u32 in the spec, so times are limited to u32::MAX.
     pub const MAX_TIMESTAMP: i64 = u32::MAX as i64;
 
+    /// Returns a [`LockTime`] that is always unlocked.
+    ///
+    /// The lock time is set to the block height of the genesis block.
+    pub fn unlocked() -> Self {
+        LockTime::Height(block::Height(0))
+    }
+
     /// Returns the minimum LockTime::Time, as a LockTime.
     ///
     /// Users should not construct lock times less than `min_lock_timestamp`.

zebra-chain/src/transparent.rs

@@ -151,6 +151,13 @@ impl fmt::Display for Input {
 }
 
 impl Input {
+    /// Returns the input's sequence number.
+    pub fn sequence(&self) -> u32 {
+        match self {
+            Input::PrevOut { sequence, .. } | Input::Coinbase { sequence, .. } => *sequence,
+        }
+    }
+
     /// If this is a `PrevOut` input, returns this input's outpoint.
     /// Otherwise, returns `None`.
     pub fn outpoint(&self) -> Option<OutPoint> {

zebra-consensus/src/block.rs

@@ -206,6 +206,7 @@ where
                         transaction: transaction.clone(),
                         known_utxos: known_utxos.clone(),
                         height,
+                        time: block.header.time,
                     });
                 async_checks.push(rsp);
             }

zebra-consensus/src/block/tests.rs

@@ -15,7 +15,7 @@ use zebra_chain::{
     },
     parameters::{Network, NetworkUpgrade},
     serialization::{ZcashDeserialize, ZcashDeserializeInto},
-    transaction::{arbitrary::transaction_to_fake_v5, Transaction},
+    transaction::{arbitrary::transaction_to_fake_v5, LockTime, Transaction},
     work::difficulty::{ExpandedDifficulty, INVALID_COMPACT_DIFFICULTY},
 };
 use zebra_script::CachedFfiTransaction;
@@ -396,7 +396,7 @@ fn founders_reward_validation_failure() -> Result<(), Report> {
         .map(|transaction| Transaction::V3 {
             inputs: transaction.inputs().to_vec(),
             outputs: vec![transaction.outputs()[0].clone()],
-            lock_time: transaction.lock_time(),
+            lock_time: transaction.lock_time().unwrap_or_else(LockTime::unlocked),
             expiry_height: Height(0),
             joinsplit_data: None,
         })
@@ -468,7 +468,7 @@ fn funding_stream_validation_failure() -> Result<(), Report> {
         .map(|transaction| Transaction::V4 {
             inputs: transaction.inputs().to_vec(),
             outputs: vec![transaction.outputs()[0].clone()],
-            lock_time: transaction.lock_time(),
+            lock_time: transaction.lock_time().unwrap_or_else(LockTime::unlocked),
             expiry_height: Height(0),
             joinsplit_data: None,
             sapling_shielded_data: None,

zebra-consensus/src/error.rs

@@ -5,9 +5,10 @@
 //! implement, and ensures that we don't reject blocks or transactions
 //! for a non-enumerated reason.
 
+use chrono::{DateTime, Utc};
 use thiserror::Error;
 
-use zebra_chain::{orchard, sapling, sprout, transparent};
+use zebra_chain::{block, orchard, sapling, sprout, transparent};
 
 use crate::{block::MAX_BLOCK_SIGOPS, BoxError};
 
@@ -56,6 +57,13 @@ pub enum TransactionError {
     #[error("coinbase inputs MUST NOT exist in mempool")]
     CoinbaseInMempool,
 
+    #[error("transaction is locked until after block height {}", _0.0)]
+    LockedUntilAfterBlockHeight(block::Height),
+
+    #[error("transaction is locked until after block time {0}")]
+    #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
+    LockedUntilAfterBlockTime(DateTime<Utc>),
+
     #[error("coinbase expiration height is invalid")]
     CoinbaseExpiration,
 

zebra-consensus/src/transaction.rs

@@ -9,6 +9,7 @@ use std::{
     task::{Context, Poll},
 };
 
+use chrono::{DateTime, Utc};
 use futures::{
     stream::{FuturesUnordered, StreamExt},
     FutureExt, TryFutureExt,
@@ -80,6 +81,8 @@ pub enum Request {
         known_utxos: Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>>,
         /// The height of the block containing this transaction.
         height: block::Height,
+        /// The time that the block was mined.
+        time: DateTime<Utc>,
     },
     /// Verify the supplied transaction as part of the mempool.
     ///
@@ -185,6 +188,14 @@ impl Request {
         }
     }
 
+    /// The block time used for lock time consensus rules validation.
+    pub fn block_time(&self) -> Option<DateTime<Utc>> {
+        match self {
+            Request::Block { time, .. } => Some(*time),
+            Request::Mempool { .. } => None,
+        }
+    }
+
     /// The network upgrade to consider for the verification.
     ///
     /// This is based on the block height from the request, and the supplied `network`.
@@ -282,6 +293,10 @@ where
             let (utxo_sender, mut utxo_receiver) = mpsc::unbounded_channel();
 
             // Do basic checks first
+            if let Some(block_time) = req.block_time() {
+                check::lock_time_has_passed(&tx, req.height(), block_time)?;
+            }
+
             check::has_inputs_and_outputs(&tx)?;
             check::has_enough_orchard_flags(&tx)?;
 

zebra-consensus/src/transaction/check.rs

@@ -4,6 +4,8 @@
 
 use std::{borrow::Cow, collections::HashSet, convert::TryFrom, hash::Hash};
 
+use chrono::{DateTime, Utc};
+
 use zebra_chain::{
     amount::{Amount, NonNegative},
     block::Height,
@@ -11,11 +13,52 @@ use zebra_chain::{
     parameters::{Network, NetworkUpgrade},
     primitives::zcash_note_encryption,
     sapling::{Output, PerSpendAnchor, Spend},
-    transaction::Transaction,
+    transaction::{LockTime, Transaction},
 };
 
 use crate::error::TransactionError;
 
+/// Checks if the transaction's lock time allows this transaction to be included in a block.
+///
+/// Consensus rule:
+///
+/// > The transaction must be finalized: either its locktime must be in the past (or less
+/// > than or equal to the current block height), or all of its sequence numbers must be
+/// > 0xffffffff.
+///
+/// [`Transaction::lock_time`] validates the transparent input sequence numbers, returning [`None`]
+/// if they indicate that the transaction is finalized by them. Otherwise, this function validates
+/// if the lock time is in the past.
+pub fn lock_time_has_passed(
+    tx: &Transaction,
+    block_height: Height,
+    block_time: DateTime<Utc>,
+) -> Result<(), TransactionError> {
+    match tx.lock_time() {
+        Some(LockTime::Height(unlock_height)) => {
+            // > The transaction can be added to any block which has a greater height.
+            // The Bitcoin documentation is wrong or outdated here,
+            // so this code is based on the `zcashd` implementation at:
+            // https://github.com/zcash/zcash/blob/1a7c2a3b04bcad6549be6d571bfdff8af9a2c814/src/main.cpp#L722
+            if block_height > unlock_height {
+                Ok(())
+            } else {
+                Err(TransactionError::LockedUntilAfterBlockHeight(unlock_height))
+            }
+        }
+        Some(LockTime::Time(unlock_time)) => {
+            // > The transaction can be added to any block whose block time is greater than the locktime.
+            // https://developer.bitcoin.org/devguide/transactions.html#locktime-and-sequence-number
+            if block_time > unlock_time {
+                Ok(())
+            } else {
+                Err(TransactionError::LockedUntilAfterBlockTime(unlock_time))
+            }
+        }
+        None => Ok(()),
+    }
+}
+
 /// Checks that the transaction has inputs and outputs.
 ///
 /// For `Transaction::V4`:

zebra-consensus/src/transaction/tests.rs

@@ -272,6 +272,7 @@ async fn v5_transaction_is_rejected_before_nu5_activation() {
                 height: canopy
                     .activation_height(network)
                     .expect("Canopy activation height is specified"),
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -323,6 +324,7 @@ async fn v5_transaction_is_accepted_after_nu5_activation_for_network(network: Ne
             height: nu5
                 .activation_height(network)
                 .expect("NU5 activation height is specified"),
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -372,6 +374,7 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -418,6 +421,7 @@ async fn v4_coinbase_transaction_is_accepted() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(HashMap::new()),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -468,6 +472,7 @@ async fn v4_transaction_with_transparent_transfer_is_rejected_by_the_script() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -518,6 +523,7 @@ async fn v4_transaction_with_conflicting_transparent_spend_is_rejected() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -584,6 +590,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_inside_joinsplit_is_rejected
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height: transaction_block_height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -655,6 +662,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_across_joinsplits_is_rejecte
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height: transaction_block_height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -709,6 +717,7 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -758,6 +767,7 @@ async fn v5_coinbase_transaction_is_accepted() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -810,6 +820,7 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -862,6 +873,7 @@ async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() {
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             height: transaction_block_height,
+            time: chrono::MAX_DATETIME,
         })
         .await;
 
@@ -931,6 +943,7 @@ fn v4_with_signed_sprout_transfer_is_accepted() {
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height: transaction_block_height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -984,6 +997,7 @@ fn v4_with_unsigned_sprout_transfer_is_rejected() {
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height: transaction_block_height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -1032,6 +1046,7 @@ fn v4_with_sapling_spends() {
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -1076,6 +1091,7 @@ fn v4_with_duplicate_sapling_spends() {
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -1122,6 +1138,7 @@ fn v4_with_sapling_outputs_and_no_spends() {
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -1169,6 +1186,7 @@ fn v5_with_sapling_spends() {
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -1216,6 +1234,7 @@ fn v5_with_duplicate_sapling_spends() {
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 
@@ -1279,6 +1298,7 @@ fn v5_with_duplicate_orchard_action() {
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 height,
+                time: chrono::MAX_DATETIME,
             })
             .await;
 

zebra-state/src/tests/setup.rs

@@ -9,7 +9,7 @@ use zebra_chain::{
         NetworkUpgrade,
     },
     serialization::ZcashDeserializeInto,
-    transaction::Transaction,
+    transaction::{LockTime, Transaction},
 };
 
 use crate::{
@@ -111,7 +111,7 @@ pub(crate) fn transaction_v4_from_coinbase(coinbase: &Transaction) -> Transactio
     Transaction::V4 {
         inputs: coinbase.inputs().to_vec(),
         outputs: coinbase.outputs().to_vec(),
-        lock_time: coinbase.lock_time(),
+        lock_time: coinbase.lock_time().unwrap_or_else(LockTime::unlocked),
         // `Height(0)` means that the expiry height is ignored
         expiry_height: coinbase.expiry_height().unwrap_or(Height(0)),
         // invalid for coinbase transactions