From 9ddf4e5ccd6440759a2eef9dfd9c98989f950e36 Mon Sep 17 00:00:00 2001 From: wshino Date: Mon, 7 Oct 2024 16:43:36 +0900 Subject: [PATCH 1/5] Make test cases as a function named files --- .../revokeDKIMPublicKeyHash.t.sol | 168 ++++++++++++++++++ .../setDKIMPublicKeyHash.t.sol} | 151 +++++++++++----- 2 files changed, 273 insertions(+), 46 deletions(-) create mode 100644 packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol rename packages/contracts/test/utils/{ECDSAOwnedDKIMRegistry.t.sol => ECDSAOwnedDKIMRegistry/setDKIMPublicKeyHash.t.sol} (67%) diff --git a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol new file mode 100644 index 00000000..3a47a60d --- /dev/null +++ b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "forge-std/console.sol"; +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import "../../../src/utils/ECDSAOwnedDKIMRegistry.sol"; +import "@openzeppelin/contracts/utils/Strings.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { + ECDSAOwnedDKIMRegistry dkim; + using console for *; + using ECDSA for *; + using Strings for *; + + string public selector = "12345"; + string public domainName = "example.com"; + // uint public signValidityDuration = 1 days; + bytes32 public publicKeyHash = bytes32(uint256(1)); + + function setUp() public { + address signer = vm.addr(1); + { + ECDSAOwnedDKIMRegistry dkimImpl = new ECDSAOwnedDKIMRegistry(); + ERC1967Proxy dkimProxy = new ERC1967Proxy( + address(dkimImpl), + abi.encodeCall(dkimImpl.initialize, (msg.sender, signer)) + ); + dkim = ECDSAOwnedDKIMRegistry(address(dkimProxy)); + } + } + + function test_Revert_IfPublicKeyHashNotSet() public { + // Attempt to revoke a public key hash that hasn't been set + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.expectRevert("publicKeyHash is not set"); + dkim.revokeDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + } + + function test_Revert_IfSignatureIsInvalid() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + + // Attempt to revoke with an invalid signature + bytes memory invalidSignature = abi.encodePacked(r, s, v + 1); // Alter the signature + vm.expectRevert(ECDSA.ECDSAInvalidSignature.selector); + dkim.revokeDKIMPublicKeyHash(selector, domainName, publicKeyHash, invalidSignature); + } + function testRevert_IfDomainNameIsDifferent() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + + // Attempt to revoke with a different domain name + string memory differentDomainName = "different.com"; + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + differentDomainName, + publicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + + vm.expectRevert("publicKeyHash is not set"); + dkim.revokeDKIMPublicKeyHash(selector, differentDomainName, publicKeyHash, revokeSig); + } + + function testRevert_IfSelectorIsDifferent() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + + // Attempt to revoke with a different selector + string memory differentSelector = "54321"; + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + + vm.expectRevert("Invalid signature"); + dkim.revokeDKIMPublicKeyHash(differentSelector, domainName, publicKeyHash, revokeSig); + } + + function test_RevokeDKIMPublicKeyHash() public { + // vm.chainId(1); + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Revoke + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + domainName, + publicKeyHash + ); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign( + 1, + MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)) + ); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + revokeSig + ); + + require(!dkim.isDKIMPublicKeyHashValid(domainName, publicKeyHash)); + } + +} \ No newline at end of file diff --git a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry.t.sol b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/setDKIMPublicKeyHash.t.sol similarity index 67% rename from packages/contracts/test/utils/ECDSAOwnedDKIMRegistry.t.sol rename to packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/setDKIMPublicKeyHash.t.sol index 9c5aa1e6..fdfd8e8b 100644 --- a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry.t.sol +++ b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/setDKIMPublicKeyHash.t.sol @@ -4,11 +4,11 @@ pragma solidity ^0.8.12; import "forge-std/Test.sol"; import "forge-std/console.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import "../../src/utils/ECDSAOwnedDKIMRegistry.sol"; +import "../../../src/utils/ECDSAOwnedDKIMRegistry.sol"; import "@openzeppelin/contracts/utils/Strings.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; -contract ECDSAOwnedDKIMRegistryTest is Test { +contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { ECDSAOwnedDKIMRegistry dkim; using console for *; using ECDSA for *; @@ -31,6 +31,106 @@ contract ECDSAOwnedDKIMRegistryTest is Test { } } + function test_Revert_IfInvalidSelector() public { + string memory invalidSelector = ""; // Example of an invalid selector (empty string) + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + invalidSelector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.expectRevert("Invalid selector"); + dkim.setDKIMPublicKeyHash(invalidSelector, domainName, publicKeyHash, signature); + } + + function test_Revert_IfInvalidDomainName() public { + string memory invalidDomainName = ""; // Example of an invalid domain name (empty string) + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + invalidDomainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.expectRevert("Invalid domain name"); + dkim.setDKIMPublicKeyHash(selector, invalidDomainName, publicKeyHash, signature); + } + + function test_MinLengthSelectorAndDomainName() public { + string memory minSelector = "a"; + string memory minDomainName = "b"; + bytes32 minPublicKeyHash = bytes32(uint256(1)); + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + minSelector, + minDomainName, + minPublicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + dkim.setDKIMPublicKeyHash(minSelector, minDomainName, minPublicKeyHash, signature); + require(dkim.isDKIMPublicKeyHashValid(minDomainName, minPublicKeyHash), "Invalid public key hash"); + } + + function test_MaxLengthSelectorAndDomainName() public { + string memory maxSelector = new string(256); + string memory maxDomainName = new string(256); + bytes32 aPublicKeyHash = bytes32(uint256(1)); + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + maxSelector, + maxDomainName, + aPublicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + dkim.setDKIMPublicKeyHash(maxSelector, maxDomainName, aPublicKeyHash, signature); + require(dkim.isDKIMPublicKeyHashValid(maxDomainName, aPublicKeyHash), "Invalid public key hash"); + } + + function test_Revert_IfInvalidPublicKeyHash() public { + bytes32 zeroPublicKeyHash = bytes32(0); + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + zeroPublicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + vm.expectRevert("Invalid public key hash"); + dkim.setDKIMPublicKeyHash(selector, domainName, zeroPublicKeyHash, signature); + } + + function test_MaxValuePublicKeyHash() public { + bytes32 maxPublicKeyHash = bytes32(type(uint256).max); + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + maxPublicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + + dkim.setDKIMPublicKeyHash(selector, domainName, maxPublicKeyHash, signature); + require(dkim.isDKIMPublicKeyHashValid(domainName, maxPublicKeyHash), "Invalid public key hash"); + } + function test_SetDKIMPublicKeyHash() public { string memory signedMsg = dkim.computeSignedMsg( dkim.SET_PREFIX(), @@ -103,49 +203,8 @@ contract ECDSAOwnedDKIMRegistryTest is Test { ); } - function test_RevokeDKIMPublicKeyHash() public { - // vm.chainId(1); - string memory signedMsg = dkim.computeSignedMsg( - dkim.SET_PREFIX(), - selector, - domainName, - publicKeyHash - ); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - bytes(signedMsg) - ); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); - bytes memory signature = abi.encodePacked(r, s, v); - dkim.setDKIMPublicKeyHash( - selector, - domainName, - publicKeyHash, - signature - ); - - // Revoke - string memory revokeMsg = dkim.computeSignedMsg( - dkim.REVOKE_PREFIX(), - selector, - domainName, - publicKeyHash - ); - (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign( - 1, - MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)) - ); - bytes memory revokeSig = abi.encodePacked(r1, s1, v1); - dkim.revokeDKIMPublicKeyHash( - selector, - domainName, - publicKeyHash, - revokeSig - ); - - require(!dkim.isDKIMPublicKeyHashValid(domainName, publicKeyHash)); - } - function test_RevertIfDuplicated() public { + function test_Revert_IfDuplicated() public { // vm.chainId(1); string memory signedMsg = dkim.computeSignedMsg( dkim.SET_PREFIX(), @@ -180,7 +239,7 @@ contract ECDSAOwnedDKIMRegistryTest is Test { ); } - function test_RevertIfRevorked() public { + function test_Revert_IfRevorked() public { // vm.chainId(1); string memory signedMsg = dkim.computeSignedMsg( dkim.SET_PREFIX(), @@ -241,7 +300,7 @@ contract ECDSAOwnedDKIMRegistryTest is Test { ); } - function test_RevertIfSignatureInvalid() public { + function test_Revert_IfSignatureInvalid() public { // vm.chainId(1); string memory signedMsg = dkim.computeSignedMsg( dkim.SET_PREFIX(), From 2d6971a3799539d1ca100fd0b3f656bee2cd984c Mon Sep 17 00:00:00 2001 From: wshino Date: Sat, 12 Oct 2024 23:15:43 +0900 Subject: [PATCH 2/5] Add test cases for DKIM registries --- .../ECDSAOwnedDKIMRegistry/changeSigner.t.sol | 48 +++ .../computeSignedMsg.t.sol | 49 +++ .../isDKIMPublicKeyHashValid.t.sol | 106 ++++++ .../revokeDKIMPublicKeyHash.t.sol | 302 ++++++++++++++++-- .../test/utils/ForwardDKIMRegistry/base.t.sol | 22 ++ .../changeSourceDKIMRegistry.t.sol | 50 +++ .../isDKIMPublicKeyHashValid.t.sol | 61 ++++ ...ForUpgradeFromECDSAOwnedDKIMRegistry.t.sol | 51 +++ 8 files changed, 671 insertions(+), 18 deletions(-) create mode 100644 packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/changeSigner.t.sol create mode 100644 packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/computeSignedMsg.t.sol create mode 100644 packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/isDKIMPublicKeyHashValid.t.sol create mode 100644 packages/contracts/test/utils/ForwardDKIMRegistry/base.t.sol create mode 100644 packages/contracts/test/utils/ForwardDKIMRegistry/changeSourceDKIMRegistry.t.sol create mode 100644 packages/contracts/test/utils/ForwardDKIMRegistry/isDKIMPublicKeyHashValid.t.sol create mode 100644 packages/contracts/test/utils/ForwardDKIMRegistry/resetStorageForUpgradeFromECDSAOwnedDKIMRegistry.t.sol diff --git a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/changeSigner.t.sol b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/changeSigner.t.sol new file mode 100644 index 00000000..38be5160 --- /dev/null +++ b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/changeSigner.t.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "../../../src/utils/ECDSAOwnedDKIMRegistry.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +contract ECDSAOwnedDKIMRegistryTest_changeSigner is Test { + ECDSAOwnedDKIMRegistry dkim; + + function setUp() public { + address signer = vm.addr(1); + ECDSAOwnedDKIMRegistry dkimImpl = new ECDSAOwnedDKIMRegistry(); + ERC1967Proxy dkimProxy = new ERC1967Proxy( + address(dkimImpl), + abi.encodeCall(dkimImpl.initialize, (msg.sender, signer)) + ); + dkim = ECDSAOwnedDKIMRegistry(address(dkimProxy)); + } + + function test_ChangeSigner() public { + address owner = dkim.owner(); + address newSigner = vm.addr(2); + + vm.startPrank(owner); + dkim.changeSigner(newSigner); + vm.stopPrank(); + + assertEq(dkim.signer(), newSigner, "Signer was not changed correctly"); + } + + function test_Revert_IfNewSignerIsZeroAddress() public { + address owner = dkim.owner(); + vm.startPrank(owner); + + vm.expectRevert("Invalid signer"); + dkim.changeSigner(address(0)); + } + + function test_Revert_IfNewSignerIsSame() public { + address owner = dkim.owner(); + address currentSigner = dkim.signer(); + + vm.startPrank(owner); + vm.expectRevert("Same signer"); + dkim.changeSigner(currentSigner); + } +} diff --git a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/computeSignedMsg.t.sol b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/computeSignedMsg.t.sol new file mode 100644 index 00000000..657585f7 --- /dev/null +++ b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/computeSignedMsg.t.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "../../../src/utils/ECDSAOwnedDKIMRegistry.sol"; + +contract ECDSAOwnedDKIMRegistryTest_computeSignedMsg is Test { + using Strings for *; + + ECDSAOwnedDKIMRegistry dkim; + + function setUp() public { + address signer = vm.addr(1); + ECDSAOwnedDKIMRegistry dkimImpl = new ECDSAOwnedDKIMRegistry(); + dkimImpl.initialize(msg.sender, signer); + dkim = dkimImpl; + } + + function test_computeSignedMsg() public { + string memory prefix = "SET:"; + string memory selector = "12345"; + string memory domainName = "example.com"; + bytes32 publicKeyHash = bytes32(uint256(1)); + + string memory expectedMsg = string.concat( + prefix, + "selector=", + selector, + ";domain=", + domainName, + ";public_key_hash=", + uint256(publicKeyHash).toHexString(), + ";" + ); + + string memory computedMsg = dkim.computeSignedMsg( + prefix, + selector, + domainName, + publicKeyHash + ); + + assertEq( + computedMsg, + expectedMsg, + "Computed message does not match expected message" + ); + } +} diff --git a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/isDKIMPublicKeyHashValid.t.sol b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/isDKIMPublicKeyHashValid.t.sol new file mode 100644 index 00000000..1bdb58d7 --- /dev/null +++ b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/isDKIMPublicKeyHashValid.t.sol @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "../../../src/utils/ECDSAOwnedDKIMRegistry.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +contract ECDSAOwnedDKIMRegistryTest_isDKIMPublicKeyHashValid is Test { + ECDSAOwnedDKIMRegistry dkim; + + string public selector = "12345"; + string public domainName = "example.com"; + bytes32 public publicKeyHash = bytes32(uint256(1)); + + function setUp() public { + address signer = vm.addr(1); + ECDSAOwnedDKIMRegistry dkimImpl = new ECDSAOwnedDKIMRegistry(); + ERC1967Proxy dkimProxy = new ERC1967Proxy( + address(dkimImpl), + abi.encodeCall(dkimImpl.initialize, (msg.sender, signer)) + ); + dkim = ECDSAOwnedDKIMRegistry(address(dkimProxy)); + } + + function test_IsDKIMPublicKeyHashValid_True() public { + // Set a valid public key hash + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Check if the public key hash is valid + bool isValid = dkim.isDKIMPublicKeyHashValid(domainName, publicKeyHash); + require(isValid, "Public key hash should be valid"); + } + + function test_IsDKIMPublicKeyHashValid_False() public { + // Check if a non-set public key hash is invalid + bytes32 nonExistentPublicKeyHash = bytes32(uint256(2)); + bool isValid = dkim.isDKIMPublicKeyHashValid( + domainName, + nonExistentPublicKeyHash + ); + require(!isValid, "Public key hash should not be valid"); + } + + function test_IsDKIMPublicKeyHashValid_AfterRevoke() public { + // Set and then revoke a public key hash + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Revoke the public key hash + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + revokeSig + ); + + // Check if the public key hash is invalid after revocation + bool isValid = dkim.isDKIMPublicKeyHashValid(domainName, publicKeyHash); + require( + !isValid, + "Public key hash should not be valid after revocation" + ); + } +} diff --git a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol index 3a47a60d..35eafdb9 100644 --- a/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol +++ b/packages/contracts/test/utils/ECDSAOwnedDKIMRegistry/revokeDKIMPublicKeyHash.t.sol @@ -8,7 +8,7 @@ import "../../../src/utils/ECDSAOwnedDKIMRegistry.sol"; import "@openzeppelin/contracts/utils/Strings.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; -contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { +contract ECDSAOwnedDKIMRegistryTest_revokeDKIMPublicKeyHash is Test { ECDSAOwnedDKIMRegistry dkim; using console for *; using ECDSA for *; @@ -39,12 +39,239 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { domainName, publicKeyHash ); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); bytes memory signature = abi.encodePacked(r, s, v); vm.expectRevert("publicKeyHash is not set"); - dkim.revokeDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + } + + function test_Revert_IfSignerIsIncorrect() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Attempt to revoke with a signature from a different signer + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(2, digest); // Different signer + bytes memory invalidSignature = abi.encodePacked(r1, s1, v1); + + vm.expectRevert("Invalid signature"); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + invalidSignature + ); + } + + // invalid selectorをチェックするテストケース + function test_Revert_IfSelectorIsInvalid() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Attempt to revoke with an invalid selector + string memory invalidSelector = ""; + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + invalidSelector, + domainName, + publicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + + vm.expectRevert("Invalid selector"); + dkim.revokeDKIMPublicKeyHash( + invalidSelector, + domainName, + publicKeyHash, + revokeSig + ); + } + // Invalid domain nameエラーをチェックするテストケース + function test_Revert_IfDomainNameIsInvalid() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Attempt to revoke with an invalid domain name + string memory invalidDomainName = ""; + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + invalidDomainName, + publicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + + vm.expectRevert("Invalid domain name"); + dkim.revokeDKIMPublicKeyHash( + selector, + invalidDomainName, + publicKeyHash, + revokeSig + ); + } + // Invalid public key hashエラーをチェックするテストケース + function test_Revert_IfPublicKeyHashIsInvalid() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Attempt to revoke with an invalid public key hash + bytes32 invalidPublicKeyHash = bytes32(0); + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + domainName, + invalidPublicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + + vm.expectRevert("Invalid public key hash"); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + invalidPublicKeyHash, + revokeSig + ); + } + // publicKeyHash is already revokedエラーをチェックするテストケース + function test_Revert_IfPublicKeyHashIsAlreadyRevoked() public { + // Set a valid public key hash first + string memory signedMsg = dkim.computeSignedMsg( + dkim.SET_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); + bytes memory signature = abi.encodePacked(r, s, v); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); + + // Revoke the public key hash + string memory revokeMsg = dkim.computeSignedMsg( + dkim.REVOKE_PREFIX(), + selector, + domainName, + publicKeyHash + ); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); + bytes memory revokeSig = abi.encodePacked(r1, s1, v1); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + revokeSig + ); + + // Mock the call to dkimRegistry.isDKIMPublicKeyHashValid to return true + vm.mockCall( + address(dkim.dkimRegistry()), + abi.encodeWithSelector( + IDKIMRegistry.isDKIMPublicKeyHashValid.selector + ), + abi.encode(true) + ); + // Attempt to revoke the already revoked public key hash + vm.expectRevert("publicKeyHash is already revoked"); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + revokeSig + ); } function test_Revert_IfSignatureIsInvalid() public { @@ -55,17 +282,29 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { domainName, publicKeyHash ); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); bytes memory signature = abi.encodePacked(r, s, v); - dkim.setDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); // Attempt to revoke with an invalid signature bytes memory invalidSignature = abi.encodePacked(r, s, v + 1); // Alter the signature vm.expectRevert(ECDSA.ECDSAInvalidSignature.selector); - dkim.revokeDKIMPublicKeyHash(selector, domainName, publicKeyHash, invalidSignature); + dkim.revokeDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + invalidSignature + ); } - function testRevert_IfDomainNameIsDifferent() public { + function test_Revert_IfDomainNameIsDifferent() public { // Set a valid public key hash first string memory signedMsg = dkim.computeSignedMsg( dkim.SET_PREFIX(), @@ -73,10 +312,17 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { domainName, publicKeyHash ); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); bytes memory signature = abi.encodePacked(r, s, v); - dkim.setDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); // Attempt to revoke with a different domain name string memory differentDomainName = "different.com"; @@ -86,15 +332,22 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { differentDomainName, publicKeyHash ); - bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); bytes memory revokeSig = abi.encodePacked(r1, s1, v1); vm.expectRevert("publicKeyHash is not set"); - dkim.revokeDKIMPublicKeyHash(selector, differentDomainName, publicKeyHash, revokeSig); + dkim.revokeDKIMPublicKeyHash( + selector, + differentDomainName, + publicKeyHash, + revokeSig + ); } - function testRevert_IfSelectorIsDifferent() public { + function test_Revert_IfSelectorIsDifferent() public { // Set a valid public key hash first string memory signedMsg = dkim.computeSignedMsg( dkim.SET_PREFIX(), @@ -102,10 +355,17 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { domainName, publicKeyHash ); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash(bytes(signedMsg)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + bytes(signedMsg) + ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, digest); bytes memory signature = abi.encodePacked(r, s, v); - dkim.setDKIMPublicKeyHash(selector, domainName, publicKeyHash, signature); + dkim.setDKIMPublicKeyHash( + selector, + domainName, + publicKeyHash, + signature + ); // Attempt to revoke with a different selector string memory differentSelector = "54321"; @@ -115,12 +375,19 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { domainName, publicKeyHash ); - bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash(bytes(revokeMsg)); + bytes32 revokeDigest = MessageHashUtils.toEthSignedMessageHash( + bytes(revokeMsg) + ); (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(1, revokeDigest); bytes memory revokeSig = abi.encodePacked(r1, s1, v1); vm.expectRevert("Invalid signature"); - dkim.revokeDKIMPublicKeyHash(differentSelector, domainName, publicKeyHash, revokeSig); + dkim.revokeDKIMPublicKeyHash( + differentSelector, + domainName, + publicKeyHash, + revokeSig + ); } function test_RevokeDKIMPublicKeyHash() public { @@ -164,5 +431,4 @@ contract ECDSAOwnedDKIMRegistryTest_setDKIMPublicKeyHash is Test { require(!dkim.isDKIMPublicKeyHashValid(domainName, publicKeyHash)); } - -} \ No newline at end of file +} diff --git a/packages/contracts/test/utils/ForwardDKIMRegistry/base.t.sol b/packages/contracts/test/utils/ForwardDKIMRegistry/base.t.sol new file mode 100644 index 00000000..3076ec7b --- /dev/null +++ b/packages/contracts/test/utils/ForwardDKIMRegistry/base.t.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "@zk-email/contracts/DKIMRegistry.sol"; + +contract MockDKIMRegistry is IDKIMRegistry { + mapping(string => bytes32) public validHashes; + + function setValidHash( + string memory domainName, + bytes32 publicKeyHash + ) public { + validHashes[domainName] = publicKeyHash; + } + + function isDKIMPublicKeyHashValid( + string memory domainName, + bytes32 publicKeyHash + ) public view override returns (bool) { + return validHashes[domainName] == publicKeyHash; + } +} diff --git a/packages/contracts/test/utils/ForwardDKIMRegistry/changeSourceDKIMRegistry.t.sol b/packages/contracts/test/utils/ForwardDKIMRegistry/changeSourceDKIMRegistry.t.sol new file mode 100644 index 00000000..436c40bc --- /dev/null +++ b/packages/contracts/test/utils/ForwardDKIMRegistry/changeSourceDKIMRegistry.t.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "./base.t.sol"; +import "../../../src/utils/ForwardDKIMRegistry.sol"; + +contract ForwardDKIMRegistryTest_changeSourceDKIMRegistry is Test { + ForwardDKIMRegistry forwardDKIMRegistry; + MockDKIMRegistry mockDKIMRegistry; + MockDKIMRegistry newMockDKIMRegistry; + + function setUp() public { + mockDKIMRegistry = new MockDKIMRegistry(); + newMockDKIMRegistry = new MockDKIMRegistry(); + forwardDKIMRegistry = new ForwardDKIMRegistry(); + forwardDKIMRegistry.initialize( + address(this), + address(mockDKIMRegistry) + ); + } + + function testChangeSourceDKIMRegistry_Success() public { + forwardDKIMRegistry.changeSourceDKIMRegistry( + address(newMockDKIMRegistry) + ); + assertEq( + address(forwardDKIMRegistry.sourceDKIMRegistry()), + address(newMockDKIMRegistry), + "The source DKIMRegistry should be updated." + ); + } + + function testChangeSourceDKIMRegistry_InvalidAddress() public { + vm.expectRevert("Invalid address"); + forwardDKIMRegistry.changeSourceDKIMRegistry(address(0)); + } + + function testChangeSourceDKIMRegistry_SameAddress() public { + vm.expectRevert("Same source DKIMRegistry"); + forwardDKIMRegistry.changeSourceDKIMRegistry(address(mockDKIMRegistry)); + } + + function testChangeSourceDKIMRegistry_SelfAddress() public { + vm.expectRevert("Cannot set self as source DKIMRegistry"); + forwardDKIMRegistry.changeSourceDKIMRegistry( + address(forwardDKIMRegistry) + ); + } +} diff --git a/packages/contracts/test/utils/ForwardDKIMRegistry/isDKIMPublicKeyHashValid.t.sol b/packages/contracts/test/utils/ForwardDKIMRegistry/isDKIMPublicKeyHashValid.t.sol new file mode 100644 index 00000000..f87ad076 --- /dev/null +++ b/packages/contracts/test/utils/ForwardDKIMRegistry/isDKIMPublicKeyHashValid.t.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "./base.t.sol"; +import "../../../src/utils/ForwardDKIMRegistry.sol"; + +contract ForwardDKIMRegistryTest_isDKIMPublicKeyHashValid is Test { + ForwardDKIMRegistry forwardDKIMRegistry; + MockDKIMRegistry mockDKIMRegistry; + + function setUp() public { + mockDKIMRegistry = new MockDKIMRegistry(); + forwardDKIMRegistry = new ForwardDKIMRegistry(); + forwardDKIMRegistry.initialize( + address(this), + address(mockDKIMRegistry) + ); + } + + function testIsDKIMPublicKeyHashValid_ValidHash() public { + string memory domainName = "example.com"; + bytes32 validHash = keccak256(abi.encodePacked("validPublicKey")); + + mockDKIMRegistry.setValidHash(domainName, validHash); + + bool isValid = forwardDKIMRegistry.isDKIMPublicKeyHashValid( + domainName, + validHash + ); + assertTrue(isValid, "The public key hash should be valid."); + } + + function testIsDKIMPublicKeyHashValid_InvalidHash() public { + string memory domainName = "example.com"; + bytes32 validHash = keccak256(abi.encodePacked("validPublicKey")); + bytes32 invalidHash = keccak256(abi.encodePacked("invalidPublicKey")); + + mockDKIMRegistry.setValidHash(domainName, validHash); + + bool isValid = forwardDKIMRegistry.isDKIMPublicKeyHashValid( + domainName, + invalidHash + ); + assertFalse(isValid, "The public key hash should be invalid."); + } + + function testIsDKIMPublicKeyHashValid_NoHashSet() public { + string memory domainName = "example.com"; + bytes32 someHash = keccak256(abi.encodePacked("somePublicKey")); + + bool isValid = forwardDKIMRegistry.isDKIMPublicKeyHashValid( + domainName, + someHash + ); + assertFalse( + isValid, + "The public key hash should be invalid as no hash is set." + ); + } +} diff --git a/packages/contracts/test/utils/ForwardDKIMRegistry/resetStorageForUpgradeFromECDSAOwnedDKIMRegistry.t.sol b/packages/contracts/test/utils/ForwardDKIMRegistry/resetStorageForUpgradeFromECDSAOwnedDKIMRegistry.t.sol new file mode 100644 index 00000000..601b59d3 --- /dev/null +++ b/packages/contracts/test/utils/ForwardDKIMRegistry/resetStorageForUpgradeFromECDSAOwnedDKIMRegistry.t.sol @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "../../../src/utils/ForwardDKIMRegistry.sol"; +import "./base.t.sol"; // Import the MockDKIMRegistry + +contract ForwardDKIMRegistryTest_resetStorageForUpgradeFromECDSAOwnedDKIMRegistry is + Test +{ + ForwardDKIMRegistry forwardDKIMRegistry; + MockDKIMRegistry mockDKIMRegistry; + address newSourceDKIMRegistry; + + function setUp() public { + mockDKIMRegistry = new MockDKIMRegistry(); + forwardDKIMRegistry = new ForwardDKIMRegistry(); + forwardDKIMRegistry.initialize( + address(this), + address(mockDKIMRegistry) + ); + newSourceDKIMRegistry = address(new MockDKIMRegistry()); + } + + function testResetStorageForUpgradeFromECDSAOwnedDKIMRegistry() public { + // Set a new source DKIMRegistry + forwardDKIMRegistry.resetStorageForUpgradeFromECDSAOwnedDKIMRegistry( + newSourceDKIMRegistry + ); + + // Verify that the sourceDKIMRegistry is updated + assertEq( + address(forwardDKIMRegistry.sourceDKIMRegistry()), + newSourceDKIMRegistry, + "The source DKIMRegistry should be updated." + ); + + // Verify that the storage slot for signer is reset + bytes32 signerSlot = keccak256(abi.encodePacked(uint256(1))); + bytes32 expectedSignerValue = bytes32(0); + bytes32 actualSignerValue; + assembly { + actualSignerValue := sload(signerSlot) + } + assertEq( + actualSignerValue, + expectedSignerValue, + "The signer storage slot should be reset to zero." + ); + } +} From a7a717778b79721b236f994bdd5e51fc1f56cc59 Mon Sep 17 00:00:00 2001 From: wshino Date: Sat, 12 Oct 2024 23:34:13 +0900 Subject: [PATCH 3/5] Add test cases for EmailAccountRecovery --- ...mailAccountRecovery_handleAcceptance.t.sol | 220 ++++++++++++++++++ .../EmailAccountRecovery_handleRecovery.t.sol | 122 +++++++++- ...EmailAccountRecovery_requestGuardian.t.sol | 99 +++++--- 3 files changed, 411 insertions(+), 30 deletions(-) diff --git a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleAcceptance.t.sol b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleAcceptance.t.sol index 5d3e3428..3e689134 100644 --- a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleAcceptance.t.sol +++ b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleAcceptance.t.sol @@ -8,6 +8,7 @@ import {RecoveryController} from "../helpers/RecoveryController.sol"; import {StructHelper} from "../helpers/StructHelper.sol"; import {SimpleWallet} from "../helpers/SimpleWallet.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol"; contract EmailAccountRecoveryTest_handleAcceptance is StructHelper { constructor() {} @@ -35,6 +36,225 @@ contract EmailAccountRecoveryTest_handleAcceptance is StructHelper { ); } + function testExpectRevertHandleAcceptanceInvalidRecoveredAccount() public { + skipIfZkSync(); + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + emailAuthMsg.templateId = recoveryController + .computeAcceptanceTemplateId(0); + emailAuthMsg.commandParams[0] = abi.encode(address(0x0)); // Invalid account + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid command params")); + recoveryController.handleAcceptance(emailAuthMsg, 0); + vm.stopPrank(); + } + + function testExpectRevertHandleAcceptanceInvalidTemplateId() public { + skipIfZkSync(); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + uint templateId = recoveryController.computeAcceptanceTemplateId( + templateIdx + ); + emailAuthMsg.templateId = 999; // invalid template id + bytes[] memory commandParamsForAcceptance = new bytes[](1); + commandParamsForAcceptance[0] = abi.encode(address(simpleWallet)); + emailAuthMsg.commandParams = commandParamsForAcceptance; + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid template id")); + recoveryController.handleAcceptance(emailAuthMsg, 0); + vm.stopPrank(); + } + + function testExpectRevertHandleAcceptanceInvalidEmailAuthMsgStructure() + public + { + skipIfZkSync(); + + requestGuardian(); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + + uint templateIdx = 0; + + // Create an invalid EmailAuthMsg with empty commandParams + EmailAuthMsg memory emailAuthMsg; + emailAuthMsg.templateId = recoveryController + .computeAcceptanceTemplateId(templateIdx); + emailAuthMsg.commandParams = new bytes[](0); // Invalid structure + + vm.mockCall( + address(recoveryController.emailAuthImplementationAddr()), + abi.encodeWithSelector(EmailAuth.authEmail.selector, emailAuthMsg), + abi.encode(0x0) + ); + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid command params")); + recoveryController.handleAcceptance(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleAcceptanceInvalidVerifier() public { + skipIfZkSync(); + + requestGuardian(); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + uint templateId = recoveryController.computeAcceptanceTemplateId( + templateIdx + ); + emailAuthMsg.templateId = templateId; + bytes[] memory commandParamsForAcceptance = new bytes[](1); + commandParamsForAcceptance[0] = abi.encode(address(simpleWallet)); + emailAuthMsg.commandParams = commandParamsForAcceptance; + + // Set Verifier address to address(0) + vm.store( + address(recoveryController), + bytes32(uint256(0)), // Assuming Verifier is the 1st storage slot in RecoveryController + bytes32(uint256(0)) + ); + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid verifier address")); + recoveryController.handleAcceptance(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleAcceptanceInvalidDKIMRegistry() public { + skipIfZkSync(); + + requestGuardian(); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + uint templateId = recoveryController.computeAcceptanceTemplateId( + templateIdx + ); + emailAuthMsg.templateId = templateId; + bytes[] memory commandParamsForAcceptance = new bytes[](1); + commandParamsForAcceptance[0] = abi.encode(address(simpleWallet)); + emailAuthMsg.commandParams = commandParamsForAcceptance; + + // Set DKIMRegistry address to address(0) + vm.store( + address(recoveryController), + bytes32(uint256(1)), // Assuming DKIMRegistry is the 2nd storage slot in RecoveryController + bytes32(uint256(0)) + ); + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid dkim registry address")); + recoveryController.handleAcceptance(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleAcceptanceInvalidEmailAuthImplementationAddr() + public + { + skipIfZkSync(); + + requestGuardian(); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + uint templateId = recoveryController.computeAcceptanceTemplateId( + templateIdx + ); + emailAuthMsg.templateId = templateId; + bytes[] memory commandParamsForAcceptance = new bytes[](1); + commandParamsForAcceptance[0] = abi.encode(address(simpleWallet)); + emailAuthMsg.commandParams = commandParamsForAcceptance; + + // Set EmailAuthImplementationAddr address to address(0) + vm.store( + address(recoveryController), + bytes32(uint256(2)), // Assuming EmailAuthImplementationAddr is the 3rd storage slot in RecoveryController + bytes32(uint256(0)) + ); + + vm.startPrank(someRelayer); + vm.expectRevert( + abi.encodeWithSelector( + ERC1967Utils.ERC1967InvalidImplementation.selector, + address(0) + ) + ); + recoveryController.handleAcceptance(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleAcceptanceInvalidController() public { + skipIfZkSync(); + + // First, request and accept a guardian + requestGuardian(); + uint templateIdx = 0; + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + uint templateId = recoveryController.computeAcceptanceTemplateId( + templateIdx + ); + emailAuthMsg.templateId = templateId; + bytes[] memory commandParamsForAcceptance = new bytes[](1); + commandParamsForAcceptance[0] = abi.encode(address(simpleWallet)); + emailAuthMsg.commandParams = commandParamsForAcceptance; + + vm.mockCall( + address(recoveryController.emailAuthImplementationAddr()), + abi.encodeWithSelector(EmailAuth.authEmail.selector, emailAuthMsg), + abi.encode(0x0) + ); + + vm.prank(someRelayer); + recoveryController.handleAcceptance(emailAuthMsg, templateIdx); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.ACCEPTED, + "Guardian should be accepted" + ); + + // Now, set an invalid controller for the guardian's EmailAuth contract + address invalidController = address(0x1234); + vm.mockCall( + guardian, + abi.encodeWithSelector(bytes4(keccak256("controller()"))), + abi.encode(invalidController) + ); + + // Try to handle acceptance again, which should fail due to invalid controller + vm.expectRevert("invalid controller"); + vm.prank(someRelayer); + recoveryController.handleAcceptance(emailAuthMsg, templateIdx); + } + function testHandleAcceptance() public { skipIfZkSync(); diff --git a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleRecovery.t.sol b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleRecovery.t.sol index ccaca48e..f324f20a 100644 --- a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleRecovery.t.sol +++ b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_handleRecovery.t.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.12; import "forge-std/Test.sol"; import "forge-std/console.sol"; import {EmailAuth, EmailAuthMsg} from "../../src/EmailAuth.sol"; -import {RecoveryController} from "../helpers/RecoveryController.sol"; +import {RecoveryController, EmailAccountRecovery} from "../helpers/RecoveryController.sol"; import {StructHelper} from "../helpers/StructHelper.sol"; import {SimpleWallet} from "../helpers/SimpleWallet.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; @@ -73,6 +73,126 @@ contract EmailAccountRecoveryTest_handleRecovery is StructHelper { ); } +function testExpectRevertHandleRecoveryInvalidRecoveredAccount() public { + skipIfZkSync(); + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + emailAuthMsg.templateId = recoveryController.computeRecoveryTemplateId(0); + emailAuthMsg.commandParams[0] = abi.encode(address(0x0)); // Invalid account + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid account in email")); + recoveryController.handleRecovery(emailAuthMsg, 0); + vm.stopPrank(); +} + function testExpectRevertHandleRecoveryInvalidAccountInEmail() public { + skipIfZkSync(); + + handleAcceptance(); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + emailAuthMsg.templateId = recoveryController.computeRecoveryTemplateId( + templateIdx + ); + bytes[] memory commandParamsForRecovery = new bytes[](2); + commandParamsForRecovery[0] = abi.encode(address(0x0)); // Invalid account + commandParamsForRecovery[1] = abi.encode(newSigner); + emailAuthMsg.commandParams = commandParamsForRecovery; + + vm.mockCall( + address(recoveryController.emailAuthImplementationAddr()), + abi.encodeWithSelector(EmailAuth.authEmail.selector, emailAuthMsg), + abi.encode(0x0) + ); + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid account in email")); + recoveryController.handleRecovery(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleRecoveryGuardianCodeLengthZero() public { + skipIfZkSync(); + + handleAcceptance(); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + emailAuthMsg.templateId = recoveryController.computeRecoveryTemplateId( + templateIdx + ); + bytes[] memory commandParamsForRecovery = new bytes[](2); + commandParamsForRecovery[0] = abi.encode(simpleWallet); + commandParamsForRecovery[1] = abi.encode(newSigner); + emailAuthMsg.commandParams = commandParamsForRecovery; + + // Mock guardian with no code + vm.etch(guardian, ""); + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("guardian is not deployed")); + recoveryController.handleRecovery(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleRecoveryTemplateIdMismatch() public { + skipIfZkSync(); + + handleAcceptance(); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + emailAuthMsg.templateId = 999; // Invalid template ID + bytes[] memory commandParamsForRecovery = new bytes[](2); + commandParamsForRecovery[0] = abi.encode(simpleWallet); + commandParamsForRecovery[1] = abi.encode(newSigner); + emailAuthMsg.commandParams = commandParamsForRecovery; + + vm.mockCall( + address(recoveryController.emailAuthImplementationAddr()), + abi.encodeWithSelector(EmailAuth.authEmail.selector, emailAuthMsg), + abi.encode(0x0) + ); + + vm.startPrank(someRelayer); + vm.expectRevert(bytes("invalid template id")); + recoveryController.handleRecovery(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + + function testExpectRevertHandleRecoveryAuthEmailFailure() public { + skipIfZkSync(); + + handleAcceptance(); + + uint templateIdx = 0; + + EmailAuthMsg memory emailAuthMsg = buildEmailAuthMsg(); + emailAuthMsg.templateId = recoveryController.computeRecoveryTemplateId( + templateIdx + ); + bytes[] memory commandParamsForRecovery = new bytes[](2); + commandParamsForRecovery[0] = abi.encode(simpleWallet); + commandParamsForRecovery[1] = abi.encode(newSigner); + emailAuthMsg.commandParams = commandParamsForRecovery; + + // Mock authEmail to fail + vm.mockCallRevert( + address(recoveryController.emailAuthImplementationAddr()), + abi.encodeWithSelector(EmailAuth.authEmail.selector, emailAuthMsg), + "REVERT_MESSAGE" + ); + + vm.startPrank(someRelayer); + vm.expectRevert(); // Expect any revert due to authEmail failure + recoveryController.handleRecovery(emailAuthMsg, templateIdx); + vm.stopPrank(); + } + function testHandleRecovery() public { skipIfZkSync(); diff --git a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_requestGuardian.t.sol b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_requestGuardian.t.sol index e03bdaf3..3e760fb5 100644 --- a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_requestGuardian.t.sol +++ b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_requestGuardian.t.sol @@ -10,48 +10,32 @@ import {SimpleWallet} from "../helpers/SimpleWallet.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; contract EmailAccountRecoveryTest_requestGuardian is StructHelper { + using stdStorage for StdStorage; + constructor() {} function setUp() public override { super.setUp(); } - function testRequestGuardian() public { + function testExpectRevertRequestGuardianRecoveryInProgress() public { skipIfZkSync(); setUp(); - require( - recoveryController.guardians(guardian) == - RecoveryController.GuardianStatus.NONE - ); - vm.startPrank(deployer); recoveryController.requestGuardian(guardian); - vm.stopPrank(); - - require( - recoveryController.guardians(guardian) == - RecoveryController.GuardianStatus.REQUESTED - ); - } - // function testRequestGuardianNotOwner() public { - // setUp(); + // Simulate recovery in progress + stdstore + .target(address(recoveryController)) + .sig("isRecovering(address)") + .with_key(address(deployer)) + .checked_write(true); - // require( - // recoveryController.guardians(guardian) == - // recoveryController.GuardianStatus.NONE - // ); - - // vm.startPrank(receiver); - // recoveryController.requestGuardian(guardian); - // vm.stopPrank(); - - // require( - // recoveryController.guardians(guardian) == - // recoveryController.GuardianStatus.NONE - // ); - // } + vm.expectRevert(bytes("recovery in progress")); + recoveryController.requestGuardian(address(0x123)); // Try to request a new guardian + vm.stopPrank(); + } function testExpectRevertRequestGuardianInvalidGuardian() public { skipIfZkSync(); @@ -85,4 +69,61 @@ contract EmailAccountRecoveryTest_requestGuardian is StructHelper { recoveryController.requestGuardian(guardian); vm.stopPrank(); } + + function testRequestGuardian() public { + skipIfZkSync(); + + setUp(); + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.NONE + ); + + vm.startPrank(deployer); + recoveryController.requestGuardian(guardian); + vm.stopPrank(); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + } + + function testMultipleGuardianRequests() public { + skipIfZkSync(); + + address anotherGuardian = vm.addr(9); + setUp(); + vm.startPrank(deployer); + recoveryController.requestGuardian(guardian); + recoveryController.requestGuardian(anotherGuardian); // Assuming anotherGuardian is defined + vm.stopPrank(); + + require( + recoveryController.guardians(guardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + require( + recoveryController.guardians(anotherGuardian) == + RecoveryController.GuardianStatus.REQUESTED + ); + } + + // function testRequestGuardianNotOwner() public { + // setUp(); + + // require( + // recoveryController.guardians(guardian) == + // recoveryController.GuardianStatus.NONE + // ); + + // vm.startPrank(receiver); + // recoveryController.requestGuardian(guardian); + // vm.stopPrank(); + + // require( + // recoveryController.guardians(guardian) == + // recoveryController.GuardianStatus.NONE + // ); + // } } From 2b8f8a4e053f0dd1f30cb75ef3c71142b970b122 Mon Sep 17 00:00:00 2001 From: wshino Date: Sat, 12 Oct 2024 23:45:00 +0900 Subject: [PATCH 4/5] Add general test cases --- .../EmailAccountRecovery_general.t.sol | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_general.t.sol diff --git a/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_general.t.sol b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_general.t.sol new file mode 100644 index 00000000..7f574df4 --- /dev/null +++ b/packages/contracts/test/EmailAccountRecovery/EmailAccountRecovery_general.t.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.12; + +import "forge-std/Test.sol"; +import "../helpers/StructHelper.sol"; + +contract EmailAccountRecoveryTest_general is Test, StructHelper { + function test_erifier() public { + assertEq(recoveryController.verifier(), address(verifier)); + } + + function test_DKIM() public { + assertEq(recoveryController.dkim(), address(dkim)); + } + + function test_EmailAuthImplementation() public { + assertEq(recoveryController.emailAuthImplementation(), address(emailAuth)); + } +} \ No newline at end of file From 0d317d320231f11a74de6a19a87e62ffe77dc319 Mon Sep 17 00:00:00 2001 From: wshino Date: Wed, 16 Oct 2024 21:27:13 +0900 Subject: [PATCH 5/5] Add upgrade test --- packages/contracts/test/EmailAuth.t.sol | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/contracts/test/EmailAuth.t.sol b/packages/contracts/test/EmailAuth.t.sol index d4010ff5..ac7d1341 100644 --- a/packages/contracts/test/EmailAuth.t.sol +++ b/packages/contracts/test/EmailAuth.t.sol @@ -492,4 +492,36 @@ contract EmailAuthTest is StructHelper { vm.expectRevert("only controller"); emailAuth.setTimestampCheckEnabled(false); } + + function testUpgradeEmailAuth() public { + vm.startPrank(deployer); + + // Deploy new implementation + EmailAuth newImplementation = new EmailAuth(); + + // Execute upgrade using proxy + // Upgrade implementation through proxy contract + ERC1967Proxy proxy = new ERC1967Proxy( + address(emailAuth), + abi.encodeCall( + emailAuth.initialize, + (deployer, accountSalt, deployer) + ) + ); + EmailAuth emailAuthProxy = EmailAuth(payable(proxy)); + bytes32 beforeAccountSalt = emailAuthProxy.accountSalt(); + + // Upgrade to new implementation through proxy + emailAuthProxy.upgradeToAndCall( + address(newImplementation), + new bytes(0) + ); + + bytes32 afterAccountSalt = emailAuthProxy.accountSalt(); + + // Verify the upgrade + assertEq(beforeAccountSalt, afterAccountSalt); + + vm.stopPrank(); + } }