Test Hardhat with EDR in third-party repositories

Wodann commented 9 months ago

Depends on

[x] #273

Definition of Done For quality assurance, we need to test Hardhat with EDR in the following third-party repositories:

[x] Test in OpenZeppelin
[x] Test in https://github.com/Uniswap/v3-core
[x] Test in https://github.com/Synthetixio/synthetix (one error in EDR mode which looks like a bug in the test suite)
[x] Test in https://github.com/NexusMutual/smart-contracts (can't get it running due to various plugin errors)
[x] Test in https://github.com/ProjectOpenSea/seaport/ (1 test failure due to accessing HH VM private properties)
[x] Test in https://github.com/smartcontractkit/chainlink/tree/develop/contracts (hardhat-waffle errors, but these errors also occur with the latest released HH version, so we ignore them)
[x] Test in https://github.com/rocket-pool/rocketpool/
[x] Test in https://github.com/neptune-mutual-blue/protocol (same 29 failures in EJS and EDR mode)
[x] Test in https://github.com/safe-global/safe-contracts
[x] Test in https://github.com/taikoxyz/taiko-mono (switched to Foundry)
[x] Test parallel mode in a sample project
[x] Decide whether to test more based on https://www.notion.so/nomicfoundation/Testing-with-real-projects-bd896ef0b11c45fbb41eeb6df16e6584

This can be combined with these performance testing tests

agostbiro commented 7 months ago

Failures discovered so far:

https://github.com/NomicFoundation/edr/issues/281 ✅ https://github.com/NomicFoundation/edr/issues/282 ✅ https://github.com/NomicFoundation/edr/issues/283 ✅

https://github.com/NomicFoundation/edr/issues/287 ✅ https://github.com/NomicFoundation/edr/issues/288 ✅

agostbiro commented 7 months ago

rocketpool@6a9dbfd8 is 4x faster in EDR mode than with EthereumJS and all tests pass.

Tested with EDR version https://github.com/NomicFoundation/hardhat/commits/30001589a

agostbiro commented 7 months ago

safe-contracts@914d0f8 is 1.5x faster in EDR mode than with EthereumJS and all tests pass.

Tested with EDR version https://github.com/NomicFoundation/hardhat/commits/954c6a631

agostbiro commented 7 months ago

`neptune-mutual-blue/protocol@8db6480`

Same 29 failures in EJS and EDR mode. ✅

EDR mode is 9% slower due to memory pressure from large state (~60k transactions in the test suite). 👎

`uniswap/v3-core@d8b1c63`.

All tests pass in EDR mode and there is one failure in EJS mode. ✅

EDR is 1.5x faster. 👍

Tests must be ran with rm -rf test/__snapshots__ && npx hardhat compile && time npx hardhat test, otherwise there are snapshot mismatch errors due to gas cost. This is expected.

`synthetix@9a3a109f`

Test failures due to lack of Smock support and one sporadic failure in EDR mode. ❌

When running a subset of tests where EDR succeeds (npx hardhat test --grep "Perps"), EDR is 3x faster. 👍

EDR starts swapping with state cache size 64 on a full test run, had to be reduced to 10. Need to revisit once a full test run passes. 👎

`seaport@4f4e7c20`

Same test fails in EDR and EJS mode ✅ , but the the cause of the failures is different. In EJS mode the returned value doesn't match the expected, in EDR mode it fails, because the test accesses a private property of the VM. I think it's ok to ignore this.

EDR is 20% faster. 👍

agostbiro commented 7 months ago

`NexusMutual/smart-contracts@e0294fa4`

Doesn't run with edr/main Hardhat in either EJS or EDR mode due to hardhat-tracer error: ❌

Require stack:
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/utils/check-opcodes.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/utils/index.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/chai/message-call.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/chai/index.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/index.js
- /home/ubuntu/dev/smart-contracts/hardhat.config.js/index.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat/internal/core/config/config-loading.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat/internal/cli/cli.js
- /home/ubuntu/dev/smart-contracts/node_modules/hardhat/internal/cli/bootstrap.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1134:15)
    at Function.Module._load (node:internal/modules/cjs/loader:975:27)
    at Module.require (node:internal/modules/cjs/loader:1225:19)
    at require (node:internal/modules/helpers:177:18)
    at Object.<anonymous> (/home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/src/utils/check-opcodes.ts:1:1)
    at Module._compile (node:internal/modules/cjs/loader:1356:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1414:10)
    at Module.load (node:internal/modules/cjs/loader:1197:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1013:12)
    at Module.require (node:internal/modules/cjs/loader:1225:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/utils/check-opcodes.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/utils/index.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/chai/message-call.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/chai/index.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat-tracer/dist/src/index.js',
    '/home/ubuntu/dev/smart-contracts/hardhat.config.js/index.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat/internal/core/config/config-loading.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat/internal/cli/cli.js',
    '/home/ubuntu/dev/smart-contracts/node_modules/hardhat/internal/cli/bootstrap.js'
  ]
}

`chainlink@4c4ac47228`

Uses parallel mode. Many test failures. One failure only in parallel mode. ❌

`taiko-mono@c1ed8b7`

Switched to Foundry, so it's not applicable.

agostbiro commented 6 months ago

One failure in synthetix@9a3a109f with hardhat@d351c2b50 and smock@f87cb73:

1) Contract: BaseSynthetixBridge (unit tests)
       when all the deps are mocked
         when the target is deployed and the proxy is set
           initiateSynthTransfer
             fails if synth is not enabled:
     AssertionError: expected 'VM Exception while processing transac…' to include 'Transaction reverted without a reason…'
      at Object.assertRevert [as revert] (test/utils/index.js:495:12)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at Context.<anonymous> (test/contracts/BaseSynthetixBridge.unit.js:207:6)

This is safe to ignore, because it looks like they're relying on an older bug of Hardhat, where a transaction that reverted with a reason string would show up with a "reverted without a reason" error. (h/t @fvictorio for the explanation.

NomicFoundation / edr