Merge pull request #7 from rhinestonewtf/feature/4337-compliance

fix: various bugs/missing features
rhinestonewtf · Jul 8, 2024 · ff8df1d · ff8df1d
2 parents ebef70c + bedaf2f
commit ff8df1d
Show file tree

Hide file tree

Showing 22 changed files with 1,178 additions and 711 deletions.
diff --git a/audits/ackee-blockchain-rhinestone-safe7579-report.pdf b/audits/ackee-blockchain-rhinestone-safe7579-report.pdf
diff --git a/broadcast/Deploy.s.sol/11155111/run-1715275639.json b/broadcast/Deploy.s.sol/11155111/run-1715275639.json
diff --git a/broadcast/Deploy.s.sol/11155111/run-latest.json b/broadcast/Deploy.s.sol/11155111/run-latest.json
diff --git a/package.json b/package.json
@@ -26,9 +26,12 @@
     "test:optimized": "pnpm run build:optimized && FOUNDRY_PROFILE=test-optimized forge test"
   },
   "dependencies": {
-    "@rhinestone/module-bases": "github:rhinestonewtf/module-bases",
     "@ERC4337/account-abstraction": "github:kopy-kat/account-abstraction#develop",
+    "@ERC4337/account-abstraction-v0.6": "github:eth-infinitism/account-abstraction#v0.6.0",
+    "@rhinestone/erc4337-validation": "0.0.1-alpha.2",
+    "@rhinestone/module-bases": "github:rhinestonewtf/module-bases",
     "@rhinestone/sentinellist": "github:rhinestonewtf/sentinellist",
+    "@rhinestone/checknsignatures": "github:rhinestonewtf/checknsignatures",
     "@safe-global/safe-contracts": "^1.4.1",
     "ds-test": "github:dapphub/ds-test",
     "erc7579": "github:erc7579/erc7579-implementation",

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol
@@ -5,6 +5,11 @@ import { Script } from "forge-std/Script.sol";
 import { Safe7579 } from "src/Safe7579.sol";
 import { Safe7579Launchpad } from "src/Safe7579Launchpad.sol";
 import { IERC7484 } from "src/interfaces/IERC7484.sol";
+import { MockRegistry } from "test/mocks/MockRegistry.sol";
+import { Safe } from "@safe-global/safe-contracts/contracts/Safe.sol";
+import { SafeProxyFactory } from
+    "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
+import { MockValidator } from "test/mocks/MockValidator.sol";
 
 /**
  * @title Deploy
@@ -15,10 +20,16 @@ contract DeployScript is Script {
         bytes32 salt = bytes32(uint256(0));
 
         address entryPoint = address(0x0000000071727De22E5E9d8BAf0edAc6f37da032);
-        IERC7484 registry = IERC7484(0xe0cde9239d16bEf05e62Bbf7aA93e420f464c826);
+        IERC7484 registry = IERC7484(0x25A4b2F363678E13A0A5DB79b712dE00347a593E);
 
         vm.startBroadcast(vm.envUint("PK"));
 
+        // new MockValidator{ salt: salt }();
+
+        // new Safe{ salt: salt }();
+        // new SafeProxyFactory{ salt: salt }();
+
+        // IERC7484 registry = new MockRegistry{ salt: salt }();
         new Safe7579{ salt: salt }();
         new Safe7579Launchpad{ salt: salt }(entryPoint, registry);
 

diff --git a/src/ISafe7579.sol b/src/ISafe7579.sol
@@ -2,18 +2,18 @@
 pragma solidity ^0.8.20;
 
 import "./DataTypes.sol";
-import { IERC7579Account } from "./interfaces//IERC7579Account.sol";
-
+import { IERC7579Account } from "./interfaces/IERC7579Account.sol";
 import { ModeCode } from "./lib/ModeLib.sol";
 import { PackedUserOperation } from
     "@ERC4337/account-abstraction/contracts/core/UserOperationLib.sol";
+import { ISafeOp } from "./interfaces/ISafeOp.sol";
 
 /**
  * @title ERC7579 Adapter for Safe accounts.
  * creates full ERC7579 compliance to Safe accounts
  * @author rhinestone | zeroknots.eth, Konrad Kopp (@kopy-kat)
  */
-interface ISafe7579 is IERC7579Account {
+interface ISafe7579 is IERC7579Account, ISafeOp {
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
     /*                         Validation                         */
     /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
@@ -191,7 +191,7 @@ interface ISafe7579 is IERC7579Account {
      * @dev Note: this function DOES NOT call onInstall() on the validator modules or emit
      * ModuleInstalled events. this has to be done by the launchpad
      */
-    function launchpadValidators(ModuleInit[] memory validators) external;
+    function initializeAccountWithValidators(ModuleInit[] memory validators) external;
 
     /**
      * Configure the Safe7579 with a IERC7484 registry
@@ -233,14 +233,7 @@ interface ISafe7579 is IERC7579Account {
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
     /*                        Query Misc                          */
     /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
-    function supportsExecutionMode(ModeCode encodedMode) external pure returns (bool supported);
-    function supportsModule(uint256 moduleTypeId) external pure returns (bool);
-    function accountId() external view returns (string memory accountImplementationId);
 
-    /**
-     * Domain Separator for EIP-712.
-     */
-    function domainSeparator() external view returns (bytes32);
     /**
      * Safe7579 is using validator selection encoding in the userop nonce.
      * to make it easier for SDKs / devs to integrate, this function can be

diff --git a/src/Safe7579.sol b/src/Safe7579.sol
@@ -6,7 +6,6 @@ import {
     CallType,
     ExecType,
     ModeCode,
-    ModeLib,
     EXECTYPE_DEFAULT,
     EXECTYPE_TRY,
     CALLTYPE_SINGLE,
@@ -24,7 +23,7 @@ import {
 import { ModuleInstallUtil } from "./utils/DCUtil.sol";
 import { AccessControl } from "./core/AccessControl.sol";
 import { Initializer } from "./core/Initializer.sol";
-import { ISafeOp, SAFE_OP_TYPEHASH } from "./interfaces/ISafeOp.sol";
+import { SafeOp } from "./core/SafeOp.sol";
 import { ISafe } from "./interfaces/ISafe.sol";
 import { ISafe7579 } from "./ISafe7579.sol";
 import {
@@ -34,6 +33,7 @@ import {
 import { _packValidationData } from "@ERC4337/account-abstraction/contracts/core/Helpers.sol";
 import { IEntryPoint } from "@ERC4337/account-abstraction/contracts/interfaces/IEntryPoint.sol";
 import { IERC1271 } from "./interfaces/IERC1271.sol";
+import { SupportViewer } from "./core/SupportViewer.sol";
 
 uint256 constant MULTITYPE_MODULE = 0;
 
@@ -50,8 +50,7 @@ uint256 constant MULTITYPE_MODULE = 0;
  * event emissions to be done via the SafeProxy as msg.sender using Safe's
  * "executeTransactionFromModule" features.
  */
-contract Safe7579 is ISafe7579, ISafeOp, AccessControl, Initializer {
-    using UserOperationLib for PackedUserOperation;
+contract Safe7579 is ISafe7579, SafeOp, SupportViewer, AccessControl, Initializer {
     using ExecutionLib for bytes;
 
     bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH =
@@ -301,12 +300,8 @@ contract Safe7579 is ISafe7579, ISafeOp, AccessControl, Initializer {
         view
         returns (uint256 validationData)
     {
-        (
-            bytes memory operationData,
-            uint48 validAfter,
-            uint48 validUntil,
-            bytes calldata signatures
-        ) = _getSafeOp(userOp);
+        (bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes memory signatures)
+        = getSafeOp(userOp, entryPoint());
         try ISafe((msg.sender)).checkSignatures(keccak256(operationData), operationData, signatures)
         {
             // The timestamps are validated by the entry point,
@@ -460,43 +455,6 @@ contract Safe7579 is ISafe7579, ISafeOp, AccessControl, Initializer {
         });
     }
 
-    /**
-     * @inheritdoc ISafe7579
-     */
-    function supportsExecutionMode(ModeCode encodedMode)
-        external
-        pure
-        override
-        returns (bool supported)
-    {
-        CallType callType;
-        ExecType execType;
-        // solhint-disable-next-line no-inline-assembly
-        assembly {
-            callType := encodedMode
-            execType := shl(8, encodedMode)
-        }
-        if (callType == CALLTYPE_BATCH) supported = true;
-        else if (callType == CALLTYPE_SINGLE) supported = true;
-        else if (callType == CALLTYPE_DELEGATECALL) supported = true;
-        else return false;
-
-        if (supported && execType == EXECTYPE_DEFAULT) return supported;
-        else if (supported && execType == EXECTYPE_TRY) return supported;
-        else return false;
-    }
-
-    /**
-     * @inheritdoc ISafe7579
-     */
-    function supportsModule(uint256 moduleTypeId) external pure override returns (bool) {
-        if (moduleTypeId == MODULE_TYPE_VALIDATOR) return true;
-        else if (moduleTypeId == MODULE_TYPE_EXECUTOR) return true;
-        else if (moduleTypeId == MODULE_TYPE_FALLBACK) return true;
-        else if (moduleTypeId == MODULE_TYPE_HOOK) return true;
-        else return false;
-    }
-
     /**
      * @inheritdoc ISafe7579
      */
@@ -510,6 +468,9 @@ contract Safe7579 is ISafe7579, ISafeOp, AccessControl, Initializer {
         returns (bool)
     {
         if (moduleType == MODULE_TYPE_VALIDATOR) {
+            // Safe7579 adapter allows for validator fallback to Safe's checkSignatures().
+            // It can thus be considered a valid validator module
+            if (module == msg.sender) return true;
             return _isValidatorInstalled(module);
         } else if (moduleType == MODULE_TYPE_EXECUTOR) {
             return _isExecutorInstalled(module);
@@ -522,99 +483,6 @@ contract Safe7579 is ISafe7579, ISafeOp, AccessControl, Initializer {
         }
     }
 
-    /**
-     * @inheritdoc ISafe7579
-     */
-    function accountId() external view returns (string memory accountImplementationId) {
-        string memory safeVersion = ISafe(msg.sender).VERSION();
-        return string(abi.encodePacked("safe-", safeVersion, ".erc7579.v0.0.1"));
-    }
-
-    /**
-     * @dev Decodes an ERC-4337 user operation into a Safe operation.
-     * @param userOp The ERC-4337 user operation.
-     * @return operationData Encoded EIP-712 Safe operation data bytes used for signature
-     * verification.
-     * @return validAfter The timestamp the user operation is valid from.
-     * @return validUntil The timestamp the user operation is valid until.
-     * @return signatures The Safe owner signatures extracted from the user operation.
-     */
-    function _getSafeOp(PackedUserOperation calldata userOp)
-        internal
-        view
-        returns (
-            bytes memory operationData,
-            uint48 validAfter,
-            uint48 validUntil,
-            bytes calldata signatures
-        )
-    {
-        // Extract additional Safe operation fields from the user operation signature which is
-        // encoded as:
-        // `abi.encodePacked(validAfter, validUntil, signatures)`
-        {
-            bytes calldata sig = userOp.signature;
-            validAfter = uint48(bytes6(sig[0:6]));
-            validUntil = uint48(bytes6(sig[6:12]));
-            signatures = sig[12:];
-        }
-
-        // It is important that **all** user operation fields are represented in the `SafeOp` data
-        // somehow, to prevent
-        // user operations from being submitted that do not fully respect the user preferences. The
-        // only exception is
-        // the `signature` bytes. Note that even `initCode` needs to be represented in the operation
-        // data, otherwise
-        // it can be replaced with a more expensive initialization that would charge the user
-        // additional fees.
-        {
-            // In order to work around Solidity "stack too deep" errors related to too many stack
-            // variables, manually
-            // encode the `SafeOp` fields into a memory `struct` for computing the EIP-712
-            // struct-hash. This works
-            // because the `EncodedSafeOpStruct` struct has no "dynamic" fields so its memory layout
-            // is identical to the
-            // result of `abi.encode`-ing the individual fields.
-            EncodedSafeOpStruct memory encodedSafeOp = EncodedSafeOpStruct({
-                typeHash: SAFE_OP_TYPEHASH,
-                safe: msg.sender,
-                nonce: userOp.nonce,
-                initCodeHash: keccak256(userOp.initCode),
-                callDataHash: keccak256(userOp.callData),
-                callGasLimit: userOp.unpackCallGasLimit(),
-                verificationGasLimit: userOp.unpackVerificationGasLimit(),
-                preVerificationGas: userOp.preVerificationGas,
-                maxFeePerGas: userOp.unpackMaxFeePerGas(),
-                maxPriorityFeePerGas: userOp.unpackMaxPriorityFeePerGas(),
-                paymasterAndDataHash: keccak256(userOp.paymasterAndData),
-                validAfter: validAfter,
-                validUntil: validUntil,
-                entryPoint: entryPoint()
-            });
-
-            bytes32 safeOpStructHash;
-            // solhint-disable-next-line no-inline-assembly
-            assembly ("memory-safe") {
-                // Since the `encodedSafeOp` value's memory layout is identical to the result of
-                // `abi.encode`-ing the
-                // individual `SafeOp` fields, we can pass it directly to `keccak256`. Additionally,
-                // there are 14
-                // 32-byte fields to hash, for a length of `14 * 32 = 448` bytes.
-                safeOpStructHash := keccak256(encodedSafeOp, 448)
-            }
-
-            operationData =
-                abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeOpStructHash);
-        }
-    }
-
-    /**
-     * @inheritdoc ISafe7579
-     */
-    function domainSeparator() public view returns (bytes32) {
-        return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, block.chainid, this));
-    }
-
     /**
      * @inheritdoc ISafe7579
      */