From 8075d616fb731de5ef50567a013b329cfda0033f Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 1 May 2023 21:13:33 -0300 Subject: [PATCH] feat(mempool): add ZIP-317 rules to mempool (#6556) * add ZIP-317 rules to mempool * fix some docs * rustfmt * fix import * typo * fix tests * fix tests 2 * fix tests 3 * fix tests 4 * fix tests 5 * move constant * fix constant for feature * document/quote zip rules * add Minimum Fee Rate rule * change(mempool): Refactor the ZIP-317 minimum fee rate calculation to use usize (#6585) * Refactor the minimum fee rate calculation to use usize * Check for overflow if constants change * remove 1 rule check, fix docs --------- Co-authored-by: teor --- zebra-chain/src/transaction.rs | 4 +- zebra-chain/src/transaction/unmined.rs | 10 +- zebra-chain/src/transaction/unmined/zip317.rs | 94 ++++++ .../src/transaction/unmined/zip317/tests.rs | 18 ++ zebra-consensus/src/error.rs | 4 + zebra-consensus/src/transaction.rs | 7 +- zebra-consensus/src/transaction/tests.rs | 273 ++++++++++++++++-- zebra-consensus/src/transaction/tests/prop.rs | 9 +- .../methods/get_block_template_rpcs/zip317.rs | 6 +- .../components/inbound/tests/fake_peer_set.rs | 52 ++-- .../src/components/mempool/storage/tests.rs | 9 +- .../components/mempool/storage/tests/prop.rs | 32 +- .../mempool/storage/tests/vectors.rs | 9 +- zebrad/src/components/mempool/tests/vector.rs | 26 +- 14 files changed, 482 insertions(+), 71 deletions(-) create mode 100644 zebra-chain/src/transaction/unmined/zip317/tests.rs diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index d6de1f895fb..0b2c25583d2 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -34,7 +34,9 @@ pub use serialize::{ MIN_TRANSPARENT_TX_V5_SIZE, }; pub use sighash::{HashType, SigHash}; -pub use unmined::{UnminedTx, UnminedTxId, VerifiedUnminedTx, MEMPOOL_TRANSACTION_COST_THRESHOLD}; +pub use unmined::{ + zip317, UnminedTx, UnminedTxId, VerifiedUnminedTx, MEMPOOL_TRANSACTION_COST_THRESHOLD, +}; use crate::{ amount::{Amount, Error as AmountError, NegativeAllowed, NonNegative}, diff --git a/zebra-chain/src/transaction/unmined.rs b/zebra-chain/src/transaction/unmined.rs index 9dfc03a27f6..9c081018c97 100644 --- a/zebra-chain/src/transaction/unmined.rs +++ b/zebra-chain/src/transaction/unmined.rs @@ -36,7 +36,7 @@ use proptest_derive::Arbitrary; #[allow(unused_imports)] use crate::block::MAX_BLOCK_BYTES; -mod zip317; +pub mod zip317; /// The minimum cost value for a transaction in the mempool. /// @@ -353,17 +353,19 @@ impl VerifiedUnminedTx { transaction: UnminedTx, miner_fee: Amount, legacy_sigop_count: u64, - ) -> Self { + ) -> Result { let fee_weight_ratio = zip317::conventional_fee_weight_ratio(&transaction, miner_fee); let unpaid_actions = zip317::unpaid_actions(&transaction, miner_fee); - Self { + zip317::mempool_checks(unpaid_actions, miner_fee, transaction.size)?; + + Ok(Self { transaction, miner_fee, legacy_sigop_count, fee_weight_ratio, unpaid_actions, - } + }) } /// Returns `true` if the transaction pays at least the [ZIP-317] conventional fee. diff --git a/zebra-chain/src/transaction/unmined/zip317.rs b/zebra-chain/src/transaction/unmined/zip317.rs index cc4c998c48c..44ef709aacd 100644 --- a/zebra-chain/src/transaction/unmined/zip317.rs +++ b/zebra-chain/src/transaction/unmined/zip317.rs @@ -5,6 +5,7 @@ use std::cmp::max; use num_integer::div_ceil; +use thiserror::Error; use crate::{ amount::{Amount, NonNegative}, @@ -13,6 +14,9 @@ use crate::{ transaction::{Transaction, UnminedTx}, }; +#[cfg(test)] +mod tests; + /// The marginal fee for the ZIP-317 fee calculation, in zatoshis per logical action. // // TODO: allow Amount in constants @@ -37,6 +41,27 @@ const BLOCK_PRODUCTION_WEIGHT_RATIO_CAP: f32 = 4.0; /// This avoids special handling for transactions with zero weight. const MIN_BLOCK_PRODUCTION_SUBSTITUTE_FEE: i64 = 1; +/// The ZIP-317 recommended limit on the number of unpaid actions per block. +/// `block_unpaid_action_limit` in ZIP-317. +pub const BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT: u32 = 50; + +/// The minimum fee per kilobyte for Zebra mempool transactions. +/// Also used as the minimum fee for a mempool transaction. +/// +/// Based on `DEFAULT_MIN_RELAY_TX_FEE` in `zcashd`: +/// +/// +/// This is a `usize` to simplify transaction size-based calculation code. +pub const MIN_MEMPOOL_TX_FEE_RATE: usize = 100; + +/// The fee cap for [`MIN_MEMPOOL_TX_FEE_RATE`] minimum required mempool fees. +/// +/// Based on `LEGACY_DEFAULT_FEE` in `zcashd`: +/// +/// +/// This is a `usize` to simplify transaction size-based calculation code. +pub const MEMPOOL_TX_FEE_REQUIREMENT_CAP: usize = 1000; + /// Returns the conventional fee for `transaction`, as defined by [ZIP-317]. /// /// [ZIP-317]: https://zips.z.cash/zip-0317#fee-calculation @@ -139,3 +164,72 @@ fn conventional_actions(transaction: &Transaction) -> u32 { max(GRACE_ACTIONS, logical_actions) } + +/// Make ZIP-317 checks before inserting a transaction into the mempool. +pub fn mempool_checks( + unpaid_actions: u32, + miner_fee: Amount, + transaction_size: usize, +) -> Result<(), Error> { + // # Standard Rule + // + // > If a transaction has more than `block_unpaid_action_limit` "unpaid actions" as defined by the + // > Recommended algorithm for block template construction, it will never be mined by that algorithm. + // > Nodes MAY drop these transactions. + // + // + if unpaid_actions > BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT { + return Err(Error::UnpaidActions); + } + + // # Standard Rule + // + // > Nodes that normally relay transactions are expected to do so for transactions that pay at least the + // > conventional fee as specified in this ZIP. + // + // + // + // In Zebra, we use a similar minimum fee rate to `zcashd` v5.5.0 and later. + // Transactions must pay a fee of at least 100 zatoshis per 1000 bytes of serialized size, + // with a maximum fee of 1000 zatoshis. + // + // + // + // In zcashd this is `DEFAULT_MIN_RELAY_TX_FEE` and `LEGACY_DEFAULT_FEE`: + // + // + + const KILOBYTE: usize = 1000; + + // This calculation can't overflow, because transactions are limited to 2 MB, + // and usize is at least 4 GB. + assert!( + MIN_MEMPOOL_TX_FEE_RATE + < usize::MAX / usize::try_from(MAX_BLOCK_BYTES).expect("constant fits in usize"), + "the fee rate multiplication must never overflow", + ); + + let min_fee = (MIN_MEMPOOL_TX_FEE_RATE * transaction_size / KILOBYTE) + .clamp(MIN_MEMPOOL_TX_FEE_RATE, MEMPOOL_TX_FEE_REQUIREMENT_CAP); + let min_fee: u64 = min_fee + .try_into() + .expect("clamped value always fits in u64"); + let min_fee: Amount = min_fee.try_into().expect("clamped value is positive"); + + if miner_fee < min_fee { + return Err(Error::FeeBelowMinimumRate); + } + + Ok(()) +} + +/// Errors related to ZIP-317. +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +#[allow(missing_docs)] +pub enum Error { + #[error("Unpaid actions is higher than the limit")] + UnpaidActions, + + #[error("Transaction fee is below the minimum fee rate")] + FeeBelowMinimumRate, +} diff --git a/zebra-chain/src/transaction/unmined/zip317/tests.rs b/zebra-chain/src/transaction/unmined/zip317/tests.rs new file mode 100644 index 00000000000..fb708a73c0b --- /dev/null +++ b/zebra-chain/src/transaction/unmined/zip317/tests.rs @@ -0,0 +1,18 @@ +//! ZIP-317 tests. + +use super::{mempool_checks, Amount, Error}; +#[test] +fn zip317_unpaid_actions_err() { + let check = mempool_checks(51, Amount::try_from(1).unwrap(), 1); + + assert!(check.is_err()); + assert_eq!(check.err(), Some(Error::UnpaidActions)); +} + +#[test] +fn zip317_minimum_rate_fee_err() { + let check = mempool_checks(50, Amount::try_from(1).unwrap(), 1000); + + assert!(check.is_err()); + assert_eq!(check.err(), Some(Error::FeeBelowMinimumRate)); +} diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index d089073eb29..e5852368a98 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -221,6 +221,10 @@ pub enum TransactionError { outpoint: transparent::OutPoint, min_spend_height: block::Height, }, + + #[error("failed to verify ZIP-317 transaction rules, transaction was not inserted to mempool")] + #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))] + Zip317(#[from] zebra_chain::transaction::zip317::Error), } impl From for TransactionError { diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 28546485b0a..744b8570c95 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -466,14 +466,15 @@ where miner_fee, legacy_sigop_count, }, - Request::Mempool { transaction, .. } => Response::Mempool { - transaction: VerifiedUnminedTx::new( + Request::Mempool { transaction, .. } => { + let transaction = VerifiedUnminedTx::new( transaction, miner_fee.expect( "unexpected mempool coinbase transaction: should have already rejected", ), legacy_sigop_count, - ), + )?; + Response::Mempool { transaction } }, }; diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 89fd0276762..9f9fd6fbfd5 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -23,7 +23,7 @@ use zebra_chain::{ fake_v5_transactions_for_network, insert_fake_orchard_shielded_data, test_transactions, transactions_from_blocks, }, - Hash, HashType, JoinSplitData, LockTime, Transaction, + zip317, Hash, HashType, JoinSplitData, LockTime, Transaction, }, transparent::{self, CoinbaseData}, }; @@ -240,7 +240,12 @@ async fn mempool_request_with_present_input_is_accepted() { .activation_height(Network::Mainnet) .expect("Canopy activation height is specified"); let fund_height = (height - 1).expect("fake source fund block height is too small"); - let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10001).expect("invalid value"), + ); // Create a non-coinbase V4 tx with the last valid expiry height. let tx = Transaction::V4 { @@ -302,7 +307,12 @@ async fn mempool_request_with_invalid_lock_time_is_rejected() { .activation_height(Network::Mainnet) .expect("Canopy activation height is specified"); let fund_height = (height - 1).expect("fake source fund block height is too small"); - let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a non-coinbase V4 tx with the last valid expiry height. let tx = Transaction::V4 { @@ -376,7 +386,12 @@ async fn mempool_request_with_unlocked_lock_time_is_accepted() { .activation_height(Network::Mainnet) .expect("Canopy activation height is specified"); let fund_height = (height - 1).expect("fake source fund block height is too small"); - let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10001).expect("invalid value"), + ); // Create a non-coinbase V4 tx with the last valid expiry height. let tx = Transaction::V4 { @@ -438,7 +453,12 @@ async fn mempool_request_with_lock_time_max_sequence_number_is_accepted() { .activation_height(Network::Mainnet) .expect("Canopy activation height is specified"); let fund_height = (height - 1).expect("fake source fund block height is too small"); - let (mut input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + let (mut input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10001).expect("invalid value"), + ); // Ignore the lock time. input.set_sequence(u32::MAX); @@ -503,7 +523,12 @@ async fn mempool_request_with_past_lock_time_is_accepted() { .activation_height(Network::Mainnet) .expect("Canopy activation height is specified"); let fund_height = (height - 1).expect("fake source fund block height is too small"); - let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10001).expect("invalid value"), + ); // Create a non-coinbase V4 tx with the last valid expiry height. let tx = Transaction::V4 { @@ -577,7 +602,12 @@ async fn mempool_request_with_immature_spent_is_rejected() { .activation_height(Network::Mainnet) .expect("Canopy activation height is specified"); let fund_height = (height - 1).expect("fake source fund block height is too small"); - let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10001).expect("invalid value"), + ); // Create a non-coinbase V4 tx with the last valid expiry height. let tx = Transaction::V4 { @@ -836,7 +866,12 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() { (transaction_block_height - 1).expect("fake source fund block height is too small"); // Create a fake transparent transfer that should succeed - let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fake_source_fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a V4 transaction let transaction = Transaction::V4 { @@ -881,7 +916,12 @@ async fn v4_transaction_with_last_valid_expiry_height() { .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); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a non-coinbase V4 tx with the last valid expiry height. let transaction = Transaction::V4 { @@ -965,7 +1005,12 @@ async fn v4_transaction_with_too_low_expiry_height() { .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); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // 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"); @@ -1010,7 +1055,12 @@ async fn v4_transaction_with_exceeding_expiry_height() { 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); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // This expiry height exceeds the maximum defined by the specification. let expiry_height = block::Height(500_000_000); @@ -1163,7 +1213,12 @@ async fn v4_transaction_with_transparent_transfer_is_rejected_by_the_script() { (transaction_block_height - 1).expect("fake source fund block height is too small"); // Create a fake transparent transfer that should not succeed - let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, false, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fake_source_fund_height, + false, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a V4 transaction let transaction = Transaction::V4 { @@ -1213,7 +1268,12 @@ async fn v4_transaction_with_conflicting_transparent_spend_is_rejected() { (transaction_block_height - 1).expect("fake source fund block height is too small"); // Create a fake transparent transfer that should succeed - let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fake_source_fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a V4 transaction let transaction = Transaction::V4 { @@ -1401,7 +1461,12 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() { (transaction_block_height - 1).expect("fake source fund block height is too small"); // Create a fake transparent transfer that should succeed - let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fake_source_fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a V5 transaction let transaction = Transaction::V5 { @@ -1447,7 +1512,12 @@ async fn v5_transaction_with_last_valid_expiry_height() { .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); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a non-coinbase V5 tx with the last valid expiry height. let transaction = Transaction::V5 { @@ -1602,7 +1672,12 @@ async fn v5_transaction_with_too_low_expiry_height() { .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); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // 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"); @@ -1648,7 +1723,12 @@ async fn v5_transaction_with_exceeding_expiry_height() { 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); + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // This expiry height exceeds the maximum defined by the specification. let expiry_height = block::Height(500_000_000); @@ -1753,7 +1833,12 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() { (transaction_block_height - 1).expect("fake source fund block height is too small"); // Create a fake transparent transfer that should not succeed - let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, false, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fake_source_fund_height, + false, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a V5 transaction let transaction = Transaction::V5 { @@ -1805,7 +1890,12 @@ async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() { (transaction_block_height - 1).expect("fake source fund block height is too small"); // Create a fake transparent transfer that should succeed - let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, true, 0); + let (input, output, known_utxos) = mock_transparent_transfer( + fake_source_fund_height, + true, + 0, + Amount::try_from(1).expect("invalid value"), + ); // Create a V4 transaction let transaction = Transaction::V5 { @@ -2263,6 +2353,7 @@ fn mock_transparent_transfer( previous_utxo_height: block::Height, script_should_succeed: bool, outpoint_index: u32, + previous_output_value: Amount, ) -> ( transparent::Input, transparent::Output, @@ -2286,7 +2377,7 @@ fn mock_transparent_transfer( }; let previous_output = transparent::Output { - value: Amount::try_from(1).expect("1 is an invalid amount"), + value: previous_output_value, lock_script, }; @@ -2726,3 +2817,145 @@ fn shielded_outputs_are_not_decryptable_for_fake_v5_blocks() { ); } } + +#[tokio::test] +async fn mempool_zip317_error() { + let mut state: MockService<_, _, _, _> = MockService::build().for_prop_tests(); + let verifier = Verifier::new(Network::Mainnet, state.clone()); + + let height = NetworkUpgrade::Nu5 + .activation_height(Network::Mainnet) + .expect("Nu5 activation height is specified"); + let fund_height = (height - 1).expect("fake source fund block height is too small"); + + // Will produce a small enough miner fee to fail the check. + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10).expect("invalid value"), + ); + + // Create a non-coinbase V5 tx. + let tx = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + network_upgrade: NetworkUpgrade::Nu5, + expiry_height: height, + sapling_shielded_data: None, + orchard_shielded_data: None, + }; + + let input_outpoint = match tx.inputs()[0] { + transparent::Input::PrevOut { outpoint, .. } => outpoint, + transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"), + }; + + tokio::spawn(async move { + state + .expect_request(zebra_state::Request::UnspentBestChainUtxo(input_outpoint)) + .await + .expect("verifier should call mock state service with correct request") + .respond(zebra_state::Response::UnspentBestChainUtxo( + known_utxos + .get(&input_outpoint) + .map(|utxo| utxo.utxo.clone()), + )); + + state + .expect_request_that(|req| { + matches!( + req, + zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_) + ) + }) + .await + .expect("verifier should call mock state service with correct request") + .respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors); + }); + + let verifier_response = verifier + .oneshot(Request::Mempool { + transaction: tx.into(), + height, + }) + .await; + + // Mempool refuses to add this transaction into storage. + assert!(verifier_response.is_err()); + assert_eq!( + verifier_response.err(), + Some(TransactionError::Zip317(zip317::Error::FeeBelowMinimumRate)) + ); +} + +#[tokio::test] +async fn mempool_zip317_ok() { + let mut state: MockService<_, _, _, _> = MockService::build().for_prop_tests(); + let verifier = Verifier::new(Network::Mainnet, state.clone()); + + let height = NetworkUpgrade::Nu5 + .activation_height(Network::Mainnet) + .expect("Nu5 activation height is specified"); + let fund_height = (height - 1).expect("fake source fund block height is too small"); + + // Will produce a big enough miner fee to pass the check. + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10001).expect("invalid value"), + ); + + // Create a non-coinbase V5 tx. + let tx = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + network_upgrade: NetworkUpgrade::Nu5, + expiry_height: height, + sapling_shielded_data: None, + orchard_shielded_data: None, + }; + + let input_outpoint = match tx.inputs()[0] { + transparent::Input::PrevOut { outpoint, .. } => outpoint, + transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"), + }; + + tokio::spawn(async move { + state + .expect_request(zebra_state::Request::UnspentBestChainUtxo(input_outpoint)) + .await + .expect("verifier should call mock state service with correct request") + .respond(zebra_state::Response::UnspentBestChainUtxo( + known_utxos + .get(&input_outpoint) + .map(|utxo| utxo.utxo.clone()), + )); + + state + .expect_request_that(|req| { + matches!( + req, + zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_) + ) + }) + .await + .expect("verifier should call mock state service with correct request") + .respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors); + }); + + let verifier_response = verifier + .oneshot(Request::Mempool { + transaction: tx.into(), + height, + }) + .await; + + assert!( + verifier_response.is_ok(), + "expected successful verification, got: {verifier_response:?}" + ); +} diff --git a/zebra-consensus/src/transaction/tests/prop.rs b/zebra-consensus/src/transaction/tests/prop.rs index aaddb3649d6..a2c22377464 100644 --- a/zebra-consensus/src/transaction/tests/prop.rs +++ b/zebra-consensus/src/transaction/tests/prop.rs @@ -7,6 +7,7 @@ use proptest::{collection::vec, prelude::*}; use tower::ServiceExt; use zebra_chain::{ + amount::Amount, block, parameters::{Network, NetworkUpgrade}, serialization::arbitrary::{datetime_full, datetime_u32}, @@ -387,8 +388,12 @@ fn mock_transparent_transfers( .try_into() .expect("too many mock transparent transfers requested"); - let (input, output, new_utxos) = - mock_transparent_transfer(fake_source_fund_height, true, outpoint_index); + let (input, output, new_utxos) = mock_transparent_transfer( + fake_source_fund_height, + true, + outpoint_index, + Amount::try_from(1).expect("invalid value"), + ); inputs.push(input); outputs.push(output); diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs b/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs index 2b086522667..3737f339ef0 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs @@ -15,7 +15,7 @@ use zebra_chain::{ amount::NegativeOrZero, block::{Height, MAX_BLOCK_BYTES}, parameters::Network, - transaction::VerifiedUnminedTx, + transaction::{zip317::BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT, VerifiedUnminedTx}, transparent, }; use zebra_consensus::MAX_BLOCK_SIGOPS; @@ -24,10 +24,6 @@ use crate::methods::get_block_template_rpcs::{ get_block_template::generate_coinbase_transaction, types::transaction::TransactionTemplate, }; -/// The ZIP-317 recommended limit on the number of unpaid actions per block. -/// `block_unpaid_action_limit` in ZIP-317. -pub const BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT: u32 = 50; - /// Selects mempool transactions for block production according to [ZIP-317], /// using a fake coinbase transaction and the mempool. /// diff --git a/zebrad/src/components/inbound/tests/fake_peer_set.rs b/zebrad/src/components/inbound/tests/fake_peer_set.rs index 9a9b797c027..1a226390c82 100644 --- a/zebrad/src/components/inbound/tests/fake_peer_set.rs +++ b/zebrad/src/components/inbound/tests/fake_peer_set.rs @@ -164,11 +164,14 @@ async fn mempool_push_transaction() -> Result<(), crate::BoxError> { .expect("unexpected non-mempool request"); // Set a dummy fee and sigops. - responder.respond(transaction::Response::from(VerifiedUnminedTx::new( - transaction, - Amount::zero(), - 0, - ))); + responder.respond(transaction::Response::from( + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )); }); let (push_response, _) = futures::join!(request, verification); @@ -266,11 +269,14 @@ async fn mempool_advertise_transaction_ids() -> Result<(), crate::BoxError> { .expect("unexpected non-mempool request"); // Set a dummy fee and sigops. - responder.respond(transaction::Response::from(VerifiedUnminedTx::new( - transaction, - Amount::zero(), - 0, - ))); + responder.respond(transaction::Response::from( + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )); }); let (advertise_response, _, _) = futures::join!(request, peer_set_responder, verification); @@ -365,11 +371,14 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> { .expect("unexpected non-mempool request"); // Set a dummy fee and sigops. - responder.respond(transaction::Response::from(VerifiedUnminedTx::new( - transaction, - Amount::zero(), - 0, - ))); + responder.respond(transaction::Response::from( + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )); }); let (push_response, _) = futures::join!(request, verification); @@ -499,11 +508,14 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> { .expect("unexpected non-mempool request"); // Set a dummy fee and sigops. - responder.respond(transaction::Response::from(VerifiedUnminedTx::new( - transaction, - Amount::zero(), - 0, - ))); + responder.respond(transaction::Response::from( + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )); }); let (push_response, _) = futures::join!(request, verification); diff --git a/zebrad/src/components/mempool/storage/tests.rs b/zebrad/src/components/mempool/storage/tests.rs index 8fdf5431a39..cad844108c4 100644 --- a/zebrad/src/components/mempool/storage/tests.rs +++ b/zebrad/src/components/mempool/storage/tests.rs @@ -37,5 +37,12 @@ pub fn unmined_transactions_in_blocks( selected_blocks .flat_map(|block| block.transactions) .map(UnminedTx::from) - .map(|transaction| VerifiedUnminedTx::new(transaction, Amount::zero(), 0)) + .map(|transaction| { + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass") + }) } diff --git a/zebrad/src/components/mempool/storage/tests/prop.rs b/zebrad/src/components/mempool/storage/tests/prop.rs index 9b3a447c9e0..eca65935acb 100644 --- a/zebrad/src/components/mempool/storage/tests/prop.rs +++ b/zebrad/src/components/mempool/storage/tests/prop.rs @@ -475,8 +475,20 @@ impl SpendConflictTestInput { }; ( - VerifiedUnminedTx::new(first.0.into(), Amount::zero(), 0), - VerifiedUnminedTx::new(second.0.into(), Amount::zero(), 0), + VerifiedUnminedTx::new( + first.0.into(), + // make sure miner fee is big enough for all cases + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + VerifiedUnminedTx::new( + second.0.into(), + // make sure miner fee is big enough for all cases + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), ) } @@ -493,8 +505,20 @@ impl SpendConflictTestInput { Self::remove_orchard_conflicts(&mut first, &mut second); ( - VerifiedUnminedTx::new(first.0.into(), Amount::zero(), 0), - VerifiedUnminedTx::new(second.0.into(), Amount::zero(), 0), + VerifiedUnminedTx::new( + first.0.into(), + // make sure miner fee is big enough for all cases + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + VerifiedUnminedTx::new( + second.0.into(), + // make sure miner fee is big enough for all cases + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), ) } diff --git a/zebrad/src/components/mempool/storage/tests/vectors.rs b/zebrad/src/components/mempool/storage/tests/vectors.rs index 5977662f19b..35827574946 100644 --- a/zebrad/src/components/mempool/storage/tests/vectors.rs +++ b/zebrad/src/components/mempool/storage/tests/vectors.rs @@ -271,7 +271,14 @@ fn mempool_expired_basic_for_network(network: Network) -> Result<()> { let tx_id = tx.unmined_id(); // Insert the transaction into the mempool, with a fake zero miner fee and sigops - storage.insert(VerifiedUnminedTx::new(tx.into(), Amount::zero(), 0))?; + storage.insert( + VerifiedUnminedTx::new( + tx.into(), + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )?; assert_eq!(storage.transaction_count(), 1); diff --git a/zebrad/src/components/mempool/tests/vector.rs b/zebrad/src/components/mempool/tests/vector.rs index 3dad0f1a466..4dbed9426bb 100644 --- a/zebrad/src/components/mempool/tests/vector.rs +++ b/zebrad/src/components/mempool/tests/vector.rs @@ -805,11 +805,14 @@ async fn mempool_reverifies_after_tip_change() -> Result<(), Report> { .expect("unexpected non-mempool request"); // Set a dummy fee and sigops. - responder.respond(transaction::Response::from(VerifiedUnminedTx::new( - transaction, - Amount::zero(), - 0, - ))); + responder.respond(transaction::Response::from( + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )); }) .await; @@ -862,11 +865,14 @@ async fn mempool_reverifies_after_tip_change() -> Result<(), Report> { .expect("unexpected non-mempool request"); // Set a dummy fee and sigops. - responder.respond(transaction::Response::from(VerifiedUnminedTx::new( - transaction, - Amount::zero(), - 0, - ))); + responder.respond(transaction::Response::from( + VerifiedUnminedTx::new( + transaction, + Amount::try_from(1_000_000).expect("invalid value"), + 0, + ) + .expect("verification should pass"), + )); }) .await;