From 4519ce4e0b796f107281104a007fe5433ae37ed3 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 11 May 2022 16:24:09 +0200 Subject: [PATCH] Workaround hardhat-gas-reporter bug with removing --deploy-fixture In https://github.com/keep-network/keep-core/pull/2970 we introduced a workaround for a bug in hardhat-gas-reporter plugin https://github.com/cgewecke/hardhat-gas-reporter/issues/86. It turned out that there is a much simpler workaround, which is based on just removing `--deploy-fixture` flag. With the flag removed it is required that we load a global snapshot of the deployment (`deployments.fixture()`) instead of loading a specific tag (`deployments.fixture()`. Which we do anyway after one of a recent code updates. We removed all the traces of the previous workaround. --- solidity/ecdsa/README.adoc | 8 -- .../ecdsa/deploy/04_deploy_wallet_registry.ts | 73 +++++-------------- .../09_transfer_proxy_admin_ownership.ts | 4 - solidity/ecdsa/package.json | 3 +- .../test/WalletRegistry.Deployment.test.ts | 10 +-- .../ecdsa/test/WalletRegistry.Upgrade.test.ts | 8 +- solidity/ecdsa/test/fixtures/index.ts | 4 + 7 files changed, 27 insertions(+), 83 deletions(-) diff --git a/solidity/ecdsa/README.adoc b/solidity/ecdsa/README.adoc index 0387059eb9..d2e9919845 100644 --- a/solidity/ecdsa/README.adoc +++ b/solidity/ecdsa/README.adoc @@ -452,14 +452,6 @@ You can run them by doing: yarn test ``` -WARNING: Due to a link:https://github.com/cgewecke/hardhat-gas-reporter/issues/86[bug #86 in `hardhat-gas-reporter` plugin] -we had to workaround deployment scripts to obtain gas usage report after test -execution. -We introduced an alternative deployment path as a workaround. -To execute tests with the alternative deployment path run `yarn test:gas-reporter-workaround`. -Please keep in mind that `yarn test` is preferred way for testing. Use alternative -path only if you want to get the gas report. - === Deploy contracts To deploy contract execute: diff --git a/solidity/ecdsa/deploy/04_deploy_wallet_registry.ts b/solidity/ecdsa/deploy/04_deploy_wallet_registry.ts index 76cb3c25b6..ba53336510 100644 --- a/solidity/ecdsa/deploy/04_deploy_wallet_registry.ts +++ b/solidity/ecdsa/deploy/04_deploy_wallet_registry.ts @@ -1,11 +1,9 @@ -import type { Contract } from "ethers" import type { HardhatRuntimeEnvironment } from "hardhat/types" import type { DeployFunction } from "hardhat-deploy/types" -import type { DeployResult } from "hardhat-deploy/dist/types" const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { const { getNamedAccounts, deployments, ethers, helpers } = hre - const { deployer, esdm } = await getNamedAccounts() + const { deployer } = await getNamedAccounts() const { log } = deployments const SortitionPool = await deployments.get("SortitionPool") @@ -27,59 +25,28 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { log: true, }) - // FIXME: As a workaround for a bug in hardhat-gas-reporter #86 we need to provide - // alternative deployment script to obtain a gas report. - // #86: https://github.com/cgewecke/hardhat-gas-reporter/issues/86 - let walletRegistry: DeployResult | Contract - if (process.env.GAS_REPORTER_BUG_WORKAROUND === "true") { - walletRegistry = await deployments.deploy("WalletRegistry", { - contract: "WalletRegistryStub", - from: deployer, - args: [SortitionPool.address, TokenStaking.address], + const walletRegistry = await helpers.upgrades.deployProxy("WalletRegistry", { + contractName: + deployments.getNetworkName() === "hardhat" + ? "WalletRegistryStub" + : undefined, + initializerArgs: [ + EcdsaDkgValidator.address, + RandomBeacon.address, + ReimbursementPool.address, + ], + factoryOpts: { + signer: await ethers.getSigner(deployer), libraries: { EcdsaInactivity: EcdsaInactivity.address, }, - proxy: { - proxyContract: "TransparentUpgradeableProxy", - viaAdminContract: "DefaultProxyAdmin", - owner: esdm, - execute: { - init: { - methodName: "initialize", - args: [ - EcdsaDkgValidator.address, - RandomBeacon.address, - ReimbursementPool.address, - ], - }, - }, - }, - log: true, - }) - } else { - walletRegistry = await helpers.upgrades.deployProxy("WalletRegistry", { - contractName: - deployments.getNetworkName() === "hardhat" - ? "WalletRegistryStub" - : undefined, - initializerArgs: [ - EcdsaDkgValidator.address, - RandomBeacon.address, - ReimbursementPool.address, - ], - factoryOpts: { - signer: await ethers.getSigner(deployer), - libraries: { - EcdsaInactivity: EcdsaInactivity.address, - }, - }, - proxyOpts: { - constructorArgs: [SortitionPool.address, TokenStaking.address], - unsafeAllow: ["external-library-linking"], - kind: "transparent", - }, - }) - } + }, + proxyOpts: { + constructorArgs: [SortitionPool.address, TokenStaking.address], + unsafeAllow: ["external-library-linking"], + kind: "transparent", + }, + }) await helpers.ownable.transferOwnership( "SortitionPool", diff --git a/solidity/ecdsa/deploy/09_transfer_proxy_admin_ownership.ts b/solidity/ecdsa/deploy/09_transfer_proxy_admin_ownership.ts index b38af17b42..158aa8ea18 100644 --- a/solidity/ecdsa/deploy/09_transfer_proxy_admin_ownership.ts +++ b/solidity/ecdsa/deploy/09_transfer_proxy_admin_ownership.ts @@ -21,7 +21,3 @@ export default func func.tags = ["TransferProxyAdminOwnership"] func.dependencies = ["WalletRegistry"] -// FIXME: As a workaround for a bug in hardhat-gas-reporter #86 we need to provide -// alternative deployment script to obtain a gas report. -// #86: https://github.com/cgewecke/hardhat-gas-reporter/issues/86 -func.skip = async () => process.env.GAS_REPORTER_BUG_WORKAROUND === "true" diff --git a/solidity/ecdsa/package.json b/solidity/ecdsa/package.json index e3f8ff6b76..9ed432132a 100644 --- a/solidity/ecdsa/package.json +++ b/solidity/ecdsa/package.json @@ -27,8 +27,7 @@ "lint:config:fix": "prettier --write '**/*.@(json|yaml)'", "clean": "hardhat clean", "build": "hardhat compile", - "test": "hardhat test --deploy-fixture", - "test:gas-reporter-workaround": "GAS_REPORTER_BUG_WORKAROUND=true yarn test", + "test": "hardhat test", "deploy": "hardhat deploy --export export.json", "prepack": "tsc -p tsconfig.export.json && hardhat export-artifacts export/artifacts" }, diff --git a/solidity/ecdsa/test/WalletRegistry.Deployment.test.ts b/solidity/ecdsa/test/WalletRegistry.Deployment.test.ts index 69c271baa4..ad948e6cb3 100644 --- a/solidity/ecdsa/test/WalletRegistry.Deployment.test.ts +++ b/solidity/ecdsa/test/WalletRegistry.Deployment.test.ts @@ -47,15 +47,7 @@ describe("WalletRegistry - Deployment", async () => { walletRegistry.address ) - // FIXME: As a workaround for a bug in hardhat-gas-reporter #86 we need to provide - // alternative deployment script to obtain a gas report. - // Here we align tests to work with alternative deployment path. - // #86: https://github.com/cgewecke/hardhat-gas-reporter/issues/86 - if (process.env.GAS_REPORTER_BUG_WORKAROUND === "true") { - proxyAdmin = (await ethers.getContract("DefaultProxyAdmin")) as ProxyAdmin - } else { - proxyAdmin = (await upgrades.admin.getInstance()) as ProxyAdmin - } + proxyAdmin = (await upgrades.admin.getInstance()) as ProxyAdmin expect(deployer.address, "deployer is the same as governance").not.equal( governance.address diff --git a/solidity/ecdsa/test/WalletRegistry.Upgrade.test.ts b/solidity/ecdsa/test/WalletRegistry.Upgrade.test.ts index 8717047d3c..aec4d5fd56 100644 --- a/solidity/ecdsa/test/WalletRegistry.Upgrade.test.ts +++ b/solidity/ecdsa/test/WalletRegistry.Upgrade.test.ts @@ -24,13 +24,7 @@ describe("WalletRegistry - Upgrade", async () => { let proxyAdminOwner: SignerWithAddress let EcdsaInactivity: Contract - before(async function () { - // FIXME: As a workaround for a bug in hardhat-gas-reporter #86 we need to provide - // alternative deployment script to obtain a gas report. - // Here we ignore tests that are related to the original deployment script. - // #86: https://github.com/cgewecke/hardhat-gas-reporter/issues/86 - if (process.env.GAS_REPORTER_BUG_WORKAROUND === "true") this.skip() - + before(async () => { proxyAdminOwner = await ethers.getNamedSigner("esdm") EcdsaInactivity = await ethers.getContract("EcdsaInactivity") }) diff --git a/solidity/ecdsa/test/fixtures/index.ts b/solidity/ecdsa/test/fixtures/index.ts index 5ebc6ed99e..419aff684d 100644 --- a/solidity/ecdsa/test/fixtures/index.ts +++ b/solidity/ecdsa/test/fixtures/index.ts @@ -60,6 +60,10 @@ export const walletRegistryFixture = deployments.createFixture( operators: Operator[] reimbursementPool: ReimbursementPool }> => { + // Due to a [bug] in hardhat-gas-reporter plugin we avoid using `--deploy-fixture` + // flag of `hardhat-deploy` plugin. This requires us to load a global fixture + // (`deployments.fixture()`) instead of loading a specific tag (`deployments.fixture()`. + // bug: https://github.com/cgewecke/hardhat-gas-reporter/issues/86 await deployments.fixture() const walletRegistry: WalletRegistryStub & WalletRegistry =