diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index 9d39fb3ff38..8a7c0781904 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -1,11 +1,16 @@ -use crate::serialization::SerializationError; +//! Block height. + +use crate::serialization::{SerializationError, ZcashDeserialize}; + +use byteorder::{LittleEndian, ReadBytesExt}; use std::{ convert::TryFrom, + io, ops::{Add, Sub}, }; -/// The height of a block is the length of the chain back to the genesis block. +/// The length of the chain back to the genesis block. /// /// Block heights can't be added, but they can be *subtracted*, /// to get a difference of block heights, represented as an `i32`, @@ -29,32 +34,34 @@ impl std::str::FromStr for Height { } impl Height { - /// The minimum Height. + /// The minimum [`Height`]. /// /// Due to the underlying type, it is impossible to construct block heights - /// less than `Height::MIN`. + /// less than [`Height::MIN`]. /// - /// Style note: Sometimes, `Height::MIN` is less readable than + /// Style note: Sometimes, [`Height::MIN`] is less readable than /// `Height(0)`. Use whichever makes sense in context. pub const MIN: Height = Height(0); - /// The maximum Height. + /// The maximum [`Height`]. + /// + /// Users should not construct block heights greater than [`Height::MAX`]. /// - /// Users should not construct block heights greater than `Height::MAX`. - pub const MAX: Height = Height(499_999_999); + /// The spec says *"Implementations MUST support block heights up to and + /// including 2^31 − 1"*. + /// + /// Note that `u32::MAX / 2 == 2^31 - 1 == i32::MAX`. + pub const MAX: Height = Height(u32::MAX / 2); - /// The maximum Height as a u32, for range patterns. + /// The maximum [`Height`] as a [`u32`], for range patterns. /// /// `Height::MAX.0` can't be used in match range patterns, use this /// alias instead. pub const MAX_AS_U32: u32 = Self::MAX.0; - /// The maximum expiration Height that is allowed in all transactions - /// previous to Nu5 and in non-coinbase transactions from Nu5 activation height - /// and above. - /// - /// TODO: This is currently the same as `Height::MAX` but that change in #1113. - /// Remove this TODO when that happens. + /// The maximum expiration [`Height`] that is allowed in all transactions + /// previous to Nu5 and in non-coinbase transactions from Nu5 activation + /// height and above. pub const MAX_EXPIRY_HEIGHT: Height = Height(499_999_999); } @@ -125,44 +132,73 @@ impl Sub for Height { } } +impl ZcashDeserialize for Height { + fn zcash_deserialize(mut reader: R) -> Result { + let height = reader.read_u32::()?; + + if height > Self::MAX.0 { + return Err(SerializationError::Parse("Height exceeds maximum height")); + } + + Ok(Self(height)) + } +} + #[test] fn operator_tests() { zebra_test::init(); + // Elementary checks. assert_eq!(Some(Height(2)), Height(1) + Height(1)); assert_eq!(None, Height::MAX + Height(1)); + + let height = Height(u32::pow(2, 31) - 2); + assert!(height < Height::MAX); + + let max_height = (height + Height(1)).expect("this addition should produce the max height"); + assert!(height < max_height); + assert!(max_height <= Height::MAX); + assert_eq!(Height::MAX, max_height); + assert_eq!(None, max_height + Height(1)); + // Bad heights aren't caught at compile-time or runtime, until we add or subtract assert_eq!(None, Height(Height::MAX_AS_U32 + 1) + Height(0)); - assert_eq!(None, Height(i32::MAX as u32) + Height(0)); + assert_eq!(None, Height(i32::MAX as u32) + Height(1)); assert_eq!(None, Height(u32::MAX) + Height(0)); assert_eq!(Some(Height(2)), Height(1) + 1); assert_eq!(None, Height::MAX + 1); + // Adding negative numbers assert_eq!(Some(Height(1)), Height(2) + -1); assert_eq!(Some(Height(0)), Height(1) + -1); assert_eq!(None, Height(0) + -1); assert_eq!(Some(Height(Height::MAX_AS_U32 - 1)), Height::MAX + -1); + // Bad heights aren't caught at compile-time or runtime, until we add or subtract // `+ 0` would also cause an error here, but it triggers a spurious clippy lint assert_eq!(None, Height(Height::MAX_AS_U32 + 1) + 1); assert_eq!(None, Height(i32::MAX as u32) + 1); assert_eq!(None, Height(u32::MAX) + 1); + // Adding negative numbers - assert_eq!(None, Height(i32::MAX as u32) + -1); + assert_eq!(None, Height(i32::MAX as u32 + 1) + -1); assert_eq!(None, Height(u32::MAX) + -1); assert_eq!(Some(Height(1)), Height(2) - 1); assert_eq!(Some(Height(0)), Height(1) - 1); assert_eq!(None, Height(0) - 1); assert_eq!(Some(Height(Height::MAX_AS_U32 - 1)), Height::MAX - 1); + // Subtracting negative numbers assert_eq!(Some(Height(2)), Height(1) - -1); assert_eq!(Some(Height::MAX), Height(Height::MAX_AS_U32 - 1) - -1); assert_eq!(None, Height::MAX - -1); + // Bad heights aren't caught at compile-time or runtime, until we add or subtract - assert_eq!(None, Height(i32::MAX as u32) - 1); + assert_eq!(None, Height(i32::MAX as u32 + 1) - 1); assert_eq!(None, Height(u32::MAX) - 1); + // Subtracting negative numbers assert_eq!(None, Height(Height::MAX_AS_U32 + 1) - -1); assert_eq!(None, Height(i32::MAX as u32) - -1); diff --git a/zebra-chain/src/sapling/tests/prop.rs b/zebra-chain/src/sapling/tests/prop.rs index 4911cb9c990..6d6ffb6d46c 100644 --- a/zebra-chain/src/sapling/tests/prop.rs +++ b/zebra-chain/src/sapling/tests/prop.rs @@ -1,3 +1,5 @@ +//! Sapling prop tests. + use proptest::prelude::*; use crate::{ @@ -120,7 +122,7 @@ proptest! { let tx = Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: block::Height(0), joinsplit_data: None, sapling_shielded_data: Some(shielded_v4), @@ -182,7 +184,7 @@ proptest! { let tx = Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: block::Height(0), joinsplit_data: None, sapling_shielded_data: Some(shielded_v4), diff --git a/zebra-chain/src/serialization/error.rs b/zebra-chain/src/serialization/error.rs index 5d42b333f94..4b11b56a064 100644 --- a/zebra-chain/src/serialization/error.rs +++ b/zebra-chain/src/serialization/error.rs @@ -1,3 +1,5 @@ +//! Errors for transaction serialization. + use std::{array::TryFromSliceError, io, num::TryFromIntError, str::Utf8Error}; use thiserror::Error; diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index f8fb51443eb..1e86fbd0768 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -524,12 +524,13 @@ impl Arbitrary for Memo { type Strategy = BoxedStrategy; } +/// Generates arbitrary [`LockTime`]s. impl Arbitrary for LockTime { type Parameters = (); fn arbitrary_with(_args: ()) -> Self::Strategy { prop_oneof![ - (block::Height::MIN.0..=block::Height::MAX.0) + (block::Height::MIN.0..=LockTime::MAX_HEIGHT.0) .prop_map(|n| LockTime::Height(block::Height(n))), (LockTime::MIN_TIMESTAMP..=LockTime::MAX_TIMESTAMP) .prop_map(|n| { LockTime::Time(Utc.timestamp(n as i64, 0)) }) diff --git a/zebra-chain/src/transaction/lock_time.rs b/zebra-chain/src/transaction/lock_time.rs index 4039bd1cf3c..5d5f2f4102f 100644 --- a/zebra-chain/src/transaction/lock_time.rs +++ b/zebra-chain/src/transaction/lock_time.rs @@ -1,9 +1,11 @@ +//! Transaction LockTime. + use std::{convert::TryInto, io}; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use chrono::{DateTime, TimeZone, Utc}; -use crate::block; +use crate::block::{self, Height}; use crate::serialization::{SerializationError, ZcashDeserialize, ZcashSerialize}; /// A Bitcoin-style `locktime`, representing either a block height or an epoch @@ -11,32 +13,50 @@ use crate::serialization::{SerializationError, ZcashDeserialize, ZcashSerialize} /// /// # Invariants /// -/// Users should not construct a `LockTime` with: -/// - a `block::Height` greater than MAX_BLOCK_HEIGHT, +/// Users should not construct a [`LockTime`] with: +/// - a [`block::Height`] greater than [`LockTime::MAX_HEIGHT`], /// - a timestamp before 6 November 1985 -/// (Unix timestamp less than MIN_LOCK_TIMESTAMP), or +/// (Unix timestamp less than [`LockTime::MIN_TIMESTAMP`]), or /// - a timestamp after 5 February 2106 -/// (Unix timestamp greater than MAX_LOCK_TIMESTAMP). +/// (Unix timestamp greater than [`LockTime::MAX_TIMESTAMP`]). #[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub enum LockTime { - /// Unlock at a particular block height. + /// The transaction can only be included in a block if the block height is + /// strictly greater than this height Height(block::Height), - /// Unlock at a particular time. + /// The transaction can only be included in a block if the block time is + /// strictly greater than this timestamp Time(DateTime), } impl LockTime { - /// The minimum LockTime::Time, as a timestamp in seconds. + /// The minimum [`LockTime::Time`], as a Unix timestamp in seconds. + /// + /// Users should not construct [`LockTime`]s with [`LockTime::Time`] lower + /// than [`LockTime::MIN_TIMESTAMP`]. /// - /// Users should not construct lock times less than `MIN_TIMESTAMP`. + /// If a [`LockTime`] is supposed to be lower than + /// [`LockTime::MIN_TIMESTAMP`], then a particular [`LockTime::Height`] + /// applies instead, as described in the spec. pub const MIN_TIMESTAMP: i64 = 500_000_000; - /// The maximum LockTime::Time, as a timestamp in seconds. + /// The maximum [`LockTime::Time`], as a timestamp in seconds. /// - /// Users should not construct lock times greater than `MAX_TIMESTAMP`. - /// LockTime is u32 in the spec, so times are limited to u32::MAX. + /// Users should not construct lock times with timestamps greater than + /// [`LockTime::MAX_TIMESTAMP`]. LockTime is [`u32`] in the spec, so times + /// are limited to [`u32::MAX`]. pub const MAX_TIMESTAMP: i64 = u32::MAX as i64; + /// The maximum [`LockTime::Height`], as a block height. + /// + /// Users should not construct lock times with a block height greater than + /// [`LockTime::MAX_TIMESTAMP`]. + /// + /// If a [`LockTime`] is supposed to be greater than + /// [`LockTime::MAX_HEIGHT`], then a particular [`LockTime::Time`] applies + /// instead, as described in the spec. + pub const MAX_HEIGHT: Height = Height((Self::MIN_TIMESTAMP - 1) as u32); + /// Returns a [`LockTime`] that is always unlocked. /// /// The lock time is set to the block height of the genesis block. @@ -44,23 +64,23 @@ impl LockTime { LockTime::Height(block::Height(0)) } - /// Returns the minimum LockTime::Time, as a LockTime. + /// Returns the minimum [`LockTime::Time`], as a [`LockTime`]. /// - /// Users should not construct lock times less than `min_lock_timestamp`. + /// Users should not construct lock times with timestamps lower than the + /// value returned by this function. // - // When `Utc.timestamp` stabilises as a const function, we can make this a - // const function. - pub fn min_lock_time() -> LockTime { + // TODO: replace Utc.timestamp with DateTime32 (#2211) + pub fn min_lock_time_timestamp() -> LockTime { LockTime::Time(Utc.timestamp(Self::MIN_TIMESTAMP, 0)) } - /// Returns the maximum LockTime::Time, as a LockTime. + /// Returns the maximum [`LockTime::Time`], as a [`LockTime`]. /// - /// Users should not construct lock times greater than `max_lock_timestamp`. + /// Users should not construct lock times with timestamps greater than the + /// value returned by this function. // - // When `Utc.timestamp` stabilises as a const function, we can make this a - // const function. - pub fn max_lock_time() -> LockTime { + // TODO: replace Utc.timestamp with DateTime32 (#2211) + pub fn max_lock_time_timestamp() -> LockTime { LockTime::Time(Utc.timestamp(Self::MAX_TIMESTAMP, 0)) } } @@ -82,10 +102,10 @@ impl ZcashSerialize for LockTime { impl ZcashDeserialize for LockTime { fn zcash_deserialize(mut reader: R) -> Result { let n = reader.read_u32::()?; - if n <= block::Height::MAX.0 { + if n < Self::MIN_TIMESTAMP.try_into().expect("fits in u32") { Ok(LockTime::Height(block::Height(n))) } else { - // This can't panic, because all u32 values are valid `Utc.timestamp`s + // This can't panic, because all u32 values are valid `Utc.timestamp`s. Ok(LockTime::Time(Utc.timestamp(n.into(), 0))) } } diff --git a/zebra-chain/src/transaction/tests/vectors.rs b/zebra-chain/src/transaction/tests/vectors.rs index a71619b4acb..00f0289e978 100644 --- a/zebra-chain/src/transaction/tests/vectors.rs +++ b/zebra-chain/src/transaction/tests/vectors.rs @@ -23,7 +23,7 @@ use super::super::*; lazy_static! { pub static ref EMPTY_V5_TX: Transaction = Transaction::V5 { network_upgrade: NetworkUpgrade::Nu5, - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: block::Height(0), inputs: Vec::new(), outputs: Vec::new(), @@ -302,7 +302,7 @@ fn empty_v4_round_trip() { let tx = Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: block::Height(0), joinsplit_data: None, sapling_shielded_data: None, diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index ea3f979b7d9..31fda8bad35 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -153,8 +153,8 @@ where .coinbase_height() .ok_or(BlockError::MissingHeight(hash))?; - // TODO: support block heights up to u32::MAX (#1113) - // In practice, these blocks are invalid anyway, because their parent block doesn't exist. + // Zebra does not support heights greater than + // [`block::Height::MAX`]. if height > block::Height::MAX { Err(BlockError::MaxHeight(height, hash, block::Height::MAX))?; } diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index b788c45bc84..ead70e4a18c 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -310,8 +310,8 @@ pub fn coinbase_outputs_are_decryptable( /// Returns `Ok(())` if the expiry height for the coinbase transaction is valid /// according to specifications [7.1] and [ZIP-203]. /// -/// [7.1]: https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus -/// [ZIP-203]: https://zips.z.cash/zip-0203 +/// [7.1]: +/// [ZIP-203]: pub fn coinbase_expiry_height( block_height: &Height, coinbase: &Transaction, @@ -319,51 +319,41 @@ pub fn coinbase_expiry_height( ) -> Result<(), TransactionError> { let expiry_height = coinbase.expiry_height(); - match NetworkUpgrade::Nu5.activation_height(network) { - // If Nu5 does not have a height, apply the pre-Nu5 rule. - None => validate_expiry_height_max(expiry_height, true, block_height, coinbase), - Some(activation_height) => { - // # Consensus - // - // > [NU5 onward] The nExpiryHeight field of a coinbase transaction MUST be equal - // > to its block height. - // - // https://zips.z.cash/protocol/protocol.pdf#txnconsensus - if *block_height >= activation_height { - match expiry_height { - None => Err(TransactionError::CoinbaseExpiryBlockHeight { - expiry_height, - block_height: *block_height, - transaction_hash: coinbase.hash(), - })?, - Some(expiry) => { - if expiry != *block_height { - return Err(TransactionError::CoinbaseExpiryBlockHeight { - expiry_height, - block_height: *block_height, - transaction_hash: coinbase.hash(), - })?; - } - } - } + // TODO: replace `if let` with `expect` after NU5 mainnet activation + if let Some(nu5_activation_height) = NetworkUpgrade::Nu5.activation_height(network) { + // # Consensus + // + // > [NU5 onward] The nExpiryHeight field of a coinbase transaction + // > MUST be equal to its block height. + // + // + if *block_height >= nu5_activation_height { + if expiry_height != Some(*block_height) { + return Err(TransactionError::CoinbaseExpiryBlockHeight { + expiry_height, + block_height: *block_height, + transaction_hash: coinbase.hash(), + }); + } else { return Ok(()); } - // # Consensus - // - // > [Overwinter to Canopy inclusive, pre-NU5] `nExpiryHeight` MUST be less than - // > or equal to 499999999. - // - // https://zips.z.cash/protocol/protocol.pdf#txnconsensus - validate_expiry_height_max(expiry_height, true, block_height, coinbase) } } + + // # Consensus + // + // > [Overwinter to Canopy inclusive, pre-NU5] nExpiryHeight MUST be less than + // > or equal to 499999999. + // + // + validate_expiry_height_max(expiry_height, true, block_height, coinbase) } -/// Returns `Ok(())` if the expiry height for a non coinbase transaction is valid -/// according to specifications [7.1] and [ZIP-203]. +/// Returns `Ok(())` if the expiry height for a non coinbase transaction is +/// valid according to specifications [7.1] and [ZIP-203]. /// -/// [7.1]: https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus -/// [ZIP-203]: https://zips.z.cash/zip-0203 +/// [7.1]: +/// [ZIP-203]: pub fn non_coinbase_expiry_height( block_height: &Height, transaction: &Transaction, @@ -379,21 +369,27 @@ pub fn non_coinbase_expiry_height( // > [NU5 onward] nExpiryHeight MUST be less than or equal to 499999999 // > for non-coinbase transactions. // + // + validate_expiry_height_max(expiry_height, false, block_height, transaction)?; + + // # Consensus + // // > [Overwinter onward] If a transaction is not a coinbase transaction and its - // > nExpiryHeight field is nonzero, then it MUST NOT be mined at a block height - // > greater than its nExpiryHeight. + // > nExpiryHeight field is nonzero, then it MUST NOT be mined at a block + // > height greater than its nExpiryHeight. // - // https://zips.z.cash/protocol/protocol.pdf#txnconsensus - validate_expiry_height_max(expiry_height, false, block_height, transaction)?; + // validate_expiry_height_mined(expiry_height, block_height, transaction)?; } Ok(()) } -/// Validate the consensus rule: nExpiryHeight MUST be less than or equal to 499999999. +/// Checks that the expiry height of a transaction does not exceed the maximal +/// value. /// -/// The remaining arguments are not used for validation, -/// they are only used to create errors. +/// Only the `expiry_height` parameter is used for the check. The +/// remaining parameters are used to give details about the error when the check +/// fails. fn validate_expiry_height_max( expiry_height: Option, is_coinbase: bool, @@ -414,11 +410,10 @@ fn validate_expiry_height_max( Ok(()) } -/// Validate the consensus rule: If a transaction is not a coinbase transaction -/// and its nExpiryHeight field is nonzero, then it MUST NOT be mined at a block -/// height greater than its nExpiryHeight. +/// Checks that a transaction does not exceed its expiry height. /// -/// The `transaction` is only used to create errors. +/// The `transaction` parameter is only used to give details about the error +/// when the check fails. fn validate_expiry_height_mined( expiry_height: Option, block_height: &Height, diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 82d4f1a9d73..f0ff3f64893 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -1,3 +1,5 @@ +//! Tests for Zcash transaction consensus checks. + use std::{ cmp::max, collections::HashMap, @@ -380,6 +382,227 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() { ); } +/// Tests if a non-coinbase V4 transaction with the last valid expiry height is +/// accepted. +#[tokio::test] +async fn v4_transaction_with_last_valid_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = NetworkUpgrade::Canopy + .activation_height(Network::Mainnet) + .expect("Canopy activation height is specified"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + + // Create a non-coinbase V4 tx with the last valid expiry height. + let transaction = Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: block_height, + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result.expect("unexpected error response").tx_id(), + transaction.unmined_id() + ); +} + +/// Tests if a coinbase V4 transaction with an expiry height lower than the +/// block height is accepted. +/// +/// Note that an expiry height lower than the block height is considered +/// *expired* for *non-coinbase* transactions. +#[tokio::test] +async fn v4_coinbase_transaction_with_low_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = NetworkUpgrade::Canopy + .activation_height(Network::Mainnet) + .expect("Canopy activation height is specified"); + + let (input, output) = mock_coinbase_transparent_output(block_height); + + // This is a correct expiry height for coinbase V4 transactions. + let expiry_height = (block_height - 1).expect("original block height is too small"); + + // Create a coinbase V4 tx. + let transaction = Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result.expect("unexpected error response").tx_id(), + transaction.unmined_id() + ); +} + +/// Tests if an expired non-coinbase V4 transaction is rejected. +#[tokio::test] +async fn v4_transaction_with_too_low_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = NetworkUpgrade::Canopy + .activation_height(Network::Mainnet) + .expect("Canopy activation height is specified"); + + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + + // This expiry height is too low so that the tx should seem expired to the verifier. + let expiry_height = (block_height - 1).expect("original block height is too small"); + + // Create a non-coinbase V4 tx. + let transaction = Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::ExpiredTransaction { + expiry_height, + block_height, + transaction_hash: transaction.hash(), + }) + ); +} + +/// Tests if a non-coinbase V4 transaction with an expiry height exceeding the +/// maximum is rejected. +#[tokio::test] +async fn v4_transaction_with_exceeding_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = block::Height::MAX; + + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + + // This expiry height exceeds the maximum defined by the specification. + let expiry_height = block::Height(500_000_000); + + // Create a non-coinbase V4 tx. + let transaction = Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::MaximumExpiryHeight { + expiry_height, + is_coinbase: false, + block_height, + transaction_hash: transaction.hash(), + }) + ); +} + +/// Tests if a coinbase V4 transaction with an expiry height exceeding the +/// maximum is rejected. +#[tokio::test] +async fn v4_coinbase_transaction_with_exceeding_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = block::Height::MAX; + + let (input, output) = mock_coinbase_transparent_output(block_height); + + // This expiry height exceeds the maximum defined by the specification. + let expiry_height = block::Height(500_000_000); + + // Create a coinbase V4 tx. + let transaction = Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::MaximumExpiryHeight { + expiry_height, + is_coinbase: true, + block_height, + transaction_hash: transaction.hash(), + }) + ); +} + /// Test if V4 coinbase transaction is accepted. #[tokio::test] async fn v4_coinbase_transaction_is_accepted() { @@ -717,6 +940,277 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() { ); } +/// Tests if a non-coinbase V5 transaction with the last valid expiry height is +/// accepted. +#[tokio::test] +async fn v5_transaction_with_last_valid_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Testnet, state_service); + + let block_height = NetworkUpgrade::Nu5 + .activation_height(Network::Testnet) + .expect("Nu5 activation height for testnet is specified"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + + // Create a non-coinbase V5 tx with the last valid expiry height. + let transaction = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: block_height, + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade: NetworkUpgrade::Nu5, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result.expect("unexpected error response").tx_id(), + transaction.unmined_id() + ); +} + +/// Tests that a coinbase V5 transaction is accepted only if its expiry height +/// is equal to the height of the block the transaction belongs to. +#[tokio::test] +async fn v5_coinbase_transaction_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Testnet, state_service); + + let block_height = NetworkUpgrade::Nu5 + .activation_height(Network::Testnet) + .expect("Nu5 activation height for testnet is specified"); + + let (input, output) = mock_coinbase_transparent_output(block_height); + + // Create a coinbase V5 tx with an expiry height that matches the height of + // the block. Note that this is the only valid expiry height for a V5 + // coinbase tx. + let transaction = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: block_height, + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade: NetworkUpgrade::Nu5, + }; + + let result = verifier + .clone() + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result.expect("unexpected error response").tx_id(), + transaction.unmined_id() + ); + + // Increment the expiry height so that it becomes invalid. + let new_expiry_height = (block_height + 1).expect("transaction block height is too large"); + let mut new_transaction = transaction.clone(); + + *new_transaction.expiry_height_mut() = new_expiry_height; + + let result = verifier + .clone() + .oneshot(Request::Block { + transaction: Arc::new(new_transaction.clone()), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::CoinbaseExpiryBlockHeight { + expiry_height: Some(new_expiry_height), + block_height, + transaction_hash: new_transaction.hash(), + }) + ); + + // Decrement the expiry height so that it becomes invalid. + let new_expiry_height = (block_height - 1).expect("transaction block height is too low"); + let mut new_transaction = transaction.clone(); + + *new_transaction.expiry_height_mut() = new_expiry_height; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(new_transaction.clone()), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::CoinbaseExpiryBlockHeight { + expiry_height: Some(new_expiry_height), + block_height, + transaction_hash: new_transaction.hash(), + }) + ); +} + +/// Tests if an expired non-coinbase V5 transaction is rejected. +#[tokio::test] +async fn v5_transaction_with_too_low_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Testnet, state_service); + + let block_height = NetworkUpgrade::Nu5 + .activation_height(Network::Testnet) + .expect("Nu5 activation height for testnet is specified"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + + // This expiry height is too low so that the tx should seem expired to the verifier. + let expiry_height = (block_height - 1).expect("original block height is too small"); + + // Create a non-coinbase V5 tx. + let transaction = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade: NetworkUpgrade::Nu5, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::ExpiredTransaction { + expiry_height, + block_height, + transaction_hash: transaction.hash(), + }) + ); +} + +/// Tests if a non-coinbase V5 transaction with an expiry height exceeding the +/// maximum is rejected. +#[tokio::test] +async fn v5_transaction_with_exceeding_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = block::Height::MAX; + + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + + // This expiry height exceeds the maximum defined by the specification. + let expiry_height = block::Height(500_000_000); + + // Create a non-coinbase V5 tx. + let transaction = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade: NetworkUpgrade::Nu5, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::MaximumExpiryHeight { + expiry_height, + is_coinbase: false, + block_height, + transaction_hash: transaction.hash(), + }) + ); +} + +/// Tests if a coinbase V5 transaction with an expiry height exceeding the +/// maximum is rejected. +#[tokio::test] +async fn v5_coinbase_transaction_with_exceeding_expiry_height() { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = Verifier::new(Network::Mainnet, state_service); + + let block_height = block::Height::MAX; + + let (input, output) = mock_coinbase_transparent_output(block_height); + + // This expiry height exceeds the maximum defined by the specification. + let expiry_height = block::Height(500_000_000); + + // Create a coinbase V4 tx. + let transaction = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height, + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade: NetworkUpgrade::Nu5, + }; + + let result = verifier + .oneshot(Request::Block { + transaction: Arc::new(transaction.clone()), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: chrono::MAX_DATETIME, + }) + .await; + + assert_eq!( + result, + Err(TransactionError::MaximumExpiryHeight { + expiry_height, + is_coinbase: true, + block_height, + transaction_hash: transaction.hash(), + }) + ); +} + /// Test if V5 coinbase transaction is accepted. #[tokio::test] async fn v5_coinbase_transaction_is_accepted() { diff --git a/zebra-consensus/src/transaction/tests/prop.rs b/zebra-consensus/src/transaction/tests/prop.rs index c2c3d4bcf57..49ddfd228af 100644 --- a/zebra-consensus/src/transaction/tests/prop.rs +++ b/zebra-consensus/src/transaction/tests/prop.rs @@ -229,13 +229,14 @@ proptest! { } } -/// Generate an arbitrary block height after the Sapling activation height on an arbitrary network. +/// Generates an arbitrary [`block::Height`] after the Sapling activation height +/// on an arbitrary network. /// -/// A proptest [`Strategy`] that generates random tuples with +/// A proptest [`Strategy`] that generates random tuples with: /// -/// - a network (mainnet or testnet) -/// - a block height between the Sapling activation height (inclusive) on that network and the -/// maximum block height. +/// - a network (mainnet or testnet); +/// - a block height between the Sapling activation height (inclusive) on that +/// network and the maximum transaction expiry height. fn sapling_onwards_strategy() -> impl Strategy { any::().prop_flat_map(|network| { let start_height_value = NetworkUpgrade::Sapling @@ -243,7 +244,7 @@ fn sapling_onwards_strategy() -> impl Strategy .expect("Sapling to have an activation height") .0; - let end_height_value = block::Height::MAX.0; + let end_height_value = block::Height::MAX_EXPIRY_HEIGHT.0; (start_height_value..=end_height_value) .prop_map(move |height_value| (network, block::Height(height_value))) diff --git a/zebra-state/src/service/check/tests/anchors.rs b/zebra-state/src/service/check/tests/anchors.rs index 09ec325bb87..fc5b0960be5 100644 --- a/zebra-state/src/service/check/tests/anchors.rs +++ b/zebra-state/src/service/check/tests/anchors.rs @@ -117,7 +117,7 @@ fn prepare_sprout_block(mut block_to_prepare: Block, reference_block: Block) -> .push(Arc::new(Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), joinsplit_data, sapling_shielded_data: None, @@ -173,7 +173,7 @@ fn check_sapling_anchors() { block1.transactions.push(Arc::new(Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), joinsplit_data: None, sapling_shielded_data, @@ -220,7 +220,7 @@ fn check_sapling_anchors() { block2.transactions.push(Arc::new(Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), joinsplit_data: None, sapling_shielded_data, diff --git a/zebra-state/src/service/check/tests/nullifier.rs b/zebra-state/src/service/check/tests/nullifier.rs index f9a58005ce8..bc31a6cfb13 100644 --- a/zebra-state/src/service/check/tests/nullifier.rs +++ b/zebra-state/src/service/check/tests/nullifier.rs @@ -934,7 +934,7 @@ fn transaction_v4_with_joinsplit_data( Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), joinsplit_data, sapling_shielded_data: None, @@ -1002,7 +1002,7 @@ fn transaction_v4_with_sapling_shielded_data( Transaction::V4 { inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), joinsplit_data: None, sapling_shielded_data, @@ -1039,7 +1039,7 @@ fn transaction_v5_with_orchard_shielded_data( network_upgrade: Nu5, inputs: Vec::new(), outputs: Vec::new(), - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), sapling_shielded_data: None, orchard_shielded_data, diff --git a/zebra-state/src/service/check/tests/utxo.rs b/zebra-state/src/service/check/tests/utxo.rs index a162811566a..b64ade7ac26 100644 --- a/zebra-state/src/service/check/tests/utxo.rs +++ b/zebra-state/src/service/check/tests/utxo.rs @@ -930,7 +930,7 @@ fn transaction_v4_with_transparent_data( let mut transaction = Transaction::V4 { inputs, outputs, - lock_time: LockTime::min_lock_time(), + lock_time: LockTime::min_lock_time_timestamp(), expiry_height: Height(0), joinsplit_data: None, sapling_shielded_data: None, diff --git a/zebra-state/src/service/check/utxo.rs b/zebra-state/src/service/check/utxo.rs index 17294a3586c..d9e9e230550 100644 --- a/zebra-state/src/service/check/utxo.rs +++ b/zebra-state/src/service/check/utxo.rs @@ -193,7 +193,6 @@ pub fn transparent_coinbase_spend( match spend_restriction { OnlyShieldedOutputs { spend_height } => { let min_spend_height = utxo.height + block::Height(MIN_TRANSPARENT_COINBASE_MATURITY); - // TODO: allow full u32 range of block heights (#1113) let min_spend_height = min_spend_height.expect("valid UTXOs have coinbase heights far below Height::MAX"); if spend_height >= min_spend_height {