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

rpc-alt: finish queryTransactionBlocks #21010

Merged
merged 20 commits into from
Feb 13, 2025
Merged

rpc-alt: finish queryTransactionBlocks #21010

merged 20 commits into from
Feb 13, 2025

Conversation

amnn
Copy link
Contributor

@amnn amnn commented Jan 30, 2025

Description

Completing the filter implementations for suix_queryTransactionBlocks. It was also helpful to implement sui_getCheckpoint because 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:

sui$ cargo nextest run -p sui-indexer-alt-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.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • gRPC:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

@amnn amnn requested review from emmazzz, gegaowp and wlmyng January 30, 2025 13:30
@amnn amnn self-assigned this Jan 30, 2025
Copy link

vercel bot commented Jan 30, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2025 4:24pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Feb 13, 2025 4:24pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Feb 13, 2025 4:24pm

@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env January 30, 2025 13:30 — with GitHub Actions Inactive
@amnn amnn changed the title Amnn/rpc query tx rpc-alt: finish queryTransactionBlocks Jan 30, 2025
Base automatically changed from amnn/rpc-past-obj to main January 30, 2025 13:57
@amnn amnn force-pushed the amnn/rpc-query-tx branch from 453d8d3 to 0b2bfc6 Compare January 30, 2025 16:41
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env January 30, 2025 16:41 — with GitHub Actions Inactive
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env January 31, 2025 14:35 — with GitHub Actions Inactive
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env January 31, 2025 15:45 — with GitHub Actions Inactive
@amnn amnn mentioned this pull request Jan 31, 2025
7 tasks
amnn added 20 commits February 13, 2025 12:24
## 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.
## Description

Add support for fetching individual dynamic fields.

## Test plan
New E2E tests:

```
sui$ cargo nextest run -p sui-indexer-alt-e2e-tests -- dynamic_fields/
```
## Description
Rename `Cursor` to `JsonCursor`, and introduce a generic `Cursor` trait,
so that we can introduce different cursor formats. This is in
preparation for adding a `BcsCursor`, similar to what we have on the
GraphQL side.

## Test plan
CI (this is a refactoring)
## Description

Basic support for getting an address's owned objects (without filtering
by type).

The performance of these queries could be improved by changing index
order (such that the Object IDs are ordered similarly to
cp_sequence_number), which will be done in a follow-up PR.
of

## Test plan

New E2E tests:

```
sui$ cargo nextest run -p sui-indexer-alt-e2e-tests -- objects/query
```

Query performance was also validated by inspecting query plans on the
production database (with and without a cursor present).
## Description
Filter owned objects by packages.

## Test plan
New E2E tests:

```
sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \
  -- objects/query/by_package
```
## Description
Filtering owned objects by their type's module.

## Test plan
New E2E tests:

```
sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \
  -- objects/query/by_module
```
## Description
Filter owned objects by their type name (ignoring type parameters).

## Test plan
New E2E tests:

```
sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \
  -- objects/query/by_name
```
## Description
Filter owned objects by it's type's type parameters.

## Test plan

```
sui$ cargo nextest run -p sui-indexer-alt-e2e-tests \
  -- objects/query/by_type_params
```
## Description

Don't need the lifetime parameter on `impl SuiTestAdapter`.

## Test plan

CI
## Description

`CompiledPackage::published_dependency_ids` is a duplicate of
`CompiledPackage::get_dependency_storage_package_ids`. Get rid of it and
replace its only call-site.

## Test plan

CI
## Description
If we find a duplicate module when creating a `Modules` map, include the
module's ID in the output.

## Test plan
Hit this error when debugging something else.
@amnn
Copy link
Contributor Author

amnn commented Feb 13, 2025

Staging the accepted prefix of the stack in this PR to land.

@amnn amnn merged commit 343bb2a into main Feb 13, 2025
158 of 164 checks passed
@amnn amnn deleted the amnn/rpc-query-tx branch February 13, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants