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 1 commit
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
19 changes: 15 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 @@ -335,6 +335,9 @@ impl fmt::Display for VerifiedUnminedTx {
.finish()
}
}
/// 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;
teor2345 marked this conversation as resolved.
Show resolved Hide resolved

impl VerifiedUnminedTx {
/// Create a new verified unmined transaction from an unmined transaction,
Expand All @@ -343,17 +346,25 @@ 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 {
// Make the mempool checks.
zip317::mempool_checks(
unpaid_actions,
BLOCK_PRODUCTION_UNPAID_ACTION_LIMIT,
miner_fee,
transaction.conventional_fee,
)?;

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
36 changes: 36 additions & 0 deletions zebra-chain/src/transaction/unmined/zip317.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@

use std::cmp::max;

use thiserror::Error;

use crate::{
amount::{Amount, NonNegative},
block::MAX_BLOCK_BYTES,
serialization::ZcashSerialize,
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 Down Expand Up @@ -153,3 +158,34 @@ fn div_ceil(quotient: usize, divisor: usize) -> usize {
// as long as the addition doesn't overflow or underflow.
(quotient + divisor - 1) / divisor
}

/// Make ZIP-317 checks before inserting a transaction into the mempool.
pub fn mempool_checks(
unpaid_actions: u32,
unpaid_action_limit: u32,
miner_fee: Amount<NonNegative>,
conventional_fee: Amount<NonNegative>,
) -> Result<(), Error> {
// Check unpaid actions is below the thershold.
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved
if unpaid_actions > unpaid_action_limit {
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
return Err(Error::UnpaidActions);
}

// Check the fee is not below the calculated conventional fee for the transaction.
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
if miner_fee < conventional_fee {
return Err(Error::MinerFee);
}

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 too low")]
MinerFee,
}
28 changes: 28 additions & 0 deletions zebra-chain/src/transaction/unmined/zip317/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//! ZIP-317 tests.

use super::{mempool_checks, Amount, Error};
#[test]
fn zip317_unpaid_actions_err() {
let check = mempool_checks(
51,
50,
Amount::try_from(1).unwrap(),
Amount::try_from(10000).unwrap(),
);

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

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

assert!(check.is_err());
assert_eq!(check.err(), Some(Error::MinerFee));
}
4 changes: 4 additions & 0 deletions zebra-consensus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ pub enum TransactionError {
#[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
// TODO: turn this into a typed error
ValidateMempoolLockTimeError(String),

#[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 @@ -460,14 +460,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