Return transaction fee (#2876)

* Get the transaction fee from utxos

* Return the transaction fee from the verifier

* Avoid calculating the fee for coinbase transactions

Coinbase transactions don't have fees. In case of a coinbase transaction, the
verifier returns a zero fee.

* Update the result obtained by `Downloads`

zebra-consensus/src/error.rs

@@ -99,6 +99,9 @@ pub enum TransactionError {
 
     #[error("adding to the sprout pool is disabled after Canopy")]
     DisabledAddToSproutPool,
+
+    #[error("could not calculate the transaction fee")]
+    IncorrectFee,
 }
 
 impl From<BoxError> for TransactionError {

zebra-consensus/src/transaction.rs

@@ -18,6 +18,7 @@ use tower::{Service, ServiceExt};
 use tracing::Instrument;
 
 use zebra_chain::{
+    amount::{Amount, NonNegative},
     block, orchard,
     parameters::{Network, NetworkUpgrade},
     primitives::Groth16Proof,
@@ -102,7 +103,7 @@ pub enum Request {
 ///
 /// [`Mempool`] requests are uniquely identified by the [`UnminedTxId`]
 /// variant for their transaction version.
-pub type Response = zebra_chain::transaction::UnminedTxId;
+pub type Response = (zebra_chain::transaction::UnminedTxId, Amount<NonNegative>);
 
 impl Request {
     /// The transaction to verify that's in this request.
@@ -251,34 +252,24 @@ where
                 spent_utxos.insert(script_rsp.spent_outpoint, script_rsp.spent_utxo);
             }
 
-            // temporary assertions for testing ticket #2440
-            //
-            // TODO: use spent_utxos to calculate the transaction fee (#2779)
-            //       and remove these assertions
-            if tx.has_valid_coinbase_transaction_inputs() {
-                assert_eq!(
-                    spent_utxos.len(),
-                    0,
-                    "already checked that coinbase transactions don't spend UTXOs"
-                );
-            } else if spent_utxos.len() < tx.inputs().len() {
-                // TODO: replace with double-spend check in PR #2843
-                return Err(TransactionError::InternalDowncastError(format!(
-                    "transparent double-spend within a transaction: \
-                     expected {} input UTXOs, got {} unique spent UTXOs",
-                    tx.inputs().len(),
-                    spent_utxos.len()
-                )));
-            } else {
-                assert_eq!(
-                    spent_utxos.len(),
-                    tx.inputs().len(),
-                    "unexpected excess looked-up spent UTXOs in transaction: \
-                     expected exactly one UTXO per verified transparent input"
-                );
+            // Get the `value_balance` to calculate the transaction fee.
+            let value_balance = tx.value_balance(&spent_utxos);
+
+            // Initialize the transaction fee to zero.
+            let mut tx_fee = Amount::<NonNegative>::zero();
+
+            // Calculate the fee only for non-coinbase transactions.
+            if !tx.has_valid_coinbase_transaction_inputs() {
+                tx_fee = match value_balance {
+                    Ok(vb) => match vb.remaining_transaction_value() {
+                        Ok(tx_rtv) => tx_rtv,
+                        Err(_) => return Err(TransactionError::IncorrectFee),
+                    },
+                    Err(_) => return Err(TransactionError::IncorrectFee),
+                };
             }
 
-            Ok(id)
+            Ok((id, tx_fee))
         }
         .instrument(span)
         .boxed()

zebra-consensus/src/transaction/tests.rs

@@ -274,7 +274,10 @@ async fn v5_transaction_is_accepted_after_nu5_activation_for_network(network: Ne
         })
         .await;
 
-    assert_eq!(result, Ok(expected_hash));
+    assert_eq!(
+        result.expect("expected a tx_id and tx_fee").0,
+        expected_hash
+    );
 }
 
 /// Test if V4 transaction with transparent funds is accepted.
@@ -320,7 +323,10 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() {
         })
         .await;
 
-    assert_eq!(result, Ok(transaction_hash));
+    assert_eq!(
+        result.expect("expected a tx_id and tx_fee").0,
+        transaction_hash
+    );
 }
 
 /// Test if V4 coinbase transaction is accepted.
@@ -363,7 +369,10 @@ async fn v4_coinbase_transaction_is_accepted() {
         })
         .await;
 
-    assert_eq!(result, Ok(transaction_hash));
+    assert_eq!(
+        result.expect("expected a tx_id and tx_fee").0,
+        transaction_hash
+    );
 }
 
 /// Test if V4 transaction with transparent funds is rejected if the source script prevents it.
@@ -464,7 +473,10 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() {
         })
         .await;
 
-    assert_eq!(result, Ok(transaction_hash));
+    assert_eq!(
+        result.expect("expected a tx_id and tx_fee").0,
+        transaction_hash
+    );
 }
 
 /// Test if V5 coinbase transaction is accepted.
@@ -510,7 +522,10 @@ async fn v5_coinbase_transaction_is_accepted() {
         })
         .await;
 
-    assert_eq!(result, Ok(transaction_hash));
+    assert_eq!(
+        result.expect("expected a tx_id and tx_fee").0,
+        transaction_hash
+    );
 }
 
 /// Test if V5 transaction with transparent funds is rejected if the source script prevents it.
@@ -627,7 +642,10 @@ fn v4_with_signed_sprout_transfer_is_accepted() {
             })
             .await;
 
-        assert_eq!(result, Ok(expected_hash));
+        assert_eq!(
+            result.expect("expected a tx_id and tx_fee").0,
+            expected_hash
+        );
     });
 }
 
@@ -725,7 +743,10 @@ fn v4_with_sapling_spends() {
             })
             .await;
 
-        assert_eq!(result, Ok(expected_hash));
+        assert_eq!(
+            result.expect("expected a tx_id and tx_fee").0,
+            expected_hash
+        );
     });
 }
 
@@ -766,7 +787,10 @@ fn v4_with_sapling_outputs_and_no_spends() {
             })
             .await;
 
-        assert_eq!(result, Ok(expected_hash));
+        assert_eq!(
+            result.expect("expected a tx_id and tx_fee").0,
+            expected_hash
+        );
     });
 }
 
@@ -810,7 +834,10 @@ fn v5_with_sapling_spends() {
             })
             .await;
 
-        assert_eq!(result, Ok(expected_hash));
+        assert_eq!(
+            result.expect("expected a tx_id and tx_fee").0,
+            expected_hash
+        );
     });
 }
 

zebra-test/src/mock_service.rs

@@ -356,7 +356,7 @@ impl<Request, Response, Error> MockService<Request, Response, PanicAssertion, Er
     /// #     let mut service = mock_service.clone();
     /// #
     /// let call = tokio::spawn(mock_service.clone().oneshot(1));
-    ///  
+    ///
     /// mock_service.expect_request_that(|request| *request > 0).await.respond("response");
     ///
     /// assert!(matches!(call.await, Ok(Ok("response"))));

zebrad/src/components/inbound/tests.rs

@@ -8,6 +8,7 @@ use tower::{buffer::Buffer, builder::ServiceBuilder, util::BoxService, Service,
 use tracing::Span;
 
 use zebra_chain::{
+    amount::Amount,
     block::Block,
     parameters::Network,
     serialization::ZcashDeserializeInto,
@@ -117,7 +118,10 @@ async fn mempool_push_transaction() -> Result<(), crate::BoxError> {
     // Simulate a successful transaction verification
     let verification = tx_verifier.expect_request_that(|_| true).map(|responder| {
         let txid = responder.request().tx_id();
-        responder.respond(txid);
+
+        // Set a dummy fee.
+        let tx_fee = Amount::zero();
+        responder.respond((txid, tx_fee));
     });
     let (response, _) = futures::join!(request, verification);
     match response {
@@ -205,7 +209,10 @@ async fn mempool_advertise_transaction_ids() -> Result<(), crate::BoxError> {
     // Simulate a successful transaction verification
     let verification = tx_verifier.expect_request_that(|_| true).map(|responder| {
         let txid = responder.request().tx_id();
-        responder.respond(txid);
+
+        // Set a dummy fee.
+        let tx_fee = Amount::zero();
+        responder.respond((txid, tx_fee));
     });
     let (response, _, _) = futures::join!(request, peer_set_responder, verification);
 
@@ -292,7 +299,10 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> {
     // Simulate a successful transaction verification
     let verification = tx_verifier.expect_request_that(|_| true).map(|responder| {
         tx1_id = responder.request().tx_id();
-        responder.respond(tx1_id);
+
+        // Set a dummy fee.
+        let tx_fee = Amount::zero();
+        responder.respond((tx1_id, tx_fee));
     });
     let (response, _) = futures::join!(request, verification);
     match response {
@@ -381,7 +391,10 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> {
     // Simulate a successful transaction verification
     let verification = tx_verifier.expect_request_that(|_| true).map(|responder| {
         tx2_id = responder.request().tx_id();
-        responder.respond(tx2_id);
+
+        // Set a dummy fee.
+        let tx_fee = Amount::zero();
+        responder.respond((tx2_id, tx_fee));
     });
     let (response, _) = futures::join!(request, verification);
     match response {

zebrad/src/components/mempool/downloads.rs

@@ -300,7 +300,7 @@ where
                     transaction: tx.clone(),
                     height,
                 })
-                .map_ok(|_hash| tx)
+                .map_ok(|(_tx_id, _tx_fee)| tx)
                 .await;
 
             tracing::debug!(?txid, ?result, "verified transaction for the mempool");