diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index b7fd6740ecc..2b0013a5a3d 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -302,10 +302,12 @@ where })?; check::miner_fees_are_valid( - &block, + &coinbase_tx, + height, block_miner_fees, expected_block_subsidy, expected_deferred_amount, + &network, )?; // Finally, submit the block for contextual verification. diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index eca45e8bc0c..189cfdc8493 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -8,7 +8,7 @@ use zebra_chain::{ amount::{Amount, Error as AmountError, NonNegative}, block::{Block, Hash, Header, Height}, parameters::{subsidy::FundingStreamReceiver, Network, NetworkUpgrade}, - transaction, + transaction::{self, Transaction}, work::{ difficulty::{ExpandedDifficulty, ParameterDifficulty as _}, equihash, @@ -234,22 +234,24 @@ pub fn subsidy_is_valid( /// /// [7.1.2]: https://zips.z.cash/protocol/protocol.pdf#txnconsensus pub fn miner_fees_are_valid( - block: &Block, + coinbase_tx: &Transaction, + height: Height, block_miner_fees: Amount, expected_block_subsidy: Amount, expected_deferred_amount: Amount, + network: &Network, ) -> Result<(), BlockError> { - let coinbase = block.transactions.first().ok_or(SubsidyError::NoCoinbase)?; - - let transparent_value_balance: Amount = subsidy::general::output_amounts(coinbase) + let transparent_value_balance = subsidy::general::output_amounts(coinbase_tx) .iter() .sum::, AmountError>>() .map_err(|_| SubsidyError::SumOverflow)? .constrain() .expect("positive value always fit in `NegativeAllowed`"); - let sapling_value_balance = coinbase.sapling_value_balance().sapling_amount(); - let orchard_value_balance = coinbase.orchard_value_balance().orchard_amount(); + let sapling_value_balance = coinbase_tx.sapling_value_balance().sapling_amount(); + let orchard_value_balance = coinbase_tx.orchard_value_balance().orchard_amount(); + // TODO: Update the quote below once its been updated for NU6. + // // # Consensus // // > The total value in zatoshi of transparent outputs from a coinbase transaction, @@ -260,17 +262,26 @@ pub fn miner_fees_are_valid( // // The expected lockbox funding stream output of the coinbase transaction is also subtracted // from the block subsidy value plus the transaction fees paid by transactions in this block. - // - // TODO: Update the quote from the protocol specification once its been updated to reflect the changes in - // https://zips.z.cash/draft-nuttycom-funding-allocation and https://zips.z.cash/draft-hopwood-coinbase-balance. let left = (transparent_value_balance - sapling_value_balance - orchard_value_balance) .map_err(|_| SubsidyError::SumOverflow)?; let right = (expected_block_subsidy + block_miner_fees - expected_deferred_amount) .map_err(|_| SubsidyError::SumOverflow)?; - if left > right { - Err(SubsidyError::InvalidMinerFees)?; - } + // TODO: Updadte the quotes below if the final phrasing changes in the spec for NU6. + // + // # Consensus + // + // > [Pre-NU6] The total output of a coinbase transaction MUST NOT be greater than its total + // input. + // + // > [NU6 onward] The total output of a coinbase transaction MUST be equal to its total input. + if if NetworkUpgrade::current(network, height) < NetworkUpgrade::Nu6 { + left > right + } else { + left != right + } { + Err(SubsidyError::InvalidMinerFees)? + }; Ok(()) } diff --git a/zebra-consensus/src/block/subsidy/general.rs b/zebra-consensus/src/block/subsidy/general.rs index 233f7935c1c..03ebac36d21 100644 --- a/zebra-consensus/src/block/subsidy/general.rs +++ b/zebra-consensus/src/block/subsidy/general.rs @@ -2,6 +2,8 @@ //! //! [7.8]: https://zips.z.cash/protocol/protocol.pdf#subsidies +// TODO: Move the contents of this mod to the parent mod and remove this mod. + use std::collections::HashSet; use zebra_chain::{ diff --git a/zebra-consensus/src/block/tests.rs b/zebra-consensus/src/block/tests.rs index 8f202a716e2..d8f74bf7b2f 100644 --- a/zebra-consensus/src/block/tests.rs +++ b/zebra-consensus/src/block/tests.rs @@ -509,8 +509,12 @@ fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> { for (&height, block) in block_iter { let height = Height(height); if height > network.slow_start_shift() { - let block = Block::zcash_deserialize(&block[..]).expect("block should deserialize"); + let coinbase_tx = check::coinbase_is_first( + &Block::zcash_deserialize(&block[..]).expect("block should deserialize"), + )?; + let expected_block_subsidy = block_subsidy(height, &network)?; + // TODO: Add link to lockbox stream ZIP let expected_deferred_amount = subsidy::funding_streams::funding_stream_values( height, @@ -521,17 +525,16 @@ fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> { .remove(&FundingStreamReceiver::Deferred) .unwrap_or_default(); - // fake the miner fee to a big amount - let miner_fees = Amount::try_from(MAX_MONEY / 2).unwrap(); - - // Validate - let result = check::miner_fees_are_valid( - &block, - miner_fees, + assert!(check::miner_fees_are_valid( + &coinbase_tx, + height, + // Set the miner fees to a high-enough amount. + Amount::try_from(MAX_MONEY / 2).unwrap(), expected_block_subsidy, expected_deferred_amount, - ); - assert!(result.is_ok()); + &network, + ) + .is_ok(),); } } @@ -542,9 +545,8 @@ fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> { fn miner_fees_validation_failure() -> Result<(), Report> { let _init_guard = zebra_test::init(); let network = Network::Mainnet; - let block = - Arc::::zcash_deserialize(&zebra_test::vectors::BLOCK_MAINNET_347499_BYTES[..]) - .expect("block should deserialize"); + let block = Block::zcash_deserialize(&zebra_test::vectors::BLOCK_MAINNET_347499_BYTES[..]) + .expect("block should deserialize"); let height = block.coinbase_height().expect("valid coinbase height"); let expected_block_subsidy = block_subsidy(height, &network)?; // TODO: Add link to lockbox stream ZIP @@ -554,22 +556,21 @@ fn miner_fees_validation_failure() -> Result<(), Report> { .remove(&FundingStreamReceiver::Deferred) .unwrap_or_default(); - // fake the miner fee to a small amount - let miner_fees = Amount::zero(); - - // Validate - let result = check::miner_fees_are_valid( - &block, - miner_fees, - expected_block_subsidy, - expected_deferred_amount, + assert_eq!( + check::miner_fees_are_valid( + check::coinbase_is_first(&block)?.as_ref(), + height, + // Set the miner fee to an invalid amount. + Amount::zero(), + expected_block_subsidy, + expected_deferred_amount, + &network + ), + Err(BlockError::Transaction(TransactionError::Subsidy( + SubsidyError::InvalidMinerFees, + ))) ); - let expected = Err(BlockError::Transaction(TransactionError::Subsidy( - SubsidyError::InvalidMinerFees, - ))); - assert_eq!(expected, result); - Ok(()) } diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 33974746fd9..997c7b33e62 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -3236,10 +3236,10 @@ async fn trusted_chain_sync_handles_forks_correctly() -> Result<()> { /// Test successful block template submission as a block proposal or submission on a custom Testnet. /// /// This test can be run locally with: -/// `RUSTFLAGS='--cfg zcash_unstable="nu6"' cargo test --package zebrad --test acceptance --features getblocktemplate-rpcs -- nu6_funding_streams --exact --show-output` +/// `RUSTFLAGS='--cfg zcash_unstable="nu6"' cargo test --package zebrad --test acceptance --features getblocktemplate-rpcs -- nu6_funding_streams_and_coinbase_balance --exact --show-output` #[tokio::test(flavor = "multi_thread")] #[cfg(all(feature = "getblocktemplate-rpcs", zcash_unstable = "nu6"))] -async fn nu6_funding_streams() -> Result<()> { +async fn nu6_funding_streams_and_coinbase_balance() -> Result<()> { use zebra_chain::{ chain_sync_status::MockSyncStatus, parameters::{ @@ -3470,6 +3470,49 @@ async fn nu6_funding_streams() -> Result<()> { "invalid block with excessive coinbase output value should be rejected" ); + // Use an invalid coinbase transaction (with an output value less than than the `block_subsidy + miner_fees - expected_lockbox_funding_stream`) + let network = base_network_params + .clone() + .with_post_nu6_funding_streams(ConfiguredFundingStreams { + height_range: Some(Height(1)..Height(100)), + recipients: make_configured_recipients_with_lockbox_numerator(20), + }) + .to_network(); + + let (coinbase_txn, default_roots) = generate_coinbase_and_roots( + &network, + Height(block_template.height), + &miner_address, + &[], + history_tree.clone(), + true, + vec![], + ); + + let block_template = GetBlockTemplate { + coinbase_txn, + block_commitments_hash: default_roots.block_commitments_hash, + light_client_root_hash: default_roots.block_commitments_hash, + final_sapling_root_hash: default_roots.block_commitments_hash, + default_roots, + ..block_template + }; + + let proposal_block = proposal_block_from_template(&block_template, None, NetworkUpgrade::Nu6)?; + + // Submit the invalid block with an excessive coinbase input value + let submit_block_response = get_block_template_rpc_impl + .submit_block(HexData(proposal_block.zcash_serialize_to_vec()?), None) + .await?; + + tracing::info!(?submit_block_response, "submitted invalid block"); + + assert_eq!( + submit_block_response, + submit_block::Response::ErrorResponse(submit_block::ErrorResponse::Rejected), + "invalid block with insufficient coinbase output value should be rejected" + ); + // Check that the original block template can be submitted successfully let proposal_block = proposal_block_from_template(&valid_original_block_template, None, NetworkUpgrade::Nu6)?;