rpc-alt: showBalanceChanges for getTransactionBlock #20944
Conversation
## Description Add an index on `tx_digests` from digest to sequence number (the reverse of the primary key index). This is to make it easier to look up balance changes (keyed by sequence number in the database) based on their digest (the key we use for the KV store). The alternative was to make `tx_balances` keyed on digest, but in that case we would still need to retain the sequence number, because it is used for pruning. ## Test plan CI (Ran `./generate_schema.sh`)
## Description Rather than implementing the `StoredTransaction` DataLoader directly on `TransactionDigest` as the key type, introduce a new type wrapping the digest. This is so that we can implement other data loaders that use `TransactionDigest` as the key. ## Test plan CI
## Description Implement showBalanceChanges for getTransactionBlock. Data is fetched using a data loader, and the response logic has changed slightly so that the output corresponding to each response option gets its own function (rather than handling everything in one function). This refactoring was done to handle the fact that if balance changes are requested we: - Want to perform an extra DB load, which we ideally want to do concurrently with fetching the transaction. - Want to use the presence or absence of an `Option<StoredTxBalanceChange>` to decide whether to add the corresponding output, rather than the flag from the option. ## Test plan New E2E test: ``` sui$ cargo nextest run -p sui-indexer-alt-e2e-tests ```
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
amnn
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
There was a problem hiding this comment.
Will need to run these operations proactively before landing this PR -- I'll hold off on landing until I am back and can coordinate that with our various races etc.
| response(ctx, &stored, &options) | ||
| .await | ||
| .map_err(internal_error) | ||
| // Balance changes might not be present because of pruning, in which case we return |
There was a problem hiding this comment.
Is this the right behaviour w.r.t. pruning or should we produce an error if this happens? This behaviour matches what we do for GraphQL (just don't return the field that was pruned), but we do that mainly because we don't want the absence of one field to ruin the entire query. In JSON-RPC, we don't really have compound queries in the same way, but "options" are kind of analogous, so it seems to fit.
There was a problem hiding this comment.
Seems easier for callers to handle this way, they can do some additional filtering on the response side, as opposed to sending two requests in case the first requesting balance changes fails
There was a problem hiding this comment.
I flip-flopped on this. Here I opted for nulling out the field, but then when I started implementing showObjectChanges, I realised I couldn't do it so cleanly there, because I could end up with one (historical) object having been pruned, but everything else existing, it seemed strange to react to that by nulling out just the object changes field, so I introduced a new error for when some requested data was pruned. Maybe we can discuss the pro's/con's on Monday.
Description
Implement
showBalanceChangesforgetTransactionBlock. Data is fetched using a data loader, and the response logic has changed slightly so that the output corresponding to each response option gets its own function (rather than handling everything in one function).This refactoring was done to handle the fact that if balance changes are requested we:
Want to perform an extra DB load, which we ideally want to do concurrently with fetching the transaction.
Want to use the presence or absence of an
Option<StoredTxBalanceChange>to decide whether to add the corresponding output, rather than the flag from the option.Test plan
New E2E test:
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.