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 new getTransactions endpoint and database backend for transactions #174

Merged
merged 11 commits into from
May 23, 2024

Conversation

aditya1702
Copy link
Contributor

@aditya1702 aditya1702 commented May 13, 2024

What

This PR merges all the new changes related to transactions. This includes:

Why

Part of #115

Known limitations

NA

Shaptic and others added 3 commits May 3, 2024 11:08
…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 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
@aditya1702 aditya1702 self-assigned this May 13, 2024
@aditya1702 aditya1702 marked this pull request as draft May 13, 2024 20:56
@psheth9 psheth9 added the api-change Any changes to existing API or addition of new API label May 15, 2024
@Shaptic Shaptic changed the title Merge new transactions changes in master Add new getTransactions endpoint and database backend for transactions May 16, 2024
# Conflicts:
#	cmd/soroban-rpc/internal/config/config.go
#	cmd/soroban-rpc/internal/daemon/daemon.go
#	cmd/soroban-rpc/internal/ingest/service.go
#	cmd/soroban-rpc/internal/ingest/service_test.go
#	cmd/soroban-rpc/internal/jsonrpc.go
It's unnecessary because we aren't using that interface anymore.
@aditya1702 aditya1702 marked this pull request as ready for review May 17, 2024 19:53
* Add more changelog details on new endpoint

* Fixup spacing
@2opremio
Copy link
Contributor

I would be interested to get a performance comparison between the in-memory implementation and the db implementation.

@Shaptic
Copy link
Contributor

Shaptic commented May 22, 2024

@2opremio me too! We can do a basic evaluation by seeing the change in metrics after updating (is RPC in Grafana?), but I can also try to see the results of the BenchmarkTransactionFetch after reverting #141 👍

@aditya1702 aditya1702 merged commit c818bb8 into main May 23, 2024
22 checks passed
@aditya1702 aditya1702 deleted the database-backend branch May 23, 2024 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change Any changes to existing API or addition of new API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants