CW20: Remove IncreaseAllowance / DecreaseAllowance

[miohtama]

(Not sure if this is the right place to leave feedback on the CW20 token standard, but none of my friends in the Cosmos ecosystem could know better.)

CW20 tokens have IncreaseAllowance and DecreaseAllowance logic copied from ERC-20. Originally not part of the ERC-20 spec, these functions were added afterwards. They do not serve a meaningful purpose and are mostly security theatre.

The origin of these functions is in the questionable discussion of security researchers claiming for a security vulnerability exists where such does not really exist. Such snake-oily behaviour was common during the early Ethereum smart contract years 2017/2018, because security researchers could get reputation and extra audit work by making blog headlines with posts like "all ERC-20 tokens have a vulnerability." Namely, the "race condition" explanation behind these calls does not make any sense as was created there just someone to make name for themselves as security researchers. Because the people pushing these calls were noisy and annoying, the rest of the ecosystem ended up adopting them, instead of tolerating the constant yelling.

Anyone who thinks logically will see through this security issue fallacy

Scenario A)

• A user has given a token allowance for someone and this someone can do whatever with the allowed tokens, like steal them

Scenario B)

• A user has given a token allowance for someone
• Because of the "race condition", someone can see the token allowance is about to change in a new message and steals the tokens

Scenario B is not a real threat scenario, because if you give someone an allowance and they want to steal your tokens, they will do it the right way. They are not going to wait for you to reissue approve() call.

Because CosmWasm is a new high-quality ecosystem, I would recommend CosmWasm not to copy increase/decrease allowance baggage from the past, as its only purpose has been serving as a security theatre.

• These messages do not serve any meaningful purpose
• They make the CW20 unnecessarily complicated
• Having any token methods outside send() and transfer() confuse end user leading them being scammed

[uint]

Anyone who thinks logically will see through this security issue fallacy

That's a bit of an absolute.

Please note we don't have anything like approve. We only have the increase/decrease calls for mutating the allowance.

I don't think we want to break this API at this stage.

[miohtama]

The solution discussed in the Ethereum community was an IncreaseAllowance and DecreaseAllowance operator (instead of Approve). To originally set an approval, use IncreaseAllowance, which works fine with no previous allowance. DecreaseAllowance is meant to be robust, that is if you decrease by more than the current allowance (eg. the user spent some in the middle), it will just round down to 0 and not make any underflow error.

I don't think there is any kind of race condition which would be possible with just a simple one SetAllowance message. Or if there is some sort of scenario where because of the security two different messages are needed, it is not discussed in the spec.

Thus, the current spec seems to be cargo culting ERC-20 and adding extra complexity when extra complexity is not warranted. If there is a specific reason for IncreaseAllowance / DecreaseAllowance it is not obvious from the spec. But it is clear that ERC-20 increaseAllowance and decreaseAllowance was an unnecessary complication with no real value.

The downside of adding extra messages and complexity for the token spec is that

• There is no point to add extra complexity when extra complexity is not warranted, as it is not good architectural design

[nbaum]

I don't think there is any kind of race condition which would be possible with just a simple one SetAllowance message.

There are really two messages: GetAllowance and SetAllowance.

If I want to increase your allowance by N tokens, the race condition exists between checking your current allowance and setting the new allowance.

1. I set your allowance to 1,000.
2. I decide to give you an extra 2,000, so I check to see your current allowance: it's 1,000.
3. You spend 1,000.
4. I set your allowance to 3,000.
5. Now you can spend an additional 3,000 - I've accidentally given you 4,000.

You could automate this by watching mempool for my message 4 and trying to frontrun your spend. If you win, you get free money. If you lose, you simply spend some of the new allowance.

With increase/decrease allowance:

1. I increase your allowance (from zero) by 1,000.
2. I decide to give you an extra 2,000, so I increase your allowance by 2,000.
3. Now you can spend 3,000.

Whether or not you try to frontrun me, the total amount I give you is never more than 3,000.