diff --git a/src/EmailRecoveryManager.sol b/src/EmailRecoveryManager.sol index 40564781..66433238 100644 --- a/src/EmailRecoveryManager.sol +++ b/src/EmailRecoveryManager.sol @@ -81,13 +81,6 @@ abstract contract EmailRecoveryManager is mapping(address account => PreviousRecoveryRequest previousRecoveryRequest) internal previousRecoveryRequests; - modifier onlyWhenActive() { - if (killSwitchEnabled) { - revert KillSwitchEnabled(); - } - _; - } - constructor( address _verifier, address _dkimRegistry, diff --git a/src/GuardianManager.sol b/src/GuardianManager.sol index 55a5e319..1eeafa0c 100644 --- a/src/GuardianManager.sol +++ b/src/GuardianManager.sol @@ -40,6 +40,18 @@ abstract contract GuardianManager is IGuardianManager { _; } + /** + * @notice Modifier to check if the kill switch has been enabled + * @dev This impacts EmailRecoveryManager & GuardianManager + */ + modifier onlyWhenActive() { + bool killSwitchEnabled = IEmailRecoveryManager(address(this)).killSwitchEnabled(); + if (killSwitchEnabled) { + revert KillSwitchEnabled(); + } + _; + } + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* GUARDIAN LOGIC */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ @@ -120,7 +132,14 @@ abstract contract GuardianManager is IGuardianManager { * @param guardian The address of the guardian to be added * @param weight The weight assigned to the guardian */ - function addGuardian(address guardian, uint256 weight) public onlyWhenNotRecovering { + function addGuardian( + address guardian, + uint256 weight + ) + public + onlyWhenNotRecovering + onlyWhenActive + { // Threshold can only be 0 at initialization. // Check ensures that setup function should be called first if (guardianConfigs[msg.sender].threshold == 0) { @@ -165,7 +184,7 @@ abstract contract GuardianManager is IGuardianManager { * no recovery is in process * @param guardian The address of the guardian to be removed */ - function removeGuardian(address guardian) external onlyWhenNotRecovering { + function removeGuardian(address guardian) external onlyWhenNotRecovering onlyWhenActive { GuardianConfig memory guardianConfig = guardianConfigs[msg.sender]; GuardianStorage memory guardianStorage = guardiansStorage[msg.sender].get(guardian); @@ -197,7 +216,7 @@ abstract contract GuardianManager is IGuardianManager { * only if no recovery is in process * @param threshold The new threshold for guardian approvals */ - function changeThreshold(uint256 threshold) external onlyWhenNotRecovering { + function changeThreshold(uint256 threshold) external onlyWhenNotRecovering onlyWhenActive { // Threshold can only be 0 at initialization. // Check ensures that setup function should be called first if (guardianConfigs[msg.sender].threshold == 0) { diff --git a/src/interfaces/IEmailRecoveryManager.sol b/src/interfaces/IEmailRecoveryManager.sol index ee715079..1da5cc26 100644 --- a/src/interfaces/IEmailRecoveryManager.sol +++ b/src/interfaces/IEmailRecoveryManager.sol @@ -91,7 +91,6 @@ interface IEmailRecoveryManager { /* ERRORS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - error KillSwitchEnabled(); error InvalidVerifier(); error InvalidDkimRegistry(); error InvalidEmailAuthImpl(); @@ -124,6 +123,8 @@ interface IEmailRecoveryManager { /* FUNCTIONS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + function killSwitchEnabled() external returns (bool); + function getRecoveryConfig(address account) external view returns (RecoveryConfig memory); function getRecoveryRequest(address account) diff --git a/src/interfaces/IGuardianManager.sol b/src/interfaces/IGuardianManager.sol index 2f6ac278..25a02ae5 100644 --- a/src/interfaces/IGuardianManager.sol +++ b/src/interfaces/IGuardianManager.sol @@ -28,6 +28,7 @@ interface IGuardianManager { event ChangedThreshold(address indexed account, uint256 threshold); error RecoveryInProcess(); + error KillSwitchEnabled(); error IncorrectNumberOfWeights(uint256 guardianCount, uint256 weightCount); error ThresholdCannotBeZero(); error InvalidGuardianAddress(address guardian); diff --git a/test/integration/OwnableValidatorRecovery/EmailRecoveryModule/EmailRecoveryModule.t.sol b/test/integration/OwnableValidatorRecovery/EmailRecoveryModule/EmailRecoveryModule.t.sol index 8097f1b8..b41fd66b 100644 --- a/test/integration/OwnableValidatorRecovery/EmailRecoveryModule/EmailRecoveryModule.t.sol +++ b/test/integration/OwnableValidatorRecovery/EmailRecoveryModule/EmailRecoveryModule.t.sol @@ -352,7 +352,7 @@ contract OwnableValidatorRecovery_EmailRecoveryModule_Integration_Test is executeRecoveryFlowForAccount(accountAddress1, guardians1, recoveryDataHash1, recoveryData1); // new module should not be useable - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); EmailAccountRecovery(newRecoveryModuleAddress).completeRecovery( accountAddress1, abi.encodePacked("1") ); @@ -386,7 +386,7 @@ contract OwnableValidatorRecovery_EmailRecoveryModule_Integration_Test is instance1.uninstallModule(MODULE_TYPE_EXECUTOR, emailRecoveryModuleAddress, ""); // the second module installation should fail after the kill switch is enabled. - instance1.expect4337Revert(IEmailRecoveryManager.KillSwitchEnabled.selector); + instance1.expect4337Revert(IGuardianManager.KillSwitchEnabled.selector); instance1.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: emailRecoveryModuleAddress, diff --git a/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol b/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol index 3297894a..cd22a70d 100644 --- a/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol +++ b/test/integration/OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol @@ -486,7 +486,7 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test string memory currentAccountType = vm.envOr("ACCOUNT_TYPE", string("")); if (bytes(currentAccountType).length != 0) { vm.skip(true); - } + } executeRecoveryFlowForAccount(accountAddress1, guardians1, recoveryDataHash1, recoveryData1); address updatedOwner1 = validator.owners(accountAddress1); assertEq(updatedOwner1, newOwner1); @@ -497,7 +497,7 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test IEmailRecoveryManager(emailRecoveryModuleAddress).toggleKillSwitch(); vm.stopPrank(); - instance1.expect4337Revert(IEmailRecoveryManager.KillSwitchEnabled.selector); + instance1.expect4337Revert(IGuardianManager.KillSwitchEnabled.selector); instance1.installModule({ moduleTypeId: MODULE_TYPE_EXECUTOR, module: emailRecoveryModuleAddress, diff --git a/test/unit/EmailRecoveryManager/acceptGuardian.t.sol b/test/unit/EmailRecoveryManager/acceptGuardian.t.sol index 55725c28..e1808ec2 100644 --- a/test/unit/EmailRecoveryManager/acceptGuardian.t.sol +++ b/test/unit/EmailRecoveryManager/acceptGuardian.t.sol @@ -38,7 +38,7 @@ contract EmailRecoveryManager_acceptGuardian_Test is UnitBase { emailRecoveryModule.toggleKillSwitch(); vm.stopPrank(); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.exposed_acceptGuardian( guardians1[0], templateIdx, commandParams, nullifier ); diff --git a/test/unit/EmailRecoveryManager/cancelExpiredRecovery.t.sol b/test/unit/EmailRecoveryManager/cancelExpiredRecovery.t.sol index f4149908..1d8063e9 100644 --- a/test/unit/EmailRecoveryManager/cancelExpiredRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/cancelExpiredRecovery.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.25; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { UnitBase } from "../UnitBase.t.sol"; +import { IGuardianManager } from "src/interfaces/IGuardianManager.sol"; import { IEmailRecoveryManager } from "src/interfaces/IEmailRecoveryManager.sol"; contract EmailRecoveryManager_cancelExpiredRecovery_Test is UnitBase { @@ -17,7 +18,7 @@ contract EmailRecoveryManager_cancelExpiredRecovery_Test is UnitBase { emailRecoveryModule.toggleKillSwitch(); vm.stopPrank(); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.cancelExpiredRecovery(accountAddress1); } diff --git a/test/unit/EmailRecoveryManager/cancelRecovery.t.sol b/test/unit/EmailRecoveryManager/cancelRecovery.t.sol index 3a03dd1c..a5f3dd40 100644 --- a/test/unit/EmailRecoveryManager/cancelRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/cancelRecovery.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.25; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { UnitBase } from "../UnitBase.t.sol"; +import { IGuardianManager } from "src/interfaces/IGuardianManager.sol"; import { IEmailRecoveryManager } from "src/interfaces/IEmailRecoveryManager.sol"; contract EmailRecoveryManager_cancelRecovery_Test is UnitBase { @@ -17,7 +18,7 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase { emailRecoveryModule.toggleKillSwitch(); vm.stopPrank(); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.cancelRecovery(); } diff --git a/test/unit/EmailRecoveryManager/completeRecovery.t.sol b/test/unit/EmailRecoveryManager/completeRecovery.t.sol index 1df67954..e18d98df 100644 --- a/test/unit/EmailRecoveryManager/completeRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/completeRecovery.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.25; import { UnitBase } from "../UnitBase.t.sol"; +import { IGuardianManager } from "src/interfaces/IGuardianManager.sol"; import { IEmailRecoveryManager } from "src/interfaces/IEmailRecoveryManager.sol"; contract EmailRecoveryManager_completeRecovery_Test is UnitBase { @@ -14,7 +15,7 @@ contract EmailRecoveryManager_completeRecovery_Test is UnitBase { emailRecoveryModule.toggleKillSwitch(); vm.stopPrank(); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.completeRecovery(accountAddress1, recoveryData); } diff --git a/test/unit/EmailRecoveryManager/configureRecovery.t.sol b/test/unit/EmailRecoveryManager/configureRecovery.t.sol index eabcfda4..0990fad2 100644 --- a/test/unit/EmailRecoveryManager/configureRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/configureRecovery.t.sol @@ -21,7 +21,7 @@ contract EmailRecoveryManager_configureRecovery_Test is UnitBase { vm.stopPrank(); vm.startPrank(accountAddress1); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.exposed_configureRecovery( guardians1, guardianWeights, threshold, delay, expiry ); diff --git a/test/unit/EmailRecoveryManager/processRecovery.t.sol b/test/unit/EmailRecoveryManager/processRecovery.t.sol index 73f2d4c5..b66dec48 100644 --- a/test/unit/EmailRecoveryManager/processRecovery.t.sol +++ b/test/unit/EmailRecoveryManager/processRecovery.t.sol @@ -9,6 +9,7 @@ import { CommandHandlerType } from "../../Base.t.sol"; import { IEmailRecoveryManager } from "src/interfaces/IEmailRecoveryManager.sol"; import { IEmailRecoveryCommandHandler } from "src/interfaces/IEmailRecoveryCommandHandler.sol"; import { GuardianStatus } from "src/libraries/EnumerableGuardianMap.sol"; +import { IGuardianManager } from "src/interfaces/IGuardianManager.sol"; contract EmailRecoveryManager_processRecovery_Test is UnitBase { using ModuleKitHelpers for *; @@ -50,7 +51,7 @@ contract EmailRecoveryManager_processRecovery_Test is UnitBase { emailRecoveryModule.toggleKillSwitch(); vm.stopPrank(); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.exposed_processRecovery( guardians1[0], templateIdx, commandParams, nullifier ); diff --git a/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol b/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol index 43f7a56c..de7030e2 100644 --- a/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol +++ b/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol @@ -24,7 +24,7 @@ contract EmailRecoveryManager_updateRecoveryConfig_Test is UnitBase { vm.stopPrank(); vm.startPrank(accountAddress1); - vm.expectRevert(IEmailRecoveryManager.KillSwitchEnabled.selector); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); emailRecoveryModule.updateRecoveryConfig(recoveryConfig); } diff --git a/test/unit/GuardianManager/addGuardian.t.sol b/test/unit/GuardianManager/addGuardian.t.sol index 82e9f91d..de3d1aaf 100644 --- a/test/unit/GuardianManager/addGuardian.t.sol +++ b/test/unit/GuardianManager/addGuardian.t.sol @@ -14,6 +14,16 @@ contract GuardianManager_addGuardian_Test is UnitBase { super.setUp(); } + function test_AddGuardian_RevertWhen_KillSwitchEnabled() public { + vm.prank(killSwitchAuthorizer); + emailRecoveryModule.toggleKillSwitch(); + vm.stopPrank(); + + vm.startPrank(accountAddress1); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); + emailRecoveryModule.addGuardian(guardians1[0], guardianWeights[0]); + } + function test_AddGuardian_RevertWhen_AlreadyRecovering() public { acceptGuardian(accountAddress1, guardians1[0], emailRecoveryModuleAddress); acceptGuardian(accountAddress1, guardians1[1], emailRecoveryModuleAddress); diff --git a/test/unit/GuardianManager/changeThreshold.t.sol b/test/unit/GuardianManager/changeThreshold.t.sol index 7daeb64c..f5f19038 100644 --- a/test/unit/GuardianManager/changeThreshold.t.sol +++ b/test/unit/GuardianManager/changeThreshold.t.sol @@ -9,6 +9,16 @@ contract GuardianManager_changeThreshold_Test is UnitBase { super.setUp(); } + function test_RevertWhen_KillSwitchEnabled() public { + vm.prank(killSwitchAuthorizer); + emailRecoveryModule.toggleKillSwitch(); + vm.stopPrank(); + + vm.startPrank(accountAddress1); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); + emailRecoveryModule.changeThreshold(threshold); + } + function test_RevertWhen_AlreadyRecovering() public { acceptGuardian(accountAddress1, guardians1[0], emailRecoveryModuleAddress); acceptGuardian(accountAddress1, guardians1[1], emailRecoveryModuleAddress); diff --git a/test/unit/GuardianManager/removeGuardian.t.sol b/test/unit/GuardianManager/removeGuardian.t.sol index ae520efc..7e26f320 100644 --- a/test/unit/GuardianManager/removeGuardian.t.sol +++ b/test/unit/GuardianManager/removeGuardian.t.sol @@ -10,6 +10,18 @@ contract GuardianManager_removeGuardian_Test is UnitBase { super.setUp(); } + function test_RemoveGuardian_RevertWhen_KillSwitchEnabled() public { + address guardian = guardians1[0]; + + vm.prank(killSwitchAuthorizer); + emailRecoveryModule.toggleKillSwitch(); + vm.stopPrank(); + + vm.startPrank(accountAddress1); + vm.expectRevert(IGuardianManager.KillSwitchEnabled.selector); + emailRecoveryModule.removeGuardian(guardian); + } + function test_RemoveGuardian_RevertWhen_AlreadyRecovering() public { address guardian = guardians1[0]; diff --git a/test/unit/assertErrorSelectors.t.sol b/test/unit/assertErrorSelectors.t.sol index 7a5e8a56..5c3a1a56 100644 --- a/test/unit/assertErrorSelectors.t.sol +++ b/test/unit/assertErrorSelectors.t.sol @@ -48,7 +48,6 @@ contract LogErrorSelectors_Test is Test { } function test_IEmailRecoveryManager_AssertSelectors() public pure { - assertEq(IEmailRecoveryManager.KillSwitchEnabled.selector, bytes4(0xec7fb2a0)); assertEq(IEmailRecoveryManager.InvalidVerifier.selector, bytes4(0xbaa3de5f)); assertEq(IEmailRecoveryManager.InvalidDkimRegistry.selector, bytes4(0x260ce05b)); assertEq(IEmailRecoveryManager.InvalidEmailAuthImpl.selector, bytes4(0xe98100fb)); @@ -117,6 +116,7 @@ contract LogErrorSelectors_Test is Test { function test_IGuardianManager_AssertSelectors() public pure { assertEq(IGuardianManager.RecoveryInProcess.selector, bytes4(0xf90ea6fc)); + assertEq(IGuardianManager.KillSwitchEnabled.selector, bytes4(0xec7fb2a0)); assertEq(IGuardianManager.IncorrectNumberOfWeights.selector, bytes4(0x166e79bd)); assertEq(IGuardianManager.ThresholdCannotBeZero.selector, bytes4(0xf4124166)); assertEq(IGuardianManager.InvalidGuardianAddress.selector, bytes4(0x1af74975));