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(consensus): Allow transactions spending coinbase outputs to have transparent outputs on Regtest #9085

Merged
merged 6 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions zebra-chain/src/block/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ where
+ Copy
+ 'static,
{
let mut spend_restriction = transaction.coinbase_spend_restriction(height);
let mut spend_restriction = transaction.coinbase_spend_restriction(&Network::Mainnet, height);
let mut new_inputs = Vec::new();
let mut spent_outputs = HashMap::new();

Expand Down Expand Up @@ -650,7 +650,8 @@ where
+ 'static,
{
let has_shielded_outputs = transaction.has_shielded_outputs();
let delete_transparent_outputs = CoinbaseSpendRestriction::OnlyShieldedOutputs { spend_height };
let delete_transparent_outputs =
CoinbaseSpendRestriction::CheckCoinbaseMaturity { spend_height };
let mut attempts: usize = 0;

// choose an arbitrary spendable UTXO, in hash set order
Expand Down
41 changes: 41 additions & 0 deletions zebra-chain/src/parameters/network/testnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ pub struct ParametersBuilder {
target_difficulty_limit: ExpandedDifficulty,
/// A flag for disabling proof-of-work checks when Zebra is validating blocks
disable_pow: bool,
/// Whether to allow transactions with transparent outputs to spend coinbase outputs,
/// similar to `fCoinbaseMustBeShielded` in zcashd.
should_allow_unshielded_coinbase_spends: bool,
/// The pre-Blossom halving interval for this network
pre_blossom_halving_interval: HeightDiff,
/// The post-Blossom halving interval for this network
Expand Down Expand Up @@ -271,6 +274,7 @@ impl Default for ParametersBuilder {
should_lock_funding_stream_address_period: false,
pre_blossom_halving_interval: PRE_BLOSSOM_HALVING_INTERVAL,
post_blossom_halving_interval: POST_BLOSSOM_HALVING_INTERVAL,
should_allow_unshielded_coinbase_spends: false,
}
}
}
Expand Down Expand Up @@ -439,6 +443,15 @@ impl ParametersBuilder {
self
}

/// Sets the `disable_pow` flag to be used in the [`Parameters`] being built.
pub fn with_unshielded_coinbase_spends(
mut self,
should_allow_unshielded_coinbase_spends: bool,
) -> Self {
self.should_allow_unshielded_coinbase_spends = should_allow_unshielded_coinbase_spends;
self
}

/// Sets the pre and post Blosssom halving intervals to be used in the [`Parameters`] being built.
pub fn with_halving_interval(mut self, pre_blossom_halving_interval: HeightDiff) -> Self {
if self.should_lock_funding_stream_address_period {
Expand All @@ -464,6 +477,7 @@ impl ParametersBuilder {
should_lock_funding_stream_address_period: _,
target_difficulty_limit,
disable_pow,
should_allow_unshielded_coinbase_spends,
pre_blossom_halving_interval,
post_blossom_halving_interval,
} = self;
Expand All @@ -478,6 +492,7 @@ impl ParametersBuilder {
post_nu6_funding_streams,
target_difficulty_limit,
disable_pow,
should_allow_unshielded_coinbase_spends,
pre_blossom_halving_interval,
post_blossom_halving_interval,
}
Expand Down Expand Up @@ -516,6 +531,7 @@ impl ParametersBuilder {
should_lock_funding_stream_address_period: _,
target_difficulty_limit,
disable_pow,
should_allow_unshielded_coinbase_spends,
pre_blossom_halving_interval,
post_blossom_halving_interval,
} = Self::default();
Expand All @@ -528,6 +544,8 @@ impl ParametersBuilder {
&& self.post_nu6_funding_streams == post_nu6_funding_streams
&& self.target_difficulty_limit == target_difficulty_limit
&& self.disable_pow == disable_pow
&& self.should_allow_unshielded_coinbase_spends
== should_allow_unshielded_coinbase_spends
&& self.pre_blossom_halving_interval == pre_blossom_halving_interval
&& self.post_blossom_halving_interval == post_blossom_halving_interval
}
Expand Down Expand Up @@ -560,6 +578,9 @@ pub struct Parameters {
target_difficulty_limit: ExpandedDifficulty,
/// A flag for disabling proof-of-work checks when Zebra is validating blocks
disable_pow: bool,
/// Whether to allow transactions with transparent outputs to spend coinbase outputs,
/// similar to `fCoinbaseMustBeShielded` in zcashd.
should_allow_unshielded_coinbase_spends: bool,
/// Pre-Blossom halving interval for this network
pre_blossom_halving_interval: HeightDiff,
/// Post-Blossom halving interval for this network
Expand Down Expand Up @@ -597,6 +618,7 @@ impl Parameters {
// This value is chosen to match zcashd, see: <https://github.com/zcash/zcash/blob/master/src/chainparams.cpp#L654>
.with_target_difficulty_limit(U256::from_big_endian(&[0x0f; 32]))
.with_disable_pow(true)
.with_unshielded_coinbase_spends(true)
.with_slow_start_interval(Height::MIN)
// Removes default Testnet activation heights if not configured,
// most network upgrades are disabled by default for Regtest in zcashd
Expand Down Expand Up @@ -645,6 +667,7 @@ impl Parameters {
post_nu6_funding_streams,
target_difficulty_limit,
disable_pow,
should_allow_unshielded_coinbase_spends,
pre_blossom_halving_interval,
post_blossom_halving_interval,
} = Self::new_regtest(None, None);
Expand All @@ -657,6 +680,8 @@ impl Parameters {
&& self.post_nu6_funding_streams == post_nu6_funding_streams
&& self.target_difficulty_limit == target_difficulty_limit
&& self.disable_pow == disable_pow
&& self.should_allow_unshielded_coinbase_spends
== should_allow_unshielded_coinbase_spends
&& self.pre_blossom_halving_interval == pre_blossom_halving_interval
&& self.post_blossom_halving_interval == post_blossom_halving_interval
}
Expand Down Expand Up @@ -711,6 +736,12 @@ impl Parameters {
self.disable_pow
}

/// Returns true if this network should allow transactions with transparent outputs
/// that spend coinbase outputs.
pub fn should_allow_unshielded_coinbase_spends(&self) -> bool {
self.should_allow_unshielded_coinbase_spends
}

/// Returns the pre-Blossom halving interval for this network
pub fn pre_blossom_halving_interval(&self) -> HeightDiff {
self.pre_blossom_halving_interval
Expand Down Expand Up @@ -786,4 +817,14 @@ impl Network {
self.post_nu6_funding_streams()
}
}

/// Returns true if this network should allow transactions with transparent outputs
/// that spend coinbase outputs.
pub fn should_allow_unshielded_coinbase_spends(&self) -> bool {
if let Self::Testnet(params) = self {
params.should_allow_unshielded_coinbase_spends()
upbqdn marked this conversation as resolved.
Show resolved Hide resolved
} else {
false
}
}
}
13 changes: 7 additions & 6 deletions zebra-chain/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub use unmined::{
use crate::{
amount::{Amount, Error as AmountError, NegativeAllowed, NonNegative},
block, orchard,
parameters::{ConsensusBranchId, NetworkUpgrade},
parameters::{ConsensusBranchId, Network, NetworkUpgrade},
primitives::{ed25519, Bctv14Proof, Groth16Proof},
sapling,
serialization::ZcashSerialize,
Expand Down Expand Up @@ -303,14 +303,15 @@ impl Transaction {
/// assuming it is mined at `spend_height`.
pub fn coinbase_spend_restriction(
&self,
network: &Network,
spend_height: block::Height,
) -> CoinbaseSpendRestriction {
if self.outputs().is_empty() {
// we know this transaction must have shielded outputs,
// because of other consensus rules
OnlyShieldedOutputs { spend_height }
if self.outputs().is_empty() || network.should_allow_unshielded_coinbase_spends() {
// we know this transaction must have shielded outputs if it has no
// transparent outputs, because of other consensus rules.
CheckCoinbaseMaturity { spend_height }
} else {
SomeTransparentOutputs
DisallowCoinbaseSpend
}
}

Expand Down
10 changes: 7 additions & 3 deletions zebra-chain/src/transparent/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,14 @@ impl OrderedUtxo {
)]
pub enum CoinbaseSpendRestriction {
/// The UTXO is spent in a transaction with one or more transparent outputs
SomeTransparentOutputs,
/// on a network where coinbase outputs must not be spent by transactions
/// with transparent outputs.
DisallowCoinbaseSpend,

/// The UTXO is spent in a transaction which only has shielded outputs
OnlyShieldedOutputs {
/// The UTXO is spent in a transaction which only has shielded outputs, or
/// transactions spending coinbase outputs may have transparent outputs on
/// this network.
CheckCoinbaseMaturity {
/// The height at which the UTXO is spent
spend_height: block::Height,
},
Expand Down
4 changes: 3 additions & 1 deletion zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ where
// WONTFIX: Return an error for Request::Block as well to replace this check in
// the state once #2336 has been implemented?
if req.is_mempool() {
Self::check_maturity_height(&req, &spent_utxos)?;
Self::check_maturity_height(&network, &req, &spent_utxos)?;
}

let cached_ffi_transaction =
Expand Down Expand Up @@ -728,10 +728,12 @@ where
/// mature and valid for the request height, or a [`TransactionError`] if the transaction
/// spends transparent coinbase outputs that are immature and invalid for the request height.
pub fn check_maturity_height(
network: &Network,
request: &Request,
spent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
) -> Result<(), TransactionError> {
check::tx_transparent_coinbase_spends_maturity(
network,
request.transaction(),
request.height(),
request.known_utxos(),
Expand Down
3 changes: 2 additions & 1 deletion zebra-consensus/src/transaction/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ fn validate_expiry_height_mined(
/// Returns `Ok(())` if spent transparent coinbase outputs are
/// valid for the block height, or a [`Err(TransactionError)`](TransactionError)
pub fn tx_transparent_coinbase_spends_maturity(
network: &Network,
tx: Arc<Transaction>,
height: Height,
block_new_outputs: Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>>,
Expand All @@ -488,7 +489,7 @@ pub fn tx_transparent_coinbase_spends_maturity(
.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 spend_restriction = tx.coinbase_spend_restriction(network, height);

zebra_state::check::transparent_coinbase_spend(spend, spend_restriction, &utxo)?;
}
Expand Down
98 changes: 96 additions & 2 deletions zebra-consensus/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use zebra_chain::{
},
zip317, Hash, HashType, JoinSplitData, LockTime, Transaction,
},
transparent::{self, CoinbaseData},
transparent::{self, CoinbaseData, CoinbaseSpendRestriction},
};

use zebra_node_services::mempool;
Expand Down Expand Up @@ -739,7 +739,7 @@ async fn mempool_request_with_immature_spend_is_rejected() {
transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"),
};

let spend_restriction = tx.coinbase_spend_restriction(height);
let spend_restriction = tx.coinbase_spend_restriction(&Network::Mainnet, height);

let coinbase_spend_height = Height(5);

Expand Down Expand Up @@ -807,6 +807,100 @@ async fn mempool_request_with_immature_spend_is_rejected() {
);
}

/// Tests that calls to the transaction verifier with a mempool request that spends
/// mature coinbase outputs to transparent outputs will return Ok() on Regtest.
#[tokio::test]
async fn mempool_request_with_transparent_coinbase_spend_is_accepted_on_regtest() {
let _init_guard = zebra_test::init();
upbqdn marked this conversation as resolved.
Show resolved Hide resolved

let network = Network::new_regtest(None, Some(1_000));
let mut state: MockService<_, _, _, _> = MockService::build().for_unit_tests();
let verifier = Verifier::new_for_tests(&network, state.clone());

let height = NetworkUpgrade::Nu6
.activation_height(&network)
.expect("NU6 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,
Amount::try_from(10001).expect("invalid value"),
);

// Create a non-coinbase V5 tx with the last valid expiry height.
let tx = Transaction::V5 {
network_upgrade: NetworkUpgrade::Nu6,
inputs: vec![input],
outputs: vec![output],
lock_time: LockTime::min_lock_time_timestamp(),
expiry_height: height,
sapling_shielded_data: None,
orchard_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 spend_restriction = tx.coinbase_spend_restriction(&network, height);

assert_eq!(
spend_restriction,
CoinbaseSpendRestriction::CheckCoinbaseMaturity {
spend_height: height
}
);

let coinbase_spend_height = Height(5);

let utxo = known_utxos
.get(&input_outpoint)
.map(|utxo| {
let mut utxo = utxo.utxo.clone();
utxo.height = coinbase_spend_height;
utxo.from_coinbase = true;
utxo
})
.expect("known_utxos should contain the outpoint");

zebra_state::check::transparent_coinbase_spend(input_outpoint, spend_restriction, &utxo)
.expect("check should pass");

tokio::spawn(async move {
state
.expect_request(zebra_state::Request::BestChainNextMedianTimePast)
.await
.respond(zebra_state::Response::BestChainNextMedianTimePast(
DateTime32::MAX,
));

state
.expect_request(zebra_state::Request::UnspentBestChainUtxo(input_outpoint))
.await
.respond(zebra_state::Response::UnspentBestChainUtxo(Some(utxo)));

state
.expect_request_that(|req| {
matches!(
req,
zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_)
)
})
.await
.respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors);
});

verifier
.oneshot(Request::Mempool {
transaction: tx.into(),
height,
})
.await
.expect("verification of transaction with mature spend to transparent outputs should pass");
upbqdn marked this conversation as resolved.
Show resolved Hide resolved
}

/// Tests that errors from the read state service are correctly converted into
/// transaction verifier errors.
#[tokio::test]
Expand Down
6 changes: 3 additions & 3 deletions zebra-state/src/service/check/tests/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn accept_shielded_mature_coinbase_utxo_spend() {
let ordered_utxo = transparent::OrderedUtxo::new(output, created_height, 0);

let min_spend_height = Height(created_height.0 + MIN_TRANSPARENT_COINBASE_MATURITY);
let spend_restriction = transparent::CoinbaseSpendRestriction::OnlyShieldedOutputs {
let spend_restriction = transparent::CoinbaseSpendRestriction::CheckCoinbaseMaturity {
spend_height: min_spend_height,
};

Expand Down Expand Up @@ -78,7 +78,7 @@ fn reject_unshielded_coinbase_utxo_spend() {
};
let ordered_utxo = transparent::OrderedUtxo::new(output, created_height, 0);

let spend_restriction = transparent::CoinbaseSpendRestriction::SomeTransparentOutputs;
let spend_restriction = transparent::CoinbaseSpendRestriction::DisallowCoinbaseSpend;

let result =
check::utxo::transparent_coinbase_spend(outpoint, spend_restriction, ordered_utxo.as_ref());
Expand All @@ -104,7 +104,7 @@ fn reject_immature_coinbase_utxo_spend() {
let min_spend_height = Height(created_height.0 + MIN_TRANSPARENT_COINBASE_MATURITY);
let spend_height = Height(min_spend_height.0 - 1);
let spend_restriction =
transparent::CoinbaseSpendRestriction::OnlyShieldedOutputs { spend_height };
transparent::CoinbaseSpendRestriction::CheckCoinbaseMaturity { spend_height };

let result =
check::utxo::transparent_coinbase_spend(outpoint, spend_restriction, ordered_utxo.as_ref());
Expand Down
Loading
Loading