From dd1265cb1dc834b129c6daf6aae7bdae28521d9e Mon Sep 17 00:00:00 2001
From: Pascal Marco Caversaccio <pcaversaccio@users.noreply.github.com>
Date: Wed, 12 Apr 2023 22:33:50 +0200
Subject: [PATCH] Improve `ERC4626` test coverage (#4134)

Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
---
 .../mocks/token/ERC20ExcessDecimalsMock.sol   |  9 +++++
 remappings.txt                                |  1 +
 test/token/ERC20/extensions/ERC4626.t.sol     | 34 ++++++++++++++++---
 test/token/ERC20/extensions/ERC4626.test.js   | 23 +++++++++++++
 4 files changed, 62 insertions(+), 5 deletions(-)
 create mode 100644 contracts/mocks/token/ERC20ExcessDecimalsMock.sol
 create mode 100644 remappings.txt

diff --git a/contracts/mocks/token/ERC20ExcessDecimalsMock.sol b/contracts/mocks/token/ERC20ExcessDecimalsMock.sol
new file mode 100644
index 000000000..0fb35a607
--- /dev/null
+++ b/contracts/mocks/token/ERC20ExcessDecimalsMock.sol
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+contract ERC20ExcessDecimalsMock {
+    function decimals() public pure returns (uint256) {
+        return type(uint256).max;
+    }
+}
diff --git a/remappings.txt b/remappings.txt
new file mode 100644
index 000000000..2479e3d26
--- /dev/null
+++ b/remappings.txt
@@ -0,0 +1 @@
+openzeppelin/=contracts/
diff --git a/test/token/ERC20/extensions/ERC4626.t.sol b/test/token/ERC20/extensions/ERC4626.t.sol
index 95514531c..f220086d1 100644
--- a/test/token/ERC20/extensions/ERC4626.t.sol
+++ b/test/token/ERC20/extensions/ERC4626.t.sol
@@ -1,18 +1,42 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
-import "erc4626-tests/ERC4626.test.sol";
+import {ERC4626Test} from "erc4626-tests/ERC4626.test.sol";
 
-import {SafeCast} from "../../../../contracts/utils/math/SafeCast.sol";
-import {ERC20Mock} from "../../../../contracts/mocks/ERC20Mock.sol";
-import {ERC4626Mock} from "../../../../contracts/mocks/ERC4626Mock.sol";
+import {SafeCast} from "openzeppelin/utils/math/SafeCast.sol";
+import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol";
+import {ERC4626} from "openzeppelin/token/ERC20/extensions/ERC4626.sol";
+
+import {ERC20Mock} from "openzeppelin/mocks/ERC20Mock.sol";
+import {ERC4626Mock} from "openzeppelin/mocks/ERC4626Mock.sol";
+import {ERC4626OffsetMock} from "openzeppelin/mocks/token/ERC4626OffsetMock.sol";
+
+contract ERC4626VaultOffsetMock is ERC4626OffsetMock {
+    constructor(
+        ERC20 underlying_,
+        uint8 offset_
+    ) ERC20("My Token Vault", "MTKNV") ERC4626(underlying_) ERC4626OffsetMock(offset_) {}
+}
 
 contract ERC4626StdTest is ERC4626Test {
+    ERC20 private _underlying = new ERC20Mock();
+
     function setUp() public override {
-        _underlying_ = address(new ERC20Mock());
+        _underlying_ = address(_underlying);
         _vault_ = address(new ERC4626Mock(_underlying_));
         _delta_ = 0;
         _vaultMayBeEmpty = true;
         _unlimitedAmount = true;
     }
+
+    /**
+     * @dev Check the case where calculated `decimals` value overflows the `uint8` type.
+     */
+    function testFuzzDecimalsOverflow(uint8 offset) public {
+        /// @dev Remember that the `_underlying` exhibits a `decimals` value of 18.
+        offset = uint8(bound(uint256(offset), 238, uint256(type(uint8).max)));
+        ERC4626VaultOffsetMock erc4626VaultOffsetMock = new ERC4626VaultOffsetMock(_underlying, offset);
+        vm.expectRevert();
+        erc4626VaultOffsetMock.decimals();
+    }
 }
diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js
index c9e5a4098..ee0998717 100644
--- a/test/token/ERC20/extensions/ERC4626.test.js
+++ b/test/token/ERC20/extensions/ERC4626.test.js
@@ -5,6 +5,7 @@ const ERC20Decimals = artifacts.require('$ERC20DecimalsMock');
 const ERC4626 = artifacts.require('$ERC4626');
 const ERC4626OffsetMock = artifacts.require('$ERC4626OffsetMock');
 const ERC4626FeesMock = artifacts.require('$ERC4626FeesMock');
+const ERC20ExcessDecimalsMock = artifacts.require('ERC20ExcessDecimalsMock');
 
 contract('ERC4626', function (accounts) {
   const [holder, recipient, spender, other, user1, user2] = accounts;
@@ -21,6 +22,28 @@ contract('ERC4626', function (accounts) {
     }
   });
 
+  it('asset has not yet been created', async function () {
+    const vault = await ERC4626.new('', '', other);
+    expect(await vault.decimals()).to.be.bignumber.equal(decimals);
+  });
+
+  it('underlying excess decimals', async function () {
+    const token = await ERC20ExcessDecimalsMock.new();
+    const vault = await ERC4626.new('', '', token.address);
+    expect(await vault.decimals()).to.be.bignumber.equal(decimals);
+  });
+
+  it('decimals overflow', async function () {
+    for (const offset of [243, 250, 255].map(web3.utils.toBN)) {
+      const token = await ERC20Decimals.new('', '', decimals);
+      const vault = await ERC4626OffsetMock.new(name + ' Vault', symbol + 'V', token.address, offset);
+      await expectRevert(
+        vault.decimals(),
+        'reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block)',
+      );
+    }
+  });
+
   for (const offset of [0, 6, 18].map(web3.utils.toBN)) {
     const parseToken = token => web3.utils.toBN(10).pow(decimals).muln(token);
     const parseShare = share => web3.utils.toBN(10).pow(decimals.add(offset)).muln(share);