From 780a8a0587c72b345d9ecac951c5a7ffb248f875 Mon Sep 17 00:00:00 2001 From: zeroknots Date: Tue, 25 Jun 2024 12:02:09 +0100 Subject: [PATCH 1/6] fix: eip 712 --- .gitignore | 1 + src/lib/AttestationLib.sol | 16 +++++++++++++++- test/ModuleRegistration.t.sol | 2 -- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 259c9313..67e182fe 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # Compiler files cache/ out/ +out-optimized/ .DS_Store docs/ diff --git a/src/lib/AttestationLib.sol b/src/lib/AttestationLib.sol index b136d9ce..1b0f4406 100644 --- a/src/lib/AttestationLib.sol +++ b/src/lib/AttestationLib.sol @@ -64,7 +64,21 @@ library AttestationLib { * @return _hash the hash */ function hash(AttestationRequest calldata request, uint256 nonce) internal pure returns (bytes32 _hash) { - _hash = keccak256(abi.encode(ATTEST_TYPEHASH, keccak256(abi.encode(request)), nonce)); + _hash = keccak256( + abi.encode( + ATTEST_TYPEHASH, + keccak256( + abi.encode( + ATTEST_REQUEST_TYPEHASH, + request.moduleAddr, + request.expirationTime, + keccak256(request.data), + keccak256(abi.encodePacked(request.moduleTypes)) + ) + ), + nonce + ) + ); } /** diff --git a/test/ModuleRegistration.t.sol b/test/ModuleRegistration.t.sol index 62424295..13558742 100644 --- a/test/ModuleRegistration.t.sol +++ b/test/ModuleRegistration.t.sol @@ -50,7 +50,6 @@ contract ModuleRegistrationTest is BaseTest { invalidUID = ResolverUID.wrap("1"); vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, invalidUID)); registry.registerModule(invalidUID, address(newModule), ""); - } function test_WhenRegisteringAModuleOnAValidResolverUID() external prankWithAccount(moduleDev1) { @@ -67,7 +66,6 @@ contract ModuleRegistrationTest is BaseTest { vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, ResolverUID.wrap(bytes32("foobar")))); registry.registerModule(ResolverUID.wrap(bytes32("foobar")), address(newModule), ""); - } function test_WhenRegisteringTwoModulesWithTheSameBytecode() external prankWithAccount(moduleDev1) { From 8e1535f01acc51a96910d50f4a1ba3bbd94b7358 Mon Sep 17 00:00:00 2001 From: zeroknots Date: Tue, 25 Jun 2024 17:03:03 +0100 Subject: [PATCH 2/6] add array --- src/lib/AttestationLib.sol | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/lib/AttestationLib.sol b/src/lib/AttestationLib.sol index 1b0f4406..93f16bad 100644 --- a/src/lib/AttestationLib.sol +++ b/src/lib/AttestationLib.sol @@ -88,7 +88,23 @@ library AttestationLib { * @return _hash the hash */ function hash(AttestationRequest[] calldata requests, uint256 nonce) internal pure returns (bytes32 _hash) { - _hash = keccak256(abi.encode(ATTEST_ARRAY_TYPEHASH, keccak256(abi.encode(requests)), nonce)); + bytes memory concatinatedAttestations; + + uint256 length = requests.length; + for (uint256 i; i < length; i++) { + concatinatedAttestations = abi.encodePacked( + concatinatedAttestations, // concat previous + abi.encode( + ATTEST_REQUEST_TYPEHASH, + requests[i].moduleAddr, + requests[i].expirationTime, + keccak256(requests[i].data), + keccak256(abi.encodePacked(requests[i].moduleTypes)) + ) + ); + } + + _hash = keccak256(abi.encode(ATTEST_ARRAY_TYPEHASH, keccak256(concatinatedAttestations), nonce)); } /** From e7a95fadb8d4e1016e415847928c1f00778d0ef1 Mon Sep 17 00:00:00 2001 From: zeroknots Date: Tue, 25 Jun 2024 17:03:49 +0100 Subject: [PATCH 3/6] fix test errors --- test/ModuleRegistration.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/ModuleRegistration.t.sol b/test/ModuleRegistration.t.sol index 13558742..153a19cd 100644 --- a/test/ModuleRegistration.t.sol +++ b/test/ModuleRegistration.t.sol @@ -45,11 +45,11 @@ contract ModuleRegistrationTest is BaseTest { ResolverUID invalidUID = ResolverUID.wrap(hex"00"); vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, invalidUID)); - registry.registerModule(invalidUID, address(newModule), ""); + registry.registerModule(invalidUID, address(newModule), "",""); invalidUID = ResolverUID.wrap("1"); vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, invalidUID)); - registry.registerModule(invalidUID, address(newModule), ""); + registry.registerModule(invalidUID, address(newModule), "",""); } function test_WhenRegisteringAModuleOnAValidResolverUID() external prankWithAccount(moduleDev1) { @@ -65,7 +65,7 @@ contract ModuleRegistrationTest is BaseTest { MockModule newModule = new MockModule(); vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, ResolverUID.wrap(bytes32("foobar")))); - registry.registerModule(ResolverUID.wrap(bytes32("foobar")), address(newModule), ""); + registry.registerModule(ResolverUID.wrap(bytes32("foobar")), address(newModule), "", ""); } function test_WhenRegisteringTwoModulesWithTheSameBytecode() external prankWithAccount(moduleDev1) { From d7c4d12063020bb5d4d8ba82b426b0500fad456d Mon Sep 17 00:00:00 2001 From: zeroknots Date: Tue, 25 Jun 2024 17:10:47 +0100 Subject: [PATCH 4/6] added names to EIP712 hashes --- src/lib/AttestationLib.sol | 13 +++++++------ test/ModuleRegistration.t.sol | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/lib/AttestationLib.sol b/src/lib/AttestationLib.sol index 93f16bad..e1a35637 100644 --- a/src/lib/AttestationLib.sol +++ b/src/lib/AttestationLib.sol @@ -6,18 +6,19 @@ import { SSTORE2 } from "solady/utils/SSTORE2.sol"; library AttestationLib { // The hash of the data type used to relay calls to the attest function. It's the value of - bytes32 internal constant ATTEST_REQUEST_TYPEHASH = keccak256("AttestationRequest(address,uint48,bytes,uint256[])"); + bytes32 internal constant ATTEST_REQUEST_TYPEHASH = + keccak256("AttestationRequest(address moduleAddress,uint48 expirationTime,bytes data,uint256[] moduleTypes)"); bytes32 internal constant ATTEST_TYPEHASH = - keccak256("SignedAttestationRequest(AttestationRequest,uint256 nonce)AttestationRequest(address,uint48,bytes,uint256[])"); + keccak256("SignedAttestationRequest(AttestationRequest,uint256 nonce)AttestationRequest(address moduleAddress,uint48 expirationTime,bytes data,uint256[] moduleTypes)"); bytes32 internal constant ATTEST_ARRAY_TYPEHASH = - keccak256("SignedAttestationRequests(AttestationRequest[],uint256 nonce)AttestationRequest(address,uint48,bytes,uint256[])"); + keccak256("SignedAttestationRequests(AttestationRequest[],uint256 nonce)AttestationRequest(address moduleAddress,uint48 expirationTimme,bytes data,uint256[] moduleTypes)"); // The hash of the data type used to relay calls to the revoke function. It's the value of - bytes32 internal constant REVOKE_REQUEST_TYPEHASH = keccak256("RevocationRequest(address)"); + bytes32 internal constant REVOKE_REQUEST_TYPEHASH = keccak256("RevocationRequest(address moduleAddress)"); bytes32 internal constant REVOKE_TYPEHASH = - keccak256("SignedRevocationRequest(RevocationRequest,uint256 nonce)RevocationRequest(address)"); + keccak256("SignedRevocationRequest(RevocationRequest,uint256 nonce)RevocationRequest(address moduleAddress)"); bytes32 internal constant REVOKE_ARRAY_TYPEHASH = - keccak256("SignedRevocationRequests(RevocationRequest[],uint256 nonce)RevocationRequest(address)"); + keccak256("SignedRevocationRequests(RevocationRequest[],uint256 nonce)RevocationRequest(address moduleAddress)"); /** * Helper function to SSTORE2 read an attestation diff --git a/test/ModuleRegistration.t.sol b/test/ModuleRegistration.t.sol index 153a19cd..89f61d25 100644 --- a/test/ModuleRegistration.t.sol +++ b/test/ModuleRegistration.t.sol @@ -45,11 +45,11 @@ contract ModuleRegistrationTest is BaseTest { ResolverUID invalidUID = ResolverUID.wrap(hex"00"); vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, invalidUID)); - registry.registerModule(invalidUID, address(newModule), "",""); + registry.registerModule(invalidUID, address(newModule), "", ""); invalidUID = ResolverUID.wrap("1"); vm.expectRevert(abi.encodeWithSelector(IRegistry.InvalidResolverUID.selector, invalidUID)); - registry.registerModule(invalidUID, address(newModule), "",""); + registry.registerModule(invalidUID, address(newModule), "", ""); } function test_WhenRegisteringAModuleOnAValidResolverUID() external prankWithAccount(moduleDev1) { From 89ae6bc00d0c6bb4c568553d8e12d1f1f4a08033 Mon Sep 17 00:00:00 2001 From: zeroknots Date: Wed, 26 Jun 2024 11:17:50 +0100 Subject: [PATCH 5/6] fix: EIP712 according to michal's feedback --- src/lib/AttestationLib.sol | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/lib/AttestationLib.sol b/src/lib/AttestationLib.sol index e1a35637..a34462b6 100644 --- a/src/lib/AttestationLib.sol +++ b/src/lib/AttestationLib.sol @@ -8,17 +8,19 @@ library AttestationLib { // The hash of the data type used to relay calls to the attest function. It's the value of bytes32 internal constant ATTEST_REQUEST_TYPEHASH = keccak256("AttestationRequest(address moduleAddress,uint48 expirationTime,bytes data,uint256[] moduleTypes)"); - bytes32 internal constant ATTEST_TYPEHASH = - keccak256("SignedAttestationRequest(AttestationRequest,uint256 nonce)AttestationRequest(address moduleAddress,uint48 expirationTime,bytes data,uint256[] moduleTypes)"); - bytes32 internal constant ATTEST_ARRAY_TYPEHASH = - keccak256("SignedAttestationRequests(AttestationRequest[],uint256 nonce)AttestationRequest(address moduleAddress,uint48 expirationTimme,bytes data,uint256[] moduleTypes)"); + bytes32 internal constant ATTEST_TYPEHASH = keccak256( + "SignedAttestationRequest(AttestationRequest,uint256 nonce)AttestationRequest(address moduleAddress,uint48 expirationTime,bytes data,uint256[] moduleTypes)" + ); + bytes32 internal constant ATTEST_ARRAY_TYPEHASH = keccak256( + "SignedAttestationRequests(AttestationRequest[],uint256 nonce)AttestationRequest(address moduleAddress,uint48 expirationTimme,bytes data,uint256[] moduleTypes)" + ); // The hash of the data type used to relay calls to the revoke function. It's the value of bytes32 internal constant REVOKE_REQUEST_TYPEHASH = keccak256("RevocationRequest(address moduleAddress)"); bytes32 internal constant REVOKE_TYPEHASH = keccak256("SignedRevocationRequest(RevocationRequest,uint256 nonce)RevocationRequest(address moduleAddress)"); bytes32 internal constant REVOKE_ARRAY_TYPEHASH = - keccak256("SignedRevocationRequests(RevocationRequest[],uint256 nonce)RevocationRequest(address moduleAddress)"); + keccak256("SignedRevocationRequests(RevocationRequest[],uint256 nonce)RevocationRequest(address moduleAddressq)"); /** * Helper function to SSTORE2 read an attestation @@ -95,12 +97,14 @@ library AttestationLib { for (uint256 i; i < length; i++) { concatinatedAttestations = abi.encodePacked( concatinatedAttestations, // concat previous - abi.encode( - ATTEST_REQUEST_TYPEHASH, - requests[i].moduleAddr, - requests[i].expirationTime, - keccak256(requests[i].data), - keccak256(abi.encodePacked(requests[i].moduleTypes)) + keccak256( + abi.encode( + ATTEST_REQUEST_TYPEHASH, + requests[i].moduleAddr, + requests[i].expirationTime, + keccak256(requests[i].data), + keccak256(abi.encodePacked(requests[i].moduleTypes)) + ) ) ); } From 5e1d62ce429c976afa53677c461b738b449e298d Mon Sep 17 00:00:00 2001 From: zeroknots Date: Wed, 26 Jun 2024 11:22:39 +0100 Subject: [PATCH 6/6] fix: adding EIP712 to revocation request --- src/lib/AttestationLib.sol | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/lib/AttestationLib.sol b/src/lib/AttestationLib.sol index a34462b6..3ce69fd9 100644 --- a/src/lib/AttestationLib.sol +++ b/src/lib/AttestationLib.sol @@ -119,7 +119,7 @@ library AttestationLib { * @return _hash the hash */ function hash(RevocationRequest calldata request, uint256 nonce) internal pure returns (bytes32 _hash) { - _hash = keccak256(abi.encode(REVOKE_TYPEHASH, keccak256(abi.encode(request)), nonce)); + _hash = keccak256(abi.encode(REVOKE_TYPEHASH, keccak256(abi.encode(REVOKE_REQUEST_TYPEHASH, request.moduleAddr)), nonce)); } /** @@ -129,6 +129,16 @@ library AttestationLib { * @return _hash the hash */ function hash(RevocationRequest[] calldata requests, uint256 nonce) internal pure returns (bytes32 _hash) { - _hash = keccak256(abi.encode(REVOKE_ARRAY_TYPEHASH, keccak256(abi.encode(requests)), nonce)); + bytes memory concatinatedAttestations; + + uint256 length = requests.length; + for (uint256 i; i < length; i++) { + concatinatedAttestations = abi.encodePacked( + concatinatedAttestations, // concat previous + keccak256(abi.encode(REVOKE_REQUEST_TYPEHASH, requests[i].moduleAddr)) + ); + } + + _hash = keccak256(abi.encode(REVOKE_ARRAY_TYPEHASH, keccak256(concatinatedAttestations), nonce)); } }