-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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]: Deeplinks not displaying error for networks not added to wallet and displaying Confirm screen for wrong network #11966
Labels
regression-RC-7.34.0
Regression bug that was found in release candidate (RC) for release 7.34.0
release-7.35.0
Issue or pull request that will be included in release 7.35.0
release-blocker
This bug is blocking the next release
Sev1-high
An issue that may have caused fund loss or access to wallet in the past & may still be ongoing
team-sdk
SDK team
type-bug
Something isn't working
Comments
chrisleewilcox
added
type-bug
Something isn't working
team-sdk
SDK team
release-blocker
This bug is blocking the next release
Sev1-high
An issue that may have caused fund loss or access to wallet in the past & may still be ongoing
labels
Oct 23, 2024
metamaskbot
added
the
regression-RC-7.34.0
Regression bug that was found in release candidate (RC) for release 7.34.0
label
Oct 23, 2024
Fixed by #12048 |
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 28, 2024
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR validates the chain id before handling the deeplink for making an ethereum send. ## **Related issues** Fixes: [11966](#11966) ## **Manual testing steps** 1. Open the deeplink https://metamask.app.link/send/0x2990079bcdEe240329a520d2444386FC119da21a@56?value=3e18 2. Ensure that BNB is not added as a chain on the wallet 3. You should see an error "Unable to find network with chain id" ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> https://github.com/user-attachments/assets/c790fcdc-7737-4718-b5b5-f117151ea0a2 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] 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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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.
metamaskbot
added
the
release-7.35.0
Issue or pull request that will be included in release 7.35.0
label
Oct 28, 2024
runway-github bot
added a commit
that referenced
this issue
Oct 28, 2024
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR validates the chain id before handling the deeplink for making an ethereum send. ## **Related issues** Fixes: [11966](#11966) ## **Manual testing steps** 1. Open the deeplink https://metamask.app.link/send/0x2990079bcdEe240329a520d2444386FC119da21a@56?value=3e18 2. Ensure that BNB is not added as a chain on the wallet 3. You should see an error "Unable to find network with chain id" ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> https://github.com/user-attachments/assets/c790fcdc-7737-4718-b5b5-f117151ea0a2 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] 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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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.
runway-github bot
added a commit
that referenced
this issue
Oct 28, 2024
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR validates the chain id before handling the deeplink for making an ethereum send. ## **Related issues** Fixes: [11966](#11966) ## **Manual testing steps** 1. Open the deeplink https://metamask.app.link/send/0x2990079bcdEe240329a520d2444386FC119da21a@56?value=3e18 2. Ensure that BNB is not added as a chain on the wallet 3. You should see an error "Unable to find network with chain id" ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> https://github.com/user-attachments/assets/c790fcdc-7737-4718-b5b5-f117151ea0a2 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] 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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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.
runway-github bot
pushed a commit
that referenced
this issue
Oct 28, 2024
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR validates the chain id before handling the deeplink for making an ethereum send. ## **Related issues** Fixes: [11966](#11966) ## **Manual testing steps** 1. Open the deeplink https://metamask.app.link/send/0x2990079bcdEe240329a520d2444386FC119da21a@56?value=3e18 2. Ensure that BNB is not added as a chain on the wallet 3. You should see an error "Unable to find network with chain id" ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> https://github.com/user-attachments/assets/c790fcdc-7737-4718-b5b5-f117151ea0a2 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] 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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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.
7 tasks
sethkfman
pushed a commit
that referenced
this issue
Oct 29, 2024
- fix: validate chain before send (#12048) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR validates the chain id before handling the deeplink for making an ethereum send. ## **Related issues** Fixes: [11966](#11966) ## **Manual testing steps** 1. Open the deeplink https://metamask.app.link/send/0x2990079bcdEe240329a520d2444386FC119da21a@56?value=3e18 2. Ensure that BNB is not added as a chain on the wallet 3. You should see an error "Unable to find network with chain id" ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> https://github.com/user-attachments/assets/c790fcdc-7737-4718-b5b5-f117151ea0a2 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] 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 - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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. [d1a58ab](d1a58ab) Co-authored-by: Mpendulo Ndlovu <mpendulo@elefantel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
regression-RC-7.34.0
Regression bug that was found in release candidate (RC) for release 7.34.0
release-7.35.0
Issue or pull request that will be included in release 7.35.0
release-blocker
This bug is blocking the next release
Sev1-high
An issue that may have caused fund loss or access to wallet in the past & may still be ongoing
team-sdk
SDK team
type-bug
Something isn't working
Describe the bug
Tapping on a send deeplink for a network that has not been added to the wallet will still create the Confirm Send screen for the selected network which can potentially send incorrect tokens. Tapping on a deeplink for a network that hasn't been added to the wallet then the mobile app should display an alert/error that indicates network not recognized and not do anything else (should not create the Confirm screen).
BNB Send deeplink: https://metamask.app.link/send/0x2990079bcdEe240329a520d2444386FC119da21a@56?value=3e18
Expected behavior
Tapping on a deeplink for a network that hasn't been added to the wallet then the mobile app should display an alert/error that indicates network not recognized and not do anything else (should not create the Confirm screen).
Screenshots/Recordings
Screen.Recording.2024-10-22.at.6.18.43.PM.mov
7.33.0
Steps to reproduce
GIVEN I don't have BNB network added to my wallet
WHEN I tap a send deeplink for BNB network
THEN mobile app is sent to the foreground of device
AND device alert warning indicating network not recognized (FAILED: no alert is displayed)
AND no other action is taken (FAILED: Confirm screen for sending assets is displayed)
Error messages or log output
No response
Detection stage
During release testing
Version
7.34.0
Build type
None
Device
Any
Operating system
iOS, Android
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: