Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "use storage ledger version for get txn by hash" #14713

Merged
merged 2 commits into from
Sep 21, 2024
Merged

Revert "use storage ledger version for get txn by hash" #14713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4577693
Revert "use storage ledgerinfo for txn APIs"
areshand Sep 20, 2024
a050fb0
Revert "skip version checking on get txn by hash"
areshand Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions api/src/transactions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,8 +793,7 @@ 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(), &ledger_info)
Expand Down Expand Up @@ -833,8 +832,7 @@ 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(), &ledger_info)
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/db/include/aptosdb_reader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl DbReader for AptosDB {
gauged_api("get_transaction_by_hash", || {
self.ledger_db
.transaction_db()
.get_transaction_version_by_hash(&hash)?
.get_transaction_version_by_hash(&hash, ledger_version)?
.map(|v| self.get_transaction_with_proof(v, ledger_version, fetch_events))
.transpose()
})
Expand Down
6 changes: 5 additions & 1 deletion storage/aptosdb/src/ledger_db/transaction_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 7 additions & 3 deletions storage/aptosdb/src/ledger_db/transaction_db_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ 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, version as Version - 1).unwrap(), None);
}
}

prop_assert!(transaction_db.get_transaction(num_txns as Version).is_err());
Expand Down Expand Up @@ -105,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());
Expand All @@ -116,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);
}
}
}
Expand Down
Loading