Skip to content

Commit

Permalink
Workaround hardhat-gas-reporter bug with removing --deploy-fixture
Browse files Browse the repository at this point in the history
In #2970 we introduced a
workaround for a bug in hardhat-gas-reporter plugin
cgewecke/hardhat-gas-reporter#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(<tag>)`. Which we do anyway after one of a
recent code updates.

We removed all the traces of the previous workaround.
  • Loading branch information
nkuba committed May 11, 2022
1 parent 222be83 commit 4519ce4
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 83 deletions.
8 changes: 0 additions & 8 deletions solidity/ecdsa/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
73 changes: 20 additions & 53 deletions solidity/ecdsa/deploy/04_deploy_wallet_registry.ts
Original file line number Diff line number Diff line change
@@ -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")
Expand All @@ -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",
Expand Down
4 changes: 0 additions & 4 deletions solidity/ecdsa/deploy/09_transfer_proxy_admin_ownership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
3 changes: 1 addition & 2 deletions solidity/ecdsa/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
10 changes: 1 addition & 9 deletions solidity/ecdsa/test/WalletRegistry.Deployment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions solidity/ecdsa/test/WalletRegistry.Upgrade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down
4 changes: 4 additions & 0 deletions solidity/ecdsa/test/fixtures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(<tag>)`.
// bug: https://github.com/cgewecke/hardhat-gas-reporter/issues/86
await deployments.fixture()

const walletRegistry: WalletRegistryStub & WalletRegistry =
Expand Down

0 comments on commit 4519ce4

Please sign in to comment.