Solidity ERC-20 DeFi ERC-20 Approve – Current Best Practices for ERC20 Approval in Solidity

defierc-20erc-20-approvesolidity

I've been reading some (old) discussions about the potential attack vector of using ERC20 approve (for example: https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729) but I've been noticing that in practice, most DeFi apps (like the 1inch router or the pancake swap router for example) simply have the user do something like this

approve(RouterAddress, MAX_UINT);

instead of following the flow that seems to be recommended, e.g.

approve(RouterAddress, 0);
approve(RouterAddress, required_amount);

I'm wondering why this practice is seen as ok.

I personally think this is the better way, and since the approval is for a routing contract, (in my mind) there is essentially zero risk of this attack vector. Not to mention it's way less cumbersome for actual users. But these are old discussions, and I'm wondering what the current thoughts about this issue are from solidity devs.

My main question is this: theoretically, if I had a my own contract holding $100 million WETH (or some other ERC20 token), is it really okay to say something like:

approve(RouterAddress, MAX_UINT);

I understand for the average user, approving infinite amounts is probably not a huge deal, but what about a contract that was holding a sizable amount of money. Is it still safe? Is there any threat of this attack vector if the approval is for another contract itself? Any thoughts are greatly appreciated 🙂

Best Answer

IMO it is, as long as you trust the contract you approve to be good. limited allowances are definitely safer (in the case an exploit would be discovered somehow that would allow a malicious actor to call transferFrom through a contract to its wallet. But that seems highly unlikely) but they are more cumbersome as you said. However when i'm writing code that needs to use approve i usually approve the exact amount i need to transfer each time as it only costs a few units of gas more on each call, isnt harder to implement and doesnt change anything on the user side.

Related Topic