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

Sending private-net ETH: ETH Amount differs from user entry by factor of ≈ USD price of mainnet ETH #9929

Closed
wbt opened this issue Nov 21, 2020 · 10 comments

Comments

@wbt
Copy link
Contributor

wbt commented Nov 21, 2020

Describe the bug
When using MetaMask to send ETH, the amount shown in confirmation and actually sent differs from what the user entered by a factor of 472.89.

Example:
image

If I click Next,
image

If I click Confirm,
image
and then
image
Clicking on it produces details showing the amount sent in the summary and the amount the user entered in the Activity Log:
image
Also notice the "Gas Price (GWEI)" is expressed as a large number of dollars, rather than in GWEI.
When I then look at the receiving account, which had no prior transactions, I see that the higher amount was actually sent:
image


Here's another demo, passing on what should be 1 ETH from that account to another:
image
Clicking Next:
image
Clicking Confirm:
image
and after the transaction is mined:
image
Clicking on it produces details showing the amount sent in the summary and the amount the user entered in the Activity Log:
image
Also notice the "Gas Price (GWEI)" is expressed as a large number of dollars, rather than in GWEI.

Note that if the scaled-up amount plus gas fees exceed the account balance, you will be unable to send. This is what first tipped me off to the bug, when I was unable to send an amount <1/4% the balance of an account, with an insufficient funds error.
image

Testing across several orders of magnitude, the actual amount appears to scale perfectly linearly with the amount entered by the user, but I don't know where that particular multiplier comes from.

Steps to reproduce (REQUIRED)

  1. Set up a network running on localhost, with some account that has a high balance as well as an additional empty account.
  2. Connect to that network in MetaMask, and select the account.
  3. Click "Send."
  4. As a recipient, click "Transfer between my accounts."
  5. Select another account as the recipient.
  6. Enter an amount you wish to send. Note that if (the amount * 472.89) + (the gas price * the gas limit) exceeds the sending account balance, you will get an insufficient funds error.
  7. Click "Next."
  8. Notice that the amount the user is asked to approve is a transfer amount of 472.89x what they entered, plus the gas fee.
  9. Notice that the transaction is presented in the past tense: "SENT ETHER" as if it's already completed, prior to user confirmation.
  10. You don't need to actually confirm the transaction to see this bug in action. However, if you do, you will see that the higher amount is both sent and received.
  11. If you do complete the transaction, you will see "Send Ether" in the transaction history, showing the higher amount actually sent.
  12. If you click on the details, you will again see "Send Ether" as the title, and the higher amount, plus a value labeled GWEI and listed in dollars, as well as an Activity Log showing the amount you entered.

Expected behavior

  1. The amount of ETH to be sent, as entered by the user on the Send ETH screen, exactly matches what is shown on the confirmation screen and what is sent, and what is shown as having been sent.
  2. The gas price suggested is not drawn from mainnet ETH Gas Station or similar when not on mainnet.
  3. Prior to confirmation, the transaction is labeled "Send ETH" and after confirmation it's "Sent ETH," rather than the other way around.
  4. In the transaction details, the value next to "Gas Price (GWEI)" is a gas price in GWEI, matching what the user entered (or left as it was filled in by default based on mainnet statistics), not a large number of dollars.
  5. The value of the transaction shown in the Activity Log in the transaction details view matches the amount actually being sent.

There might be several different issues here, but the combination is very concerning about the trustworthiness of MetaMask as a wallet.

Browser details (please complete the following information):

  • OS: Windows 10
  • Hardware Wallet: Not in use here
  • Browser: Chrome 86.0.4240.183 (Official Build) (64-bit)
  • MetaMask Version: 8.1.3
@tmashuang
Copy link
Contributor

tmashuang commented Nov 21, 2020

Set up a network running on localhost,

Can you expand on this?

@wbt
Copy link
Contributor Author

wbt commented Nov 21, 2020

Downloading & running Ganache might work; I used geth with the puppeth tool. There might be some tutorials out there for geth.

I found in one case that resetting the account a few times caused the multiplier to rise to 512.27. After removing and re-adding a network which was different than the one used in testing for this Issue, the multipiler for that network (which had been at 472.89 and then 512.27) fell to 1. I don't know why.

@tmashuang
Copy link
Contributor

tmashuang commented Nov 21, 2020

How are you adding the network? What are you inputting in the fields?

@tmashuang tmashuang reopened this Nov 21, 2020
@wbt
Copy link
Contributor Author

wbt commented Nov 21, 2020

I'm adding the networks by going to custom RPC and entering the URL (beginning with http...) with port number and chainID. This happened across multiple independent chains with different RPC endpoints and chainIds. The chainId is not one of the preprogrammed ids assigned to mainnnet (1) or a known testnet.
On reflection, I think the multiplier being applied may be the then-current ETH price in USD according to some exchange which Metamask might be using to source conversions. MetaMask probably updated its sense of the price during the time when I was trying various resets.
You may notice in the screenshots it's saying "No conversion rate available" because it's private-net ETH with no market value and no way for Metamask to get a price signal on that valueless ETH. Maybe in that case it's incorrectly applying conversions where it shouldn't?

@tmashuang
Copy link
Contributor

tmashuang commented Nov 21, 2020

Can you set the Currency Symbol to ETH, and/or to something else, this sounds related to #9492?

@wbt
Copy link
Contributor Author

wbt commented Nov 23, 2020

The symbol was already set to ETH. (ETH is fine, it's only used for a gas fee anyway). If I get back to seeing transactions with a multiplier >>1, I'll try setting the currency symbol to something else and see if that changes things.
I did just observe this on another user's machine, who typically sticks to one private network (the same one where I first observed this) plus mainnet, and one account, generally without doing resets or using advanced features. I didn't catch the exact multiplier, but it seemed to be in the 600x range. That would be consistent with the current USD price of ETH (dev continues kicking self for not hodling more).

@wbt wbt changed the title Sending ETH: Amount differs from user entry by 472.89x Sending private-net ETH: ETH Amount differs from user entry by factor of ≈ USD price of mainnet ETH Nov 23, 2020
@wbt
Copy link
Contributor Author

wbt commented Nov 23, 2020

Today I saw this again on one of the networks where I'd originally observed it. From the same sending account as above, I entered a transaction to send 10 ETH to a new account, and that got translated as 5971.4 ETH (a factor of 597.14, at a time when Coindesk reported the ETH price as 595.73 USD). Several minutes later after the ETH price had gone up a bit, I drafted a transaction and saw the multiplier had changed to 597.14; I rejected that transaction. I then went to Settings -> Networks and modified the network setting to use TST as the symbol (short for test).
That same account in the screenshots above looked like this, with the complete account history shown:
image
When clicking on details for the same 1 ETH transaction in the screenshot above, I got this:
image
I then went to initiate a new transaction to send another 1 ETH/TST to the same address as the prior single-ETH test:
image
Notice that the Asset box says "ETH" in big letters but "TST" after the numeric balance. In general I would expect those to match even when the network symbol isn't ETH.
Then I click Next and see this:
image
Again, at the top is "Sent Ether" with Sent in the past tense and Ether instead of TST. There's no longer an ETH logo in front of the number, as on the prior page and in screenshots above.
I clicked Confirm and the transaction confirms, showing a new History entry with "Send ETH" and "-1 TST" (again tense and symbol issue on "Send ETH"). The recipient balance shows as exactly 2 TST.
So far that looks much better.
I then changed the symbol to ETH. In drafting transactions I was still at a multiplier of 1.
I then cleared out the symbol field in network settings for that network, and in drafting transactions, had a multiplier of 597.14 again, with the balance of the same account at 52546.6396 ETH.
I changed the symbol back to ETH and the balance reverted to 87.9972 ETH, and draft transactions had a multiplier of 1.
Back to a blank network symbol, and the high multiplier returned.
Back to ETH as network symbol, and I was back at the expected balance with a multiplier of 1.

My working hypothesis is now that this issue occurs only when the network Symbol, marked "(optional)", is blank.
Since "optional" appears to be inaccurate, I would at least replace that with 'e.g. "ETH"' to show an example of the expected value, and definitely improve the handling when that is blank. MetaMask has some multiplication going on where it shouldn't!

@tmashuang
Copy link
Contributor

Closing as a duplicate of #9492.

@tmashuang
Copy link
Contributor

Also related to #9701

@wbt
Copy link
Contributor Author

wbt commented Dec 4, 2020

FWIW, I don't really agree with the closure, as I think this issue calls out a few details missed in the other such as the "Gas Price (GWEI)" being expressed as a large number of dollars, "Send Ether" vs. "Sent Ether" messaging, and a fair amount of detail provided here.
The gas price source issue (#2 in Expected Behavior) has been broken off as #10003.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants