Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change(chain): Drop transactions with unpaid actions #8638

Merged
merged 5 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
but only feature flags instead.
- 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))
- Zebra now drops transactions with unpaid actions.
upbqdn marked this conversation as resolved.
Show resolved Hide resolved

## [Zebra 1.7.0](https://github.com/ZcashFoundation/zebra/releases/tag/v1.7.0) - 2024-05-07

Expand Down
17 changes: 13 additions & 4 deletions zebra-chain/src/transaction/unmined/zip317.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -178,7 +182,7 @@ pub fn mempool_checks(
// > Nodes MAY drop these transactions.
//
// <https://zips.z.cash/zip-0317#transaction-relaying>
if unpaid_actions > BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT {
if unpaid_actions > BLOCK_UNPAID_ACTION_LIMIT {
return Err(Error::UnpaidActions);
}

Expand All @@ -198,6 +202,11 @@ pub fn mempool_checks(
// In zcashd this is `DEFAULT_MIN_RELAY_TX_FEE` and `LEGACY_DEFAULT_FEE`:
// <https://github.com/zcash/zcash/blob/f512291ff20098291442e83713de89bcddc07546/src/main.h#L71-L72>
// <https://github.com/zcash/zcash/blob/9e856cfc5b81aa2607a16a23ff5584ea10014de6/src/amount.h#L35-L36>
//
// ## Note
//
// If the check above, which checks the number of unpaid actions, doesn't fail, then there is no
// way for the legacy check below to fail. This renders the legacy check redundant.
upbqdn marked this conversation as resolved.
Show resolved Hide resolved

const KILOBYTE: usize = 1000;

Expand Down
9 changes: 7 additions & 2 deletions zebra-chain/src/transaction/unmined/zip317/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@
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));
}

#[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())
}
6 changes: 3 additions & 3 deletions zebra-consensus/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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))
);
}

Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Loading