-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Add ERC3156 extension of ERC20 (flash minting and lending) #2543
Changes from all commits
b263508
6130c24
0a6f007
5f9a11c
d1a156e
e346ef6
ac7d0d9
a0f4762
14f12e7
0a0a6f4
1650488
1cb7ef0
90c774d
b1c1f6c
9178432
3cec88e
d64e6e6
6b9bcdc
67df750
5f7617b
581b23b
3039307
17db268
684f85c
2e61d2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
/** | ||
* @dev Interface of the ERC3156 FlashBorrower, as defined in | ||
* https://eips.ethereum.org/EIPS/eip-3156[ERC-3156]. | ||
*/ | ||
interface IERC3156FlashBorrower { | ||
/** | ||
* @dev Receive a flash loan. | ||
* @param initiator The initiator of the loan. | ||
* @param token The loan currency. | ||
* @param amount The amount of tokens lent. | ||
* @param fee The additional amount of tokens to repay. | ||
* @param data Arbitrary data structure, intended to contain user-defined parameters. | ||
* @return The keccak256 hash of "ERC3156FlashBorrower.onFlashLoan" | ||
*/ | ||
function onFlashLoan( | ||
address initiator, | ||
address token, | ||
uint256 amount, | ||
uint256 fee, | ||
bytes calldata data | ||
) external returns (bytes32); | ||
} | ||
|
||
/** | ||
* @dev Interface of the ERC3156 FlashLender, as defined in | ||
* https://eips.ethereum.org/EIPS/eip-3156[ERC-3156]. | ||
*/ | ||
interface IERC3156FlashLender { | ||
/** | ||
* @dev The amount of currency available to be lended. | ||
* @param token The loan currency. | ||
* @return The amount of `token` that can be borrowed. | ||
*/ | ||
function maxFlashLoan( | ||
address token | ||
) external view returns (uint256); | ||
|
||
/** | ||
* @dev The fee to be charged for a given loan. | ||
* @param token The loan currency. | ||
* @param amount The amount of tokens lent. | ||
* @return The amount of `token` to be charged for the loan, on top of the returned principal. | ||
*/ | ||
function flashFee( | ||
address token, | ||
uint256 amount | ||
) external view returns (uint256); | ||
|
||
/** | ||
* @dev Initiate a flash loan. | ||
* @param receiver The receiver of the tokens in the loan, and the receiver of the callback. | ||
* @param token The loan currency. | ||
* @param amount The amount of tokens lent. | ||
* @param data Arbitrary data structure, intended to contain user-defined parameters. | ||
*/ | ||
function flashLoan( | ||
IERC3156FlashBorrower receiver, | ||
address token, | ||
uint256 amount, | ||
bytes calldata data | ||
) external returns (bool); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
|
||
import "../token/ERC20/IERC20.sol"; | ||
import "../interfaces/IERC3156.sol"; | ||
import "../utils/Address.sol"; | ||
|
||
/** | ||
* @dev WARNING: this IERC3156FlashBorrower mock implementation is for testing purposes ONLY. | ||
* Writing a secure flash lock borrower is not an easy task, and should be done with the utmost care. | ||
* This is not an example of how it should be done, and no pattern present in this mock should be considered secure. | ||
* Following best practices, always have your contract properly audited before using them to manipulate important funds on | ||
* live networks. | ||
*/ | ||
contract ERC3156FlashBorrowerMock is IERC3156FlashBorrower { | ||
bytes32 constant internal RETURN_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan"); | ||
|
||
bool immutable _enableApprove; | ||
bool immutable _enableReturn; | ||
|
||
event BalanceOf(address token, address account, uint256 value); | ||
event TotalSupply(address token, uint256 value); | ||
|
||
constructor(bool enableReturn, bool enableApprove) { | ||
_enableApprove = enableApprove; | ||
_enableReturn = enableReturn; | ||
} | ||
|
||
function onFlashLoan( | ||
address /*initiator*/, | ||
address token, | ||
uint256 amount, | ||
uint256 fee, | ||
bytes calldata data | ||
) public override returns (bytes32) { | ||
require(msg.sender == token); | ||
|
||
emit BalanceOf(token, address(this), IERC20(token).balanceOf(address(this))); | ||
emit TotalSupply(token, IERC20(token).totalSupply()); | ||
|
||
if (data.length > 0) { | ||
// WARNING: This code is for testing purposes only! Do not use. | ||
Address.functionCall(token, data); | ||
Amxx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if (_enableApprove) { | ||
IERC20(token).approve(token, amount + fee); | ||
} | ||
|
||
return _enableReturn ? RETURN_VALUE : bytes32(0); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
|
||
import "../token/ERC20/extensions/draft-ERC20FlashMint.sol"; | ||
|
||
contract ERC20FlashMintMock is ERC20FlashMint { | ||
constructor ( | ||
string memory name, | ||
string memory symbol, | ||
address initialAccount, | ||
uint256 initialBalance | ||
) ERC20(name, symbol) { | ||
_mint(initialAccount, initialBalance); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "../../../interfaces/IERC3156.sol"; | ||
import "../ERC20.sol"; | ||
|
||
/** | ||
* @dev Implementation of the ERC3156 Flash loans extension, as defined in | ||
* https://eips.ethereum.org/EIPS/eip-3156[ERC-3156]. | ||
* | ||
* Adds the {flashLoan} method, which provides flash loan support at the token | ||
* level. By default there is no fee, but this can be changed by overriding {flashFee}. | ||
*/ | ||
abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { | ||
bytes32 constant private RETURN_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan"); | ||
|
||
/** | ||
* @dev Returns the maximum amount of tokens available for loan. | ||
* @param token The address of the token that is requested. | ||
* @return The amont of token that can be loaned. | ||
*/ | ||
function maxFlashLoan(address token) public view override returns (uint256) { | ||
return token == address(this) ? type(uint256).max - ERC20.totalSupply() : 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have clear reaons to use I'm not saying we should change anything but I'd like to understand the reasoning, and maybe it would be a good idea to document it in a comment here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was under the impression that, since But If you'd rather use the overloaded version I have no strong argument. (worst case scenario some flashMint call revert, but that would not break the ERC20 token. |
||
} | ||
|
||
/** | ||
* @dev Returns the fee applied when doing flash loans. By default this | ||
* implementation has 0 fees. This function can be overloaded to make | ||
* the flash loan mechanism deflationary. | ||
* @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) 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; | ||
} | ||
|
||
/** | ||
* @dev Performs a flash loan. New tokens are minted and sent to the | ||
* `receiver`, who is required to implement the {IERC3156FlashBorrower} | ||
* interface. By the end of the flash loan, the receiver is expected to own | ||
* amount + fee tokens and have them approved back to the token contract itself so | ||
* they can be burned. | ||
* @param receiver The receiver of the flash loan. Should implement the | ||
* {IERC3156FlashBorrower.onFlashLoan} interface. | ||
* @param token The token to be flash loaned. Only `address(this)` is | ||
* supported. | ||
* @param amount The amount of tokens to be loaned. | ||
* @param data An arbitrary datafield that is passed to the receiver. | ||
* @return `true` is the flash loan was successfull. | ||
*/ | ||
function flashLoan( | ||
IERC3156FlashBorrower receiver, | ||
address token, | ||
uint256 amount, | ||
bytes calldata data | ||
) | ||
public virtual override returns (bool) | ||
{ | ||
uint256 fee = flashFee(token, amount); | ||
_mint(address(receiver), amount); | ||
require(receiver.onFlashLoan(msg.sender, token, amount, fee, data) == RETURN_VALUE, "ERC20FlashMint: invalid return value"); | ||
uint256 currentAllowance = allowance(address(receiver), address(this)); | ||
require(currentAllowance >= amount + fee, "ERC20FlashMint: allowance does not allow refund"); | ||
_approve(address(receiver), address(this), currentAllowance - amount - fee); | ||
_burn(address(receiver), amount + fee); | ||
return true; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
/* eslint-disable */ | ||
|
||
const { BN, constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers'); | ||
const { expect } = require('chai'); | ||
const { MAX_UINT256, ZERO_ADDRESS, ZERO_BYTES32 } = constants; | ||
|
||
const ERC20FlashMintMock = artifacts.require('ERC20FlashMintMock'); | ||
const ERC3156FlashBorrowerMock = artifacts.require('ERC3156FlashBorrowerMock'); | ||
|
||
contract('ERC20FlashMint', function (accounts) { | ||
const [ initialHolder, other ] = accounts; | ||
|
||
const name = 'My Token'; | ||
const symbol = 'MTKN'; | ||
|
||
const initialSupply = new BN(100); | ||
const loanAmount = new BN(10000000000000); | ||
|
||
beforeEach(async function () { | ||
this.token = await ERC20FlashMintMock.new(name, symbol, initialHolder, initialSupply); | ||
}); | ||
|
||
describe('maxFlashLoan', function () { | ||
it('token match', async function () { | ||
expect(await this.token.maxFlashLoan(this.token.address)).to.be.bignumber.equal(MAX_UINT256.sub(initialSupply)); | ||
}); | ||
|
||
it('token mismatch', async function () { | ||
expect(await this.token.maxFlashLoan(ZERO_ADDRESS)).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'); | ||
}); | ||
}); | ||
|
||
describe('flashLoan', function () { | ||
Amxx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
it('success', async function () { | ||
const receiver = await ERC3156FlashBorrowerMock.new(true, true); | ||
const { tx } = await this.token.flashLoan(receiver.address, this.token.address, loanAmount, '0x'); | ||
|
||
expectEvent.inTransaction(tx, this.token, 'Transfer', { from: ZERO_ADDRESS, to: receiver.address, value: loanAmount }); | ||
expectEvent.inTransaction(tx, this.token, 'Transfer', { from: receiver.address, to: ZERO_ADDRESS, value: loanAmount }); | ||
expectEvent.inTransaction(tx, receiver, 'BalanceOf', { token: this.token.address, account: receiver.address, value: loanAmount }); | ||
expectEvent.inTransaction(tx, receiver, 'TotalSupply', { token: this.token.address, value: initialSupply.add(loanAmount) }); | ||
|
||
expect(await this.token.totalSupply()).to.be.bignumber.equal(initialSupply); | ||
expect(await this.token.balanceOf(receiver.address)).to.be.bignumber.equal('0'); | ||
expect(await this.token.allowance(receiver.address, this.token.address)).to.be.bignumber.equal('0'); | ||
}); | ||
|
||
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'), | ||
'ERC20FlashMint: invalid return value', | ||
); | ||
}); | ||
|
||
it ('missing approval', async function () { | ||
const receiver = await ERC3156FlashBorrowerMock.new(true, false); | ||
await expectRevert( | ||
this.token.flashLoan(receiver.address, this.token.address, loanAmount, '0x'), | ||
'ERC20FlashMint: allowance does not allow refund', | ||
); | ||
}); | ||
|
||
it ('unavailable funds', async function () { | ||
const receiver = await ERC3156FlashBorrowerMock.new(true, true); | ||
const data = this.token.contract.methods.transfer(other, 10).encodeABI(); | ||
await expectRevert( | ||
this.token.flashLoan(receiver.address, this.token.address, loanAmount, data), | ||
'ERC20: burn amount exceeds balance', | ||
); | ||
}); | ||
|
||
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. | ||
await expectRevert.unspecified(this.token.flashLoan(receiver.address, this.token.address, MAX_UINT256, data)); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is useless if you don't verify that you trust msg.sender.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a mock, its not designed to be used outside our testing suit anyway.
The argument used to be that doing a functionCall with arbitrary data to an arbitrary address (line 38) is bad, and we should not show that. Someone could use this endpoint with an arbitrary token address, 0 amount, 0 fee, and an arbitrary payload, which could transfer token held by the receiver.
I "fixed" that by saying that (within our testsuite) only the token should advertize onFlashLoan ... which is not how the ERC might me used in the broader sens, but corresponds to the usage we push in this example.
TLDR: I disagree, it prevents line 38 being used to do arbitrary calls to arbitrary address ... anyone using this method to do anything nasty will only be able to do it to themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm concerned about is that coding flash borrowers that are safe is not trivial. Coders using the OZ flash lending implementation might come into the OZ repo to see how you did it, and they will find an unsafe implementation of a flash borrower.
Going that extra mile and making sure that your flash borrower is a decent example on how to code one would be useful for the community.
My mock flash borrower is not great itself, but it could quickly be polished up, and I was able to fully test the reference implementation with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I share these concerns. I think the mock should be either absurdly wrong or have a huge warning, or it should be correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hear your concern, and I do agree we should have extensive documentation in this source code saying this is just for testing, and should not be considered a good example of how to do FlashBorrower.
That being said, I'm having a hard time to see which security issues are not covered by the
require(msg.sender == token)
check. Do you have anything in mind ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, you might be right. I can't think of any scenario where
require (token == msg.sender)
let's a malicious lender through. It might be protection enough for a borrower that only accepts loans from flash minters, and not from generic flash lenders.A bit of weird scenario to discriminate lenders on their internal implementation, but maybe not unsafe.