Solidity-coverage: Issue when running 0.7.12 with Hardhat

[alsco77]

Description

Hey @cgewecke :wave:

I noticed you pushed the 0.7.12 with Hardhat support so thought i'd give it a go and try to use.

Added 0.7.12, added to hardhat.config.ts, and ran hardhat coverage and it is causing the build to fail with some internal issue.

Bug

Error in plugin solidity-coverage: TypeError: _this.eth.setRequestManager is not a function

HardhatPluginError: TypeError: _this.eth.setRequestManager is not a function
at SimpleTaskDefinition.action (/home/alex/projects/stability-labs/mstable/mStable-contracts/node_modules/solidity-coverage/plugins/hardhat.plugin.js:209:35)

Details

Also, now normal tests are failing with a Typescript error (maybe related - looks like possibly the plugin is causing the contracts to be compiled differently). You can see full suite details (Typechain etc) here in this PR

See hardhat config here https://github.com/mstable/mStable-contracts/blob/gas-reductions/hardhat.config.ts

[cgewecke]

Thanks @alsco77! Will investigate.

[cgewecke]

@alsco77

I think the following steps get the coverage command to the same stage test is at on that branch.

coverage and typechain have a hard time w/ each other and this needs to be documented much more clearly in the README.

Fixing _this.eth.setRequestManager is not a function

rm yarn.lock
rm -rf node_modules
yarn

[EDIT - Everything below here mostly wrong. Ignore and see further below.]

Fixing typechain problems

In package.json, tell coverage to use build/contracts as the default location to store instrumented compilation artifacts. This folder will be deleted after each coverage run. (The coverage plugin will quietly complain that it can't create this folder but you should be able to ignore that).

-    "coverage": "node --max_old_space_size=6144 node_modules/.bin/hardhat coverage --network coverage --solcoverjs ./.solcover.js --testfiles 'test/**/Test*.ts' --show-stack-traces",
+    "coverage": "node --max_old_space_size=6144 node_modules/.bin/hardhat coverage --temp 'build/contracts' --testfiles 'test/**/Test*.ts' --show-stack-traces",

Rewrite .solcover.js as:

// This is already a dependency of solidity-coverage
const shell = require('shelljs');

module.exports = {
  onCompileComplete: async function (_config) {
    await run("typechain");
  },
  onIstanbulComplete: async function (_config) {
    // We need to do this because solcover generates bespoke artifacts.
    shell.rm("-rf", "./build");
    shell.rm("-rf", "./typechain");
  },
  skipFiles: [
    'Migrations.sol',
    'interfaces',
    'integrations',
    'z_mocks',
    'shared/InitializableReentrancyGuard.sol',
    'integrations'
  ]
};

At this point npm run coverage fails with the same error I see when running npm test

test-utils/machines/massetMachine.ts:317:16 - error TS2352: Conversion of type 'Promise<MockERC20Instance>' to type 'MockERC20Instance' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Promise<MockERC20Instance>' is missing the following properties from type 'MockERC20Instance': addMinter, allowance, approve, balanceOf, and 16 more.

317         return c_MockERC20.at(x.address) as t.MockERC20Instance;
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Not certain this is a success yet, please just circle back here if/when you discover more problems.

[cgewecke]

[EDIT - Everything here mostly wrong. Ignore and see further below.]

Actually...I have no idea what's going on.

This line in the .solcover.js should be changed to follow your typechain config

- shell.rm("-rf", "./typechain");
+ shell.rm("-rf, "./types/generated");

It also looks like typechain is already running automatically after compile so the onCompileComplete hook may also be removable.

Maybe it would be helpful to have that branch in a state where it can be cloned, installed w/ solidity-coverage at 0.7.12, plus npm test runs clean out of the box.

Are the ts-node/register requires in the hardhat config necessary? Would like to minimize everything TS that seems atypical if possible.

[cgewecke]

Looked into this more and it seems that when ts-node launches it immediately gathers all the type information.

If a task generates types during runtime and tries to use them, they won't be found.

So, the solution is probably to have a simple .solcover.js

module.exports = {
  skipFiles: [
    'Migrations.sol',
    'interfaces',
    'integrations',
    'z_mocks',
    'shared/InitializableReentrancyGuard.sol',
    'integrations'
  ]
};

and just run typechain separately before coverage, e.g

"coverage": "npx hardhat typechain && node --max_old_space_size=6144 node_modules/.bin/hardhat coverage --temp 'build/contracts' --testfiles 'test/**/Test*.ts' --show-stack-traces"

Note: the coverage command will still need to use the --temp 'build/contracts' flag.

[alsco77]

Awesome.. thanks so much for digging into this @cgewecke

Solved both the type issues and the coverage issue with the commands you suggested - see commit
https://github.com/mstable/mStable-contracts/pull/115/commits/9c1246bf076e465697d27ddec26b2fc96d4c2e61

Do you have a gitcoin grant page or somewhere I can send a tip? Reach out on TG if you want @alsco77

[cgewecke]

Thanks @alsco77! Glad that worked 🙂