Add getTransactions endpoint
#136
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aditya1702
changed the title
getTransactions endpoint
Shaptic
reviewed
Apr 16, 2024
Shaptic
reviewed
Apr 17, 2024
Shaptic
reviewed
Apr 17, 2024
Shaptic
reviewed
Apr 17, 2024
# Conflicts: # go.mod # go.sum
Shaptic
reviewed
May 8, 2024
Shaptic
approved these changes
May 9, 2024
| LedgerLoop: | ||
| for ledgerSeq := start.LedgerSequence; ledgerSeq <= int32(ledgerRange.LastLedger.Sequence); ledgerSeq++ { | ||
| // Get ledger close meta from db | ||
| ledger, found, err := h.ledgerReader.GetLedger(ctx, uint32(ledgerSeq)) |
There was a problem hiding this comment.
This is a thought for the future: we should probably do something more like getLedgers() and then process ledgers in batches so that we don't have to have one SQL query for each iteration (while still respecting limit, of course, but a little extra memory is worth saving queries).
We can include this as part of (or after, at the very least) the getLedgers work in #111.
There was a problem hiding this comment.
@Shaptic Yeah I agree. We should do the batch processing as its more efficient.
tamirms
reviewed
May 9, 2024
tamirms
reviewed
May 9, 2024
tamirms
reviewed
May 9, 2024
tamirms
reviewed
May 9, 2024
tamirms
reviewed
May 10, 2024
Shaptic
reviewed
May 10, 2024
| @@ -135,7 +144,7 @@ LedgerLoop: | |||
| err = errors.Errorf("ledger close meta not found: %d", ledgerSeq) | |||
| } | |||
| return GetTransactionsResponse{}, &jrpc2.Error{ | |||
| Code: jrpc2.InvalidParams, | |||
| Code: jrpc2.InternalError, | |||
There was a problem hiding this comment.
why internal now? if it's not in the DB then their request is out of bounds
There was a problem hiding this comment.
same comment for all of the below error changes imo
There was a problem hiding this comment.
@Shaptic Yeah you are correct. I refactored this code again to use InvalidParams for "ledger close meta not found" or for any other db related error, use the InternalError code.
However, for these 3 errors:
- https://github.com/stellar/soroban-rpc/pull/136/files#diff-5276db8001730f32b267cab637b316958b69aef6f8184343338855fdc118b700R158
- https://github.com/stellar/soroban-rpc/pull/136/files#diff-5276db8001730f32b267cab637b316958b69aef6f8184343338855fdc118b700R169
- https://github.com/stellar/soroban-rpc/pull/136/files#diff-5276db8001730f32b267cab637b316958b69aef6f8184343338855fdc118b700R169
InternalError would make more sense dont you think? Since they are generated due to some internal parsing issues?
tamirms
reviewed
May 13, 2024
tamirms
reviewed
May 13, 2024
tamirms
approved these changes
May 13, 2024
tamirms
reviewed
May 13, 2024
aditya1702
added a commit
that referenced
this pull request
May 23, 2024
…ons (#174) * Increase `getTransaction` window via ingestion into a local database (#141) * Add common interface between in-memory and on-disk transaction storage * Revert "Add common interface between in-memory and on-disk transaction storage" This reverts commit 3196ade. * Add initial implementation of DB-backed tx store * Drop all references to the in-memory transaction store * Actually filter for the tx hash you want 🤦 * Drop remaining reference to transaction store * Update stellar/go to latest master * Add a bunch of logging/verbose errors, another column to DB * Propogate contexts, fix all bugs; e2e flow works! * Add metric for tx ingestion * Split TransactionHandler into separate readers and writers * Move tx reads into their own isolated, read-only db tx * Add in-memory cache for ledger ranges * Remove cache: read/write are separate so it doesn't work * Add trimming of transactions table based on retention window * Tests work (but dont pass)! Plus some fixes based on test cases 👍 * Tidy up the go.mod file * Fixup the way ledger ranges are handled * Add prometheus metrics back * Drop columns and logging that aren't useful * Incorporate errors into ledger range retrieval * Return instead of log errors in getTransaction * Fixup tests: an empty db isn't a fetch error * Make 24hr the new default transaction retention window, fixup errors * Expect the correct retention window in tests * Use raw byte hash over hex for 1/2 the column space * Rename interfaces to avoid overloading 'Transaction' term * PR feedback: variable rename * Separate tx parsing into its own function * Modify queries to not use transactions * Pass around the logger rather than using the global * Fixup all of the tests to pass a logger * Use subqueries to get around () sqlite limitations * Use .Select() to simplify querying * Add db unit test but it's failing :( * Prefer custom error over EOF or DB nonsense * Prepare randoms in advance, writer for the ledger as well * Pass daemon to db to hook in metrics * Check another error in e2e test * Feedback: use no-op interface, drop metric * Refactor method tests to use 'fake' tx/ledger backend * Simplify code a bit * Fixup mocking interface * Move test function back, fixup metrics and tests * Add `getTransactions` endpoint (#136) * Add cursor for getTransactions * Add validation of pagination and range * implement getTransactions handler - 1 * Add getLedgers * Read a single ledger instead of getting all ledgers * Add ParseCursor method for tx cursor * Cursor changes - 1 * Cursor changes - 2 * Cursor changes - 3 * revert go-mod changes * Use reader.Seek() * Use reader.Seek() - 2 * Add config-options for tx pagination limits * Fix failing cursor test * Go mod changes * Go mod changes - 2 * Add unittests - 1 * Update go.mod and go.sum * Add integration tests * Update go.mod and go.sum * Add ledgerSeq to error string * Add docstrings * Change transactions limits * add defensive check for error other than EOF * add defensive check for error other than EOF - 2 * Change ledger sequence to uint32 * Add comments/docstrings * Include only cursor in response * Use toid instead of new cursor * Revert cursor changes * Return cursor as string in result * Refactor reader.Seek error handling * Small refactoring * Remove startLedger check * Remove endLedger * Import fix * Fix failing tests * Refactor to use new transaction db * Refactor mocks * Refactor unittests for using the new db changes * Refactor integration test * Add config vars for max requests and request duration * Fix failing test * Use transactionInfo struct instead of db.Transactions * Start indexing from 1 instead of 0 for toid * Operation index from 1 * Add lines to make sure structs implement interfaces * Remove omitempty * rename test func * make txInfo struct public and convert string cursor to int * Use sendSuccessfulTransaction helper func * Convert cursor to string in request * Change jrpc response codes * Change ledger close meta code to invalid-params * Revert back to InvalidParams error for reader.Read() * Refactor if-else statement * Refactor if-else statement - 2 * Add documentation on opting into the new db backend (#182) * Refactor getFeeStats changes to add db changes * Remove code making `GetLedgerRange` conform to interface (#186) It's unnecessary because we aren't using that interface anymore. * Add changelog entry for `getTransactions` (#183) * Add more changelog details on new endpoint * Fixup spacing --------- Co-authored-by: George <Shaptic@users.noreply.github.com> Co-authored-by: George Kudrayvtsev <george@stellar.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Implement the
getTransactionsendpoint. It reads the ledger close meta stored in sqlite and decodes the necessary transaction information.A quick summary of what is added in the PR:
getTransactionsWhy
See #110
Known limitations
N/A