From f996293aa9bed57efd5e6e7be9dd2f258e0a802c Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 15:14:09 +0200 Subject: [PATCH 1/7] Remove `depositWormholeTbtc` function from `L2WormholeGateway` Here we remove the aforementioned function in order to simplify the contract and make `receiveTbtc` the only function allowed to mint canonical L2 TBTC. The `depositWormholeTbtc` function is not really needed and is just a convenience function that allows minting canonical L2 TBTC using Wormhole TBTC in rare cases when this cannot happen automatically, e.g. due to exceeded minting limits. However, even if minting does not happen automatically, the Wormhole TBTC tokens can be easily bridged back to L1 TBTC. That said, removing `depositWormholeTbtc` should not affect UX dramatically. In return, it greatly simplifies the contract and reduce potential attack surface. --- solidity/contracts/l2/L2WormholeGateway.sol | 21 ------ solidity/test/l2/L2WormholeGateway.test.ts | 71 --------------------- 2 files changed, 92 deletions(-) diff --git a/solidity/contracts/l2/L2WormholeGateway.sol b/solidity/contracts/l2/L2WormholeGateway.sol index 5f97b304f..5a1622f04 100644 --- a/solidity/contracts/l2/L2WormholeGateway.sol +++ b/solidity/contracts/l2/L2WormholeGateway.sol @@ -331,27 +331,6 @@ contract L2WormholeGateway is emit WormholeTbtcReceived(receiver, amount); } - /// @notice Allows to deposit Wormhole tBTC token in exchange for canonical - /// tBTC. Useful in a situation when user received wormhole tBTC - /// instead of canonical tBTC. One example of such situation is - /// when the minting limit was exceeded but the user minted anyway. - /// @dev Requirements: - /// - The sender must have at least `amount` of the Wormhole tBTC and - /// it has to be approved for L2WormholeGateway. - /// - The minting limit must allow for minting the given amount. - /// @param amount The amount of Wormhole tBTC to deposit. - function depositWormholeTbtc(uint256 amount) external { - require( - mintedAmount + amount <= mintingLimit, - "Minting limit exceeded" - ); - - emit WormholeTbtcDeposited(msg.sender, amount); - mintedAmount += amount; - bridgeToken.safeTransferFrom(msg.sender, address(this), amount); - tbtc.mint(msg.sender, amount); - } - /// @notice Lets the governance to update the tBTC gateway address on the /// chain with the given Wormhole ID. /// @dev Use toWormholeAddress function to convert between Ethereum and diff --git a/solidity/test/l2/L2WormholeGateway.test.ts b/solidity/test/l2/L2WormholeGateway.test.ts index 5cb91c83d..9c9be2723 100644 --- a/solidity/test/l2/L2WormholeGateway.test.ts +++ b/solidity/test/l2/L2WormholeGateway.test.ts @@ -665,77 +665,6 @@ describe("L2WormholeGateway", () => { }) }) - describe("depositWormholeTbtc", () => { - const amount = to1e18(129) - - context("when the minting limit is not exceeded", () => { - let tx: ContractTransaction - - before(async () => { - await createSnapshot() - - await wormholeBridgeStub.mintWormholeToken(depositor1.address, amount) - - await wormholeTbtc.connect(depositor1).approve(gateway.address, amount) - tx = await gateway.connect(depositor1).depositWormholeTbtc(amount) - }) - - after(async () => { - await restoreSnapshot() - }) - - it("should transfer wormhole tBTC from user to the gateway", async () => { - expect(tx) - .to.emit(wormholeTbtc, "Transfer") - .withArgs(depositor1.address, gateway.address, amount) - }) - - it("should mint canonical tBTC to the user", async () => { - expect(await canonicalTbtc.balanceOf(depositor1.address)).to.equal( - amount - ) - }) - - it("should emit the WormholeTbtcDeposited event", async () => { - await expect(tx) - .to.emit(gateway, "WormholeTbtcDeposited") - .withArgs(depositor1.address, amount) - }) - - it("should increase the minted amount counter", async () => { - expect(await gateway.mintedAmount()).to.equal(amount) - }) - }) - - context("when the minting limit is exceeded", () => { - before(async () => { - await createSnapshot() - - await gateway.connect(governance).updateMintingLimit(amount) - - await wormholeBridgeStub.mintWormholeToken( - depositor1.address, - amount.mul(2) - ) - await wormholeTbtc - .connect(depositor1) - .approve(gateway.address, amount.mul(2)) - - await gateway.connect(depositor1).depositWormholeTbtc(amount) - }) - - after(async () => { - await restoreSnapshot() - }) - - it("should revert", async () => { - await expect( - gateway.connect(depositor1).depositWormholeTbtc(amount) - ).to.be.revertedWith("Minting limit exceeded") - }) - }) - }) - describe("updateGatewayAddress", () => { context("when called by a third party", () => { it("should revert", async () => { From f6c7c13cd41e0e4841cc5f72960ba4bc8c8b9675 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 15:30:10 +0200 Subject: [PATCH 2/7] Make `receiveTbtc` non-reentrant The Wormhole Token Bridge contract has protection against redeeming the same VAA again. When a Token Bridge VAA is redeemed, its message body hash is stored in a map. This map is used to check whether the hash has already been set in this map. For this reason, `receiveTbtc` does not have to be nonReentrant in theory. However, to make this function future-proof and non-dependent on Wormhole Bridge implementation, we are making it nonReentrant anyway. --- solidity/contracts/l2/L2WormholeGateway.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/solidity/contracts/l2/L2WormholeGateway.sol b/solidity/contracts/l2/L2WormholeGateway.sol index 5a1622f04..fa46dd066 100644 --- a/solidity/contracts/l2/L2WormholeGateway.sol +++ b/solidity/contracts/l2/L2WormholeGateway.sol @@ -285,10 +285,12 @@ contract L2WormholeGateway is /// the same VAA again. When a Token Bridge VAA is redeemed, its /// message body hash is stored in a map. This map is used to check /// whether the hash has already been set in this map. For this reason, - /// this function does not have to be nonReentrant. + /// this function does not have to be nonReentrant in theory. However, + /// to make this function non-dependent on Wormhole Bridge implementation, + /// we are making it nonReentrant anyway. /// @param encodedVm A byte array containing a Wormhole VAA signed by the /// guardians. - function receiveTbtc(bytes calldata encodedVm) external { + function receiveTbtc(bytes calldata encodedVm) external nonReentrant { // ITokenBridge.completeTransferWithPayload completes a contract-controlled // transfer of an ERC20 token. Calling this function is not enough to // ensure L2WormholeGateway received Wormhole tBTC representation. From d46ca3603609c28d5e61dcfecdbcf561c147e78c Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 15:32:49 +0200 Subject: [PATCH 3/7] Adjust some comments around `receiveTbtc` --- solidity/contracts/l2/L2WormholeGateway.sol | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/solidity/contracts/l2/L2WormholeGateway.sol b/solidity/contracts/l2/L2WormholeGateway.sol index fa46dd066..88fec4514 100644 --- a/solidity/contracts/l2/L2WormholeGateway.sol +++ b/solidity/contracts/l2/L2WormholeGateway.sol @@ -281,6 +281,7 @@ contract L2WormholeGateway is /// - The receiver of the canonical tBTC should be abi-encoded in the /// payload. /// - The receiver of the canonical tBTC must not be the zero address. + /// /// The Wormhole Token Bridge contract has protection against redeeming /// the same VAA again. When a Token Bridge VAA is redeemed, its /// message body hash is stored in a map. This map is used to check @@ -320,15 +321,13 @@ contract L2WormholeGateway is if (mintedAmount + amount > mintingLimit) { bridgeToken.safeTransfer(receiver, amount); } else { - // The function is non-reentrant given bridge.completeTransferWithPayload - // call that does not allow to use the same VAA again. + // The function is non-reentrant. // slither-disable-next-line reentrancy-benign mintedAmount += amount; tbtc.mint(receiver, amount); } - // The function is non-reentrant given bridge.completeTransferWithPayload - // call that does not allow to use the same VAA again. + // The function is non-reentrant. // slither-disable-next-line reentrancy-events emit WormholeTbtcReceived(receiver, amount); } From 340578c6a7d1af36704e66172fee74f0a6348b86 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 17:31:45 +0200 Subject: [PATCH 4/7] Upgrade `ArbitrumWormholeGateway` script --- ...anual_upgrade_arbitrum_wormhole_gateway.ts | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 cross-chain/arbitrum/deploy_l2/32_manual_upgrade_arbitrum_wormhole_gateway.ts diff --git a/cross-chain/arbitrum/deploy_l2/32_manual_upgrade_arbitrum_wormhole_gateway.ts b/cross-chain/arbitrum/deploy_l2/32_manual_upgrade_arbitrum_wormhole_gateway.ts new file mode 100644 index 000000000..573c475ca --- /dev/null +++ b/cross-chain/arbitrum/deploy_l2/32_manual_upgrade_arbitrum_wormhole_gateway.ts @@ -0,0 +1,77 @@ +import type { Artifact, HardhatRuntimeEnvironment } from "hardhat/types" +import type { DeployFunction, Deployment } from "hardhat-deploy/types" +import { ContractFactory } from "ethers" + +const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { + const { ethers, helpers, deployments } = hre + + const { deployer } = await helpers.signers.getNamedSigners() + + const proxyDeployment: Deployment = await deployments.get( + "ArbitrumWormholeGateway" + ) + const implementationContractFactory: ContractFactory = + await ethers.getContractFactory("L2WormholeGateway", { + signer: deployer, + }) + + // Deploy new implementation contract + const newImplementationAddress: string = (await hre.upgrades.prepareUpgrade( + proxyDeployment, + implementationContractFactory, + { + kind: "transparent", + } + )) as string + + deployments.log( + `new implementation contract deployed at: ${newImplementationAddress}` + ) + + // Assemble proxy upgrade transaction. + const proxyAdmin = await hre.upgrades.admin.getInstance() + const proxyAdminOwner = await proxyAdmin.owner() + + const upgradeTxData = await proxyAdmin.interface.encodeFunctionData( + "upgrade", + [proxyDeployment.address, newImplementationAddress] + ) + + deployments.log( + `proxy admin owner ${proxyAdminOwner} is required to upgrade proxy implementation with transaction:\n` + + `\t\tfrom: ${proxyAdminOwner}\n` + + `\t\tto: ${proxyAdmin.address}\n` + + `\t\tdata: ${upgradeTxData}` + ) + + // Update Deployment Artifact + const gatewayArtifact: Artifact = + hre.artifacts.readArtifactSync("L2WormholeGateway") + + await deployments.save("ArbitrumWormholeGateway", { + ...proxyDeployment, + abi: gatewayArtifact.abi, + implementation: newImplementationAddress, + }) + + // Contracts can be verified on L2 Arbiscan in a similar way as we do it on + // L1 Etherscan + if (hre.network.tags.arbiscan) { + // We use `verify` instead of `verify:verify` as the `verify` task is defined + // in "@openzeppelin/hardhat-upgrades" to verify the proxy’s implementation + // contract, the proxy itself and any proxy-related contracts, as well as + // link the proxy to the implementation contract’s ABI on (Ether)scan. + await hre.run("verify", { + address: newImplementationAddress, + constructorArgsParams: proxyDeployment.args, + }) + } +} + +export default func + +func.tags = ["ManualUpgradeArbitrumWormholeGateway"] + +// Comment this line when running an upgrade. +// yarn deploy --tags ManualUpgradeArbitrumWormholeGateway --network +func.skip = async () => true From 4ba4c23801d9d542437099a7d5f684977a6f617c Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 17:31:54 +0200 Subject: [PATCH 5/7] Upgrade `BaseWormholeGateway` script --- ...24_manual_upgrade_base_wormhole_gateway.ts | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 cross-chain/base/deploy_l2/24_manual_upgrade_base_wormhole_gateway.ts diff --git a/cross-chain/base/deploy_l2/24_manual_upgrade_base_wormhole_gateway.ts b/cross-chain/base/deploy_l2/24_manual_upgrade_base_wormhole_gateway.ts new file mode 100644 index 000000000..e33277e4f --- /dev/null +++ b/cross-chain/base/deploy_l2/24_manual_upgrade_base_wormhole_gateway.ts @@ -0,0 +1,77 @@ +import type { Artifact, HardhatRuntimeEnvironment } from "hardhat/types" +import type { DeployFunction, Deployment } from "hardhat-deploy/types" +import { ContractFactory } from "ethers" + +const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { + const { ethers, helpers, deployments } = hre + + const { deployer } = await helpers.signers.getNamedSigners() + + const proxyDeployment: Deployment = await deployments.get( + "BaseWormholeGateway" + ) + const implementationContractFactory: ContractFactory = + await ethers.getContractFactory("L2WormholeGateway", { + signer: deployer, + }) + + // Deploy new implementation contract + const newImplementationAddress: string = (await hre.upgrades.prepareUpgrade( + proxyDeployment, + implementationContractFactory, + { + kind: "transparent", + } + )) as string + + deployments.log( + `new implementation contract deployed at: ${newImplementationAddress}` + ) + + // Assemble proxy upgrade transaction. + const proxyAdmin = await hre.upgrades.admin.getInstance() + const proxyAdminOwner = await proxyAdmin.owner() + + const upgradeTxData = await proxyAdmin.interface.encodeFunctionData( + "upgrade", + [proxyDeployment.address, newImplementationAddress] + ) + + deployments.log( + `proxy admin owner ${proxyAdminOwner} is required to upgrade proxy implementation with transaction:\n` + + `\t\tfrom: ${proxyAdminOwner}\n` + + `\t\tto: ${proxyAdmin.address}\n` + + `\t\tdata: ${upgradeTxData}` + ) + + // Update Deployment Artifact + const gatewayArtifact: Artifact = + hre.artifacts.readArtifactSync("L2WormholeGateway") + + await deployments.save("BaseWormholeGateway", { + ...proxyDeployment, + abi: gatewayArtifact.abi, + implementation: newImplementationAddress, + }) + + // Contracts can be verified on L2 Basescan in a similar way as we do it on + // L1 Etherscan + if (hre.network.tags.basescan) { + // We use `verify` instead of `verify:verify` as the `verify` task is defined + // in "@openzeppelin/hardhat-upgrades" to verify the proxy’s implementation + // contract, the proxy itself and any proxy-related contracts, as well as + // link the proxy to the implementation contract’s ABI on (Ether)scan. + await hre.run("verify", { + address: newImplementationAddress, + constructorArgsParams: proxyDeployment.args, + }) + } +} + +export default func + +func.tags = ["ManualUpgradeBaseWormholeGateway"] + +// Comment this line when running an upgrade. +// yarn deploy --tags ManualUpgradeBaseWormholeGateway --network +func.skip = async () => true From 9ff82128206966062d17850e979521661c888ded Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 17:38:37 +0200 Subject: [PATCH 6/7] Upgrade `OptimismWormholeGateway` script --- ...anual_upgrade_optimism_wormhole_gateway.ts | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 cross-chain/optimism/deploy_l2/32_manual_upgrade_optimism_wormhole_gateway.ts diff --git a/cross-chain/optimism/deploy_l2/32_manual_upgrade_optimism_wormhole_gateway.ts b/cross-chain/optimism/deploy_l2/32_manual_upgrade_optimism_wormhole_gateway.ts new file mode 100644 index 000000000..0bfbfc0cd --- /dev/null +++ b/cross-chain/optimism/deploy_l2/32_manual_upgrade_optimism_wormhole_gateway.ts @@ -0,0 +1,77 @@ +import type { Artifact, HardhatRuntimeEnvironment } from "hardhat/types" +import type { DeployFunction, Deployment } from "hardhat-deploy/types" +import { ContractFactory } from "ethers" + +const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { + const { ethers, helpers, deployments } = hre + + const { deployer } = await helpers.signers.getNamedSigners() + + const proxyDeployment: Deployment = await deployments.get( + "OptimismWormholeGateway" + ) + const implementationContractFactory: ContractFactory = + await ethers.getContractFactory("L2WormholeGateway", { + signer: deployer, + }) + + // Deploy new implementation contract + const newImplementationAddress: string = (await hre.upgrades.prepareUpgrade( + proxyDeployment, + implementationContractFactory, + { + kind: "transparent", + } + )) as string + + deployments.log( + `new implementation contract deployed at: ${newImplementationAddress}` + ) + + // Assemble proxy upgrade transaction. + const proxyAdmin = await hre.upgrades.admin.getInstance() + const proxyAdminOwner = await proxyAdmin.owner() + + const upgradeTxData = await proxyAdmin.interface.encodeFunctionData( + "upgrade", + [proxyDeployment.address, newImplementationAddress] + ) + + deployments.log( + `proxy admin owner ${proxyAdminOwner} is required to upgrade proxy implementation with transaction:\n` + + `\t\tfrom: ${proxyAdminOwner}\n` + + `\t\tto: ${proxyAdmin.address}\n` + + `\t\tdata: ${upgradeTxData}` + ) + + // Update Deployment Artifact + const gatewayArtifact: Artifact = + hre.artifacts.readArtifactSync("L2WormholeGateway") + + await deployments.save("OptimismWormholeGateway", { + ...proxyDeployment, + abi: gatewayArtifact.abi, + implementation: newImplementationAddress, + }) + + // Contracts can be verified on L2 Optimism Etherscan in a similar way as we do it on + // L1 Etherscan + if (hre.network.tags.optimism_etherscan) { + // We use `verify` instead of `verify:verify` as the `verify` task is defined + // in "@openzeppelin/hardhat-upgrades" to verify the proxy’s implementation + // contract, the proxy itself and any proxy-related contracts, as well as + // link the proxy to the implementation contract’s ABI on (Ether)scan. + await hre.run("verify", { + address: newImplementationAddress, + constructorArgsParams: proxyDeployment.args, + }) + } +} + +export default func + +func.tags = ["ManualUpgradeOptimismWormholeGateway"] + +// Comment this line when running an upgrade. +// yarn deploy --tags ManualUpgradeOptimismWormholeGateway --network +func.skip = async () => true From b4204ba34af0422a5464bdc259add5db9718cccd Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 9 Oct 2023 17:48:33 +0200 Subject: [PATCH 7/7] Upgrade `PolygonWormholeGateway` script --- ...manual_upgrade_polygon_wormhole_gateway.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 cross-chain/polygon/deploy_sidechain/32_manual_upgrade_polygon_wormhole_gateway.ts diff --git a/cross-chain/polygon/deploy_sidechain/32_manual_upgrade_polygon_wormhole_gateway.ts b/cross-chain/polygon/deploy_sidechain/32_manual_upgrade_polygon_wormhole_gateway.ts new file mode 100644 index 000000000..d5b381edb --- /dev/null +++ b/cross-chain/polygon/deploy_sidechain/32_manual_upgrade_polygon_wormhole_gateway.ts @@ -0,0 +1,83 @@ +import type { Artifact, HardhatRuntimeEnvironment } from "hardhat/types" +import type { DeployFunction, Deployment } from "hardhat-deploy/types" +import { ContractFactory } from "ethers" + +const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { + const { ethers, helpers, deployments } = hre + + const { deployer } = await helpers.signers.getNamedSigners() + + const proxyDeployment: Deployment = await deployments.get( + "PolygonWormholeGateway" + ) + const implementationContractFactory: ContractFactory = + await ethers.getContractFactory("L2WormholeGateway", { + signer: deployer, + }) + + // Deploy new implementation contract + const newImplementationAddress: string = (await hre.upgrades.prepareUpgrade( + proxyDeployment, + implementationContractFactory, + { + kind: "transparent", + } + )) as string + + deployments.log( + `new implementation contract deployed at: ${newImplementationAddress}` + ) + + // Assemble proxy upgrade transaction. + const proxyAdmin = await hre.upgrades.admin.getInstance() + const proxyAdminOwner = await proxyAdmin.owner() + + const upgradeTxData = await proxyAdmin.interface.encodeFunctionData( + "upgrade", + [proxyDeployment.address, newImplementationAddress] + ) + + deployments.log( + `proxy admin owner ${proxyAdminOwner} is required to upgrade proxy implementation with transaction:\n` + + `\t\tfrom: ${proxyAdminOwner}\n` + + `\t\tto: ${proxyAdmin.address}\n` + + `\t\tdata: ${upgradeTxData}` + ) + + // Update Deployment Artifact + const gatewayArtifact: Artifact = + hre.artifacts.readArtifactSync("L2WormholeGateway") + + await deployments.save("PolygonWormholeGateway", { + ...proxyDeployment, + abi: gatewayArtifact.abi, + implementation: newImplementationAddress, + }) + + // Contracts can be verified on L2 Polygonscan in a similar way as we do it on + // L1 Etherscan + if (hre.network.tags.polygonscan) { + if (hre.network.name === "mumbai") { + // Polygonscan might not include the recently added proxy transaction right + // after deployment. We need to wait some time so that transaction is + // visible on Polygonscan. + await new Promise((resolve) => setTimeout(resolve, 10000)) // 10sec + } + // We use `verify` instead of `verify:verify` as the `verify` task is defined + // in "@openzeppelin/hardhat-upgrades" to verify the proxy’s implementation + // contract, the proxy itself and any proxy-related contracts, as well as + // link the proxy to the implementation contract’s ABI on (Ether)scan. + await hre.run("verify", { + address: newImplementationAddress, + constructorArgsParams: proxyDeployment.args, + }) + } +} + +export default func + +func.tags = ["ManualUpgradePolygonWormholeGateway"] + +// Comment this line when running an upgrade. +// yarn deploy --tags ManualUpgradePolygonWormholeGateway --network +func.skip = async () => true