merge-queue: embarking main (1424115) and [#5508 + #5537] together

zebra-chain/src/block/merkle.rs

@@ -6,7 +6,7 @@ use hex::{FromHex, ToHex};
 
 use crate::{
     serialization::sha256d,
-    transaction::{self, Transaction, UnminedTx, UnminedTxId},
+    transaction::{self, Transaction, UnminedTx, UnminedTxId, VerifiedUnminedTx},
 };
 
 #[cfg(any(any(test, feature = "proptest-impl"), feature = "proptest-impl"))]
@@ -204,6 +204,18 @@ impl std::iter::FromIterator<UnminedTxId> for Root {
     }
 }
 
+impl std::iter::FromIterator<VerifiedUnminedTx> for Root {
+    fn from_iter<I>(transactions: I) -> Self
+    where
+        I: IntoIterator<Item = VerifiedUnminedTx>,
+    {
+        transactions
+            .into_iter()
+            .map(|tx| tx.transaction.id.mined_id())
+            .collect()
+    }
+}
+
 impl std::iter::FromIterator<transaction::Hash> for Root {
     /// # Panics
     ///
@@ -351,6 +363,23 @@ impl std::iter::FromIterator<UnminedTx> for AuthDataRoot {
     }
 }
 
+impl std::iter::FromIterator<VerifiedUnminedTx> for AuthDataRoot {
+    fn from_iter<I>(transactions: I) -> Self
+    where
+        I: IntoIterator<Item = VerifiedUnminedTx>,
+    {
+        transactions
+            .into_iter()
+            .map(|tx| {
+                tx.transaction
+                    .id
+                    .auth_digest()
+                    .unwrap_or(AUTH_DIGEST_PLACEHOLDER)
+            })
+            .collect()
+    }
+}
+
 impl std::iter::FromIterator<UnminedTxId> for AuthDataRoot {
     fn from_iter<I>(tx_ids: I) -> Self
     where

zebra-chain/src/transaction/unmined.rs

@@ -284,23 +284,33 @@ pub struct VerifiedUnminedTx {
 
     /// The transaction fee for this unmined transaction.
     pub miner_fee: Amount<NonNegative>,
+
+    /// The number of legacy signature operations in this transaction's
+    /// transparent inputs and outputs.
+    pub legacy_sigop_count: u64,
 }
 
 impl fmt::Display for VerifiedUnminedTx {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("VerifiedUnminedTx")
             .field("transaction", &self.transaction)
             .field("miner_fee", &self.miner_fee)
+            .field("legacy_sigop_count", &self.legacy_sigop_count)
             .finish()
     }
 }
 
 impl VerifiedUnminedTx {
-    /// Create a new verified unmined transaction from a transaction and its fee.
-    pub fn new(transaction: UnminedTx, miner_fee: Amount<NonNegative>) -> Self {
+    /// Create a new verified unmined transaction from a transaction, its fee and the legacy sigop count.
+    pub fn new(
+        transaction: UnminedTx,
+        miner_fee: Amount<NonNegative>,
+        legacy_sigop_count: u64,
+    ) -> Self {
         Self {
             transaction,
             miner_fee,
+            legacy_sigop_count,
         }
     }
 

zebra-consensus/src/block.rs

@@ -229,9 +229,7 @@ where
                     response
                 );
 
-                legacy_sigop_count += response
-                    .legacy_sigop_count()
-                    .expect("block transaction responses must have a legacy sigop count");
+                legacy_sigop_count += response.legacy_sigop_count();
 
                 // Coinbase transactions consume the miner fee,
                 // so they don't add any value to the block's total miner fee.

zebra-consensus/src/transaction.rs

@@ -249,7 +249,9 @@ impl Response {
 
     /// The miner fee for the transaction in this response.
     ///
-    /// Coinbase transactions do not have a miner fee.
+    /// Coinbase transactions do not have a miner fee,
+    /// and they don't need UTXOs to calculate their value balance,
+    /// because they don't spend any inputs.
     pub fn miner_fee(&self) -> Option<Amount<NonNegative>> {
         match self {
             Response::Block { miner_fee, .. } => *miner_fee,
@@ -259,15 +261,12 @@ impl Response {
 
     /// The number of legacy transparent signature operations in this transaction's
     /// inputs and outputs.
-    ///
-    /// Zebra does not check the legacy sigop count for mempool transactions,
-    /// because it is a standard rule (not a consensus rule).
-    pub fn legacy_sigop_count(&self) -> Option<u64> {
+    pub fn legacy_sigop_count(&self) -> u64 {
         match self {
             Response::Block {
                 legacy_sigop_count, ..
-            } => Some(*legacy_sigop_count),
-            Response::Mempool { .. } => None,
+            } => *legacy_sigop_count,
+            Response::Mempool { transaction, .. } => transaction.legacy_sigop_count,
         }
     }
 
@@ -420,18 +419,21 @@ where
                 };
             }
 
+            let legacy_sigop_count = cached_ffi_transaction.legacy_sigop_count()?;
+
             let rsp = match req {
                 Request::Block { .. } => Response::Block {
                     tx_id,
                     miner_fee,
-                    legacy_sigop_count: cached_ffi_transaction.legacy_sigop_count()?,
+                    legacy_sigop_count,
                 },
                 Request::Mempool { transaction, .. } => Response::Mempool {
                     transaction: VerifiedUnminedTx::new(
                         transaction,
                         miner_fee.expect(
                             "unexpected mempool coinbase transaction: should have already rejected",
                         ),
+                        legacy_sigop_count,
                     ),
                 },
             };

zebra-node-services/src/mempool.rs

@@ -4,7 +4,10 @@
 
 use std::collections::HashSet;
 
-use zebra_chain::transaction::{Hash, UnminedTx, UnminedTxId};
+use zebra_chain::transaction::{self, UnminedTx, UnminedTxId};
+
+#[cfg(feature = "getblocktemplate-rpcs")]
+use zebra_chain::transaction::VerifiedUnminedTx;
 
 use crate::BoxError;
 
@@ -22,24 +25,28 @@ pub use self::gossip::Gossip;
 /// because all mempool transactions must be verified.
 #[derive(Debug, Eq, PartialEq)]
 pub enum Request {
-    /// Query all transaction IDs in the mempool.
+    /// Query all [`UnminedTxId`]s in the mempool.
     TransactionIds,
 
-    /// Query matching  transactions in the mempool,
+    /// Query matching [`UnminedTx`] in the mempool,
     /// using a unique set of [`UnminedTxId`]s.
     TransactionsById(HashSet<UnminedTxId>),
 
-    /// Query matching  transactions in the mempool,
-    /// using a unique set of [`struct@Hash`]s. Pre-V5 transactions are matched
+    /// Query matching [`UnminedTx`] in the mempool,
+    /// using a unique set of [`transaction::Hash`]es. Pre-V5 transactions are matched
     /// directly; V5 transaction are matched just by the Hash, disregarding
     /// the [`AuthDigest`](zebra_chain::transaction::AuthDigest).
-    TransactionsByMinedId(HashSet<Hash>),
+    TransactionsByMinedId(HashSet<transaction::Hash>),
 
-    /// Get all the transactions in the mempool.
+    /// Get all the [`VerifiedUnminedTx`] in the mempool.
     ///
-    /// Equivalent to `TransactionsById(TransactionIds)`.
+    /// Equivalent to `TransactionsById(TransactionIds)`,
+    /// but each transaction also includes the `miner_fee` and `legacy_sigop_count` fields.
+    //
+    // TODO: make the Transactions response return VerifiedUnminedTx,
+    //       and remove the FullTransactions variant
     #[cfg(feature = "getblocktemplate-rpcs")]
-    Transactions,
+    FullTransactions,
 
     /// Query matching cached rejected transaction IDs in the mempool,
     /// using a unique set of [`UnminedTxId`]s.
@@ -80,18 +87,25 @@ pub enum Request {
 /// confirm that the mempool has been checked for newly verified transactions.
 #[derive(Debug)]
 pub enum Response {
-    /// Returns all transaction IDs from the mempool.
+    /// Returns all [`UnminedTxId`]s from the mempool.
     TransactionIds(HashSet<UnminedTxId>),
 
-    /// Returns matching transactions from the mempool.
+    /// Returns matching [`UnminedTx`] from the mempool.
     ///
     /// Since the [`Request::TransactionsById`] request is unique,
     /// the response transactions are also unique. The same applies to
     /// [`Request::TransactionsByMinedId`] requests, since the mempool does not allow
     /// different transactions with different mined IDs.
     Transactions(Vec<UnminedTx>),
 
-    /// Returns matching cached rejected transaction IDs from the mempool,
+    /// Returns all [`VerifiedUnminedTx`] in the mempool.
+    //
+    // TODO: make the Transactions response return VerifiedUnminedTx,
+    //       and remove the FullTransactions variant
+    #[cfg(feature = "getblocktemplate-rpcs")]
+    FullTransactions(Vec<VerifiedUnminedTx>),
+
+    /// Returns matching cached rejected [`UnminedTxId`]s from the mempool,
     RejectedTransactionIds(HashSet<UnminedTxId>),
 
     /// Returns a list of queue results.

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -214,18 +214,18 @@ where
         // Since this is a very large RPC, we use separate functions for each group of fields.
         async move {
             // TODO: put this in a separate get_mempool_transactions() function
-            let request = mempool::Request::Transactions;
+            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::Transactions(transactions) = response {
+            let transactions = if let mempool::Response::FullTransactions(transactions) = response {
                 // TODO: select transactions according to ZIP-317 (#5473)
                 transactions
             } else {
-                unreachable!("unmatched response to a mempool::Transactions request");
+                unreachable!("unmatched response to a mempool::FullTransactions request");
             };
 
             let merkle_root;

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

@@ -3,7 +3,7 @@
 use zebra_chain::{
     amount::{self, Amount, NonNegative},
     block::merkle::AUTH_DIGEST_PLACEHOLDER,
-    transaction::{self, SerializedTransaction, UnminedTx},
+    transaction::{self, SerializedTransaction, VerifiedUnminedTx},
 };
 
 /// Transaction data and fields needed to generate blocks using the `getblocktemplate` RPC.
@@ -39,13 +39,9 @@ where
     /// Non-coinbase transactions must be `NonNegative`.
     /// The Coinbase transaction `fee` is the negative sum of the fees of the transactions in
     /// the block, so their fee must be `NegativeOrZero`.
-    //
-    // TODO: add a fee field to mempool transactions, based on the verifier response.
     pub(crate) fee: Amount<FeeConstraint>,
 
     /// The number of transparent signature operations in this transaction.
-    //
-    // TODO: add a sigops field to mempool transactions, based on the verifier response.
     pub(crate) sigops: u64,
 
     /// Is this transaction required in the block?
@@ -55,30 +51,32 @@ where
 }
 
 // Convert from a mempool transaction to a transaction template.
-impl From<&UnminedTx> for TransactionTemplate<NonNegative> {
-    fn from(tx: &UnminedTx) -> Self {
+impl From<&VerifiedUnminedTx> for TransactionTemplate<NonNegative> {
+    fn from(tx: &VerifiedUnminedTx) -> Self {
         Self {
-            data: tx.transaction.as_ref().into(),
-            hash: tx.id.mined_id(),
-            auth_digest: tx.id.auth_digest().unwrap_or(AUTH_DIGEST_PLACEHOLDER),
+            data: tx.transaction.transaction.as_ref().into(),
+            hash: tx.transaction.id.mined_id(),
+            auth_digest: tx
+                .transaction
+                .id
+                .auth_digest()
+                .unwrap_or(AUTH_DIGEST_PLACEHOLDER),
 
             // Always empty, not supported by Zebra's mempool.
             depends: Vec::new(),
 
-            // TODO: add a fee field to mempool transactions, based on the verifier response.
-            fee: Amount::zero(),
+            fee: tx.miner_fee,
 
-            // TODO: add a sigops field to mempool transactions, based on the verifier response.
-            sigops: 0,
+            sigops: tx.legacy_sigop_count,
 
             // Zebra does not require any transactions except the coinbase transaction.
             required: false,
         }
     }
 }
 
-impl From<UnminedTx> for TransactionTemplate<NonNegative> {
-    fn from(tx: UnminedTx) -> Self {
+impl From<VerifiedUnminedTx> for TransactionTemplate<NonNegative> {
+    fn from(tx: VerifiedUnminedTx) -> Self {
         Self::from(&tx)
     }
 }

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

@@ -128,7 +128,7 @@ async fn test_rpc_response_data_for_network(network: Network) {
     // - as we have the mempool mocked we need to expect a request and wait for a response,
     // which will be an empty mempool in this case.
     let mempool_req = mempool
-        .expect_request_that(|_request| true)
+        .expect_request_that(|request| matches!(request, mempool::Request::TransactionIds))
         .map(|responder| {
             responder.respond(mempool::Response::TransactionIds(
                 std::collections::HashSet::new(),
@@ -152,9 +152,11 @@ async fn test_rpc_response_data_for_network(network: Network) {
     // `getrawtransaction`
     //
     // - similar to `getrawmempool` described above, a mempool request will be made to get the requested
-    // transaction from the mempoo, response will be empty as we have this transaction in state
+    // transaction from the mempool, response will be empty as we have this transaction in state
     let mempool_req = mempool
-        .expect_request_that(|_request| true)
+        .expect_request_that(|request| {
+            matches!(request, mempool::Request::TransactionsByMinedId(_))
+        })
         .map(|responder| {
             responder.respond(mempool::Response::Transactions(vec![]));
         });

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

@@ -61,9 +61,9 @@ pub async fn test_responses<State>(
     let get_block_template = tokio::spawn(get_block_template_rpc.get_block_template());
 
     mempool
-        .expect_request(mempool::Request::Transactions)
+        .expect_request(mempool::Request::FullTransactions)
         .await
-        .respond(mempool::Response::Transactions(vec![]));
+        .respond(mempool::Response::FullTransactions(vec![]));
 
     let get_block_template = get_block_template
         .await

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

@@ -758,9 +758,9 @@ async fn rpc_getblocktemplate() {
     let get_block_template = tokio::spawn(get_block_template_rpc.get_block_template());
 
     mempool
-        .expect_request(mempool::Request::Transactions)
+        .expect_request(mempool::Request::FullTransactions)
         .await
-        .respond(mempool::Response::Transactions(vec![]));
+        .respond(mempool::Response::FullTransactions(vec![]));
 
     let get_block_template = get_block_template
         .await

zebra-state/src/service.rs

@@ -609,14 +609,17 @@ impl StateService {
             .contains(&prepared.hash)
         {
             let (rsp_tx, rsp_rx) = oneshot::channel();
-            let _ = rsp_tx.send(Err("block already sent to be committed to the state".into()));
+            let _ = rsp_tx.send(Err(
+                "block has already been sent to be committed to the state".into(),
+            ));
             return rsp_rx;
         }
 
         if self.read_service.db.contains_height(prepared.height) {
             let (rsp_tx, rsp_rx) = oneshot::channel();
             let _ = rsp_tx.send(Err(
-                "block height is already committed to the finalized state".into(),
+                "block height is in the finalized state: block is already committed to the state"
+                    .into(),
             ));
             return rsp_rx;
         }