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

Update to newer version of openzeppelin #438

Open
PatrickAlphaC opened this issue May 2, 2024 · 2 comments
Open

Update to newer version of openzeppelin #438

PatrickAlphaC opened this issue May 2, 2024 · 2 comments

Comments

@PatrickAlphaC
Copy link

safeApprove is deprecated, use safeIncreaseAllowance instead.

@vladbochok
Copy link
Member

Thanks @PatrickAlphaC. This is correct, but forceApprove may be better our case, wdyt?

In the only usage of safeApprove, the intention is to approve to 0, before actual approve to support USDT like tokens. This can be achieved by using forceApprove instead.

@PatrickAlphaC
Copy link
Author

You introduce a race condition if you always use forceApprove. Here is the scenario:

  1. Bob forceApprove Sally to spend 10 tokens.
  2. Bob decides he actually wants her to be able to spend 15 instead of 10, so sends another forceApprove transaction for 15 tokens.
  3. Before Bob's transaction lands, Sally notices this, and front-runs the transaction, spends the 10 tokens. Now Bob's forceApprove transaction hits for an additional 15.
  4. Sally is able to spend 25 tokens instead of the 15 she was supposed to spend.

If instead, Bob used safeIncreaseAllowance, he would have increased the allowance by 5, so Sally only would have ever been able to spend the 15 tokens Bob originally intended.

Additionally, the new openzeppelin contracts use forceApprove inside of the safeIncreaseAllowance function anyways.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/53b5d84212b73388a2a7096ff503693dae692726/contracts/token/ERC20/utils/SafeERC20.sol#L52

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

No branches or pull requests

2 participants