hardhat icon indicating copy to clipboard operation
hardhat copied to clipboard

Rare tests failure after migrating to ESM

Open fullkomnun opened this issue 1 year ago • 1 comments

Version of Hardhat

2.19.5

What happened?

After successfully migrating a typescript hardhat project to ESM, following this great guide so most sources including unit tests, deployment scripts and depolyments tests are ESM except for a few common utils, typechain bindings and hardhat config/tasks which are still commonsJS ('.cts') everything works most of the time but we get this weird (and rare) test failure both locally (macos) and in GitHub CI (ubuntu):

     NotImplementedError: Method 'HardhatEthersProvider.resolveName' is not implemented
      at HardhatEthersProvider.resolveName (<REDACTED>/solidity/node_modules/@nomicfoundation/hardhat-ethers/src/internal/hardhat-ethers-provider.ts:355:11)
      at HardhatEthersSigner.resolveName (<REDACTED>/solidity/node_modules/@nomicfoundation/hardhat-ethers/src/signers.ts:108:26)
      at resolveAddress (<REDACTED>/solidity/node_modules/ethers/src.ts/address/checks.ts:113:46)
      at <REDACTED>/solidity/node_modules/ethers/src.ts/contract/contract.ts:172:60
      at ParamType.#walkAsync (<REDACTED>/solidity/node_modules/ethers/src.ts/abi/fragments.ts:779:24)
      at <REDACTED>/solidity/node_modules/ethers/src.ts/abi/fragments.ts:771:45
      at Array.forEach (<anonymous>)
      at ParamType.#walkAsync (<REDACTED>/solidity/node_modules/ethers/src.ts/abi/fragments.ts:770:20)
      at ParamType.walkAsync (<REDACTED>/solidity/node_modules/ethers/src.ts/abi/fragments.ts:797:24)
      at <REDACTED>/solidity/node_modules/ethers/src.ts/contract/contract.ts:170:22 

It does not happen often and clearing the environement resolves the issue (locally: npm ci + rerun, ci: clearing npm cache + rerun). Test are being run using hardhat test and not by directly invoking mocha.

Other details @nomicfoundation/hardhat-ethers: v3.0.5 node: v20.10 OS: macOS Sonoma 14.3.1 (Apple Silicon) / GitHub CI 'ubunutu-latest' runner

Minimal reproduction steps

Just unit tests, using mocha+typechain+EthersV6, sequentially or in parallel, not using coverage or gas reporting.

Search terms

esm NotImplementedError hardhat-ethers

fullkomnun avatar Feb 14 '24 10:02 fullkomnun

Hi @fullkomnun, could you please provide a minimal reproducible example? For instance, a GitHub repository with the minimal code needed to reproduce the error. Thanks

ChristopherDedominici avatar Feb 16 '24 13:02 ChristopherDedominici

OK, was able to track down the root cause and it is not a hardhat issue so closing.

fullkomnun avatar Mar 03 '24 16:03 fullkomnun

Some of us could encouter the same issue, you should have explain a bit more.

IxDaddy avatar Mar 19 '24 23:03 IxDaddy

@fullkomnun what was the issue?

JGJP avatar Jun 16 '24 11:06 JGJP

@alexdupre @JGJP

I think chances that this would be relevant and beneficial for others is pretty slim but here we go:

So the root cause was a rabid unit test in that project that in the spirit of property based testing, was generating a random Ethereum address by doing "0x" + toBigInt(randomBytes(20)).toString(16)

The problem with this code was that if the most significant byte (the first byte) of the randomly generated data happens to be small enough that its hexadecimal representation requires a single digit, it might start with a leading zero. When this leading zero is included during the hexadecimal conversion, you end up with a 41-character string. However, standard Ethereum addresses only have 40 characters in their hexadecimal form (excluding the '0x' prefix) which was reproducing only once in a while and resulted in the error I reported above...

Mitigation: Applying proper padding like toBigInt(randomBytes(20)).toString(16).padStart(40, '0') Better Mitigation: just use Ethers.js like ethers.Wallet.createRandom().address

fullkomnun avatar Jul 03 '24 10:07 fullkomnun

Not sure why you tagged me.

alexdupre avatar Jul 03 '24 10:07 alexdupre