change(consensus): Require that coinbase transactions balance exactly…

… after NU6 activation (#8727)

* Addresses clippy lints

* checks network magic and returns early from `is_regtest()`

* Moves  `subsidy.rs` to `zebra-chain`, refactors funding streams into structs, splits them into pre/post NU6 funding streams, and adds them as a field on `testnet::Parameters`

* Replaces Vec with HashMap, adds `ConfiguredFundingStreams` type and conversion logic with constraints.

Minor refactors

* Empties recipients list

* Adds a comment on num_addresses calculation being invalid for configured Testnets, but that being okay since configured testnet parameters are checked when they're being built

* Documentation fixes, minor cleanup, renames a test, adds TODOs, and fixes test logic

* Removes unnecessary `ParameterSubsidy` impl for &Network, adds docs and TODOs

* Adds a "deferred" FundingStreamReceiver, adds a post-NU6 funding streams, updates the `miner_fees_are_valid()` and `subsidy_is_valid()` functions to check that the deferred pool contribution is valid and that there are no unclaimed block subsidies after NU6 activation, and adds some TODOs

* adds `lockbox_input_value()` fn

* Adds TODOs for linking to relevant ZIPs and updating height ranges

* Adds `nu6_lockbox_funding_stream` acceptance test

* updates funding stream values test to check post-NU6 funding streams too, adds Mainnet/Testnet NU6 activation heights, fixes lints/compilation issue

* Reverts Mainnet/Testnet NU6 activation height definitions, updates `test_funding_stream_values()` to use a configured testnet with the post-NU6 Mainnet funding streams height range

* reverts unnecessary refactor

* appease clippy

* Adds a test for `lockbox_input_value()`

* Applies suggestions from code review

* Fixes potential panic

* Fixes bad merge

* Update zebra-chain/src/parameters/network_upgrade.rs

* Updates acceptance test to check that invalid blocks are rejected

* Checks that the original valid block template at height 2 is accepted as a block submission

* Reverts changes for coinbase should balance exactly ZIP

* updates test name

* Updates deferred pool funding stream name to "Lockbox", moves post-NU6 height ranges to constants, updates TODO

* Updates `get_block_subsidy()` RPC method to exclude lockbox funding stream from `fundingstreams` field

* Adds a TODO for updating `FundingStreamReceiver::name()` method docs

* Updates `FundingStreamRecipient::new()` to accept an iterator of items instead of an option of an iterator, updates a comment quoting the coinbase transaction balance consensus rule to note that the current code is inconsistent with the protocol spec, adds a TODO for updating the quote there once the protocol spec has been updated.

* Uses FPF Testnet address for post-NU6 testnet funding streams

* Updates the NU6 consensus branch id

* checks that coinbase transactions balance exactly

* updates test name

* Add a TODO

* Refactor `miner_fees_are_valid`

---------

Co-authored-by: Pili Guerra <[email protected]>
Co-authored-by: Marek <[email protected]>

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.

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<NonNegative>,
     expected_block_subsidy: Amount<NonNegative>,
     expected_deferred_amount: Amount<NonNegative>,
+    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::<Result<Amount<NonNegative>, 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(())
 }

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::{

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::<Block>::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(())
 }
 

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)?;