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!: read malleable fields from transaction status on subscription #2962

Merged
merged 4 commits into from
Aug 19, 2024

Conversation

nedsalk
Copy link
Contributor

@nedsalk nedsalk commented Aug 16, 2024

Release notes

In this release, we:

  • Reduced required roundtrips & bandwidth for submitting TXs

Summary

The TransactionResponse class can now work with transaction ids as well as the TransactionRequest class.

For transaction ids the use case is as previous - you use it to fetch the whole transaction as-is on the node.

The newly added TransactionRequest integration serves the use case when a transaction was just submitted in Provider.sendTransaction. Given that TransactionRequest already contains most of the transaction data except the malleable fields that are only set upon transaction execution (e.g. receipts), we don't need to fetch the whole transaction from the node; rather, submitAndAwait and statusChange subscriptions now return that malleable data, so we only take that data and integrate it with TransactionRequest.toTransaction to give a full TransactionSummary.

I have removed TransactionResult.gqlTransaction because it assumed that the whole gqlTransaction would always be fetched inside TransactionResponse. Given that this is no longer the case, we cannot support this field. There's no real regression here, however, because TransactionResult.transaction contains the same data.

Breaking Changes

Removed TransactionResult.gqlTransaction. You can use the TransactionResult.transaction field and other fields on the response instead, which have all the data that TransactionResult.gqlTransaction has.

// before
const { gqlTransaction } = await new TransactionResponse('your-tx-id').waitForResult();
// after
const { transaction, id, fee, ...rest } = await new TransactionResponse('your-tx-id').waitForResult();

Checklist

  • All changes are covered by tests (or not applicable)
  • All changes are documented (or not applicable)
  • I reviewed the entire PR myself (preferably, on GH UI)
  • I described all Breaking Changes (or there's none)

@nedsalk nedsalk added the feat Issue is a feature label Aug 16, 2024
@nedsalk nedsalk self-assigned this Aug 16, 2024
@nedsalk nedsalk changed the title feat!: read malleable fields from transaction status feat!: read malleable fields from transaction status on subscription Aug 16, 2024
@nedsalk nedsalk force-pushed the ns/feat/read-subscription-receipts branch from 51b9314 to 3ef94b9 Compare August 16, 2024 12:31
@nedsalk nedsalk force-pushed the ns/feat/read-subscription-receipts branch 3 times, most recently from c444e3a to 0a7be3c Compare August 16, 2024 13:34
@nedsalk nedsalk marked this pull request as ready for review August 16, 2024 13:37
@nedsalk nedsalk force-pushed the ns/feat/read-subscription-receipts branch from 0a7be3c to 68470f9 Compare August 16, 2024 16:16
@nedsalk nedsalk force-pushed the ns/feat/read-subscription-receipts branch from 4702a98 to 98aaafd Compare August 16, 2024 16:47
petertonysmith94

This comment was marked as resolved.

Copy link
Contributor

@petertonysmith94 petertonysmith94 left a comment

Choose a reason for hiding this comment

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

Small nitty comments :)

@petertonysmith94 petertonysmith94 dismissed their stale review August 18, 2024 10:26

Incorrect approval

Copy link
Contributor

@petertonysmith94 petertonysmith94 left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.21%(+0.03%) 71.41%(-0.04%) 77.35%(+0.04%) 79.34%(+0.03%)
Changed Files:
Ok File (✨=New File) Lines Branches Functions Statements
🔴 packages/account/src/providers/generated/operations.ts 94.2%
(+0.04%)
100%
(+0%)
81.81%
(+0%)
94.7%
(+0.04%)
🔴 packages/account/src/providers/transaction-response/transaction-response.ts 76.74%
(+3.73%)
62.74%
(+2.03%)
93.75%
(+3.75%)
76.74%
(+3.73%)
🔴 ✨ packages/utils/src/index.ts 0%
(+0%)
100%
(+100%)
0%
(+0%)
0%
(+0%)

@nedsalk nedsalk merged commit 7f50d40 into master Aug 19, 2024
20 checks passed
@nedsalk nedsalk deleted the ns/feat/read-subscription-receipts branch August 19, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Issue is a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use receipts from TransactionStatus
5 participants