fix: transaction controller startup #4658
Merged
+673
−795
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
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.
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
|
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:
|
I absolutely can if required. But they are each just one line changes so wanted to avoid the admin overhead. |
|
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. |
matthewwalsh0
changed the title
pedronfigueiredo
approved these changes
Sep 11, 2024
OGPoyraz
approved these changes
Sep 13, 2024
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.
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