change(rpc): Match zcashd's block template exactly (#5867)

* Make Zebra's getblocktemplate like zcashd's

* Update snapshots

* Add missing docs

* Fix typo

* Sort coinbase outputs by serialized script for zcashd

* Sort excluding the length byte, make transaction order always stable

* Update snapshots

* Explain that `zcashd` doesn't seem to have a fixed transaction order

zebra-chain/src/transaction/builder.rs

@@ -20,6 +20,10 @@ impl Transaction {
         //
         // These consensus rules apply to v5 coinbase transactions after NU5 activation:
         //
+        // > If effectiveVersion ≥ 5 then this condition MUST hold:
+        // > tx_in_count > 0 or nSpendsSapling > 0 or
+        // > (nActionsOrchard > 0 and enableSpendsOrchard = 1).
+        //
         // > A coinbase transaction for a block at block height greater than 0 MUST have
         // > a script that, as its first item, encodes the block height height as follows. ...
         // > let heightBytes be the signed little-endian representation of height,
@@ -49,24 +53,34 @@ impl Transaction {
         // > the value in zatoshi of block subsidy plus the transaction fees
         // > paid by transactions in this block.
         //
+        // > If effectiveVersion ≥ 5 then this condition MUST hold:
+        // > tx_out_count > 0 or nOutputsSapling > 0 or
+        // > (nActionsOrchard > 0 and enableOutputsOrchard = 1).
+        //
         // <https://zips.z.cash/protocol/protocol.pdf#txnconsensus>
-        let outputs = outputs
+        let outputs: Vec<_> = outputs
             .into_iter()
             .map(|(amount, lock_script)| transparent::Output::new_coinbase(amount, lock_script))
             .collect();
+        assert!(
+            !outputs.is_empty(),
+            "invalid coinbase transaction: must have at least one output"
+        );
 
         Transaction::V5 {
             // > The transaction version number MUST be 4 or 5. ...
-            // > If the transaction version number is 5 then the version group ID MUST be 0x26A7270A.
+            // > If the transaction version number is 5 then the version group ID
+            // > MUST be 0x26A7270A.
             // > If effectiveVersion ≥ 5, the nConsensusBranchId field MUST match the consensus
             // > branch ID used for SIGHASH transaction hashes, as specified in [ZIP-244].
             network_upgrade: NetworkUpgrade::current(network, height),
 
-            // There is no documented consensus rule for the lock time field in coinbase transactions,
-            // so we just leave it unlocked. (We could also set it to `height`.)
+            // There is no documented consensus rule for the lock time field in coinbase
+            // transactions, so we just leave it unlocked. (We could also set it to `height`.)
             lock_time: LockTime::unlocked(),
 
-            // > The nExpiryHeight field of a coinbase transaction MUST be equal to its block height.
+            // > The nExpiryHeight field of a coinbase transaction MUST be equal to its
+            // > block height.
             expiry_height: height,
 
             inputs,
@@ -83,4 +97,62 @@ impl Transaction {
             orchard_shielded_data: None,
         }
     }
+
+    /// Returns a new version 4 coinbase transaction for `network` and `height`,
+    /// which contains the specified `outputs`.
+    ///
+    /// If `like_zcashd` is true, try to match the coinbase transactions generated by `zcashd`
+    /// in the `getblocktemplate` RPC.
+    pub fn new_v4_coinbase(
+        _network: Network,
+        height: Height,
+        outputs: impl IntoIterator<Item = (Amount<NonNegative>, transparent::Script)>,
+        like_zcashd: bool,
+    ) -> Transaction {
+        // `zcashd` includes an extra byte after the coinbase height in the coinbase data,
+        // and a sequence number of u32::MAX.
+        let mut extra_data = None;
+        let mut sequence = None;
+
+        if like_zcashd {
+            extra_data = Some(vec![0x00]);
+            sequence = Some(u32::MAX);
+        }
+
+        // # Consensus
+        //
+        // See the other consensus rules above in new_v5_coinbase().
+        //
+        // > If effectiveVersion < 5, then at least one of tx_in_count, nSpendsSapling,
+        // > and nJoinSplit MUST be nonzero.
+        let inputs = vec![transparent::Input::new_coinbase(
+            height, extra_data, sequence,
+        )];
+
+        // > If effectiveVersion < 5, then at least one of tx_out_count, nOutputsSapling,
+        // > and nJoinSplit MUST be nonzero.
+        let outputs: Vec<_> = outputs
+            .into_iter()
+            .map(|(amount, lock_script)| transparent::Output::new_coinbase(amount, lock_script))
+            .collect();
+        assert!(
+            !outputs.is_empty(),
+            "invalid coinbase transaction: must have at least one output"
+        );
+
+        // > The transaction version number MUST be 4 or 5. ...
+        // > If the transaction version number is 4 then the version group ID MUST be 0x892F2085.
+        Transaction::V4 {
+            lock_time: LockTime::unlocked(),
+
+            expiry_height: height,
+
+            inputs,
+            outputs,
+
+            // Zebra does not support shielded coinbase yet.
+            joinsplit_data: None,
+            sapling_shielded_data: None,
+        }
+    }
 }

zebra-chain/src/transaction/serialize.rs

@@ -990,7 +990,9 @@ impl TrustedPreallocate for transparent::Output {
 /// A serialized transaction.
 ///
 /// Stores bytes that are guaranteed to be deserializable into a [`Transaction`].
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+///
+/// Sorts in lexicographic order of the transaction's serialized data.
+#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct SerializedTransaction {
     bytes: Vec<u8>,
 }

zebra-chain/src/transparent/script.rs

@@ -9,7 +9,7 @@ use crate::serialization::{
 };
 
 /// An encoding of a Bitcoin script.
-#[derive(Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 #[cfg_attr(
     any(test, feature = "proptest-impl"),
     derive(proptest_derive::Arbitrary)

zebra-consensus/src/block/subsidy/funding_streams.rs

@@ -2,6 +2,8 @@
 //!
 //! [7.8]: https://zips.z.cash/protocol/protocol.pdf#subsidies
 
+use std::{collections::HashMap, str::FromStr};
+
 use zebra_chain::{
     amount::{Amount, Error, NonNegative},
     block::Height,
@@ -12,8 +14,6 @@ use zebra_chain::{
 
 use crate::{block::subsidy::general::block_subsidy, parameters::subsidy::*};
 
-use std::{collections::HashMap, str::FromStr};
-
 #[cfg(test)]
 mod tests;
 

zebra-consensus/src/lib.rs

@@ -47,16 +47,19 @@ pub mod chain;
 pub mod error;
 
 pub use block::{
-    subsidy::funding_streams::funding_stream_address,
-    subsidy::funding_streams::funding_stream_values, subsidy::funding_streams::new_coinbase_script,
-    subsidy::general::miner_subsidy, VerifyBlockError, MAX_BLOCK_SIGOPS,
+    subsidy::{
+        funding_streams::{funding_stream_address, funding_stream_values, new_coinbase_script},
+        general::miner_subsidy,
+    },
+    VerifyBlockError, MAX_BLOCK_SIGOPS,
 };
 pub use chain::VerifyChainError;
 pub use checkpoint::{
     CheckpointList, VerifyCheckpointError, MAX_CHECKPOINT_BYTE_COUNT, MAX_CHECKPOINT_HEIGHT_GAP,
 };
 pub use config::Config;
 pub use error::BlockError;
+pub use parameters::FundingStreamReceiver;
 pub use primitives::{ed25519, groth16, halo2, redjubjub, redpallas};
 
 /// A boxed [`std::error::Error`].

zebra-consensus/src/parameters/subsidy.rs

@@ -48,8 +48,13 @@ pub const FIRST_HALVING_TESTNET: Height = Height(1_116_000);
 /// The funding stream receiver categories.
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 pub enum FundingStreamReceiver {
+    /// The Electric Coin Company (Bootstrap Foundation) funding stream.
     Ecc,
+
+    /// The Zcash Foundation funding stream.
     ZcashFoundation,
+
+    /// The Major Grants (Zcash Community Grants) funding stream.
     MajorGrants,
 }
 

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -317,6 +317,11 @@ where
         &self,
         parameters: Option<get_block_template::JsonParameters>,
     ) -> BoxFuture<Result<GetBlockTemplate>> {
+        // Should we generate coinbase transactions that are exactly like zcashd's?
+        //
+        // This is useful for testing, but either way Zebra should obey the consensus rules.
+        const COINBASE_LIKE_ZCASHD: bool = true;
+
         // Clone Config
         let network = self.network;
         let miner_address = self.miner_address;
@@ -549,6 +554,7 @@ where
                 next_block_height,
                 miner_address,
                 mempool_txs,
+                COINBASE_LIKE_ZCASHD,
             )
             .await;
 
@@ -563,16 +569,17 @@ where
                 miner_address,
                 &mempool_txs,
                 chain_tip_and_local_time.history_tree.clone(),
+                COINBASE_LIKE_ZCASHD,
             );
 
             let response = GetBlockTemplate::new(
-                next_block_height,
                 &chain_tip_and_local_time,
                 server_long_poll_id,
                 coinbase_txn,
                 &mempool_txs,
                 default_roots,
                 submit_old,
+                COINBASE_LIKE_ZCASHD,
             );
 
             Ok(response)

zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs

@@ -1,6 +1,6 @@
 //! Support functions for the `get_block_template()` RPC.
 
-use std::{iter, sync::Arc};
+use std::{collections::HashMap, iter, sync::Arc};
 
 use jsonrpc_core::{Error, ErrorCode, Result};
 use tower::{Service, ServiceExt};
@@ -17,7 +17,9 @@ use zebra_chain::{
     transaction::{Transaction, UnminedTx, VerifiedUnminedTx},
     transparent,
 };
-use zebra_consensus::{funding_stream_address, funding_stream_values, miner_subsidy};
+use zebra_consensus::{
+    funding_stream_address, funding_stream_values, miner_subsidy, FundingStreamReceiver,
+};
 use zebra_node_services::mempool;
 use zebra_state::GetBlockTemplateChainInfo;
 
@@ -175,16 +177,21 @@ where
 // - Response processing
 
 /// Generates and returns the coinbase transaction and default roots.
+///
+/// If `like_zcashd` is true, try to match the coinbase transactions generated by `zcashd`
+/// in the `getblocktemplate` RPC.
 pub fn generate_coinbase_and_roots(
     network: Network,
     height: Height,
     miner_address: transparent::Address,
     mempool_txs: &[VerifiedUnminedTx],
     history_tree: Arc<zebra_chain::history_tree::HistoryTree>,
+    like_zcashd: bool,
 ) -> (TransactionTemplate<NegativeOrZero>, DefaultRoots) {
     // Generate the coinbase transaction
     let miner_fee = calculate_miner_fee(mempool_txs);
-    let coinbase_txn = generate_coinbase_transaction(network, height, miner_address, miner_fee);
+    let coinbase_txn =
+        generate_coinbase_transaction(network, height, miner_address, miner_fee, like_zcashd);
 
     // Calculate block default roots
     //
@@ -199,15 +206,23 @@ pub fn generate_coinbase_and_roots(
 // - Coinbase transaction processing
 
 /// Returns a coinbase transaction for the supplied parameters.
+///
+/// If `like_zcashd` is true, try to match the coinbase transactions generated by `zcashd`
+/// in the `getblocktemplate` RPC.
 pub fn generate_coinbase_transaction(
     network: Network,
     height: Height,
     miner_address: transparent::Address,
     miner_fee: Amount<NonNegative>,
+    like_zcashd: bool,
 ) -> UnminedTx {
-    let outputs = standard_coinbase_outputs(network, height, miner_address, miner_fee);
+    let outputs = standard_coinbase_outputs(network, height, miner_address, miner_fee, like_zcashd);
 
-    Transaction::new_v5_coinbase(network, height, outputs).into()
+    if like_zcashd {
+        Transaction::new_v4_coinbase(network, height, outputs, like_zcashd).into()
+    } else {
+        Transaction::new_v5_coinbase(network, height, outputs).into()
+    }
 }
 
 /// Returns the total miner fee for `mempool_txs`.
@@ -225,36 +240,78 @@ pub fn calculate_miner_fee(mempool_txs: &[VerifiedUnminedTx]) -> Amount<NonNegat
 /// for `network`, `height` and `miner_fee`.
 ///
 /// Only works for post-Canopy heights.
+///
+/// If `like_zcashd` is true, try to match the coinbase transactions generated by `zcashd`
+/// in the `getblocktemplate` RPC.
 pub fn standard_coinbase_outputs(
     network: Network,
     height: Height,
     miner_address: transparent::Address,
     miner_fee: Amount<NonNegative>,
+    like_zcashd: bool,
 ) -> Vec<(Amount<NonNegative>, transparent::Script)> {
     let funding_streams = funding_stream_values(height, network)
         .expect("funding stream value calculations are valid for reasonable chain heights");
 
-    let mut funding_streams: Vec<(Amount<NonNegative>, transparent::Address)> = funding_streams
-        .iter()
-        .map(|(receiver, amount)| (*amount, funding_stream_address(height, network, *receiver)))
+    // Optional TODO: move this into a zebra_consensus function?
+    let funding_streams: HashMap<
+        FundingStreamReceiver,
+        (Amount<NonNegative>, transparent::Address),
+    > = funding_streams
+        .into_iter()
+        .map(|(receiver, amount)| {
+            (
+                receiver,
+                (amount, funding_stream_address(height, network, receiver)),
+            )
+        })
         .collect();
-    // The HashMap returns funding streams in an arbitrary order,
-    // but Zebra's snapshot tests expect the same order every time.
-    funding_streams.sort_by_key(|(amount, _address)| *amount);
 
     let miner_reward = miner_subsidy(height, network)
         .expect("reward calculations are valid for reasonable chain heights")
         + miner_fee;
     let miner_reward =
         miner_reward.expect("reward calculations are valid for reasonable chain heights");
 
-    let mut coinbase_outputs = funding_streams;
+    combine_coinbase_outputs(funding_streams, miner_address, miner_reward, like_zcashd)
+}
+
+/// Combine the miner reward and funding streams into a list of coinbase amounts and addresses.
+///
+/// If `like_zcashd` is true, try to match the coinbase transactions generated by `zcashd`
+/// in the `getblocktemplate` RPC.
+fn combine_coinbase_outputs(
+    funding_streams: HashMap<FundingStreamReceiver, (Amount<NonNegative>, transparent::Address)>,
+    miner_address: transparent::Address,
+    miner_reward: Amount<NonNegative>,
+    like_zcashd: bool,
+) -> Vec<(Amount<NonNegative>, transparent::Script)> {
+    // Combine all the funding streams with the miner reward.
+    let mut coinbase_outputs: Vec<(Amount<NonNegative>, transparent::Address)> = funding_streams
+        .into_iter()
+        .map(|(_receiver, (amount, address))| (amount, address))
+        .collect();
     coinbase_outputs.push((miner_reward, miner_address));
 
-    coinbase_outputs
+    let mut coinbase_outputs: Vec<(Amount<NonNegative>, transparent::Script)> = coinbase_outputs
         .iter()
         .map(|(amount, address)| (*amount, address.create_script_from_address()))
-        .collect()
+        .collect();
+
+    // The HashMap returns funding streams in an arbitrary order,
+    // but Zebra's snapshot tests expect the same order every time.
+    if like_zcashd {
+        // zcashd sorts outputs in serialized data order, excluding the length field
+        coinbase_outputs.sort_by_key(|(_amount, script)| script.clone());
+    } else {
+        // Zebra sorts by amount then script.
+        //
+        // Since the sort is stable, equal amounts will remain sorted by script.
+        coinbase_outputs.sort_by_key(|(_amount, script)| script.clone());
+        coinbase_outputs.sort_by_key(|(amount, _script)| *amount);
+    }
+
+    coinbase_outputs
 }
 
 // - Transaction roots processing