Merge pull request #2113 from AleoHQ/fix/fee

Fixes fee computation for block reward, adds missing fee check

ledger/benches/transaction.rs

@@ -80,7 +80,7 @@ function hello:
 
     c.bench_function("Transaction::Deploy - verify", |b| {
         let transaction = vm.deploy(&private_key, &program, Some(records[0].clone()), 600000, None, rng).unwrap();
-        b.iter(|| assert!(vm.verify_transaction(&transaction, None)))
+        b.iter(|| vm.check_transaction(&transaction, None).unwrap())
     });
 }
 

ledger/block/src/helpers/target.rs

@@ -17,20 +17,27 @@ use console::prelude::{ensure, Result};
 /// A safety bound (sanity-check) for the coinbase reward.
 pub const MAX_COINBASE_REWARD: u64 = 190_258_739; // Coinbase reward at block 1.
 
-/// Calculate the block reward, given the total supply, block time, and coinbase reward.
-///     R_staking = floor((0.05 * S) / H_Y1) + CR / 2
+/// Calculate the block reward, given the total supply, block time, coinbase reward, and transaction fees.
+///     R_staking = floor((0.05 * S) / H_Y1) + CR / 2 + TX_F.
 ///     S = Total supply.
 ///     H_Y1 = Expected block height at year 1.
 ///     CR = Coinbase reward.
-pub const fn block_reward(total_supply: u64, block_time: u16, coinbase_reward: u64) -> u64 {
+///     TX_F = Transaction fees.
+pub const fn block_reward(total_supply: u64, block_time: u16, coinbase_reward: u64, transaction_fees: u64) -> u64 {
     // Compute the expected block height at year 1.
     let block_height_at_year_1 = block_height_at_year(block_time, 1);
     // Compute the annual reward: (0.05 * S).
     let annual_reward = (total_supply / 1000) * 50;
     // Compute the block reward: (0.05 * S) / H_Y1.
     let block_reward = annual_reward / block_height_at_year_1 as u64;
-    // Return the sum of the block and coinbase rewards.
-    block_reward + coinbase_reward / 2
+    // Return the sum of the block reward, coinbase reward, and transaction fees.
+    block_reward + (coinbase_reward / 2) + transaction_fees
+}
+
+/// Calculate the puzzle reward, given the coinbase reward.
+pub const fn puzzle_reward(coinbase_reward: u64) -> u64 {
+    // Return the coinbase reward divided by 2.
+    coinbase_reward / 2
 }
 
 /// Calculates the coinbase reward for a given block.
@@ -270,15 +277,15 @@ mod tests {
 
     #[test]
     fn test_block_reward() {
-        let reward = block_reward(CurrentNetwork::STARTING_SUPPLY, CurrentNetwork::BLOCK_TIME, 0);
+        let reward = block_reward(CurrentNetwork::STARTING_SUPPLY, CurrentNetwork::BLOCK_TIME, 0, 0);
         assert_eq!(reward, EXPECTED_STAKING_REWARD);
 
         // Increasing the anchor time will increase the reward.
-        let larger_reward = block_reward(CurrentNetwork::STARTING_SUPPLY, CurrentNetwork::BLOCK_TIME + 1, 0);
+        let larger_reward = block_reward(CurrentNetwork::STARTING_SUPPLY, CurrentNetwork::BLOCK_TIME + 1, 0, 0);
         assert!(reward < larger_reward);
 
         // Decreasing the anchor time will decrease the reward.
-        let smaller_reward = block_reward(CurrentNetwork::STARTING_SUPPLY, CurrentNetwork::BLOCK_TIME - 1, 0);
+        let smaller_reward = block_reward(CurrentNetwork::STARTING_SUPPLY, CurrentNetwork::BLOCK_TIME - 1, 0, 0);
         assert!(reward > smaller_reward);
     }
 

ledger/block/src/transaction/fee/mod.rs

@@ -90,10 +90,10 @@ impl<N: Network> Fee<N> {
     pub fn amount(&self) -> Result<U64<N>> {
         // Retrieve the base fee amount.
         let base_fee_amount = self.base_amount()?;
-
         // Retrieve the priority fee amount.
         let priority_fee_amount = self.priority_amount()?;
-
+        // Return the amount.
+        // Note: Use of saturating add is safe, as the sum cannot exceed a u64.
         Ok(U64::new(base_fee_amount.saturating_add(*priority_fee_amount)))
     }
 
@@ -109,7 +109,6 @@ impl<N: Network> Fee<N> {
             },
             None => bail!("Missing output in fee transition"),
         };
-
         // Retrieve the base fee (in microcredits) as a plaintext value.
         match self.transition.inputs().get(base_fee_index) {
             Some(Input::Public(_, Some(Plaintext::Literal(Literal::U64(microcredits), _)))) => Ok(*microcredits),
@@ -129,7 +128,6 @@ impl<N: Network> Fee<N> {
             },
             None => bail!("Missing output in fee transition"),
         };
-
         // Retrieve the priority fee (in microcredits) as a plaintext value.
         match self.transition.inputs().get(priority_fee_index) {
             Some(Input::Public(_, Some(Plaintext::Literal(Literal::U64(microcredits), _)))) => Ok(*microcredits),
@@ -236,10 +234,10 @@ pub mod test_helpers {
         let credits = transaction.records().next().unwrap().1.clone();
         // Decrypt the record.
         let credits = credits.decrypt(&private_key.try_into().unwrap()).unwrap();
-        // Set the base fee amount.
-        let base_fee = 10_000_000;
-        // Set the priority fee amount.
-        let priority_fee = 1_000;
+        // Sample a base fee in microcredits.
+        let base_fee_in_microcredits = 10_000_000;
+        // Sample a priority fee in microcredits.
+        let priority_fee_in_microcredits = 1_000;
 
         // Initialize the process.
         let process = Process::load().unwrap();
@@ -248,8 +246,8 @@ pub mod test_helpers {
             .authorize_fee_private::<CurrentAleo, _>(
                 &private_key,
                 credits,
-                base_fee,
-                priority_fee,
+                base_fee_in_microcredits,
+                priority_fee_in_microcredits,
                 deployment_or_execution_id,
                 rng,
             )

ledger/block/src/transactions/confirmed/mod.rs

@@ -265,6 +265,15 @@ impl<N: Network> ConfirmedTransaction<N> {
         }
     }
 
+    /// Returns the rejected object, if the confirmed transaction is rejected.
+    pub fn to_rejected(&self) -> Option<&Rejected<N>> {
+        match self {
+            ConfirmedTransaction::AcceptedDeploy(..) | ConfirmedTransaction::AcceptedExecute(..) => None,
+            ConfirmedTransaction::RejectedDeploy(_, _, rejected, _) => Some(rejected),
+            ConfirmedTransaction::RejectedExecute(_, _, rejected, _) => Some(rejected),
+        }
+    }
+
     /// Returns the unconfirmed transaction ID, which is defined as the transaction ID prior to confirmation.
     /// When a transaction is rejected, its fee transition is used to construct the confirmed transaction ID,
     /// changing the original transaction ID.

ledger/block/src/verify.rs

@@ -51,19 +51,10 @@ impl<N: Network> Block<N> {
             expected_proof_target,
             expected_last_coinbase_target,
             expected_last_coinbase_timestamp,
-            mut expected_block_reward,
+            expected_block_reward,
             expected_puzzle_reward,
         ) = self.verify_solutions(previous_block, current_puzzle, current_epoch_challenge)?;
 
-        // Add the priority fees to the block reward.
-        for confirmed_transacation in self.transactions.iter() {
-            // Get the priority fee amount for the transaction.
-            let priority_fee_amount = confirmed_transacation.transaction().priority_fee_amount()?;
-
-            // Add the priority fee to the block reward.
-            expected_block_reward = expected_block_reward.saturating_add(*priority_fee_amount);
-        }
-
         // Ensure the block ratifications are correct.
         self.verify_ratifications(expected_block_reward, expected_puzzle_reward)?;
 
@@ -366,10 +357,16 @@ impl<N: Network> Block<N> {
             u64::try_from(previous_block.cumulative_proof_target())?,
             previous_block.coinbase_target(),
         )?;
+
+        // Calculate the expected transaction fees.
+        let expected_transaction_fees =
+            self.transactions.iter().map(|tx| Ok(*tx.priority_fee_amount()?)).sum::<Result<u64>>()?;
+
         // Compute the expected block reward.
-        let expected_block_reward = block_reward(N::STARTING_SUPPLY, N::BLOCK_TIME, expected_coinbase_reward);
+        let expected_block_reward =
+            block_reward(N::STARTING_SUPPLY, N::BLOCK_TIME, expected_coinbase_reward, expected_transaction_fees);
         // Compute the expected puzzle reward.
-        let expected_puzzle_reward = expected_coinbase_reward.saturating_div(2);
+        let expected_puzzle_reward = puzzle_reward(expected_coinbase_reward);
 
         Ok((
             expected_cumulative_weight,

ledger/src/advance.rs

@@ -187,7 +187,7 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
         };
 
         // Calculate the coinbase reward.
-        let coinbase_reward = ledger_block::coinbase_reward(
+        let coinbase_reward = coinbase_reward(
             next_height,
             N::STARTING_SUPPLY,
             N::ANCHOR_HEIGHT,
@@ -197,34 +197,6 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
             latest_coinbase_target,
         )?;
 
-        // Compute the block reward.
-        let mut block_reward = ledger_block::block_reward(N::STARTING_SUPPLY, N::BLOCK_TIME, coinbase_reward);
-        // Compute the puzzle reward.
-        let puzzle_reward = coinbase_reward.saturating_div(2);
-
-        // Add the priority fees to the block reward.
-        for transaction in candidate_transactions.iter() {
-            // Get the priority fee for the transaction.
-            let priority_fee_amount = transaction.priority_fee_amount()?;
-            // Add the priority fee to the block reward.
-            block_reward = block_reward.saturating_add(*priority_fee_amount);
-        }
-
-        // TODO (howardwu): We must first process the candidate ratifications to filter out invalid ratifications.
-        //  We must ensure Ratify::Genesis is only present in the genesis block.
-        // Construct the ratifications.
-        // Attention: Do not change the order of the ratifications.
-        let candidate_ratifications = [Ratify::BlockReward(block_reward), Ratify::PuzzleReward(puzzle_reward)]
-                .into_iter()
-                // Lastly, we must append the candidate ratifications.
-                .chain(candidate_ratifications.into_iter()).collect::<Vec<_>>();
-
-        // Construct the subdag root.
-        let subdag_root = match subdag {
-            Some(subdag) => subdag.to_subdag_root()?,
-            None => Field::zero(),
-        };
-
         // Construct the finalize state.
         let state = FinalizeGlobalState::new::<N>(
             next_round,
@@ -234,12 +206,23 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
             previous_block.hash(),
         )?;
         // Speculate over the ratifications, solutions, and transactions.
-        let (ratifications, transactions, aborted_transaction_ids, ratified_finalize_operations) =
-            self.vm.speculate(state, &candidate_ratifications, solutions.as_ref(), candidate_transactions.iter())?;
+        let (ratifications, transactions, aborted_transaction_ids, ratified_finalize_operations) = self.vm.speculate(
+            state,
+            Some(coinbase_reward),
+            candidate_ratifications,
+            solutions.as_ref(),
+            candidate_transactions.iter(),
+        )?;
 
         // Compute the ratifications root.
         let ratifications_root = ratifications.to_ratifications_root()?;
 
+        // Construct the subdag root.
+        let subdag_root = match subdag {
+            Some(subdag) => subdag.to_subdag_root()?,
+            None => Field::zero(),
+        };
+
         // Construct the metadata.
         let metadata = Metadata::new(
             N::ID,

ledger/src/check_next_block.rs

@@ -78,32 +78,9 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
             block.previous_hash(),
         )?;
 
-        // Reconstruct the candidate ratifications to verify the speculation.
-        let candidate_ratifications = block.ratifications().iter().cloned().collect::<Vec<_>>();
-
-        // Reconstruct the unconfirmed transactions to verify the speculation.
-        let unconfirmed_transactions = block
-            .transactions()
-            .iter()
-            .map(|confirmed| confirmed.to_unconfirmed_transaction())
-            .collect::<Result<Vec<_>>>()?;
-
-        // Speculate over the unconfirmed transactions.
-        let (ratifications, confirmed_transactions, aborted_transaction_ids, ratified_finalize_operations) =
-            self.vm.speculate(state, &candidate_ratifications, block.solutions(), unconfirmed_transactions.iter())?;
-        // Ensure there are no aborted transaction IDs from this speculation.
-        // Note: There should be no aborted transactions, because we are checking a block,
-        // where any aborted transactions should be in the aborted transaction ID list, not in transactions.
-        ensure!(aborted_transaction_ids.is_empty(), "Aborted transactions found in the block (from speculation)");
-
-        // Ensure the ratifications after speculation match.
-        if block.ratifications() != &ratifications {
-            bail!("The ratifications after speculation do not match the ratifications in the block");
-        }
-        // Ensure the transactions after speculation match.
-        if block.transactions() != &confirmed_transactions {
-            bail!("The transactions after speculation do not match the transactions in the block");
-        }
+        // Ensure speculation over the unconfirmed transactions is correct.
+        let ratified_finalize_operations =
+            self.vm.check_speculate(state, block.ratifications(), block.solutions(), block.transactions())?;
 
         // Ensure the block is correct.
         block.verify(

ledger/src/check_transaction_basic.rs

@@ -17,39 +17,6 @@ use super::*;
 impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
     /// Checks the given transaction is well-formed and unique.
     pub fn check_transaction_basic(&self, transaction: &Transaction<N>, rejected_id: Option<Field<N>>) -> Result<()> {
-        let transaction_id = transaction.id();
-
-        /* Fee */
-
-        // If the transaction contains only 1 transition, and the transition is a split, then the fee can be skipped.
-        let is_fee_required = match transaction.execution() {
-            Some(execution) => !(execution.len() == 1 && transaction.contains_split()),
-            None => true,
-        };
-
-        if is_fee_required {
-            // Retrieve the transaction base fee.
-            let base_fee_amount = *transaction.base_fee_amount()?;
-            // Retrieve the minimum cost of the transaction.
-            let (cost, _) = match transaction {
-                // Compute the deployment cost.
-                Transaction::Deploy(_, _, deployment, _) => synthesizer::deployment_cost(deployment)?,
-                // Compute the execution cost.
-                Transaction::Execute(_, execution, _) => synthesizer::execution_cost(self.vm(), execution)?,
-                // TODO (howardwu): Plug in the Rejected struct, to compute the cost.
-                Transaction::Fee(_, _) => (0, (0, 0)),
-            };
-            // Ensure the transaction has a sufficient fee.
-            if cost > base_fee_amount {
-                bail!("Transaction '{transaction_id}' has an insufficient fee - expected at least {cost} microcredits")
-            }
-        }
-
-        /* Transaction */
-
-        // Ensure the transaction is valid.
-        self.vm().check_transaction(transaction, rejected_id)?;
-
-        Ok(())
+        self.vm().check_transaction(transaction, rejected_id)
     }
 }

ledger/src/tests.rs

@@ -233,7 +233,7 @@ finalize foo:
     // Deploy.
     let transaction = ledger.vm.deploy(&private_key, &program, credits, 0, None, rng).unwrap();
     // Verify.
-    assert!(ledger.vm().verify_transaction(&transaction, None));
+    ledger.vm().check_transaction(&transaction, None).unwrap();
 
     // Construct the next block.
     let block =
@@ -287,7 +287,7 @@ finalize foo:
     let transaction =
         ledger.vm.execute(&private_key, ("dummy.aleo", "foo"), inputs, Some(sufficient_record), 0, None, rng).unwrap();
     // Verify.
-    assert!(ledger.vm.verify_transaction(&transaction, None));
+    ledger.vm.check_transaction(&transaction, None).unwrap();
     // Ensure that the ledger deems the transaction valid.
     assert!(ledger.check_transaction_basic(&transaction, None).is_ok());
 }
@@ -419,7 +419,7 @@ finalize foo:
     // Deploy.
     let transaction = ledger.vm.deploy(&private_key, &program, None, 0, None, rng).unwrap();
     // Verify.
-    assert!(ledger.vm().verify_transaction(&transaction, None));
+    ledger.vm().check_transaction(&transaction, None).unwrap();
 
     // Construct the next block.
     let block =

ledger/test-helpers/src/lib.rs

@@ -191,10 +191,10 @@ pub fn sample_fee_private(deployment_or_execution_id: Field<CurrentNetwork>, rng
     let credits = transaction.records().next().unwrap().1.clone();
     // Decrypt the record.
     let credits = credits.decrypt(&private_key.try_into().unwrap()).unwrap();
-    // Set the base fee amount.
-    let base_fee = 10_000_000;
-    // Set the priority fee amount.
-    let priority_fee = 1_000;
+    // Sample a base fee in microcredits.
+    let base_fee_in_microcredits = 10_000_000;
+    // Sample a priority fee in microcredits.
+    let priority_fee_in_microcredits = 1_000;
 
     // Initialize the process.
     let process = Process::load().unwrap();
@@ -203,8 +203,8 @@ pub fn sample_fee_private(deployment_or_execution_id: Field<CurrentNetwork>, rng
         .authorize_fee_private::<CurrentAleo, _>(
             &private_key,
             credits,
-            base_fee,
-            priority_fee,
+            base_fee_in_microcredits,
+            priority_fee_in_microcredits,
             deployment_or_execution_id,
             rng,
         )
@@ -245,16 +245,22 @@ pub fn sample_fee_public_hardcoded(rng: &mut TestRng) -> Fee<CurrentNetwork> {
 pub fn sample_fee_public(deployment_or_execution_id: Field<CurrentNetwork>, rng: &mut TestRng) -> Fee<CurrentNetwork> {
     // Sample the genesis block, transaction, and private key.
     let (block, _, private_key) = crate::sample_genesis_block_and_components(rng);
-    // Set the base fee amount.
-    let base_fee = 10_000_000;
-    // Set the priority fee amount.
-    let priority_fee = 1_000;
+    // Sample a base fee in microcredits.
+    let base_fee_in_microcredits = 10_000_000;
+    // Sample a priority fee in microcredits.
+    let priority_fee_in_microcredits = 1_000;
 
     // Initialize the process.
     let process = Process::load().unwrap();
     // Authorize the fee.
     let authorization = process
-        .authorize_fee_public::<CurrentAleo, _>(&private_key, base_fee, priority_fee, deployment_or_execution_id, rng)
+        .authorize_fee_public::<CurrentAleo, _>(
+            &private_key,
+            base_fee_in_microcredits,
+            priority_fee_in_microcredits,
+            deployment_or_execution_id,
+            rng,
+        )
         .unwrap();
     // Construct the fee trace.
     let (_, mut trace) = process.execute::<CurrentAleo>(authorization).unwrap();