Merge pull request #2970 from keep-network/ecdsa-fix-gas-reporter

ECDSA: Workaround hardhat-gas-reporter bug There is a bug in hardhat-gas-reporter that we need to work around in order to obtain a gas report for tests: cgewecke/hardhat-gas-reporter#86 Long story short: we cannot use hre.ethers in the deployment scripts. Here we introduce an alternative deployment path just to get the report generated correctly. The alternative path usage is determined by the GAS_REPORTER_BUG_WORKAROUND env property set to true. To run the tests with the alternative path to get the gas report execute yarn test:gas-reporter-workaround command. It will execute the same tests (upgrade tests were skipped as they are strictly related to the original deployment path). yarn test is still a preferred way to run the tests, as it does the deployment exactly the way we want it to test for mainnet. Use the alternative path only if you wish to get the gas report. Once a bug in the hardhat-gas-reporter is fixed, we should revert all the changes related to the workaround.
keep-network · May 10, 2022 · 801cb68 · 801cb68
2 parents 4905877 + 5dd438e
commit 801cb68
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 26 deletions.
diff --git a/solidity/ecdsa/README.adoc b/solidity/ecdsa/README.adoc
@@ -452,6 +452,14 @@ 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/02_deploy_reimbursement_pool.ts b/solidity/ecdsa/deploy/02_deploy_reimbursement_pool.ts
@@ -2,11 +2,11 @@ import type { HardhatRuntimeEnvironment } from "hardhat/types"
 import type { DeployFunction } from "hardhat-deploy/types"
 
 const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
-  const { getNamedAccounts, ethers, deployments } = hre
+  const { getNamedAccounts, deployments } = hre
   const { deployer } = await getNamedAccounts()
 
   const staticGas = 40_800 // gas amount consumed by the refund() + tx cost
-  const maxGasPrice = ethers.utils.parseUnits("500", "gwei")
+  const maxGasPrice = 500_000_000_000 // 500 Gwei
 
   const ReimbursementPool = await deployments.deploy("ReimbursementPool", {
     from: deployer,

diff --git a/solidity/ecdsa/deploy/04_deploy_wallet_registry.ts b/solidity/ecdsa/deploy/04_deploy_wallet_registry.ts
@@ -1,9 +1,11 @@
+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 } = await getNamedAccounts()
+  const { deployer, esdm } = await getNamedAccounts()
   const { log } = deployments
 
   const SortitionPool = await deployments.get("SortitionPool")
@@ -25,27 +27,59 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => {
     log: true,
   })
 
-  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),
+  // 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],
       libraries: {
         EcdsaInactivity: EcdsaInactivity.address,
       },
-    },
-    proxyOpts: {
-      constructorArgs: [SortitionPool.address, TokenStaking.address],
-      unsafeAllow: ["external-library-linking"],
-    },
-  })
+      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",
+      },
+    })
+  }
 
   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
@@ -21,3 +21,7 @@ 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
@@ -28,6 +28,7 @@
     "clean": "hardhat clean",
     "build": "hardhat compile",
     "test": "hardhat test --deploy-fixture",
+    "test:gas-reporter-workaround": "GAS_REPORTER_BUG_WORKAROUND=true yarn 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
@@ -47,7 +47,15 @@ describe("WalletRegistry - Deployment", async () => {
         walletRegistry.address
       )
 
-    proxyAdmin = (await upgrades.admin.getInstance()) as ProxyAdmin
+    // 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
+    }
 
     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
@@ -24,7 +24,13 @@ describe("WalletRegistry - Upgrade", async () => {
   let proxyAdminOwner: SignerWithAddress
   let EcdsaInactivity: Contract
 
-  before(async () => {
+  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()
+
     proxyAdminOwner = await ethers.getNamedSigner("esdm")
     EcdsaInactivity = await ethers.getContract("EcdsaInactivity")
   })

diff --git a/solidity/ecdsa/yarn.lock b/solidity/ecdsa/yarn.lock
@@ -970,9 +970,9 @@
     "@openzeppelin/contracts" "^4.1.0"
 
 "@threshold-network/solidity-contracts@>1.2.0-dev <1.2.0-ropsten":
-  version "1.2.0-dev.5"
-  resolved "https://registry.yarnpkg.com/@threshold-network/solidity-contracts/-/solidity-contracts-1.2.0-dev.5.tgz#270d5e6bf9b4b25c8fbf48ccfcce5f3ef3868d69"
-  integrity sha512-PxmSUw+mUmCCaWyHYvkd5lKzx7TcdC1NbZ1KjV7wfCPA02SHxS8HOjpEdrvQpXJOcdlwnvh1kfPSa9ZtWQ/Z9A==
+  version "1.2.0-dev.6"
+  resolved "https://registry.yarnpkg.com/@threshold-network/solidity-contracts/-/solidity-contracts-1.2.0-dev.6.tgz#00f78102c81e8c724e805fd646f23f02941ab4cd"
+  integrity sha512-hTSFDxfDmYxSZHy3JIzcuiRf8tAvFxK4oRDAWBTrLlmOcVLQgK7OT+CfXC5qiku9bK+GOEuQmqmeQLQZLieG8Q==
   dependencies:
     "@keep-network/keep-core" ">1.8.0-dev <1.8.0-pre"
     "@openzeppelin/contracts" "^4.5"