diff --git a/test/e2e/UpgradeAuthenticator.t.sol b/test/e2e/UpgradeAuthenticator.t.sol new file mode 100644 index 00000000..b1319696 --- /dev/null +++ b/test/e2e/UpgradeAuthenticator.t.sol @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +pragma solidity ^0.8; + +import {GPv2AllowListAuthentication} from "src/contracts/GPv2AllowListAuthentication.sol"; + +import {Helper} from "./Helper.sol"; + +interface IEIP173Proxy { + function upgradeTo(address) external; + function transferOwnership(address) external; + function owner() external view returns (address); +} + +contract GPv2AllowListAuthenticationV2 is GPv2AllowListAuthentication { + function newMethod() external pure returns (uint256) { + return 1337; + } +} + +contract UpgradeAuthenticatorTest is Helper(false) { + GPv2AllowListAuthenticationV2 v2Impl; + + function setUp() public override { + super.setUp(); + v2Impl = new GPv2AllowListAuthenticationV2(); + } + + function test_should_upgrade_authenticator() external { + vm.expectRevert(); + GPv2AllowListAuthenticationV2(address(authenticator)).newMethod(); + + vm.prank(owner); + IEIP173Proxy(address(authenticator)).upgradeTo(address(v2Impl)); + + assertEq( + GPv2AllowListAuthenticationV2(address(authenticator)).newMethod(), 1337, "proxy didnt update as expected" + ); + } + + function test_should_preserve_storage() external { + address newSolver = makeAddr("newSolver"); + address newManager = makeAddr("newManager"); + + vm.startPrank(owner); + GPv2AllowListAuthentication(address(authenticator)).addSolver(newSolver); + GPv2AllowListAuthentication(address(authenticator)).setManager(newManager); + + IEIP173Proxy(address(authenticator)).upgradeTo(address(v2Impl)); + vm.stopPrank(); + + assertEq(authenticator.isSolver(newSolver), true, "solver not retained in storage after proxy upgrade"); + assertEq( + GPv2AllowListAuthentication(address(authenticator)).manager(), + newManager, + "manager not retained in storage after proxy upgrade" + ); + } + + function test_should_allow_proxy_owner_to_change_manager() external { + // transfer ownership to a new address and then assert the behavior + // to have a proxy owner that is different address than manager + address newOwner = makeAddr("newOwner"); + vm.prank(owner); + IEIP173Proxy(address(authenticator)).transferOwnership(newOwner); + + address newManager = makeAddr("newManager"); + vm.prank(newOwner); + GPv2AllowListAuthentication(address(authenticator)).setManager(newManager); + + assertEq( + GPv2AllowListAuthentication(address(authenticator)).manager(), + newManager, + "proxy owner couldnt update manager" + ); + } + + function test_should_be_able_to_transfer_proxy_ownership() external { + address newOwner = makeAddr("newOwner"); + vm.prank(owner); + IEIP173Proxy(address(authenticator)).transferOwnership(newOwner); + + assertEq(IEIP173Proxy(address(authenticator)).owner(), newOwner, "ownership didnt transfer as expected"); + } + + function test_should_revert_when_upgrading_with_the_authentication_manager() external { + address newManager = makeAddr("newManager"); + vm.prank(owner); + GPv2AllowListAuthentication(address(authenticator)).setManager(newManager); + + vm.prank(newManager); + vm.expectRevert("NOT_AUTHORIZED"); + IEIP173Proxy(address(authenticator)).upgradeTo(address(v2Impl)); + } + + function test_should_revert_when_not_upgrading_with_the_proxy_owner() external { + address nobody = makeAddr("nobody"); + vm.prank(nobody); + vm.expectRevert("NOT_AUTHORIZED"); + IEIP173Proxy(address(authenticator)).upgradeTo(address(v2Impl)); + } +} diff --git a/test/e2e/upgradeAuthenticator.test.ts b/test/e2e/upgradeAuthenticator.test.ts deleted file mode 100644 index abfa0bd2..00000000 --- a/test/e2e/upgradeAuthenticator.test.ts +++ /dev/null @@ -1,144 +0,0 @@ -import { expect } from "chai"; -import { Contract, Wallet } from "ethers"; -import { deployments, ethers } from "hardhat"; - -import { proxyInterface } from "../../src/ts"; - -import { deployTestContracts } from "./fixture"; - -async function rejectError( - promise: Promise, -): Promise { - try { - await promise; - return undefined; - } catch (err) { - if (err instanceof Error) { - return err; - } else { - throw new Error("Invalid rejection output"); - } - } -} - -async function upgrade( - proxyOwner: Wallet, - contractName: string, - newContractName: string, -) { - // Note that deterministic deployment and gasLimit are not needed/used here as deployment args. - await deployments.deploy(contractName, { - contract: newContractName, - // From differs from initial deployment here since the proxy owner is the Authenticator manager. - from: proxyOwner.address, - proxy: true, - }); -} - -describe("E2E: Upgrade Authenticator", () => { - let authenticator: Contract; - let deployer: Wallet; - let owner: Wallet; - let manager: Wallet; - let nobody: Wallet; - let newOwner: Wallet; - let newManager: Wallet; - let solver: Wallet; - - beforeEach(async () => { - ({ - authenticator, - deployer, - owner, - manager, - wallets: [nobody, newOwner, newManager, solver], - } = await deployTestContracts()); - }); - - it("should upgrade authenticator", async () => { - const GPv2AllowListAuthenticationV2 = await ethers.getContractFactory( - "GPv2AllowListAuthenticationV2", - deployer, - ); - // Note that, before the upgrade this is actually the old instance - const authenticatorV2 = GPv2AllowListAuthenticationV2.attach( - authenticator.address, - ); - // This method doesn't exist before upgrade - await expect(authenticatorV2.newMethod()).to.be.reverted; - - await upgrade( - owner, - "GPv2AllowListAuthentication", - "GPv2AllowListAuthenticationV2", - ); - // This method should exist on after upgrade - expect(await authenticatorV2.newMethod()).to.equal(1337); - }); - - it("should preserve storage", async () => { - await authenticator.connect(manager).addSolver(solver.address); - await authenticator.connect(manager).setManager(newManager.address); - - // Upgrade after storage is set with **proxy owner**; - await upgrade( - owner, - "GPv2AllowListAuthentication", - "GPv2AllowListAuthenticationV2", - ); - - const GPv2AllowListAuthenticationV2 = await ethers.getContractFactory( - "GPv2AllowListAuthenticationV2", - deployer, - ); - const authenticatorV2 = GPv2AllowListAuthenticationV2.attach( - authenticator.address, - ); - - // Both, the listed solvers and updated manager are still set - expect(await authenticatorV2.isSolver(solver.address)).to.equal(true); - expect(await authenticatorV2.manager()).to.equal(newManager.address); - }); - - it("should allow the proxy owner to change the manager", async () => { - await authenticator.connect(owner).setManager(newManager.address); - expect(await authenticator.manager()).to.equal(newManager.address); - }); - - it("should be able to transfer proxy ownership", async () => { - const proxy = proxyInterface(authenticator); - await proxy.connect(owner).transferOwnership(newOwner.address); - expect(await proxy.owner()).to.equal(newOwner.address); - - await upgrade( - newOwner, - "GPv2AllowListAuthentication", - "GPv2AllowListAuthenticationV2", - ); - }); - - it("should revert when not upgrading with the authentication manager", async () => { - await authenticator.connect(owner).setManager(newManager.address); - expect( - await rejectError( - upgrade( - newManager, - "GPv2AllowListAuthentication", - "GPv2AllowListAuthenticationV2", - ), - ), - ).to.not.be.undefined; - }); - - it("should revert when not upgrading with the proxy owner", async () => { - expect( - await rejectError( - upgrade( - nobody, - "GPv2AllowListAuthentication", - "GPv2AllowListAuthenticationV2", - ), - ), - ).to.not.be.undefined; - }); -}); diff --git a/test/src/GPv2AllowListAuthenticationV2.sol b/test/src/GPv2AllowListAuthenticationV2.sol deleted file mode 100644 index 40dc6ca2..00000000 --- a/test/src/GPv2AllowListAuthenticationV2.sol +++ /dev/null @@ -1,11 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-or-later -// solhint-disable-next-line compiler-version -pragma solidity >=0.7.6 <0.9.0; - -import "src/contracts/GPv2AllowListAuthentication.sol"; - -contract GPv2AllowListAuthenticationV2 is GPv2AllowListAuthentication { - function newMethod() external pure returns (uint256) { - return 1337; - } -}