diff --git a/README.md b/README.md index a71557d3..cce53150 100644 --- a/README.md +++ b/README.md @@ -142,8 +142,8 @@ The following parameters can be set: ```sh export ETH_RPC_URL='https://rpc.url.example.com' -export NEW_OWNER=0x1111111111111111111111111111111111111111 -export RESET_MANAGER=true # true if the new owner should also become the manager, false otherwise +export NEW_OWNER=0x1111111111111111111111111111111111111111 +export NEW_MANAGER=0x2222222222222222222222222222222222222222 ``` To test run the script from a specific owner (sender): @@ -155,7 +155,7 @@ forge script script/TransferOwnership.s.sol:TransferOwnership --rpc-url "$ETH_RP To actually execute the transaction: ```sh -forge script script/TransferOwnership.s.sol:TransferOwnership --rpc-url "$ETH_RPC_URL" --private-key 0x0000000000000000000000000000000000000000000000000000000000000001 --broadcast +forge script script/TransferOwnership.s.sol:TransferOwnership --rpc-url "$ETH_RPC_URL" --private-key 0x0000000000000000000000000000000000000000000000000000000000000001 --broadcast --slow ``` ## Releases diff --git a/script/TransferOwnership.s.sol b/script/TransferOwnership.s.sol index 656cfac8..2b53fcf2 100644 --- a/script/TransferOwnership.s.sol +++ b/script/TransferOwnership.s.sol @@ -11,7 +11,7 @@ import {NetworksJson} from "./lib/NetworksJson.sol"; contract TransferOwnership is NetworksJson { // Required input string private constant INPUT_ENV_NEW_OWNER = "NEW_OWNER"; - string private constant INPUT_ENV_RESET_MANAGER = "RESET_MANAGER"; + string private constant INPUT_ENV_NEW_MANAGER = "NEW_MANAGER"; // Optional input string private constant INPUT_ENV_AUTHENTICATOR_PROXY = "AUTHENTICATOR_PROXY"; @@ -19,7 +19,7 @@ contract TransferOwnership is NetworksJson { struct ScriptParams { address newOwner; - bool resetManager; + address newManager; ERC173 authenticatorProxy; } @@ -46,19 +46,17 @@ contract TransferOwnership is NetworksJson { // Make sure to reset the manager BEFORE transferring ownership, or else // we will not be able to do it once we lose permissions. - if (params.resetManager) { - console.log( - string.concat( - "Setting new solver manager from ", - vm.toString(authenticator.manager()), - " to ", - vm.toString(params.newOwner) - ) - ); - vm.broadcast(msg.sender); - authenticator.setManager(params.newOwner); - console.log("Set new solver manager account."); - } + console.log( + string.concat( + "Setting new solver manager from ", + vm.toString(authenticator.manager()), + " to ", + vm.toString(params.newManager) + ) + ); + vm.broadcast(msg.sender); + authenticator.setManager(params.newManager); + console.log("Set new solver manager account."); console.log( string.concat( @@ -68,11 +66,14 @@ contract TransferOwnership is NetworksJson { vm.broadcast(msg.sender); params.authenticatorProxy.transferOwnership(params.newOwner); console.log("Set new owner of the authenticator proxy."); + + console.log(string.concat("Final owner: ", vm.toString(params.authenticatorProxy.owner()))); + console.log(string.concat("Final manager: ", vm.toString(authenticator.manager()))); } function paramsFromEnv() internal view returns (ScriptParams memory) { address newOwner = vm.envAddress(INPUT_ENV_NEW_OWNER); - bool resetManager = vm.envBool(INPUT_ENV_RESET_MANAGER); + address newManager = vm.envAddress(INPUT_ENV_NEW_MANAGER); address authenticatorProxy; try vm.envAddress(INPUT_ENV_AUTHENTICATOR_PROXY) returns (address env) { @@ -95,11 +96,8 @@ contract TransferOwnership is NetworksJson { } } - return ScriptParams({ - newOwner: newOwner, - resetManager: resetManager, - authenticatorProxy: ERC173(authenticatorProxy) - }); + return + ScriptParams({newOwner: newOwner, newManager: newManager, authenticatorProxy: ERC173(authenticatorProxy)}); } function checkIsProxy(address candidate) internal view { diff --git a/test/script/TransferOwnership.t.sol b/test/script/TransferOwnership.t.sol index 77511606..ed50785c 100644 --- a/test/script/TransferOwnership.t.sol +++ b/test/script/TransferOwnership.t.sol @@ -30,32 +30,19 @@ contract TestTransferOwnership is Test { proxyAsAuthenticator = GPv2AllowListAuthentication(deployed); } - function test_transfers_proxy_ownership_and_resets_manager() public { + function test_transfers_proxy_ownership_and_updates_manager() public { address newOwner = makeAddr("TestTransferOwnership: new proxy owner"); + address newManager = makeAddr("TestTransferOwnership: new authenticator manager"); assertEq(proxy.owner(), owner); assertEq(proxyAsAuthenticator.manager(), owner); TransferOwnership.ScriptParams memory params = - TransferOwnership.ScriptParams({newOwner: newOwner, authenticatorProxy: proxy, resetManager: true}); + TransferOwnership.ScriptParams({newOwner: newOwner, authenticatorProxy: proxy, newManager: newManager}); script.runWith(params); assertEq(proxy.owner(), newOwner, "did not change the owner"); - assertEq(proxyAsAuthenticator.manager(), newOwner, "did not change the manager"); - } - - function test_only_transfers_proxy_ownership() public { - address newOwner = makeAddr("TestTransferOwnership: new proxy owner"); - assertEq(proxy.owner(), owner); - assertEq(proxyAsAuthenticator.manager(), owner); - - TransferOwnership.ScriptParams memory params = - TransferOwnership.ScriptParams({newOwner: newOwner, authenticatorProxy: proxy, resetManager: false}); - - script.runWith(params); - - assertEq(proxy.owner(), newOwner, "did not change the owner"); - assertEq(proxyAsAuthenticator.manager(), owner, "changed the manager"); + assertEq(proxyAsAuthenticator.manager(), newManager, "did not change the manager"); } function test_reverts_if_no_proxy_at_target() public { @@ -63,7 +50,7 @@ contract TestTransferOwnership is Test { TransferOwnership.ScriptParams memory params = TransferOwnership.ScriptParams({ newOwner: makeAddr("some owner"), authenticatorProxy: ERC173(notAProxy), - resetManager: false + newManager: makeAddr("some manager") }); vm.expectRevert(bytes(string.concat("No code at target authenticator proxy ", vm.toString(notAProxy), "."))); @@ -75,7 +62,7 @@ contract TestTransferOwnership is Test { TransferOwnership.ScriptParams memory params = TransferOwnership.ScriptParams({ newOwner: makeAddr("some owner"), authenticatorProxy: ERC173(noERC173Proxy), - resetManager: false + newManager: makeAddr("some manager") }); vm.etch(noERC173Proxy, hex"1337"); vm.mockCall( @@ -99,7 +86,7 @@ contract TestTransferOwnership is Test { TransferOwnership.ScriptParams memory params = TransferOwnership.ScriptParams({ newOwner: makeAddr("some owner"), authenticatorProxy: ERC173(revertingProxy), - resetManager: false + newManager: makeAddr("some manager") }); vm.etch(revertingProxy, hex"1337"); vm.mockCallRevert(