5. change(rpc): Implement coinbase conversion to RPC `TransactionTemplate` type (#5554)

* Split out a select_mempool_transactions() function

* Add some TODOs

* Cleanup redundant dependencies

* Draft conversion from coinbase Transactions into TransactionTemplates

* Document a non-coinbase requirement for remaining_transaction_balance()

* Add a Network field to the getblocktemplate RPC handler

* Clarify an error message

* Re-raise panics in the getblocktemplate task, for better debugging

* Fix how the fake coinbase transaction is created

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Cargo.lock

@@ -5398,6 +5398,7 @@ dependencies = [
  "zebra-consensus",
  "zebra-network",
  "zebra-node-services",
+ "zebra-script",
  "zebra-state",
  "zebra-test",
 ]

zebra-chain/src/value_balance.rs

@@ -142,7 +142,7 @@ where
 }
 
 impl ValueBalance<NegativeAllowed> {
-    /// Assumes that this value balance is a transaction value balance,
+    /// Assumes that this value balance is a non-coinbase transaction value balance,
     /// and returns the remaining value in the transaction value pool.
     ///
     /// # Consensus

zebra-rpc/Cargo.toml

@@ -46,18 +46,17 @@ zebra-chain = { path = "../zebra-chain" }
 zebra-consensus = { path = "../zebra-consensus" }
 zebra-network = { path = "../zebra-network" }
 zebra-node-services = { path = "../zebra-node-services" }
+zebra-script = { path = "../zebra-script" }
 zebra-state = { path = "../zebra-state" }
 
 [dev-dependencies]
 insta = { version = "1.21.0", features = ["redactions", "json"] }
 proptest = "0.10.1"
 proptest-derive = "0.3.0"
-serde_json = "1.0.87"
 thiserror = "1.0.37"
 
 tokio = { version = "1.21.2", features = ["full", "tracing", "test-util"] }
 
 zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] }
-zebra-consensus = { path = "../zebra-consensus" }
 zebra-state = { path = "../zebra-state", features = ["proptest-impl"] }
-zebra-test = { path = "../zebra-test/" }
+zebra-test = { path = "../zebra-test" }

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -1,6 +1,6 @@
 //! RPC methods related to mining only available with `getblocktemplate-rpcs` rust feature.
 
-use std::sync::Arc;
+use std::{iter, sync::Arc};
 
 use futures::{FutureExt, TryFutureExt};
 use jsonrpc_core::{self, BoxFuture, Error, ErrorCode, Result};
@@ -8,11 +8,17 @@ use jsonrpc_derive::rpc;
 use tower::{buffer::Buffer, Service, ServiceExt};
 
 use zebra_chain::{
-    amount::Amount,
+    amount::{self, Amount, NonNegative},
     block::Height,
-    block::{self, Block},
+    block::{
+        self,
+        merkle::{self, AuthDataRoot},
+        Block,
+    },
     chain_tip::ChainTip,
+    parameters::Network,
     serialization::ZcashDeserializeInto,
+    transaction::{UnminedTx, VerifiedUnminedTx},
 };
 use zebra_consensus::{BlockError, VerifyBlockError, VerifyChainError, VerifyCheckpointError};
 use zebra_node_services::mempool;
@@ -126,6 +132,9 @@ where
     // Configuration
     //
     // TODO: add mining config for getblocktemplate RPC miner address
+    //
+    /// The configured network for this RPC service.
+    _network: Network,
 
     // Services
     //
@@ -166,12 +175,14 @@ where
 {
     /// Create a new instance of the handler for getblocktemplate RPCs.
     pub fn new(
+        network: Network,
         mempool: Buffer<Mempool, mempool::Request>,
         state: State,
         latest_chain_tip: Tip,
         chain_verifier: ChainVerifier,
     ) -> Self {
         Self {
+            _network: network,
             mempool,
             state,
             latest_chain_tip,
@@ -250,35 +261,63 @@ where
         // Since this is a very large RPC, we use separate functions for each group of fields.
         async move {
             let _tip_height = best_chain_tip_height(&latest_chain_tip)?;
+            let mempool_txs = select_mempool_transactions(mempool).await?;
 
-            // TODO: put this in a separate get_mempool_transactions() function
+            let miner_fee = miner_fee(&mempool_txs);
-            let request = mempool::Request::FullTransactions;
-            let response = mempool.oneshot(request).await.map_err(|error| Error {
-                code: ErrorCode::ServerError(0),
-                message: error.to_string(),
-                data: None,
-            })?;
 
-            let transactions = if let mempool::Response::FullTransactions(transactions) = response {
+            /*
-                // TODO: select transactions according to ZIP-317 (#5473)
+            Fake a "coinbase" transaction by duplicating a mempool transaction,
-                transactions
+            or fake a response.
+
+            This is temporarily required for the tests to pass.
+
+            TODO: create a method Transaction::new_v5_coinbase(network, tip_height, miner_fee),
+                  and call it here.
+             */
+            let coinbase_tx = if mempool_txs.is_empty() {
+                let empty_string = String::from("");
+                return Ok(GetBlockTemplate {
+                    capabilities: vec![],
+                    version: 0,
+                    previous_block_hash: GetBlockHash([0; 32].into()),
+                    block_commitments_hash: [0; 32].into(),
+                    light_client_root_hash: [0; 32].into(),
+                    final_sapling_root_hash: [0; 32].into(),
+                    default_roots: DefaultRoots {
+                        merkle_root: [0; 32].into(),
+                        chain_history_root: [0; 32].into(),
+                        auth_data_root: [0; 32].into(),
+                        block_commitments_hash: [0; 32].into(),
+                    },
+                    transactions: Vec::new(),
+                    coinbase_txn: TransactionTemplate {
+                        data: Vec::new().into(),
+                        hash: [0; 32].into(),
+                        auth_digest: [0; 32].into(),
+                        depends: Vec::new(),
+                        fee: Amount::zero(),
+                        sigops: 0,
+                        required: true,
+                    },
+                    target: empty_string.clone(),
+                    min_time: 0,
+                    mutable: vec![],
+                    nonce_range: empty_string.clone(),
+                    sigop_limit: 0,
+                    size_limit: 0,
+                    cur_time: 0,
+                    bits: empty_string,
+                    height: 0,
+                });
             } else {
-                unreachable!("unmatched response to a mempool::FullTransactions request");
+                mempool_txs[0].transaction.clone()
             };
 
-            let merkle_root;
+            let (merkle_root, auth_data_root) =
-            let auth_data_root;
+                calculate_transaction_roots(&coinbase_tx, &mempool_txs);
 
-            // TODO: add the coinbase transaction to these lists, and delete the is_empty() check
+            // Convert into TransactionTemplates
-            if !transactions.is_empty() {
+            let mempool_txs = mempool_txs.iter().map(Into::into).collect();
-                merkle_root = transactions.iter().cloned().collect();
-                auth_data_root = transactions.iter().cloned().collect();
-            } else {
-                merkle_root = [0; 32].into();
-                auth_data_root = [0; 32].into();
-            }
-
-            let transactions = transactions.iter().map(Into::into).collect();
 
             let empty_string = String::from("");
             Ok(GetBlockTemplate {
@@ -297,28 +336,9 @@ where
                     block_commitments_hash: [0; 32].into(),
                 },
 
-                transactions,
+                transactions: mempool_txs,
 
-                // TODO: move to a separate function in the transactions module
+                coinbase_txn: TransactionTemplate::from_coinbase(&coinbase_tx, miner_fee),
-                coinbase_txn: TransactionTemplate {
-                    // TODO: generate coinbase transaction data
-                    data: vec![].into(),
-
-                    // TODO: calculate from transaction data
-                    hash: [0; 32].into(),
-                    auth_digest: [0; 32].into(),
-
-                    // Always empty for coinbase transactions.
-                    depends: Vec::new(),
-
-                    // TODO: negative sum of transactions.*.fee
-                    fee: Amount::zero(),
-
-                    // TODO: sigops used by the generated transaction data
-                    sigops: 0,
-
-                    required: true,
-                },
 
                 target: empty_string.clone(),
 
@@ -416,6 +436,70 @@ where
     }
 }
 
+// get_block_template support methods
+
+/// Returns selected transactions in the `mempool`, or an error if the mempool has failed.
+///
+/// TODO: select transactions according to ZIP-317 (#5473)
+pub async fn select_mempool_transactions<Mempool>(
+    mempool: Mempool,
+) -> Result<Vec<VerifiedUnminedTx>>
+where
+    Mempool: Service<
+            mempool::Request,
+            Response = mempool::Response,
+            Error = zebra_node_services::BoxError,
+        > + 'static,
+    Mempool::Future: Send,
+{
+    let response = mempool
+        .oneshot(mempool::Request::FullTransactions)
+        .await
+        .map_err(|error| Error {
+            code: ErrorCode::ServerError(0),
+            message: error.to_string(),
+            data: None,
+        })?;
+
+    if let mempool::Response::FullTransactions(transactions) = response {
+        // TODO: select transactions according to ZIP-317 (#5473)
+        Ok(transactions)
+    } else {
+        unreachable!("unmatched response to a mempool::FullTransactions request");
+    }
+}
+
+/// Returns the total miner fee for `mempool_txs`.
+pub fn miner_fee(mempool_txs: &[VerifiedUnminedTx]) -> Amount<NonNegative> {
+    let miner_fee: amount::Result<Amount<NonNegative>> =
+        mempool_txs.iter().map(|tx| tx.miner_fee).sum();
+
+    miner_fee.expect(
+        "invalid selected transactions: \
+         fees in a valid block can not be more than MAX_MONEY",
+    )
+}
+
+/// Returns the transaction effecting and authorizing roots
+/// for `coinbase_tx` and `mempool_txs`.
+//
+// TODO: should this be spawned into a cryptographic operations pool?
+//       (it would only matter if there were a lot of small transactions in a block)
+pub fn calculate_transaction_roots(
+    coinbase_tx: &UnminedTx,
+    mempool_txs: &[VerifiedUnminedTx],
+) -> (merkle::Root, AuthDataRoot) {
+    let block_transactions =
+        || iter::once(coinbase_tx).chain(mempool_txs.iter().map(|tx| &tx.transaction));
+
+    let merkle_root = block_transactions().cloned().collect();
+    let auth_data_root = block_transactions().cloned().collect();
+
+    (merkle_root, auth_data_root)
+}
+
+// get_block_hash support methods
+
 /// Given a potentially negative index, find the corresponding `Height`.
 ///
 /// This function is used to parse the integer index argument of `get_block_hash`.

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

@@ -1,10 +1,11 @@
 //! The `TransactionTemplate` type is part of the `getblocktemplate` RPC method output.
 
 use zebra_chain::{
-    amount::{self, Amount, NonNegative},
+    amount::{self, Amount, NegativeOrZero, NonNegative},
     block::merkle::AUTH_DIGEST_PLACEHOLDER,
-    transaction::{self, SerializedTransaction, VerifiedUnminedTx},
+    transaction::{self, SerializedTransaction, UnminedTx, VerifiedUnminedTx},
 };
+use zebra_script::CachedFfiTransaction;
 
 /// Transaction data and fields needed to generate blocks using the `getblocktemplate` RPC.
 #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
@@ -50,9 +51,14 @@ where
     pub(crate) required: bool,
 }
 
-// Convert from a mempool transaction to a transaction template.
+// Convert from a mempool transaction to a non-coinbase transaction template.
 impl From<&VerifiedUnminedTx> for TransactionTemplate<NonNegative> {
     fn from(tx: &VerifiedUnminedTx) -> Self {
+        assert!(
+            !tx.transaction.transaction.is_coinbase(),
+            "unexpected coinbase transaction in mempool"
+        );
+
         Self {
             data: tx.transaction.transaction.as_ref().into(),
             hash: tx.transaction.id.mined_id(),
@@ -80,3 +86,45 @@ impl From<VerifiedUnminedTx> for TransactionTemplate<NonNegative> {
         Self::from(&tx)
     }
 }
+
+impl TransactionTemplate<NegativeOrZero> {
+    /// Convert from a generated coinbase transaction into a coinbase transaction template.
+    ///
+    /// `miner_fee` is the total miner fees for the block, excluding newly created block rewards.
+    //
+    // TODO: use a different type for generated coinbase transactions?
+    pub fn from_coinbase(tx: &UnminedTx, miner_fee: Amount<NonNegative>) -> Self {
+        assert!(
+            tx.transaction.is_coinbase(),
+            "invalid generated coinbase transaction: \
+             must have exactly one input, which must be a coinbase input",
+        );
+
+        let miner_fee = miner_fee
+            .constrain()
+            .expect("negating a NonNegative amount always results in a valid NegativeOrZero");
+
+        let legacy_sigop_count = CachedFfiTransaction::new(tx.transaction.clone(), Vec::new())
+            .legacy_sigop_count()
+            .expect(
+                "invalid generated coinbase transaction: \
+                 failure in zcash_script sigop count",
+            );
+
+        Self {
+            data: tx.transaction.as_ref().into(),
+            hash: tx.id.mined_id(),
+            auth_digest: tx.id.auth_digest().unwrap_or(AUTH_DIGEST_PLACEHOLDER),
+
+            // Always empty, coinbase transactions never have inputs.
+            depends: Vec::new(),
+
+            fee: miner_fee,
+
+            sigops: legacy_sigop_count,
+
+            // Zcash requires a coinbase transaction.
+            required: true,
+        }
+    }
+}

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs

@@ -65,6 +65,7 @@ pub async fn test_responses<State, ReadState>(
     .await;
 
     let get_block_template_rpc = GetBlockTemplateRpcImpl::new(
+        network,
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip,

zebra-rpc/src/methods/tests/vectors.rs

@@ -650,6 +650,7 @@ async fn rpc_getblockcount() {
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+        Mainnet,
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip.clone(),
@@ -693,6 +694,7 @@ async fn rpc_getblockcount_empty_state() {
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+        Mainnet,
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip.clone(),
@@ -742,6 +744,7 @@ async fn rpc_getblockhash() {
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+        Mainnet,
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip.clone(),
@@ -777,6 +780,8 @@ async fn rpc_getblockhash() {
 #[cfg(feature = "getblocktemplate-rpcs")]
 #[tokio::test(flavor = "multi_thread")]
 async fn rpc_getblocktemplate() {
+    use std::panic;
+
     let _init_guard = zebra_test::init();
 
     // Create a continuous chain of mainnet blocks from genesis
@@ -805,6 +810,7 @@ async fn rpc_getblocktemplate() {
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+        Mainnet,
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip.clone(),
@@ -820,7 +826,12 @@ async fn rpc_getblocktemplate() {
 
     let get_block_template = get_block_template
         .await
-        .expect("unexpected panic in getblocktemplate RPC task")
+        .unwrap_or_else(|error| match error.try_into_panic() {
+            Ok(panic_object) => panic::resume_unwind(panic_object),
+            Err(cancelled_error) => {
+                panic!("getblocktemplate task was unexpectedly cancelled: {cancelled_error:?}")
+            }
+        })
         .expect("unexpected error in getblocktemplate RPC call");
 
     assert!(get_block_template.capabilities.is_empty());
@@ -880,6 +891,7 @@ async fn rpc_submitblock_errors() {
 
     // Init RPC
     let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
+        Mainnet,
         Buffer::new(mempool.clone(), 1),
         read_state,
         latest_chain_tip.clone(),

zebra-rpc/src/server.rs

@@ -91,6 +91,7 @@ impl RpcServer {
             {
                 // Initialize the getblocktemplate rpc method handler
                 let get_block_template_rpc_impl = GetBlockTemplateRpcImpl::new(
+                    network,
                     mempool.clone(),
                     state.clone(),
                     latest_chain_tip.clone(),