readme update

README.md

@@ -10,7 +10,6 @@
 
 - Support [ERC-4337: Account Abstraction](https://eips.ethereum.org/EIPS/eip-4337)
 - [Modular design ](https://hackmd.io/3gbndH7tSl2J1EbNePJ3Yg)
-- Implement [asset / keystore](https://hackmd.io/-YY8jD7IQ7qfEZaDepXZsA?view) separation architecture
 - Upgradability: The smart contract for this wallet can be upgraded in a secure way to add new features or fix vulnerabilities in the future.
 - Stablecoin pay gas: Users can pay transaction gas fees with stablecoins such as USDC, USDT, DAI, etc.
 
@@ -42,34 +41,22 @@ All contracts are held within the `soul-wallet-contract/contracts` folder.
 ```
 contracts
 ├── abstract
+├── automation
 ├── dev
 │   └── tokens
 ├── factory
 ├── hooks
 │   └── 2fa
 ├── interfaces
-├── keystore
-│   ├── L1
-│   │   ├── base
-│   │   └── interfaces
-│   └── interfaces
 ├── libraries
 ├── modules
 │   ├── interfaces
-│   ├── keystore
-│   │   ├── arbitrum
+│   ├── socialRecovery
 │   │   ├── base
-│   │   ├── interfaces
-│   │   └── optimism
-│   ├── securityControlModule
-│   │   └── trustedContractManager
-│   │       ├── trustedHookManager
-│   │       ├── trustedModuleManager
-│   │       └── trustedValidatorManager
+│   │   └── interfaces
 │   └── upgrade
 ├── paymaster
 │   └── interfaces
-├── proxy
 └── validator
     └── libraries
 ```
@@ -114,7 +101,7 @@ contract NewModule is BaseModule {
 
 ### Hook
 
-o integrate a new hook, your contract should inherit `IHook` interface. This interface will define the standard structure and functionalities for your hooks.
+To integrate a new hook, your contract should inherit `IHook` interface. This interface will define the standard structure and functionalities for your hooks.
 
 ```solidity
 

contracts/automation/AaveUsdcSaveAutomation.sol

@@ -7,6 +7,10 @@ interface IAaveV3 {
     function supply(address asset, uint256 amount, address onBehalfOf, uint16 referralCode) external;
 }
 
+/**
+ * @title AaveUsdcSaveAutomation
+ * @dev This contract allows a bot to deposit USDC to Aave on behalf of a user.
+ */
 contract AaveUsdcSaveAutomation is Ownable {
     using SafeERC20 for IERC20;
 
@@ -18,6 +22,9 @@ contract AaveUsdcSaveAutomation is Ownable {
     IAaveV3 immutable aave;
     mapping(address => bool) public bots;
 
+    /**
+     * @dev Modifier to make a function callable only by a bot.
+     */
     modifier onlyBot() {
         require(bots[msg.sender], "no permission");
         _;
@@ -29,12 +36,24 @@ contract AaveUsdcSaveAutomation is Ownable {
         usdcToken.approve(address(aave), 2 ** 256 - 1);
     }
 
+    /**
+     * @notice Deposits USDC to Aave on behalf of a user
+     * @dev This function can only be called by a bot
+     * @param _user The address of the user for whom to deposit USDC
+     * @param amount The amount of USDC to deposit
+     */
     function depositUsdcToAave(address _user, uint256 amount) public onlyBot {
         usdcToken.safeTransferFrom(_user, address(this), amount);
         aave.supply(address(usdcToken), amount, _user, 0);
         emit UsdcDepositedToAave(_user, amount);
     }
 
+    /**
+     * @notice Deposits USDC to Aave on behalf of multiple users
+     * @dev This function can only be called by a bot
+     * @param _users An array of addresses of the users for whom to deposit USDC
+     * @param amounts An array of amounts of USDC to deposit for each user
+     */
     function depositUsdcToAaveBatch(address[] calldata _users, uint256[] calldata amounts) public onlyBot {
         require(_users.length == amounts.length, "invalid input");
         for (uint256 i = 0; i < _users.length; i++) {

contracts/libraries/Errors.sol

@@ -6,35 +6,24 @@ library Errors {
     error ADDRESS_NOT_EXISTS();
     error DATA_ALREADY_EXISTS();
     error DATA_NOT_EXISTS();
-    error CALLER_MUST_BE_ENTRYPOINT();
     error CALLER_MUST_BE_SELF_OR_MODULE();
     error CALLER_MUST_BE_MODULE();
     error HASH_ALREADY_APPROVED();
     error HASH_ALREADY_REJECTED();
     error INVALID_ADDRESS();
-    error INVALID_GUARD_HOOK_DATA();
     error INVALID_SELECTOR();
     error INVALID_SIGNTYPE();
-    error MODULE_ADDRESS_EMPTY();
-    error MODULE_NOT_SUPPORT_INTERFACE();
-    error MODULE_SELECTOR_UNAUTHORIZED();
     error MODULE_SELECTORS_EMPTY();
     error MODULE_EXECUTE_FROM_MODULE_RECURSIVE();
-    error NO_OWNER();
     error SELECTOR_ALREADY_EXISTS();
     error SELECTOR_NOT_EXISTS();
-    error UNSUPPORTED_SIGNTYPE();
     error INVALID_LOGIC_ADDRESS();
     error SAME_LOGIC_ADDRESS();
     error UPGRADE_FAILED();
     error NOT_IMPLEMENTED();
     error INVALID_SIGNATURE();
-    error ALERADY_INITIALIZED();
-    error INVALID_KEY();
-    error NOT_INITIALIZED();
     error INVALID_TIME_RANGE();
     error UNAUTHORIZED();
     error INVALID_DATA();
     error GUARDIAN_SIGNATURE_INVALID();
-    error UNTRUSTED_KEYSTORE_LOGIC();
 }

contracts/modules/socialRecovery/README.md

@@ -0,0 +1,51 @@
+# SocialRecoveryModule
+
+The `SocialRecoveryModule` is a Solidity contract that can be installed in wallets to enable a social recovery mechanism. This module allows a user to designate a list of guardians for their wallet and establish a recovery threshold. If a wallet is lost or compromised, the guardians can initiate a recovery process by signing a special EIP712 signature. However, this recovery process is subject to a user-defined time lock period, and the guardians can only execute the recovery after this period has passed. This mechanism ensures that the user's assets remain secure and recoverable, even in unforeseen circumstances.
+
+## Recovery flow
+
+![social recovery flow](socialReoceryFlow.png)
+
+- Step 1: Users install the Social Recovery Module in their SoulWallet. They need to configure the guardian hash and the execution delay period when installing the module. The guardian hash refers to the keccak256 hash of the GuardianData, ensuring the privacy of guardian identities. Others cannot check your guardians' settings on-chain and they are only revealed when the user initiates the social recovery process.
+
+  ```solidity
+    struct GuardianData {
+          address[] guardians;
+          uint256 threshold;
+          uint256 salt;
+      }
+  ```
+
+- Step 2: When users want to recover their wallet using the guardians, they have to contact the guardians to sign an EIP-712 based signature and use the following scheme:
+
+  - EIP712Domain
+
+  ```json
+  {
+    "EIP712Domain": [
+      { "type": "uint256", "name": "chainId" },
+      { "type": "address", "name": "SocialRecovery" }
+    ]
+  }
+  ```
+
+  - SocialRecovery
+
+  ```json
+  {
+    "SocialRecovery": [
+      { "type": "address", "name": "wallet" },
+      { "type": "uint256", "name": "nonce" },
+      { "type": "bytes32[]", "name": "newOwners" }
+    ]
+  }
+  ```
+
+  Once the signatures are collected and the threshold is met, it can call scheduleRecovery to enter the waiting queue for recovery.
+
+- Step 3: If the timelock period has passed, one can call `executeRecovery` to perform the recovery. The social recovery module will then reset the owners based on the previous setting.
+
+## Considerations
+
+- Users can call `cancelAllRecovery` to invalidate transactions in the pending queue.
+- Users can call `setGuardian` to change guardians' settings without a timelock.

contracts/modules/socialRecovery/SocialRecoveryModule.sol

@@ -1,21 +1,34 @@
-pragma solidity ^0.8.17;
+pragma solidity ^0.8.20;
 
 import "../BaseModule.sol";
 import "./base/BaseSocialRecovery.sol";
 
+/**
+ * @title SocialRecoveryModule
+ * @dev This contract extends BaseModule and BaseSocialRecovery to provide social recovery functionality for a wallet.
+ * It allows a wallet owner to set a list of guardians and a recovery delay period. If the wallet is lost or compromised,
+ * the guardians can recover the wallet after the delay period has passed.
+ */
 contract SocialRecoveryModule is BaseModule, BaseSocialRecovery {
     bytes4 private constant _FUNC_RESET_OWNER = bytes4(keccak256("resetOwner(bytes32)"));
     bytes4 private constant _FUNC_RESET_OWNERS = bytes4(keccak256("resetOwners(bytes32[])"));
     mapping(address => bool) walletInited;
 
     constructor() EIP712("SocialRecovery", "1") {}
 
+    /**
+     * @dev De-initializes the social recovery settings for the sender's wallet.
+     */
     function _deInit() internal override {
         address _sender = sender();
         _clearWalletSocialRecoveryInfo(_sender);
         walletInited[_sender] = false;
     }
 
+    /**
+     * @dev Initializes the social recovery settings for the sender's wallet.
+     * @param _data The encoded guardian hash and delay period.
+     */
     function _init(bytes calldata _data) internal override {
         address _sender = sender();
         (bytes32 guardianHash, uint256 delayPeroid) = abi.decode(_data, (bytes32, uint256));
@@ -24,10 +37,19 @@ contract SocialRecoveryModule is BaseModule, BaseSocialRecovery {
         walletInited[_sender] = true;
     }
 
+    /**
+     * @dev Checks if the social recovery settings for a wallet have been initialized.
+     * @param wallet The address of the wallet.
+     * @return A boolean indicating whether the social recovery settings for the wallet have been initialized.
+     */
     function inited(address wallet) internal view override returns (bool) {
         return walletInited[wallet];
     }
 
+    /**
+     * @dev Returns the list of functions required by this module.
+     * @return An array of function selectors.
+     */
     function requiredFunctions() external pure override returns (bytes4[] memory) {
         bytes4[] memory functions = new bytes4[](2);
         functions[0] = _FUNC_RESET_OWNER;

contracts/modules/socialRecovery/base/BaseSocialRecovery.sol

@@ -7,6 +7,15 @@ import "@openzeppelin/contracts/interfaces/IERC1271.sol";
 import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 
+/**
+ * @title BaseSocialRecovery
+ * @dev This abstract contract provides the base implementation for the social recovery functionality.
+ * It implements the ISocialRecovery interface and extends the EIP712 contract.
+ * It allows a user to designate a list of guardians for their wallet and establish a recovery threshold.
+ * If a wallet is lost or compromised, the guardians can initiate a recovery process by signing a special EIP712 signature.
+ * However, this recovery process is subject to a user-defined time lock period, and can only execute the recovery after this period has passed.
+ * This mechanism ensures that the user's assets remain secure and recoverable, even in unforeseen circumstances.
+ */
 abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
     using ECDSA for bytes32;
 
@@ -61,26 +70,26 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
             return OperationState.Ready;
         }
     }
+
     /**
      * @dev Returns whether an operation is pending or not. Note that a "pending" operation may also be "ready".
      */
-
     function isOperationPending(address wallet, bytes32 id) public view returns (bool) {
         OperationState state = getOperationState(wallet, id);
         return state == OperationState.Waiting || state == OperationState.Ready;
     }
+
     /**
      * @dev Returns whether an operation is ready for execution. Note that a "ready" operation is also "pending".
      */
-
     function isOperationReady(address wallet, bytes32 id) public view returns (bool) {
         return getOperationState(wallet, id) == OperationState.Ready;
     }
+
     /**
      * @dev Returns whether an id corresponds to a registered operation. This
      * includes both Waiting, Ready, and Done operations.
      */
-
     function isOperationSet(address wallet, bytes32 id) public view returns (bool) {
         return getOperationState(wallet, id) != OperationState.Unset;
     }
@@ -89,13 +98,23 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
         return socialRecoveryInfo[wallet].operationValidAt[id];
     }
 
+    /**
+     * @notice modify the guardian hash for a wallet
+     * @dev Emits a GuardianSet event
+     * @param newGuardianHash The new guardian hash
+     */
     function setGuardian(bytes32 newGuardianHash) external {
         address wallet = _msgSender();
         socialRecoveryInfo[wallet].guardianHash = newGuardianHash;
         _increaseNonce(wallet);
         emit GuardianSet(wallet, newGuardianHash);
     }
 
+    /**
+     * @notice Sets the recovery time lock period for a wallet
+     * @dev Emits a DelayPeriodSet event
+     * @param newDelay The new delay period
+     */
     function setDelayPeriod(uint256 newDelay) external {
         address wallet = _msgSender();
         socialRecoveryInfo[wallet].delayPeriod = newDelay;
@@ -108,10 +127,14 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
         _increaseNonce(wallet);
         emit RecoveryCancelled(wallet, 0);
     }
+
     /**
-     * @dev Considering that not all contract are EIP-1271 compatible
+     * @notice Approves a hash for the sender
+     * the hash is the eip712 hash of the recover operation for guardian to sign
+     * @dev Considering that not all contracts are EIP-1271 compatible, this function could be called by the guardian if the guardian is a smart contract.
+     * It emits an ApproveHash event.
+     * @param hash The hash to be approved
      */
-
     function approveHash(bytes32 hash) external {
         bytes32 key = _approveKey(msg.sender, hash);
         if (approvedHashes[key] == 1) {
@@ -120,6 +143,14 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
         approvedHashes[key] = 1;
         emit ApproveHash(msg.sender, hash);
     }
+    /**
+     *
+     * @notice Rejects a hash for the sender
+     * the hash is the eip712 hash of the recover operation for guardian to sign
+     * @dev Considering that not all contracts are EIP-1271 compatible, this function could be called by the guardian if the guardian is a smart contract.
+     * It emits a RejectHash event.
+     * @param hash The hash to be rejected
+     */
 
     function rejectHash(bytes32 hash) external {
         bytes32 key = _approveKey(msg.sender, hash);
@@ -130,6 +161,14 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
         emit RejectHash(msg.sender, hash);
     }
 
+    /**
+     * @notice Schedules a recovery operation for a wallet
+     * @param wallet The address of the wallet
+     * @param newOwners The new owners to be set for the wallet
+     * @param rawGuardian The raw guardian data
+     * @param guardianSignature The signature of the guardian
+     * @return recoveryId The ID of the recovery operation
+     */
     function scheduleRecovery(
         address wallet,
         bytes32[] calldata newOwners,
@@ -147,15 +186,19 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
         emit RecoveryScheduled(wallet, recoveryId, scheduleTime);
     }
 
+    /**
+     * @notice Executes a recovery operation for a wallet
+     * @param wallet The address of the wallet
+     * @param newOwners The new owners to be set for the wallet
+     */
     function executeRecovery(address wallet, bytes32[] calldata newOwners) external override {
         bytes32 recoveryId = hashOperation(wallet, walletNonce(wallet), abi.encode(newOwners));
         if (!isOperationReady(wallet, recoveryId)) {
             revert UNEXPECTED_OPERATION_STATE(wallet, recoveryId, _encodeStateBitmap(OperationState.Ready));
         }
-        _recoveryOwner(wallet, newOwners);
-
         _setRecoveryDone(wallet, recoveryId);
         _increaseNonce(wallet);
+        _recoveryOwner(wallet, newOwners);
         emit RecoveryExecuted(wallet, recoveryId);
     }
 
@@ -168,6 +211,10 @@ abstract contract BaseSocialRecovery is ISocialRecovery, EIP712 {
         soulwallet.resetOwners(newOwners);
     }
 
+    /**
+     * @notice Verifies the guardian's signature
+     * @dev This function checks the signature type and verifies it accordingly. It supports EIP-1271 signatures for smart contract wallet, approved hashes, and EOA signatures.
+     */
     function _verifyGuardianSignature(
         address wallet,
         uint256 nonce,