Migrate Ownable tests (OpenZeppelin#4657)

Co-authored-by: ernestognw <[email protected]>

.github/workflows/checks.yml

@@ -64,6 +64,8 @@ jobs:
           cp -rnT contracts lib/openzeppelin-contracts/contracts
       - name: Transpile to upgradeable
         run: bash scripts/upgradeable/transpile.sh
+      - name: Compile contracts # TODO: Remove after migrating tests to ethers
+        run: npm run compile
       - name: Run tests
         run: npm run test
       - name: Check linearisation of the inheritance graph
@@ -92,7 +94,10 @@ jobs:
       - uses: actions/checkout@v4
       - name: Set up environment
         uses: ./.github/actions/setup
-      - run: npm run coverage
+      - name: Compile contracts # TODO: Remove after migrating tests to ethers
+        run: npm run compile
+      - name: Run coverage
+        run: npm run coverage
       - uses: codecov/codecov-action@v3
         with:
           token: ${{ secrets.CODECOV_TOKEN }}

hardhat.config.js

@@ -55,7 +55,9 @@ const argv = require('yargs/yargs')()
     },
   }).argv;
 
-require('@nomiclabs/hardhat-truffle5');
+require('@nomiclabs/hardhat-truffle5'); // deprecated
+require('@nomicfoundation/hardhat-toolbox');
+require('@nomicfoundation/hardhat-ethers');
 require('hardhat-ignore-warnings');
 require('hardhat-exposed');
 require('solidity-docgen');
@@ -69,7 +71,7 @@ for (const f of fs.readdirSync(path.join(__dirname, 'hardhat'))) {
   require(path.join(__dirname, 'hardhat', f));
 }
 
-const withOptimizations = argv.gas || argv.compileMode === 'production';
+const withOptimizations = argv.gas || argv.coverage || argv.compileMode === 'production';
 
 /**
  * @type import('hardhat/config').HardhatUserConfig
@@ -99,7 +101,6 @@ module.exports = {
   },
   networks: {
     hardhat: {
-      blockGasLimit: 10000000,
       allowUnlimitedContractSize: !withOptimizations,
     },
   },

hardhat/env-artifacts.js

@@ -1,18 +1,40 @@
 const { HardhatError } = require('hardhat/internal/core/errors');
 
-// Modifies `artifacts.require(X)` so that instead of X it loads the XUpgradeable contract.
+function isExpectedError(e, suffix) {
+  // HH700: Artifact not found - from https://hardhat.org/hardhat-runner/docs/errors#HH700
+  return HardhatError.isHardhatError(e) && e.number === 700 && suffix !== '';
+}
+
+// Modifies the artifact require functions so that instead of X it loads the XUpgradeable contract.
 // This allows us to run the same test suite on both the original and the transpiled and renamed Upgradeable contracts.
+extendEnvironment(hre => {
+  const suffixes = ['UpgradeableWithInit', 'Upgradeable', ''];
 
-extendEnvironment(env => {
-  const artifactsRequire = env.artifacts.require;
+  // Truffe (deprecated)
+  const originalRequire = hre.artifacts.require;
+  hre.artifacts.require = function (name) {
+    for (const suffix of suffixes) {
+      try {
+        return originalRequire.call(this, name + suffix);
+      } catch (e) {
+        if (isExpectedError(e, suffix)) {
+          continue;
+        } else {
+          throw e;
+        }
+      }
+    }
+    throw new Error('Unreachable');
+  };
 
-  env.artifacts.require = name => {
-    for (const suffix of ['UpgradeableWithInit', 'Upgradeable', '']) {
+  // Ethers
+  const originalReadArtifact = hre.artifacts.readArtifact;
+  hre.artifacts.readArtifact = async function (name) {
+    for (const suffix of suffixes) {
       try {
-        return artifactsRequire(name + suffix);
+        return await originalReadArtifact.call(this, name + suffix);
       } catch (e) {
-        // HH700: Artifact not found - from https://hardhat.org/hardhat-runner/docs/errors#HH700
-        if (HardhatError.isHardhatError(e) && e.number === 700 && suffix !== '') {
+        if (isExpectedError(e, suffix)) {
           continue;
         } else {
           throw e;

hardhat/env-contract.js

@@ -1,24 +1,40 @@
-extendEnvironment(env => {
-  const { contract } = env;
+// Remove the default account from the accounts list used in tests, in order
+// to protect tests against accidentally passing due to the contract
+// deployer being used subsequently as function caller
+//
+// This operation affects:
+// - the accounts (and signersAsPromise) parameters of `contract` blocks
+// - the return of hre.ethers.getSigners()
+extendEnvironment(hre => {
+  // TODO: replace with a mocha root hook.
+  // (see https://github.com/sc-forks/solidity-coverage/issues/819#issuecomment-1762963679)
+  if (!process.env.COVERAGE) {
+    // override hre.ethers.getSigner()
+    // note that we don't just discard the first signer, we also cache the value to improve speed.
+    const originalGetSigners = hre.ethers.getSigners;
+    const filteredSignersAsPromise = originalGetSigners().then(signers => signers.slice(1));
+    hre.ethers.getSigners = () => filteredSignersAsPromise;
+  }
 
-  env.contract = function (name, body) {
-    const { takeSnapshot } = require('@nomicfoundation/hardhat-network-helpers');
-
-    contract(name, accounts => {
-      // reset the state of the chain in between contract test suites
+  // override hre.contract
+  const originalContract = hre.contract;
+  hre.contract = function (name, body) {
+    originalContract.call(this, name, accounts => {
       let snapshot;
 
       before(async function () {
+        // reset the state of the chain in between contract test suites
+        // TODO: this should be removed when migration to ethers is over
+        const { takeSnapshot } = require('@nomicfoundation/hardhat-network-helpers');
         snapshot = await takeSnapshot();
       });
 
       after(async function () {
+        // reset the state of the chain in between contract test suites
+        // TODO: this should be removed when migration to ethers is over
         await snapshot.restore();
       });
 
-      // remove the default account from the accounts list used in tests, in order
-      // to protect tests against accidentally passing due to the contract
-      // deployer being used subsequently as function caller
       body(accounts.slice(1));
     });
   };