Merge pull request #2376 from AleoHQ/verify-in-speculate

[HackerOne-2311934] Verify transactions prior to speculation

ledger/src/advance.rs

@@ -16,10 +16,11 @@ use super::*;
 
 impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
     /// Returns a candidate for the next block in the ledger, using a committed subdag and its transmissions.
-    pub fn prepare_advance_to_next_quorum_block(
+    pub fn prepare_advance_to_next_quorum_block<R: Rng + CryptoRng>(
         &self,
         subdag: Subdag<N>,
         transmissions: IndexMap<TransmissionID<N>, Transmission<N>>,
+        rng: &mut R,
     ) -> Result<Block<N>> {
         // Retrieve the latest block as the previous block (for the next block).
         let previous_block = self.latest_block();
@@ -30,7 +31,7 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
         ensure!(ratifications.is_empty(), "Ratifications are currently unsupported from the memory pool");
         // Construct the block template.
         let (header, ratifications, solutions, aborted_solution_ids, transactions, aborted_transaction_ids) =
-            self.construct_block_template(&previous_block, Some(&subdag), ratifications, solutions, transactions)?;
+            self.construct_block_template(&previous_block, Some(&subdag), ratifications, solutions, transactions, rng)?;
 
         // Construct the new quorum block.
         Block::new_quorum(
@@ -68,6 +69,7 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
                 candidate_ratifications,
                 candidate_solutions,
                 candidate_transactions,
+                rng,
             )?;
 
         // Construct the new beacon block.
@@ -168,13 +170,14 @@ where
 impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
     /// Constructs a block template for the next block in the ledger.
     #[allow(clippy::type_complexity)]
-    fn construct_block_template(
+    fn construct_block_template<R: Rng + CryptoRng>(
         &self,
         previous_block: &Block<N>,
         subdag: Option<&Subdag<N>>,
         candidate_ratifications: Vec<Ratify<N>>,
         candidate_solutions: Vec<ProverSolution<N>>,
         candidate_transactions: Vec<Transaction<N>>,
+        rng: &mut R,
     ) -> Result<(
         Header<N>,
         Ratifications<N>,
@@ -317,6 +320,7 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
             candidate_ratifications,
             &solutions,
             candidate_transactions.iter(),
+            rng,
         )?;
 
         // Compute the ratifications root.

ledger/src/check_next_block.rs

@@ -14,8 +14,6 @@
 
 use super::*;
 
-use rand::{rngs::StdRng, SeedableRng};
-
 impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
     /// Checks the given block is valid next block.
     pub fn check_next_block<R: CryptoRng + Rng>(&self, block: &Block<N>, rng: &mut R) -> Result<()> {
@@ -38,14 +36,6 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
             }
         }
 
-        // Ensure each transaction is well-formed and unique.
-        let transactions = block.transactions();
-        let rngs = (0..transactions.len()).map(|_| StdRng::from_seed(rng.gen())).collect::<Vec<_>>();
-        cfg_iter!(transactions).zip(rngs).try_for_each(|(transaction, mut rng)| {
-            self.check_transaction_basic(transaction, transaction.to_rejected_id()?, &mut rng)
-                .map_err(|e| anyhow!("Invalid transaction found in the transactions list: {e}"))
-        })?;
-
         // TODO (howardwu): Remove this after moving the total supply into credits.aleo.
         {
             // // Retrieve the latest total supply.
@@ -78,9 +68,9 @@ impl<N: Network, C: ConsensusStorage<N>> Ledger<N, C> {
             block.previous_hash(),
         )?;
 
-        // Ensure speculation over the unconfirmed transactions is correct.
+        // Ensure speculation over the unconfirmed transactions is correct and ensure each transaction is well-formed and unique.
         let ratified_finalize_operations =
-            self.vm.check_speculate(state, block.ratifications(), block.solutions(), block.transactions())?;
+            self.vm.check_speculate(state, block.ratifications(), block.solutions(), block.transactions(), rng)?;
 
         // Retrieve the committee lookback.
         let committee_lookback = {

ledger/src/tests.rs

@@ -1251,6 +1251,105 @@ function simple_output:
     assert_eq!(block.aborted_transaction_ids(), &vec![transaction_3_id]);
 }
 
+#[test]
+fn test_abort_fee_transaction() {
+    let rng = &mut TestRng::default();
+
+    // Initialize the test environment.
+    let crate::test_helpers::TestEnv { ledger, private_key, address, .. } = crate::test_helpers::sample_test_env(rng);
+
+    // Construct valid transaction for the ledger.
+    let inputs = [Value::from_str(&format!("{address}")).unwrap(), Value::from_str("1000u64").unwrap()];
+    let transaction = ledger
+        .vm
+        .execute(&private_key, ("credits.aleo", "transfer_public"), inputs.clone().into_iter(), None, 0, None, rng)
+        .unwrap();
+    let transaction_id = transaction.id();
+
+    // Convert a fee transaction.
+    let transaction_to_convert_to_fee = ledger
+        .vm
+        .execute(&private_key, ("credits.aleo", "transfer_public"), inputs.into_iter(), None, 0, None, rng)
+        .unwrap();
+    let fee_transaction = Transaction::from_fee(transaction_to_convert_to_fee.fee_transition().unwrap()).unwrap();
+    let fee_transaction_id = fee_transaction.id();
+
+    // Create a block using a fee transaction.
+    let block = ledger
+        .prepare_advance_to_next_beacon_block(&private_key, vec![], vec![], vec![fee_transaction, transaction], rng)
+        .unwrap();
+
+    // Check that the block aborts the invalid transaction.
+    assert_eq!(block.aborted_transaction_ids(), &vec![fee_transaction_id]);
+    assert_eq!(block.transaction_ids().collect::<Vec<_>>(), vec![&transaction_id]);
+
+    // Check that the next block is valid.
+    ledger.check_next_block(&block, rng).unwrap();
+
+    // Add the block to the ledger.
+    ledger.advance_to_next_block(&block).unwrap();
+}
+
+#[test]
+fn test_abort_invalid_transaction() {
+    let rng = &mut TestRng::default();
+
+    // Initialize the test environment.
+    let crate::test_helpers::TestEnv { ledger, private_key, address, .. } = crate::test_helpers::sample_test_env(rng);
+
+    // Initialize a new VM.
+    let vm = VM::from(ConsensusStore::<CurrentNetwork, ConsensusMemory<CurrentNetwork>>::open(None).unwrap()).unwrap();
+
+    // Construct a custom genesis block.
+    let custom_genesis = vm.genesis_beacon(&private_key, rng).unwrap();
+
+    // Update the VM.
+    vm.add_next_block(&custom_genesis).unwrap();
+
+    // Generate a transaction that will be invalid on another network.
+    let inputs = [Value::from_str(&format!("{address}")).unwrap(), Value::from_str("1000u64").unwrap()];
+    let invalid_transaction = vm
+        .execute(&private_key, ("credits.aleo", "transfer_public"), inputs.clone().into_iter(), None, 0, None, rng)
+        .unwrap();
+    let invalid_transaction_id = invalid_transaction.id();
+
+    // Check that the ledger deems this transaction invalid.
+    assert!(ledger.check_transaction_basic(&invalid_transaction, None, rng).is_err());
+
+    // Construct valid transactions for the ledger.
+    let valid_transaction_1 = ledger
+        .vm
+        .execute(&private_key, ("credits.aleo", "transfer_public"), inputs.clone().into_iter(), None, 0, None, rng)
+        .unwrap();
+    let valid_transaction_2 = ledger
+        .vm
+        .execute(&private_key, ("credits.aleo", "transfer_public"), inputs.into_iter(), None, 0, None, rng)
+        .unwrap();
+    let valid_transaction_id_1 = valid_transaction_1.id();
+    let valid_transaction_id_2 = valid_transaction_2.id();
+
+    // Create a block.
+    let block = ledger
+        .prepare_advance_to_next_beacon_block(
+            &private_key,
+            vec![],
+            vec![],
+            vec![valid_transaction_1, invalid_transaction, valid_transaction_2],
+            rng,
+        )
+        .unwrap();
+
+    // Check that the block aborts the invalid transaction.
+    assert_eq!(block.aborted_transaction_ids(), &vec![invalid_transaction_id]);
+    assert_eq!(block.transaction_ids().collect::<Vec<_>>(), vec![&valid_transaction_id_1, &valid_transaction_id_2]);
+
+    // Check that the next block is valid.
+    ledger.check_next_block(&block, rng).unwrap();
+
+    // Add the block to the ledger.
+    ledger.advance_to_next_block(&block).unwrap();
+}
+
 #[test]
 fn test_deployment_duplicate_program_id() {
     let rng = &mut TestRng::default();

synthesizer/Cargo.toml

@@ -131,6 +131,9 @@ version = "1.0"
 version = "2.0"
 features = [ "serde", "rayon" ]
 
+[dependencies.itertools]
+version = "0.11.0"
+
 [dependencies.lru]
 version = "0.12"
 
@@ -153,9 +156,6 @@ version = "1.0.73"
 [dev-dependencies.criterion]
 version = "0.5"
 
-[dev-dependencies.itertools]
-version = "0.11.0"
-
 [dev-dependencies.ledger-committee]
 package = "snarkvm-ledger-committee"
 path = "../ledger/committee"

synthesizer/src/vm/finalize.rs

@@ -16,8 +16,12 @@ use super::*;
 
 use ledger_committee::{MAX_DELEGATORS, MIN_DELEGATOR_STAKE, MIN_VALIDATOR_STAKE};
 
+use rand::{rngs::StdRng, SeedableRng};
+
 impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
     /// Speculates on the given list of transactions in the VM.
+    /// This function aborts all transactions that are not are well-formed or unique.
+    ///
     ///
     /// Returns the confirmed transactions, aborted transaction IDs,
     /// and finalize operations from pre-ratify and post-ratify.
@@ -28,32 +32,74 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
     ///     `Ratify::BlockReward(block_reward)` and `Ratify::PuzzleReward(puzzle_reward)`
     ///     to the front of the `ratifications` list.
     #[inline]
-    pub fn speculate<'a>(
+    pub fn speculate<'a, R: Rng + CryptoRng>(
         &self,
         state: FinalizeGlobalState,
         coinbase_reward: Option<u64>,
         candidate_ratifications: Vec<Ratify<N>>,
         candidate_solutions: &Solutions<N>,
         candidate_transactions: impl ExactSizeIterator<Item = &'a Transaction<N>>,
+        rng: &mut R,
     ) -> Result<(Ratifications<N>, Transactions<N>, Vec<N::TransactionID>, Vec<FinalizeOperation<N>>)> {
         let timer = timer!("VM::speculate");
 
+        // Collect the candidate transactions into a vector.
+        let candidate_transactions: Vec<_> = candidate_transactions.collect::<Vec<_>>();
+        let candidate_transaction_ids: Vec<_> = candidate_transactions.iter().map(|tx| tx.id()).collect();
+
+        // If the transactions are not part of the genesis block, ensure each transaction is well-formed and unique. Abort any transactions that are not.
+        let (verified_transactions, verification_aborted_transactions) =
+            match self.block_store().find_block_height_from_state_root(self.block_store().current_state_root())? {
+                // If the current state root does not exist in the block store, then the genesis block has not been introduced yet.
+                None => (candidate_transactions, vec![]),
+                // Verify transactions for all non-genesis cases.
+                _ => {
+                    let rngs =
+                        (0..candidate_transactions.len()).map(|_| StdRng::from_seed(rng.gen())).collect::<Vec<_>>();
+                    // Verify the transactions and collect the error message if there is one.
+                    cfg_into_iter!(candidate_transactions).zip(rngs).partition_map(|(transaction, mut rng)| {
+                        // Abort the transaction if it is a fee transaction.
+                        if transaction.is_fee() {
+                            return Either::Right((
+                                transaction,
+                                "Fee transactions are not allowed in speculate".to_string(),
+                            ));
+                        }
+                        // Verify the transaction.
+                        match self.check_transaction(transaction, None, &mut rng) {
+                            Ok(_) => Either::Left(transaction),
+                            Err(e) => Either::Right((transaction, e.to_string())),
+                        }
+                    })
+                }
+            };
+
         // Performs a **dry-run** over the list of ratifications, solutions, and transactions.
-        let (ratifications, confirmed_transactions, aborted_transactions, ratified_finalize_operations) = self
-            .atomic_speculate(
+        let (ratifications, confirmed_transactions, speculation_aborted_transactions, ratified_finalize_operations) =
+            self.atomic_speculate(
                 state,
                 coinbase_reward,
                 candidate_ratifications,
                 candidate_solutions,
-                candidate_transactions,
+                verified_transactions.into_iter(),
             )?;
 
-        // Convert the aborted transactions into aborted transaction IDs.
-        let mut aborted_transaction_ids = Vec::with_capacity(aborted_transactions.len());
-        for (tx, error) in aborted_transactions {
-            warn!("Speculation safely aborted a transaction - {error} ({})", tx.id());
-            aborted_transaction_ids.push(tx.id());
-        }
+        // Get the aborted transaction ids.
+        let verification_aborted_transaction_ids = verification_aborted_transactions.iter().map(|(tx, e)| (tx.id(), e));
+        let speculation_aborted_transaction_ids = speculation_aborted_transactions.iter().map(|(tx, e)| (tx.id(), e));
+        let unordered_aborted_transaction_ids: IndexMap<N::TransactionID, &String> =
+            verification_aborted_transaction_ids.chain(speculation_aborted_transaction_ids).collect();
+
+        // Filter and order the aborted transaction ids according to candidate_transactions
+        let aborted_transaction_ids: Vec<_> = candidate_transaction_ids
+            .into_iter()
+            .filter_map(|tx_id| {
+                unordered_aborted_transaction_ids.get(&tx_id).map(|error| {
+                    warn!("Speculation safely aborted a transaction - {error} ({tx_id})");
+                    tx_id
+                })
+            })
+            .collect();
 
         finish!(timer, "Finished dry-run of the transactions");
 
@@ -67,18 +113,30 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
     }
 
     /// Checks the speculation on the given transactions in the VM.
+    /// This function also ensure that the given transactions are well-formed and unique.
     ///
     /// Returns the finalize operations from pre-ratify and post-ratify.
     #[inline]
-    pub fn check_speculate(
+    pub fn check_speculate<R: Rng + CryptoRng>(
         &self,
         state: FinalizeGlobalState,
         ratifications: &Ratifications<N>,
         solutions: &Solutions<N>,
         transactions: &Transactions<N>,
+        rng: &mut R,
     ) -> Result<Vec<FinalizeOperation<N>>> {
         let timer = timer!("VM::check_speculate");
 
+        // Ensure each transaction is well-formed and unique.
+        // NOTE: We perform the transaction checks here prior to `atomic_speculate` because we must
+        // ensure that the `Fee` transactions are valid. We can't unify the transaction checks in `atomic_speculate`
+        // because we run speculation on the unconfirmed variant of the transactions.
+        let rngs = (0..transactions.len()).map(|_| StdRng::from_seed(rng.gen())).collect::<Vec<_>>();
+        cfg_iter!(transactions).zip(rngs).try_for_each(|(transaction, mut rng)| {
+            self.check_transaction(transaction, transaction.to_rejected_id()?, &mut rng)
+                .map_err(|e| anyhow!("Invalid transaction found in the transactions list: {e}"))
+        })?;
+
         // Reconstruct the candidate ratifications to verify the speculation.
         let candidate_ratifications = ratifications.iter().cloned().collect::<Vec<_>>();
         // Reconstruct the unconfirmed transactions to verify the speculation.
@@ -1135,6 +1193,7 @@ finalize transfer_public:
             vec![],
             &None.into(),
             transactions.iter(),
+            rng,
         )?;
 
         // Construct the metadata associated with the block.
@@ -1399,7 +1458,14 @@ finalize transfer_public:
 
         // Prepare the confirmed transactions.
         let (ratifications, confirmed_transactions, aborted_transaction_ids, _) = vm
-            .speculate(sample_finalize_state(1), None, vec![], &None.into(), [deployment_transaction.clone()].iter())
+            .speculate(
+                sample_finalize_state(1),
+                None,
+                vec![],
+                &None.into(),
+                [deployment_transaction.clone()].iter(),
+                rng,
+            )
             .unwrap();
         assert_eq!(confirmed_transactions.len(), 1);
         assert!(aborted_transaction_ids.is_empty());
@@ -1687,7 +1753,7 @@ function ped_hash:
 
             // Speculatively execute the transaction. Ensure that this call does not panic and returns a rejected transaction.
             let (_, confirmed_transactions, aborted_transaction_ids, _) = vm
-                .speculate(sample_finalize_state(1), None, vec![], &None.into(), [transaction.clone()].iter())
+                .speculate(sample_finalize_state(1), None, vec![], &None.into(), [transaction.clone()].iter(), rng)
                 .unwrap();
             assert!(aborted_transaction_ids.is_empty());