From 9ab6d0e4128302490a293cb2061f43cd6d6b9599 Mon Sep 17 00:00:00 2001 From: Jan-Felix Date: Tue, 1 Feb 2022 09:46:46 +0100 Subject: [PATCH 1/5] mark implementation contract as initialized on deployment --- packages/contracts/contracts/TokenLock.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/contracts/contracts/TokenLock.sol b/packages/contracts/contracts/TokenLock.sol index 06d6ad8..aee942d 100644 --- a/packages/contracts/contracts/TokenLock.sol +++ b/packages/contracts/contracts/TokenLock.sol @@ -26,6 +26,8 @@ contract TokenLock is OwnableUpgradeable, IERC20 { /// ERC-20 function is not supported error NotSupported(); + constructor() initializer {} + function initialize( address _owner, address _token, From 2bb232854931d25d3f897593d35fa835ef931534 Mon Sep 17 00:00:00 2001 From: Jan-Felix Date: Tue, 1 Feb 2022 10:01:04 +0100 Subject: [PATCH 2/5] added test --- packages/contracts/test/TokenLock.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/contracts/test/TokenLock.spec.ts b/packages/contracts/test/TokenLock.spec.ts index cd895de..fe0d179 100644 --- a/packages/contracts/test/TokenLock.spec.ts +++ b/packages/contracts/test/TokenLock.spec.ts @@ -75,6 +75,19 @@ describe("TokenLock", () => { ).to.be.revertedWith("Ownable: caller is not the owner") }) + it("initializes the implementation contract on deployment", async () => { + const TokenLock = await ethers.getContractFactory("TokenLock", { + signer: owner, + }) + + const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000" + + const tokenLock = await TokenLock.deploy() + await expect( + tokenLock.initialize(ZERO_ADDRESS, ZERO_ADDRESS, 0, 0, "", "") + ).to.be.revertedWith("Initializable: contract is already initialized") + }) + it("should revert with NotSupported() if one the unsupported ERC-20 functions is called", async () => { const { TokenLock, token } = await setupTest() From d0d479e5f5f016750299acf7ecae855d51f80d90 Mon Sep 17 00:00:00 2001 From: Jan-Felix Date: Tue, 1 Feb 2022 10:32:25 +0100 Subject: [PATCH 3/5] fix coverage --- packages/contracts/contracts/TokenLock.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/contracts/contracts/TokenLock.sol b/packages/contracts/contracts/TokenLock.sol index aee942d..1388b41 100644 --- a/packages/contracts/contracts/TokenLock.sol +++ b/packages/contracts/contracts/TokenLock.sol @@ -26,6 +26,7 @@ contract TokenLock is OwnableUpgradeable, IERC20 { /// ERC-20 function is not supported error NotSupported(); + /// @custom:oz-upgrades-unsafe-allow constructor constructor() initializer {} function initialize( From b754e5ab09abcb8ddf6ab52d590b44cf2600fdc5 Mon Sep 17 00:00:00 2001 From: Jan-Felix Date: Thu, 3 Feb 2022 10:58:06 +0100 Subject: [PATCH 4/5] support non-proxy deployment --- packages/contracts/contracts/TokenLock.sol | 12 +- .../contracts/src/tasks/constructorArgs.ts | 10 ++ packages/contracts/src/tasks/initialDeploy.ts | 21 ++- packages/contracts/src/tasks/upgrade.ts | 7 +- packages/contracts/src/tasks/verify.ts | 3 +- packages/contracts/test/TokenLock.spec.ts | 140 +++++++++++------- 6 files changed, 127 insertions(+), 66 deletions(-) create mode 100644 packages/contracts/src/tasks/constructorArgs.ts diff --git a/packages/contracts/contracts/TokenLock.sol b/packages/contracts/contracts/TokenLock.sol index 1388b41..0bddace 100644 --- a/packages/contracts/contracts/TokenLock.sol +++ b/packages/contracts/contracts/TokenLock.sol @@ -26,8 +26,16 @@ contract TokenLock is OwnableUpgradeable, IERC20 { /// ERC-20 function is not supported error NotSupported(); - /// @custom:oz-upgrades-unsafe-allow constructor - constructor() initializer {} + constructor( + address _owner, + address _token, + uint256 _depositDeadline, + uint256 _lockDuration, + string memory _name, + string memory _symbol + ) { + initialize(_owner, _token, _depositDeadline, _lockDuration, _name, _symbol); + } function initialize( address _owner, diff --git a/packages/contracts/src/tasks/constructorArgs.ts b/packages/contracts/src/tasks/constructorArgs.ts new file mode 100644 index 0000000..e8801a8 --- /dev/null +++ b/packages/contracts/src/tasks/constructorArgs.ts @@ -0,0 +1,10 @@ +type Args = [string, string, string, string, string, string] +const DEAD_ADDRESS = "0x000000000000000000000000000000000000dead" +export const constructorArgs: Args = [ + DEAD_ADDRESS, + DEAD_ADDRESS, + "0", + "0", + "", + "", +] diff --git a/packages/contracts/src/tasks/initialDeploy.ts b/packages/contracts/src/tasks/initialDeploy.ts index 7b70ea2..6473c54 100644 --- a/packages/contracts/src/tasks/initialDeploy.ts +++ b/packages/contracts/src/tasks/initialDeploy.ts @@ -1,6 +1,7 @@ import "hardhat-deploy" import "@nomiclabs/hardhat-ethers" import { task, types } from "hardhat/config" +import { constructorArgs } from "./constructorArgs" task("initialDeploy", "Deploys a fresh TokenLock contract") .addParam("owner", "Address of the owner", undefined, types.string) @@ -33,14 +34,18 @@ task("initialDeploy", "Deploys a fresh TokenLock contract") const [caller] = await hre.ethers.getSigners() console.log("Using the account:", caller.address) const TokenLock = await hre.ethers.getContractFactory("TokenLock") - const tokenLock = await hre.upgrades.deployProxy(TokenLock, [ - taskArgs.owner, - taskArgs.token, - taskArgs.depositDeadline, - taskArgs.lockDuration, - taskArgs.name, - taskArgs.symbol, - ]) + const tokenLock = await hre.upgrades.deployProxy( + TokenLock, + [ + taskArgs.owner, + taskArgs.token, + taskArgs.depositDeadline, + taskArgs.lockDuration, + taskArgs.name, + taskArgs.symbol, + ], + { constructorArgs } + ) console.log("TokenLock proxy deployed to:", tokenLock.address) console.log("Waiting for deploy transaction to be mined...") diff --git a/packages/contracts/src/tasks/upgrade.ts b/packages/contracts/src/tasks/upgrade.ts index 0a5e275..0f1905c 100644 --- a/packages/contracts/src/tasks/upgrade.ts +++ b/packages/contracts/src/tasks/upgrade.ts @@ -2,6 +2,7 @@ import "hardhat-deploy" import "@nomiclabs/hardhat-ethers" import { task, types } from "hardhat/config" +import { constructorArgs } from "./constructorArgs" task("upgrade", "Upgrades the logic of an existing TokenLock contract") .addParam( @@ -14,7 +15,11 @@ task("upgrade", "Upgrades the logic of an existing TokenLock contract") const [caller] = await hre.ethers.getSigners() console.log("Using the account:", caller.address) const TokenLock = await hre.ethers.getContractFactory("TokenLock") - const tokenLock = await hre.upgrades.upgradeProxy(taskArgs.proxy, TokenLock) + const tokenLock = await hre.upgrades.upgradeProxy( + taskArgs.proxy, + TokenLock, + { constructorArgs } + ) console.log( `Latest version of the implementation deployed to: ${tokenLock.address}` diff --git a/packages/contracts/src/tasks/verify.ts b/packages/contracts/src/tasks/verify.ts index 05ac679..a3eaab4 100644 --- a/packages/contracts/src/tasks/verify.ts +++ b/packages/contracts/src/tasks/verify.ts @@ -1,4 +1,5 @@ import { task, types } from "hardhat/config" +import { constructorArgs } from "./constructorArgs" task("verifyEtherscan", "Verifies the contract on etherscan") .addParam( @@ -10,6 +11,6 @@ task("verifyEtherscan", "Verifies the contract on etherscan") .setAction(async (taskArgs, hardhatRuntime) => { await hardhatRuntime.run("verify", { address: taskArgs.implementation, - constructorArgsParams: [], + constructorArgsParams: constructorArgs, }) }) diff --git a/packages/contracts/test/TokenLock.spec.ts b/packages/contracts/test/TokenLock.spec.ts index fe0d179..07f00fc 100644 --- a/packages/contracts/test/TokenLock.spec.ts +++ b/packages/contracts/test/TokenLock.spec.ts @@ -6,6 +6,10 @@ import { BigNumber } from "@ethersproject/bignumber" import { TokenLock as TokenLockT } from "../typechain-types" import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers" +import { Signer } from "ethers" +import { FactoryOptions } from "hardhat/types" + +type Args = [string, string, number, number, string, string] describe("TokenLock", () => { let owner: SignerWithAddress @@ -16,6 +20,29 @@ describe("TokenLock", () => { const now = Math.round(Date.now() / 1000) const oneWeek = 7 * 24 * 60 * 60 + const DEAD_ADDRESS = "0x000000000000000000000000000000000000dead" + const implementationInitArgs: Args = [ + DEAD_ADDRESS, + DEAD_ADDRESS, + 0, + 0, + "", + "", + ] + + const deployTokenLockProxy = async ( + args: Args, + signerOrOptions?: Signer | FactoryOptions | undefined + ): Promise => { + const TokenLock = await ethers.getContractFactory( + "TokenLock", + signerOrOptions + ) + return (await upgrades.deployProxy(TokenLock, args, { + constructorArgs: implementationInitArgs, + })) as TokenLockT + } + const setupTest = deployments.createFixture(async () => { ;[owner, user] = await ethers.getSigners() @@ -32,7 +59,7 @@ describe("TokenLock", () => { it("is upgradable", async () => { const { TokenLock, token } = await setupTest() - const instance = await upgrades.deployProxy(TokenLock, [ + const instance = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, @@ -46,32 +73,37 @@ describe("TokenLock", () => { const upgraded = (await upgrades.upgradeProxy( instance.address, - TokenLockV2 + TokenLockV2, + { constructorArgs: implementationInitArgs } )) as TokenLockT expect(await upgraded.name()).to.equal("Locked TestToken") }) it("cannot be upgraded by others", async () => { const { token } = await setupTest() - const TokenLock = await ethers.getContractFactory("TokenLock", { - signer: owner, - }) - const instance = (await upgrades.deployProxy(TokenLock, [ - owner.address, - token.address, - now + oneWeek, - 2 * oneWeek, - "Locked TestToken", - "LTT", - ])) as TokenLockT + const instance = await deployTokenLockProxy( + [ + owner.address, + token.address, + now + oneWeek, + 2 * oneWeek, + "Locked TestToken", + "LTT", + ], + { + signer: owner, + } + ) const TokenLockV2 = await ethers.getContractFactory("TokenLock", { signer: user, }) await expect( - upgrades.upgradeProxy(instance.address, TokenLockV2) + upgrades.upgradeProxy(instance.address, TokenLockV2, { + constructorArgs: implementationInitArgs, + }) ).to.be.revertedWith("Ownable: caller is not the owner") }) @@ -82,23 +114,23 @@ describe("TokenLock", () => { const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000" - const tokenLock = await TokenLock.deploy() + const tokenLock = await TokenLock.deploy(...implementationInitArgs) await expect( tokenLock.initialize(ZERO_ADDRESS, ZERO_ADDRESS, 0, 0, "", "") ).to.be.revertedWith("Initializable: contract is already initialized") }) it("should revert with NotSupported() if one the unsupported ERC-20 functions is called", async () => { - const { TokenLock, token } = await setupTest() + const { token } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await expect( tokenLock.allowance(user.address, user.address) @@ -116,16 +148,16 @@ describe("TokenLock", () => { describe("initialize", () => { it("sets the owner and stores the provided arguments", async () => { - const { TokenLock, token } = await setupTest() + const { token } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) expect(await tokenLock.owner()).to.equal(owner.address) expect(await tokenLock.token()).to.equal(token.address) @@ -138,15 +170,15 @@ describe("TokenLock", () => { describe("deposit()", () => { it("reverts if the sender has no sufficient balance", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await expect( tokenLock.connect(user).deposit(ONE.mul(2)) @@ -154,15 +186,15 @@ describe("TokenLock", () => { }) it("reverts if the sender has not provided sufficient allowance", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE.div(2)) @@ -172,15 +204,15 @@ describe("TokenLock", () => { }) it("reverts if the deposit deadline has passed", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now - oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE) @@ -190,19 +222,19 @@ describe("TokenLock", () => { }) it("reverts if the token transfer is unsuccessful", async () => { - const { TokenLock } = await setupTest() + await setupTest() const FailingToken = await ethers.getContractFactory( "TestTokenFailingTransferFrom" ) const failingToken = await FailingToken.deploy(18) - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const tokenLock = await deployTokenLockProxy([ owner.address, failingToken.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await failingToken.mint(user.address, ONE) await failingToken.connect(user).approve(tokenLock.address, ONE) @@ -213,15 +245,15 @@ describe("TokenLock", () => { }) it("transfers the deposited tokens into the lock contract", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE) @@ -231,15 +263,15 @@ describe("TokenLock", () => { }) it("mints lock claim tokens to the sender equal to the deposited amount", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE) @@ -251,15 +283,15 @@ describe("TokenLock", () => { }) it("emits a Transfer event", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE) @@ -271,15 +303,15 @@ describe("TokenLock", () => { describe("withdraw", () => { const setupWithLocked = async (weeksSinceLockEnd: number) => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE) await tokenLock.connect(user).deposit(ONE) @@ -306,19 +338,19 @@ describe("TokenLock", () => { }) it("reverts if the token transfer is unsuccessful", async () => { - const { TokenLock } = await setupTest() + await setupTest() const FailingToken = await ethers.getContractFactory( "TestTokenFailingTransfer" ) const failingToken = await FailingToken.deploy(18) - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const tokenLock = await deployTokenLockProxy([ owner.address, failingToken.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await failingToken.mint(user.address, ONE) await failingToken.connect(user).approve(tokenLock.address, ONE) @@ -370,16 +402,16 @@ describe("TokenLock", () => { describe("decimals", () => { it("has the same value as the token that is locked", async () => { - const { TokenLock, Token } = await setupTest() + const { Token } = await setupTest() const tokenWith17Decimals = await Token.deploy(17) - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const tokenLock = await deployTokenLockProxy([ owner.address, tokenWith17Decimals.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) expect(await tokenLock.decimals()).to.equal(17) }) @@ -387,15 +419,15 @@ describe("TokenLock", () => { describe("balanceOf", () => { it("returns the correct balance for the given address", async () => { - const { token, TokenLock } = await setupTest() - const tokenLock = (await upgrades.deployProxy(TokenLock, [ + const { token } = await setupTest() + const tokenLock = await deployTokenLockProxy([ owner.address, token.address, now + oneWeek, 2 * oneWeek, "Locked TestToken", "LTT", - ])) as TokenLockT + ]) await token.connect(user).approve(tokenLock.address, ONE) await tokenLock.connect(user).deposit(ONE) From 42e226bcda86923e206a9cabc36c9464f8ef3462 Mon Sep 17 00:00:00 2001 From: Jan-Felix Date: Thu, 3 Feb 2022 11:09:26 +0100 Subject: [PATCH 5/5] fix coverage --- packages/contracts/contracts/TokenLock.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/contracts/contracts/TokenLock.sol b/packages/contracts/contracts/TokenLock.sol index 0bddace..7b08beb 100644 --- a/packages/contracts/contracts/TokenLock.sol +++ b/packages/contracts/contracts/TokenLock.sol @@ -26,6 +26,7 @@ contract TokenLock is OwnableUpgradeable, IERC20 { /// ERC-20 function is not supported error NotSupported(); + /// @custom:oz-upgrades-unsafe-allow constructor constructor( address _owner, address _token,