sorts mempool_txs before generating merkle root (#5953)

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -27,7 +27,7 @@ use crate::methods::{
         },
         get_block_template::{
             check_miner_address, check_synced_to_tip, fetch_mempool_transactions,
-            fetch_state_tip_and_local_time, generate_coinbase_and_roots, validate_block_proposal,
+            fetch_state_tip_and_local_time, validate_block_proposal,
         },
         types::{
             get_block_template::GetBlockTemplate, get_mining_info, hex_data::HexData,
@@ -566,24 +566,12 @@ where
 
             // - After this point, the template only depends on the previously fetched data.
 
-            // Generate the coinbase transaction and default roots
-            //
-            // TODO: move expensive root, hash, and tree cryptography to a rayon thread?
-            let (coinbase_txn, default_roots) = generate_coinbase_and_roots(
-                network,
-                next_block_height,
-                miner_address,
-                &mempool_txs,
-                chain_tip_and_local_time.history_tree.clone(),
-                COINBASE_LIKE_ZCASHD,
-            );
-
             let response = GetBlockTemplate::new(
+                network,
+                miner_address,
                 &chain_tip_and_local_time,
                 server_long_poll_id,
-                coinbase_txn,
-                &mempool_txs,
-                default_roots,
+                mempool_txs,
                 submit_old,
                 COINBASE_LIKE_ZCASHD,
             );

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

@@ -3,8 +3,10 @@
 use zebra_chain::{
     amount,
     block::{ChainHistoryBlockTxAuthCommitmentHash, MAX_BLOCK_BYTES, ZCASH_BLOCK_VERSION},
+    parameters::Network,
     serialization::DateTime32,
     transaction::VerifiedUnminedTx,
+    transparent,
     work::difficulty::{CompactDifficulty, ExpandedDifficulty},
 };
 use zebra_consensus::MAX_BLOCK_SIGOPS;
@@ -16,6 +18,7 @@ use crate::methods::{
             GET_BLOCK_TEMPLATE_CAPABILITIES_FIELD, GET_BLOCK_TEMPLATE_MUTABLE_FIELD,
             GET_BLOCK_TEMPLATE_NONCE_RANGE_FIELD,
         },
+        get_block_template::generate_coinbase_and_roots,
         types::{
             default_roots::DefaultRoots, long_poll::LongPollId, transaction::TransactionTemplate,
         },
@@ -177,11 +180,11 @@ impl GetBlockTemplate {
     /// If `like_zcashd` is true, try to match the coinbase transactions generated by `zcashd`
     /// in the `getblocktemplate` RPC.
     pub fn new(
+        network: Network,
+        miner_address: transparent::Address,
         chain_tip_and_local_time: &GetBlockTemplateChainInfo,
         long_poll_id: LongPollId,
-        coinbase_txn: TransactionTemplate<amount::NegativeOrZero>,
-        mempool_txs: &[VerifiedUnminedTx],
-        default_roots: DefaultRoots,
+        mempool_txs: Vec<VerifiedUnminedTx>,
         submit_old: Option<bool>,
         like_zcashd: bool,
     ) -> Self {
@@ -190,20 +193,41 @@ impl GetBlockTemplate {
             (chain_tip_and_local_time.tip_height + 1).expect("tip is far below Height::MAX");
 
         // Convert transactions into TransactionTemplates
-        let mut mempool_txs: Vec<TransactionTemplate<amount::NonNegative>> =
-            mempool_txs.iter().map(Into::into).collect();
+        let mut mempool_txs_with_templates: Vec<(
+            TransactionTemplate<amount::NonNegative>,
+            VerifiedUnminedTx,
+        )> = mempool_txs
+            .into_iter()
+            .map(|tx| ((&tx).into(), tx))
+            .collect();
 
         // Transaction selection returns transactions in an arbitrary order,
         // but Zebra's snapshot tests expect the same order every time.
         if like_zcashd {
             // Sort in serialized data order, excluding the length byte.
             // `zcashd` sometimes seems to do this, but other times the order is arbitrary.
-            mempool_txs.sort_by_key(|tx| tx.data.clone());
+            mempool_txs_with_templates.sort_by_key(|(tx_template, _tx)| tx_template.data.clone());
         } else {
             // Sort by hash, this is faster.
-            mempool_txs.sort_by_key(|tx| tx.hash.bytes_in_display_order());
+            mempool_txs_with_templates
+                .sort_by_key(|(tx_template, _tx)| tx_template.hash.bytes_in_display_order());
         }
 
+        let (mempool_tx_templates, mempool_txs): (Vec<_>, Vec<_>) =
+            mempool_txs_with_templates.into_iter().unzip();
+
+        // Generate the coinbase transaction and default roots
+        //
+        // TODO: move expensive root, hash, and tree cryptography to a rayon thread?
+        let (coinbase_txn, default_roots) = generate_coinbase_and_roots(
+            network,
+            next_block_height,
+            miner_address,
+            &mempool_txs,
+            chain_tip_and_local_time.history_tree.clone(),
+            like_zcashd,
+        );
+
         // Convert difficulty
         let target = chain_tip_and_local_time
             .expected_difficulty
@@ -228,7 +252,7 @@ impl GetBlockTemplate {
             final_sapling_root_hash: default_roots.block_commitments_hash,
             default_roots,
 
-            transactions: mempool_txs,
+            transactions: mempool_tx_templates,
 
             coinbase_txn,