From d5aaa4bb6ac59f9757faf902b2e31bd04ad9be27 Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Wed, 26 Mar 2025 19:37:06 -0400 Subject: [PATCH 1/3] simplify nested ownership transfer task --- .../2025-01-15-nested-ownership-transfer/.env | 16 +- .../Makefile | 75 ++------- .../README.md | 149 ++++++++++++++++++ .../foundry.toml | 1 - .../script/AddNestedSigner.s.sol | 71 --------- .../script/AddSigner.s.sol | 75 --------- .../script/DeploySafes.s.sol | 95 +++++++++++ .../script/RemoveSigner.s.sol | 70 -------- .../script/SetThreshold.s.sol | 43 ----- .../script/UpdateSigners.s.sol | 113 +++++++++++++ 10 files changed, 378 insertions(+), 330 deletions(-) create mode 100644 mainnet/2025-01-15-nested-ownership-transfer/README.md delete mode 100644 mainnet/2025-01-15-nested-ownership-transfer/script/AddNestedSigner.s.sol delete mode 100644 mainnet/2025-01-15-nested-ownership-transfer/script/AddSigner.s.sol create mode 100644 mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol delete mode 100644 mainnet/2025-01-15-nested-ownership-transfer/script/RemoveSigner.s.sol delete mode 100644 mainnet/2025-01-15-nested-ownership-transfer/script/SetThreshold.s.sol create mode 100644 mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol diff --git a/mainnet/2025-01-15-nested-ownership-transfer/.env b/mainnet/2025-01-15-nested-ownership-transfer/.env index 0f860353..9d74a942 100644 --- a/mainnet/2025-01-15-nested-ownership-transfer/.env +++ b/mainnet/2025-01-15-nested-ownership-transfer/.env @@ -1,12 +1,10 @@ -OP_COMMIT=dff5f16c510e7f44f1be0574372ccb08bfec045c -BASE_CONTRACTS_COMMIT=d98da18a2732b97d6318ebd9e73708e3eea84ffe +OP_COMMIT=2073f4059bd806af3e8b76b820aa3fa0b42016d0 +BASE_CONTRACTS_COMMIT=cdedd0fe728eb1f9d63eaa4c6e59138cfb3803d3 -# 3-of-6 -OWNER_SAFE=0x9855054731540A48b28990B63DcF4f33d8AE46A1 +L1_GNOSIS_SAFE_IMPLEMENTATION=0x41675C099F32341bf84BFc5382aF534df5C7461a +L1_GNOSIS_COMPATIBILITY_FALLBACK_HANDLER=0xfd0732Dc9E303f09fCEf3a7388Ad10A83459Ec99 -# 2-of-2 -SIGNER_TO_ADD= -SIGNER_TO_KEEP= +OWNER_SAFE=0x9855054731540A48b28990B63DcF4f33d8AE46A1 -# 7-of-10 -NESTED_SIGNER_TO_ADD= +SAFE_B_OWNERS_ENCODED= +SAFE_B_THRESHOLD=7 diff --git a/mainnet/2025-01-15-nested-ownership-transfer/Makefile b/mainnet/2025-01-15-nested-ownership-transfer/Makefile index 710d7758..52712400 100644 --- a/mainnet/2025-01-15-nested-ownership-transfer/Makefile +++ b/mainnet/2025-01-15-nested-ownership-transfer/Makefile @@ -6,68 +6,21 @@ ifndef LEDGER_ACCOUNT override LEDGER_ACCOUNT = 0 endif -## -# MultisigBuilder commands -## +.PHONY: deps +deps: + forge install --no-git safe-global/safe-smart-account@21dc82410445637820f600c7399a804ad55841d5 -.PHONY: sign-add-signer -sign-add-signer: - $(GOPATH)/bin/eip712sign --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" -- \ - forge script --rpc-url $(L1_RPC_URL) AddSigner --sig "sign()" - -.PHONY: run-add-signer -run-add-signer: - forge script --rpc-url $(L1_RPC_URL) \ - AddSigner --sig "run(bytes)" $(SIGNATURES) \ - --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" \ - --broadcast - -.PHONY: sign-remove-signer -sign-remove-signer: - $(GOPATH)/bin/eip712sign --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" -- \ - forge script --rpc-url $(L1_RPC_URL) RemoveSigner --sig "sign()" - -.PHONY: run-remove-signer -run-remove-signer: - forge script --rpc-url $(L1_RPC_URL) \ - RemoveSigner --sig "run(bytes)" $(SIGNATURES) \ - --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" \ - --broadcast - -## -# NestedMultisigBuilder commands -## +.PHONY: deploy +deploy: + forge script --rpc-url $(L1_RPC_URL) DeploySafes --account testnet-admin --broadcast -vvvv -.PHONY: sign-add-nested-signer -sign-add-nested-signer: +.PHONY: sign +sign: $(GOPATH)/bin/eip712sign --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" -- \ - forge script --rpc-url $(L1_RPC_URL) AddNestedSigner \ - --sig "sign(address)" $(SIGNER_TO_ADD) - -.PHONY: approve-add-nested-signer -approve-add-nested-signer: - forge script --rpc-url $(L1_RPC_URL) AddNestedSigner \ - --sig "approve(address,bytes)" $(SIGNER_TO_ADD) $(SIGNATURES) \ - --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast - -.PHONY: execute-add-nested-signer -execute-add-nested-signer: - forge script --rpc-url $(L1_RPC_URL) AddNestedSigner \ - --sig "run()" --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast - -.PHONY: sign-set-threshold -sign-set-threshold: - $(GOPATH)/bin/eip712sign --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" -- \ - forge script --rpc-url $(L1_RPC_URL) SetThreshold \ - --sig "sign(address)" $(NESTED_SIGNER_TO_ADD) - -.PHONY: approve-set-threshold -approve-set-threshold: - forge script --rpc-url $(L1_RPC_URL) SetThreshold \ - --sig "approve(address,bytes)" $(NESTED_SIGNER_TO_ADD) $(SIGNATURES) \ - --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast + forge script --rpc-url $(L1_RPC_URL) UpdateSigners --sig "sign()" -.PHONY: execute-set-threshold -execute-set-threshold: - forge script --rpc-url $(L1_RPC_URL) SetThreshold \ - --sig "run()" --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast +.PHONY: execute +execute: + forge script --rpc-url $(L1_RPC_URL) UpdateSigners \ + --sig "run(bytes)" $(SIGNATURES) \ + --ledger --hd-paths "m/44'/60'/$(LEDGER_ACCOUNT)'/0/0" --broadcast -vvvv diff --git a/mainnet/2025-01-15-nested-ownership-transfer/README.md b/mainnet/2025-01-15-nested-ownership-transfer/README.md new file mode 100644 index 00000000..5d1a0bcf --- /dev/null +++ b/mainnet/2025-01-15-nested-ownership-transfer/README.md @@ -0,0 +1,149 @@ +# Nested Ownership Transfer + +Status: READY TO DEPLOY + +## Procedure + +### 1. Update repo: + +```bash +cd contract-deployments +git pull +cd mainnet/2025-01-15-nested-ownership-transfer +make deps +``` + +### 2. Setup Ledger + +Your Ledger needs to be connected and unlocked. The Ethereum +application needs to be opened on Ledger with the message "Application +is ready". + +### 3. Run relevant script(s) + +#### 3.1 Deploy new Safes + +```bash +make deploy +``` + +This will output the new addresses of the `SafeA` and `SafeB` contracts to an `addresses.json` file. You will need to commit this file to the repo before signers can sign. + +#### 3.2 Sign the transaction + +```bash +make sign +``` + +You will see a "Simulation link" from the output. + +Paste this URL in your browser. A prompt may ask you to choose a +project, any project will do. You can create one if necessary. + +Click "Simulate Transaction". + +We will be performing 3 validations and extract the domain hash and message hash to approve on your Ledger: + +1. Validate integrity of the simulation. +2. Validate correctness of the state diff. +3. Validate and extract domain hash and message hash to approve. + +##### 3.2.1 Validate integrity of the simulation. + +Make sure you are on the "Overview" tab of the tenderly simulation, to +validate integrity of the simulation, we need to check the following: + +1. "Network": Check the network is Mainnet. +2. "Timestamp": Check the simulation is performed on a block with a + recent timestamp (i.e. close to when you run the script). +3. "Sender": Check the address shown is your signer account. If not see the derivation path Note above. + +##### 3.2.2. Validate correctness of the state diff. + +Now click on the "State" tab, and refer to the [State Validations](./VALIDATION.md) instructions for the transaction you are signing. +Once complete return to this document to complete the signing. + +##### 3.2.3. Extract the domain hash and the message hash to approve. + +Now that we have verified the transaction performs the right +operation, we need to extract the domain hash and the message hash to +approve. + +Go back to the "Overview" tab, and find the +`GnosisSafe.checkSignatures` call. This call's `data` parameter +contains both the domain hash and the message hash that will show up +in your Ledger. + +It will be a concatenation of `0x1901`, the domain hash, and the +message hash: `0x1901[domain hash][message hash]`. + +Note down this value. You will need to compare it with the ones +displayed on the Ledger screen at signing. + +Once the validations are done, it's time to actually sign the +transaction. + +> [!WARNING] +> This is the most security critical part of the playbook: make sure the +> domain hash and message hash in the following two places match: +> +> 1. On your Ledger screen. +> 2. In the Tenderly simulation. You should use the same Tenderly +> simulation as the one you used to verify the state diffs, instead +> of opening the new one printed in the console. +> +> There is no need to verify anything printed in the console. There is +> no need to open the new Tenderly simulation link either. + +After verification, sign the transaction. You will see the `Data`, +`Signer` and `Signature` printed in the console. Format should be +something like this: + +```shell +Data: +Signer:
+Signature: +``` + +Double check the signer address is the right one. + +##### 3.2.4 Send the output to Facilitator(s) + +Nothing has occurred onchain - these are offchain signatures which +will be collected by Facilitators for execution. Execution can occur +by anyone once a threshold of signatures are collected, so a +Facilitator will do the final execution for convenience. + +Share the `Data`, `Signer` and `Signature` with the Facilitator, and +congrats, you are done! + +### [For Facilitator ONLY] How to execute + +#### Execute the transaction + +1. IMPORTANT: Ensure op-challenger has been updated before executing. +1. Collect outputs from all participating signers. +1. Concatenate all signatures and export it as the `SIGNATURES` + environment variable, i.e. `export +SIGNATURES="[SIGNATURE1][SIGNATURE2]..."`. +1. Run the `make execute` command as described below to execute the transaction. + +For example, if the quorum is 2 and you get the following outputs: + +```shell +Data: 0xDEADBEEF +Signer: 0xC0FFEE01 +Signature: AAAA +``` + +```shell +Data: 0xDEADBEEF +Signer: 0xC0FFEE02 +Signature: BBBB +``` + +Then you should run: + +```bash +SIGNATURES=AAAABBBB make execute +``` diff --git a/mainnet/2025-01-15-nested-ownership-transfer/foundry.toml b/mainnet/2025-01-15-nested-ownership-transfer/foundry.toml index 14499ab0..ded43cd5 100644 --- a/mainnet/2025-01-15-nested-ownership-transfer/foundry.toml +++ b/mainnet/2025-01-15-nested-ownership-transfer/foundry.toml @@ -6,7 +6,6 @@ broadcast = 'records' fs_permissions = [{ access = "read-write", path = "./" }] optimizer = true optimizer_runs = 999999 -solc_version = "0.8.15" via-ir = false remappings = [ '@eth-optimism-bedrock/=lib/optimism/packages/contracts-bedrock/', diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/AddNestedSigner.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/AddNestedSigner.s.sol deleted file mode 100644 index 565443e2..00000000 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/AddNestedSigner.s.sol +++ /dev/null @@ -1,71 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import "@base-contracts/script/universal/NestedMultisigBuilder.sol"; - -// Adds new multisig signer to existing multisig and updates threshold to 1 -// New signer should be a 7-of-10 multisig -contract AddNestedSigner is NestedMultisigBuilder { - uint256 internal constant _THRESHOLD = 1; - - uint256 internal constant _EXPECTED_NEW_SIGNER_THRESHOLD = 7; - uint256 internal constant _EXPECTED_NEW_SIGNER_OWNER_COUNT = 10; - - uint256 internal constant _EXPECTED_STARTING_OWNER_THRESHOLD = 1; - uint256 internal constant _EXPECTED_STARTING_OWNER_COUNT = 1; - - address internal _NESTED_SIGNER_TO_ADD = vm.envAddress("NESTED_SIGNER_TO_ADD"); - address internal _OWNER_SAFE = vm.envAddress("OWNER_SAFE"); - - function setUp() public view { - require( - IGnosisSafe(_NESTED_SIGNER_TO_ADD).getThreshold() == _EXPECTED_NEW_SIGNER_THRESHOLD, - "Precheck child signer threshold mismatch" - ); - require( - IGnosisSafe(_NESTED_SIGNER_TO_ADD).getOwners().length == _EXPECTED_NEW_SIGNER_OWNER_COUNT, - "Precheck child signer owner count mismatch" - ); - - require( - IGnosisSafe(_OWNER_SAFE).getThreshold() == _EXPECTED_STARTING_OWNER_THRESHOLD, - "Precheck owner threshold mismatch" - ); - require( - IGnosisSafe(_OWNER_SAFE).getOwners().length == _EXPECTED_STARTING_OWNER_COUNT, "Precheck length mismatch" - ); - } - - function _postCheck(Vm.AccountAccess[] memory, Simulation.Payload memory) internal view override { - require( - IGnosisSafe(_NESTED_SIGNER_TO_ADD).getThreshold() == _EXPECTED_NEW_SIGNER_THRESHOLD, - "Postcheck child signer threshold mismatch" - ); - require( - IGnosisSafe(_NESTED_SIGNER_TO_ADD).getOwners().length == _EXPECTED_NEW_SIGNER_OWNER_COUNT, - "Postcheck child signer owner count mismatch" - ); - - address[] memory ownerSafeOwners = IGnosisSafe(_OWNER_SAFE).getOwners(); - require(ownerSafeOwners[0] == _NESTED_SIGNER_TO_ADD, "Postcheck new signer mismatch"); - - require(IGnosisSafe(_OWNER_SAFE).getThreshold() == _THRESHOLD, "Postcheck threshold mismatch"); - require(ownerSafeOwners.length == _EXPECTED_STARTING_OWNER_COUNT + 1, "Postcheck length mismatch"); - } - - function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); - - calls[0] = IMulticall3.Call3({ - target: _OWNER_SAFE, - allowFailure: false, - callData: abi.encodeCall(IGnosisSafe.addOwnerWithThreshold, (_NESTED_SIGNER_TO_ADD, _THRESHOLD)) - }); - - return calls; - } - - function _ownerSafe() internal view override returns (address) { - return _OWNER_SAFE; - } -} diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/AddSigner.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/AddSigner.s.sol deleted file mode 100644 index 01cb69f3..00000000 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/AddSigner.s.sol +++ /dev/null @@ -1,75 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import "@base-contracts/script/universal/MultisigBuilder.sol"; - -// Adds `_SIGNER_TO_ADD` as an owner to `_OWNER_SAFE` and sets threshold to 1 -contract AddSigner is MultisigBuilder { - uint256 internal constant _THRESHOLD = 1; - - uint256 internal constant _EXPECTED_NEW_SIGNER_THRESHOLD = 3; - uint256 internal constant _EXPECTED_NEW_SIGNER_OWNER_COUNT = 6; - - uint256 internal constant _EXPECTED_STARTING_OWNER_THRESHOLD = 3; - uint256 internal constant _EXPECTED_STARTING_OWNER_COUNT = 6; - - address internal _SIGNER_TO_ADD = vm.envAddress("SIGNER_TO_ADD"); - address internal _OWNER_SAFE = vm.envAddress("OWNER_SAFE"); - - function setUp() public view { - address[] memory newSignerOwners = IGnosisSafe(_SIGNER_TO_ADD).getOwners(); - address[] memory ownerSafeOwners = IGnosisSafe(_OWNER_SAFE).getOwners(); - - require( - IGnosisSafe(_SIGNER_TO_ADD).getThreshold() == _EXPECTED_NEW_SIGNER_THRESHOLD, - "Precheck new signer threshold mismatch" - ); - require(newSignerOwners.length == _EXPECTED_NEW_SIGNER_OWNER_COUNT, "Precheck length mismatch"); - - require( - IGnosisSafe(_OWNER_SAFE).getThreshold() == _EXPECTED_STARTING_OWNER_THRESHOLD, - "Precheck owner threshold mismatch" - ); - require(ownerSafeOwners.length == _EXPECTED_STARTING_OWNER_COUNT, "Precheck owner count mismatch"); - - for (uint256 i; i < ownerSafeOwners.length; i++) { - require(ownerSafeOwners[i] == newSignerOwners[i], "Precheck owner mismatch"); - } - } - - function _postCheck(Vm.AccountAccess[] memory, Simulation.Payload memory) internal view override { - address[] memory newSignerOwners = IGnosisSafe(_SIGNER_TO_ADD).getOwners(); - address[] memory ownerSafeOwners = IGnosisSafe(_OWNER_SAFE).getOwners(); - - require( - IGnosisSafe(_SIGNER_TO_ADD).getThreshold() == _EXPECTED_NEW_SIGNER_THRESHOLD, - "Postcheck new signer threshold mismatch" - ); - require(newSignerOwners.length == _EXPECTED_NEW_SIGNER_OWNER_COUNT, "Postcheck length mismatch"); - - require(IGnosisSafe(_OWNER_SAFE).getThreshold() == _THRESHOLD, "Postcheck owner threshold mismatch"); - require(ownerSafeOwners.length == _EXPECTED_STARTING_OWNER_COUNT + 1, "Postcheck owner count mismatch"); - - require(ownerSafeOwners[0] == _SIGNER_TO_ADD, "Postcheck new signer mismatch"); - - for (uint256 i; i < newSignerOwners.length; i++) { - require(ownerSafeOwners[i + 1] == newSignerOwners[i], "Postcheck owner mismatch"); - } - } - - function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); - - calls[0] = IMulticall3.Call3({ - target: _OWNER_SAFE, - allowFailure: false, - callData: abi.encodeCall(IGnosisSafe.addOwnerWithThreshold, (_SIGNER_TO_ADD, _THRESHOLD)) - }); - - return calls; - } - - function _ownerSafe() internal view override returns (address) { - return _OWNER_SAFE; - } -} diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol new file mode 100644 index 00000000..e57b54c7 --- /dev/null +++ b/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Script} from "forge-std/Script.sol"; +import {Safe} from "safe-smart-account/contracts/Safe.sol"; +import {SafeProxy} from "safe-smart-account/contracts/proxies/SafeProxy.sol"; +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; +import {console} from "forge-std/console.sol"; + +contract DeploySafes is Script { + using Strings for address; + + address private SAFE_IMPLEMENTATION = vm.envAddress("L1_GNOSIS_SAFE_IMPLEMENTATION"); + address private FALLBACK_HANDLER = vm.envAddress("L1_GNOSIS_COMPATIBILITY_FALLBACK_HANDLER"); + address private OWNER_SAFE = vm.envAddress("OWNER_SAFE"); + address private zAddr; + + address[] private OWNER_SAFE_OWNERS; + uint256 private OWNER_SAFE_THRESHOLD; + + address[] private SAFE_B_OWNERS; + uint256 private SAFE_B_THRESHOLD; + + function run() public { + Safe ownerSafe = Safe(payable(OWNER_SAFE)); + OWNER_SAFE_OWNERS = ownerSafe.getOwners(); + OWNER_SAFE_THRESHOLD = ownerSafe.getThreshold(); + + SAFE_B_OWNERS = abi.decode(vm.envBytes("SAFE_B_OWNERS_ENCODED"), (address[])); + SAFE_B_THRESHOLD = vm.envUint("SAFE_B_THRESHOLD"); + + console.log("Deploying SafeA with owners:"); + _printOwners(OWNER_SAFE_OWNERS); + + console.log("Deploying SafeB with owners:"); + _printOwners(SAFE_B_OWNERS); + + console.log("Threshold of SafeA:", OWNER_SAFE_THRESHOLD); + console.log("Threshold of SafeB:", SAFE_B_THRESHOLD); + + vm.startBroadcast(); + // First safe maintains the same owners + threshold as the current owner safe + address safeA = _createAndInitProxy(OWNER_SAFE_OWNERS, OWNER_SAFE_THRESHOLD); + // Second safe just uses threshold of 1 + address safeB = _createAndInitProxy(SAFE_B_OWNERS, SAFE_B_THRESHOLD); + vm.stopBroadcast(); + _postCheck(safeA, safeB); + + vm.writeFile( + "addresses.json", + string.concat( + "{", "\"SafeA\": \"", safeA.toHexString(), "\",", "\"SafeB\": \"", safeB.toHexString(), "\"" "}" + ) + ); + } + + function _postCheck(address safeAAddress, address safeBAddress) private view { + Safe safeA = Safe(payable(safeAAddress)); + Safe safeB = Safe(payable(safeBAddress)); + + address[] memory safeAOwners = safeA.getOwners(); + uint256 safeAThreshold = safeA.getThreshold(); + + address[] memory safeBOwners = safeB.getOwners(); + uint256 safeBThreshold = safeB.getThreshold(); + + require(safeAThreshold == OWNER_SAFE_THRESHOLD, "PostCheck 1"); + require(safeBThreshold == SAFE_B_THRESHOLD, "PostCheck 2"); + + require(safeAOwners.length == OWNER_SAFE_OWNERS.length, "PostCheck 3"); + require(safeBOwners.length == SAFE_B_OWNERS.length, "PostCheck 4"); + + for (uint256 i; i < safeAOwners.length; i++) { + require(safeAOwners[i] == OWNER_SAFE_OWNERS[i], "PostCheck 5"); + } + + for (uint256 i; i < safeBOwners.length; i++) { + require(safeBOwners[i] == SAFE_B_OWNERS[i], "PostCheck 6"); + } + + console.log("PostCheck passed"); + } + + function _createAndInitProxy(address[] memory owners, uint256 threshold) private returns (address) { + Safe proxy = Safe(payable(address(new SafeProxy(SAFE_IMPLEMENTATION)))); + proxy.setup(owners, threshold, zAddr, "", FALLBACK_HANDLER, zAddr, 0, payable(zAddr)); + return address(proxy); + } + + function _printOwners(address[] memory owners) private pure { + for (uint256 i; i < owners.length; i++) { + console.logAddress(owners[i]); + } + } +} diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/RemoveSigner.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/RemoveSigner.s.sol deleted file mode 100644 index 3415d4fa..00000000 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/RemoveSigner.s.sol +++ /dev/null @@ -1,70 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import "@base-contracts/script/universal/MultisigBuilder.sol"; - -// Removes a signer from a multisig. -contract RemoveSigner is MultisigBuilder { - uint256 private constant _THRESHOLD = 1; - - uint256 internal constant _EXPECTED_KEEPER_THRESHOLD = 3; - uint256 internal constant _EXPECTED_KEEPER_OWNER_COUNT = 6; - - address internal _SIGNER_TO_KEEP = vm.envAddress("SIGNER_TO_KEEP"); - address internal _OWNER_SAFE = vm.envAddress("OWNER_SAFE"); - - address prevOwner; - address ownerToRemove; - uint256 ownerCountBefore; - - function setUp() public { - require( - IGnosisSafe(_SIGNER_TO_KEEP).getThreshold() == _EXPECTED_KEEPER_THRESHOLD, "Precheck threshold mismatch" - ); - require( - IGnosisSafe(_SIGNER_TO_KEEP).getOwners().length == _EXPECTED_KEEPER_OWNER_COUNT, "Precheck length mismatch" - ); - - address[] memory currentOwners = IGnosisSafe(_OWNER_SAFE).getOwners(); - uint256 length = currentOwners.length; - - require(length > 1, "No signers to remove"); - - prevOwner = currentOwners[length - 2]; - ownerToRemove = currentOwners[length - 1]; - ownerCountBefore = length; - - require(ownerToRemove != _SIGNER_TO_KEEP, "Owner to remove is the new signer"); - } - - function _postCheck(Vm.AccountAccess[] memory, Simulation.Payload memory) internal view override { - require( - IGnosisSafe(_SIGNER_TO_KEEP).getThreshold() == _EXPECTED_KEEPER_THRESHOLD, "Precheck threshold mismatch" - ); - require( - IGnosisSafe(_SIGNER_TO_KEEP).getOwners().length == _EXPECTED_KEEPER_OWNER_COUNT, "Precheck length mismatch" - ); - - require(IGnosisSafe(_OWNER_SAFE).getThreshold() == _THRESHOLD, "Postcheck threshold mismatch"); - require(IGnosisSafe(_OWNER_SAFE).getOwners().length == ownerCountBefore - 1, "Postcheck: Invalid owner count"); - - require(!IGnosisSafe(_OWNER_SAFE).isOwner(ownerToRemove), "Postcheck: Owner not removed"); - require(IGnosisSafe(_OWNER_SAFE).isOwner(_SIGNER_TO_KEEP), "Postcheck: New signer was removed"); - } - - function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); - - calls[0] = IMulticall3.Call3({ - target: _OWNER_SAFE, - allowFailure: false, - callData: abi.encodeCall(IGnosisSafe.removeOwner, (prevOwner, ownerToRemove, _THRESHOLD)) - }); - - return calls; - } - - function _ownerSafe() internal view override returns (address) { - return _OWNER_SAFE; - } -} diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/SetThreshold.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/SetThreshold.s.sol deleted file mode 100644 index e0d3ce0d..00000000 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/SetThreshold.s.sol +++ /dev/null @@ -1,43 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import "@base-contracts/script/universal/NestedMultisigBuilder.sol"; - -// Sets threshold of nested multisig -contract SetThreshold is NestedMultisigBuilder { - uint256 internal constant _THRESHOLD = 2; - - uint256 internal constant _EXPECTED_STARTING_OWNER_THRESHOLD = 1; - uint256 internal constant _EXPECTED_OWNER_COUNT = 2; - - address internal _OWNER_SAFE = vm.envAddress("OWNER_SAFE"); - - function setUp() public view { - require( - IGnosisSafe(_OWNER_SAFE).getThreshold() == _EXPECTED_STARTING_OWNER_THRESHOLD, - "Precheck owner threshold mismatch" - ); - require(IGnosisSafe(_OWNER_SAFE).getOwners().length == _EXPECTED_OWNER_COUNT, "Precheck length mismatch"); - } - - function _postCheck(Vm.AccountAccess[] memory, Simulation.Payload memory) internal view override { - require(IGnosisSafe(_OWNER_SAFE).getThreshold() == _THRESHOLD, "Postcheck threshold mismatch"); - require(IGnosisSafe(_OWNER_SAFE).getOwners().length == _EXPECTED_OWNER_COUNT, "Postcheck length mismatch"); - } - - function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); - - calls[0] = IMulticall3.Call3({ - target: _OWNER_SAFE, - allowFailure: false, - callData: abi.encodeCall(IGnosisSafe.changeThreshold, (_THRESHOLD)) - }); - - return calls; - } - - function _ownerSafe() internal view override returns (address) { - return _OWNER_SAFE; - } -} diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol new file mode 100644 index 00000000..75d5b082 --- /dev/null +++ b/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Vm} from "forge-std/Vm.sol"; +import {stdJson} from "forge-std/StdJson.sol"; +import {IMulticall3} from "forge-std/interfaces/IMulticall3.sol"; + +import {MultisigBuilder} from "@base-contracts/script/universal/MultisigBuilder.sol"; +import {Simulation} from "@base-contracts/script/universal/Simulation.sol"; +import {Safe} from "safe-smart-account/contracts/Safe.sol"; +import {OwnerManager} from "safe-smart-account/contracts/base/OwnerManager.sol"; + +// Adds `SAFE_A` and `SAFE_B` as owners to `OWNER_SAFE` and sets threshold to 2 +// `SAFE_A` should have same owners as `OWNER_SAFE` +// `SAFE_A` should have same threshold as `OWNER_SAFE` +// `SAFE_B` should be a 7-of-10 multisig +contract UpdateSigners is MultisigBuilder { + using stdJson for string; + + address payable private SAFE_A; + address payable private SAFE_B; + + uint256 internal constant THRESHOLD = 2; + address payable internal OWNER_SAFE = payable(vm.envAddress("OWNER_SAFE")); + string private ADDRESSES; + + address[] private A_OWNERS; + address[] private B_OWNERS; + + uint256 private A_THRESHOLD; + uint256 private B_THRESHOLD; + + function setUp() public { + string memory rootPath = vm.projectRoot(); + string memory path = string.concat(rootPath, "/addresses.json"); + ADDRESSES = vm.readFile(path); + + SAFE_A = payable(ADDRESSES.readAddress(".SafeA")); + SAFE_B = payable(ADDRESSES.readAddress(".SafeB")); + + A_OWNERS = Safe(SAFE_A).getOwners(); + A_THRESHOLD = Safe(SAFE_A).getThreshold(); + + B_OWNERS = Safe(SAFE_B).getOwners(); + B_THRESHOLD = Safe(SAFE_B).getThreshold(); + + address[] memory ownerSafeOwners = Safe(OWNER_SAFE).getOwners(); + uint256 ownerSafeThreshold = Safe(OWNER_SAFE).getThreshold(); + + require(ownerSafeOwners.length == A_OWNERS.length, "Precheck owner count mismatch - A"); + + require(ownerSafeThreshold == A_THRESHOLD, "Precheck threshold mismatch - A"); + + for (uint256 i; i < ownerSafeOwners.length; i++) { + require(ownerSafeOwners[i] == A_OWNERS[i], "Precheck owner mismatch - A"); + } + } + + function _postCheck(Vm.AccountAccess[] memory, Simulation.Payload memory) internal view override { + address[] memory aOwners = Safe(SAFE_A).getOwners(); + address[] memory bOwners = Safe(SAFE_B).getOwners(); + + uint256 aThreshold = Safe(SAFE_A).getThreshold(); + uint256 bThreshold = Safe(SAFE_B).getThreshold(); + + address[] memory ownerSafeOwners = Safe(OWNER_SAFE).getOwners(); + + uint256 ownerSafeThreshold = Safe(OWNER_SAFE).getThreshold(); + + require(aThreshold == A_THRESHOLD, "Postcheck new signer threshold mismatch - A"); + require(bThreshold == B_THRESHOLD, "Postcheck new signer threshold mismatch - B"); + + require(aOwners.length == A_OWNERS.length, "Postcheck length mismatch - A"); + require(bOwners.length == B_OWNERS.length, "Postcheck length mismatch - B"); + + require(ownerSafeThreshold == THRESHOLD, "Postcheck owner threshold mismatch"); + require(ownerSafeOwners.length == 2, "Postcheck owner count mismatch"); + + require(ownerSafeOwners[0] == SAFE_B, "Postcheck new signer mismatch - B"); + require(ownerSafeOwners[1] == SAFE_A, "Postcheck new signer mismatch - A"); + } + + function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2 + A_OWNERS.length); + + calls[0] = IMulticall3.Call3({ + target: OWNER_SAFE, + allowFailure: false, + callData: abi.encodeCall(OwnerManager.addOwnerWithThreshold, (SAFE_A, THRESHOLD)) + }); + calls[1] = IMulticall3.Call3({ + target: OWNER_SAFE, + allowFailure: false, + callData: abi.encodeCall(OwnerManager.addOwnerWithThreshold, (SAFE_B, THRESHOLD)) + }); + + address prevOwner = SAFE_A; + + for (uint256 i; i < A_OWNERS.length; i++) { + calls[2 + i] = IMulticall3.Call3({ + target: OWNER_SAFE, + allowFailure: false, + callData: abi.encodeCall(OwnerManager.removeOwner, (prevOwner, A_OWNERS[i], THRESHOLD)) + }); + } + + return calls; + } + + function _ownerSafe() internal view override returns (address) { + return OWNER_SAFE; + } +} From 7f26d6320a543a99d1bc54aee4930626616f5078 Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Mon, 31 Mar 2025 14:18:35 -0400 Subject: [PATCH 2/3] add more ownership transfer checks --- .../script/DeploySafes.s.sol | 16 +++++++++++----- .../script/UpdateSigners.s.sol | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol index e57b54c7..e1c9d038 100644 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol +++ b/mainnet/2025-01-15-nested-ownership-transfer/script/DeploySafes.s.sol @@ -29,6 +29,12 @@ contract DeploySafes is Script { SAFE_B_OWNERS = abi.decode(vm.envBytes("SAFE_B_OWNERS_ENCODED"), (address[])); SAFE_B_THRESHOLD = vm.envUint("SAFE_B_THRESHOLD"); + require(OWNER_SAFE_OWNERS.length == 6, "Owner safe owners length must be 6"); + require(SAFE_B_OWNERS.length == 10, "Safe B owners length must be 10"); + + require(OWNER_SAFE_THRESHOLD == 3, "Owner safe threshold must be 3"); + require(SAFE_B_THRESHOLD == 7, "Safe B threshold must be 7"); + console.log("Deploying SafeA with owners:"); _printOwners(OWNER_SAFE_OWNERS); @@ -41,7 +47,7 @@ contract DeploySafes is Script { vm.startBroadcast(); // First safe maintains the same owners + threshold as the current owner safe address safeA = _createAndInitProxy(OWNER_SAFE_OWNERS, OWNER_SAFE_THRESHOLD); - // Second safe just uses threshold of 1 + // Second safe specifies its own owners + threshold address safeB = _createAndInitProxy(SAFE_B_OWNERS, SAFE_B_THRESHOLD); vm.stopBroadcast(); _postCheck(safeA, safeB); @@ -64,11 +70,11 @@ contract DeploySafes is Script { address[] memory safeBOwners = safeB.getOwners(); uint256 safeBThreshold = safeB.getThreshold(); - require(safeAThreshold == OWNER_SAFE_THRESHOLD, "PostCheck 1"); - require(safeBThreshold == SAFE_B_THRESHOLD, "PostCheck 2"); + require(safeAThreshold == 3, "PostCheck 1"); + require(safeBThreshold == 7, "PostCheck 2"); - require(safeAOwners.length == OWNER_SAFE_OWNERS.length, "PostCheck 3"); - require(safeBOwners.length == SAFE_B_OWNERS.length, "PostCheck 4"); + require(safeAOwners.length == 6, "PostCheck 3"); + require(safeBOwners.length == 10, "PostCheck 4"); for (uint256 i; i < safeAOwners.length; i++) { require(safeAOwners[i] == OWNER_SAFE_OWNERS[i], "PostCheck 5"); diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol index 75d5b082..53856d20 100644 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol +++ b/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol @@ -41,9 +41,15 @@ contract UpdateSigners is MultisigBuilder { A_OWNERS = Safe(SAFE_A).getOwners(); A_THRESHOLD = Safe(SAFE_A).getThreshold(); + require(A_OWNERS.length == 6, "A owners length must be 6"); + require(A_THRESHOLD == 3, "A threshold must be 3"); + B_OWNERS = Safe(SAFE_B).getOwners(); B_THRESHOLD = Safe(SAFE_B).getThreshold(); + require(B_OWNERS.length == 10, "B owners length must be 10"); + require(B_THRESHOLD == 7, "B threshold must be 7"); + address[] memory ownerSafeOwners = Safe(OWNER_SAFE).getOwners(); uint256 ownerSafeThreshold = Safe(OWNER_SAFE).getThreshold(); @@ -78,6 +84,14 @@ contract UpdateSigners is MultisigBuilder { require(ownerSafeOwners[0] == SAFE_B, "Postcheck new signer mismatch - B"); require(ownerSafeOwners[1] == SAFE_A, "Postcheck new signer mismatch - A"); + + for (uint256 i; i < aOwners.length; i++) { + require(aOwners[i] == A_OWNERS[i], "PostCheck 5"); + } + + for (uint256 i; i < bOwners.length; i++) { + require(bOwners[i] == B_OWNERS[i], "PostCheck 6"); + } } function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { From 1256cde018a574110c7889ca7be459ae3cc31b07 Mon Sep 17 00:00:00 2001 From: Baptiste Oueriagli Date: Thu, 3 Apr 2025 10:26:13 +0200 Subject: [PATCH 3/3] invalidate review --- .../script/UpdateSigners.s.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol b/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol index 53856d20..e9332e43 100644 --- a/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol +++ b/mainnet/2025-01-15-nested-ownership-transfer/script/UpdateSigners.s.sol @@ -54,9 +54,7 @@ contract UpdateSigners is MultisigBuilder { uint256 ownerSafeThreshold = Safe(OWNER_SAFE).getThreshold(); require(ownerSafeOwners.length == A_OWNERS.length, "Precheck owner count mismatch - A"); - require(ownerSafeThreshold == A_THRESHOLD, "Precheck threshold mismatch - A"); - for (uint256 i; i < ownerSafeOwners.length; i++) { require(ownerSafeOwners[i] == A_OWNERS[i], "Precheck owner mismatch - A"); }