-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[Bug]: Multichain - transaction generated on the wrong network #25528
Labels
regression-develop
Regression bug that was found on development branch, but not yet present in production
regression-RC-12.0.0
release-12.2.0
Issue or pull request that will be included in release 12.2.0
Sev3-low
Low severity; minimal to no impact upon users
team-wallet-ux
type-bug
Comments
I think this is fixed by the commit just added to the v12 branch f5631fb |
@sleepytanya why did you remove the release-blocker label? |
7 tasks
Slack thread link with the details about the decision to remove the release-blocker label. |
adonesky1
added a commit
that referenced
this issue
Jun 28, 2024
…ingNetworkSwitch` (#25562) ## **Description** Though signature requests like `personal_sign`, `eth_signTypedData`, `eth_signTypedData_v3` and `eth_signTypedData_v4` do not depend on state of network connections, these confirmations do use nicknames/addressbook state which is dependent on globally selected network state for parsing signatures and injecting nicknaming where possible. The queueing system introduced with Amon Hen v1 (v12.0.0 release) introduces certain conditions in which these signature confirmations will be rendered on the wrong network. Though this doesn't actually result in faulty signatures, we should switch to the appropriate/expected network for the UX reasons described above. Adding `eth_signTypedData_v3` and `eth_signTypedData` to the `methodsRequiringNetworkSwitch` array will cause the network to switch to the selected network for the requesting origin before initializing the confirmation. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25562?quickstart=1) ## **Related issues** Fixes: #25528 See this slack thread: https://consensys.slack.com/archives/C06FXU326RL/p1719429561925649?thread_ts=1719415715.492249&cid=C06FXU326RL ## **Manual testing steps** See videos below 👇 ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-extension/assets/34557516/3e32fce7-046c-4856-893d-a85083877327 ### **After** <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-extension/assets/34557516/306d88aa-f5f3-4eae-a8e9-30b621c02626 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: seaona <[email protected]>
adonesky1
added a commit
that referenced
this issue
Jun 28, 2024
…ingNetworkSwitch` (#25562) ## **Description** Though signature requests like `personal_sign`, `eth_signTypedData`, `eth_signTypedData_v3` and `eth_signTypedData_v4` do not depend on state of network connections, these confirmations do use nicknames/addressbook state which is dependent on globally selected network state for parsing signatures and injecting nicknaming where possible. The queueing system introduced with Amon Hen v1 (v12.0.0 release) introduces certain conditions in which these signature confirmations will be rendered on the wrong network. Though this doesn't actually result in faulty signatures, we should switch to the appropriate/expected network for the UX reasons described above. Adding `eth_signTypedData_v3` and `eth_signTypedData` to the `methodsRequiringNetworkSwitch` array will cause the network to switch to the selected network for the requesting origin before initializing the confirmation. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25562?quickstart=1) ## **Related issues** Fixes: #25528 See this slack thread: https://consensys.slack.com/archives/C06FXU326RL/p1719429561925649?thread_ts=1719415715.492249&cid=C06FXU326RL ## **Manual testing steps** See videos below 👇 ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-extension/assets/34557516/3e32fce7-046c-4856-893d-a85083877327 ### **After** <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-extension/assets/34557516/306d88aa-f5f3-4eae-a8e9-30b621c02626 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: seaona <[email protected]>
7 tasks
chloeYue
pushed a commit
that referenced
this issue
Jun 29, 2024
…signTypedData_v3` to `methodsRequiringNetworkSwitch` (#25592) ## **Description** Cherry-pick #25562 for Version-v12.0.0 ## **Related issues** Fixes: #25528 See this slack thread: https://consensys.slack.com/archives/C06FXU326RL/p1719429561925649?thread_ts=1719415715.492249&cid=C06FXU326RL ## **Manual testing steps** See videos below 👇 ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-extension/assets/34557516/3e32fce7-046c-4856-893d-a85083877327 ### **After** <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-extension/assets/34557516/306d88aa-f5f3-4eae-a8e9-30b621c02626 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: seaona <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
regression-develop
Regression bug that was found on development branch, but not yet present in production
regression-RC-12.0.0
release-12.2.0
Issue or pull request that will be included in release 12.2.0
Sev3-low
Low severity; minimal to no impact upon users
team-wallet-ux
type-bug
Describe the bug
When a transaction is initiated on one network, subsequent transaction that was initially created on a different network may be executed on the incorrect network.
Blue badge shows '...' instead of numbers'.
Expected behavior
Screenshots/Recordings
1.mov
Steps to reproduce
Error messages or log output
No response
Version
develop
Build type
None
Browser
Chrome
Operating system
MacOS
Hardware wallet
No response
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: