diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e1e841ba73..361b5498e41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Fixed a bug with trailing characters in the openapi spec method descriptions ([#8597](https://github.com/ZcashFoundation/zebra/pull/8597)) - Added default constructions for several RPC method responses ([#8616](https://github.com/ZcashFoundation/zebra/pull/8616)) - Added Windows to the list of supported platforms in Tier 2 ([#8637](https://github.com/ZcashFoundation/zebra/pull/8637)) +- Zebra now drops transactions with unpaid actions in block templates and from the mempool. ## [Zebra 1.7.0](https://github.com/ZcashFoundation/zebra/releases/tag/v1.7.0) - 2024-05-07 diff --git a/zebra-chain/src/transaction/unmined/zip317.rs b/zebra-chain/src/transaction/unmined/zip317.rs index e9f4a757e53..b9f04ec7597 100644 --- a/zebra-chain/src/transaction/unmined/zip317.rs +++ b/zebra-chain/src/transaction/unmined/zip317.rs @@ -41,9 +41,13 @@ 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; +/// If a tx has more than `BLOCK_UNPAID_ACTION_LIMIT` "unpaid actions", it will never be mined by +/// the [_Recommended algorithm for block template construction_][alg-def], implemented in Zebra +/// [here][alg-impl]. +/// +/// [alg-def]: https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction +/// [alg-impl]: https://github.com/zcashfoundation/zebra/blob/95e4d0973caac075b47589f6a05f9d744acd3db3/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs#L39 +pub const BLOCK_UNPAID_ACTION_LIMIT: u32 = 0; /// The minimum fee per kilobyte for Zebra mempool transactions. /// Also used as the minimum fee for a mempool transaction. @@ -178,7 +182,7 @@ pub fn mempool_checks( // > Nodes MAY drop these transactions. // // - if unpaid_actions > BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT { + if unpaid_actions > BLOCK_UNPAID_ACTION_LIMIT { return Err(Error::UnpaidActions); } @@ -198,6 +202,12 @@ pub fn mempool_checks( // In zcashd this is `DEFAULT_MIN_RELAY_TX_FEE` and `LEGACY_DEFAULT_FEE`: // // + // + // ## Note + // + // If the check above for the maximum number of unpaid actions passes with + // [`BLOCK_UNPAID_ACTION_LIMIT`] set to zero, then there is no way for the legacy check below to + // fail. This renders the legacy check redundant in that case. const KILOBYTE: usize = 1000; diff --git a/zebra-chain/src/transaction/unmined/zip317/tests.rs b/zebra-chain/src/transaction/unmined/zip317/tests.rs index fb708a73c0b..883a596ef19 100644 --- a/zebra-chain/src/transaction/unmined/zip317/tests.rs +++ b/zebra-chain/src/transaction/unmined/zip317/tests.rs @@ -3,7 +3,7 @@ use super::{mempool_checks, Amount, Error}; #[test] fn zip317_unpaid_actions_err() { - let check = mempool_checks(51, Amount::try_from(1).unwrap(), 1); + let check = mempool_checks(1, Amount::try_from(1).unwrap(), 1); assert!(check.is_err()); assert_eq!(check.err(), Some(Error::UnpaidActions)); @@ -11,8 +11,13 @@ fn zip317_unpaid_actions_err() { #[test] fn zip317_minimum_rate_fee_err() { - let check = mempool_checks(50, Amount::try_from(1).unwrap(), 1000); + let check = mempool_checks(0, Amount::try_from(1).unwrap(), 1000); assert!(check.is_err()); assert_eq!(check.err(), Some(Error::FeeBelowMinimumRate)); } + +#[test] +fn zip317_mempool_checks_ok() { + assert!(mempool_checks(0, Amount::try_from(100).unwrap(), 1000).is_ok()) +} diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 34244b38b47..0a4c21bb039 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -2945,7 +2945,7 @@ async fn mempool_zip317_error() { fund_height, true, 0, - Amount::try_from(10).expect("invalid value"), + Amount::try_from(10).expect("valid amount"), ); // Create a non-coinbase V5 tx. @@ -2998,7 +2998,7 @@ async fn mempool_zip317_error() { assert!(verifier_response.is_err()); assert_eq!( verifier_response.err(), - Some(TransactionError::Zip317(zip317::Error::FeeBelowMinimumRate)) + Some(TransactionError::Zip317(zip317::Error::UnpaidActions)) ); } @@ -3017,7 +3017,7 @@ async fn mempool_zip317_ok() { fund_height, true, 0, - Amount::try_from(10001).expect("invalid value"), + Amount::try_from(10_001).expect("valid amount"), ); // Create a non-coinbase V5 tx. 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 4a6cf92c0d3..3f0979dc266 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::{zip317::BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT, VerifiedUnminedTx}, + transaction::{zip317::BLOCK_UNPAID_ACTION_LIMIT, VerifiedUnminedTx}, transparent, }; use zebra_consensus::MAX_BLOCK_SIGOPS; @@ -64,7 +64,7 @@ pub async fn select_mempool_transactions( // Set up limit tracking let mut remaining_block_bytes: usize = MAX_BLOCK_BYTES.try_into().expect("fits in memory"); let mut remaining_block_sigops = MAX_BLOCK_SIGOPS; - let mut remaining_block_unpaid_actions: u32 = BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT; + let mut remaining_block_unpaid_actions: u32 = BLOCK_UNPAID_ACTION_LIMIT; // Adjust the limits based on the coinbase transaction remaining_block_bytes -= fake_coinbase_tx.data.as_ref().len();