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(rpc): Select getblocktemplate RPC transactions according to ZIP-317 #5724

Merged
merged 7 commits into from
Dec 1, 2022
Merged
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 23 additions & 3 deletions zebra-chain/src/transaction/unmined.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl From<&Arc<Transaction>> for UnminedTx {
/// A verified unmined transaction, and the corresponding transaction fee.
///
/// This transaction has been fully verified, in the context of the mempool.
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
pub struct VerifiedUnminedTx {
/// The unmined transaction.
Expand All @@ -302,6 +302,14 @@ pub struct VerifiedUnminedTx {
/// The number of legacy signature operations in this transaction's
/// transparent inputs and outputs.
pub legacy_sigop_count: u64,

/// The block production fee weight for `transaction`, as defined by [ZIP-317].
///
/// This is not consensus-critical, so we use `f32` for efficient calculations
/// when the mempool holds a large number of transactions.
///
/// [ZIP-317]: https://zips.z.cash/zip-0317#block-production
pub block_production_fee_weight: f32,
}

impl fmt::Display for VerifiedUnminedTx {
Expand All @@ -315,19 +323,31 @@ impl fmt::Display for VerifiedUnminedTx {
}

impl VerifiedUnminedTx {
/// Create a new verified unmined transaction from a transaction, its fee and the legacy sigop count.
/// Create a new verified unmined transaction from an unmined transaction,
/// its miner fee, and its legacy sigop count.
pub fn new(
transaction: UnminedTx,
miner_fee: Amount<NonNegative>,
legacy_sigop_count: u64,
) -> Self {
let block_production_fee_weight =
zip317::block_production_fee_weight(&transaction, miner_fee);

Self {
transaction,
miner_fee,
legacy_sigop_count,
block_production_fee_weight,
}
}

/// Returns `true` if the transaction pays at least the [ZIP-317] conventional fee.
///
/// [ZIP-317]: https://zips.z.cash/zip-0317#mempool-size-limiting
pub fn pays_conventional_fee(&self) -> bool {
self.miner_fee >= self.transaction.conventional_fee
}

/// The cost in bytes of the transaction, as defined in [ZIP-401].
///
/// A reflection of the work done by the network in processing them (proof
Expand Down Expand Up @@ -365,7 +385,7 @@ impl VerifiedUnminedTx {
pub fn eviction_weight(&self) -> u64 {
let mut cost = self.cost();

if self.miner_fee < self.transaction.conventional_fee {
if !self.pays_conventional_fee() {
cost += MEMPOOL_TRANSACTION_LOW_FEE_PENALTY
}

Expand Down
43 changes: 36 additions & 7 deletions zebra-chain/src/transaction/unmined/zip317.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
//! The [ZIP-317 conventional fee calculation](https://zips.z.cash/zip-0317#fee-calculation)
//! for [UnminedTx]s.
//! An implementation of the [ZIP-317] fee calculations for [UnminedTx]s:
//! - [conventional fee](https://zips.z.cash/zip-0317#fee-calculation)
//! - [block production transaction weight](https://zips.z.cash/zip-0317#block-production)

use std::cmp::max;

use crate::{
amount::{Amount, NonNegative},
serialization::ZcashSerialize,
transaction::Transaction,
transaction::{Transaction, UnminedTx},
};

// For doc links
#[allow(unused_imports)]
use crate::transaction::UnminedTx;

/// The marginal fee for the ZIP-317 fee calculation, in zatoshis per logical action.
//
// TODO: allow Amount<NonNegative> in constants
Expand All @@ -27,6 +24,26 @@ const P2PKH_STANDARD_INPUT_SIZE: usize = 150;
/// The standard size of p2pkh outputs for the ZIP-317 fee calculation, in bytes.
const P2PKH_STANDARD_OUTPUT_SIZE: usize = 34;

/// The recommended weight cap for ZIP-317 block production.
const MAX_BLOCK_PRODUCTION_WEIGHT: f32 = 4.0;

/// Zebra's custom minimum weight for ZIP-317 block production,
/// based on half the [ZIP-203] recommended transaction expiry height of 40 blocks.
///
/// This ensures all transactions have a non-zero probability of being mined,
/// which simplifies our implementation.
///
/// If blocks are full, this makes it likely that very low fee transactions
/// will be mined:
/// - after approximately 20 blocks delay,
/// - but before they expire.
///
/// Note: Small transactions that pay the legacy ZIP-313 conventional fee have twice this weight.
/// If blocks are full, they will be mined after approximately 10 blocks delay.
///
/// [ZIP-203]: https://zips.z.cash/zip-0203#changes-for-blossom>
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
const MIN_BLOCK_PRODUCTION_WEIGHT: f32 = 1.0 / 20.0;

/// 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 @@ -72,6 +89,18 @@ pub fn conventional_fee(transaction: &Transaction) -> Amount<NonNegative> {
conventional_fee.expect("conventional fee is positive and limited by serialized size limit")
}

/// Returns the block production fee weight for `transaction`, as defined by [ZIP-317].
///
/// [ZIP-317]: https://zips.z.cash/zip-0317#block-production
pub fn block_production_fee_weight(transaction: &UnminedTx, miner_fee: Amount<NonNegative>) -> f32 {
let miner_fee = i64::from(miner_fee) as f32;
let conventional_fee = i64::from(transaction.conventional_fee) as f32;

let uncapped_weight = miner_fee / conventional_fee;

uncapped_weight.clamp(MIN_BLOCK_PRODUCTION_WEIGHT, MAX_BLOCK_PRODUCTION_WEIGHT)
}

/// Divide `quotient` by `divisor`, rounding the result up to the nearest integer.
///
/// # Correctness
Expand Down
2 changes: 1 addition & 1 deletion zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ pub enum Request {

/// The response type for the transaction verifier service.
/// Responses identify the transaction that was verified.
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub enum Response {
/// A response to a block transaction verification request.
Block {
Expand Down
5 changes: 5 additions & 0 deletions zebra-rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ default = []

# Experimental mining RPC support
getblocktemplate-rpcs = [
"rand",
"zebra-consensus/getblocktemplate-rpcs",
"zebra-state/getblocktemplate-rpcs",
"zebra-node-services/getblocktemplate-rpcs",
Expand Down Expand Up @@ -54,6 +55,10 @@ tracing-futures = "0.2.5"
hex = { version = "0.4.3", features = ["serde"] }
serde = { version = "1.0.147", features = ["serde_derive"] }

# Experimental feature getblocktemplate-rpcs
rand = { version = "0.8.5", package = "rand", optional = true }

# Test-only feature proptest-impl
proptest = { version = "0.10.1", optional = true }
proptest-derive = { version = "0.3.0", optional = true }

Expand Down
35 changes: 3 additions & 32 deletions zebra-rpc/src/methods/get_block_template_rpcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ use crate::methods::{

pub mod config;
pub mod constants;

pub(crate) mod types;
pub(crate) mod zip317;

/// The max estimated distance to the chain tip for the getblocktemplate method
// Set to 30 in case the local time is a little ahead.
Expand Down Expand Up @@ -310,7 +312,7 @@ where
});
}

let mempool_txs = select_mempool_transactions(mempool).await?;
let mempool_txs = zip317::select_mempool_transactions(mempool).await?;

let miner_fee = miner_fee(&mempool_txs);

Expand Down Expand Up @@ -448,37 +450,6 @@ where

// get_block_template support methods

/// Returns selected transactions in the `mempool`, or an error if the mempool has failed.
///
/// TODO: select transactions according to ZIP-317 (#5473)
pub async fn select_mempool_transactions<Mempool>(
mempool: Mempool,
) -> Result<Vec<VerifiedUnminedTx>>
where
Mempool: Service<
mempool::Request,
Response = mempool::Response,
Error = zebra_node_services::BoxError,
> + 'static,
Mempool::Future: Send,
{
let response = mempool
.oneshot(mempool::Request::FullTransactions)
.await
.map_err(|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
})?;

if let mempool::Response::FullTransactions(transactions) = response {
// TODO: select transactions according to ZIP-317 (#5473)
Ok(transactions)
} else {
unreachable!("unmatched response to a mempool::FullTransactions request");
}
}

/// Returns the total miner fee for `mempool_txs`.
pub fn miner_fee(mempool_txs: &[VerifiedUnminedTx]) -> Amount<NonNegative> {
let miner_fee: amount::Result<Amount<NonNegative>> =
Expand Down
Loading