contract/money: properly integrate txs fees into block rewards

bin/darkfid/src/task/miner.rs

@@ -19,11 +19,11 @@
 use std::sync::Arc;
 
 use darkfi::{
-    blockchain::BlockInfo,
+    blockchain::{BlockInfo, Header},
     rpc::{jsonrpc::JsonNotification, util::JsonValue},
     system::{StoppableTask, Subscription},
     tx::{ContractCallLeaf, Transaction, TransactionBuilder},
-    util::encoding::base64,
+    util::{encoding::base64, time::Timestamp},
     validator::{
         consensus::{Fork, Proposal},
         utils::best_fork_index,
@@ -328,10 +328,17 @@ async fn generate_next_block(
     pk: &ProvingKey,
     verify_fees: bool,
 ) -> Result<(BigUint, BlockInfo)> {
-    // Grab extended fork next block height
+    // Grab forks' last block proposal(previous)
     let last_proposal = extended_fork.last_proposal()?;
+
+    // Grab forks' next block height
     let next_block_height = last_proposal.block.header.height + 1;
 
+    // Grab forks' unproposed transactions
+    let (mut txs, fees) = extended_fork
+        .unproposed_txs(&extended_fork.blockchain, next_block_height, verify_fees)
+        .await?;
+
     // We are deriving the next secret key for optimization.
     // Next secret is the poseidon hash of:
     //  [prefix, current(previous) secret, signing(block) height].
@@ -340,11 +347,20 @@ async fn generate_next_block(
     *secret = SecretKey::from(next_secret);
 
     // Generate reward transaction
-    let tx = generate_transaction(next_block_height, secret, recipient, zkbin, pk)?;
+    let tx = generate_transaction(next_block_height, fees, secret, recipient, zkbin, pk)?;
+    txs.push(tx);
+
+    // Generate the new header
+    let header = Header::new(last_proposal.hash, next_block_height, Timestamp::current_time(), 0);
+
+    // Generate the block
+    let mut next_block = BlockInfo::new_empty(header);
+
+    // Add transactions to the block
+    next_block.append_txs(txs);
 
-    // Generate next block proposal
+    // Grab the next mine target
     let target = extended_fork.module.next_mine_target()?;
-    let next_block = extended_fork.generate_unsigned_block(tx, verify_fees).await?;
 
     Ok((target, next_block))
 }
@@ -352,6 +368,7 @@ async fn generate_next_block(
 /// Auxiliary function to generate a Money::PoWReward transaction.
 fn generate_transaction(
     block_height: u32,
+    fees: u64,
     secret: &SecretKey,
     recipient: &PublicKey,
     zkbin: &ZkBinary,
@@ -366,6 +383,7 @@ fn generate_transaction(
         secret: *secret,
         recipient: *recipient,
         block_height,
+        fees,
         spend_hook,
         user_data,
         mint_zkbin: zkbin.clone(),

bin/darkfid/src/tests/harness.rs

@@ -184,6 +184,7 @@ impl Harness {
             secret: keypair.secret,
             recipient: keypair.public,
             block_height,
+            fees: 0,
             spend_hook,
             user_data,
             mint_zkbin: zkbin.clone(),

src/contract/money/src/client/pow_reward_v1.rs

@@ -68,6 +68,8 @@ pub struct PoWRewardCallBuilder {
     pub recipient: PublicKey,
     /// Rewarded block height
     pub block_height: u32,
+    /// Rewarded block transactions paid fees
+    pub fees: u64,
     /// Merkle tree of coins used to create inclusion proofs
     /// Spend hook for the output
     pub spend_hook: FuncId,
@@ -152,13 +154,12 @@ impl PoWRewardCallBuilder {
     }
 
     pub fn build(&self) -> Result<PoWRewardCallDebris> {
-        let reward = expected_reward(self.block_height);
-        assert!(reward != 0);
+        let reward = expected_reward(self.block_height) + self.fees;
         self._build(reward)
     }
 
     /// This function should only be used for testing, as PoW reward values are predefined
     pub fn build_with_custom_reward(&self, reward: u64) -> Result<PoWRewardCallDebris> {
-        self._build(reward)
+        self._build(reward + self.fees)
     }
 }

src/contract/money/src/entrypoint.rs

@@ -33,10 +33,9 @@ use crate::{
     },
     MoneyFunction, EMPTY_COINS_TREE_ROOT, MONEY_CONTRACT_COINS_TREE,
     MONEY_CONTRACT_COIN_MERKLE_TREE, MONEY_CONTRACT_COIN_ROOTS_TREE, MONEY_CONTRACT_DB_VERSION,
-    MONEY_CONTRACT_INFO_TREE, MONEY_CONTRACT_LATEST_COIN_ROOT,
+    MONEY_CONTRACT_FEES_TREE, MONEY_CONTRACT_INFO_TREE, MONEY_CONTRACT_LATEST_COIN_ROOT,
     MONEY_CONTRACT_LATEST_NULLIFIER_ROOT, MONEY_CONTRACT_NULLIFIERS_TREE,
     MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_TOKEN_FREEZE_TREE,
-    MONEY_CONTRACT_TOTAL_FEES_PAID,
 };
 
 /// `Money::Fee` functions
@@ -163,6 +162,15 @@ fn init_contract(cid: ContractId, _ix: &[u8]) -> ContractResult {
         wasm::db::db_init(cid, MONEY_CONTRACT_TOKEN_FREEZE_TREE)?;
     }
 
+    // Set up a database tree to hold the fees paid for each block
+    // k=height_bytes, v=fees_paid_bytes
+    if wasm::db::db_lookup(cid, MONEY_CONTRACT_FEES_TREE).is_err() {
+        let fees_db = wasm::db::db_init(cid, MONEY_CONTRACT_FEES_TREE)?;
+        // Initialize the first two accumulators
+        wasm::db::db_set(fees_db, &serialize(&0_u32), &serialize(&0_u64))?;
+        wasm::db::db_set(fees_db, &serialize(&1_u32), &serialize(&0_u64))?;
+    }
+
     // Set up a database tree for arbitrary data
     let info_db = match wasm::db::db_lookup(cid, MONEY_CONTRACT_INFO_TREE) {
         Ok(v) => v,
@@ -178,9 +186,6 @@ fn init_contract(cid: ContractId, _ix: &[u8]) -> ContractResult {
             coin_tree.encode(&mut coin_tree_data)?;
             wasm::db::db_set(info_db, MONEY_CONTRACT_COIN_MERKLE_TREE, &coin_tree_data)?;
 
-            // Initialize the paid fees accumulator
-            wasm::db::db_set(info_db, MONEY_CONTRACT_TOTAL_FEES_PAID, &serialize(&0_u64))?;
-
             // Initialize coins and nulls latest root field
             // This will result in exhausted gas so we use a precalculated value:
             //let root = coin_tree.root(0).unwrap();

src/contract/money/src/entrypoint/fee_v1.rs

@@ -38,9 +38,9 @@ use crate::{
     error::MoneyError,
     model::{MoneyFeeParamsV1, MoneyFeeUpdateV1, DARK_TOKEN_ID},
     MoneyFunction, MONEY_CONTRACT_COINS_TREE, MONEY_CONTRACT_COIN_MERKLE_TREE,
-    MONEY_CONTRACT_COIN_ROOTS_TREE, MONEY_CONTRACT_INFO_TREE, MONEY_CONTRACT_LATEST_COIN_ROOT,
-    MONEY_CONTRACT_LATEST_NULLIFIER_ROOT, MONEY_CONTRACT_NULLIFIERS_TREE,
-    MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_TOTAL_FEES_PAID,
+    MONEY_CONTRACT_COIN_ROOTS_TREE, MONEY_CONTRACT_FEES_TREE, MONEY_CONTRACT_INFO_TREE,
+    MONEY_CONTRACT_LATEST_COIN_ROOT, MONEY_CONTRACT_LATEST_NULLIFIER_ROOT,
+    MONEY_CONTRACT_NULLIFIERS_TREE, MONEY_CONTRACT_NULLIFIER_ROOTS_TREE,
     MONEY_CONTRACT_ZKAS_FEE_NS_V1,
 };
 
@@ -108,10 +108,10 @@ pub(crate) fn money_fee_process_instruction_v1(
 
     // Access the necessary databases where there is information to
     // validate this state transition.
-    let info_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_INFO_TREE)?;
     let coins_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_COINS_TREE)?;
     let nullifiers_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_NULLIFIERS_TREE)?;
     let coin_roots_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_COIN_ROOTS_TREE)?;
+    let fees_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_FEES_TREE)?;
 
     // Fees can only be paid using the native token, so we'll compare
     // the token commitments with this one:
@@ -177,15 +177,17 @@ pub(crate) fn money_fee_process_instruction_v1(
         return Err(MoneyError::ValueMismatch.into())
     }
 
-    // Accumulate the paid fee
+    // Accumulate the height paid fee
+    let verifying_block_height = wasm::util::get_verifying_block_height()?;
     let mut paid_fee: u64 =
-        deserialize(&wasm::db::db_get(info_db, MONEY_CONTRACT_TOTAL_FEES_PAID)?.unwrap())?;
+        deserialize(&wasm::db::db_get(fees_db, &serialize(&verifying_block_height))?.unwrap())?;
     paid_fee += fee;
 
     // At this point the state transition has passed, so we create a state update.
     let update = MoneyFeeUpdateV1 {
         nullifier: params.input.nullifier,
         coin: params.output.coin,
+        height: verifying_block_height,
         fee: paid_fee,
     };
     let mut update_data = vec![];
@@ -206,8 +208,9 @@ pub(crate) fn money_fee_process_update_v1(
     let nullifiers_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_NULLIFIERS_TREE)?;
     let coin_roots_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_COIN_ROOTS_TREE)?;
     let nullifier_roots_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_NULLIFIER_ROOTS_TREE)?;
+    let fees_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_FEES_TREE)?;
 
-    wasm::db::db_set(info_db, MONEY_CONTRACT_TOTAL_FEES_PAID, &serialize(&update.fee))?;
+    wasm::db::db_set(fees_db, &serialize(&update.height), &serialize(&update.fee))?;
 
     wasm::merkle::sparse_merkle_insert_batch(
         info_db,

src/contract/money/src/entrypoint/pow_reward_v1.rs

@@ -31,9 +31,10 @@ use crate::{
     error::MoneyError,
     model::{MoneyPoWRewardParamsV1, MoneyPoWRewardUpdateV1, DARK_TOKEN_ID},
     MoneyFunction, MONEY_CONTRACT_COINS_TREE, MONEY_CONTRACT_COIN_MERKLE_TREE,
-    MONEY_CONTRACT_COIN_ROOTS_TREE, MONEY_CONTRACT_INFO_TREE, MONEY_CONTRACT_LATEST_COIN_ROOT,
-    MONEY_CONTRACT_LATEST_NULLIFIER_ROOT, MONEY_CONTRACT_NULLIFIERS_TREE,
-    MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_ZKAS_MINT_NS_V1,
+    MONEY_CONTRACT_COIN_ROOTS_TREE, MONEY_CONTRACT_FEES_TREE, MONEY_CONTRACT_INFO_TREE,
+    MONEY_CONTRACT_LATEST_COIN_ROOT, MONEY_CONTRACT_LATEST_NULLIFIER_ROOT,
+    MONEY_CONTRACT_NULLIFIERS_TREE, MONEY_CONTRACT_NULLIFIER_ROOTS_TREE,
+    MONEY_CONTRACT_ZKAS_MINT_NS_V1,
 };
 
 /// `get_metadata` function for `Money::PoWRewardV1`
@@ -108,13 +109,18 @@ pub(crate) fn money_pow_reward_process_instruction_v1(
         return Err(MoneyError::TransferClearInputNonNativeToken.into())
     }
 
-    // Verify reward value matches the expected one for this block height
-    let expected_reward = expected_reward(verifying_block_height);
+    // Grab the currect height accumulated fees
+    let fees_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_FEES_TREE)?;
+    let paid_fee: u64 =
+        deserialize(&wasm::db::db_get(fees_db, &serialize(&verifying_block_height))?.unwrap())?;
+
+    // Verify reward value matches the expected one for this block height,
+    // including the paid fees.
+    let expected_reward = expected_reward(verifying_block_height) + paid_fee;
     if params.input.value != expected_reward {
         msg!(
-            "[PoWRewardV1] Error: Reward value({}) is not the block height({}) expected one: {}",
+            "[PoWRewardV1] Error: Reward value({}) is not the expected one: {}",
             params.input.value,
-            verifying_block_height,
             expected_reward
         );
         return Err(MoneyError::ValueMismatch.into())
@@ -148,7 +154,8 @@ pub(crate) fn money_pow_reward_process_instruction_v1(
     }
 
     // Create a state update. We only need the new coin.
-    let update = MoneyPoWRewardUpdateV1 { coin: params.output.coin };
+    let update =
+        MoneyPoWRewardUpdateV1 { coin: params.output.coin, height: verifying_block_height };
     let mut update_data = vec![];
     update_data.write_u8(MoneyFunction::PoWRewardV1 as u8)?;
     update.encode(&mut update_data)?;
@@ -167,6 +174,11 @@ pub(crate) fn money_pow_reward_process_update_v1(
     let nullifiers_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_NULLIFIERS_TREE)?;
     let coin_roots_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_COIN_ROOTS_TREE)?;
     let nullifier_roots_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_NULLIFIER_ROOTS_TREE)?;
+    let fees_db = wasm::db::db_lookup(cid, MONEY_CONTRACT_FEES_TREE)?;
+
+    // Generate the accumulator for the next height
+    msg!("[PowRewardV1] Creating next height fees acummulator");
+    wasm::db::db_set(fees_db, &serialize(&(update.height + 1)), &serialize(&0_u64))?;
 
     // This will just make a snapshot to match the coins one
     msg!("[PowRewardV1] Updating nullifiers snapshot");

src/contract/money/src/lib.rs

@@ -75,13 +75,13 @@ pub const MONEY_CONTRACT_COIN_ROOTS_TREE: &str = "coin_roots";
 pub const MONEY_CONTRACT_NULLIFIERS_TREE: &str = "nullifiers";
 pub const MONEY_CONTRACT_NULLIFIER_ROOTS_TREE: &str = "nullifier_roots";
 pub const MONEY_CONTRACT_TOKEN_FREEZE_TREE: &str = "token_freezes";
+pub const MONEY_CONTRACT_FEES_TREE: &str = "fees";
 
 // These are keys inside the info tree
 pub const MONEY_CONTRACT_DB_VERSION: &[u8] = b"db_version";
 pub const MONEY_CONTRACT_COIN_MERKLE_TREE: &[u8] = b"coins_tree";
 pub const MONEY_CONTRACT_LATEST_COIN_ROOT: &[u8] = b"last_coins_root";
 pub const MONEY_CONTRACT_LATEST_NULLIFIER_ROOT: &[u8] = b"last_nullifiers_root";
-pub const MONEY_CONTRACT_TOTAL_FEES_PAID: &[u8] = b"total_fees_paid";
 
 /// Precalculated root hash for a tree containing only a single Fp::ZERO coin.
 /// Used to save gas.

src/contract/money/src/model/mod.rs

@@ -186,7 +186,9 @@ pub struct MoneyFeeUpdateV1 {
     pub nullifier: Nullifier,
     /// Minted coin
     pub coin: Coin,
-    /// Fee paid
+    /// Block height the fee was verified against
+    pub height: u32,
+    /// Height accumulated fee paid
     pub fee: u64,
 }
 
@@ -283,4 +285,6 @@ pub struct MoneyPoWRewardParamsV1 {
 pub struct MoneyPoWRewardUpdateV1 {
     /// The newly minted coin
     pub coin: Coin,
+    /// Block height the call was verified against
+    pub height: u32,
 }

src/contract/test-harness/src/money_pow_reward.rs

@@ -47,6 +47,7 @@ impl TestHarness {
         holder: &Holder,
         recipient: Option<&Holder>,
         reward: Option<u64>,
+        fees: Option<u64>,
     ) -> Result<(Transaction, MoneyPoWRewardParamsV1)> {
         let wallet = self.holders.get(holder).unwrap();
 
@@ -62,11 +63,15 @@ impl TestHarness {
             wallet.keypair.public
         };
 
+        // If there's fees paid, use them, otherwise set to zero
+        let fees = fees.unwrap_or_default();
+
         // Build the transaction
         let builder = PoWRewardCallBuilder {
             secret: wallet.keypair.secret,
             recipient,
             block_height: last_block.header.height + 1,
+            fees,
             spend_hook: FuncId::none(),
             user_data: pallas::Base::ZERO,
             mint_zkbin: mint_zkbin.clone(),
@@ -102,7 +107,7 @@ impl TestHarness {
     ) -> Result<Vec<OwnCoin>> {
         // Build the POW reward transaction
         info!("Building PoWReward transaction for {:?}", miner);
-        let (tx, params) = self.pow_reward(miner, None, None).await?;
+        let (tx, params) = self.pow_reward(miner, None, None, None).await?;
 
         // Fetch the last block in the blockchain
         let wallet = self.holders.get(miner).unwrap();

src/validator/consensus.rs

@@ -18,10 +18,7 @@
 
 use std::collections::{HashMap, HashSet};
 
-use darkfi_sdk::{
-    crypto::{MerkleTree, SecretKey},
-    tx::TransactionHash,
-};
+use darkfi_sdk::{crypto::MerkleTree, tx::TransactionHash};
 use darkfi_serial::{async_trait, SerialDecodable, SerialEncodable};
 use log::{debug, info};
 use num_bigint::BigUint;
@@ -31,10 +28,9 @@ use smol::lock::RwLock;
 use crate::{
     blockchain::{
         block_store::{BlockDifficulty, BlockRanks},
-        BlockInfo, Blockchain, BlockchainOverlay, BlockchainOverlayPtr, Header, HeaderHash,
+        BlockInfo, Blockchain, BlockchainOverlay, BlockchainOverlayPtr, HeaderHash,
     },
     tx::Transaction,
-    util::time::Timestamp,
     validator::{
         pow::PoWModule,
         utils::{best_fork_index, block_rank, find_extended_fork_index},
@@ -557,56 +553,6 @@ impl Fork {
         })
     }
 
-    /// Generate an unsigned block containing all pending transactions.
-    pub async fn generate_unsigned_block(
-        &self,
-        producer_tx: Transaction,
-        verify_fees: bool,
-    ) -> Result<BlockInfo> {
-        // Grab forks' last block proposal(previous)
-        let previous = self.last_proposal()?;
-
-        // Grab forks' next block height
-        let next_block_height = previous.block.header.height + 1;
-
-        // Grab forks' unproposed transactions
-        let mut unproposed_txs =
-            self.unproposed_txs(&self.blockchain, next_block_height, verify_fees).await?;
-        unproposed_txs.push(producer_tx);
-
-        // Generate the new header
-        let header =
-            Header::new(previous.block.hash(), next_block_height, Timestamp::current_time(), 0);
-
-        // Generate the block
-        let mut block = BlockInfo::new_empty(header);
-
-        // Add transactions to the block
-        block.append_txs(unproposed_txs);
-
-        Ok(block)
-    }
-
-    /// Generate a block proposal containing all pending transactions.
-    /// Proposal is signed using provided secret key, which must also
-    /// have signed the provided proposal transaction.
-    pub async fn generate_signed_proposal(
-        &self,
-        producer_tx: Transaction,
-        secret_key: &SecretKey,
-        verify_fees: bool,
-    ) -> Result<Proposal> {
-        let mut block = self.generate_unsigned_block(producer_tx, verify_fees).await?;
-
-        // Sign block
-        block.sign(secret_key);
-
-        // Generate the block proposal from the block
-        let proposal = Proposal::new(block);
-
-        Ok(proposal)
-    }
-
     /// Auxiliary function to append a proposal and update current fork rank.
     pub async fn append_proposal(&mut self, proposal: &Proposal) -> Result<()> {
         // Grab next mine target and difficulty
@@ -664,23 +610,24 @@ impl Fork {
         Ok(proposal.block.header.height + 1)
     }
 
-    /// Auxiliary function to retrieve unproposed valid transactions.
+    /// Auxiliary function to retrieve unproposed valid transactions,
+    /// along with their total paid fees.
     pub async fn unproposed_txs(
         &self,
         blockchain: &Blockchain,
         verifying_block_height: u32,
         verify_fees: bool,
-    ) -> Result<Vec<Transaction>> {
+    ) -> Result<(Vec<Transaction>, u64)> {
         // Check if our mempool is not empty
         if self.mempool.is_empty() {
-            return Ok(vec![])
+            return Ok((vec![], 0))
         }
 
         // Transactions Merkle tree
         let mut tree = MerkleTree::new(1);
 
         // Gas accumulator
-        let mut _gas_used = 0;
+        let mut gas_paid = 0;
 
         // Map of ZK proof verifying keys for the current transaction batch
         let mut vks: HashMap<[u8; 32], HashMap<String, VerifyingKey>> = HashMap::new();
@@ -720,7 +667,7 @@ impl Fork {
             )
             .await
             {
-                Ok(gas) => _gas_used += gas,
+                Ok((_, gas)) => gas_paid += gas,
                 Err(e) => {
                     debug!(target: "validator::consensus::unproposed_txs", "Transaction verification failed: {}", e);
                     overlay.lock().unwrap().revert_to_checkpoint()?;
@@ -738,7 +685,7 @@ impl Fork {
             }
         }
 
-        Ok(unproposed_txs)
+        Ok((unproposed_txs, gas_paid))
     }
 
     /// Auxiliary function to create a full clone using BlockchainOverlay::full_clone.

src/validator/mod.rs

@@ -160,7 +160,7 @@ impl Validator {
         // Purge new trees
         overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
 
-        verify_result
+        Ok(verify_result?.0)
     }
 
     /// The node retrieves a transaction, validates its state transition,

src/validator/verification.rs

@@ -519,12 +519,13 @@ pub async fn verify_transaction(
     tree: &mut MerkleTree,
     verifying_keys: &mut HashMap<[u8; 32], HashMap<String, VerifyingKey>>,
     verify_fee: bool,
-) -> Result<u64> {
+) -> Result<(u64, u64)> {
     let tx_hash = tx.hash();
     debug!(target: "validator::verification::verify_transaction", "Validating transaction {}", tx_hash);
 
     // Gas accumulator
     let mut gas_used = 0;
+    let mut gas_paid = 0;
 
     // Verify calls indexes integrity
     if verify_fee {
@@ -712,6 +713,7 @@ pub async fn verify_transaction(
             );
             return Err(TxVerifyFailed::InsufficientFee.into())
         }
+        gas_paid = fee;
     }
 
     // When we're done looping and executing over the tx's contract calls and
@@ -750,7 +752,7 @@ pub async fn verify_transaction(
     append_tx_to_merkle_tree(tree, tx);
 
     debug!(target: "validator::verification::verify_transaction", "Transaction {} verified successfully", tx_hash);
-    Ok(gas_used)
+    Ok((gas_used, gas_paid))
 }
 
 /// Apply given [`Transaction`] to the provided overlay.
@@ -862,7 +864,7 @@ pub async fn verify_transactions(
         match verify_transaction(overlay, verifying_block_height, tx, tree, &mut vks, verify_fees)
             .await
         {
-            Ok(gas) => gas_used += gas,
+            Ok((gas, _)) => gas_used += gas,
             Err(e) => {
                 warn!(target: "validator::verification::verify_transactions", "Transaction verification failed: {}", e);
                 erroneous_txs.push(tx.clone());