From b59b1b5cea066a74467aa80525f6c7e9dce2301b Mon Sep 17 00:00:00 2001 From: saucepoint Date: Fri, 2 Aug 2024 22:02:45 -0400 Subject: [PATCH 1/8] WOOF WOOF WOOF BARK BARK WOOF BARK --- ...tionManager_multicall_initialize_mint.snap | 2 +- .forge-snapshots/PositionManager_permit.snap | 2 +- ...PositionManager_permit_secondPosition.snap | 2 +- .../PositionManager_permit_twice.snap | 2 +- remappings.txt | 1 - src/base/EIP712.sol | 48 +++++++++++++++++++ src/base/ERC721Permit.sol | 9 +--- src/interfaces/IEIP712.sol | 6 +++ src/interfaces/IERC721Permit.sol | 4 -- test/ERC721Permit.t.sol | 2 +- test/position-managers/Permit.t.sol | 2 +- 11 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 src/base/EIP712.sol create mode 100644 src/interfaces/IEIP712.sol diff --git a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap index 2f927ade..15fd1f72 100644 --- a/.forge-snapshots/PositionManager_multicall_initialize_mint.snap +++ b/.forge-snapshots/PositionManager_multicall_initialize_mint.snap @@ -1 +1 @@ -416516 \ No newline at end of file +416538 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit.snap b/.forge-snapshots/PositionManager_permit.snap index b2faa7cc..20d48590 100644 --- a/.forge-snapshots/PositionManager_permit.snap +++ b/.forge-snapshots/PositionManager_permit.snap @@ -1 +1 @@ -79585 \ No newline at end of file +79660 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_secondPosition.snap b/.forge-snapshots/PositionManager_permit_secondPosition.snap index 60f10b03..5a205f6d 100644 --- a/.forge-snapshots/PositionManager_permit_secondPosition.snap +++ b/.forge-snapshots/PositionManager_permit_secondPosition.snap @@ -1 +1 @@ -62497 \ No newline at end of file +62572 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_twice.snap b/.forge-snapshots/PositionManager_permit_twice.snap index 3df69310..660ed1ad 100644 --- a/.forge-snapshots/PositionManager_permit_twice.snap +++ b/.forge-snapshots/PositionManager_permit_twice.snap @@ -1 +1 @@ -45397 \ No newline at end of file +45472 \ No newline at end of file diff --git a/remappings.txt b/remappings.txt index c7951f83..e7868fe9 100644 --- a/remappings.txt +++ b/remappings.txt @@ -1,5 +1,4 @@ @uniswap/v4-core/=lib/v4-core/ -@openzeppelin/=lib/v4-core/lib/openzeppelin-contracts/ forge-gas-snapshot/=lib/v4-core/lib/forge-gas-snapshot/src/ ds-test/=lib/v4-core/lib/forge-std/lib/ds-test/src/ forge-std/=lib/v4-core/lib/forge-std/src/ diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol new file mode 100644 index 00000000..05fe9ae0 --- /dev/null +++ b/src/base/EIP712.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IEIP712} from "../interfaces/IEIP712.sol"; + +/// @notice Generic EIP712 implementation +/// @dev Maintains cross-chain replay protection in the event of a fork +/// @dev Reference: https://github.com/Uniswap/permit2/blob/main/src/EIP712.sol +/// @dev Reference: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/EIP712.sol +contract EIP712 is IEIP712 { + // Cache the domain separator as an immutable value, but also store the chain id that it + // corresponds to, in order to invalidate the cached domain separator if the chain id changes. + bytes32 private immutable _CACHED_DOMAIN_SEPARATOR; + uint256 private immutable _CACHED_CHAIN_ID; + address private immutable _CACHED_THIS; + bytes32 private immutable _HASHED_NAME; + bytes32 private immutable _HASHED_VERSION; + + bytes32 private constant _TYPE_HASH = + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + + constructor(string memory name, string memory version) { + _HASHED_NAME = keccak256(bytes(name)); + _HASHED_VERSION = keccak256(bytes(version)); + + _CACHED_CHAIN_ID = block.chainid; + _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(); + _CACHED_THIS = address(this); + } + + /// @notice Returns the domain separator for the current chain. + /// @dev Uses cached version if chainid and address are unchanged from construction. + function DOMAIN_SEPARATOR() public view override returns (bytes32) { + return address(this) == _CACHED_THIS && block.chainid == _CACHED_CHAIN_ID + ? _CACHED_DOMAIN_SEPARATOR + : _buildDomainSeparator(); + } + + /// @notice Builds a domain separator using the current chainId and contract address. + function _buildDomainSeparator() private view returns (bytes32) { + return keccak256(abi.encode(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION, block.chainid, address(this))); + } + + /// @notice Creates an EIP-712 typed data hash + function _hashTypedData(bytes32 dataHash) internal view returns (bytes32) { + return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR(), dataHash)); + } +} diff --git a/src/base/ERC721Permit.sol b/src/base/ERC721Permit.sol index 7a4acc4d..316068db 100644 --- a/src/base/ERC721Permit.sol +++ b/src/base/ERC721Permit.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.24; import {IERC721} from "forge-std/interfaces/IERC721.sol"; import {ERC721} from "solmate/src/tokens/ERC721.sol"; -import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; +import {EIP712} from "./EIP712.sol"; import {ERC721PermitHashLibrary} from "../libraries/ERC721PermitHash.sol"; import {SignatureVerification} from "permit2/src/libraries/SignatureVerification.sol"; @@ -21,11 +21,6 @@ abstract contract ERC721Permit is ERC721, IERC721Permit, EIP712, UnorderedNonce EIP712(name_, version_) {} - /// @inheritdoc IERC721Permit - function DOMAIN_SEPARATOR() external view returns (bytes32) { - return _domainSeparatorV4(); - } - /// @inheritdoc IERC721Permit function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, bytes calldata signature) external @@ -37,7 +32,7 @@ abstract contract ERC721Permit is ERC721, IERC721Permit, EIP712, UnorderedNonce if (spender == owner) revert NoSelfPermit(); bytes32 hash = ERC721PermitHashLibrary.hash(spender, tokenId, nonce, deadline); - signature.verify(_hashTypedDataV4(hash), owner); + signature.verify(_hashTypedData(hash), owner); _useUnorderedNonce(owner, nonce); _approve(owner, spender, tokenId); diff --git a/src/interfaces/IEIP712.sol b/src/interfaces/IEIP712.sol new file mode 100644 index 00000000..355c443f --- /dev/null +++ b/src/interfaces/IEIP712.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface IEIP712 { + function DOMAIN_SEPARATOR() external view returns (bytes32); +} diff --git a/src/interfaces/IERC721Permit.sol b/src/interfaces/IERC721Permit.sol index 646c1af0..29a6fe2a 100644 --- a/src/interfaces/IERC721Permit.sol +++ b/src/interfaces/IERC721Permit.sol @@ -8,10 +8,6 @@ interface IERC721Permit { error NoSelfPermit(); error Unauthorized(); - /// @notice The domain separator used in the permit signature - /// @return The domain seperator used in encoding of permit signature - function DOMAIN_SEPARATOR() external view returns (bytes32); - /// @notice Approve of a specific token ID for spending by spender via signature /// @param spender The account that is being approved /// @param tokenId The ID of the token that is being approved for spending diff --git a/test/ERC721Permit.t.sol b/test/ERC721Permit.t.sol index d7126223..d2a26434 100644 --- a/test/ERC721Permit.t.sol +++ b/test/ERC721Permit.t.sol @@ -69,7 +69,7 @@ contract ERC721PermitTest is Test { function test_domainSeparator() public view { assertEq( - IERC721Permit(address(erc721Permit)).DOMAIN_SEPARATOR(), + erc721Permit.DOMAIN_SEPARATOR(), keccak256( abi.encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), diff --git a/test/position-managers/Permit.t.sol b/test/position-managers/Permit.t.sol index 7da608a2..01655e38 100644 --- a/test/position-managers/Permit.t.sol +++ b/test/position-managers/Permit.t.sol @@ -60,7 +60,7 @@ contract PermitTest is Test, PosmTestSetup { function test_domainSeparator() public view { assertEq( - IERC721Permit(address(lpm)).DOMAIN_SEPARATOR(), + ERC721Permit(address(lpm)).DOMAIN_SEPARATOR(), keccak256( abi.encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), From 4cd765a7c9399ab6d935ab59509df8d032e3c974 Mon Sep 17 00:00:00 2001 From: saucepoint Date: Fri, 2 Aug 2024 23:55:13 -0400 Subject: [PATCH 2/8] remove address(this) check --- .forge-snapshots/PositionManager_permit.snap | 2 +- .forge-snapshots/PositionManager_permit_secondPosition.snap | 2 +- .forge-snapshots/PositionManager_permit_twice.snap | 2 +- src/base/EIP712.sol | 4 +--- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.forge-snapshots/PositionManager_permit.snap b/.forge-snapshots/PositionManager_permit.snap index 20d48590..a03aeb87 100644 --- a/.forge-snapshots/PositionManager_permit.snap +++ b/.forge-snapshots/PositionManager_permit.snap @@ -1 +1 @@ -79660 \ No newline at end of file +79624 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_secondPosition.snap b/.forge-snapshots/PositionManager_permit_secondPosition.snap index 5a205f6d..b08d0231 100644 --- a/.forge-snapshots/PositionManager_permit_secondPosition.snap +++ b/.forge-snapshots/PositionManager_permit_secondPosition.snap @@ -1 +1 @@ -62572 \ No newline at end of file +62536 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_twice.snap b/.forge-snapshots/PositionManager_permit_twice.snap index 660ed1ad..6c9b3ad3 100644 --- a/.forge-snapshots/PositionManager_permit_twice.snap +++ b/.forge-snapshots/PositionManager_permit_twice.snap @@ -1 +1 @@ -45472 \ No newline at end of file +45436 \ No newline at end of file diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol index 05fe9ae0..82ddff0e 100644 --- a/src/base/EIP712.sol +++ b/src/base/EIP712.sol @@ -12,7 +12,6 @@ contract EIP712 is IEIP712 { // corresponds to, in order to invalidate the cached domain separator if the chain id changes. bytes32 private immutable _CACHED_DOMAIN_SEPARATOR; uint256 private immutable _CACHED_CHAIN_ID; - address private immutable _CACHED_THIS; bytes32 private immutable _HASHED_NAME; bytes32 private immutable _HASHED_VERSION; @@ -25,13 +24,12 @@ contract EIP712 is IEIP712 { _CACHED_CHAIN_ID = block.chainid; _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(); - _CACHED_THIS = address(this); } /// @notice Returns the domain separator for the current chain. /// @dev Uses cached version if chainid and address are unchanged from construction. function DOMAIN_SEPARATOR() public view override returns (bytes32) { - return address(this) == _CACHED_THIS && block.chainid == _CACHED_CHAIN_ID + return block.chainid == _CACHED_CHAIN_ID ? _CACHED_DOMAIN_SEPARATOR : _buildDomainSeparator(); } From cd8b7605a6eb8b9480fdc1e76b353de85692e87d Mon Sep 17 00:00:00 2001 From: saucepoint Date: Fri, 2 Aug 2024 23:57:09 -0400 Subject: [PATCH 3/8] forge fmt --- src/base/EIP712.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol index 82ddff0e..73f5b636 100644 --- a/src/base/EIP712.sol +++ b/src/base/EIP712.sol @@ -29,9 +29,7 @@ contract EIP712 is IEIP712 { /// @notice Returns the domain separator for the current chain. /// @dev Uses cached version if chainid and address are unchanged from construction. function DOMAIN_SEPARATOR() public view override returns (bytes32) { - return block.chainid == _CACHED_CHAIN_ID - ? _CACHED_DOMAIN_SEPARATOR - : _buildDomainSeparator(); + return block.chainid == _CACHED_CHAIN_ID ? _CACHED_DOMAIN_SEPARATOR : _buildDomainSeparator(); } /// @notice Builds a domain separator using the current chainId and contract address. From 9419e44ec4c8cc17e858362ee1ff7ef750bf3ad9 Mon Sep 17 00:00:00 2001 From: saucepoint Date: Sat, 3 Aug 2024 00:15:28 -0400 Subject: [PATCH 4/8] discard version from EIP712 --- .forge-snapshots/PositionManager_permit.snap | 2 +- .../PositionManager_permit_secondPosition.snap | 2 +- .forge-snapshots/PositionManager_permit_twice.snap | 2 +- src/PositionManager.sol | 2 +- src/base/EIP712.sol | 8 +++----- src/base/ERC721Permit.sol | 5 +---- test/ERC721Permit.t.sol | 6 ++---- test/mocks/MockERC721Permit.sol | 2 +- test/position-managers/Permit.t.sol | 3 +-- 9 files changed, 12 insertions(+), 20 deletions(-) diff --git a/.forge-snapshots/PositionManager_permit.snap b/.forge-snapshots/PositionManager_permit.snap index a03aeb87..c3bc0823 100644 --- a/.forge-snapshots/PositionManager_permit.snap +++ b/.forge-snapshots/PositionManager_permit.snap @@ -1 +1 @@ -79624 \ No newline at end of file +79636 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_secondPosition.snap b/.forge-snapshots/PositionManager_permit_secondPosition.snap index b08d0231..fa2386f9 100644 --- a/.forge-snapshots/PositionManager_permit_secondPosition.snap +++ b/.forge-snapshots/PositionManager_permit_secondPosition.snap @@ -1 +1 @@ -62536 \ No newline at end of file +62524 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_twice.snap b/.forge-snapshots/PositionManager_permit_twice.snap index 6c9b3ad3..7cfd8d74 100644 --- a/.forge-snapshots/PositionManager_permit_twice.snap +++ b/.forge-snapshots/PositionManager_permit_twice.snap @@ -1 +1 @@ -45436 \ No newline at end of file +45412 \ No newline at end of file diff --git a/src/PositionManager.sol b/src/PositionManager.sol index 88873384..e9e1a4c0 100644 --- a/src/PositionManager.sol +++ b/src/PositionManager.sol @@ -60,7 +60,7 @@ contract PositionManager is constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2) BaseActionsRouter(_poolManager) Permit2Forwarder(_permit2) - ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM", "1") + ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM") {} /// @notice Reverts if the deadline has passed diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol index 73f5b636..a4074350 100644 --- a/src/base/EIP712.sol +++ b/src/base/EIP712.sol @@ -13,14 +13,12 @@ contract EIP712 is IEIP712 { bytes32 private immutable _CACHED_DOMAIN_SEPARATOR; uint256 private immutable _CACHED_CHAIN_ID; bytes32 private immutable _HASHED_NAME; - bytes32 private immutable _HASHED_VERSION; bytes32 private constant _TYPE_HASH = - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); - constructor(string memory name, string memory version) { + constructor(string memory name) { _HASHED_NAME = keccak256(bytes(name)); - _HASHED_VERSION = keccak256(bytes(version)); _CACHED_CHAIN_ID = block.chainid; _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(); @@ -34,7 +32,7 @@ contract EIP712 is IEIP712 { /// @notice Builds a domain separator using the current chainId and contract address. function _buildDomainSeparator() private view returns (bytes32) { - return keccak256(abi.encode(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION, block.chainid, address(this))); + return keccak256(abi.encode(_TYPE_HASH, _HASHED_NAME, block.chainid, address(this))); } /// @notice Creates an EIP-712 typed data hash diff --git a/src/base/ERC721Permit.sol b/src/base/ERC721Permit.sol index 316068db..1605b79c 100644 --- a/src/base/ERC721Permit.sol +++ b/src/base/ERC721Permit.sol @@ -16,10 +16,7 @@ abstract contract ERC721Permit is ERC721, IERC721Permit, EIP712, UnorderedNonce using SignatureVerification for bytes; /// @notice Computes the nameHash and versionHash - constructor(string memory name_, string memory symbol_, string memory version_) - ERC721(name_, symbol_) - EIP712(name_, version_) - {} + constructor(string memory name_, string memory symbol_) ERC721(name_, symbol_) EIP712(name_) {} /// @inheritdoc IERC721Permit function permit(address spender, uint256 tokenId, uint256 deadline, uint256 nonce, bytes calldata signature) diff --git a/test/ERC721Permit.t.sol b/test/ERC721Permit.t.sol index d2a26434..4048ad37 100644 --- a/test/ERC721Permit.t.sol +++ b/test/ERC721Permit.t.sol @@ -19,13 +19,12 @@ contract ERC721PermitTest is Test { string constant name = "Mock ERC721Permit"; string constant symbol = "MOCK721"; - string constant version = "1"; function setUp() public { (alice, alicePK) = makeAddrAndKey("ALICE"); (bob, bobPK) = makeAddrAndKey("BOB"); - erc721Permit = new MockERC721Permit(name, symbol, version); + erc721Permit = new MockERC721Permit(name, symbol); } // --- Test the overriden approval --- @@ -72,9 +71,8 @@ contract ERC721PermitTest is Test { erc721Permit.DOMAIN_SEPARATOR(), keccak256( abi.encode( - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"), keccak256(bytes(name)), - keccak256(bytes(version)), block.chainid, address(erc721Permit) ) diff --git a/test/mocks/MockERC721Permit.sol b/test/mocks/MockERC721Permit.sol index d6e82a3e..ba1c732d 100644 --- a/test/mocks/MockERC721Permit.sol +++ b/test/mocks/MockERC721Permit.sol @@ -6,7 +6,7 @@ import {ERC721Permit} from "../../src/base/ERC721Permit.sol"; contract MockERC721Permit is ERC721Permit { uint256 public lastTokenId; - constructor(string memory name, string memory symbol, string memory version) ERC721Permit(name, symbol, version) {} + constructor(string memory name, string memory symbol) ERC721Permit(name, symbol) {} function tokenURI(uint256) public pure override returns (string memory) { return ""; diff --git a/test/position-managers/Permit.t.sol b/test/position-managers/Permit.t.sol index 01655e38..91a22a25 100644 --- a/test/position-managers/Permit.t.sol +++ b/test/position-managers/Permit.t.sol @@ -63,9 +63,8 @@ contract PermitTest is Test, PosmTestSetup { ERC721Permit(address(lpm)).DOMAIN_SEPARATOR(), keccak256( abi.encode( - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"), keccak256("Uniswap V4 Positions NFT"), // storage is private on EIP712.sol so we need to hardcode these - keccak256("1"), block.chainid, address(lpm) ) From 27d394e70679e2f05406e91bec3951f2a40c816f Mon Sep 17 00:00:00 2001 From: saucepoint <98790946+saucepoint@users.noreply.github.com> Date: Sat, 3 Aug 2024 00:31:32 -0400 Subject: [PATCH 5/8] Update src/base/EIP712.sol Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> --- src/base/EIP712.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol index a4074350..6cf8d171 100644 --- a/src/base/EIP712.sol +++ b/src/base/EIP712.sol @@ -25,7 +25,7 @@ contract EIP712 is IEIP712 { } /// @notice Returns the domain separator for the current chain. - /// @dev Uses cached version if chainid and address are unchanged from construction. + /// @dev Uses cached version if chainid is unchanged from construction. function DOMAIN_SEPARATOR() public view override returns (bytes32) { return block.chainid == _CACHED_CHAIN_ID ? _CACHED_DOMAIN_SEPARATOR : _buildDomainSeparator(); } From e553340e69597e497d3f999ab8d3a271761cf567 Mon Sep 17 00:00:00 2001 From: saucepoint Date: Sat, 3 Aug 2024 00:48:25 -0400 Subject: [PATCH 6/8] natspec for delegatecall error --- src/base/EIP712.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol index 6cf8d171..6c0148ab 100644 --- a/src/base/EIP712.sol +++ b/src/base/EIP712.sol @@ -5,6 +5,7 @@ import {IEIP712} from "../interfaces/IEIP712.sol"; /// @notice Generic EIP712 implementation /// @dev Maintains cross-chain replay protection in the event of a fork +/// @dev Should not be delegatecall'd because DOMAIN_SEPARATOR returns the cached hash and does not recompute with the delegatecallers address /// @dev Reference: https://github.com/Uniswap/permit2/blob/main/src/EIP712.sol /// @dev Reference: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/EIP712.sol contract EIP712 is IEIP712 { From 4a74d8d5e46ae85d795cc23e5fbd5cdeeb55c6a3 Mon Sep 17 00:00:00 2001 From: saucepoint Date: Sat, 3 Aug 2024 22:15:36 -0400 Subject: [PATCH 7/8] assembly optimization, pr nits --- .forge-snapshots/PositionManager_permit.snap | 2 +- .../PositionManager_permit_secondPosition.snap | 2 +- .../PositionManager_permit_twice.snap | 2 +- src/base/EIP712.sol | 16 ++++++++++++---- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/.forge-snapshots/PositionManager_permit.snap b/.forge-snapshots/PositionManager_permit.snap index c3bc0823..8ebdf959 100644 --- a/.forge-snapshots/PositionManager_permit.snap +++ b/.forge-snapshots/PositionManager_permit.snap @@ -1 +1 @@ -79636 \ No newline at end of file +79566 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_secondPosition.snap b/.forge-snapshots/PositionManager_permit_secondPosition.snap index fa2386f9..972b19ad 100644 --- a/.forge-snapshots/PositionManager_permit_secondPosition.snap +++ b/.forge-snapshots/PositionManager_permit_secondPosition.snap @@ -1 +1 @@ -62524 \ No newline at end of file +62454 \ No newline at end of file diff --git a/.forge-snapshots/PositionManager_permit_twice.snap b/.forge-snapshots/PositionManager_permit_twice.snap index 7cfd8d74..a3347104 100644 --- a/.forge-snapshots/PositionManager_permit_twice.snap +++ b/.forge-snapshots/PositionManager_permit_twice.snap @@ -1 +1 @@ -45412 \ No newline at end of file +45342 \ No newline at end of file diff --git a/src/base/EIP712.sol b/src/base/EIP712.sol index 6c0148ab..5b4ea0f5 100644 --- a/src/base/EIP712.sol +++ b/src/base/EIP712.sol @@ -15,8 +15,8 @@ contract EIP712 is IEIP712 { uint256 private immutable _CACHED_CHAIN_ID; bytes32 private immutable _HASHED_NAME; - bytes32 private constant _TYPE_HASH = - keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); + /// @dev equal to keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)") + bytes32 private constant _TYPE_HASH = 0x8cad95687ba82c2ce50e74f7b754645e5117c3a5bec8151c0726d5857980a866; constructor(string memory name) { _HASHED_NAME = keccak256(bytes(name)); @@ -37,7 +37,15 @@ contract EIP712 is IEIP712 { } /// @notice Creates an EIP-712 typed data hash - function _hashTypedData(bytes32 dataHash) internal view returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR(), dataHash)); + function _hashTypedData(bytes32 dataHash) internal view returns (bytes32 digest) { + // equal to keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR(), dataHash)); + bytes32 domainSeparator = DOMAIN_SEPARATOR(); + assembly ("memory-safe") { + let fmp := mload(0x40) + mstore(fmp, hex"1901") + mstore(add(fmp, 0x02), domainSeparator) + mstore(add(fmp, 0x22), dataHash) + digest := keccak256(fmp, 0x42) + } } } From 0826c12c23f69c19b234f7247df565e124e449c1 Mon Sep 17 00:00:00 2001 From: saucepoint Date: Sat, 3 Aug 2024 22:15:52 -0400 Subject: [PATCH 8/8] test to validate assembly --- test/EIP712.t.sol | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 test/EIP712.t.sol diff --git a/test/EIP712.t.sol b/test/EIP712.t.sol new file mode 100644 index 00000000..badf6c9c --- /dev/null +++ b/test/EIP712.t.sol @@ -0,0 +1,47 @@ +//SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "forge-std/Test.sol"; + +import {EIP712} from "../src/base/EIP712.sol"; + +contract EIP712Test is EIP712, Test { + constructor() EIP712("EIP712Test") {} + + function setUp() public {} + + function test_domainSeparator() public view { + assertEq( + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"), + keccak256(bytes("EIP712Test")), + block.chainid, + address(this) + ) + ) + ); + } + + function test_hashTypedData() public view { + bytes32 dataHash = keccak256(abi.encodePacked("data")); + assertEq(_hashTypedData(dataHash), keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR(), dataHash))); + } + + function test_rebuildDomainSeparator() public { + uint256 chainId = 4444; + vm.chainId(chainId); + assertEq( + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"), + keccak256(bytes("EIP712Test")), + chainId, + address(this) + ) + ) + ); + } +}