fixed bug in validation

rhinestonewtf · kopy-kat · Jul 8, 2024 · Jun 27, 2024 · Jun 27, 2024 · Jun 27, 2024
commit 349ff7274a366f221b87e02f355a6fb59bfe0d3e
diff --git a/src/Safe7579Launchpad.sol b/src/Safe7579Launchpad.sol
@@ -19,6 +19,7 @@ import { SafeStorage } from "@safe-global/safe-contracts/contracts/libraries/Saf
 import { MODULE_TYPE_VALIDATOR } from "erc7579/interfaces/IERC7579Module.sol";
 import { CheckSignatures } from "@rhinestone/checknsignatures/src/CheckNSignatures.sol";
 import { LibSort } from "solady/utils/LibSort.sol";
+import "forge-std/console2.sol";
 
 /**
  * Launchpad to deploy a Safe account and connect the Safe7579 adapter.
@@ -222,33 +223,40 @@ contract Safe7579Launchpad is IAccount, SafeStorage {
         // ensure that the call was successful
         if (!success) revert InvalidUserOperationData();
 
+        // to support validation with the safe native signers, we allow to set the validator module
+        // to address(0)
         if (validator == address(0)) {
             if (!_isValidSafeSigners(userOpHash, userOp)) {
                 return _packValidationData({ sigFailed: true, validUntil: 0, validAfter: 0 });
             } else {
-                return _packValidationData({ sigFailed: false, validUntil: 0, validAfter: 0 });
+                validationData =
+                    _packValidationData({ sigFailed: false, validUntil: 0, validAfter: 0 });
             }
         }
-
-        // Call onInstall on each validator module to set up the validators.
-        // Since this function is delegatecalled by the SafeProxy, the Validator Module is called
-        // with msg.sender == SafeProxy.
-        bool userOpValidatorInstalled;
-        uint256 validatorsLength = initData.validators.length;
-        for (uint256 i; i < validatorsLength; i++) {
-            address validatorModule = initData.validators[i].module;
-            IValidator(validatorModule).onInstall(initData.validators[i].initData);
-            emit ModuleInstalled(MODULE_TYPE_VALIDATOR, validatorModule);
-
-            if (validatorModule == validator) userOpValidatorInstalled = true;
-        }
-        // Ensure that the validator module selected in the userOp was
-        // part of the validators in InitData
-        if (!userOpValidatorInstalled) {
-            return _packValidationData({ sigFailed: true, validUntil: 0, validAfter: 0 });
+        // if the validator module is non address(0), we validate the userOp with the validator
+        // module
+        else {
+            // Call onInstall on each validator module to set up the validators.
+            // Since this function is delegatecalled by the SafeProxy, the Validator Module is
+            // called
+            // with msg.sender == SafeProxy.
+            bool userOpValidatorInstalled;
+            uint256 validatorsLength = initData.validators.length;
+            for (uint256 i; i < validatorsLength; i++) {
+                address validatorModule = initData.validators[i].module;
+                IValidator(validatorModule).onInstall(initData.validators[i].initData);
+                emit ModuleInstalled(MODULE_TYPE_VALIDATOR, validatorModule);
+
+                if (validatorModule == validator) userOpValidatorInstalled = true;
+            }
+            // Ensure that the validator module selected in the userOp was
+            // part of the validators in InitData
+            if (!userOpValidatorInstalled) {
+                return _packValidationData({ sigFailed: true, validUntil: 0, validAfter: 0 });
+            }
+            // validate userOp with selected validation module.
+            validationData = IValidator(validator).validateUserOp(userOp, userOpHash);
         }
-        // validate userOp with selected validation module.
-        validationData = IValidator(validator).validateUserOp(userOp, userOpHash);
 
         // pay back gas to EntryPoint
         if (missingAccountFunds > 0) {
@@ -277,10 +285,17 @@ contract Safe7579Launchpad is IAccount, SafeStorage {
         owners.uniquifySorted();
 
         uint256 length = owners.length;
+        console2.log("owners", owners.length, safeSetupCallData.threshold);
+
+        uint256 validSigs;
         for (uint256 i; i < length; i++) {
-            signers.searchSorted(owners[i]);
-            if (i == safeSetupCallData.threshold) {
-                return true;
+            (bool found,) = signers.searchSorted(owners[i]);
+            if (found) {
+                uint256 _validSigs = validSigs + 1;
+                if (_validSigs >= safeSetupCallData.threshold) {
+                    return true;
+                }
+                validSigs = _validSigs;
             }
         }
         return false;

diff --git a/test/LaunchpadSafeSigner.t.sol b/test/LaunchpadSafeSigner.t.sol
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.23;
+
+import "forge-std/Test.sol";
+import { Safe7579 } from "src/Safe7579.sol";
+import { ISafe7579 } from "src/ISafe7579.sol";
+import { IERC7484 } from "src/interfaces/IERC7484.sol";
+import "src/DataTypes.sol";
+import { ModuleManager } from "src/core/ModuleManager.sol";
+import { MockValidator } from "module-bases/mocks/MockValidator.sol";
+import { MockRegistry } from "./mocks/MockRegistry.sol";
+import { MockExecutor } from "./mocks/MockExecutor.sol";
+import { MockFallback } from "./mocks/MockFallback.sol";
+import { ExecutionLib } from "erc7579/lib/ExecutionLib.sol";
+import { ModeLib } from "erc7579/lib/ModeLib.sol";
+import { IERC7579Account, Execution } from "erc7579/interfaces/IERC7579Account.sol";
+import { MockTarget } from "./mocks/MockTarget.sol";
+
+import { Safe } from "@safe-global/safe-contracts/contracts/Safe.sol";
+import {
+    SafeProxy,
+    SafeProxyFactory
+} from "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
+import { LibClone } from "solady/utils/LibClone.sol";
+import { Safe7579Launchpad } from "src/Safe7579Launchpad.sol";
+
+import { Solarray } from "solarray/Solarray.sol";
+import "./dependencies/EntryPoint.sol";
+
+import { Simulator } from "@rhinestone/erc4337-validation/src/Simulator.sol";
+
+contract LaunchpadSafeSignerBase is Test {
+    using Simulator for PackedUserOperation; // or UserOperation
+
+    Safe7579 safe7579;
+    Safe singleton;
+    Safe safe;
+    SafeProxyFactory safeProxyFactory;
+    Safe7579Launchpad launchpad;
+
+    MockValidator defaultValidator;
+    MockExecutor defaultExecutor;
+    MockTarget target;
+
+    Account signer1 = makeAccount("signer1");
+    Account signer2 = makeAccount("signer2");
+
+    IEntryPoint entrypoint;
+    bytes userOpInitCode;
+    IERC7484 registry;
+
+    struct Setup {
+        address singleton;
+        address signerFactory;
+        bytes signerData;
+        address setupTo;
+        bytes setupData;
+        address fallbackHandler;
+    }
+
+    function setUp() public virtual {
+        // Set up EntryPoint
+        entrypoint = etchEntrypoint();
+        singleton = new Safe();
+        safeProxyFactory = new SafeProxyFactory();
+        registry = new MockRegistry();
+        safe7579 = new Safe7579();
+        launchpad = new Safe7579Launchpad(address(entrypoint), registry);
+
+        // Set up Modules
+        defaultValidator = new MockValidator();
+        defaultExecutor = new MockExecutor();
+        target = new MockTarget();
+
+        bytes32 salt;
+
+        ModuleInit[] memory validators = new ModuleInit[](1);
+        validators[0] = ModuleInit({ module: address(defaultValidator), initData: bytes("") });
+        ModuleInit[] memory executors = new ModuleInit[](1);
+        executors[0] = ModuleInit({ module: address(defaultExecutor), initData: bytes("") });
+        ModuleInit[] memory fallbacks = new ModuleInit[](0);
+        ModuleInit[] memory hooks = new ModuleInit[](0);
+
+        Safe7579Launchpad.InitData memory initData = Safe7579Launchpad.InitData({
+            singleton: address(singleton),
+            owners: Solarray.addresses(signer1.addr),
+            threshold: 1,
+            setupTo: address(launchpad),
+            setupData: abi.encodeCall(
+                Safe7579Launchpad.initSafe7579,
+                (
+                    address(safe7579),
+                    executors,
+                    fallbacks,
+                    hooks,
+                    Solarray.addresses(makeAddr("attester1"), makeAddr("attester2")),
+                    2
+                )
+            ),
+            safe7579: ISafe7579(safe7579),
+            validators: validators,
+            callData: abi.encodeCall(
+                IERC7579Account.execute,
+                (
+                    ModeLib.encodeSimpleSingle(),
+                    ExecutionLib.encodeSingle({
+                        target: address(target),
+                        value: 0,
+                        callData: abi.encodeCall(MockTarget.set, (1337))
+                    })
+                )
+            )
+        });
+        bytes32 initHash = launchpad.hash(initData);
+
+        bytes memory factoryInitializer =
+            abi.encodeCall(Safe7579Launchpad.preValidationSetup, (initHash, address(0), ""));
+
+        PackedUserOperation memory userOp = getDefaultUserOp(address(safe), address(0));
+
+        {
+            userOp.callData = abi.encodeCall(Safe7579Launchpad.setupSafe, (initData));
+            userOp.initCode = _initCode(factoryInitializer, salt);
+        }
+
+        address predict = launchpad.predictSafeAddress({
+            singleton: address(launchpad),
+            safeProxyFactory: address(safeProxyFactory),
+            creationCode: type(SafeProxy).creationCode,
+            salt: salt,
+            factoryInitializer: factoryInitializer
+        });
+        console2.log("Predicted address: ", predict);
+        userOp.sender = predict;
+        assertEq(userOp.sender, predict);
+
+        bytes32 userOpHash = entrypoint.getUserOpHash(userOp);
+        bytes memory signatures;
+        uint8 v;
+        bytes32 r;
+        bytes32 s;
+        (v, r, s) = vm.sign(signer1.key, userOpHash);
+        address signer = address(uint160(uint256(r)));
+        signatures = abi.encodePacked(r, s, v);
+        (v, r, s) = vm.sign(signer2.key, userOpHash);
+        signatures = abi.encodePacked(signatures, abi.encodePacked(r, s, v));
+
+        userOp.signature = signatures;
+
+        PackedUserOperation[] memory userOps = new PackedUserOperation[](1);
+        userOps[0] = userOp;
+        deal(address(userOp.sender), 1 ether);
+
+        userOp.simulateUserOp(address(entrypoint));
+        entrypoint.handleOps(userOps, payable(address(0x69)));
+
+        safe = Safe(payable(predict));
+
+        assertEq(target.value(), 1337);
+    }
+
+    function _initCode(
+        bytes memory initializer,
+        bytes32 salt
+    )
+        internal
+        view
+        returns (bytes memory _initCode)
+    {
+        _initCode = abi.encodePacked(
+            address(safeProxyFactory),
+            abi.encodeCall(
+                SafeProxyFactory.createProxyWithNonce,
+                (address(launchpad), initializer, uint256(salt))
+            )
+        );
+    }
+
+    function test_foo() public {
+        assertTrue(true);
+    }
+
+    function getDefaultUserOp(
+        address account,
+        address validator
+    )
+        internal
+        view
+        returns (PackedUserOperation memory userOp)
+    {
+        userOp = PackedUserOperation({
+            sender: account,
+            nonce: safe7579.getNonce(account, validator),
+            initCode: "",
+            callData: "",
+            accountGasLimits: bytes32(abi.encodePacked(uint128(2e6), uint128(2e6))),
+            preVerificationGas: 2e6,
+            gasFees: bytes32(abi.encodePacked(uint128(2e6), uint128(2e6))),
+            paymasterAndData: bytes(""),
+            signature: abi.encodePacked(hex"41414141")
+        });
+    }
+}