"Approve" confirmation says "Unlimited" even though it's not

[tennox]

Describe the bug
Follow-up to MetaMask/Design#38 and in specific this comment by @pi0neerpat MetaMask/Design#38 (comment) as @Gudahtt requested to open a new issue.

I encountered that MetaMask would show "Unlimited" even though I requested a specific (small) amount:

Only in case of requesting 0 it would show the expected "Proposed Approval Limit":

Steps to reproduce
Steps to reproduce the behavior, libraries used with version number, and/or any setup information to easily reproduce:

1. tokenContract.approve(spenderAddress, BigNumber.from(ethers.constants.WeiPerEther))
2. Click on 'Edit permissions'

Expected behavior
Show "Proposed Approval Limit" UI except when the amount is the maximum uint265 amount 1.157...e+59

Browser details (please complete the following information):

• OS: Pop!OS
• Browser: Version 1.24.64 Chromium: 90.0.4430.72 (Official Build) beta (64-bit)
• MetaMask Version: 9.5.2

[tennox]

See also: ethereum/EIPs#717

[danjm]

To reproduce using https://metamask.github.io/test-dapp/:

1. Switch to a Rinkeby or Ropsten account that has some ETH
2. Go to https://metamask.github.io/test-dapp/
3. Connect
4. Click the "Create Token" button and confirm the transaction in the popup
5. Wait for the transaction to be confirmed
6. Once the transaction is confirm, the contract address will appear under the "Token:" label in the "Send Tokens" section of the test dapp page.
7. Copy the token contract address
8. Open MetaMask in a new tab, so that the test dapp remains open.
9. Click "Import Tokens" on the main screen of MetaMask.
10. Paste the copied address in the "Token Contract Address" field
11. Click "Add Custom Token" and confirm
12. Using the send feature within MetaMask, send 5 of the newly added TST tokens to another one of your MetaMask accounts
13. After the send is complete, you should have 5 TST in your current account
14. Go back to the tab where you first opened https://metamask.github.io/test-dapp/
15. Click the "Approve Tokens" button
16. On the confirmation screen, click "View full transaction details" and then "Edit" in the "Permission" section

You should now see it say "Unlimited"

I believe this is what appens when the requested approval amount is greater than the users current token balance.

[david0xd]

@danjm Can you please tell us when showing the Unlimited label is correct?
We would like to know in which use case scenario the "Unlimited" is correct.
It would help us to perform the QA testing and make sure that we're not breaking the other flows when doing this.
Thanks!

[EHaracic]

@danjm in your scenario above we need to have screen as below?

[tennox]

@danjm Can you please tell us when showing the Unlimited label is correct? We would like to know in which use case scenario the "Unlimited" is correct. It would help us to perform the QA testing and make sure that we're not breaking the other flows when doing this. Thanks!

I'd say this:

Expected behavior
Show "Proposed Approval Limit" UI except when the amount is 0 for the maximum uint265 amount 1.157...e+59

[david0xd]

Thank you @tennox that makes sense.

Proposed Approval Limit is when:
amount > 0 && amount < max(uint256)

Unlimited is when:
amount == 0 || amount == max(uint256)

Question: Is 0 for the amount considered as unlimited or should be handled in a different way?

Asking @danjm to confirm.

[tennox]

Sorry, I was confused on that one. "Unlimited" is only in case of MAX_UINT, not in case of zero - as that is actually zero (i.e. withdraw allowance)

Source:
https://ethereum.stackexchange.com/a/86938
https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#IERC20-approve-address-uint256-

[danjm]

Yeah, this:

Proposed Approval Limit is when:
amount < max(uint256)

Unlimited is when:
amount == max(uint256)