-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
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
nedsalk
changed the title
feat!: read malleable fields from transaction status
feat!: read malleable fields from transaction status on subscription
Aug 16, 2024
nedsalk
force-pushed
the
ns/feat/read-subscription-receipts
branch
from
August 16, 2024 12:31
51b9314
to
3ef94b9
Compare
nedsalk
force-pushed
the
ns/feat/read-subscription-receipts
branch
3 times, most recently
from
August 16, 2024 13:34
c444e3a
to
0a7be3c
Compare
nedsalk
requested review from
digorithm,
arboleya,
Torres-ssf,
Dhaiwat10,
danielbate,
petertonysmith94 and
maschad
as code owners
August 16, 2024 13:37
nedsalk
force-pushed
the
ns/feat/read-subscription-receipts
branch
from
August 16, 2024 16:16
0a7be3c
to
68470f9
Compare
nedsalk
force-pushed
the
ns/feat/read-subscription-receipts
branch
from
August 16, 2024 16:47
4702a98
to
98aaafd
Compare
Torres-ssf
reviewed
Aug 16, 2024
nedsalk
commented
Aug 16, 2024
packages/account/src/providers/transaction-response/transaction-response.ts
Show resolved
Hide resolved
4 tasks
nedsalk
force-pushed
the
ns/feat/read-subscription-receipts
branch
from
August 16, 2024 20:56
5e68b4f
to
4617e15
Compare
arboleya
reviewed
Aug 16, 2024
petertonysmith94
previously approved these changes
Aug 18, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nitty comments :)
packages/account/src/providers/transaction-response/transaction-response.ts
Show resolved
Hide resolved
petertonysmith94
approved these changes
Aug 18, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
danielbate
approved these changes
Aug 19, 2024
packages/account/src/providers/transaction-response/transaction-response.ts
Show resolved
Hide resolved
Torres-ssf
approved these changes
Aug 19, 2024
Coverage Report:
Changed Files:
|
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.
TransactionStatus
#1644Release notes
In this release, we:
Summary
The
TransactionResponse
class can now work with transaction ids as well as theTransactionRequest
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 inProvider.sendTransaction
. Given thatTransactionRequest
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
andstatusChange
subscriptions now return that malleable data, so we only take that data and integrate it withTransactionRequest.toTransaction
to give a fullTransactionSummary
.I have removed
TransactionResult.gqlTransaction
because it assumed that the wholegqlTransaction
would always be fetched insideTransactionResponse
. Given that this is no longer the case, we cannot support this field. There's no real regression here, however, becauseTransactionResult.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 thatTransactionResult.gqlTransaction
has.Checklist