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

feat: Add receipts to transaction status #1504

Merged
merged 25 commits into from
Nov 28, 2023

Conversation

bvrooman
Copy link
Contributor

@bvrooman bvrooman commented Nov 21, 2023

Related issues:

This PR adds receipts to the transaction status objects returned by transaction-related GraphQL queries. When querying for the status of a transaction, a status of Failed or Success will now include the transaction receipts.

Note that this PR does not remove Receipts from OpaqueTransaction or other places that would constitute a breaking change, thus preserving existing behaviour. We can remove receipts from redundant areas in a follow up PR after the beta release.

@bvrooman bvrooman self-assigned this Nov 22, 2023
@@ -1699,6 +1699,7 @@ where
db.update_tx_status(
id,
TransactionStatus::Success {
tx_id: *id,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to store tx_id in the database because we can fetch it from the key. The GraphQL type may have this field, while the type that we use across fuel-core does not.

@bvrooman bvrooman marked this pull request as ready for review November 25, 2023 00:39
@bvrooman bvrooman requested a review from a team November 25, 2023 00:39
@bvrooman bvrooman changed the title feat: add receipts to tx status feat: Add receipts to transaction status Nov 25, 2023
Comment on lines 437 to 438
// TODO: Remove `receipts` now that receipts are queryable from the `Success` or `Failure` status
let receipts = self.receipts(&tx_id).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove it in this PR=)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this query and replaced it with a block to read the receipts from the transaction status retrieved earlier in this function. This introduces an extra clone() because we now duplicate the receipts. This is okay for now - in the future, we can remove this function entirely because it is now redundant, and the submit_and_await_commit endpoint will return receipts for all successful and failed transactions. I have added a TODO inline to highlight this.

crates/client/src/client/schema/tx.rs Show resolved Hide resolved
Comment on lines 245 to 246
impl From<(TxId, TxStatus)> for TransactionStatus {
fn from((tx_id, s): (TxId, TxStatus)) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you can fetch tx_id from the query. You don't need to join them together from different sources

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed over a call: This TransactionStatus is part of fuel-core, and the TxId is being correctly retrieved from the query. I have replaced the implementation of From on this tuple with a constructor for more clarity.

@bvrooman bvrooman merged commit 0acfb04 into master Nov 28, 2023
30 checks passed
@bvrooman bvrooman deleted the bvrooman/feat/add-receipts-to-tx-status branch November 28, 2023 00:06
@xgreenx xgreenx mentioned this pull request Dec 12, 2023
xgreenx added a commit that referenced this pull request Dec 12, 2023
## Version v0.22.0

### Added

- [#1515](#1515): Added
support of `--version` command for `fuel-core-keygen` binary.
- [#1504](#1504): A `Success`
or `Failure` variant of `TransactionStatus` returned by a query now
contains the associated receipts generated by transaction execution.

#### Breaking
- [#1531](#1531): Make
`fuel-core-executor` `no_std` compatible. It affects the `fuel-core`
crate because it uses the `fuel-core-executor` crate. The change is
breaking because of moved types.
- [#1524](#1524): Adds
information about connected peers to the GQL API.

### Changed

- [#1517](#1517): Changed
default gossip heartbeat interval to 500ms.
- [#1520](#1520): Extract
`executor` into `fuel-core-executor` crate.

### Fixed

#### Breaking
- [#1536](#1536): The change
fixes the contracts tables to not touch SMT nodes of foreign contracts.
Before, it was possible to invalidate the SMT from another contract. It
is a breaking change and requires re-calculating the whole state from
the beginning with new SMT roots.

## What's Changed
* Added support of `--version` command for `fuel-core-keygen` binary by
@xgreenx in #1515
* Weekly `cargo update` by @github-actions in
#1518
* feat: Add receipts to transaction status by @bvrooman in
#1504
* chore: Update gossip heartbeat interval by @bvrooman in
#1517
* Weekly `cargo update` by @github-actions in
#1525
* Run beta 4 e2e test each 30 minutes by @xgreenx in
#1528
* Fix broken link. by @lukema95 in
#1526
* Extract executor into fuel-core-executor crate by @Salka1988 in
#1520
* Weekly `cargo update` by @github-actions in
#1534
* Make `fuel-core-executor` `no_std` compatible by @Salka1988 in
#1532
* Fixed contracts tables to touch only own SMT nodes by @xgreenx in
#1536
* Show info about connected peers in the gql api by @Voxelot in
#1524
* fix committed gql schema by @Voxelot in
#1540

## New Contributors
* @lukema95 made their first contribution in
#1526

**Full Changelog**:
v0.21.0...v0.22.0
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.

Add receipts to status subscription
3 participants