rpc-alt: finish queryTransactionBlocks #21010
Open
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
amnn
force-pushed
the
amnn/rpc-query-tx
branch
from
453d8d3 to
0b2bfc6
Compare
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
amnn
commented
Jan 31, 2025
| TX: ExpressionMethods + Expression<SqlType = BigInt>, | ||
| TX::IsAggregate: MixedAggregates<Never, Output = No>, | ||
| { | ||
| query = query.filter(tx_sequence_number.ge(sql::<BigInt>(&format!( |
There was a problem hiding this comment.
I will want to write some tests for this, I considered doing that using the E2E test runner, but that's probably too big a lift, so I'll likely port over Xun's test environment code (that's to come).
7 tasks
bmwill
reviewed
Jan 31, 2025
amnn
force-pushed
the
amnn/rpc-query-tx
branch
from
25973bc to
06263ff
Compare
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
## Descriptioon Filter transactions based on the functions they call. ## Test plan New E2E tests: ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests -- transactions/query ```
## Description I needed to add a checkpoint data loader, so I also added the JSON-RPC endpoint as a way to test the loader. ## Test plan ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests -- checkpoints ```
## Description Filter down to transactions from a particular checkpoint by loading their digests from that checkpoint. ## Test plan New E2E tests: ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \ -- transactions/query/by_checkpoint ```
## Description Filter transactions based on their relationships with addresses (senders and recipients). This completes support for transaction block querying. ## Test plan ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \ -- transactions/query/by_address ```
## Description Pull out the common parts of fetching a page of transaction digests into a helper function, and use that in all the filter implementations. ## Test plan Use existing tests to confirm behaviour is the same: ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \ -- transactions/query ```
## Description Refactor how `queryTransactionBlocks` works so that filters that rely on `tx_*` tables (which produce sequence numbers) don't join against `tx_digests`, but instead produce sequence numbers directly. These are later converted into digests using a data loader. This change was motivated by profiling the previous pattern (joining against `tx_digests`) on our production tables and noticing that this would often choose poor query plans. ## Test plan ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests ```
## Description Based on an observation from @wlmyng, query efficiency improves when you explicit bound the query by the reader low-watermark, because it allows the database to avoid scanning a number of dead tuples it hasn't cleaned up since pruning. ## Test plan Existing E2E tests, and local testing with our mainnet DB.
## Description When using `response::transaction` to convert a stored transaction into a `SuiTransactionBlockResponse`, as part of `queryTransactionBlocks`, we also need to detect a `NotFound` error, (usually a user error), and translate that into an internal error, because we shouldn't be able to query a transaction block that we can't get the contents for. ## Test plan Tested against our production database which doesn't include `kv_transactions` (so does trigger this internal error).
## Description When applying a lowerbound to transaction queries, we need to take the max lowerbound between the filter table and `tx_digests`. ## Test plan Run a pruned indexer locally and confirm that it doesn't produce an error when fetching the first unpruned page of transactions for any given query.
amnn
force-pushed
the
amnn/rpc-query-tx
branch
from
06263ff to
52d477c
Compare
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
This was referenced Feb 6, 2025
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.
Description
Completing the filter implementations for
suix_queryTransactionBlocks. It was also helpful to implementsui_getCheckpointbecause the checkpoint filter is implemented by loading a checkpoint, which means we implement the checkpoint data loader, and it is helpful to then have a way to test that directly.The PR is split into multiple self-contained commits, each with their own descriptions and tests, each responsible for implementing filters over one table.
Test plan
New E2E tests:
Stack
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.