Social Recovery Wallet Challenge

README.md

@@ -1,6 +1,6 @@
 # Social Recovery Wallet Challenge - ETH Tech Tree
 
-Mother's day, 2023. You decide to send your mom some ETH to help her learn more about your world. You set up a new MetaMask wallet and write down the seed phrase on a nice peice of flowered stationary. You briefly consider custodying the phrase on her behalf, but ultimately decide against it. To understand your cypherpunk values, she needs to truly own her new gift. She's extatic. She immediately hops online, and for the next few days, continues to explore the rich new world that is web3. Then...disaster strikes. Her laptop dies and she's LOST HER SEED PHRASE.
+Mother's day, 2023. You decide to send your mom some ETH to help her learn more about your world. You set up a new MetaMask wallet and write down the seed phrase on a nice piece of flowered stationary. You briefly consider custodying the phrase on her behalf, but ultimately decide against it. To understand your cypherpunk values, she needs to truly own her new gift. She's ecstatic. She immediately hops online, and for the next few days, continues to explore the rich new world that is web3. Then...disaster strikes. Her laptop dies and she's LOST HER SEED PHRASE.
 
 ## Contents
 - [Requirements](#requirements)

packages/foundry/contracts/SocialRecoveryWallet.sol

@@ -26,6 +26,10 @@ contract SocialRecoveryWallet {
     error SocialRecoveryWallet__WalletInRecovery();
     /// @dev The guardian attempting to vote for recovery has already voted
     error SocialRecoveryWallet__AlreadyVoted();
+    /// @dev The `call()` function reverted when trying to send ETH or call another contract
+    error SocialRecoveryWallet__CallFailed();
+    /// @dev The threshold is set higher than the number of guardians
+    error SocialRecoveryWallet__ThresholdTooHigh();
 
     ///////////////////
     // State Variables
@@ -34,8 +38,11 @@ contract SocialRecoveryWallet {
 
     /// @dev Whether or not the wallet is actively being recovered
     bool public inRecovery;
+
     /// @dev The number of guardian votes required to recover the wallet
     uint256 public threshold;
+    /// @dev The number of guardians
+    uint256 public numGuardians;
     /// @dev A counter to keep track of the current recovery round
     uint256 public currRound;
 
@@ -97,25 +104,38 @@ contract SocialRecoveryWallet {
     constructor(address[] memory _guardians, uint256 _threshold) {
         owner = msg.sender;
         threshold = _threshold;
+        numGuardians = 0;
         currRound = 0;
         for (uint i = 0; i < _guardians.length; i++) {
-            isGuardian[_guardians[i]] = true;
+            if (!isGuardian[_guardians[i]]) {
+                isGuardian[_guardians[i]] = true;
+                numGuardians++;
+            }
         }
     }
 
     /*
      * @param _callee: The address of the contract or EOA you want to call
      * @param _value: The amount of ETH you're sending, if any
+     * Requirements:
+     * - Calls the address at _callee with the value and data passed
+     * - Emits a `SocialRecoveryWallet__CallFailed` error if the call reverts
      */
-    function sendEth(address _callee, uint256 _value) external onlyOwner notBeingRecovered returns (bytes memory) {
-        (bool success, bytes memory result) = _callee.call{value: _value}("");
-        require(success, "external call reverted");
+    function call(address _callee, uint256 _value, bytes calldata _data) external onlyOwner notBeingRecovered returns (bytes memory) {
+        (bool success, bytes memory result) = _callee.call{value: _value}(_data);
+        if (!success) {
+            revert SocialRecoveryWallet__CallFailed();
+        }
         return result;
     }
 
     /*
      * @notice The function for the first guardian to call in order to initiate the recovery process for the wallet
      * @param _proposedOwner: the address of the new owner that will take control of the wallet
+     * Requirements:
+     * - Puts contract into recovery mode
+     * - Records the proposed owner, current round, and the vote of the guardian making the call
+     * - Emits a `RecoveryInitiated` event
      */
     function initiateRecovery(address _proposedOwner) onlyGuardian notBeingRecovered external {
         currRound++;
@@ -132,8 +152,15 @@ contract SocialRecoveryWallet {
     /*
      * @notice For other guardians to call after the recovery process has been initiated. If the threshold is met, ownership the wallet will transfered and the recovery process completed
      * @param _proposedOwner: the address of the new owner that will take control of the wallet
+     * Requirements:
+     * - Records the vote of the guardian making the call
+     * - Emits a `RecoverySupported` event
+     * - If threshold is met:
+        * - Changes the owner of the wallet
+        * - Takes contract out of recovery mode
+        * - Emits a `RecoveryExecuted` event
      */
-    function supportRecovery(address _proposedOwner) onlyGuardian isBeingRecovered external {
+    function supportRecovery(address _proposedOwner) external onlyGuardian isBeingRecovered {
         if (recoveryRoundToGuardianVoted[currRecovery.round][msg.sender]) {
             revert SocialRecoveryWallet__AlreadyVoted();
         }
@@ -149,4 +176,36 @@ contract SocialRecoveryWallet {
             emit RecoveryExecuted(currRecovery.proposedOwner);
         }
     }
+
+    /*
+     * @param _guardian: The address of the contract or EOA to be added as a guardian
+     * Requirements:
+     * - Records the address as a guardian
+     */
+     function addGuardian(address _guardian) external onlyOwner {
+         isGuardian[_guardian] = true;
+         numGuardians++;
+     }
+
+    /*
+     * @param _guardian: The address of the contract or EOA to be removed as a guardian
+     * Requirements:
+     * - Removes the record of the address as a guardian
+     */
+     function removeGuardian(address _guardian) external onlyOwner {
+         delete isGuardian[_guardian];
+         numGuardians--;
+     }
+
+    /*
+     * @param _threshold: The number of guardian votes required to recover the wallet
+     * Requirements:
+     * - Sets the contract's threshold to the input
+     */
+     function setThreshold(uint256 _threshold) external onlyOwner {
+        if (_threshold > numGuardians) {
+            revert SocialRecoveryWallet__ThresholdTooHigh();
+        }
+        threshold = _threshold;
+     }
 }

packages/foundry/script/Deploy.s.sol

@@ -7,11 +7,7 @@ import "./DeployHelpers.s.sol";
 contract DeployScript is ScaffoldETHDeploy {
     error InvalidPrivateKey(string);
 
-    address guardian0 = 0x0b3aA6f7e5be55E7012A8677779B41487B424F70;
-    address guardian1 = 0x09F1E981Ac9c32D3E88819b0cE091Dc27f9cf857;
-    address guardian2 = 0x62bA14f9BBAe5aF1fE4b4cA4339d9ee332750E3F;
-
-    address[] chosenGuardianList = [guardian0, guardian1, guardian2];
+    address[] chosenGuardianList;
 
     function run() external {
         uint256 deployerPrivateKey = setupLocalhostEnv();

packages/foundry/test/SocialRecoveryWallet.t.sol

@@ -2,10 +2,13 @@
 pragma solidity ^0.8.13;
 
 import "forge-std/Test.sol";
+import "forge-std/StdUtils.sol";
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "../contracts/SocialRecoveryWallet.sol";
 
 contract SocialRecoveryWalletTest is Test {
     SocialRecoveryWallet public socialRecoveryWallet;
+    ERC20 public dai;
 
     address alice = makeAddr("alice");
 
@@ -22,6 +25,7 @@ contract SocialRecoveryWalletTest is Test {
 
     function setUp() public {
         socialRecoveryWallet = new SocialRecoveryWallet(chosenGuardianList, threshold);
+        dai = new ERC20("Dai", "DAI");
         vm.deal(address(socialRecoveryWallet), 1 ether);
     }
 
@@ -38,31 +42,36 @@ contract SocialRecoveryWalletTest is Test {
         assertEq(socialRecoveryWallet.threshold(), threshold);
     }
 
-    function testCanSendEth() public {
+    function testCallRevertsWithCorrectError() public {
+        vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__CallFailed.selector)));
+        socialRecoveryWallet.call(address(this), 0, "");
+    }
+
+    function testCallCanSendEth() public {
         uint256 initialValue = alice.balance;
 
         address recipient = alice;
         uint256 amountToSend = 1000;
 
-        socialRecoveryWallet.sendEth(recipient, amountToSend);
+        socialRecoveryWallet.call(recipient, amountToSend, "");
 
         assertEq(alice.balance, initialValue + amountToSend);
     }
 
-    function testCantSendIfNotOwner() public {
+    function testCantCallIfNotOwner() public {
         uint256 initialValue = alice.balance;
 
         address recipient = alice;
         uint256 amountToSend = 1000;
 
         vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotOwner.selector)));
         vm.prank(alice);
-        socialRecoveryWallet.sendEth(recipient, amountToSend);
+        socialRecoveryWallet.call(recipient, amountToSend, "");
 
         assertEq(alice.balance, initialValue);
     }
 
-    function testCantSendIfInRecovery() public {
+    function testCantCallIfInRecovery() public {
         vm.prank(guardian0);
         socialRecoveryWallet.initiateRecovery(newOwner);
 
@@ -72,7 +81,16 @@ contract SocialRecoveryWalletTest is Test {
         uint256 amountToSend = 1000;
 
         vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__WalletInRecovery.selector)));
-        socialRecoveryWallet.sendEth(recipient, amountToSend);
+        socialRecoveryWallet.call(recipient, amountToSend, "");
+    }
+
+    function testCallCanExecuteExternalTransactions() public {
+        // Sending an ERC20 for example
+        deal(address(dai), address(socialRecoveryWallet), 500);
+        assertEq(dai.balanceOf(alice), 0);
+
+        socialRecoveryWallet.call(address(dai), 0, abi.encodeWithSignature("transfer(address,uint256)", alice, 500));
+        assertEq(dai.balanceOf(alice), 500);
     }
 
     function testCanOnlyInitiateRecoveryIfGuardian() public {
@@ -165,4 +183,73 @@ contract SocialRecoveryWalletTest is Test {
         vm.prank(guardian2);
         socialRecoveryWallet.supportRecovery(newOwner);
     }
+
+    function testAddGuardian() public {
+        address newGuardian = makeAddr("newGuardian");
+        assertFalse(socialRecoveryWallet.isGuardian(newGuardian));
+
+        vm.prank(socialRecoveryWallet.owner());
+        socialRecoveryWallet.addGuardian(newGuardian);
+
+        assertTrue(socialRecoveryWallet.isGuardian(newGuardian));
+    }
+
+    function testCantAddGuardianIfNotOwner() public {
+        address newGuardian = makeAddr("newGuardian");
+        assertFalse(socialRecoveryWallet.isGuardian(newGuardian));
+
+        vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotOwner.selector)));
+        vm.prank(alice);
+        socialRecoveryWallet.addGuardian(newGuardian);
+
+        assertFalse(socialRecoveryWallet.isGuardian(newGuardian));
+    }
+
+    function testCantRemoveGuardianIfNotOwner() public {
+        assertTrue(socialRecoveryWallet.isGuardian(guardian0));
+
+        vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotOwner.selector)));
+        vm.prank(alice);
+        socialRecoveryWallet.removeGuardian(guardian0);
+
+        assertTrue(socialRecoveryWallet.isGuardian(guardian0));
+    }
+
+    function testRemoveGuardian() public {
+        assertTrue(socialRecoveryWallet.isGuardian(guardian0));
+
+        vm.prank(socialRecoveryWallet.owner());
+        socialRecoveryWallet.removeGuardian(guardian0);
+
+        assertFalse(socialRecoveryWallet.isGuardian(guardian0));
+    }
+
+    function testCantSetThresholdIfNotOwner() public {
+        uint256 newThreshold = 2;
+
+        vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotOwner.selector)));
+        vm.prank(alice);
+        socialRecoveryWallet.setThreshold(newThreshold);
+
+        assertEq(socialRecoveryWallet.threshold(), threshold);
+    }
+
+    function testCantSetThresholdHigherThanNumGuardians() public {
+        uint256 newThreshold = 5;
+
+        vm.prank(socialRecoveryWallet.owner());
+        vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__ThresholdTooHigh.selector)));
+        socialRecoveryWallet.setThreshold(newThreshold);
+
+        assertEq(socialRecoveryWallet.threshold(), threshold);
+    }
+
+    function testSetThreshold() public {
+        uint256 newThreshold = 2;
+
+        vm.prank(socialRecoveryWallet.owner());
+        socialRecoveryWallet.setThreshold(newThreshold);
+
+        assertEq(socialRecoveryWallet.threshold(), newThreshold);
+    }
 }