Inherit asset decimals in ERC4626 (#3639)

Author: Amxx

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))

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,10 +43,14 @@ are useful to interact with third party contracts that implement them.
 
 {{IERC1820Registry}}
 
+{{IERC1822Proxiable}}
+
 {{IERC2612}}
 
 {{IERC2981}}
 
 {{IERC3156FlashLender}}
 
 {{IERC3156FlashBorrower}}
+
+{{IERC4626}}

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);
+    }
+}

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;
     }
 
     /**

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');