fix(perps): use selected EVM account for withdraw confirmation batch#32001
Draft
dan437 wants to merge 4 commits into
Draft
fix(perps): use selected EVM account for withdraw confirmation batch#32001dan437 wants to merge 4 commits into
dan437 wants to merge 4 commits into
Conversation
Perps withdraw builds an Arbitrum (EVM) transaction batch via addTransactionBatch with the publish hooks disabled, which only succeeds through the EIP-7702 path. The `from` account was the globally selected internal account, which can be non-EVM (e.g. a Solana/Snap account in the same account group). When `from` is not an EIP-7702-capable EVM keyring, the 7702 path is skipped and addTransactionBatch throws "Can't process batch", bouncing the user back with the "Your withdrawal wasn't started" toast. Resolve `from` from the selected account group's EVM account (scope eip155), matching how the rest of Perps resolves the active account, with a fallback to the previously selected account to avoid regressions.
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
PR template — items to address before "Ready for review"Warnings — informational, address before merging:
See docs/readme/ready-for-review.md for the full Definition of Ready for Review. |
selectSelectedInternalAccountByScope reads the AccountTreeController slice, which is absent in some render contexts (e.g. the confirmations developer options screen and minimal-store renders). Fall back to the globally selected account when the account tree is unavailable so the withdraw hook never throws "Cannot read properties of undefined (reading 'backgroundState')".
…state Guard the AccountTreeController read so the withdraw hook only calls selectSelectedInternalAccountByScope when that slice exists, falling back to the globally selected account otherwise. This keeps the EVM-account resolution in production while preventing "Cannot read properties of undefined (reading 'backgroundState')" in minimal-store render contexts (e.g. the confirmations developer options screen). Reverts the test-only selector mock; no test changes are needed.
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 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.
Description
Perps Withdraw intermittently fails immediately on tap with a "Something went wrong / Your withdrawal wasn't started." toast (recoverable, with "Try again"), reported on v7.79.1+ (CONF-1543).
Root cause (confirmed via Sentry). The underlying failure is
Error: Can't process batch(JSON-RPC-32603) — Sentry issue METAMASK-MOBILE-5PRH, 111 users / 423 events, spanning 7.77–7.81 including 7.79.0. The stack isaddTransactionBatch → addTransactionBatchWithHook → throw, and breadcrumbs show it fires ~120 ms after the tap with no failing RPC beforehand, so it is a deterministic logic throw, not a network blip.usePerpsWithdrawConfirmationcreates a dummyperpsWithdrawtransfer on Arbitrum viaaddTransactionBatch({ disableHook: true, disableSequential: true }). With both publish hooks disabled, that request can only be created through the EIP-7702 path. The controller takes that path only whendoesAccountSupportEIP7702(messenger, from)istrue— i.e.fromis present inKeyringController.state.keyringswith a supported keyring type (HD Key Tree/Simple Key Pair/Money Keyring). Otherwise it falls intoaddTransactionBatchWithHook, finds no publish hook, and throwsCan't process batch.The withdraw used
selectSelectedInternalAccountAddressforfrom, which returns the globally selected internal account of any type. When the selected account in the group is non-EVM (e.g. a Solana/Snap account),fromis not an EIP-7702-capable EVM keyring →Can't process batch.Fix. Resolve
fromfrom the selected account group's EVM account viaselectSelectedInternalAccountByScope(state)('eip155:1')(the selector the rest of Perps already uses, e.g.PerpsConnectionManager), with a fallback to the previously selected account to avoid regressions.Changelog
CHANGELOG entry: Fixed an intermittent error when tapping Withdraw on Perps that could prevent the withdrawal flow from starting
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/CONF-1543
Manual testing steps
Screenshots/Recordings
Before
N/A — no device build produced for this change. Production occurrences are tracked in Sentry issue METAMASK-MOBILE-5PRH.
After
N/A
Pre-merge author checklist
Performance checks (if applicable)
Pre-merge reviewer checklist