Swap out Safe requirements for generic multisig

Author: JAG-UK

src/StreamWeightActor.sol

@@ -9,7 +9,7 @@ import {FVMRewards} from "./lib/FVMRewards.sol";
 import {PendingOp, Share, WeightRecord, WeightRecordUpdate} from "./lib/FVMRewardTypes.sol";
 import {OwnersLibrary} from "./lib/Owners.sol";
 import {UnanimousGovernance} from "./lib/UnanimousGovernance.sol";
-import {IsASafe} from "./lib/IsASafe.sol";
+import {IsAMultisig} from "./lib/IsAMultisig.sol";
 
 uint64 constant SERVICE_ID = 2;
 
@@ -20,20 +20,20 @@ int256 constant STEP = 5e16; // 5%
 /// @dev Writes require unanimous owner approval, except `cancelPending`/`cancelPendingWeight`
 /// (any single owner, immediate) and `quarterlyGateCheck` (fully permissionless).
 contract StreamWeightActor is UnanimousGovernance {
-    using IsASafe for address;
+    using IsAMultisig for address;
     using OwnersLibrary for address;
 
     IServiceRewardsActor immutable SRA;
     Epoch immutable QUARTER;
     Epoch immutable HOLD;
 
     /// @notice Deploys the actor with its two initial owners, bound to a Service Rewards Actor.
-    /// @param owner1 First owner; must be a Safe.
-    /// @param owner2 Second owner; must be a Safe.
+    /// @param owner1 First owner; must be a multisig with more than one signer.
+    /// @param owner2 Second owner; must be a multisig with more than one signer.
     /// @param sra Service Rewards Actor supplying QUARTER/HOLD and gating `quarterlyGateCheck`.
     constructor(address owner1, address owner2, IServiceRewardsActor sra) {
-        owner1.isProbablyASafe();
-        owner2.isProbablyASafe();
+        owner1.requireMultisig();
+        owner2.requireMultisig();
 
         owner1.addOwner();
         owner2.addOwner();
@@ -118,9 +118,9 @@ contract StreamWeightActor is UnanimousGovernance {
 
     /// @notice Replaces one of the two owners.
     /// @param prevOwner Owner being removed.
-    /// @param newOwner Owner being added; must be a Safe.
+    /// @param newOwner Owner being added; must be a multisig with more than one signer.
     function replaceOwner(address prevOwner, address newOwner) external unanimousNoHold(keccak256(msg.data)) {
-        newOwner.isProbablyASafe();
+        newOwner.requireMultisig();
         prevOwner.removeOwner();
         newOwner.addOwner();
     }

src/lib/IsAMultisig.sol

@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+pragma solidity ^0.8.36;
+
+/// @notice The only thing we require of a multisig: it can list its signers.
+/// @dev Safe{Wallet} and the other common multisig implementations all expose this.
+interface IMultisig {
+    function getOwners() external view returns (address[] memory);
+}
+
+library IsAMultisig {
+    error NotAMultisig(address account);
+    error TooFewSigners(address account, uint256 signers);
+
+    /// @notice Requires that `account` is a contract behaving like a multisig with >1 signer.
+    /// @dev FIP-0118 mandates no specific multisig implementation, so this deliberately checks
+    /// behaviour, not identity: any contract answering `getOwners()` with two or more signers
+    /// qualifies, and EOAs (no code, so no return data) do not.
+    function requireMultisig(address account) internal view {
+        (bool ok, bytes memory ret) = account.staticcall(abi.encodeCall(IMultisig.getOwners, ()));
+        // 64 bytes is the shortest well-formed `address[]` encoding: head offset + length
+        require(ok && ret.length >= 64, NotAMultisig(account));
+        address[] memory signers = abi.decode(ret, (address[]));
+        require(signers.length > 1, TooFewSigners(account, signers.length));
+    }
+}

src/lib/IsASafe.sol

@@ -1,17 +1,17 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 pragma solidity ^0.8.36;
 
-import {SafeProxy} from "@safe/proxies/SafeProxy.sol";
-
 import {USR_FORBIDDEN, USR_ILLEGAL_ARGUMENT, USR_NOT_FOUND} from "fvm-solidity/FVMErrors.sol";
 
+import {MockMultisig} from "./mocks/MockMultisig.sol";
 import {MockRewardTest} from "./mocks/MockRewardTest.sol";
 import {WAD} from "./mocks/FVMRewardActor.sol";
 import {StreamWeightActor} from "../src/StreamWeightActor.sol";
 import {IServiceRewardsActor} from "../src/interfaces/IServiceRewardsActor.sol";
 import {PendingOp, Share, WeightRecord, WeightRecordUpdate} from "../src/lib/FVMRewardTypes.sol";
 import {Epoch} from "../src/lib/Epoch.sol";
 import {FVMRewards} from "../src/lib/FVMRewards.sol";
+import {IsAMultisig} from "../src/lib/IsAMultisig.sol";
 import {SWA_TIMELOCK} from "../src/lib/FVMRewardMethod.sol";
 
 contract StreamWeightActorTest is MockRewardTest {
@@ -27,8 +27,8 @@ contract StreamWeightActorTest is MockRewardTest {
 
     function setUp() public override {
         super.setUp();
-        owner1 = _makeSafeOwner("owner1");
-        owner2 = _makeSafeOwner("owner2");
+        owner1 = _makeMultisigOwner("owner1");
+        owner2 = _makeMultisigOwner("owner2");
 
         address sra = makeAddr("sra");
         vm.mockCall(
@@ -40,16 +40,9 @@ contract StreamWeightActorTest is MockRewardTest {
         rewardActor().mockSwa(address(actor));
     }
 
-    function _makeSafeOwner(string memory label) internal returns (address proxyAddr) {
-        address masterCopy = makeAddr(string.concat(label, "-mastercopy"));
-        vm.etch(masterCopy, new bytes(8001));
-
-        SafeProxy real = new SafeProxy(masterCopy);
-        bytes memory code = address(real).code;
-
-        proxyAddr = makeAddr(label);
-        vm.etch(proxyAddr, code);
-        vm.store(proxyAddr, bytes32(0), bytes32(uint256(uint160(masterCopy))));
+    function _makeMultisigOwner(string memory label) internal returns (address multisig) {
+        multisig = address(new MockMultisig(2));
+        vm.label(multisig, label);
     }
 
     // -------------------------------------------------------------------------
@@ -285,7 +278,7 @@ contract StreamWeightActorTest is MockRewardTest {
     // -------------------------------------------------------------------------
 
     function test_ReplaceOwner_Success_SwapsApprovalRights() public {
-        address newOwner = _makeSafeOwner("newOwner");
+        address newOwner = _makeMultisigOwner("newOwner");
 
         vm.prank(owner1);
         actor.replaceOwner(owner2, newOwner);
@@ -301,4 +294,30 @@ contract StreamWeightActorTest is MockRewardTest {
         vm.prank(newOwner);
         actor.cancelPendingWeight(PendingOp.SET_WEIGHT);
     }
+
+    // -------------------------------------------------------------------------
+    // Owner must be a multisig
+    // -------------------------------------------------------------------------
+
+    function test_Constructor_EoaOwner_RevertsNotAMultisig() public {
+        address eoa = makeAddr("eoa");
+        vm.expectRevert(abi.encodeWithSelector(IsAMultisig.NotAMultisig.selector, eoa));
+        new StreamWeightActor(owner1, eoa, IServiceRewardsActor(makeAddr("sra")));
+    }
+
+    function test_Constructor_SingleSignerOwner_RevertsTooFewSigners() public {
+        address solo = address(new MockMultisig(1));
+        vm.expectRevert(abi.encodeWithSelector(IsAMultisig.TooFewSigners.selector, solo, uint256(1)));
+        new StreamWeightActor(owner1, solo, IServiceRewardsActor(makeAddr("sra")));
+    }
+
+    function test_ReplaceOwner_EoaOwner_RevertsNotAMultisig() public {
+        address eoa = makeAddr("eoa");
+
+        vm.prank(owner1);
+        actor.replaceOwner(owner2, eoa);
+        vm.prank(owner2);
+        vm.expectRevert(abi.encodeWithSelector(IsAMultisig.NotAMultisig.selector, eoa));
+        actor.replaceOwner(owner2, eoa);
+    }
 }

test/StreamWeightActor.t.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+pragma solidity ^0.8.36;
+
+/// @notice Minimal stand-in for any multisig: it just lists its signers.
+contract MockMultisig {
+    address[] internal signers;
+
+    constructor(uint256 count) {
+        for (uint256 i = 0; i < count; i++) {
+            signers.push(address(uint160(i + 1)));
+        }
+    }
+
+    function getOwners() external view returns (address[] memory) {
+        return signers;
+    }
+}