From 33dab9f33b2a4d48bf9fe01611ea0db414227c12 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Mon, 27 May 2024 15:44:24 -0700 Subject: [PATCH 01/19] rename contract --- .../contracts/{Challenge.sol => SocialRecoveryWallet.sol} | 2 +- .../test/{Challenge.t.sol => SocialRecoveryWallet.t.sol} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/foundry/contracts/{Challenge.sol => SocialRecoveryWallet.sol} (83%) rename packages/foundry/test/{Challenge.t.sol => SocialRecoveryWallet.t.sol} (100%) diff --git a/packages/foundry/contracts/Challenge.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol similarity index 83% rename from packages/foundry/contracts/Challenge.sol rename to packages/foundry/contracts/SocialRecoveryWallet.sol index 60da771..f2e8af6 100644 --- a/packages/foundry/contracts/Challenge.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -2,6 +2,6 @@ pragma solidity >=0.8.0 <0.9.0; import { console2 } from "forge-std/console2.sol"; -contract Challenge { +contract SocialRecoveryWallet { // Add the challenges contract here } diff --git a/packages/foundry/test/Challenge.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol similarity index 100% rename from packages/foundry/test/Challenge.t.sol rename to packages/foundry/test/SocialRecoveryWallet.t.sol From 3b0dfa6de78447311907a586cbbbcd4552e9ad7c Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Thu, 30 May 2024 11:06:22 -0700 Subject: [PATCH 02/19] add test contract --- .../foundry/test/SocialRecoveryWallet.t.sol | 165 +++++++++++++++++- 1 file changed, 162 insertions(+), 3 deletions(-) diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index a2340e9..e8e7b74 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/packages/foundry/test/SocialRecoveryWallet.t.sol @@ -2,8 +2,167 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import "../contracts/Challenge.sol"; +import "../contracts/SocialRecoveryWallet.sol"; -contract ChallengeTest is Test { - // Put tests here +contract SocialRecoveryWalletTest is Test { + SocialRecoveryWallet public socialRecoveryWallet; + + address alice = makeAddr("alice"); + + address guardian0 = makeAddr("guardian0"); + address guardian1 = makeAddr("guardian1"); + address guardian2 = makeAddr("guardian2"); + address guardian3 = makeAddr("guardian3"); + + address[] chosenGuardianList = [guardian0, guardian1, guardian2, guardian3]; + + uint256 threshold = 3; + + address newOwner = makeAddr("newOwner"); + + function setUp() public { + socialRecoveryWallet = new SocialRecoveryWallet(chosenGuardianList, threshold); + vm.deal(address(socialRecoveryWallet), 1 ether); + } + + function testConstructorSetsGaurdians() public { + assertTrue(socialRecoveryWallet.isGuardian(guardian0)); + assertTrue(socialRecoveryWallet.isGuardian(guardian1)); + assertTrue(socialRecoveryWallet.isGuardian(guardian2)); + + address nonGuardian = makeAddr("nonGuardian"); + assertFalse(socialRecoveryWallet.isGuardian(nonGuardian)); + } + + function testConstructorSetsThreshold() public { + assertEq(socialRecoveryWallet.threshold(), threshold); + } + + function testCanSendEth() public { + uint256 initialValue = alice.balance; + + address recipient = alice; + uint256 amountToSend = 1000; + + socialRecoveryWallet.sendEth(recipient, amountToSend); + + assertEq(alice.balance, initialValue + amountToSend); + } + + function testCantSendIfNotOwner() 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); + + assertEq(alice.balance, initialValue); + } + + function testCantSendIfInRecovery() public { + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + + assertTrue(socialRecoveryWallet.inRecovery()); + + address recipient = alice; + uint256 amountToSend = 1000; + + vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__WalletInRecovery.selector))); + socialRecoveryWallet.sendEth(recipient, amountToSend); + } + + function testCanOnlyInitiateRecoveryIfGuardian() public { + vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotGuardian.selector))); + vm.prank(alice); + socialRecoveryWallet.initiateRecovery(alice); + } + + function testInitiateRecoveryEmitsEvent() public { + vm.expectEmit(); + emit SocialRecoveryWallet.RecoveryInitiated(guardian0, newOwner); + + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + } + + function testInitiateRecoveryPutsWalletIntoRecovery() public { + assertFalse(socialRecoveryWallet.inRecovery()); + + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + + assertTrue(socialRecoveryWallet.inRecovery()); + } + + function testCanOnlySupportRecoveryIfGuardian() public { + vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotGuardian.selector))); + address nonGuardian = makeAddr("nonGuardian"); + vm.prank(nonGuardian); + socialRecoveryWallet.supportRecovery(newOwner); + } + + function testCanOnlySupportIfWalletIsInRecovery() public { + assertEq(socialRecoveryWallet.inRecovery(), false); + + vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__WalletNotInRecovery.selector))); + vm.prank(guardian0); + socialRecoveryWallet.supportRecovery(newOwner); + } + + function testCanOnlySupportRecoveryOnce() public { + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + + vm.prank(guardian1); + socialRecoveryWallet.supportRecovery(newOwner); + + vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__AlreadyVoted.selector))); + vm.prank(guardian1); + socialRecoveryWallet.supportRecovery(newOwner); + } + + function testSupportRecoveryEmitsEvent() public { + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + + vm.expectEmit(); + emit SocialRecoveryWallet.RecoverySupported(guardian1, newOwner); + + vm.prank(guardian1); + socialRecoveryWallet.supportRecovery(newOwner); + } + + function testSupportRecoveryChangesOwnerOnceThresholdMet() public { + assertEq(socialRecoveryWallet.threshold(), 3); + assertEq(socialRecoveryWallet.owner(), address(this)); + + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + + vm.prank(guardian1); + socialRecoveryWallet.supportRecovery(newOwner); + + vm.prank(guardian2); + socialRecoveryWallet.supportRecovery(newOwner); + + assertEq(socialRecoveryWallet.owner(), newOwner); + } + + function testSupportRecoveryEmitsEventWhenRecoveryExecuted() public { + vm.prank(guardian0); + socialRecoveryWallet.initiateRecovery(newOwner); + + vm.prank(guardian1); + socialRecoveryWallet.supportRecovery(newOwner); + + vm.expectEmit(); + emit SocialRecoveryWallet.RecoveryExecuted(newOwner); + + vm.prank(guardian2); + socialRecoveryWallet.supportRecovery(newOwner); + } } From 2968672f14ca0a3174c0b97cba718dc05547c00d Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Thu, 30 May 2024 11:07:42 -0700 Subject: [PATCH 03/19] add implementation contract --- .../contracts/SocialRecoveryWallet.sol | 147 +++++++++++++++++- 1 file changed, 146 insertions(+), 1 deletion(-) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index f2e8af6..da4ad64 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -1,7 +1,152 @@ //SPDX-License-Identifier: MIT + pragma solidity >=0.8.0 <0.9.0; + import { console2 } from "forge-std/console2.sol"; +/** + * @title Social Recovery Wallet Challenge Contract + * @author BUIDL GUIDL + * @notice This challenge contract is meant to allow users to recover control of their wallet in the event of a lost seed phrase. + * @dev The natspec is meant to be paired with the README.md to help guide you through this challenge! Goodluck! + * @dev This smart contract is PURELY EDUCATIONAL, and is not to be used in production code. It is up to the user's discretion to make their own production code, run tests, have audits, etc. + */ contract SocialRecoveryWallet { - // Add the challenges contract here + + /////////////////// + // Errors + /////////////////// + /// @dev The caller of the function is not the owner of the wallet + error SocialRecoveryWallet__NotOwner(); + /// @dev The caller of the function is not a guardian + error SocialRecoveryWallet__NotGuardian(); + /// @dev The wallet is not in the recovery state when it needs to be + error SocialRecoveryWallet__WalletNotInRecovery(); + /// @dev The wallet is in the recovery state when it shouldn't be + error SocialRecoveryWallet__WalletInRecovery(); + /// @dev The guardian attempting to vote for recovery has already voted + error SocialRecoveryWallet__AlreadyVoted(); + + /////////////////// + // State Variables + /////////////////// + address public owner; + + /// @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 A counter to keep track of the current recovery round + uint256 public currRound; + + /// @dev Mapping of round number to guardian address to whether or not they have voted + mapping(uint => mapping(address => bool)) public recoveryRoundToGuardianVoted; + /// @dev Mapping to keep track of whether or not an address is a guardian + mapping(address => bool) public isGuardian; + + /// @dev Struct to keep track of the current recovery info + struct Recovery { + address proposedOwner; + uint256 votes; + uint256 round; + } + + Recovery public currRecovery; + + /////////////////// + // Events + /////////////////// + event RecoveryInitiated(address indexed by, address newProposedOwner); + event RecoverySupported(address indexed by, address newProposedOwner); + event RecoveryExecuted(address newOwner); + + /////////////////// + // Modifiers + /////////////////// + modifier onlyOwner { + if (msg.sender != owner) { + revert SocialRecoveryWallet__NotOwner(); + } + _; + } + + modifier onlyGuardian { + if (!isGuardian[msg.sender]) { + revert SocialRecoveryWallet__NotGuardian(); + } + _; + } + + modifier notBeingRecovered { + if (inRecovery) { + revert SocialRecoveryWallet__WalletInRecovery(); + } + _; + } + + modifier isBeingRecovered { + if (!inRecovery) { + revert SocialRecoveryWallet__WalletNotInRecovery(); + } + _; + } + + /////////////////// + // Functions + /////////////////// + constructor(address[] memory _guardians, uint256 _threshold) { + owner = msg.sender; + threshold = _threshold; + currRound = 0; + for (uint i = 0; i < _guardians.length; i++) { + isGuardian[_guardians[i]] = true; + } + } + + /* + * @param _callee: The address of the contract or EOA you want to call + * @param _value: The amount of ETH you're sending, if any + */ + 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"); + 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 + */ + function initiateRecovery(address _proposedOwner) onlyGuardian notBeingRecovered external { + currRound++; + currRecovery = Recovery( + _proposedOwner, + 1, + currRound + ); + recoveryRoundToGuardianVoted[currRound][msg.sender] = true; + inRecovery = true; + emit RecoveryInitiated(msg.sender, _proposedOwner); + } + + /* + * @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 + */ + function supportRecovery(address _proposedOwner) onlyGuardian isBeingRecovered external { + if (recoveryRoundToGuardianVoted[currRecovery.round][msg.sender]) { + revert SocialRecoveryWallet__AlreadyVoted(); + } + + currRecovery.votes++; + recoveryRoundToGuardianVoted[currRecovery.round][msg.sender] = true; + + emit RecoverySupported(msg.sender, _proposedOwner); + + if (currRecovery.votes >= threshold) { + owner = currRecovery.proposedOwner; + inRecovery = false; + emit RecoveryExecuted(currRecovery.proposedOwner); + } + } } From 225b8185517fd633a7e68f559ac419ec23c52071 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Thu, 30 May 2024 11:19:43 -0700 Subject: [PATCH 04/19] update Deploy script contract to deploy social recovery wallet --- packages/foundry/script/Deploy.s.sol | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/foundry/script/Deploy.s.sol b/packages/foundry/script/Deploy.s.sol index 6ba503d..a2cb624 100644 --- a/packages/foundry/script/Deploy.s.sol +++ b/packages/foundry/script/Deploy.s.sol @@ -1,12 +1,18 @@ //SPDX-License-Identifier: MIT pragma solidity ^0.8.19; -import "../contracts/Challenge.sol"; +import "../contracts/SocialRecoveryWallet.sol"; 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]; + function run() external { uint256 deployerPrivateKey = setupLocalhostEnv(); if (deployerPrivateKey == 0) { @@ -15,11 +21,12 @@ contract DeployScript is ScaffoldETHDeploy { ); } vm.startBroadcast(deployerPrivateKey); - Challenge challenge = new Challenge(); + + SocialRecoveryWallet socialRecoveryWallet = new SocialRecoveryWallet(chosenGuardianList, 2); console.logString( string.concat( - "Challenge deployed at: ", - vm.toString(address(challenge)) + "SocialRecoveryWallet deployed at: ", + vm.toString(address(socialRecoveryWallet)) ) ); vm.stopBroadcast(); From 26c1133e5576940e9de4e150289c3853aa2caf12 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Thu, 30 May 2024 12:35:24 -0700 Subject: [PATCH 05/19] update readme --- README.md | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 5bac675..0dc612a 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,6 @@ -# Template For Challenge - ETH Tech Tree -*--Change the above "Template For Challenge" to the challenge name--* -*--Add a paragraph sized story that pulls in the challenger to their mission--* +# Social Recovery Wallet Challenge - ETH Tech Tree - -*--End of story section--* +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. ## Contents - [Requirements](#requirements) @@ -27,16 +24,12 @@ foundryup ``` ## Challenge Description -*--Edit this section--* -Write challenge description here... -Here are some helpful references: -*Replace with real resource links if any* -- [one](buidlguidl.com) -- [two](buidlguidl.com) -- [three](buidlguidl.com) +A year has passed, and after the horrific debacle of last year's mother's day, you've vowed to come up with a more user-friendly wallet design. You need it to be able to withstand the loss of a seed phrase, while retaining as much autonomy as possible. + +But how?? You hearken back to your own upbringing for inspiration. Sure, your mom was a central figure, but ultimately, you realize, **it took a village**. You decide to develop your new wallet with this same strategy. What if you could select a group of trustworthy `guardian` addresses that could come together to recover the wallet after the seed phrase was lost. -*--End of challenge specific section--* +With that idea, you begin work on your project in `packages/foundry/contracts/SocialRecoveryWallet.sol`. **Don't change any existing method names** as it will break tests but feel free to add additional methods if it helps you complete the task. @@ -53,4 +46,4 @@ yarn deploy in a third terminal start the NextJS front end: ``` yarn start -``` \ No newline at end of file +``` From f94075eabd8367428afec10fec2daaaf0d0a2c54 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Fri, 31 May 2024 11:12:11 -0700 Subject: [PATCH 06/19] fix spelling --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0dc612a..94b31c9 100644 --- a/README.md +++ b/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) From 71d6bda6218aaf8241991c064cea0ce0f8d447d6 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Fri, 31 May 2024 12:56:52 -0700 Subject: [PATCH 07/19] `sendEth()` => `call()` plus custom revert error --- packages/foundry/contracts/SocialRecoveryWallet.sol | 10 +++++++--- packages/foundry/test/SocialRecoveryWallet.t.sol | 11 ++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index da4ad64..dd9f8ba 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -26,6 +26,8 @@ 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(); /////////////////// // State Variables @@ -107,9 +109,11 @@ contract SocialRecoveryWallet { * @param _callee: The address of the contract or EOA you want to call * @param _value: The amount of ETH you're sending, if any */ - 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; } diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index e8e7b74..f898c6a 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/packages/foundry/test/SocialRecoveryWallet.t.sol @@ -38,13 +38,18 @@ contract SocialRecoveryWalletTest is Test { assertEq(socialRecoveryWallet.threshold(), threshold); } + function testCallRevertsWithCorrectError() public { + vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__CallFailed.selector))); + socialRecoveryWallet.call(address(this), 0, ""); + } + function testCanSendEth() public { uint256 initialValue = alice.balance; address recipient = alice; uint256 amountToSend = 1000; - socialRecoveryWallet.sendEth(recipient, amountToSend); + socialRecoveryWallet.call(recipient, amountToSend, ""); assertEq(alice.balance, initialValue + amountToSend); } @@ -57,7 +62,7 @@ contract SocialRecoveryWalletTest is Test { vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotOwner.selector))); vm.prank(alice); - socialRecoveryWallet.sendEth(recipient, amountToSend); + socialRecoveryWallet.call(recipient, amountToSend, ""); assertEq(alice.balance, initialValue); } @@ -72,7 +77,7 @@ 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 testCanOnlyInitiateRecoveryIfGuardian() public { From bea4a5d051526028caa56a8756dc0af4120f7749 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Sun, 2 Jun 2024 14:30:39 -0700 Subject: [PATCH 08/19] test `call()` with an external tx --- .../foundry/test/SocialRecoveryWallet.t.sol | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index f898c6a..474c902 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/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); } @@ -43,7 +47,7 @@ contract SocialRecoveryWalletTest is Test { socialRecoveryWallet.call(address(this), 0, ""); } - function testCanSendEth() public { + function testCallCanSendEth() public { uint256 initialValue = alice.balance; address recipient = alice; @@ -54,7 +58,7 @@ contract SocialRecoveryWalletTest is Test { assertEq(alice.balance, initialValue + amountToSend); } - function testCantSendIfNotOwner() public { + function testCantCallIfNotOwner() public { uint256 initialValue = alice.balance; address recipient = alice; @@ -67,7 +71,7 @@ contract SocialRecoveryWalletTest is Test { assertEq(alice.balance, initialValue); } - function testCantSendIfInRecovery() public { + function testCantCallIfInRecovery() public { vm.prank(guardian0); socialRecoveryWallet.initiateRecovery(newOwner); @@ -80,6 +84,15 @@ contract SocialRecoveryWalletTest is Test { 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 { vm.expectRevert(bytes(abi.encodeWithSelector(SocialRecoveryWallet.SocialRecoveryWallet__NotGuardian.selector))); vm.prank(alice); From 9eb55173ea61d7eb74ac01ddfec555a613b3a93a Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Sun, 2 Jun 2024 14:34:00 -0700 Subject: [PATCH 09/19] reposition `external` function label --- packages/foundry/contracts/SocialRecoveryWallet.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index dd9f8ba..b1dcae8 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -137,7 +137,7 @@ 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 */ - function supportRecovery(address _proposedOwner) onlyGuardian isBeingRecovered external { + function supportRecovery(address _proposedOwner) external onlyGuardian isBeingRecovered { if (recoveryRoundToGuardianVoted[currRecovery.round][msg.sender]) { revert SocialRecoveryWallet__AlreadyVoted(); } From 7d5b347453c8de7f9d0004c633c6170c36e9b882 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Sun, 2 Jun 2024 14:52:22 -0700 Subject: [PATCH 10/19] add requirement docs to functions --- .../foundry/contracts/SocialRecoveryWallet.sol | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index b1dcae8..589d7d3 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -108,6 +108,9 @@ contract SocialRecoveryWallet { /* * @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 call(address _callee, uint256 _value, bytes calldata _data) external onlyOwner notBeingRecovered returns (bytes memory) { (bool success, bytes memory result) = _callee.call{value: _value}(_data); @@ -120,6 +123,10 @@ contract SocialRecoveryWallet { /* * @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++; @@ -136,6 +143,13 @@ 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) external onlyGuardian isBeingRecovered { if (recoveryRoundToGuardianVoted[currRecovery.round][msg.sender]) { From e3aa81d5f7b7ee396c8d6c27fa171f12a27e9cf2 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Sun, 2 Jun 2024 15:08:39 -0700 Subject: [PATCH 11/19] test add and remove guardian functions --- .../foundry/test/SocialRecoveryWallet.t.sol | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index 474c902..00f0a39 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/packages/foundry/test/SocialRecoveryWallet.t.sol @@ -183,4 +183,44 @@ 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)); + } } From aca78b84a5bbf1fbc89778c841b86fae4ec9f764 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Sun, 2 Jun 2024 15:09:14 -0700 Subject: [PATCH 12/19] add functionality to add and remove guardians --- .../foundry/contracts/SocialRecoveryWallet.sol | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index 589d7d3..96e546a 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -167,4 +167,22 @@ 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; + } + + /* + * @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]; + } } From 2c4f9636e44f0572573c23d48d58fad59152571f Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Mon, 3 Jun 2024 11:27:18 -0700 Subject: [PATCH 13/19] add tests for setThreshold() func --- .../foundry/test/SocialRecoveryWallet.t.sol | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index 00f0a39..a11f6a3 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/packages/foundry/test/SocialRecoveryWallet.t.sol @@ -223,4 +223,33 @@ contract SocialRecoveryWalletTest is Test { 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); + } } From e8867cfc2cc351b1b9b76c3e8d4383fc63301cee Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Mon, 3 Jun 2024 11:28:02 -0700 Subject: [PATCH 14/19] add `setThreshold()` function --- .../contracts/SocialRecoveryWallet.sol | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index 96e546a..0a6f01e 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -28,6 +28,8 @@ contract SocialRecoveryWallet { 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 @@ -36,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; @@ -99,9 +104,13 @@ 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++; + } } } @@ -175,6 +184,7 @@ contract SocialRecoveryWallet { */ function addGuardian(address _guardian) external onlyOwner { isGuardian[_guardian] = true; + numGuardians++; } /* @@ -184,5 +194,18 @@ contract SocialRecoveryWallet { */ 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; } } From e8507cfa7d3c3cdd28bae9f87b73ab5c46ca2133 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Mon, 3 Jun 2024 11:30:18 -0700 Subject: [PATCH 15/19] remove hardcoded guardian addresses from Deploy script --- packages/foundry/script/Deploy.s.sol | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/foundry/script/Deploy.s.sol b/packages/foundry/script/Deploy.s.sol index a2cb624..ccfeb24 100644 --- a/packages/foundry/script/Deploy.s.sol +++ b/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(); From c9377b459d6def9e03e9a1703e672d4a9115bac6 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Wed, 5 Jun 2024 11:45:54 -0700 Subject: [PATCH 16/19] add error to requirements --- packages/foundry/contracts/SocialRecoveryWallet.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index 0a6f01e..ec7fb3f 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -201,6 +201,7 @@ contract SocialRecoveryWallet { * @param _threshold: The number of guardian votes required to recover the wallet * Requirements: * - Sets the contract's threshold to the input + * - Reverts with SocialRecoveryWallet__ThresholdTooHigh if trying to set threshold higher than the number of guardians */ function setThreshold(uint256 _threshold) external onlyOwner { if (_threshold > numGuardians) { From f4dcd977220ad35f0df663c0dce394d2bd1b3d50 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Wed, 5 Jun 2024 11:55:35 -0700 Subject: [PATCH 17/19] update `testAddGuardian()` --- packages/foundry/contracts/SocialRecoveryWallet.sol | 3 +++ packages/foundry/test/SocialRecoveryWallet.t.sol | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index ec7fb3f..930d4ae 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -183,6 +183,9 @@ contract SocialRecoveryWallet { * - Records the address as a guardian */ function addGuardian(address _guardian) external onlyOwner { + if (isGuardian[_guardian]) { + return; + } isGuardian[_guardian] = true; numGuardians++; } diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index a11f6a3..2c1d00e 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/packages/foundry/test/SocialRecoveryWallet.t.sol @@ -186,12 +186,22 @@ contract SocialRecoveryWalletTest is Test { function testAddGuardian() public { address newGuardian = makeAddr("newGuardian"); + uint256 original_num_guardians = socialRecoveryWallet.numGuardians(); assertFalse(socialRecoveryWallet.isGuardian(newGuardian)); vm.prank(socialRecoveryWallet.owner()); socialRecoveryWallet.addGuardian(newGuardian); assertTrue(socialRecoveryWallet.isGuardian(newGuardian)); + assertEq(socialRecoveryWallet.numGuardians(), original_num_guardians + 1); + + // Try to add the same guardian again + original_num_guardians = socialRecoveryWallet.numGuardians(); + vm.prank(socialRecoveryWallet.owner()); + socialRecoveryWallet.addGuardian(newGuardian); + + assertTrue(socialRecoveryWallet.isGuardian(newGuardian)); + assertEq(socialRecoveryWallet.numGuardians(), original_num_guardians); } function testCantAddGuardianIfNotOwner() public { From 5d375f1d16d244955757c65c07842222b493a8d9 Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Wed, 5 Jun 2024 11:59:14 -0700 Subject: [PATCH 18/19] update `testRemoveGuardian()` --- packages/foundry/contracts/SocialRecoveryWallet.sol | 3 +++ packages/foundry/test/SocialRecoveryWallet.t.sol | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index 930d4ae..848ba96 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -196,6 +196,9 @@ contract SocialRecoveryWallet { * - Removes the record of the address as a guardian */ function removeGuardian(address _guardian) external onlyOwner { + if (!isGuardian[_guardian]) { + return; + } delete isGuardian[_guardian]; numGuardians--; } diff --git a/packages/foundry/test/SocialRecoveryWallet.t.sol b/packages/foundry/test/SocialRecoveryWallet.t.sol index 2c1d00e..9f0490f 100644 --- a/packages/foundry/test/SocialRecoveryWallet.t.sol +++ b/packages/foundry/test/SocialRecoveryWallet.t.sol @@ -226,12 +226,22 @@ contract SocialRecoveryWalletTest is Test { } function testRemoveGuardian() public { + uint256 original_num_guardians = socialRecoveryWallet.numGuardians(); assertTrue(socialRecoveryWallet.isGuardian(guardian0)); vm.prank(socialRecoveryWallet.owner()); socialRecoveryWallet.removeGuardian(guardian0); assertFalse(socialRecoveryWallet.isGuardian(guardian0)); + assertEq(socialRecoveryWallet.numGuardians(), original_num_guardians - 1); + + // Try to remove address that was never a guardian + original_num_guardians = socialRecoveryWallet.numGuardians(); + vm.prank(socialRecoveryWallet.owner()); + socialRecoveryWallet.removeGuardian(alice); + + assertFalse(socialRecoveryWallet.isGuardian(alice)); + assertEq(socialRecoveryWallet.numGuardians(), original_num_guardians); } function testCantSetThresholdIfNotOwner() public { From bff2f8082b734b7fe6c2adfdaa2f866ad0f2692b Mon Sep 17 00:00:00 2001 From: Sam Wellander Date: Wed, 5 Jun 2024 12:00:42 -0700 Subject: [PATCH 19/19] update requirements of add/remove guardian funcs --- packages/foundry/contracts/SocialRecoveryWallet.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/foundry/contracts/SocialRecoveryWallet.sol b/packages/foundry/contracts/SocialRecoveryWallet.sol index 848ba96..3f7f173 100644 --- a/packages/foundry/contracts/SocialRecoveryWallet.sol +++ b/packages/foundry/contracts/SocialRecoveryWallet.sol @@ -181,6 +181,7 @@ contract SocialRecoveryWallet { * @param _guardian: The address of the contract or EOA to be added as a guardian * Requirements: * - Records the address as a guardian + * - Updates the numGuardians variable */ function addGuardian(address _guardian) external onlyOwner { if (isGuardian[_guardian]) { @@ -194,6 +195,7 @@ contract SocialRecoveryWallet { * @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 + * - Updates the numGuardians variable */ function removeGuardian(address _guardian) external onlyOwner { if (!isGuardian[_guardian]) {