From a3b6035f89199c7f79de990fe8e57886a94cf45e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 17 May 2023 21:31:58 +0200 Subject: [PATCH 1/5] use forceApprove in safeIncreaseAllowance and safeDecreaseAllowance --- contracts/token/ERC20/utils/SafeERC20.sol | 4 ++-- test/token/ERC20/utils/SafeERC20.test.js | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 805a1565f14..23c42edebf5 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -59,7 +59,7 @@ library SafeERC20 { */ function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal { uint256 oldAllowance = token.allowance(address(this), spender); - _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, oldAllowance + value)); + forceApprove(token, spender, oldAllowance + value); } /** @@ -70,7 +70,7 @@ library SafeERC20 { unchecked { uint256 oldAllowance = token.allowance(address(this), spender); require(oldAllowance >= value, "SafeERC20: decreased allowance below zero"); - _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, oldAllowance - value)); + forceApprove(token, spender, oldAllowance - value); } } diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js index d4981dd30cc..789c1d70c75 100644 --- a/test/token/ERC20/utils/SafeERC20.test.js +++ b/test/token/ERC20/utils/SafeERC20.test.js @@ -192,17 +192,20 @@ contract('SafeERC20', function (accounts) { it('safeApprove can update approval to zero', async function () { await this.mock.$safeApprove(this.token.address, spender, 0); }); - - it('safeApprove can increase approval', async function () { - await expectRevert(this.mock.$safeIncreaseAllowance(this.token.address, spender, 10), 'USDT approval failure'); + + it('safeIncreaseAllowance works', async function () { + this.mock.$safeIncreaseAllowance(this.token.address, spender, 10); + expect(this.token.allowance(this.mock.address, spender, 90)); }); - it('safeApprove can decrease approval', async function () { - await expectRevert(this.mock.$safeDecreaseAllowance(this.token.address, spender, 10), 'USDT approval failure'); + it('safeDecreaseAllowance works', async function () { + this.mock.$safeDecreaseAllowance(this.token.address, spender, 10); + expect(this.token.allowance(this.mock.address, spender, 110)); }); it('forceApprove works', async function () { await this.mock.$forceApprove(this.token.address, spender, 200); + expect(this.token.allowance(this.mock.address, spender, 200)); }); }); }); From f995c849979f5f76181377c090175c82c4febd2d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 May 2023 09:56:43 +0200 Subject: [PATCH 2/5] fix lint --- test/token/ERC20/utils/SafeERC20.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js index 789c1d70c75..35bc780ba99 100644 --- a/test/token/ERC20/utils/SafeERC20.test.js +++ b/test/token/ERC20/utils/SafeERC20.test.js @@ -192,7 +192,7 @@ contract('SafeERC20', function (accounts) { it('safeApprove can update approval to zero', async function () { await this.mock.$safeApprove(this.token.address, spender, 0); }); - + it('safeIncreaseAllowance works', async function () { this.mock.$safeIncreaseAllowance(this.token.address, spender, 10); expect(this.token.allowance(this.mock.address, spender, 90)); From a7dd6b4a16553fc01832a4439387c79e8e0a88fc Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 19 May 2023 14:51:00 +0200 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Francisco --- test/token/ERC20/utils/SafeERC20.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js index 35bc780ba99..1f68046e220 100644 --- a/test/token/ERC20/utils/SafeERC20.test.js +++ b/test/token/ERC20/utils/SafeERC20.test.js @@ -194,12 +194,12 @@ contract('SafeERC20', function (accounts) { }); it('safeIncreaseAllowance works', async function () { - this.mock.$safeIncreaseAllowance(this.token.address, spender, 10); + await this.mock.$safeIncreaseAllowance(this.token.address, spender, 10); expect(this.token.allowance(this.mock.address, spender, 90)); }); it('safeDecreaseAllowance works', async function () { - this.mock.$safeDecreaseAllowance(this.token.address, spender, 10); + await this.mock.$safeDecreaseAllowance(this.token.address, spender, 10); expect(this.token.allowance(this.mock.address, spender, 110)); }); From 8319d86d66a027835da5f87160960686eb54854c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 19 May 2023 15:35:15 +0200 Subject: [PATCH 4/5] add changeset --- .changeset/wild-windows-trade.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/wild-windows-trade.md diff --git a/.changeset/wild-windows-trade.md b/.changeset/wild-windows-trade.md new file mode 100644 index 00000000000..c6aa863f78d --- /dev/null +++ b/.changeset/wild-windows-trade.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +Refactor SafeERC20 safeDecreaseAllowance and safeIncreaseAllowance to support USDT-like approval. From b46a61613a34d2722e2f06fa4ef942f200d69881 Mon Sep 17 00:00:00 2001 From: Francisco Date: Fri, 19 May 2023 14:12:35 -0300 Subject: [PATCH 5/5] Update wild-windows-trade.md --- .changeset/wild-windows-trade.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/wild-windows-trade.md b/.changeset/wild-windows-trade.md index c6aa863f78d..f599d0fcbbc 100644 --- a/.changeset/wild-windows-trade.md +++ b/.changeset/wild-windows-trade.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': major --- -Refactor SafeERC20 safeDecreaseAllowance and safeIncreaseAllowance to support USDT-like approval. +`SafeERC20`: Refactor `safeDecreaseAllowance` and `safeIncreaseAllowance` to support USDT-like tokens.