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

Unlimited ERC20 token allowance #717

Closed
abandeali1 opened this issue Sep 20, 2017 · 19 comments
Closed

Unlimited ERC20 token allowance #717

abandeali1 opened this issue Sep 20, 2017 · 19 comments
Labels

Comments

@abandeali1
Copy link

Summary

ERC20 provides a standard API for Ethereum smart contracts implementing tokens. While all ERC20 tokens share the same API, different implementations of the standard functions have been released by various teams. This proposal slightly modifies the implementation of the transferFrom function in a way that allows a user to provide an unlimited allowance, which can provide significant gas savings.

Specification

With this proposal, transferFrom only modifies the allowed mapping if allowed[_from][_spender] is less than the maximum unsigned integer (2 ** 256 - 1).

uint constant MAX_UINT = 2**256 - 1;

/// @dev ERC20 transferFrom, modified such that an allowance of MAX_UINT represents an unlimited allowance.
/// @param _from Address to transfer from.
/// @param _to Address to transfer to.
/// @param _value Amount to transfer.
/// @return Success of transfer.
function transferFrom(address _from, address _to, uint _value)
    public
    returns (bool)
{
    uint allowance = allowed[_from][msg.sender];
    require(balances[_from] >= _value
            && allowance >= _value
            && balances[_to] + _value >= balances[_to]);
    balances[_to] += _value;
    balances[_from] -= _value;
    if (allowance < MAX_UINT) {
        allowed[_from][msg.sender] -= _value;
    }
    Transfer(_from, _to, _value);
    return true;
}

Rationale

There are cases where a token owner is willing to give another address an unlimited allowance. With the current accepted implementation of transferFrom, _value is subtracted from the allowed mapping every single time the function is called. This is an unnecessary (and annoying) state change in the case the token owner wishes to grant an address an unlimited allowance, wasting 5K gas per call of transferFrom. This change is a huge long run efficiency gain and is completely backwards compatible.

I would also like to acknowledge @recmo, who initially proposed this solution.

@MicahZoltu
Copy link
Contributor

Are there any example scenarios where approving 2^256 once is not enough to (pragmatically) last forever? For any token with a total supply on the order of 10^26 (like Ethereum) you would have to transfer the entire token supply (through one account) ~2^169 times.

Are there tokens with such a large supply that it is realistic for a single account to see 2^256 tokens flow through it over some sane period of time?

@abandeali1
Copy link
Author

abandeali1 commented Sep 22, 2017

@MicahZoltu the issue is not that 2^256 is too small. The issue is that transferFrom unnecessarily updates state when a user intends on creating an unlimited allowance, wasting gas.

Basically, 2^256 is so large that it might as well be treated as unlimited.

@MicahZoltu
Copy link
Contributor

Ah, I see. I'm on board then. Basically UINT_MAX is considered a sentinel value for token approval and any token approvals of UINT_MAX SHOULD be interpreted as unlimited approval (to save on gas costs).

@tjayrush
Copy link

Does the standard have anything at all to say about the implementation behind the interface? Isn't it true that one can have the transferFrom function do anything it likes.

@MicahZoltu
Copy link
Contributor

transferFrom

Transfers _value amount of tokens from address _from to address _to, and MUST fire the Transfer event.

The transferFrom method is used for a withdraw workflow, allowing contracts to transfer tokens on your behalf. This can be used for example to allow a contract to transfer tokens on your behalf and/or to charge fees in sub-currencies. The function SHOULD throw unless the _from account has deliberately authorized the sender of the message via some mechanism.

Note Transfers of 0 values MUST be treated as normal transfers and fire the Transfer event.

function transferFrom(address _from, address _to, uint256 _value) returns (bool success)

@MicahZoltu
Copy link
Contributor

It appears that the spec doesn't actually say that transferFrom should deduct from approval, so I suppose that we don't actually need a spec for this. 😄 That being said, I think from a user expectation point of view there is value in being clear/specific.

@tjayrush
Copy link

That's why people have to read the source code.

@phiferd
Copy link
Contributor

phiferd commented Sep 23, 2017

Would it be better to just add another method to StandardToken like approveUnlimited(address _spender) rather than making the semantics of a particular value different than all others? I don't think it needs to be part of the ERC20 standard, although tokens could adopt this type of option (just as they can allow for other custom business logic like freezing) if they want.

Internally, StandardToken/Token could have another field mapping(address => mapping(address => bool)) unlimitedApproval to cover this case explicitly. Then update the change to:

if (!unlimitedApproval[_from][msg.sender]) {
   allowed[_from][msg.sender] -= _value;
}

@Arachnid
Copy link
Contributor

2^256 tokens is effectively unlimited anyway; I don't see much value in introducing a new API call and storage slot for an extra Boolean, personally.

@phiferd
Copy link
Contributor

phiferd commented Sep 24, 2017

Ok -- maybe I'm not clear on the use cases. Is this something a non-technical user might use through MyEtherWallet or another auto-generated UI? Will they have to enter the amount as:

115 792 089 237 316 195 423 570 985 008 687 907 853 269 984 665 640 564 039 457 584 007 913 129 639 935

I would advise against piggy-backing on an existing API and giving particular values special meaning in general. However, if you want to go that route and you don't want to add the extra storage, then at least add the following function with its own documentation:

  function approveUnlimited(address _spender) public returns (bool) {
     approve(_spender, MAX);
  }

Also, I wonder if a better option would be a standard "JointAccount" contract (probably exists already) that would allow a primary user to add authorized users. The JointAccount could hold the tokens and authorized users could spend them. To me, it seems better than putting this special logic in the StandardToken and a JointAccount approach would actually be backward compatible with tokens that have already been deployed. Again, maybe I'm just not clear on the use cases.

@GriffGreen
Copy link

This will save a lot of gas for a lot of people, I think approving an account to take out as much as they want is a very common use case of approve(), thank you so much @abandeali1

@abandeali1
Copy link
Author

@phiferd if someone wanted to set an unlimited allowance with MEW then yes, they would have to enter that amount. Realistically, users will be able to set an unlimited allowance through dApps that abstract this logic away with a simple toggle.

Here is a simplified example used by 0x protocol: traders approve a single proxy contract to transfer funds on their behalf. Traders create an "order" that is signed with their private key. When another trader fills that order, the signature is validated and the proxy contract swaps the tokens of both parties using transferFrom. This type of architecture wouldn't work well with a JointAccount contract.

@maurelian
Copy link
Contributor

My first thought is that I like this. It preserves the existing API (unless the public keyword is added to MAX_UINT), and just improves on the implementation.

Here's my paraphrasing, let me know if inaccurate:

  1. regardless of the total supply of a coin, a user may want to approve another account (probably a contract) to spend their balance without limitation.
  2. This is currently possible, as a user could approve() that contract up to 2**256-1
  3. But with this improvement, every call to transferFrom() will cost about 5K less gas by not bothering to update the allowance.

So really the net benefit is a savings of gas, which IMO is worthwhile.

@3esmit
Copy link
Contributor

3esmit commented Nov 1, 2017

Setting the approval to max value might not be enough. Imagine of an exchange account, it might want allowance unlimited, because if it set to max value in some years it might end up consuming all allowance.

@grosu
Copy link

grosu commented Jan 20, 2018

Has anybody done an empirical amortized analysis of the gas savings? Indeed, you do save gas when you want to approve some account without limitation, but on the other hand you spend gas to evaluate the unnecessary allowance < MAX_UINT condition for all the other accounts.

@fulldecent
Copy link
Contributor

I supporting this proposal to go forward as a standard. This is in widespread use already.

@wjmelements
Copy link
Contributor

The standard has been adopted by TrueUSD. trusttoken/contracts-pre22#118

wighawag referenced this issue in xaya/wchi Feb 19, 2021
Check in the basic contract code for WCHI as a simple ERC-20 token
without any extra features, plus some basic configuration scripts
for Truffle.

No unit tests are included yet.
@github-actions
Copy link

github-actions bot commented Jan 2, 2022

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jan 2, 2022
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

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

No branches or pull requests