diff --git a/script/DeploySafeRecovery.s.sol b/script/DeploySafeRecovery.s.sol index 2177ebc..caffa30 100644 --- a/script/DeploySafeRecovery.s.sol +++ b/script/DeploySafeRecovery.s.sol @@ -8,8 +8,6 @@ import { EmailRecoveryModule } from "src/modules/EmailRecoveryModule.sol"; contract DeploySafeRecoveryScript is Script { function run() public { - bytes32 salt = bytes32(uint256(0)); - address verifier = 0xEdC642bbaD91E21cCE6cd436Fdc6F040FD0fF998; address dkimRegistry = 0xC83256CCf7B94d310e49edA05077899ca036eb78; address emailAuthImpl = 0x1C76Aa365c17B40c7E944DcCdE4dC6e6D2A7b748; diff --git a/src/handlers/EmailRecoverySubjectHandler.sol b/src/handlers/EmailRecoverySubjectHandler.sol index 235fbe6..9198536 100644 --- a/src/handlers/EmailRecoverySubjectHandler.sol +++ b/src/handlers/EmailRecoverySubjectHandler.sol @@ -46,7 +46,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler { function extractRecoveredAccountFromAcceptanceSubject( bytes[] memory subjectParams, - uint256 templateIdx + uint256 /* templateIdx */ ) public pure @@ -57,7 +57,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler { function extractRecoveredAccountFromRecoverySubject( bytes[] memory subjectParams, - uint256 templateIdx + uint256 /* templateIdx */ ) public pure @@ -67,7 +67,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler { } function validateAcceptanceSubject( - uint256 templateIdx, + uint256, /* templateIdx */ bytes[] calldata subjectParams ) external @@ -84,7 +84,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler { } function validateRecoverySubject( - uint256 templateIdx, + uint256, /* templateIdx */ bytes[] calldata subjectParams, address recoveryManager ) diff --git a/src/handlers/SafeRecoverySubjectHandler.sol b/src/handlers/SafeRecoverySubjectHandler.sol index 1db22d1..81940ce 100644 --- a/src/handlers/SafeRecoverySubjectHandler.sol +++ b/src/handlers/SafeRecoverySubjectHandler.sol @@ -55,10 +55,10 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler { function extractRecoveredAccountFromAcceptanceSubject( bytes[] memory subjectParams, - uint256 templateIdx + uint256 /* templateIdx */ ) public - view + pure returns (address) { return abi.decode(subjectParams[0], (address)); @@ -66,17 +66,17 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler { function extractRecoveredAccountFromRecoverySubject( bytes[] memory subjectParams, - uint256 templateIdx + uint256 /* templateIdx */ ) public - view + pure returns (address) { return abi.decode(subjectParams[0], (address)); } function validateAcceptanceSubject( - uint256 templateIdx, + uint256, /* templateIdx */ bytes[] calldata subjectParams ) external @@ -93,7 +93,7 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler { } function validateRecoverySubject( - uint256 templateIdx, + uint256, /* templateIdx */ bytes[] calldata subjectParams, address recoveryManager ) diff --git a/test/integration/OwnableValidatorRecovery/OwnableValidatorRecoveryBase.t.sol b/test/integration/OwnableValidatorRecovery/OwnableValidatorRecoveryBase.t.sol index 18702a9..c5164da 100644 --- a/test/integration/OwnableValidatorRecovery/OwnableValidatorRecoveryBase.t.sol +++ b/test/integration/OwnableValidatorRecovery/OwnableValidatorRecoveryBase.t.sol @@ -169,13 +169,13 @@ abstract contract OwnableValidatorRecoveryBase is IntegrationBase { function handleRecovery( address recoveryModule, - bytes32 calldataHash, + bytes32 recoveryCalldataHash, bytes32 accountSalt ) public { string memory accountString = SubjectUtils.addressToChecksumHexString(accountAddress); - string memory calldataHashString = uint256(calldataHash).toHexString(32); + string memory calldataHashString = uint256(recoveryCalldataHash).toHexString(32); string memory recoveryModuleString = SubjectUtils.addressToChecksumHexString(recoveryModule); string memory subjectPart1 = string.concat("Recover account ", accountString); diff --git a/test/unit/EmailRecoveryFactory/deployModuleAndManager.t.sol b/test/unit/EmailRecoveryFactory/deployModuleAndManager.t.sol index 7cb82cf..444ea39 100644 --- a/test/unit/EmailRecoveryFactory/deployModuleAndManager.t.sol +++ b/test/unit/EmailRecoveryFactory/deployModuleAndManager.t.sol @@ -9,5 +9,7 @@ contract EmailRecoveryManager_deployModuleAndManager_Test is UnitBase { super.setUp(); } - function test_DeployModuleAndManager_Succeeds() public { } + function test_DeployModuleAndManager_Succeeds() public { + // TODO: test + } } diff --git a/test/unit/EmailRecoveryManager/constructor.t.sol b/test/unit/EmailRecoveryManager/constructor.t.sol index 85f145e..adaf886 100644 --- a/test/unit/EmailRecoveryManager/constructor.t.sol +++ b/test/unit/EmailRecoveryManager/constructor.t.sol @@ -14,7 +14,7 @@ contract EmailRecoveryManager_constructor_Test is UnitBase { function test_Constructor_RevertWhen_InvalidSubjectHandler() public { address invalidHandler = address(0); vm.expectRevert(IEmailRecoveryManager.InvalidSubjectHandler.selector); - EmailRecoveryManager emailRecoveryManager = new EmailRecoveryManager( + new EmailRecoveryManager( address(verifier), address(dkimRegistry), address(emailAuthImpl), invalidHandler ); } diff --git a/test/unit/EmailRecoveryManager/getGuardian.t.sol b/test/unit/EmailRecoveryManager/getGuardian.t.sol index 128017b..caf8704 100644 --- a/test/unit/EmailRecoveryManager/getGuardian.t.sol +++ b/test/unit/EmailRecoveryManager/getGuardian.t.sol @@ -12,5 +12,7 @@ contract EmailRecoveryManager_getGuardian_Test is UnitBase { super.setUp(); } - function test_GetGuardian_Succeeds() public { } + function test_GetGuardian_Succeeds() public { + // TODO: test + } } diff --git a/test/unit/EmailRecoveryManager/getGuardianConfig.t.sol b/test/unit/EmailRecoveryManager/getGuardianConfig.t.sol index aa1bd6b..29c6a75 100644 --- a/test/unit/EmailRecoveryManager/getGuardianConfig.t.sol +++ b/test/unit/EmailRecoveryManager/getGuardianConfig.t.sol @@ -12,5 +12,7 @@ contract EmailRecoveryManager_getGuardianConfig_Test is UnitBase { super.setUp(); } - function test_GetGuardianConfig_Succeeds() public { } + function test_GetGuardianConfig_Succeeds() public { + // TODO: test + } } diff --git a/test/unit/EmailRecoveryManager/getRecoveryConfig.t.sol b/test/unit/EmailRecoveryManager/getRecoveryConfig.t.sol index 7fa1ecd..e39cdd5 100644 --- a/test/unit/EmailRecoveryManager/getRecoveryConfig.t.sol +++ b/test/unit/EmailRecoveryManager/getRecoveryConfig.t.sol @@ -12,5 +12,7 @@ contract EmailRecoveryManager_getRecoveryConfig_Test is UnitBase { super.setUp(); } - function test_GetRecoveryConfig_Succeeds() public { } + function test_GetRecoveryConfig_Succeeds() public { + // TODO: test + } } diff --git a/test/unit/EmailRecoveryManager/getRecoveryRequest.t.sol b/test/unit/EmailRecoveryManager/getRecoveryRequest.t.sol index 8b7e44b..e03ae04 100644 --- a/test/unit/EmailRecoveryManager/getRecoveryRequest.t.sol +++ b/test/unit/EmailRecoveryManager/getRecoveryRequest.t.sol @@ -12,5 +12,7 @@ contract EmailRecoveryManager_getRecoveryRequest_Test is UnitBase { super.setUp(); } - function test_GetRecoveryRequest_Succeeds() public { } + function test_GetRecoveryRequest_Succeeds() public { + // TODO: test + } } diff --git a/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol b/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol index 2440a0e..00e6276 100644 --- a/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol +++ b/test/unit/EmailRecoveryManager/updateRecoveryConfig.t.sol @@ -88,7 +88,6 @@ contract EmailRecoveryManager_updateRecoveryConfig_Test is UnitBase { } function test_UpdateRecoveryConfig_Succeeds() public { - address newRecoveryModule = recoveryModuleAddress; uint256 newDelay = 1 days; uint256 newExpiry = 4 weeks; diff --git a/test/unit/EmailRecoveryModuleHarness.sol b/test/unit/EmailRecoveryModuleHarness.sol index a6bd300..063e35a 100644 --- a/test/unit/EmailRecoveryModuleHarness.sol +++ b/test/unit/EmailRecoveryModuleHarness.sol @@ -15,6 +15,7 @@ contract EmailRecoveryModuleHarness is EmailRecoveryModule { address account ) external + view returns (bytes4) { return allowedSelectors[validator][account]; @@ -25,6 +26,7 @@ contract EmailRecoveryModuleHarness is EmailRecoveryModule { address account ) external + view returns (address) { return selectorToValidator[selector][account]; diff --git a/test/unit/SafeRecoverySubjectHandlerHarness.sol b/test/unit/SafeRecoverySubjectHandlerHarness.sol index a7177ec..bcc90db 100644 --- a/test/unit/SafeRecoverySubjectHandlerHarness.sol +++ b/test/unit/SafeRecoverySubjectHandlerHarness.sol @@ -12,6 +12,7 @@ contract SafeRecoverySubjectHandlerHarness is SafeRecoverySubjectHandler { address oldOwner ) external + view returns (address) { return getPreviousOwnerInLinkedList(safe, oldOwner); diff --git a/test/unit/SafeUnitBase.t.sol b/test/unit/SafeUnitBase.t.sol index a8ff776..fc12b88 100644 --- a/test/unit/SafeUnitBase.t.sol +++ b/test/unit/SafeUnitBase.t.sol @@ -23,6 +23,7 @@ import { MockValidator } from "module-bases/mocks/MockValidator.sol"; import { EmailAuthMsg, EmailProof } from "ether-email-auth/packages/contracts/src/EmailAuth.sol"; import { SubjectUtils } from "ether-email-auth/packages/contracts/src/libraries/SubjectUtils.sol"; import { Solarray } from "solarray/Solarray.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { EmailRecoveryManagerHarness } from "./EmailRecoveryManagerHarness.sol"; import { EmailRecoveryManager } from "src/EmailRecoveryManager.sol"; @@ -33,6 +34,8 @@ import { MockRegistry } from "../integration/external/MockRegistry.sol"; import { IntegrationBase } from "../integration/IntegrationBase.t.sol"; abstract contract SafeUnitBase is IntegrationBase { + using Strings for uint256; + EmailRecoveryFactory emailRecoveryFactory; SafeRecoverySubjectHandlerHarness safeRecoverySubjectHandler; EmailRecoveryManager emailRecoveryManager; @@ -276,27 +279,19 @@ abstract contract SafeUnitBase is IntegrationBase { function handleRecovery( address recoveryModule, - bytes32 calldataHash, + bytes32 recoveryCalldataHash, bytes32 accountSalt ) public { - // Uncomment if getting "invalid subject" errors. Sometimes the subject needs updating - // after - // certain changes - // console2.log("accountAddress: ", accountAddress); - console2.log("recoveryModule: ", recoveryModule); - console2.log("calldataHash:"); - console2.logBytes32(calldataHash); - - // TODO: Ideally do this dynamically - string memory calldataHashString = - "0x4e66542ab78fcc7a2341586b67800e82b975078517d7d692e2aa98d2696c51d0"; - - string memory subject = string.concat( - "Recover account 0xE760ccaE42b4EA7a93A4CfA75BC649aaE1033095 via recovery module 0xD7F74A3A1d35495c1537f5377590e44A2bf44122 using recovery hash ", - calldataHashString - ); + string memory accountString = SubjectUtils.addressToChecksumHexString(accountAddress); + string memory calldataHashString = uint256(recoveryCalldataHash).toHexString(32); + string memory recoveryModuleString = SubjectUtils.addressToChecksumHexString(recoveryModule); + + string memory subjectPart1 = string.concat("Recover account ", accountString); + string memory subjectPart2 = string.concat(" via recovery module ", recoveryModuleString); + string memory subjectPart3 = string.concat(" using recovery hash ", calldataHashString); + string memory subject = string.concat(subjectPart1, subjectPart2, subjectPart3); bytes32 nullifier = keccak256(abi.encode("nullifier 2")); uint256 templateIdx = 0; diff --git a/test/unit/UnitBase.t.sol b/test/unit/UnitBase.t.sol index c4fdf6c..8558180 100644 --- a/test/unit/UnitBase.t.sol +++ b/test/unit/UnitBase.t.sol @@ -263,13 +263,13 @@ abstract contract UnitBase is RhinestoneModuleKit, Test { function handleRecovery( address recoveryModule, - bytes32 calldataHash, + bytes32 recoveryCalldataHash, bytes32 accountSalt ) public { string memory accountString = SubjectUtils.addressToChecksumHexString(accountAddress); - string memory calldataHashString = uint256(calldataHash).toHexString(32); + string memory calldataHashString = uint256(recoveryCalldataHash).toHexString(32); string memory recoveryModuleString = SubjectUtils.addressToChecksumHexString(recoveryModule); string memory subjectPart1 = string.concat("Recover account ", accountString); diff --git a/test/unit/handlers/EmailRecoverySubjectHandler/validateRecoverySubject.t.sol b/test/unit/handlers/EmailRecoverySubjectHandler/validateRecoverySubject.t.sol index 283f655..8625f99 100644 --- a/test/unit/handlers/EmailRecoverySubjectHandler/validateRecoverySubject.t.sol +++ b/test/unit/handlers/EmailRecoverySubjectHandler/validateRecoverySubject.t.sol @@ -74,10 +74,11 @@ contract EmailRecoverySubjectHandler_validateRecoverySubject_Test is UnitBase { ); } - function test_ValidateRecoverySubject_Succeeds() public { + function test_ValidateRecoverySubject_Succeeds() public view { (address account, string memory calldataHash) = emailRecoveryHandler.validateRecoverySubject( templateIdx, subjectParams, emailRecoveryManagerAddress ); assertEq(account, accountAddress); + assertEq(calldataHashString, calldataHash); } } diff --git a/test/unit/handlers/SafeRecoverySubjectHandler/getPreviousOwnerInLinkedList.t.sol b/test/unit/handlers/SafeRecoverySubjectHandler/getPreviousOwnerInLinkedList.t.sol index 44f41df..392719d 100644 --- a/test/unit/handlers/SafeRecoverySubjectHandler/getPreviousOwnerInLinkedList.t.sol +++ b/test/unit/handlers/SafeRecoverySubjectHandler/getPreviousOwnerInLinkedList.t.sol @@ -11,7 +11,7 @@ contract SafeRecoverySubjectHandler_getPreviousOwnerInLinkedList_Test is SafeUni super.setUp(); } - function test_GetPreviousOwnerInLinkedList_InvalidOwner_ReturnsSentinel() public { + function test_GetPreviousOwnerInLinkedList_InvalidOwner_ReturnsSentinel() public view { address invalidOwner = address(0); address previousOwner = safeRecoverySubjectHandler.exposed_getPreviousOwnerInLinkedList( @@ -21,7 +21,7 @@ contract SafeRecoverySubjectHandler_getPreviousOwnerInLinkedList_Test is SafeUni assertEq(previousOwner, SENTINEL_OWNERS); } - function test_GetPreviousOwnerInLinkedList_OwnerIsSentinel_ReturnsSentinel() public { + function test_GetPreviousOwnerInLinkedList_OwnerIsSentinel_ReturnsSentinel() public view { address invalidOwner = SENTINEL_OWNERS; address previousOwner = safeRecoverySubjectHandler.exposed_getPreviousOwnerInLinkedList( @@ -38,7 +38,7 @@ contract SafeRecoverySubjectHandler_getPreviousOwnerInLinkedList_Test is SafeUni safeRecoverySubjectHandler.exposed_getPreviousOwnerInLinkedList(invalidAccount, owner); } - function test_GetPreviousOwnerInLinkedList_Succeeds() public { + function test_GetPreviousOwnerInLinkedList_Succeeds() public view { address expectedPreviousOwner = address(1); address previousOwner = safeRecoverySubjectHandler.exposed_getPreviousOwnerInLinkedList(accountAddress, owner); @@ -46,5 +46,7 @@ contract SafeRecoverySubjectHandler_getPreviousOwnerInLinkedList_Test is SafeUni assertEq(expectedPreviousOwner, previousOwner); } - function test_GetPreviousOwnerInLinkedList_SucceedsWithMultipleAccounts() public { } + function test_GetPreviousOwnerInLinkedList_SucceedsWithMultipleAccounts() public { + // TODO: test + } } diff --git a/test/unit/handlers/SafeRecoverySubjectHandler/validateRecoverySubject.t.sol b/test/unit/handlers/SafeRecoverySubjectHandler/validateRecoverySubject.t.sol index 185ad59..f4d976c 100644 --- a/test/unit/handlers/SafeRecoverySubjectHandler/validateRecoverySubject.t.sol +++ b/test/unit/handlers/SafeRecoverySubjectHandler/validateRecoverySubject.t.sol @@ -84,7 +84,7 @@ contract SafeRecoverySubjectHandler_validateRecoverySubject_Test is SafeUnitBase ); } - function test_ValidateRecoverySubject_Succeeds() public { + function test_ValidateRecoverySubject_Succeeds() public view { (address account, string memory calldataHash) = safeRecoverySubjectHandler .validateRecoverySubject(templateIdx, subjectParams, emailRecoveryManagerAddress); assertEq(account, accountAddress); diff --git a/test/unit/libraries/EnumerableGuardianMap/get.t.sol b/test/unit/libraries/EnumerableGuardianMap/get.t.sol index 6a780a7..19b20ef 100644 --- a/test/unit/libraries/EnumerableGuardianMap/get.t.sol +++ b/test/unit/libraries/EnumerableGuardianMap/get.t.sol @@ -9,6 +9,10 @@ contract EnumerableGuardianMap_get_Test is UnitBase { super.setUp(); } - function test_Get_GetsExistingValue() public view { } - function test_Get_GetsNonExistentValue() public view { } + function test_Get_GetsExistingValue() public view { + // TODO: test + } + function test_Get_GetsNonExistentValue() public view { + // TODO: test + } } diff --git a/test/unit/libraries/EnumerableGuardianMap/keys.t.sol b/test/unit/libraries/EnumerableGuardianMap/keys.t.sol index b736c1d..85d6427 100644 --- a/test/unit/libraries/EnumerableGuardianMap/keys.t.sol +++ b/test/unit/libraries/EnumerableGuardianMap/keys.t.sol @@ -9,8 +9,16 @@ contract EnumerableGuardianMap_keys_Test is UnitBase { super.setUp(); } - function test_Keys_StartsEmpty() public view { } - function test_Keys_ReturnsEmptyArrayOfKeys() public view { } - function test_Keys_ReturnsArrayOfKeys() public view { } - function test_Keys_ReturnMaxArrayOfKeys() public view { } + function test_Keys_StartsEmpty() public view { + // TODO: test + } + function test_Keys_ReturnsEmptyArrayOfKeys() public view { + // TODO: test + } + function test_Keys_ReturnsArrayOfKeys() public view { + // TODO: test + } + function test_Keys_ReturnMaxArrayOfKeys() public view { + // TODO: test + } } diff --git a/test/unit/libraries/EnumerableGuardianMap/remove.t.sol b/test/unit/libraries/EnumerableGuardianMap/remove.t.sol index 6eb3ed9..42a124e 100644 --- a/test/unit/libraries/EnumerableGuardianMap/remove.t.sol +++ b/test/unit/libraries/EnumerableGuardianMap/remove.t.sol @@ -9,6 +9,10 @@ contract EnumerableGuardianMap_remove_Test is UnitBase { super.setUp(); } - function test_Remove_ReturnsFalseWhenRemovingKeysNotInTheSet() public view { } - function test_Remove_RemovesAddedKeys() public view { } + function test_Remove_ReturnsFalseWhenRemovingKeysNotInTheSet() public view { + // TODO: test + } + function test_Remove_RemovesAddedKeys() public view { + // TODO: test + } } diff --git a/test/unit/libraries/EnumerableGuardianMap/removeAll.t.sol b/test/unit/libraries/EnumerableGuardianMap/removeAll.t.sol index 978306d..979ac35 100644 --- a/test/unit/libraries/EnumerableGuardianMap/removeAll.t.sol +++ b/test/unit/libraries/EnumerableGuardianMap/removeAll.t.sol @@ -9,7 +9,14 @@ contract EnumerableGuardianMap_removeAll_Test is UnitBase { super.setUp(); } - function test_RemoveAll_RevertWhen_TooManyValuesToRemove() public view { } - function test_RemoveAll_Succeeds() public view { } - function test_RemoveAll_RemovesMaxNumberOfValues() public view { } + function test_RemoveAll_RevertWhen_TooManyValuesToRemove() public view { + // TODO: test + } + + function test_RemoveAll_Succeeds() public view { + // TODO: test + } + function test_RemoveAll_RemovesMaxNumberOfValues() public view { + // TODO: test + } } diff --git a/test/unit/modules/EmailRecoveryModule/getAllowedSelectors.t.sol b/test/unit/modules/EmailRecoveryModule/getAllowedSelectors.t.sol index 3f68048..5f3e68a 100644 --- a/test/unit/modules/EmailRecoveryModule/getAllowedSelectors.t.sol +++ b/test/unit/modules/EmailRecoveryModule/getAllowedSelectors.t.sol @@ -14,7 +14,7 @@ contract EmailRecoveryModule_getAllowedSelectors_Test is UnitBase { super.setUp(); } - function test_GetAllowedSelectors_Succeeds() public { + function test_GetAllowedSelectors_Succeeds() public view { bytes4[] memory allowedSelectors = emailRecoveryModule.getAllowedSelectors(accountAddress); assertEq(allowedSelectors.length, 1); diff --git a/test/unit/modules/EmailRecoveryModule/getAllowedValidators.t.sol b/test/unit/modules/EmailRecoveryModule/getAllowedValidators.t.sol index 547dd3e..b44dbeb 100644 --- a/test/unit/modules/EmailRecoveryModule/getAllowedValidators.t.sol +++ b/test/unit/modules/EmailRecoveryModule/getAllowedValidators.t.sol @@ -32,7 +32,7 @@ contract EmailRecoveryModule_getAllowedValidators_Test is UnitBase { assertEq(allowedValidators.length, 0); } - function test_GetAllowedValidators_SucceedsWithOneValidator() public { + function test_GetAllowedValidators_SucceedsWithOneValidator() public view { address[] memory allowedValidators = emailRecoveryModule.getAllowedValidators(accountAddress);