diff --git a/contracts/contracts/VerificationGateway.sol b/contracts/contracts/VerificationGateway.sol index b574a507..919c2abc 100644 --- a/contracts/contracts/VerificationGateway.sol +++ b/contracts/contracts/VerificationGateway.sol @@ -168,8 +168,7 @@ contract VerificationGateway IWallet wallet = IWallet(msg.sender); bytes32 existingHash = hashFromWallet[wallet]; if (existingHash == bytes32(0)) { // wallet does not yet have a bls key registered with this gateway - // set it instantly - safeSetWallet(messageSenderSignature, publicKey, wallet, signatureExpiryTimestamp); + // Can't register new wallet contracts, only what this gateway deployed. } else { // wallet already has a key registered, set after delay pendingMessageSenderSignatureFromHash[existingHash] = messageSenderSignature; @@ -235,6 +234,11 @@ contract VerificationGateway } } + require((selectorId != ProxyAdmin.upgrade.selector) + && (selectorId != ProxyAdmin.upgradeAndCall.selector), + "VG: wallet not upgradable" + ); + wallet.setAnyPending(); // ensure wallet has pre-approved encodedFunction diff --git a/contracts/test/upgrade-test.ts b/contracts/test/upgrade-test.ts index c3c8d9dc..f5c3b85c 100644 --- a/contracts/test/upgrade-test.ts +++ b/contracts/test/upgrade-test.ts @@ -1,21 +1,13 @@ import { expect } from "chai"; import { BigNumber, ContractReceipt } from "ethers"; import { solidityPack } from "ethers/lib/utils"; -import { ethers, network } from "hardhat"; +import { ethers } from "hardhat"; import { expectPubkeysEql } from "./expect"; -import { - ActionData, - BlsWalletWrapper, - getOperationResults, -} from "../clients/src"; +import { ActionData, getOperationResults } from "../clients/src"; import Fixture from "../shared/helpers/Fixture"; -import { - proxyAdminBundle, - proxyAdminCall, -} from "../shared/helpers/callProxyAdmin"; +import { proxyAdminCall } from "../shared/helpers/callProxyAdmin"; import getPublicKeyFromHash from "../shared/helpers/getPublicKeyFromHash"; -import deploy from "../shared/deploy"; const expectOperationsToSucceed = (txnReceipt: ContractReceipt) => { const opResults = getOperationResults(txnReceipt); @@ -44,7 +36,7 @@ describe("Upgrade", async function () { fx = await Fixture.getSingleton(); }); - it("should upgrade wallet contract", async () => { + it("should NOT upgrade wallet contract", async () => { const MockWalletUpgraded = await ethers.getContractFactory( "MockWalletUpgraded", ); @@ -57,243 +49,19 @@ describe("Upgrade", async function () { wallet.address, mockWalletUpgraded.address, ]); - expectOperationsToSucceed(txnReceipt1); - - // Advance time one week - const latestTimestamp = (await ethers.provider.getBlock("latest")) - .timestamp; - await network.provider.send("evm_setNextBlockTimestamp", [ - BigNumber.from(latestTimestamp) - .add(safetyDelaySeconds + 1) - .toHexString(), - ]); + expectOperationFailure(txnReceipt1, "VG: wallet not upgradable"); // make call - const txnReceipt2 = await proxyAdminCall(fx, wallet, "upgrade", [ + const txnReceipt2 = await proxyAdminCall(fx, wallet, "upgradeAndCall", [ wallet.address, mockWalletUpgraded.address, + [], ]); - expectOperationsToSucceed(txnReceipt2); - - const newBLSWallet = MockWalletUpgraded.attach(wallet.address); - await (await newBLSWallet.setNewData(wallet.address)).wait(); - await expect(newBLSWallet.newData()).to.eventually.equal(wallet.address); + expectOperationFailure(txnReceipt2, "VG: wallet not upgradable"); }); it("should register with new verification gateway", async () => { - // Deploy new verification gateway - - const [signer] = await ethers.getSigners(); - - const deployment2 = await deploy( - signer, - ethers.utils.solidityPack(["uint256"], [2]), - ); - - const vg2 = deployment2.verificationGateway; - - // Recreate hubble bls signer - const walletOldVg = await fx.createBLSWallet(); - const walletAddress = walletOldVg.address; - const blsSecret = walletOldVg.blsWalletSigner.privateKey; - - // Sign simple address message - const walletNewVg = await BlsWalletWrapper.connect( - blsSecret, - vg2.address, - vg2.provider, - ); - - const signatureExpiryTimestamp = - (await fx.provider.getBlock("latest")).timestamp + - safetyDelaySeconds + - signatureExpiryOffsetSeconds; - const addressMessage = solidityPack( - ["address", "uint256"], - [walletAddress, signatureExpiryTimestamp], - ); - const addressSignature = walletNewVg.signMessage(addressMessage); - - const proxyAdmin2Address = await vg2.walletProxyAdmin(); - // Get admin action to change proxy - const bundle = await proxyAdminBundle(fx, walletOldVg, "changeProxyAdmin", [ - walletAddress, - proxyAdmin2Address, - ]); - const changeProxyAction = bundle.operations[0].actions[0]; - - // prepare call - const txnReceipt = await proxyAdminCall( - fx, - walletOldVg, - "changeProxyAdmin", - [walletAddress, proxyAdmin2Address], - ); - expectOperationsToSucceed(txnReceipt); - - // Advance time one week - await fx.advanceTimeBy(safetyDelaySeconds + 1); - - const hash = walletOldVg.blsWalletSigner.getPublicKeyHash(); - - const setExternalWalletAction: ActionData = { - ethValue: BigNumber.from(0), - contractAddress: vg2.address, - encodedFunction: vg2.interface.encodeFunctionData("setBLSKeyForWallet", [ - addressSignature, - walletOldVg.PublicKey(), - signatureExpiryTimestamp, - ]), - }; - - const setTrustedBLSGatewayAction: ActionData = { - ethValue: BigNumber.from(0), - contractAddress: fx.verificationGateway.address, - encodedFunction: fx.verificationGateway.interface.encodeFunctionData( - "setTrustedBLSGateway", - [hash, vg2.address], - ), - }; - - // Upgrading the gateway requires these three steps: - // 1. register external wallet in vg2 - // 2. change proxy admin to that in vg2 - // 3. lastly, set wallet's new trusted gateway - // - // If (1) or (2) are skipped, then (3) should fail, and therefore the whole - // operation should fail. - - { - // Fail if setExternalWalletAction is skipped - - const { successes } = - await fx.verificationGateway.callStatic.processBundle( - walletOldVg.sign({ - nonce: BigNumber.from(1), - gas: BigNumber.from(30_000_000), - actions: [ - // skip: setExternalWalletAction, - changeProxyAction, - setTrustedBLSGatewayAction, - ], - }), - ); - - expect(successes).to.deep.equal([false]); - } - - { - // Fail if changeProxyAction is skipped - - const { successes } = - await fx.verificationGateway.callStatic.processBundle( - walletOldVg.sign({ - nonce: BigNumber.from(1), - gas: BigNumber.from(30_000_000), - actions: [ - setExternalWalletAction, - // skip: changeProxyAction, - setTrustedBLSGatewayAction, - ], - }), - ); - - expect(successes).to.deep.equal([false]); - } - - { - // Succeed if nothing is skipped - - const { successes } = - await fx.verificationGateway.callStatic.processBundle( - walletOldVg.sign({ - nonce: BigNumber.from(1), - gas: BigNumber.from(30_000_000), - actions: [ - setExternalWalletAction, - changeProxyAction, - setTrustedBLSGatewayAction, - ], - }), - ); - - expect(successes).to.deep.equal([true]); - } - - await expect(vg2.walletFromHash(hash)).to.eventually.not.equal( - walletAddress, - ); - - // Now actually perform the upgrade so we can perform some more detailed - // checks. - await fx.processBundleWithExtraGas( - walletOldVg.sign({ - nonce: BigNumber.from(1), - gas: BigNumber.from(30_000_000), - actions: [ - setExternalWalletAction, - changeProxyAction, - setTrustedBLSGatewayAction, - ], - }), - ); - - // Create required objects for data/contracts for checks - const proxyAdmin = await ethers.getContractAt( - "ProxyAdmin", - await vg2.walletProxyAdmin(), - ); - - // Direct checks corresponding to each action - await expect(vg2.walletFromHash(hash)).to.eventually.equal(walletAddress); - await expect(vg2.hashFromWallet(walletAddress)).to.eventually.equal(hash); - await expect(proxyAdmin.getProxyAdmin(walletAddress)).to.eventually.equal( - proxyAdmin.address, - ); - expectPubkeysEql( - await getPublicKeyFromHash(vg2, hash), - walletOldVg.PublicKey(), - ); - - const blsWallet = await ethers.getContractAt("BLSWallet", walletAddress); - // New verification gateway pending - await expect(blsWallet.trustedBLSGateway()).to.eventually.equal( - fx.verificationGateway.address, - ); - // Advance time one week - await fx.advanceTimeBy(safetyDelaySeconds + 1); - // set pending - await (await blsWallet.setAnyPending()).wait(); - // Check new verification gateway was set - await expect(blsWallet.trustedBLSGateway()).to.eventually.equal( - vg2.address, - ); - - await walletNewVg.syncWallet(vg2); - // Check new gateway has wallet via static call through new gateway - const bundleResult = await vg2.callStatic.processBundle( - fx.blsWalletSigner.aggregate([ - walletNewVg.sign({ - nonce: BigNumber.from(2), - gas: BigNumber.from(30_000_000), - actions: [ - { - ethValue: 0, - contractAddress: vg2.address, - encodedFunction: vg2.interface.encodeFunctionData( - "walletFromHash", - [hash], - ), - }, - ], - }), - ]), - ); - const walletFromHashAddress = ethers.utils.defaultAbiCoder.decode( - ["address"], - bundleResult.results[0][0], // first and only operation/action result - )[0]; - expect(walletFromHashAddress).to.equal(walletAddress); + // Still possible to point wallets to a new gateway if desired, just not with v1 deployment }); it("should change mapping of an address to hash", async () => {