From 44e2c69326037a6864933df12bf3ff5fddc23062 Mon Sep 17 00:00:00 2001 From: ronhuafeng Date: Fri, 9 Sep 2022 17:09:17 +0100 Subject: [PATCH] Inherit asset decimals in ERC4626 (#3639) --- CHANGELOG.md | 6 +++ contracts/interfaces/README.adoc | 6 +++ contracts/mocks/ERC4626Mock.sol | 40 +++++++++++++++- contracts/token/ERC20/extensions/ERC4626.sol | 48 +++++++++++++++++--- test/token/ERC20/extensions/ERC4626.test.js | 12 ++++- 5 files changed, 104 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29afc2a335d..549165a1cc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +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). ([#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)) @@ -21,6 +23,10 @@ * `Create2`: optimize address computation by using assembly instead of `abi.encodePacked`. ([#3600](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3600)) * `Clones`: optimized the assembly to use only the scratch space during deployments, and optimized `predictDeterministicAddress` to use lesser operations. ([#3640](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3640)) +### 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. + ### 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)) 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 diff --git a/contracts/mocks/ERC4626Mock.sol b/contracts/mocks/ERC4626Mock.sol index 71cefacaaeb..4f80b4bd75b 100644 --- a/contracts/mocks/ERC4626Mock.sol +++ b/contracts/mocks/ERC4626Mock.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.0; import "../token/ERC20/extensions/ERC4626.sol"; -// mock class using ERC20 contract ERC4626Mock is ERC4626 { constructor( IERC20Metadata asset, @@ -20,3 +19,42 @@ contract ERC4626Mock is ERC4626 { _burn(account, amount); } } + +contract ERC4626DecimalMock is ERC4626Mock { + using Math for uint256; + + uint8 private immutable _decimals; + + constructor( + IERC20Metadata asset, + string memory name, + string memory symbol, + uint8 decimalsOverride + ) ERC4626Mock(asset, name, symbol) { + _decimals = decimalsOverride; + } + + 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 fd6e1ab4031..3adb63d844c 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -26,13 +26,27 @@ import "../../../utils/math/Math.sol"; abstract contract ERC4626 is ERC20, IERC4626 { using Math for uint256; - IERC20Metadata private immutable _asset; + 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(IERC20Metadata asset_) { + constructor(IERC20 asset_) { + uint8 decimals_; + try IERC20Metadata(address(asset_)).decimals() returns (uint8 value) { + decimals_ = value; + } catch { + decimals_ = super.decimals(); + } + _asset = asset_; + _decimals = decimals_; + } + + /** @dev See {IERC20Metadata-decimals}. */ + function decimals() public view virtual override(IERC20Metadata, ERC20) returns (uint8) { + return _decimals; } /** @dev See {IERC4626-asset}. */ @@ -153,19 +167,41 @@ abstract contract ERC4626 is ERC20, IERC4626 { uint256 supply = totalSupply(); return (assets == 0 || supply == 0) - ? assets.mulDiv(10**decimals(), 10**_asset.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. + * + * 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) { + return assets; + } + /** * @dev Internal conversion function (from shares to assets) with support for rounding direction. */ function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256 assets) { uint256 supply = totalSupply(); return - (supply == 0) - ? shares.mulDiv(10**_asset.decimals(), 10**decimals(), 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. + * + * 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) { + return shares; } /** diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js index 6f3d665612b..4e6986c44cc 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'); + 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');