Skip to content
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

Closed
chrisleewilcox opened this issue Oct 23, 2024 · 1 comment · Fixed by #12048
Assignees
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
Copy link
Contributor

chrisleewilcox commented Oct 23, 2024

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
image

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

@chrisleewilcox 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
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Oct 23, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Oct 23, 2024
@metamaskbot 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
@christopherferreira9
Copy link
Contributor

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.
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Oct 28, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Oct 28, 2024
@metamaskbot 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.
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
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants