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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep ERC-20 balances and approval slots non-zero #4947

Open
CodeSandwich opened this issue Mar 8, 2024 · 2 comments
Open

Keep ERC-20 balances and approval slots non-zero #4947

CodeSandwich opened this issue Mar 8, 2024 · 2 comments
Labels

Comments

@CodeSandwich
Copy link
Contributor

CodeSandwich commented Mar 8, 2024

馃 Motivation

The ERC-20 implementation uses a whole storage slot for the account balance and a whole slot for each account-spender approval. It's simple and elegant, but in some cases it's gas inefficient. Whenever a balance is fully spent or an approval fully used, the storage slot is zeroed, which triggers a gas refund. The refunds is much lower than the cost of making the slot non-zero when the balance goes up again or a new approval is set up, and the refunds may even be completely removed in the future, for example on Ethereum. This is especially wasteful if you want to approve a 3rd party to transfer an exact amount, or when a smart contract is transferring its entire balance to another contract or to the user.

馃摑 Details

The storage slots for the balances and approvals may be kept non-zero by always setting the highest bit to 1 and ignoring it when reading the value. For example:

    uint256 private constant _MASK = type(uint256).max >> 1; 

    function balanceOf(address account) public view virtual returns (uint256) {
        return _balances[account]  & _MASK;
    }

and

    function _update(address from, address to, uint256 value) internal virtual {
(...)
        _balances[from] = fromBalance - value | ~_MASK;

The nice part is that this is fully encapsulated because the balances and the approvals storages are both private. It's also backward compatible for the most part, it doesn't matter if the mask is already applied on the values in the storage, the code will always work the same.

The ugly part is that the slots become limited to storing values of up to 2^255-1, and the ERC-20 standard doesn't support such limitation. In the real world there doesn't seem to be any widely used token needing all 256 bits to keep the balances, but some checks would need to be done in the minting function. The approvals would become even more detached from the standard because now setting any approval above 2^255 would be interpreted as setting it to 2^256-1, which means an infinite approval. This too probably would have limited real-world implications.

An alternative

A non-backward-compatible alternative would be to apply on the stored values not a mask, but an offset of +1. Values 0 and 1 would be interpreted as 0, then 2 as 1, 3 as 2 and so on. This would broaden the maximum balances and approvals to 2^256-2.

@Amxx Amxx added the idea label Mar 13, 2024
@Amxx
Copy link
Collaborator

Amxx commented Mar 13, 2024

Thank you @CodeSandwich for this idea.

Backward compatibility is a must for us (because of the upgradeable version). Its good that you approach provides that feature! This limitation of balances to 2**255-1 is an issue though. We will want to avoid silent errors that may arrive if balances overflow into the mask. That will have to be checked ... and checks add gas cost for everyone ...

I'm curious about the gas cost increase from loading and applying the mask. There is a minority cases where that indeed will save cost, but there is also a majority of cases where that will slightly increasse the costs as well.

I'll try to find banwidth to research that

@CodeSandwich
Copy link
Contributor Author

CodeSandwich commented Mar 13, 2024

I'm curious about the gas cost increase from loading and applying the mask. There is a minority cases where that indeed will save cost, but there is also a majority of cases where that will slightly increasse the costs as well.

I don't think that you should be worried about the gas cost and savings balance. The mask is a constant, so it's loaded by pushing a value to the stack, with the binary operation it will probably cost less than 10 gas in total. The savings on the other hand is 17,100 gas when the storage slot doesn't need to be set from 0 to non-0. This isn't an uncommon case for ERC-20 to drain a balance or an approval to zero, and then make it non-zero again.

This limitation of balances to 2**255-1 is an issue though. We will want to avoid silent errors that may arrive if balances overflow into the mask. That will have to be checked ... and checks add gas cost for everyone ...

I agree that this is an issue, and it may affect the consumers of OZ. I don't think that gas usage is a problem though, at least not on the OZ level. The only place where a new check is needed is in _update, where _totalSupply += value; is performed. The compiler already catches values larger than 2^256-1, but now we would need to catch values larger than 2^255-1. The nice side effect is that we would start emitting an explicit error when somebody mints too much, instead of the cryptic overflow panic. (It may be worth implementing regardless of this gas improvement)

Thanks for looking into it!

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

2 participants