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.

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

Add get_confirmed_transactions to storage-bigtable #25404

Merged
merged 4 commits into from
Jul 5, 2022

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented May 20, 2022

Problem

It's possible to get few blocks with get_confirmed_blocks_with_data but there is no method for get few transactions.

Summary of Changes

Add LedgerStorage::get_confirmed_transactions to solana-storage-bigtable.

@mergify mergify bot added the community Community contribution label May 20, 2022
@mergify mergify bot requested a review from a team May 20, 2022 13:01
@stale
Copy link

stale bot commented Jun 12, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 12, 2022
storage-bigtable/src/lib.rs Outdated Show resolved Hide resolved
storage-bigtable/src/lib.rs Outdated Show resolved Hide resolved
@stale stale bot removed stale [bot only] Added to stale content; results in auto-close after a week. labels Jun 15, 2022
@fanatid
Copy link
Contributor Author

fanatid commented Jun 23, 2022

I changed code so now transactions will be returned in the order in which they were requested

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm much happier with how these various iterators ended up. Thanks for all the polish!

.transactions
.get(index as usize)
.and_then(|tx_with_meta| {
if tx_with_meta.transaction_signature().to_string() != *signature {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: i's a pity to have to do this additional stringification, when we already have the requested Signatures. I'm not going to hold this PR up any further on this, but do you have any clever ideas?

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Jul 5, 2022
@mergify mergify bot merged commit 90d9118 into solana-labs:master Jul 5, 2022
@fanatid fanatid deleted the bt-get-transactions branch July 6, 2022 04:26
fanatid added a commit to fanatid/solana that referenced this pull request Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants