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

feat(mempool): add ZIP-317 rules to mempool #6556

Merged
merged 18 commits into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from 16 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
4 changes: 3 additions & 1 deletion zebra-chain/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
15 changes: 11 additions & 4 deletions zebra-chain/src/transaction/unmined.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -353,17 +353,24 @@ impl VerifiedUnminedTx {
transaction: UnminedTx,
miner_fee: Amount<NonNegative>,
legacy_sigop_count: u64,
) -> Self {
) -> Result<Self, zip317::Error> {
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.conventional_fee,
transaction.size,
)?;

teor2345 marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
69 changes: 69 additions & 0 deletions zebra-chain/src/transaction/unmined/zip317.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use std::cmp::max;

use num_integer::div_ceil;
use thiserror::Error;

use crate::{
amount::{Amount, NonNegative},
Expand All @@ -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<NonNegative> in constants
Expand All @@ -37,6 +41,10 @@ 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;

/// Returns the conventional fee for `transaction`, as defined by [ZIP-317].
///
/// [ZIP-317]: https://zips.z.cash/zip-0317#fee-calculation
Expand Down Expand Up @@ -139,3 +147,64 @@ 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<NonNegative>,
conventional_fee: Amount<NonNegative>,
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.
//
// <https://zips.z.cash/zip-0317#transaction-relaying>
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.
//
// <https://zips.z.cash/zip-0317#transaction-relaying>
if miner_fee < conventional_fee {
return Err(Error::FeeBelowConventional);
}

// # Standard rule
//
// > Minimum Fee Rate
// >
// > 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`.
//
// <https://github.com/ZcashFoundation/zebra/issues/5336#issuecomment-1506748801>
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
let limit = std::cmp::min(
1000,
(f32::floor(100.0 * transaction_size as f32 / 1000.0)) as u64,
);
if miner_fee < Amount::<NonNegative>::try_from(limit).expect("limit value is invalid") {
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
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 conventional fee for the transaction")]
FeeBelowConventional,

#[error("Transaction fee is below the minimum fee rate")]
FeeBelowMinimumRate,
}
41 changes: 41 additions & 0 deletions zebra-chain/src/transaction/unmined/zip317/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//! 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(),
Amount::try_from(10000).unwrap(),
1,
);

assert!(check.is_err());
assert_eq!(check.err(), Some(Error::UnpaidActions));
}

#[test]
fn zip317_convetional_fee_err() {
let check = mempool_checks(
50,
Amount::try_from(1).unwrap(),
Amount::try_from(10000).unwrap(),
1,
);

assert!(check.is_err());
assert_eq!(check.err(), Some(Error::FeeBelowConventional));
}

#[test]
fn zip317_minimum_rate_fee_err() {
let check = mempool_checks(
50,
Amount::try_from(1).unwrap(),
Amount::try_from(1).unwrap(),
1000,
);

assert!(check.is_err());
assert_eq!(check.err(), Some(Error::FeeBelowMinimumRate));
}
4 changes: 4 additions & 0 deletions zebra-consensus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ValidateContextError> for TransactionError {
Expand Down
7 changes: 4 additions & 3 deletions zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
},
};

Expand Down
Loading
Loading