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

fix(rpc): Omit transactions with transparent coinbase spends that are immature at the next block height from block templates #6510

Merged
merged 8 commits into from
Apr 18, 2023
14 changes: 14 additions & 0 deletions zebra-chain/src/transaction/unmined.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::{fmt, sync::Arc};

use crate::{
amount::{Amount, NonNegative},
block::Height,
serialization::ZcashSerialize,
transaction::{
AuthDigest, Hash,
Expand Down Expand Up @@ -322,6 +323,10 @@ pub struct VerifiedUnminedTx {
///
/// [ZIP-317]: https://zips.z.cash/zip-0317#block-production
pub fee_weight_ratio: f32,

/// The earliest/lowest block height at which this transaction's transparent coinbase
/// spends will be valid, if this transaction contains any transparent coinbase spends.
pub maturity_height: Option<Height>,
}

impl fmt::Display for VerifiedUnminedTx {
Expand All @@ -343,6 +348,7 @@ impl VerifiedUnminedTx {
transaction: UnminedTx,
miner_fee: Amount<NonNegative>,
legacy_sigop_count: u64,
maturity_height: Option<Height>,
) -> Self {
let fee_weight_ratio = zip317::conventional_fee_weight_ratio(&transaction, miner_fee);
let unpaid_actions = zip317::unpaid_actions(&transaction, miner_fee);
Expand All @@ -353,6 +359,7 @@ impl VerifiedUnminedTx {
legacy_sigop_count,
fee_weight_ratio,
unpaid_actions,
maturity_height,
}
}

Expand All @@ -363,6 +370,13 @@ impl VerifiedUnminedTx {
self.miner_fee >= self.transaction.conventional_fee
}

/// Returns `true` if the transaction will be mature at the given block height.
pub fn is_mature_at(&self, height: Height) -> bool {
self.maturity_height
.map(|maturity_height| maturity_height <= height)
.unwrap_or(true)
}

/// 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
30 changes: 29 additions & 1 deletion zebra-consensus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
use chrono::{DateTime, Utc};
use thiserror::Error;

use zebra_chain::{amount, block, orchard, sapling, sprout, transparent};
use zebra_chain::{
amount, block, orchard, sapling, sprout,
transparent::{self, MIN_TRANSPARENT_COINBASE_MATURITY},
};
use zebra_state::ValidateContextError;

use crate::{block::MAX_BLOCK_SIGOPS, BoxError};
Expand Down Expand Up @@ -193,6 +196,31 @@ pub enum TransactionError {
#[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
// TODO: turn this into a typed error
ValidateMempoolLockTimeError(String),

#[error(
"immature transparent coinbase spend: \
attempt to spend {outpoint:?} at {spend_height:?}, \
but spends are invalid before {min_spend_height:?}, \
which is {MIN_TRANSPARENT_COINBASE_MATURITY:?} blocks \
after it was created at {created_height:?}"
)]
#[non_exhaustive]
ImmatureTransparentCoinbaseSpend {
outpoint: transparent::OutPoint,
spend_height: block::Height,
min_spend_height: block::Height,
created_height: block::Height,
},

#[error(
"unshielded transparent coinbase spend: {outpoint:?} \
must be spent in a transaction which only has shielded outputs"
)]
#[non_exhaustive]
UnshieldedTransparentCoinbaseSpend {
outpoint: transparent::OutPoint,
min_spend_height: block::Height,
},
}

impl From<ValidateContextError> for TransactionError {
Expand Down
30 changes: 27 additions & 3 deletions zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,28 @@ impl Request {

/// Returns true if the request is a mempool request.
pub fn is_mempool(&self) -> bool {
matches!(self, Request::Mempool { .. })
arya2 marked this conversation as resolved.
Show resolved Hide resolved
}

/// Returns lowest height at which all transparent coinbase spends will be valid,
/// or None if this transaction has no transparent coinbase spends.
pub fn maturity_height(
&self,
spent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
) -> Option<block::Height> {
match self {
Request::Block { .. } => false,
Request::Mempool { .. } => true,
// TODO: return an error for Request::Block to replace this check in the state (#2336)
Request::Block { .. } => None,
teor2345 marked this conversation as resolved.
Show resolved Hide resolved

Request::Mempool {
transaction,
height,
} => check::tx_transparent_coinbase_spends_maturity(
transaction.transaction.clone(),
*height,
Default::default(),
spent_utxos,
),
}
}
}
Expand Down Expand Up @@ -420,7 +439,10 @@ where
unmined_tx,
))
.map(|res| {
assert!(res? == zs::Response::ValidBestChainTipNullifiersAndAnchors, "unexpected response to CheckBestChainTipNullifiersAndAnchors request");
assert!(
res? == zs::Response::ValidBestChainTipNullifiersAndAnchors,
"unexpected response to CheckBestChainTipNullifiersAndAnchors request"
);
Ok(())
}
);
Expand Down Expand Up @@ -453,6 +475,7 @@ where
}

let legacy_sigop_count = cached_ffi_transaction.legacy_sigop_count()?;
let maturity_height = req.maturity_height(&spent_utxos);

let rsp = match req {
Request::Block { .. } => Response::Block {
Expand All @@ -467,6 +490,7 @@ where
"unexpected mempool coinbase transaction: should have already rejected",
),
legacy_sigop_count,
maturity_height,
),
},
};
Expand Down
113 changes: 112 additions & 1 deletion zebra-consensus/src/transaction/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
//!
//! Code in this file can freely assume that no pre-V4 transactions are present.

use std::{borrow::Cow, collections::HashSet, convert::TryFrom, hash::Hash};
use std::{
borrow::Cow,
collections::{HashMap, HashSet},
convert::TryFrom,
arya2 marked this conversation as resolved.
Show resolved Hide resolved
hash::Hash,
sync::Arc,
};

use chrono::{DateTime, Utc};

Expand All @@ -13,6 +19,11 @@ use zebra_chain::{
parameters::{Network, NetworkUpgrade},
primitives::zcash_note_encryption,
transaction::{LockTime, Transaction},
transparent::{self, MIN_TRANSPARENT_COINBASE_MATURITY},
};

use zebra_chain::transparent::CoinbaseSpendRestriction::{
OnlyShieldedOutputs, SomeTransparentOutputs,
};

use crate::error::TransactionError;
Expand Down Expand Up @@ -462,3 +473,103 @@ fn validate_expiry_height_mined(

Ok(())
}

/// Check that `utxo` is spendable, based on the coinbase `spend_restriction`.
///
/// # Consensus
///
/// > A transaction with one or more transparent inputs from coinbase transactions
/// > MUST have no transparent outputs (i.e. tx_out_count MUST be 0).
/// > Inputs from coinbase transactions include Founders’ Reward outputs and
/// > funding stream outputs.
///
/// > A transaction MUST NOT spend a transparent output of a coinbase transaction
/// > from a block less than 100 blocks prior to the spend.
/// > Note that transparent outputs of coinbase transactions include
/// > Founders’ Reward outputs and transparent funding stream outputs.
///
/// <https://zips.z.cash/protocol/protocol.pdf#txnconsensus>
// TODO: Move tests for `zebra_state::service::check::utxo::transparent_coinbase_spend()` to this crate
// once the transaction verifier returns an error for block transactions that fail this check.
fn transparent_coinbase_spend_maturity(
outpoint: transparent::OutPoint,
spend_restriction: transparent::CoinbaseSpendRestriction,
utxo: transparent::Utxo,
) -> Result<(), TransactionError> {
if !utxo.from_coinbase {
return Ok(());
}

let min_spend_height = utxo.height + MIN_TRANSPARENT_COINBASE_MATURITY.try_into().unwrap();
let min_spend_height =
min_spend_height.expect("valid UTXOs have coinbase heights far below Height::MAX");

match spend_restriction {
OnlyShieldedOutputs { spend_height } => {
if spend_height >= min_spend_height {
Ok(())
} else {
Err(TransactionError::ImmatureTransparentCoinbaseSpend {
outpoint,
spend_height,
min_spend_height,
created_height: utxo.height,
})
}
}
SomeTransparentOutputs => Err(TransactionError::UnshieldedTransparentCoinbaseSpend {
outpoint,
min_spend_height,
}),
}
}

/// Accepts a transaction, UTXOs in the same block, and spent UTXOs from the chain.
///
/// Returns the lowest/earlier height at which every transparent coinbase spend
/// in the provided [`Transaction`] will be mature,
///
/// Returns None if the transaction has no transparent coinbase spends.
pub fn tx_transparent_coinbase_spends_maturity(
arya2 marked this conversation as resolved.
Show resolved Hide resolved
tx: Arc<Transaction>,
height: Height,
block_new_outputs: Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>>,
spent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
) -> Option<Height> {
let mut maturity_height = None;

for spend in tx.spent_outpoints() {
let utxo = block_new_outputs
.get(&spend)
.map(|ordered_utxo| ordered_utxo.utxo.clone())
.or_else(|| spent_utxos.get(&spend).cloned())
.expect("load_spent_utxos_fut.await should return an error if a utxo is missing");

let spend_restriction = tx.coinbase_spend_restriction(height);

let check_coinbase_spend_result =
transparent_coinbase_spend_maturity(spend, spend_restriction, utxo);

match check_coinbase_spend_result {
Err(TransactionError::ImmatureTransparentCoinbaseSpend {
min_spend_height, ..
})
| Err(TransactionError::UnshieldedTransparentCoinbaseSpend {
min_spend_height, ..
}) if maturity_height
.map(|h| h < min_spend_height)
.unwrap_or(true) =>
{
maturity_height = Some(min_spend_height);
}

Err(TransactionError::ImmatureTransparentCoinbaseSpend { .. })
| Err(TransactionError::UnshieldedTransparentCoinbaseSpend { .. })
| Ok(()) => {}

Err(_other) => panic!("unexpected error from transparent_coinbase_spend_maturity"),
};
}

maturity_height
}
83 changes: 82 additions & 1 deletion zebra-consensus/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use zebra_chain::{
},
Hash, HashType, JoinSplitData, LockTime, Transaction,
},
transparent::{self, CoinbaseData},
transparent::{self, CoinbaseData, MIN_TRANSPARENT_COINBASE_MATURITY},
};

use zebra_test::mock_service::MockService;
Expand Down Expand Up @@ -564,6 +564,87 @@ async fn mempool_request_with_past_lock_time_is_accepted() {
);
}

#[tokio::test]
async fn mempool_response_maturity_height_correct() {
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
let _init_guard = zebra_test::init();

let mut state: MockService<_, _, _, _> = MockService::build().for_prop_tests();
let verifier = Verifier::new(Network::Mainnet, state.clone());

let height = NetworkUpgrade::Canopy
.activation_height(Network::Mainnet)
.expect("Canopy activation height is specified");
let fund_height = (height - 1).expect("fake source fund block height is too small");
let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0);

// Create a non-coinbase V4 tx with the last valid expiry height.
let tx = Transaction::V4 {
inputs: vec![input],
outputs: vec![output],
lock_time: LockTime::min_lock_time_timestamp(),
expiry_height: height,
joinsplit_data: None,
sapling_shielded_data: None,
};

let input_outpoint = match tx.inputs()[0] {
transparent::Input::PrevOut { outpoint, .. } => outpoint,
transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"),
};

let coinbase_spend_height = Height(5);

tokio::spawn(async move {
state
.expect_request(zebra_state::Request::BestChainNextMedianTimePast)
.await
.expect("verifier should call mock state service with correct request")
.respond(zebra_state::Response::BestChainNextMedianTimePast(
DateTime32::MAX,
));

state
.expect_request(zebra_state::Request::UnspentBestChainUtxo(input_outpoint))
.await
.expect("verifier should call mock state service with correct request")
.respond(zebra_state::Response::UnspentBestChainUtxo(
known_utxos.get(&input_outpoint).map(|utxo| {
let mut utxo = utxo.utxo.clone();
utxo.height = coinbase_spend_height;
utxo.from_coinbase = true;
utxo
}),
));

state
.expect_request_that(|req| {
matches!(
req,
zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_)
)
})
.await
.expect("verifier should call mock state service with correct request")
.respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors);
});

let verifier_response = verifier
.oneshot(Request::Mempool {
transaction: tx.into(),
height,
})
.await;

let Ok(super::Response::Mempool { transaction }) = verifier_response else {
panic!("expected successful verification, got: {verifier_response:?}");
};

let expected_maturity_height =
coinbase_spend_height + MIN_TRANSPARENT_COINBASE_MATURITY.try_into().unwrap();

assert_eq!(transaction.maturity_height, expected_maturity_height, "maturity_height should be MIN_TRANSPARENT_COINBASE_MATURITY ahead the spent coinbase tx height")
}

#[test]
fn v5_transaction_with_no_outputs_fails_validation() {
let transaction = fake_v5_transactions_for_network(
Expand Down
Loading