validator: credit miner-claimable fees in block templates

bin/darkfid/src/registry/model.rs

@@ -49,6 +49,7 @@ use darkfi_sdk::{
         pasta_prelude::PrimeField,
         FuncId, MerkleTree, MONEY_CONTRACT_ID,
     },
+    fee::miner_claimable_fee,
     pasta::pallas,
     ContractCall,
 };
@@ -199,19 +200,22 @@ impl BlockTemplate {
             .await?)
     }
 
-    /// Return block fees.
+    /// Return block miner-claimable fees.
     ///
     /// Note: always check if block contains transactions before
     /// calling this function.
     pub async fn fees(&self) -> Result<u64> {
-        let mut fees = 0;
+        let mut fees: u64 = 0;
         'outer: for tx in &self.block.txs[..self.block.txs.len() - 1] {
             for call in &tx.calls {
                 if !call.data.is_money_fee() {
                     continue
                 }
 
-                fees += deserialize_async::<u64>(&call.data.data[1..9]).await?;
+                let fee_values = call.data.money_fee_values()?;
+                fees = fees
+                    .checked_add(miner_claimable_fee(fee_values.paid_fee, fee_values.burned_fee)?)
+                    .ok_or(Error::AdditionOverflow)?;
                 continue 'outer
             }
         }

doc/src/arch/fees.md

@@ -1,7 +1,8 @@
 # Transaction Fees
 
 DarkFi meters resource consumption as gas and prices transactions as a
-fee in DRK. The validator tracks gas across four categories:
+fee in DRK. The validator tracks gas across four categories, plus the
+transaction's paid and declared burned fees:
 
 ```rust
 pub struct GasData {
@@ -10,6 +11,7 @@ pub struct GasData {
     pub signatures: u64,
     pub deployments: u64,
     pub paid: u64,
+    pub burned: u64,
 }
 ```
 

src/contract/money/tests/fees.rs

@@ -18,6 +18,7 @@
 
 use darkfi::{
     tx::{ContractCallLeaf, Transaction, TransactionBuilder},
+    validator::consensus::Fork,
     Result,
 };
 use darkfi_contract_test_harness::{init_logger, Holder, TestHarness};
@@ -146,8 +147,7 @@ async fn converge_fee_tx(
     let mut burned_fee = 0;
 
     for _ in 0..64 {
-        let (tx, params, fee_params) =
-            fee_tx(th, from, to, coin, paid_fee, burned_fee).await?;
+        let (tx, params, fee_params) = fee_tx(th, from, to, coin, paid_fee, burned_fee).await?;
         let required = measure_required_fee(th, from, &tx, block_height).await?;
         let mandatory = burn_fee(required)?;
 
@@ -182,6 +182,61 @@ async fn execute_on_all(
     Ok(())
 }
 
+/// Confirm the mempool reward claim matches the fee accumulator:
+/// `unproposed_txs` must report the miner-claimable fees (`paid -
+/// burned`) that `Money::FeeV1` accumulates for the height, since the
+/// block template's `PoWRewardV1` claim is checked against exactly
+/// that value.
+#[test]
+fn fees_reward_claim() -> Result<()> {
+    smol::block_on(async {
+        init_logger();
+
+        use Holder::{Alice, Bob};
+
+        let mut th = TestHarness::new(&[Alice, Bob], true).await?;
+
+        // Alice mines two blocks so she holds coins to pay fees with
+        th.generate_block_all(&Alice).await?;
+        th.generate_block_all(&Alice).await?;
+        let height = 3;
+
+        // Build a fee-paying transaction and read back its declared
+        // public fee values
+        let coin = th.coins(&Alice).last().unwrap().clone();
+        let (tx, _, _, paid, burned) =
+            converge_fee_tx(&mut th, &Alice, &Bob, &coin, height, 0, 0).await?;
+        let fee_values = tx
+            .calls
+            .iter()
+            .find(|call| call.data.is_money_fee())
+            .unwrap()
+            .data
+            .money_fee_values()?;
+        assert_eq!(fee_values.paid_fee, paid);
+        assert_eq!(fee_values.burned_fee, burned);
+
+        // Put it in the mempool without applying it to the chain, like
+        // the real node flow does
+        let validator = th.wallet(&Alice).validator.read().await;
+        validator.blockchain.transactions.insert_pending(std::slice::from_ref(&tx))?;
+        let blockchain = validator.blockchain.clone();
+        let module = validator.consensus.module.clone();
+        drop(validator);
+
+        // Retrieve unproposed transactions the way the block template
+        // does, and check the reported fees against the accumulator
+        let mut fork = Fork::new(blockchain, module).await?;
+        let (unproposed, _, fees) = fork.unproposed_txs(height, true).await?;
+
+        assert_eq!(unproposed.len(), 1);
+        assert_eq!(fees, paid - burned);
+
+        // Thanks for reading
+        Ok(())
+    })
+}
+
 #[test]
 fn fees() -> Result<()> {
     smol::block_on(async {
@@ -239,8 +294,9 @@ fn fees() -> Result<()> {
         // 3. Over-declared burn is valid: only paid - burned is
         //    miner-claimable, the excess burn is not re-minted.
         let coin = th.coins(&Alice).last().unwrap().clone();
-        let (tx, params, fee_params, required, mandatory_burn) = converge_fee_tx(&mut th, &Alice, &Bob, &coin, height, TIP as i64, EXTRA_BURN as i64)
-            .await?;
+        let (tx, params, fee_params, required, mandatory_burn) =
+            converge_fee_tx(&mut th, &Alice, &Bob, &coin, height, TIP as i64, EXTRA_BURN as i64)
+                .await?;
         execute_on_all(&mut th, &holders, &tx, &params, &fee_params, height).await?;
         let inclusion_fee = required - mandatory_burn;
         th.generate_block_with_fees(&Bob, &holders, inclusion_fee + TIP - EXTRA_BURN).await?;
@@ -280,8 +336,7 @@ fn fees() -> Result<()> {
             .is_err());
 
         // 7. Declaring burned_fee > paid_fee is rejected.
-        let (tx, params, fee_params) =
-            fee_tx(&mut th, &Alice, &Bob, &coin, 1000, 1001).await?;
+        let (tx, params, fee_params) = fee_tx(&mut th, &Alice, &Bob, &coin, 1000, 1001).await?;
         assert!(th
             .execute_transfer_tx(&Alice, tx, &params, &fee_params, height, true)
             .await

src/validator/consensus.rs

@@ -750,8 +750,9 @@ impl Fork {
     }
 
     /// Auxiliary function to retrieve unproposed valid transactions,
-    /// along with their total gas used and total paid fees. Erroneous
-    /// transactions will be removed from the database.
+    /// along with their total gas used and total miner-claimable fees,
+    /// matching what `Money::FeeV1` accumulates for the height.
+    /// Erroneous transactions will be removed from the database.
     ///
     /// Note: Always remember to purge new trees from the database if
     /// not needed.
@@ -768,9 +769,9 @@ impl Fork {
         // Transactions Merkle tree
         let mut tree = MerkleTree::new(1);
 
-        // Total gas accumulators
+        // Total gas accumulator and miner-claimable fees accumulator
         let mut total_gas_used = 0_u64;
-        let mut total_gas_paid = 0_u64;
+        let mut total_claimable_fees = 0_u64;
 
         // Map of ZK proof verifying keys for the current transaction
         // batch.
@@ -832,9 +833,10 @@ impl Fork {
                 break
             }
 
-            // Update accumulated total gas
+            // Update accumulated total gas and miner-claimable fees
             total_gas_used = total_gas_used.saturating_add(tx_gas_used);
-            total_gas_paid = total_gas_paid.saturating_add(gas_data.paid);
+            total_claimable_fees =
+                total_claimable_fees.saturating_add(gas_data.paid.saturating_sub(gas_data.burned));
 
             // Push the tx hash into the unproposed transactions vector
             unproposed_txs.push(tx);
@@ -843,7 +845,7 @@ impl Fork {
         // Remove erroneous transactions from mempool
         self.blockchain.remove_pending_txs_hashes(&erroneous_txs)?;
 
-        Ok((unproposed_txs, total_gas_used, total_gas_paid))
+        Ok((unproposed_txs, total_gas_used, total_claimable_fees))
     }
 
     /// Auxiliary function to create a full clone using

src/validator/fees.rs

@@ -74,6 +74,8 @@ pub struct GasData {
     pub deployments: u64,
     /// Transaction paid fee
     pub paid: u64,
+    /// Transaction declared burned fee
+    pub burned: u64,
 }
 
 impl GasData {
@@ -98,6 +100,7 @@ impl std::fmt::Debug for GasData {
             .field("signatures", &self.signatures)
             .field("deployments", &self.deployments)
             .field("paid", &self.paid)
+            .field("burned", &self.burned)
             .finish()
     }
 }

src/validator/verification.rs

@@ -962,8 +962,9 @@ pub async fn verify_transaction(
         }
         debug!(target: "validator::verification::verify_transaction", "The gas paid for transaction {tx_hash}: {}", gas_data.paid);
 
-        // Store paid fee
+        // Store paid and burned fees
         gas_data.paid = fee;
+        gas_data.burned = fee_values.burned_fee;
     }
 
     // When we're done looping and executing over the tx's contract