Uh oh!
There was an error while loading. Please reload this page.
fix: transaction controller startup - #4658
Merged
Merged
Conversation
Remove initApprovals method. Export mobile constants. Fail approved transactions on startup. Improve gas fee poller error logging.
Skip validation when failing transaction. Remove approve during resubmit. Add updated rawTx to retry transactions. Fix resubmit after transaction fails.
matthewwalsh0
commented
Sep 6, 2024
MemberAuthor
@metamaskbot publish-preview |
Contributor
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Gudahtt
commented
Sep 6, 2024
Member
Thoughts on splitting this into multiple PRs? These changes seem like they would be easy to do separately, and seem unrelated to the initialization changes:
|
matthewwalsh0
commented
Sep 6, 2024
MemberAuthor
I absolutely can if required. But they are each just one line changes so wanted to avoid the admin overhead. |
Gudahtt
commented
Sep 6, 2024
Member
Since b both changes serve a distinct purpose, it would be preferred to make them separately even they are only 1-line changes, as suggested by the contributor docs. Certainly these changes don't contribute much to the size of the PR, so they aren't problematic for that reason. But if we're reviewing the commit history to understand what changed, these would not appear. And similarly if we found a bug with the initialization refactor and wanted to revert it, it would also revert these two small changes unnecessarily, which is prevented by separating them. |
pedronfigueiredo
approved these changes
Sep 11, 2024
OGPoyraz
approved these changes
Sep 13, 2024
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Explanation
Refactor the handling of incomplete transactions when initialising the
TransactionController.unapprovedtransactions are now removed entirely and theinitApprovalsmethod has been removed.initApprovalsmethod allowed the clients to re-trigger approval requests and ultimately transaction confirmations.approvedtransactions are now marked asfailed.signedtransactions are now marked asfailed.In addition:
approveTransactionflow inPendingTransactionTracker.rawTxto cancel and speed up transactions.References
Fixes#23361
Changelog
@metamask/transaction-controllerinitApprovalsmethod.beforeApproveOnInithook.unapprovedtransactions during initialisation.approvedandsignedtransactions during initialisation.Checklist