From ee4810f2cc76b801029a256d0266f6a44c92d4f4 Mon Sep 17 00:00:00 2001 From: nirban256 Date: Fri, 15 Jul 2022 14:32:50 +0530 Subject: [PATCH 01/10] added internal _flashFee without validation to ERC20FlashMint --- .../token/ERC20/extensions/ERC20FlashMint.sol | 13 ++++- .../ERC20/extensions/ERC20FlashMint.test.js | 47 ++++++++++--------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC20FlashMint.sol b/contracts/token/ERC20/extensions/ERC20FlashMint.sol index 38dbc3e2bf1..28304d2c94b 100644 --- a/contracts/token/ERC20/extensions/ERC20FlashMint.sol +++ b/contracts/token/ERC20/extensions/ERC20FlashMint.sol @@ -43,6 +43,17 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { return 0; } + /** + * @dev Returns the fee applied when doing flash loans. This function calls the {flashFee} function which returns the fee applied when doing flash loans. + * @param token The token to be flash loaned. + * @param amount The amount of tokens to be loaned. + * @return The fees applied to the corresponding flash loan. + */ + function _flashFee(address token, uint256 amount) internal view virtual returns (uint256) { + uint256 fee = flashFee(token, amount); + return fee; + } + /** * @dev Returns the receiver address of the flash fee. By default this * implementation returns the address(0) which means the fee amount will be burnt. @@ -77,7 +88,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { bytes calldata data ) public virtual override returns (bool) { require(amount <= maxFlashLoan(token), "ERC20FlashMint: amount exceeds maxFlashLoan"); - uint256 fee = flashFee(token, amount); + uint256 fee = _flashFee(token, amount); _mint(address(receiver), amount); require( receiver.onFlashLoan(msg.sender, token, amount, fee, data) == _RETURN_VALUE, diff --git a/test/token/ERC20/extensions/ERC20FlashMint.test.js b/test/token/ERC20/extensions/ERC20FlashMint.test.js index 01c08db598d..ed2a8a05b45 100644 --- a/test/token/ERC20/extensions/ERC20FlashMint.test.js +++ b/test/token/ERC20/extensions/ERC20FlashMint.test.js @@ -8,7 +8,7 @@ const ERC20FlashMintMock = artifacts.require('ERC20FlashMintMock'); const ERC3156FlashBorrowerMock = artifacts.require('ERC3156FlashBorrowerMock'); contract('ERC20FlashMint', function (accounts) { - const [ initialHolder, other, anotherAccount ] = accounts; + const [initialHolder, other, anotherAccount] = accounts; const name = 'My Token'; const symbol = 'MTKN'; @@ -30,13 +30,16 @@ contract('ERC20FlashMint', function (accounts) { }); }); - describe('flashFee', function () { - it('token match', async function () { - expect(await this.token.flashFee(this.token.address, loanAmount)).to.be.bignumber.equal('0'); - }); + describe('_flashFee', function () { - it('token mismatch', async function () { - await expectRevert(this.token.flashFee(ZERO_ADDRESS, loanAmount), 'ERC20FlashMint: wrong token'); + describe('flashFee', function () { + it('token match', async function () { + expect(await this.token.flashFee(this.token.address, loanAmount)).to.be.bignumber.equal('0'); + }); + + it('token mismatch', async function () { + await expectRevert(this.token.flashFee(ZERO_ADDRESS, loanAmount), 'ERC20FlashMint: wrong token'); + }); }); }); @@ -61,7 +64,7 @@ contract('ERC20FlashMint', function (accounts) { expect(await this.token.allowance(receiver.address, this.token.address)).to.be.bignumber.equal('0'); }); - it ('missing return value', async function () { + it('missing return value', async function () { const receiver = await ERC3156FlashBorrowerMock.new(false, true); await expectRevert( this.token.flashLoan(receiver.address, this.token.address, loanAmount, '0x'), @@ -69,7 +72,7 @@ contract('ERC20FlashMint', function (accounts) { ); }); - it ('missing approval', async function () { + it('missing approval', async function () { const receiver = await ERC3156FlashBorrowerMock.new(true, false); await expectRevert( this.token.flashLoan(receiver.address, this.token.address, loanAmount, '0x'), @@ -77,7 +80,7 @@ contract('ERC20FlashMint', function (accounts) { ); }); - it ('unavailable funds', async function () { + it('unavailable funds', async function () { const receiver = await ERC3156FlashBorrowerMock.new(true, true); const data = this.token.contract.methods.transfer(other, 10).encodeABI(); await expectRevert( @@ -86,7 +89,7 @@ contract('ERC20FlashMint', function (accounts) { ); }); - it ('more than maxFlashLoan', async function () { + it('more than maxFlashLoan', async function () { const receiver = await ERC3156FlashBorrowerMock.new(true, true); const data = this.token.contract.methods.transfer(other, 10).encodeABI(); // _mint overflow reverts using a panic code. No reason string. @@ -96,8 +99,8 @@ contract('ERC20FlashMint', function (accounts) { describe('custom flash fee & custom fee receiver', function () { const receiverInitialBalance = new BN(200000); const flashFee = new BN(5000); - - beforeEach('init reciever balance & set flash fee',async function () { + + beforeEach('init reciever balance & set flash fee', async function () { this.receiver = await ERC3156FlashBorrowerMock.new(true, true); const receipt = await this.token.mint(this.receiver.address, receiverInitialBalance); await expectEvent(receipt, 'Transfer', { from: ZERO_ADDRESS, to: this.receiver.address, value: receiverInitialBalance }); @@ -106,13 +109,13 @@ contract('ERC20FlashMint', function (accounts) { await this.token.setFlashFee(flashFee); expect(await this.token.flashFee(this.token.address, loanAmount)).to.be.bignumber.equal(flashFee); }); - + it('default flash fee receiver', async function () { const { tx } = await this.token.flashLoan(this.receiver.address, this.token.address, loanAmount, '0x'); await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: ZERO_ADDRESS, to: this.receiver.address, value: loanAmount }); - await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.receiver.address, to: ZERO_ADDRESS, value: loanAmount.add (flashFee)}); - await expectEvent.inTransaction(tx, this.receiver, 'BalanceOf', { token: this.token.address, account: this.receiver.address, value: receiverInitialBalance.add(loanAmount) }); - await expectEvent.inTransaction(tx, this.receiver, 'TotalSupply', { token: this.token.address, value: initialSupply.add (receiverInitialBalance).add(loanAmount) }); + await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.receiver.address, to: ZERO_ADDRESS, value: loanAmount.add(flashFee) }); + await expectEvent.inTransaction(tx, this.receiver, 'BalanceOf', { token: this.token.address, account: this.receiver.address, value: receiverInitialBalance.add(loanAmount) }); + await expectEvent.inTransaction(tx, this.receiver, 'TotalSupply', { token: this.token.address, value: initialSupply.add(receiverInitialBalance).add(loanAmount) }); expect(await this.token.totalSupply()).to.be.bignumber.equal(initialSupply.add(receiverInitialBalance).sub(flashFee)); expect(await this.token.balanceOf(this.receiver.address)).to.be.bignumber.equal(receiverInitialBalance.sub(flashFee)); @@ -124,15 +127,15 @@ contract('ERC20FlashMint', function (accounts) { const flashFeeReceiverAddress = anotherAccount; await this.token.setFlashFeeReceiver(flashFeeReceiverAddress); expect(await this.token.flashFeeReceiver()).to.be.eq(flashFeeReceiverAddress); - + expect(await this.token.balanceOf(flashFeeReceiverAddress)).to.be.bignumber.equal('0'); - + const { tx } = await this.token.flashLoan(this.receiver.address, this.token.address, loanAmount, '0x'); await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: ZERO_ADDRESS, to: this.receiver.address, value: loanAmount }); await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.receiver.address, to: ZERO_ADDRESS, value: loanAmount }); - await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.receiver.address, to: flashFeeReceiverAddress, value: flashFee }); - await expectEvent.inTransaction(tx, this.receiver, 'BalanceOf', { token: this.token.address, account: this.receiver.address, value: receiverInitialBalance.add(loanAmount) }); - await expectEvent.inTransaction(tx, this.receiver, 'TotalSupply', { token: this.token.address, value: initialSupply.add (receiverInitialBalance).add(loanAmount) }); + await expectEvent.inTransaction(tx, this.token, 'Transfer', { from: this.receiver.address, to: flashFeeReceiverAddress, value: flashFee }); + await expectEvent.inTransaction(tx, this.receiver, 'BalanceOf', { token: this.token.address, account: this.receiver.address, value: receiverInitialBalance.add(loanAmount) }); + await expectEvent.inTransaction(tx, this.receiver, 'TotalSupply', { token: this.token.address, value: initialSupply.add(receiverInitialBalance).add(loanAmount) }); expect(await this.token.totalSupply()).to.be.bignumber.equal(initialSupply.add(receiverInitialBalance)); expect(await this.token.balanceOf(this.receiver.address)).to.be.bignumber.equal(receiverInitialBalance.sub(flashFee)); From 9ca9f69734571cff202d4d85037150430315cb6d Mon Sep 17 00:00:00 2001 From: nirban256 Date: Fri, 15 Jul 2022 15:14:32 +0530 Subject: [PATCH 02/10] made changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06409f694c0..a7e2afd303f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * `GovernorCompatibilityBravo`: remove unused `using` statements ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506)) * `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513)) * `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481)) + * `ERC20FlashMint`: add `_flashFee' function for calling flashFee inside it. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) ### Compatibility Note From c6f992e18cd82928984aa4a675fb2d0f73475de4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 15 Jul 2022 13:43:53 +0200 Subject: [PATCH 03/10] fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7e2afd303f..215b5818c7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ * `GovernorCompatibilityBravo`: remove unused `using` statements ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506)) * `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513)) * `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481)) - * `ERC20FlashMint`: add `_flashFee' function for calling flashFee inside it. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) + * `ERC20FlashMint`: add `_flashFee` function for calling flashFee inside it. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) ### Compatibility Note From 9fbebbe014115365c08adc0395d43e75a7fd82f3 Mon Sep 17 00:00:00 2001 From: nirban256 Date: Fri, 15 Jul 2022 21:50:12 +0530 Subject: [PATCH 04/10] made the specified changes --- .../token/ERC20/extensions/ERC20FlashMint.sol | 12 ++++++------ .../token/ERC20/extensions/ERC20FlashMint.test.js | 15 ++++++--------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC20FlashMint.sol b/contracts/token/ERC20/extensions/ERC20FlashMint.sol index 28304d2c94b..a835f123621 100644 --- a/contracts/token/ERC20/extensions/ERC20FlashMint.sol +++ b/contracts/token/ERC20/extensions/ERC20FlashMint.sol @@ -38,9 +38,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { */ function flashFee(address token, uint256 amount) public view virtual override returns (uint256) { require(token == address(this), "ERC20FlashMint: wrong token"); - // silence warning about unused variable without the addition of bytecode. - amount; - return 0; + return _flashFee(token, amount); } /** @@ -50,8 +48,10 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { * @return The fees applied to the corresponding flash loan. */ function _flashFee(address token, uint256 amount) internal view virtual returns (uint256) { - uint256 fee = flashFee(token, amount); - return fee; + // silence warning about unused variable without the addition of bytecode. + token; + amount; + return 0; } /** @@ -88,7 +88,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { bytes calldata data ) public virtual override returns (bool) { require(amount <= maxFlashLoan(token), "ERC20FlashMint: amount exceeds maxFlashLoan"); - uint256 fee = _flashFee(token, amount); + uint256 fee = flashFee(token, amount); _mint(address(receiver), amount); require( receiver.onFlashLoan(msg.sender, token, amount, fee, data) == _RETURN_VALUE, diff --git a/test/token/ERC20/extensions/ERC20FlashMint.test.js b/test/token/ERC20/extensions/ERC20FlashMint.test.js index ed2a8a05b45..ca3ea407fb1 100644 --- a/test/token/ERC20/extensions/ERC20FlashMint.test.js +++ b/test/token/ERC20/extensions/ERC20FlashMint.test.js @@ -30,16 +30,13 @@ contract('ERC20FlashMint', function (accounts) { }); }); - describe('_flashFee', function () { - - describe('flashFee', function () { - it('token match', async function () { - expect(await this.token.flashFee(this.token.address, loanAmount)).to.be.bignumber.equal('0'); - }); + describe('flashFee', function () { + it('token match', async function () { + expect(await this.token.flashFee(this.token.address, loanAmount)).to.be.bignumber.equal('0'); + }); - it('token mismatch', async function () { - await expectRevert(this.token.flashFee(ZERO_ADDRESS, loanAmount), 'ERC20FlashMint: wrong token'); - }); + it('token mismatch', async function () { + await expectRevert(this.token.flashFee(ZERO_ADDRESS, loanAmount), 'ERC20FlashMint: wrong token'); }); }); From 6acd24faa39dfb0892edcc718a40ec4bbd364f22 Mon Sep 17 00:00:00 2001 From: nirban256 Date: Fri, 15 Jul 2022 23:24:22 +0530 Subject: [PATCH 05/10] added the correct changelog statement --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 215b5818c7b..7eec320b9fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ * `GovernorCompatibilityBravo`: remove unused `using` statements ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506)) * `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513)) * `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481)) - * `ERC20FlashMint`: add `_flashFee` function for calling flashFee inside it. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) + * `ERC20FlashMint`: add `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) ### Compatibility Note From ad495f71db931ae4688569f5c40313b732d4c062 Mon Sep 17 00:00:00 2001 From: nirban256 Date: Sat, 16 Jul 2022 10:12:19 +0530 Subject: [PATCH 06/10] added internal word in the suggested position --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e97496f0b2..eb09b71567a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ * `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481)) * `ERC721`: optimize burn by making approval clearing implicit instead of emitting an event. ([#3538](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3538)) * `ReentrancyGuard`: Reduce code size impact of the modifier by using internal functions. ([#3515](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3515)) - * `ERC20FlashMint`: add `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) + * `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) ### Compatibility Note From 020e729cba5e6ba88d4680350a157eb272efbc22 Mon Sep 17 00:00:00 2001 From: nirban256 Date: Thu, 21 Jul 2022 11:58:23 +0530 Subject: [PATCH 07/10] changed the flashFee to _flashFee function --- contracts/mocks/ERC20FlashMintMock.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/mocks/ERC20FlashMintMock.sol b/contracts/mocks/ERC20FlashMintMock.sol index c7772601b79..10d2738ae57 100644 --- a/contracts/mocks/ERC20FlashMintMock.sol +++ b/contracts/mocks/ERC20FlashMintMock.sol @@ -25,8 +25,8 @@ contract ERC20FlashMintMock is ERC20FlashMint { _flashFeeAmount = amount; } - function flashFee(address token, uint256 amount) public view virtual override returns (uint256) { - super.flashFee(token, amount); + function _flashFee(address token, uint256 amount) internal view override returns (uint256) { + super._flashFee(token, amount); return _flashFeeAmount; } From f2aaf1967d3634e79ad096d42b4f85d5c1e1b807 Mon Sep 17 00:00:00 2001 From: Francisco Date: Thu, 21 Jul 2022 12:53:15 -0300 Subject: [PATCH 08/10] Update ERC20FlashMintMock.sol --- contracts/mocks/ERC20FlashMintMock.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/mocks/ERC20FlashMintMock.sol b/contracts/mocks/ERC20FlashMintMock.sol index 10d2738ae57..c97a7f46c2b 100644 --- a/contracts/mocks/ERC20FlashMintMock.sol +++ b/contracts/mocks/ERC20FlashMintMock.sol @@ -26,7 +26,6 @@ contract ERC20FlashMintMock is ERC20FlashMint { } function _flashFee(address token, uint256 amount) internal view override returns (uint256) { - super._flashFee(token, amount); return _flashFeeAmount; } From a6b56b427d50d2ee2dd5ffc52b1349450170b22d Mon Sep 17 00:00:00 2001 From: Francisco Date: Fri, 22 Jul 2022 13:41:31 -0300 Subject: [PATCH 09/10] Update ERC20FlashMintMock.sol --- contracts/mocks/ERC20FlashMintMock.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/mocks/ERC20FlashMintMock.sol b/contracts/mocks/ERC20FlashMintMock.sol index c97a7f46c2b..8c994f85a12 100644 --- a/contracts/mocks/ERC20FlashMintMock.sol +++ b/contracts/mocks/ERC20FlashMintMock.sol @@ -26,6 +26,8 @@ contract ERC20FlashMintMock is ERC20FlashMint { } function _flashFee(address token, uint256 amount) internal view override returns (uint256) { + token; + amount; return _flashFeeAmount; } From fa417841852707eba894383d1d2171c6f4705581 Mon Sep 17 00:00:00 2001 From: Francisco Date: Fri, 22 Jul 2022 13:42:02 -0300 Subject: [PATCH 10/10] Update ERC20FlashMintMock.sol --- contracts/mocks/ERC20FlashMintMock.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/mocks/ERC20FlashMintMock.sol b/contracts/mocks/ERC20FlashMintMock.sol index 8c994f85a12..ff6f252e806 100644 --- a/contracts/mocks/ERC20FlashMintMock.sol +++ b/contracts/mocks/ERC20FlashMintMock.sol @@ -25,9 +25,7 @@ contract ERC20FlashMintMock is ERC20FlashMint { _flashFeeAmount = amount; } - function _flashFee(address token, uint256 amount) internal view override returns (uint256) { - token; - amount; + function _flashFee(address, uint256) internal view override returns (uint256) { return _flashFeeAmount; }