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]: Polygon conversion amount inconsistent along the Send ERC20 flow with no units #20106

Closed
seaona opened this issue Jul 19, 2023 · 2 comments
Assignees
Labels
Sev2-normal Normal severity; minor loss of service or inconvenience. team-assets type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Jul 19, 2023

Describe the bug

Problem: whenever i am sending a transaction from Polygon network, I can see how on the confirmation screen there is a conversion value that I don't know what it is.
Furthermore, on the Asset screen, I see no conversion, only ETH below the value.

polygon-tx.mp4

Steps to reproduce

  1. Select Polygon
  2. Import an ERC20 token
  3. Click Send
  4. Add recipient
  5. Add amount -- see ETH below without any value
  6. Click Next
  7. See a converted value (not sure what it is as no Units are displayed)

Error messages or log output

No response

Version

10.33.1 prod

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

@seaona seaona added type-bug team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead labels Jul 19, 2023
@Gawd71730
Copy link

Thanks

@bschorchit bschorchit added the Sev2-normal Normal severity; minor loss of service or inconvenience. label Jul 24, 2023
@bschorchit bschorchit added team-confirmations-planning (only for internal use within Confirmations team) team-assets and removed team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead team-confirmations-planning (only for internal use within Confirmations team) labels Aug 24, 2023
@sahar-fehri sahar-fehri self-assigned this Oct 5, 2023
sahar-fehri added a commit that referenced this issue Oct 11, 2023
## **Description**
This PR fixes one part of the bug shown in this github issue:
#20106

When you try to send a token on polygon network and you are on the
confirmation page; it shows the value of matic without a unit not an
icon to indicate the network.
If you are on ethereum network and you try to send an ERC20, it shows
ethereum icon.

In this PR i am using the AvatarNetwork component that is used on the
main tokens page to indicate the network of tokens.


## **Manual testing steps**

_1. Switch to polygon
_2. Choose an ERC20 token and click send.
_3. Add amount in amount input and click confirm and notice there is no
unit/network icon displayed

## **Screenshots/Recordings**

_If applicable, add screenshots and/or recordings to visualize the
before and after of your change._

### **Before**

Ethereum:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/a7ab2281-9661-4661-9d1a-2d58c91e3026)

Polygon


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/1b2d6565-34c6-4381-a228-88f6af57d2b4)

BNB


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/6a1f1285-1837-4201-ba8c-185ec1dc5f2d)


### **After**

Ethereum:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/faadf071-8ae4-4f04-8647-36cedf11d633)

Polygon:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/2cd289c5-4e60-4021-a29d-482c94ca096d)

BNB:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/a9094246-7afa-4368-aa70-03c6af9a4d67)



## **Related issues**

[_Fixes
#MMASSETS-26](https://consensyssoftware.atlassian.net/browse/MMASSETS-26)

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] 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)).
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
sahar-fehri added a commit that referenced this issue Oct 13, 2023
## **Description**
This PR solves one bug shown in this github issue:
#20106

When you try to send a token, on Polygon, it does not show the
conversion of the amount, it just shows "ETH".

If you chose "FIAT" in the settings=>general, this issue does not occur.
This only happens when you choose crypto.

PS: I have also noticed this issue on BNB chain. On Ethereum network,
all works fine. If you try to send a token on other chains, you won't be
able to see the conversion correctly and it will always just show "ETH".

## **Manual testing steps**

_1. Chose polygon network
_2. Go to settings => General and choose "MATIC" as the primary
currency.
_3. Go back to the home screen and choose a token to send (exp USDC)
_4. Click on the "send" button and choose an account
_5. Notice that if you type "5" in the amount input, you will see "ETH"
as the conversion of that amount.

(The same steps are applicable on BNB chain)

## **Screenshots/Recordings**

_If applicable, add screenshots and/or recordings to visualize the
before and after of your change._

### **Before**

On Polygon


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/56959e21-d14c-4cc4-a950-46ec240476dd)

On BNB


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/de6aac12-b250-4b56-b1d2-4f38c21fd6c2)


### **After**

On Polygon:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/19ed9179-0f55-4f90-8c9b-4cc8ee49f1a1)

On BNB:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/8c5819f3-62e5-4156-9e76-406c760c8827)


## **Related issues**

(_Fixes
#MMASSETS-26)[https://consensyssoftware.atlassian.net/browse/MMASSETS-26]

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] 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)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
k-g-j pushed a commit that referenced this issue Nov 1, 2023
## **Description**
This PR fixes one part of the bug shown in this github issue:
#20106

When you try to send a token on polygon network and you are on the
confirmation page; it shows the value of matic without a unit not an
icon to indicate the network.
If you are on ethereum network and you try to send an ERC20, it shows
ethereum icon.

In this PR i am using the AvatarNetwork component that is used on the
main tokens page to indicate the network of tokens.

## **Manual testing steps**

_1. Switch to polygon
_2. Choose an ERC20 token and click send.
_3. Add amount in amount input and click confirm and notice there is no
unit/network icon displayed

## **Screenshots/Recordings**

_If applicable, add screenshots and/or recordings to visualize the
before and after of your change._

### **Before**

Ethereum:

![image](https://github.com/MetaMask/metamask-extension/assets/10994169/a7ab2281-9661-4661-9d1a-2d58c91e3026)

Polygon

![image](https://github.com/MetaMask/metamask-extension/assets/10994169/1b2d6565-34c6-4381-a228-88f6af57d2b4)

BNB

![image](https://github.com/MetaMask/metamask-extension/assets/10994169/6a1f1285-1837-4201-ba8c-185ec1dc5f2d)

### **After**

Ethereum:

![image](https://github.com/MetaMask/metamask-extension/assets/10994169/faadf071-8ae4-4f04-8647-36cedf11d633)

Polygon:

![image](https://github.com/MetaMask/metamask-extension/assets/10994169/2cd289c5-4e60-4021-a29d-482c94ca096d)

BNB:

![image](https://github.com/MetaMask/metamask-extension/assets/10994169/a9094246-7afa-4368-aa70-03c6af9a4d67)

## **Related issues**

[_Fixes
#MMASSETS-26](https://consensyssoftware.atlassian.net/browse/MMASSETS-26)

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] 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)).
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
k-g-j pushed a commit that referenced this issue Nov 1, 2023
## **Description**
This PR solves one bug shown in this github issue:
#20106

When you try to send a token, on Polygon, it does not show the
conversion of the amount, it just shows "ETH".

If you chose "FIAT" in the settings=>general, this issue does not occur.
This only happens when you choose crypto.

PS: I have also noticed this issue on BNB chain. On Ethereum network,
all works fine. If you try to send a token on other chains, you won't be
able to see the conversion correctly and it will always just show "ETH".

## **Manual testing steps**

_1. Chose polygon network
_2. Go to settings => General and choose "MATIC" as the primary
currency.
_3. Go back to the home screen and choose a token to send (exp USDC)
_4. Click on the "send" button and choose an account
_5. Notice that if you type "5" in the amount input, you will see "ETH"
as the conversion of that amount.

(The same steps are applicable on BNB chain)

## **Screenshots/Recordings**

_If applicable, add screenshots and/or recordings to visualize the
before and after of your change._

### **Before**

On Polygon


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/56959e21-d14c-4cc4-a950-46ec240476dd)

On BNB


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/de6aac12-b250-4b56-b1d2-4f38c21fd6c2)


### **After**

On Polygon:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/19ed9179-0f55-4f90-8c9b-4cc8ee49f1a1)

On BNB:


![image](https://github.com/MetaMask/metamask-extension/assets/10994169/8c5819f3-62e5-4156-9e76-406c760c8827)


## **Related issues**

(_Fixes
#MMASSETS-26)[https://consensyssoftware.atlassian.net/browse/MMASSETS-26]

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] 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)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@Gawd71730
Copy link

Bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sev2-normal Normal severity; minor loss of service or inconvenience. team-assets type-bug
Projects
None yet
Development

No branches or pull requests

4 participants