diff --git a/.vscode/settings.json b/.vscode/settings.json index 0feca58..4c0c8df 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,5 @@ { "solidity.compileUsingRemoteVersion": "v0.8.25+commit.b61c2a91", - "solidity.formatter": "forge" + "solidity.formatter": "forge", + "solidity.packageDefaultDependenciesDirectory": "node_modules" } diff --git a/src/EmailRecoveryManager.sol b/src/EmailRecoveryManager.sol index bfe82b0..5280592 100644 --- a/src/EmailRecoveryManager.sol +++ b/src/EmailRecoveryManager.sol @@ -34,14 +34,20 @@ abstract contract EmailRecoveryManager is /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* CONSTANTS & STORAGE */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - using EnumerableMap for EnumerableMap.Bytes32ToUintMap; using EnumerableMap for EnumerableMap.AddressToUintMap; + /** * Minimum required time window between when a recovery attempt becomes valid and when it * becomes invalid */ uint256 public constant MINIMUM_RECOVERY_WINDOW = 2 days; + /** + * The cooldown period after which a subsequent recovery attempt can be initiated by the same + * guardian + */ + uint256 public constant CANCEL_EXPIRED_RECOVERY_COOLDOWN = 1 days; + /** * The command handler that returns and validates the command templates */ @@ -57,6 +63,12 @@ abstract contract EmailRecoveryManager is */ mapping(address account => RecoveryRequest recoveryRequest) internal recoveryRequests; + /** + * Account address to previous recovery request + */ + mapping(address account => PreviousRecoveryRequest previousRecoveryRequest) internal + previousRecoveryRequests; + constructor( address _verifier, address _dkimRegistry, @@ -95,46 +107,39 @@ abstract contract EmailRecoveryManager is return recoveryConfigs[account]; } + // TODO: test /** * @notice Retrieves the recovery request details for a given account * @param account The address of the account for which the recovery request details are being * retrieved - * @return executeAfter // TODO: - * @return executeBefore // TODO: + * @return executeAfter The timestamp from which the recovery request can be executed + * @return executeBefore The timestamp from which the recovery request becomes invalid + * @return currentWeight Total weight of all guardian approvals for the recovery request + * @return recoveryDataHash The keccak256 hash of the recovery data used to execute the recovery + * attempt */ function getRecoveryRequest(address account) external view - returns (uint256 executeAfter, uint256 executeBefore) - { - return (recoveryRequests[account].executeAfter, recoveryRequests[account].executeBefore); - } - - // TODO: natspec - function getRecoveryDataHashWeight( - address account, - bytes32 recoveryDataHash - ) - public - view - returns (uint256) + returns ( + uint256 executeAfter, + uint256 executeBefore, + uint256 currentWeight, + bytes32 recoveryDataHash + ) { - if(!recoveryRequests[account].recoveryDataHashWeight.contains(recoveryDataHash)) { - return 0; - } - return recoveryRequests[account].recoveryDataHashWeight.get(recoveryDataHash); + return ( + recoveryRequests[account].executeAfter, + recoveryRequests[account].executeBefore, + recoveryRequests[account].currentWeight, + recoveryRequests[account].recoveryDataHash + ); } + // TODO: test // TODO: natspec - function hasGuardianVoted( - address account, - address guardian - ) - public - view - returns (bool) - { - if(!recoveryRequests[account].guardianVoted.contains(guardian)) { + function hasGuardianVoted(address account, address guardian) public view returns (bool) { + if (!recoveryRequests[account].guardianVoted.contains(guardian)) { return false; } return recoveryRequests[account].guardianVoted.get(guardian) == 1; @@ -315,7 +320,7 @@ abstract contract EmailRecoveryManager is templateIdx, commandParams ); - if (recoveryRequests[account].executeBefore != 0) { + if (recoveryRequests[account].currentWeight > 0) { revert RecoveryInProcess(); } @@ -340,6 +345,7 @@ abstract contract EmailRecoveryManager is /* HANDLE RECOVERY */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + // TODO: update tests /** * @notice Processes a recovery request for a given account. This is the third core function * that must be called during the end-to-end recovery flow @@ -389,17 +395,33 @@ abstract contract EmailRecoveryManager is revert GuardianAlreadyVoted(); } - uint256 executeBefore = block.timestamp + recoveryConfigs[account].expiry; - recoveryRequest.executeBefore = executeBefore; - recoveryRequest.guardianVoted.set(guardian,1); + // A malicious guardian can submit an invalid recovery hash that the + // other guardians do not agree with, and also re-submit the same invalid hash once + // the expired recovery attempt has been cancelled, thereby threatening the + // liveness of the recovery attempt. + uint256 guardianCount = guardianCount(account); + bool cooldownNotExpired = + previousRecoveryRequests[account].cancelRecoveryCooldown > block.timestamp; + if ( + previousRecoveryRequests[account].previousGuardianInitiated == guardian + && cooldownNotExpired && guardianCount > 1 + ) { + revert GuardianMustWaitForCooldown(guardian); + } + + if (recoveryRequest.recoveryDataHash == bytes32(0)) { + recoveryRequest.recoveryDataHash = recoveryDataHash; + previousRecoveryRequests[account].previousGuardianInitiated = guardian; + uint256 executeBefore = block.timestamp + recoveryConfigs[account].expiry; + recoveryRequest.executeBefore = executeBefore; + } - uint256 currentWeight = 0; - if(recoveryRequest.recoveryDataHashWeight.contains(recoveryDataHash)) { - currentWeight = recoveryRequest.recoveryDataHashWeight.get(recoveryDataHash); + if (recoveryRequest.recoveryDataHash != recoveryDataHash) { + revert InvalidRecoveryDataHash(recoveryDataHash, recoveryRequest.recoveryDataHash); } - recoveryRequest.recoveryDataHashWeight.set(recoveryDataHash, currentWeight += guardianStorage.weight); - if (recoveryRequest.recoveryDataHashWeight.get(recoveryDataHash) >= guardianConfig.threshold) { + recoveryRequest.currentWeight += guardianStorage.weight; + if (recoveryRequest.currentWeight >= guardianConfig.threshold) { uint256 executeAfter = block.timestamp + recoveryConfigs[account].delay; recoveryRequest.executeAfter = executeAfter; @@ -439,13 +461,8 @@ abstract contract EmailRecoveryManager is revert NoRecoveryConfigured(); } - bytes32 recoveryDataHash = keccak256(recoveryData); - uint256 currentWeight = 0; - if(recoveryRequest.recoveryDataHashWeight.contains(recoveryDataHash)) { - currentWeight = recoveryRequest.recoveryDataHashWeight.get(recoveryDataHash); - } - if (currentWeight < threshold) { - revert NotEnoughApprovals(currentWeight, threshold); + if (recoveryRequest.currentWeight < threshold) { + revert NotEnoughApprovals(recoveryRequest.currentWeight, threshold); } if (block.timestamp < recoveryRequest.executeAfter) { @@ -456,6 +473,11 @@ abstract contract EmailRecoveryManager is revert RecoveryRequestExpired(block.timestamp, recoveryRequest.executeBefore); } + bytes32 recoveryDataHash = keccak256(recoveryData); + if (recoveryDataHash != recoveryRequest.recoveryDataHash) { + revert InvalidRecoveryDataHash(recoveryDataHash, recoveryRequest.recoveryDataHash); + } + clearRecoveryRequest(account); recover(account, recoveryData); @@ -487,13 +509,14 @@ abstract contract EmailRecoveryManager is * @dev Deletes the current recovery request associated with the caller's account */ function cancelRecovery() external { - if (recoveryRequests[msg.sender].executeBefore == 0) { + if (recoveryRequests[msg.sender].currentWeight == 0) { revert NoRecoveryInProcess(); } clearRecoveryRequest(msg.sender); emit RecoveryCancelled(msg.sender); } + // TODO: update tests /** * @notice Cancels the recovery request for a given account if it is expired. * @dev Deletes the current recovery request associated with the given account if the recovery @@ -501,7 +524,7 @@ abstract contract EmailRecoveryManager is * @param account The address of the account for which the recovery is being cancelled */ function cancelExpiredRecovery(address account) external { - if (recoveryRequests[account].executeBefore == 0) { + if (recoveryRequests[account].currentWeight == 0) { revert NoRecoveryInProcess(); } if (recoveryRequests[account].executeBefore > block.timestamp) { @@ -509,6 +532,8 @@ abstract contract EmailRecoveryManager is account, block.timestamp, recoveryRequests[account].executeBefore ); } + previousRecoveryRequests[account].cancelRecoveryCooldown = + block.timestamp + CANCEL_EXPIRED_RECOVERY_COOLDOWN; clearRecoveryRequest(account); emit RecoveryCancelled(account); } @@ -523,6 +548,7 @@ abstract contract EmailRecoveryManager is deInitRecoveryModule(account); } + // TODO: update tests /** * @notice Removes all state related to an account. * @dev Although this function is internal, it should be used carefully as it can be called by @@ -534,6 +560,7 @@ abstract contract EmailRecoveryManager is function deInitRecoveryModule(address account) internal onlyWhenNotRecovering { delete recoveryConfigs[account]; delete recoveryRequests[account]; + delete previousRecoveryRequests[account]; removeAllGuardians(account); delete guardianConfigs[account]; @@ -541,24 +568,17 @@ abstract contract EmailRecoveryManager is emit RecoveryDeInitialized(account); } + // TODO: test + // TODO: natspec function clearRecoveryRequest(address account) internal { RecoveryRequest storage recoveryRequest = recoveryRequests[account]; - uint256 lengthWeights = recoveryRequest.recoveryDataHashWeight.length(); - bytes32[] memory keysWeights = new bytes32[](lengthWeights); - for (uint256 i = 0; i < lengthWeights; i++) { - (bytes32 key, ) = recoveryRequest.recoveryDataHashWeight.at(i); - keysWeights[i] = key; - } - for(uint256 i = 0; i < lengthWeights; i++) { - recoveryRequest.recoveryDataHashWeight.remove(keysWeights[i]); - } uint256 lengthVotes = recoveryRequest.guardianVoted.length(); address[] memory keysVotes = new address[](lengthVotes); for (uint256 i = 0; i < lengthVotes; i++) { - (address key, ) = recoveryRequest.guardianVoted.at(i); + (address key,) = recoveryRequest.guardianVoted.at(i); keysVotes[i] = key; } - for(uint256 i = 0; i < lengthVotes; i++) { + for (uint256 i = 0; i < lengthVotes; i++) { recoveryRequest.guardianVoted.remove(keysVotes[i]); } delete recoveryRequests[account]; diff --git a/src/GuardianManager.sol b/src/GuardianManager.sol index 3436d9b..cfcc325 100644 --- a/src/GuardianManager.sol +++ b/src/GuardianManager.sol @@ -32,9 +32,9 @@ abstract contract GuardianManager is IGuardianManager { * @notice Modifier to check recovery status. Reverts if recovery is in process for the account */ modifier onlyWhenNotRecovering() { - (, uint256 executeBefore) = + (,, uint256 currentWeight,) = IEmailRecoveryManager(address(this)).getRecoveryRequest(msg.sender); - if (executeBefore != 0) { + if (currentWeight > 0) { revert RecoveryInProcess(); } _; @@ -252,4 +252,10 @@ abstract contract GuardianManager is IGuardianManager { function removeAllGuardians(address account) internal { guardiansStorage[account].removeAll(guardiansStorage[account].keys()); } + + // TODO: test + // TODO: natspec + function guardianCount(address account) internal view returns (uint256) { + return guardiansStorage[account].length(); + } } diff --git a/src/interfaces/IEmailRecoveryManager.sol b/src/interfaces/IEmailRecoveryManager.sol index 5d518a6..f9986a8 100644 --- a/src/interfaces/IEmailRecoveryManager.sol +++ b/src/interfaces/IEmailRecoveryManager.sol @@ -23,6 +23,13 @@ interface IEmailRecoveryManager { // request } + struct PreviousRecoveryRequest { + address previousGuardianInitiated; // the address of the guardian who initiated the previous + // recovery request. Used to prevent a malicious guardian threatening the liveness of + // the recovery attempt + uint256 cancelRecoveryCooldown; + } + /** * A struct representing the values required for a recovery request * The request state should be maintained over a single recovery attempts unless @@ -31,8 +38,9 @@ interface IEmailRecoveryManager { struct RecoveryRequest { uint256 executeAfter; // the timestamp from which the recovery request can be executed uint256 executeBefore; // the timestamp from which the recovery request becomes invalid - // mapping(bytes32 recoveryDataHash => uint256 currentWeight) recoveryDataHashWeight; // total weight of all guardian approvals for the recovery request - EnumerableMap.Bytes32ToUintMap recoveryDataHashWeight; + uint256 currentWeight; // total weight of all guardian approvals for the recovery request + bytes32 recoveryDataHash; // the keccak256 hash of the recovery data used to execute the + // recovery attempt EnumerableMap.AddressToUintMap guardianVoted; } @@ -74,6 +82,7 @@ interface IEmailRecoveryManager { GuardianStatus guardianStatus, GuardianStatus expectedGuardianStatus ); error GuardianAlreadyVoted(); + error GuardianMustWaitForCooldown(address guardian); error InvalidAccountAddress(); error NoRecoveryConfigured(); error NotEnoughApprovals(uint256 currentWeight, uint256 threshold); @@ -93,7 +102,12 @@ interface IEmailRecoveryManager { function getRecoveryRequest(address account) external view - returns (uint256 executeAfter, uint256 executeBefore); + returns ( + uint256 executeAfter, + uint256 executeBefore, + uint256 currentWeight, + bytes32 recoveryDataHash + ); function updateRecoveryConfig(RecoveryConfig calldata recoveryConfig) external; diff --git a/src/libraries/EnumerableGuardianMap.sol b/src/libraries/EnumerableGuardianMap.sol index a55b586..e1f07c5 100644 --- a/src/libraries/EnumerableGuardianMap.sol +++ b/src/libraries/EnumerableGuardianMap.sol @@ -134,4 +134,10 @@ library EnumerableGuardianMap { function keys(AddressToGuardianMap storage map) internal view returns (address[] memory) { return map._keys.values(); } + + // TODO: test + // TODO: natspec + function length(AddressToGuardianMap storage map) internal view returns (uint256) { + return map._keys.length(); + } } diff --git a/test/integration/EmailRecoveryManager/EmailRecoveryManager.integration.t.sol b/test/integration/EmailRecoveryManager/EmailRecoveryManager.integration.t.sol index 2cd3fff..a017f99 100644 --- a/test/integration/EmailRecoveryManager/EmailRecoveryManager.integration.t.sol +++ b/test/integration/EmailRecoveryManager/EmailRecoveryManager.integration.t.sol @@ -176,16 +176,16 @@ contract EmailRecoveryManager_Integration_Test is vm.warp(block.timestamp + delay); emailRecoveryModule.completeRecovery(accountAddress1, recoveryData1); - uint256 weight = emailRecoveryModule.getRecoveryDataHashWeight(accountAddress1, recoveryDataHash1); - assertEq(weight, 0); + (,, uint256 currentWeight,) = emailRecoveryModule.getRecoveryRequest(accountAddress1); + assertEq(currentWeight, 0); EmailAuthMsg memory emailAuthMsg = getRecoveryEmailAuthMessage( accountAddress1, guardians1[2], recoveryDataHash1, emailRecoveryModuleAddress ); emailRecoveryModule.handleRecovery(emailAuthMsg, templateIdx); - weight = emailRecoveryModule.getRecoveryDataHashWeight(accountAddress1, recoveryDataHash1); - assertEq(weight, 1); + (,, currentWeight,) = emailRecoveryModule.getRecoveryRequest(accountAddress1); + assertEq(currentWeight, 1); } function test_RevertWhen_CompleteRecoveryCalled_BeforeConfigureRecovery() public { @@ -272,15 +272,22 @@ contract EmailRecoveryManager_Integration_Test is emailRecoveryModule.cancelRecovery(); vm.stopPrank(); - (uint256 _executeAfter, uint256 executeBefore) = - emailRecoveryModule.getRecoveryRequest(accountAddress1); - uint256 weight = emailRecoveryModule.getRecoveryDataHashWeight(accountAddress1, recoveryDataHash1); - bool hasGuardian1Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[0]); - bool hasGuardian2Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[1]); - bool hasGuardian3Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[2]); + ( + uint256 _executeAfter, + uint256 executeBefore, + uint256 currentWeight, + bytes32 recoveryDataHash + ) = emailRecoveryModule.getRecoveryRequest(accountAddress1); + bool hasGuardian1Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[0]); + bool hasGuardian2Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[1]); + bool hasGuardian3Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[2]); assertEq(_executeAfter, 0); assertEq(executeBefore, 0); - assertEq(weight, 0); + assertEq(currentWeight, 0); + assertEq(recoveryDataHash, bytes32(0)); assertEq(hasGuardian1Voted, false); assertEq(hasGuardian2Voted, false); assertEq(hasGuardian3Voted, false); @@ -311,15 +318,22 @@ contract EmailRecoveryManager_Integration_Test is emailRecoveryModule.cancelExpiredRecovery(accountAddress1); vm.stopPrank(); - (uint256 _executeAfter, uint256 executeBefore) = - emailRecoveryModule.getRecoveryRequest(accountAddress1); - uint256 weight = emailRecoveryModule.getRecoveryDataHashWeight(accountAddress1, recoveryDataHash1); - bool hasGuardian1Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[0]); - bool hasGuardian2Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[1]); - bool hasGuardian3Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[2]); + ( + uint256 _executeAfter, + uint256 executeBefore, + uint256 currentWeight, + bytes32 recoveryDataHash + ) = emailRecoveryModule.getRecoveryRequest(accountAddress1); + bool hasGuardian1Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[0]); + bool hasGuardian2Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[1]); + bool hasGuardian3Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[2]); assertEq(_executeAfter, 0); assertEq(executeBefore, 0); - assertEq(weight, 0); + assertEq(currentWeight, 0); + assertEq(recoveryDataHash, bytes32(0)); assertEq(hasGuardian1Voted, false); assertEq(hasGuardian2Voted, false); assertEq(hasGuardian3Voted, false); @@ -343,24 +357,29 @@ contract EmailRecoveryManager_Integration_Test is emailRecoveryModule.cancelExpiredRecovery(accountAddress1); vm.stopPrank(); - (uint256 _executeAfter, uint256 executeBefore) = - emailRecoveryModule.getRecoveryRequest(accountAddress1); - uint256 weight = emailRecoveryModule.getRecoveryDataHashWeight(accountAddress1, recoveryDataHash1); - bool hasGuardian1Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[0]); - bool hasGuardian2Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[1]); - bool hasGuardian3Voted = emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[2]); + ( + uint256 _executeAfter, + uint256 executeBefore, + uint256 currentWeight, + bytes32 recoveryDataHash + ) = emailRecoveryModule.getRecoveryRequest(accountAddress1); + bool hasGuardian1Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[0]); + bool hasGuardian2Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[1]); + bool hasGuardian3Voted = + emailRecoveryModule.hasGuardianVoted(accountAddress1, guardians1[2]); assertEq(_executeAfter, 0); assertEq(executeBefore, 0); - assertEq(weight, 0); + assertEq(currentWeight, 0); + assertEq(recoveryDataHash, bytes32(0)); assertEq(hasGuardian1Voted, false); assertEq(hasGuardian2Voted, false); assertEq(hasGuardian3Voted, false); vm.expectRevert( abi.encodeWithSelector( - IEmailRecoveryManager.NotEnoughApprovals.selector, - weight, - threshold + IEmailRecoveryManager.NotEnoughApprovals.selector, currentWeight, threshold ) ); emailRecoveryModule.completeRecovery(accountAddress1, recoveryData1); diff --git a/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol b/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol index fedb80a..12ab0d2 100644 --- a/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol +++ b/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol @@ -296,6 +296,10 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test bytes memory newValidatorRecoveryData = abi.encode(validatorAddress, newChangeOwnerCalldata); bytes32 newValidatorRecoveryDataHash = keccak256(newValidatorRecoveryData); + vm.warp( + block.timestamp + emailRecoveryModule.CANCEL_EXPIRED_RECOVERY_COOLDOWN() + 1 seconds + ); + handleRecovery( accountAddress1, guardians1[0], newValidatorRecoveryDataHash, emailRecoveryModuleAddress ); @@ -329,6 +333,10 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test address updatedOwner1 = validator.owners(accountAddress1); assertEq(updatedOwner1, newOwner1); + vm.warp( + block.timestamp + emailRecoveryModule.CANCEL_EXPIRED_RECOVERY_COOLDOWN() + 1 seconds + ); + handleRecovery( accountAddress1, guardians1[0], validator2RecoveryDataHash, emailRecoveryModuleAddress ); @@ -365,6 +373,10 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test bytes memory validator2RecoveryData = abi.encode(validator2Address, newChangeOwnerCalldata); bytes32 validator2RecoveryDataHash = keccak256(validator2RecoveryData); + vm.warp( + block.timestamp + emailRecoveryModule.CANCEL_EXPIRED_RECOVERY_COOLDOWN() + 1 seconds + ); + handleRecovery( accountAddress1, guardians1[0], validator2RecoveryDataHash, emailRecoveryModuleAddress ); @@ -403,12 +415,14 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test vm.warp(block.timestamp + 12 seconds); EmailAuthMsg memory emailAuthMsg = getRecoveryEmailAuthMessage( - accountAddress1, guardians1[0], validator2RecoveryDataHash, emailRecoveryModuleAddress + accountAddress1, guardians1[1], validator2RecoveryDataHash, emailRecoveryModuleAddress ); vm.expectRevert( abi.encodeWithSelector( - IEmailRecoveryManager.GuardianAlreadyVoted.selector + IEmailRecoveryManager.InvalidRecoveryDataHash.selector, + validator2RecoveryDataHash, + recoveryDataHash1 ) ); // process recovery for validator 2 diff --git a/test/unit/EmailRecoveryManager/completeRecovery.t.sol b/test/unit/EmailRecoveryManager/completeRecovery.t.sol index 8054feb..5318d9f 100644 --- a/test/unit/EmailRecoveryManager/completeRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/completeRecovery.t.sol @@ -106,11 +106,12 @@ contract EmailRecoveryManager_completeRecovery_Test is UnitBase { vm.expectRevert( abi.encodeWithSelector( - IEmailRecoveryManager.NotEnoughApprovals.selector, - 0, - 3 + IEmailRecoveryManager.InvalidRecoveryDataHash.selector, + keccak256(invalidRecoveryData), + recoveryDataHash ) ); + emailRecoveryModule.completeRecovery(accountAddress1, invalidRecoveryData); } diff --git a/test/unit/EmailRecoveryManager/processRecovery.t.sol b/test/unit/EmailRecoveryManager/processRecovery.t.sol index f21ba04..a07c27e 100644 --- a/test/unit/EmailRecoveryManager/processRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/processRecovery.t.sol @@ -125,7 +125,30 @@ contract EmailRecoveryManager_processRecovery_Test is UnitBase { vm.expectRevert( abi.encodeWithSelector( - IEmailRecoveryManager.GuardianAlreadyVoted.selector + IEmailRecoveryManager.InvalidRecoveryDataHash.selector, + invalidRecoveryDataHash, + recoveryDataHash + ) + ); + emailRecoveryModule.exposed_processRecovery( + guardians1[1], templateIdx, commandParams, nullifier + ); + } + + function test_ProcessRecovery_RevertWhen_GuardianMustWaitForCooldown() public { + acceptGuardian(accountAddress1, guardians1[0], emailRecoveryModuleAddress); + acceptGuardian(accountAddress1, guardians1[1], emailRecoveryModuleAddress); + + emailRecoveryModule.exposed_processRecovery( + guardians1[0], templateIdx, commandParams, nullifier + ); + + vm.warp(block.timestamp + expiry); + emailRecoveryModule.cancelExpiredRecovery(accountAddress1); + + vm.expectRevert( + abi.encodeWithSelector( + IEmailRecoveryManager.GuardianMustWaitForCooldown.selector, guardians1[0] ) ); emailRecoveryModule.exposed_processRecovery( diff --git a/test/unit/assertErrorSelectors.t.sol b/test/unit/assertErrorSelectors.t.sol index 80eff57..cf21774 100644 --- a/test/unit/assertErrorSelectors.t.sol +++ b/test/unit/assertErrorSelectors.t.sol @@ -59,6 +59,8 @@ contract LogErrorSelectors_Test is Test { assertEq(IEmailRecoveryManager.RecoveryWindowTooShort.selector, bytes4(0x50799cce)); assertEq(IEmailRecoveryManager.ThresholdExceedsAcceptedWeight.selector, bytes4(0x7c3e983c)); assertEq(IEmailRecoveryManager.InvalidGuardianStatus.selector, bytes4(0x5689b51a)); + assertEq(IEmailRecoveryManager.GuardianAlreadyVoted.selector, bytes4(0xe11487a7)); + assertEq(IEmailRecoveryManager.GuardianMustWaitForCooldown.selector, bytes4(0x8035b5fd)); assertEq(IEmailRecoveryManager.InvalidAccountAddress.selector, bytes4(0x401b6ade)); assertEq(IEmailRecoveryManager.NoRecoveryConfigured.selector, bytes4(0xa66e66b6)); assertEq(IEmailRecoveryManager.NotEnoughApprovals.selector, bytes4(0x443282f5));