Revert "skip version checking on get txn by hash"

This reverts commit 1c65af5.

api/src/context.rs

@@ -863,9 +863,10 @@ impl Context {
     pub fn get_transaction_by_hash(
         &self,
         hash: HashValue,
+        ledger_version: u64,
     ) -> Result<Option<TransactionOnChainData>> {
         self.db
-            .get_transaction_by_hash(hash, true)?
+            .get_transaction_by_hash(hash, ledger_version, true)?
             .map(|t| self.convert_into_transaction_on_chain_data(t))
             .transpose()
     }

api/src/transactions.rs

@@ -796,7 +796,7 @@ impl TransactionsApi {
             let ledger_info = api_spawn_blocking(move || context.get_latest_ledger_info()).await?;
 
             let txn_data = self
-                .get_by_hash(hash.into())
+                .get_by_hash(hash.into(), &ledger_info)
                 .await
                 .context(format!("Failed to get transaction by hash {}", hash))
                 .map_err(|err| {
@@ -832,11 +832,10 @@ impl TransactionsApi {
         let context = self.context.clone();
         let accept_type = accept_type.clone();
 
-        let ledger_info =
-            api_spawn_blocking(move || context.get_latest_storage_ledger_info()).await?;
+        let ledger_info = api_spawn_blocking(move || context.get_latest_ledger_info()).await?;
 
         let txn_data = self
-            .get_by_hash(hash.into())
+            .get_by_hash(hash.into(), &ledger_info)
             .await
             .context(format!("Failed to get transaction by hash {}", hash))
             .map_err(|err| {
@@ -960,12 +959,15 @@ impl TransactionsApi {
     async fn get_by_hash(
         &self,
         hash: aptos_crypto::HashValue,
+        ledger_info: &LedgerInfo,
     ) -> anyhow::Result<Option<TransactionData>> {
         let context = self.context.clone();
-        let from_db = tokio::task::spawn_blocking(move || context.get_transaction_by_hash(hash))
-            .await
-            .context("Failed to join task to read transaction by hash")?
-            .context("Failed to read transaction by hash from DB")?;
+        let version = ledger_info.version();
+        let from_db =
+            tokio::task::spawn_blocking(move || context.get_transaction_by_hash(hash, version))
+                .await
+                .context("Failed to join task to read transaction by hash")?
+                .context("Failed to read transaction by hash from DB")?;
         Ok(match from_db {
             None => self
                 .context

peer-monitoring-service/server/src/tests.rs

@@ -638,6 +638,7 @@ mod database_mock {
             fn get_transaction_by_hash(
                 &self,
                 hash: HashValue,
+                ledger_version: Version,
                 fetch_events: bool,
             ) -> Result<Option<TransactionWithProof>>;
 

state-sync/state-sync-driver/src/tests/mocks.rs

@@ -197,6 +197,7 @@ mock! {
         fn get_transaction_by_hash(
             &self,
             hash: HashValue,
+            ledger_version: Version,
             fetch_events: bool,
         ) -> Result<Option<TransactionWithProof>>;
 

state-sync/storage-service/server/src/tests/mock.rs

@@ -245,6 +245,7 @@ mock! {
         fn get_transaction_by_hash(
             &self,
             hash: HashValue,
+            ledger_version: Version,
             fetch_events: bool,
         ) -> aptos_storage_interface::Result<Option<TransactionWithProof>>;
 

storage/aptosdb/src/db/include/aptosdb_reader.rs

@@ -125,13 +125,14 @@ impl DbReader for AptosDB {
     fn get_transaction_by_hash(
         &self,
         hash: HashValue,
+        ledger_version: Version,
         fetch_events: bool,
     ) -> Result<Option<TransactionWithProof>> {
         gauged_api("get_transaction_by_hash", || {
             self.ledger_db
                 .transaction_db()
-                .get_transaction_version_by_hash(&hash)?
-                .map(|v| self.get_transaction_with_proof(v, v, fetch_events))
+                .get_transaction_version_by_hash(&hash, ledger_version)?
+                .map(|v| self.get_transaction_with_proof(v, ledger_version, fetch_events))
                 .transpose()
         })
     }

storage/aptosdb/src/db/test_helper.rs

@@ -832,7 +832,7 @@ pub fn verify_committed_transactions(
                 .try_as_signed_user_txn()
                 .unwrap();
             let txn_with_proof = db
-                .get_transaction_by_hash(txn_to_commit.transaction().hash(), true)
+                .get_transaction_by_hash(txn_to_commit.transaction().hash(), ledger_version, true)
                 .unwrap()
                 .unwrap();
             assert_eq!(

storage/aptosdb/src/ledger_db/transaction_db.rs

@@ -69,8 +69,12 @@ impl TransactionDb {
     pub(crate) fn get_transaction_version_by_hash(
         &self,
         hash: &HashValue,
+        ledger_version: Version,
     ) -> Result<Option<Version>> {
-        self.db.get::<TransactionByHashSchema>(hash)
+        Ok(match self.db.get::<TransactionByHashSchema>(hash)? {
+            Some(version) if version <= ledger_version => Some(version),
+            _ => None,
+        })
     }
 
     pub(crate) fn commit_transactions(

storage/aptosdb/src/ledger_db/transaction_db_test.rs

@@ -33,9 +33,9 @@ proptest! {
         for (version, txn) in txns.into_iter().enumerate() {
             let hash = txn.hash();
             prop_assert_eq!(transaction_db.get_transaction(version as Version).unwrap(), txn);
-            prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&hash).unwrap(), Some(version as Version));
+            prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&hash, num_txns as Version).unwrap(), Some(version as Version));
             if version > 0 {
-                prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&hash).unwrap(), None);
+                prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&hash, version as Version - 1).unwrap(), None);
             }
         }
 
@@ -108,6 +108,7 @@ proptest! {
         let db = AptosDB::new_for_test(&tmp_dir);
         let transaction_db  = db.ledger_db.transaction_db();
         let txns = init_db(universe, gens, transaction_db);
+        let num_txns = txns.len();
 
         {
             prop_assert!(transaction_db.get_transaction(0).is_ok());
@@ -119,12 +120,12 @@ proptest! {
 
         {
             prop_assert!(transaction_db.get_transaction(1).is_ok());
-            prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&txns[1].hash()).unwrap(), Some(1));
+            prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&txns[1].hash(), num_txns as Version).unwrap(), Some(1));
             let batch = SchemaBatch::new();
             transaction_db.prune_transaction_by_hash_indices(&[txns[1].clone()], &batch).unwrap();
             transaction_db.write_schemas(batch).unwrap();
             prop_assert!(transaction_db.get_transaction(1).is_ok());
-            prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&txns[1].hash()).unwrap(), None);
+            prop_assert_eq!(transaction_db.get_transaction_version_by_hash(&txns[1].hash(), num_txns as Version).unwrap(), None);
         }
     }
 }

storage/storage-interface/src/lib.rs

@@ -148,6 +148,7 @@ pub trait DbReader: Send + Sync {
         fn get_transaction_by_hash(
             &self,
             hash: HashValue,
+            ledger_version: Version,
             fetch_events: bool,
         ) -> Result<Option<TransactionWithProof>>;