Social Recovery Module Update

.github/workflows/certora_recovery.yml

@@ -0,0 +1,68 @@
+name: certora
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches:
+      - main
+
+env:
+  CERTORAKEY: ${{ secrets.CERTORA_KEY }}
+
+jobs:
+  check-secret:
+    runs-on: ubuntu-latest
+    outputs:
+      certora-key-exists: ${{ steps.certora-key-check.outputs.defined }}
+    steps:
+      - name: Check for Certora Secret availability
+        id: certora-key-check
+        # perform secret check & put boolean result as an output
+        run: |
+          if [[ -n "${CERTORAKEY}" ]]; then
+            echo "defined=true" >> $GITHUB_OUTPUT;
+            echo "CERTORA_KEY exists"
+          else
+            echo "defined=false" >> $GITHUB_OUTPUT;
+            echo "CERTORA_KEY does not exist"
+          fi
+
+  verify:
+    needs: [check-secret]
+    if: needs.check-secret.outputs.certora-key-exists == 'true'
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        rule: ['SocialRecoveryModule', 'GuardianStorage', 'RecoveryConfirmationSignatureValidity']
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 20.x
+          cache: npm
+
+      - name: Install python
+        uses: actions/setup-python@v5
+        with:
+          python-version: 3.11
+          cache: 'pip'
+
+      - name: Install certora cli
+        run: pip install -r certora/requirements.txt
+
+      - name: Install solc
+        run: |
+          wget https://github.com/ethereum/solidity/releases/download/v0.8.20/solc-static-linux
+          chmod +x solc-static-linux
+          sudo mv solc-static-linux /usr/local/bin/solc-0.8.20
+
+      - name: Install dependencies
+        run: yarn install --frozen-lockfile
+
+      - name: Verify rule ${{ matrix.rule }}
+        run: |
+          certoraRun certora/conf/${{ matrix.rule }}.conf --wait_for_results=all

.github/workflows/test_contracts.yml

@@ -0,0 +1,27 @@
+name: test
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches:
+      - main
+
+jobs:
+  verify:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 20.x
+          cache: npm
+
+      - name: Install dependencies
+        run: yarn install --frozen-lockfile
+
+      - name: Run Tests
+        run: yarn test

.gitignore

@@ -139,4 +139,10 @@ fabric.properties
 
 # Azure Toolkit for IntelliJ plugin
 # https://plugins.jetbrains.com/plugin/8053-azure-toolkit-for-intellij
-.idea/**/azureSettings.xml
+.idea/**/azureSettings.xml
+
+# Certora
+.certora_internal
+
+# VS Code
+.vscode

README.md

@@ -10,74 +10,96 @@ Candide Wallet is a smart contract wallet for Ethereum Mainnet and EVM compatibl
 This repo includes the smart contracts used by Candide Labs.
 
 # Features
+
 - <a href="https://eips.ethereum.org/EIPS/eip-4337">EIP-4337: Account Abstraction via Entry Point Contract</a>
 - Account Recovery
 - Pay gas with ERC-20 using a Paymaster
 
 # Account Recovery
 
-*In this section, we highlight and explain the [SocialRecoveryModule.sol](./contracts/modules/social_recovery/SocialRecoveryModule.sol) contract.*
+_In this section, we highlight and explain the [SocialRecoveryModule.sol](./contracts/modules/social_recovery/SocialRecoveryModule.sol) contract._
 
-The Account Recovery module is designed to work for both a single-owner account and an n-m multi-sig account. In the case of the single-owner account, the signer key is typically stored on the user's device. More specifically, owners can add recovery methods (also known as Guardians) to change the ownership of the account, in case their signer key is lost or compromised.
+The Account Recovery module is designed to work for both a single-owner account and an n-m multi-sig account. In the case of the single-owner account, the signer key is typically stored on the user's device. More specifically, owners can add recovery addresses (also known as Guardians) to change the ownership of the account, in case their signer key is lost or compromised.
 
 Recovery methods are typical Ethereum accounts. They can be:
+
 - Family & Friends' contacts
 - Hardware wallets
 - Institutions
 - Custodial services that offer cloud-based wallets
 
 Normal operations of the Account do not require the approval of added Guardians in the module.
 
-Owners of the account decide the threshold for the number of guardians needed for recovery, as well as the number of guardians. A typical single-owner account can have 3 guardians with a threshold of 2. This increases the likelihood that a single guardian can overtake the account.
+Owners of the account decide the threshold for the number of guardians needed for recovery, as well as the number of guardians. A typical single-owner account can have 3 guardians with a threshold of 2. This decreases the likelihood that a single guardian can overtake the account.
 
 Owners are encouraged to ask their guardians to provide fresh addresses. This makes them private and eliminates the possibility of malicious guardians cooperating against an owner. By design, a guardian does not need to necessarily store value in their account to maintain their duties, even during a recovery process.
 
+Once the recovery is initiated, the owners have until the `delayPeriod` to cancel the recovery, if the initiation was done with malicious intent. Once the `delayPeriod` is over, anyone can finalize the recovery to update the ownership of that particular Safe Wallet.
+
 Account Recovery interfaces can be built with or without a backend service:
 
-- An interface without a backend service can simply let each guardian submit their signatures separately. Once the threshold is meant, anyone can call execute recovery to start the recovery period.
+- An interface without a backend service can simply let each guardian submit their signatures separately. Once the threshold is met, anyone can call execute recovery to start the recovery period.
 
 - An interface that leverages a backend service can aggregate guardians' signatures so that only the last guardian executes the transaction and pay gas fees. This is similar to how Safe's interface works when multiple owners for a multi-sig sign transactions before submitting them.
 
 ## High-Level specs of methods
 
 We assume that the signer key belongs to its real owner. The probability of the signer key being in control of someone else should be close to zero. Under this model, we can build a simple yet highly secure non-custodial wallet. To enable that model to evolve if needed, upgrading the wallet to a new implementation requires the approval of only the owner of the account.
 
+| Method                        | Owner | Guardians | Anyone | Comment                                                                                                           |
+| ----------------------------- | ----- | --------- | ------ | ----------------------------------------------------------------------------------------------------------------- |
+| `addGuardianWithThreshold`    | X     |           |        | Owner can add a guardian with a new threshold                                                                     |
+| `revokeGuardianWithThreshold` | X     |           |        | Owner can remove a guardian from its list of guardians                                                            |
+| `confirmRecovery`             |       | X         |        | Lets a single guardian approve the execution of the recovery request                                              |
+| `multiConfirmRecovery`        |       | X         |        | Lets multiple guardians approve the execution of the recovery request                                             |
+| `cancelRecovery`              | X     |           |        | Lets an owner cancel an ongoing recovery request                                                                  |
+| `finalizeRecovery`            |       |           | X      | Finalizes an ongoing recovery request if the recovery period is over. The method is public and callable by anyone |
 
-| Method                        | Owner  | Guardians| Anyone | Comment                                                                                         |
-| ----------------------------  | ------ | ------   | ------ | ----------------------------------------------------------------------------------------------- |
-|`addGuardianWithThreshold`     | X      |          |        | Owner can add a guardian with a new threshold                                                   |
-| `revokeGuardianWithThreshold` | X      |          |        | Owner can remove a guardian from its list of guardians                                          |
-| `confirmRecovery`             |        | X        |        | Lets a single guardian approve the execution of the recovery request                            |
-| `multiConfirmRecovery`        |        | X        |        | Lets multiple guardians approve the execution of the recovery request                           |
-| `cancelRecovery`              | X      |          |        | Lets an owner cancel an ongoing recovery request                                                |
-| `finalizeRecovery`            |        |          |   X    | Finalizes an ongoing recovery request if the recovery period is over. The method is public and callable by anyone |
+## Audit
 
+- [For version 0.0.1 by Ackee Blockchain](./audit/ackee-blockchain-candide-social-recovery-report.pdf)
 
 # Development
 
-
 ### Install dependencies
+
 ```
 yarn install
 ```
 
 ### Add required .env variables
+
 ```
 cp .env.example .env
 ```
 
 ## Run tests
+
 ```
 yarn build
 yarn test
 ```
 
+## Run FV
+
+```
+certoraRun certora/conf/SocialRecoveryModule.conf
+certoraRun certora/conf/GuardianStorage.conf
+certoraRun certora/conf/RecoveryConfirmationSignatureValidity.conf
+```
+
+Note: You will need to install Certora CLI and a valid Certora Key for running FV. To provide a custom `solc` path, use `--solc` flag.
+
 <!-- LICENSE -->
+
 ## License
+
 GNU General Public License v3.0
 
 <!-- ACKNOWLEDGMENTS -->
+
 ## Acknowledgments
-* <a href='https://github.com/eth-infinitism/account-abstraction'>eth-infinitism/account-abstraction</a>
-* <a href='https://github.com/safe-global/safe-contracts'>Gnosis Safe Contracts</a>
-* <a href='https://eips.ethereum.org/EIPS/eip-4337'>EIP-4337: Account Abstraction via Entry Point Contract specification </a>
+
+- <a href='https://github.com/eth-infinitism/account-abstraction'>eth-infinitism/account-abstraction</a>
+- <a href='https://github.com/safe-global/safe-contracts'>Gnosis Safe Contracts</a>
+- <a href='https://eips.ethereum.org/EIPS/eip-4337'>EIP-4337: Account Abstraction via Entry Point Contract specification </a>

certora/conf/GuardianStorage.conf

@@ -0,0 +1,27 @@
+{
+    "files": [
+        "certora/harnesses/SocialRecoveryModuleHarness.sol",
+        "certora/harnesses/SafeHarness.sol"
+    ],
+    "msg": "GuardianManager: General Ruleset",
+    // Basic sanity performs vacuity checks and the trivial invariant checks. Other options are "none" or "advanced" which are both extreme ends.
+    "rule_sanity": "basic",
+    "solc": "solc-0.8.20",
+    // Coverage info is helpful in debugging when verification fails
+    "coverage_info": "basic",
+    // To prevent "unwiding condition in a loop" counterexample
+    // https://docs.certora.com/en/latest/docs/prover/cli/options.html#optimistic-loop
+    "optimistic_loop": true,
+    // Enable grounding for faster verification so that prover uses approximation in quantified statements.
+    "prover_args": [
+        "-smt_groundQuantifiers true -depth 0"
+    ],
+    "verify": "SocialRecoveryModuleHarness:certora/specs/GuardianStorage.spec",
+    "parametric_contracts" : [
+        "SocialRecoveryModuleHarness"
+    ],
+    "packages": [
+        "@safe-global=node_modules/@safe-global",
+        "@openzeppelin=node_modules/@openzeppelin"
+    ]
+}

certora/conf/RecoveryConfirmationSignatureValidity.conf

@@ -0,0 +1,20 @@
+{
+    "files": [
+        "certora/harnesses/SocialRecoveryModuleHarness.sol",
+        "certora/harnesses/SafeHarness.sol"
+    ],   
+    "msg": "SocialRecoveryModule: Signature Checks in the MultiConfirm Recovery function",
+    "rule_sanity": "basic",
+    "solc": "solc-0.8.20",
+    "verify": "SocialRecoveryModuleHarness:certora/specs/RecoveryConfirmationSignatureValidity.spec",
+    "parametric_contracts" : [
+        "SocialRecoveryModuleHarness"
+    ],
+    "packages": [
+        "@safe-global=node_modules/@safe-global",
+        "@openzeppelin=node_modules/@openzeppelin"
+    ],
+    "loop_iter": "3",
+    "optimistic_loop": true,
+    "optimistic_hashing": true,
+}

certora/conf/SocialRecoveryModule.conf

@@ -0,0 +1,21 @@
+{
+    "files": [
+        "certora/harnesses/SocialRecoveryModuleHarness.sol",
+        "certora/harnesses/SafeHarness.sol"
+    ],   
+    "msg": "SocialRecoveryModule: General Ruleset",
+    "rule_sanity": "basic",
+    "solc": "solc-0.8.20",
+    "verify": "SocialRecoveryModuleHarness:certora/specs/SocialRecoveryModule.spec",
+    "parametric_contracts" : [
+        "SocialRecoveryModuleHarness"
+    ],
+    "packages": [
+        "@safe-global=node_modules/@safe-global",
+        "@openzeppelin=node_modules/@openzeppelin"
+    ],
+    "loop_iter": "3",
+    "optimistic_loop": true,
+    "prover_args": ["-smt_groundQuantifiers false"],
+    "optimistic_hashing": true,
+}

certora/harnesses/SafeHarness.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity >=0.8.12 <0.9.0;
+import {Safe} from "@safe-global/safe-contracts/contracts/Safe.sol";
+
+contract SafeHarness is Safe {
+}

certora/harnesses/SocialRecoveryModuleHarness.sol

@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity >=0.8.12 <0.9.0;
+import {SocialRecoveryModule} from "../../contracts/modules/social_recovery/SocialRecoveryModule.sol";
+
+contract SocialRecoveryModuleHarness is SocialRecoveryModule {
+    constructor(uint256 _recoveryPeriod) SocialRecoveryModule(_recoveryPeriod) {}
+
+    /**
+     * @notice Counts the guardians by iterating through the linked list starting at the sentinel address,
+     * instead of relying on the count storage variable.
+     * @dev This would not count "shadow" guardians that are not part of the linked list, which would
+     * never happen assuming integrity of the linked list.
+     * @param _wallet The target wallet.
+     * @return count of guardians.
+     */
+    function countGuardians(address _wallet) public view returns (uint256 count) {
+        GuardianStorageEntry storage entry = entries[_wallet];
+        address currentGuardian = entry.guardians[SENTINEL_GUARDIANS];
+
+        // The sentinel guardian pointing to address 0 is the initial state for the
+        // guardian storage entry for an account and is equivalent to an empty list
+        // where the sentinel points to itself. We handle this special case here.
+        if (currentGuardian == address(0)) {
+            return 0;
+        }
+
+        while (currentGuardian != SENTINEL_GUARDIANS) {
+            currentGuardian = entry.guardians[currentGuardian];
+            require(currentGuardian != address(0), "Guardian is address(0)");
+            count++;
+        }
+    }
+
+    /**
+     * @notice Verifies a series of signatures associated with a wallet recovery process.
+     *         The function is copied from `multiConfirmRecovery` without the storage modifications.
+     * @dev This function checks the validity and order of signatures for a wallet recovery hash.
+     *      It ensures that all signatures are from the wallet's guardians and that they are in
+     *      ascending order to prevent duplicates. Null signatures must have the sender as the signer and the sender
+     *      must be a guardian.
+     * @param _wallet The address of the wallet being recovered.
+     * @param recoveryHash The hash of the recovery data which needs to be signed by the guardians.
+     * @param _signatures An array of SignatureData structures containing the signer's address and their signature.
+     */
+    function checkSignatures(address _wallet, bytes32 recoveryHash, SignatureData[] memory _signatures) public view {
+        require(_signatures.length > 0, "SM: empty signatures");
+
+        address lastSigner = address(0);
+        for (uint256 i = 0; i < _signatures.length; i++) {
+            SignatureData memory value = _signatures[i];
+            if (value.signature.length == 0) {
+                require(isGuardian(_wallet, msg.sender), "SM: sender not a guardian");
+                require(msg.sender == value.signer, "SM: null signature should have the signer as the sender");
+            } else {
+                validateGuardianSignature(_wallet, recoveryHash, value.signer, value.signature);
+            }
+            require(value.signer > lastSigner, "SM: duplicate signers/invalid ordering");
+            lastSigner = value.signer;
+        }
+    }
+
+    /**
+     * @notice Retrieves the guardian approval count for this particular recovery request at particular nonce.
+     * @param _wallet The target wallet.
+     * @param _newOwners The new owners' addressess.
+     * @param _newThreshold The new threshold for the safe.
+     * @param _nonce The nonce of the recovery request.
+     * @return approvalCount The wallet's current recovery request
+     */
+    function getRecoveryApprovalsWithNonce(
+        address _wallet,
+        address[] calldata _newOwners,
+        uint256 _newThreshold,
+        uint256 _nonce
+    ) public view returns (uint256 approvalCount) {
+        bytes32 recoveryHash = getRecoveryHash(_wallet, _newOwners, _newThreshold, _nonce);
+        address[] memory guardians = getGuardians(_wallet);
+        approvalCount = 0;
+        for (uint256 i = 0; i < guardians.length; i++) {
+            if (confirmedHashes[recoveryHash][guardians[i]]) {
+                approvalCount++;
+            }
+        }
+    }
+
+    /**
+     * @notice Returns the hash of all the guardians of a wallet.
+     * @param _wallet The target wallet.
+     * @return guardiansHash The hash of all the guardians of a wallet.
+     */
+    function guardiansHash(address _wallet) public view returns (bytes32) {
+        return keccak256(abi.encodePacked(getGuardians(_wallet)));
+    }
+}

certora/requirements.txt

@@ -0,0 +1 @@
+certora-cli==7.6.3