From 2f37cc767da96001961c539927783cd72eadd9a3 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 15 Jul 2022 18:28:52 +0200 Subject: [PATCH 01/15] wip --- contracts/mocks/ERC4626Mock.sol | 13 +++++++++++-- contracts/token/ERC20/extensions/ERC4626.sol | 17 +++++++++++++---- test/token/ERC20/extensions/ERC4626.test.js | 4 ++-- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/contracts/mocks/ERC4626Mock.sol b/contracts/mocks/ERC4626Mock.sol index 71cefacaaeb..fb0f1cfbd35 100644 --- a/contracts/mocks/ERC4626Mock.sol +++ b/contracts/mocks/ERC4626Mock.sol @@ -6,11 +6,20 @@ import "../token/ERC20/extensions/ERC4626.sol"; // mock class using ERC20 contract ERC4626Mock is ERC4626 { + uint8 private immutable _decimals; + constructor( IERC20Metadata asset, string memory name, - string memory symbol - ) ERC20(name, symbol) ERC4626(asset) {} + string memory symbol, + uint8 decimaloverride + ) ERC20(name, symbol) ERC4626(asset) { + _decimals = decimaloverride; + } + + function decimals() public view virtual override returns (uint8) { + return _decimals; + } function mockMint(address account, uint256 amount) public { _mint(account, amount); diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index c7702856411..6d52e344b35 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -26,15 +26,24 @@ import "../../../utils/math/Math.sol"; abstract contract ERC4626 is ERC20, IERC4626 { using Math for uint256; - IERC20Metadata private immutable _asset; + IERC20 private immutable _asset; /** * @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC20 or ERC777). */ - constructor(IERC20Metadata asset_) { + constructor(IERC20 asset_) { _asset = asset_; } + /** @dev See {IERC4626-asset}. */ + function decimals() public view virtual override(IERC20Metadata, ERC20) returns (uint8) { + try IERC20Metadata(address(_asset)).decimals() returns (uint8 value) { + return value; + } catch { + return super.decimals(); + } + } + /** @dev See {IERC4626-asset}. */ function asset() public view virtual override returns (address) { return address(_asset); @@ -153,7 +162,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { uint256 supply = totalSupply(); return (assets == 0 || supply == 0) - ? assets.mulDiv(10**decimals(), 10**_asset.decimals(), rounding) + ? assets.mulDiv(10**decimals(), 10**ERC4626.decimals(), rounding) : assets.mulDiv(supply, totalAssets(), rounding); } @@ -164,7 +173,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { uint256 supply = totalSupply(); return (supply == 0) - ? shares.mulDiv(10**_asset.decimals(), 10**decimals(), rounding) + ? shares.mulDiv(10**ERC4626.decimals(), 10**decimals(), rounding) : shares.mulDiv(totalAssets(), supply, rounding); } diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js index 63b995f4001..571f0063ac7 100644 --- a/test/token/ERC20/extensions/ERC4626.test.js +++ b/test/token/ERC20/extensions/ERC4626.test.js @@ -15,7 +15,7 @@ contract('ERC4626', function (accounts) { beforeEach(async function () { this.token = await ERC20DecimalsMock.new(name, symbol, 12); - this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V'); + this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V', 18); await this.token.mint(holder, web3.utils.toWei('100')); await this.token.approve(this.vault.address, constants.MAX_UINT256, { from: holder }); @@ -400,7 +400,7 @@ contract('ERC4626', function (accounts) { it('multiple mint, deposit, redeem & withdrawal', async function () { // test designed with both asset using similar decimals this.token = await ERC20DecimalsMock.new(name, symbol, 18); - this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V'); + this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V', 18); await this.token.mint(user1, 4000); await this.token.mint(user2, 7001); From dd881a2442c2206a83dc4f7c6ed87e549dd3d0a2 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 23 Aug 2022 11:42:55 +0200 Subject: [PATCH 02/15] test decimal inheritance --- contracts/mocks/ERC4626Mock.sol | 27 +++++++++++++-------- test/token/ERC20/extensions/ERC4626.test.js | 14 +++++++++-- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/contracts/mocks/ERC4626Mock.sol b/contracts/mocks/ERC4626Mock.sol index fb0f1cfbd35..92d2cf9bf2d 100644 --- a/contracts/mocks/ERC4626Mock.sol +++ b/contracts/mocks/ERC4626Mock.sol @@ -4,8 +4,23 @@ pragma solidity ^0.8.0; import "../token/ERC20/extensions/ERC4626.sol"; -// mock class using ERC20 contract ERC4626Mock is ERC4626 { + constructor( + IERC20Metadata asset, + string memory name, + string memory symbol + ) ERC20(name, symbol) ERC4626(asset) {} + + function mockMint(address account, uint256 amount) public { + _mint(account, amount); + } + + function mockBurn(address account, uint256 amount) public { + _burn(account, amount); + } +} + +contract ERC4626DecimalMock is ERC4626Mock { uint8 private immutable _decimals; constructor( @@ -13,19 +28,11 @@ contract ERC4626Mock is ERC4626 { string memory name, string memory symbol, uint8 decimaloverride - ) ERC20(name, symbol) ERC4626(asset) { + ) ERC4626Mock(asset, name, symbol) { _decimals = decimaloverride; } function decimals() public view virtual override returns (uint8) { return _decimals; } - - function mockMint(address account, uint256 amount) public { - _mint(account, amount); - } - - function mockBurn(address account, uint256 amount) public { - _burn(account, amount); - } } diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js index 571f0063ac7..368a59459cd 100644 --- a/test/token/ERC20/extensions/ERC4626.test.js +++ b/test/token/ERC20/extensions/ERC4626.test.js @@ -3,6 +3,7 @@ const { expect } = require('chai'); const ERC20DecimalsMock = artifacts.require('ERC20DecimalsMock'); const ERC4626Mock = artifacts.require('ERC4626Mock'); +const ERC4626DecimalMock = artifacts.require('ERC4626DecimalMock'); const parseToken = (token) => (new BN(token)).mul(new BN('1000000000000')); const parseShare = (share) => (new BN(share)).mul(new BN('1000000000000000000')); @@ -15,7 +16,7 @@ contract('ERC4626', function (accounts) { beforeEach(async function () { this.token = await ERC20DecimalsMock.new(name, symbol, 12); - this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V', 18); + this.vault = await ERC4626DecimalMock.new(this.token.address, name + ' Vault', symbol + 'V', 18); await this.token.mint(holder, web3.utils.toWei('100')); await this.token.approve(this.vault.address, constants.MAX_UINT256, { from: holder }); @@ -25,9 +26,18 @@ contract('ERC4626', function (accounts) { it('metadata', async function () { expect(await this.vault.name()).to.be.equal(name + ' Vault'); expect(await this.vault.symbol()).to.be.equal(symbol + 'V'); + expect(await this.vault.decimals()).to.be.bignumber.equal('18'); expect(await this.vault.asset()).to.be.equal(this.token.address); }); + it('inherit decimals if from asset', async function () { + for (const decimals of [ 0, 9, 12, 18, 36 ].map(web3.utils.toBN)) { + const token = await ERC20DecimalsMock.new('', '', decimals); + const vault = await ERC4626Mock.new(token.address, '', ''); + expect(await vault.decimals()).to.be.bignumber.equal(decimals); + } + }); + describe('empty vault: no assets & no shares', function () { it('status', async function () { expect(await this.vault.totalAssets()).to.be.bignumber.equal('0'); @@ -400,7 +410,7 @@ contract('ERC4626', function (accounts) { it('multiple mint, deposit, redeem & withdrawal', async function () { // test designed with both asset using similar decimals this.token = await ERC20DecimalsMock.new(name, symbol, 18); - this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V', 18); + this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V'); await this.token.mint(user1, 4000); await this.token.mint(user2, 7001); From 7a46d8f0152b0b0e341e821d1cc2b3765d75f9ec Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 23 Aug 2022 13:33:26 +0200 Subject: [PATCH 03/15] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06409f694c0..9f44f1a3637 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)) + * `ERC4626`: use the same `decimals()` as the underlying asset by default (if available), while keeping the hability to override the vault's decimals. ([#3639](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3639)) ### Compatibility Note From dd4783926740be00eea5bb9fd45c1b309884202d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 15:13:36 +0200 Subject: [PATCH 04/15] Update contracts/mocks/ERC4626Mock.sol Co-authored-by: Francisco --- contracts/mocks/ERC4626Mock.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/mocks/ERC4626Mock.sol b/contracts/mocks/ERC4626Mock.sol index 92d2cf9bf2d..7194fea6980 100644 --- a/contracts/mocks/ERC4626Mock.sol +++ b/contracts/mocks/ERC4626Mock.sol @@ -27,9 +27,9 @@ contract ERC4626DecimalMock is ERC4626Mock { IERC20Metadata asset, string memory name, string memory symbol, - uint8 decimaloverride + uint8 decimalsOverride ) ERC4626Mock(asset, name, symbol) { - _decimals = decimaloverride; + _decimals = decimalsOverride; } function decimals() public view virtual override returns (uint8) { From 3adb5e35e2b4c748097288b9ecf4fc9b4b8f83dc Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 15:56:25 +0200 Subject: [PATCH 05/15] use internal virtual function for the initial conversion rate --- contracts/mocks/ERC4626Mock.sol | 10 ++++++++++ contracts/token/ERC20/extensions/ERC4626.sol | 20 ++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/contracts/mocks/ERC4626Mock.sol b/contracts/mocks/ERC4626Mock.sol index 92d2cf9bf2d..d860aa797b1 100644 --- a/contracts/mocks/ERC4626Mock.sol +++ b/contracts/mocks/ERC4626Mock.sol @@ -21,6 +21,8 @@ contract ERC4626Mock is ERC4626 { } contract ERC4626DecimalMock is ERC4626Mock { + using Math for uint256; + uint8 private immutable _decimals; constructor( @@ -35,4 +37,12 @@ contract ERC4626DecimalMock is ERC4626Mock { function decimals() public view virtual override returns (uint8) { return _decimals; } + + function _initialConvertToShares(uint256 assets, Math.Rounding rounding) internal view virtual override returns (uint256 shares) { + return assets.mulDiv(10**decimals(), 10**super.decimals(), rounding); + } + + function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual override returns (uint256 assets) { + return shares.mulDiv(10**super.decimals(), 10**decimals(), rounding); + } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 6d52e344b35..f28e06bd51f 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -162,10 +162,18 @@ abstract contract ERC4626 is ERC20, IERC4626 { uint256 supply = totalSupply(); return (assets == 0 || supply == 0) - ? assets.mulDiv(10**decimals(), 10**ERC4626.decimals(), rounding) + ? _initialConvertToShares(assets, rounding) : assets.mulDiv(supply, totalAssets(), rounding); } + /** + * @dev Internal conversion function (from assets to shares) to apply when the vault is empty. + */ + function _initialConvertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256 shares) { + rounding; + return assets; + } + /** * @dev Internal conversion function (from shares to assets) with support for rounding direction. */ @@ -173,10 +181,18 @@ abstract contract ERC4626 is ERC20, IERC4626 { uint256 supply = totalSupply(); return (supply == 0) - ? shares.mulDiv(10**ERC4626.decimals(), 10**decimals(), rounding) + ? _initialConvertToAssets(shares, rounding) : shares.mulDiv(totalAssets(), supply, rounding); } + /** + * @dev Internal conversion function (from shares to assets) to apply when the vault is empty. + */ + function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256 assets) { + rounding; + return shares; + } + /** * @dev Deposit/mint common workflow. */ From cbb8d8ebe8e7059ae19db1cb36fd16eb45168a96 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 15:57:37 +0200 Subject: [PATCH 06/15] fix lint --- contracts/mocks/ERC4626Mock.sol | 16 ++++++++++++++-- contracts/token/ERC20/extensions/ERC4626.sol | 18 +++++++++++++----- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/contracts/mocks/ERC4626Mock.sol b/contracts/mocks/ERC4626Mock.sol index a4d1f54c243..4f80b4bd75b 100644 --- a/contracts/mocks/ERC4626Mock.sol +++ b/contracts/mocks/ERC4626Mock.sol @@ -38,11 +38,23 @@ contract ERC4626DecimalMock is ERC4626Mock { return _decimals; } - function _initialConvertToShares(uint256 assets, Math.Rounding rounding) internal view virtual override returns (uint256 shares) { + function _initialConvertToShares(uint256 assets, Math.Rounding rounding) + internal + view + virtual + override + returns (uint256 shares) + { return assets.mulDiv(10**decimals(), 10**super.decimals(), rounding); } - function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual override returns (uint256 assets) { + function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) + internal + view + virtual + override + returns (uint256 assets) + { return shares.mulDiv(10**super.decimals(), 10**decimals(), rounding); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index f28e06bd51f..61cc748b4cf 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -169,7 +169,12 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** * @dev Internal conversion function (from assets to shares) to apply when the vault is empty. */ - function _initialConvertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256 shares) { + function _initialConvertToShares(uint256 assets, Math.Rounding rounding) + internal + view + virtual + returns (uint256 shares) + { rounding; return assets; } @@ -180,15 +185,18 @@ abstract contract ERC4626 is ERC20, IERC4626 { function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256 assets) { uint256 supply = totalSupply(); return - (supply == 0) - ? _initialConvertToAssets(shares, rounding) - : shares.mulDiv(totalAssets(), supply, rounding); + (supply == 0) ? _initialConvertToAssets(shares, rounding) : shares.mulDiv(totalAssets(), supply, rounding); } /** * @dev Internal conversion function (from shares to assets) to apply when the vault is empty. */ - function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256 assets) { + function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) + internal + view + virtual + returns (uint256 assets) + { rounding; return shares; } From 06fce19bfc02df1183b44117265d1e112d683bd9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 16:30:04 +0200 Subject: [PATCH 07/15] add missing interfaces to the docs --- contracts/interfaces/README.adoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contracts/interfaces/README.adoc b/contracts/interfaces/README.adoc index b6b96ffef88..5b4cedf9543 100644 --- a/contracts/interfaces/README.adoc +++ b/contracts/interfaces/README.adoc @@ -24,10 +24,12 @@ are useful to interact with third party contracts that implement them. - {IERC1363} - {IERC1820Implementer} - {IERC1820Registry} +- {IERC1822Proxiable} - {IERC2612} - {IERC2981} - {IERC3156FlashLender} - {IERC3156FlashBorrower} +- {IERC4626} == Detailed ABI @@ -41,6 +43,8 @@ are useful to interact with third party contracts that implement them. {{IERC1820Registry}} +{{IERC1822Proxiable}} + {{IERC2612}} {{IERC2981}} @@ -48,3 +52,5 @@ are useful to interact with third party contracts that implement them. {{IERC3156FlashLender}} {{IERC3156FlashBorrower}} + +{{IERC4626}} \ No newline at end of file From 4d291cae3827cbf1455018ac4544f6db32744286 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 17:03:56 +0200 Subject: [PATCH 08/15] add changelog entry --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1feacf5db50..3061f4b3a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ * `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)) * `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) - * `ERC4626`: use the same `decimals()` as the underlying asset by default (if available), while keeping the hability to override the vault's decimals. ([#3639](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3639)) + * `ERC4626`: use the same `decimals()` as the underlying asset by default (if available). ([#3639](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3639)) + * `ERC4626`: add internal `_initialConvertToShares` and `_initialConvertToAssets` functions to customize empty vaults behavior. ([#3639](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3639)) * `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)) * `ERC721`: Fix balance accounting when a custom `_beforeTokenTransfer` hook results in a transfer of the token under consideration. ([#3611](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3611)) @@ -20,6 +21,10 @@ * `ECDSA`: Remove redundant check on the `v` value. ([#3591](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3591)) * `VestingWallet`: add `releasable` getters. ([#3580](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3580)) +### Breaking changes + + * `ERC4626`: Conversion from shares to assets (and vice-versa) in an empty vault used to consider the possible mismatch between the underlying asset's and the vault's decimals. With the vault now using the assets decimals by default, this initial conversion rate is now set to 1-to-1. Developers overriding the vault decimals to a value that does not match the underlying asset may want to override the `_initialConvertToShares` and `_initialConvertToAssets` to replicate the previous behavior. Note that this change should not affect non-empty vault upgrades. + ### Deprecations * `EIP712`: Added the file `EIP712.sol` and deprecated `draft-EIP712.sol` since the EIP is no longer a Draft. Developers are encouraged to update their imports. ([#3621](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3621)) From a0a43b55e8cb3ad63cd771fe04a5d846890aa433 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 17:20:44 +0200 Subject: [PATCH 09/15] address comments from PR --- contracts/token/ERC20/extensions/ERC4626.sol | 8 ++++---- contracts/token/ERC20/extensions/draft-IERC20Permit.sol | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 61cc748b4cf..5cb5ceb5f94 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -168,14 +168,14 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** * @dev Internal conversion function (from assets to shares) to apply when the vault is empty. + * Note: make sure to keep this function consistent with {_initialConvertToAssets} when overriding it. */ - function _initialConvertToShares(uint256 assets, Math.Rounding rounding) + function _initialConvertToShares(uint256 assets, Math.Rounding /*rounding*/) internal view virtual returns (uint256 shares) { - rounding; return assets; } @@ -190,14 +190,14 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** * @dev Internal conversion function (from shares to assets) to apply when the vault is empty. + * Note: make sure to keep this function consistent with {_initialConvertToShares} when overriding it. */ - function _initialConvertToAssets(uint256 shares, Math.Rounding rounding) + function _initialConvertToAssets(uint256 shares, Math.Rounding /*rounding*/) internal view virtual returns (uint256 assets) { - rounding; return shares; } diff --git a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol index 6363b14084e..6572fddf6e4 100644 --- a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol +++ b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.0; +import "../IERC20.sol"; + /** * @dev Interface of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in * https://eips.ethereum.org/EIPS/eip-2612[EIP-2612]. @@ -11,7 +13,7 @@ pragma solidity ^0.8.0; * presenting a message signed by the account. By not relying on {IERC20-approve}, the token holder account doesn't * need to send a transaction, and thus is not required to hold Ether at all. */ -interface IERC20Permit { +interface IERC20Permit is IERC20 { /** * @dev Sets `value` as the allowance of `spender` over ``owner``'s tokens, * given ``owner``'s signed approval. From 1719e3f67cefd65dcf23e2b2b2fb56fb6fc382c1 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 24 Aug 2022 18:00:36 +0200 Subject: [PATCH 10/15] fix lint --- contracts/token/ERC20/extensions/ERC4626.sol | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 5cb5ceb5f94..6a31c813277 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -170,12 +170,10 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Internal conversion function (from assets to shares) to apply when the vault is empty. * Note: make sure to keep this function consistent with {_initialConvertToAssets} when overriding it. */ - function _initialConvertToShares(uint256 assets, Math.Rounding /*rounding*/) - internal - view - virtual - returns (uint256 shares) - { + function _initialConvertToShares( + uint256 assets, + Math.Rounding /*rounding*/ + ) internal view virtual returns (uint256 shares) { return assets; } @@ -192,12 +190,10 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Internal conversion function (from shares to assets) to apply when the vault is empty. * Note: make sure to keep this function consistent with {_initialConvertToShares} when overriding it. */ - function _initialConvertToAssets(uint256 shares, Math.Rounding /*rounding*/) - internal - view - virtual - returns (uint256 assets) - { + function _initialConvertToAssets( + uint256 shares, + Math.Rounding /*rounding*/ + ) internal view virtual returns (uint256 assets) { return shares; } From 4ac03c9cec183e402707101c212609cbd0243a94 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 25 Aug 2022 11:32:57 +0200 Subject: [PATCH 11/15] Apply suggestions from code review Co-authored-by: Francisco --- CHANGELOG.md | 2 +- contracts/token/ERC20/extensions/ERC4626.sol | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3061f4b3a4c..8b2c3930f4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ ### Breaking changes - * `ERC4626`: Conversion from shares to assets (and vice-versa) in an empty vault used to consider the possible mismatch between the underlying asset's and the vault's decimals. With the vault now using the assets decimals by default, this initial conversion rate is now set to 1-to-1. Developers overriding the vault decimals to a value that does not match the underlying asset may want to override the `_initialConvertToShares` and `_initialConvertToAssets` to replicate the previous behavior. Note that this change should not affect non-empty vault upgrades. + * `ERC4626`: Conversion from shares to assets (and vice-versa) in an empty vault used to consider the possible mismatch between the underlying asset's and the vault's decimals. This initial conversion rate is now set to 1-to-1 irrespective of decimals, which are meant for usability purposes only. The vault now uses the assets decimals by default, so off-chain the numbers should appear the same. Developers overriding the vault decimals to a value that does not match the underlying asset may want to override the `_initialConvertToShares` and `_initialConvertToAssets` to replicate the previous behavior. Note that this change should not affect non-empty vault upgrades. ### Deprecations diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 6a31c813277..2203124a304 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -168,7 +168,8 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** * @dev Internal conversion function (from assets to shares) to apply when the vault is empty. - * Note: make sure to keep this function consistent with {_initialConvertToAssets} when overriding it. + * + * NOTE: Make sure to keep this function consistent with {_initialConvertToAssets} when overriding it. */ function _initialConvertToShares( uint256 assets, @@ -188,7 +189,8 @@ abstract contract ERC4626 is ERC20, IERC4626 { /** * @dev Internal conversion function (from shares to assets) to apply when the vault is empty. - * Note: make sure to keep this function consistent with {_initialConvertToShares} when overriding it. + * + * NOTE: Make sure to keep this function consistent with {_initialConvertToShares} when overriding it. */ function _initialConvertToAssets( uint256 shares, From 6bd75f9dbcad774cc5cb8168139ecf32ff800fce Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 25 Aug 2022 14:05:41 +0200 Subject: [PATCH 12/15] =?UTF-8?q?remove=20IERC20=20=E2=86=92=20IERC20Permi?= =?UTF-8?q?t=20inheritance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/token/ERC20/extensions/draft-IERC20Permit.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol index 6572fddf6e4..6363b14084e 100644 --- a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol +++ b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol @@ -3,8 +3,6 @@ pragma solidity ^0.8.0; -import "../IERC20.sol"; - /** * @dev Interface of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in * https://eips.ethereum.org/EIPS/eip-2612[EIP-2612]. @@ -13,7 +11,7 @@ import "../IERC20.sol"; * presenting a message signed by the account. By not relying on {IERC20-approve}, the token holder account doesn't * need to send a transaction, and thus is not required to hold Ether at all. */ -interface IERC20Permit is IERC20 { +interface IERC20Permit { /** * @dev Sets `value` as the allowance of `spender` over ``owner``'s tokens, * given ``owner``'s signed approval. From 32ee6118fd49c23795bbeb1013fce4eecd6bac7e Mon Sep 17 00:00:00 2001 From: Francisco Date: Thu, 25 Aug 2022 10:48:49 -0300 Subject: [PATCH 13/15] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2c3930f4b..9fe1afaecaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ ### Breaking changes - * `ERC4626`: Conversion from shares to assets (and vice-versa) in an empty vault used to consider the possible mismatch between the underlying asset's and the vault's decimals. This initial conversion rate is now set to 1-to-1 irrespective of decimals, which are meant for usability purposes only. The vault now uses the assets decimals by default, so off-chain the numbers should appear the same. Developers overriding the vault decimals to a value that does not match the underlying asset may want to override the `_initialConvertToShares` and `_initialConvertToAssets` to replicate the previous behavior. Note that this change should not affect non-empty vault upgrades. + * `ERC4626`: Conversion from shares to assets (and vice-versa) in an empty vault used to consider the possible mismatch between the underlying asset's and the vault's decimals. This initial conversion rate is now set to 1-to-1 irrespective of decimals, which are meant for usability purposes only. The vault now uses the assets decimals by default, so off-chain the numbers should appear the same. Developers overriding the vault decimals to a value that does not match the underlying asset may want to override the `_initialConvertToShares` and `_initialConvertToAssets` to replicate the previous behavior. ### Deprecations From e5307c67e9b712d0eb7b5f7d9a8db3728025eff3 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 25 Aug 2022 16:00:47 +0200 Subject: [PATCH 14/15] fetch underlyin asset's decimal value in constructor --- contracts/token/ERC20/extensions/ERC4626.sol | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 2203124a304..b3830e3fa30 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -27,21 +27,26 @@ abstract contract ERC4626 is ERC20, IERC4626 { using Math for uint256; IERC20 private immutable _asset; + uint8 private immutable _decimals; /** * @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC20 or ERC777). */ constructor(IERC20 asset_) { + uint8 assetDecimals; + try IERC20Metadata(address(asset_)).decimals() returns (uint8 value) { + assetDecimals = value; + } catch { + assetDecimals = super.decimals(); + } + _asset = asset_; + _decimals = assetDecimals; } - /** @dev See {IERC4626-asset}. */ + /** @dev See {IERC20Metadata-decimals}. */ function decimals() public view virtual override(IERC20Metadata, ERC20) returns (uint8) { - try IERC20Metadata(address(_asset)).decimals() returns (uint8 value) { - return value; - } catch { - return super.decimals(); - } + return _decimals; } /** @dev See {IERC4626-asset}. */ From be4d899ba018a5fb891789cde9d556e79cbdeb46 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 25 Aug 2022 20:23:19 -0300 Subject: [PATCH 15/15] rename variable --- contracts/token/ERC20/extensions/ERC4626.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index b3830e3fa30..84d45c32beb 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -33,15 +33,15 @@ abstract contract ERC4626 is ERC20, IERC4626 { * @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC20 or ERC777). */ constructor(IERC20 asset_) { - uint8 assetDecimals; + uint8 decimals_; try IERC20Metadata(address(asset_)).decimals() returns (uint8 value) { - assetDecimals = value; + decimals_ = value; } catch { - assetDecimals = super.decimals(); + decimals_ = super.decimals(); } _asset = asset_; - _decimals = assetDecimals; + _decimals = decimals_; } /** @dev See {IERC20Metadata-decimals}. */