Replace OZ EIP712 #256
Replace OZ EIP712 #256
Conversation
| 79585 No newline at end of file | ||
| 79660 |
|
upon additional reading from Uniswap/permit2#51 and OpenZeppelin/openzeppelin-contracts#2852 we do not need to check
|
Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com>
src/base/EIP712.sol
Outdated
| bytes32 private immutable _HASHED_NAME; | ||
|
|
||
| bytes32 private constant _TYPE_HASH = | ||
| keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); |
There was a problem hiding this comment.
why dont we want a version string? i thought part of the idea was that we might have multiple versions? @snreynolds
There was a problem hiding this comment.
it shaves a bit of gas and sara thinks we can version in name
There was a problem hiding this comment.
it shouldnt shave anything off gas except in the constructor? after that its not calculating the typehash right?
There was a problem hiding this comment.
oh you should hardcode this hash - solidity calculates constants at runtime not compile time so itll be recalculating every time
src/base/EIP712.sol
Outdated
| } | ||
|
|
||
| /// @notice Creates an EIP-712 typed data hash | ||
| function _hashTypedData(bytes32 dataHash) internal view returns (bytes32) { |
There was a problem hiding this comment.
ok i went through OZ's contracts to figure out why theirs is cheaper lol. If you rewrite this function as:
function _hashTypedData(bytes32 dataHash) internal view returns (bytes32 digest) {
bytes32 domainSeparator = DOMAIN_SEPARATOR();
assembly("memory-safe") {
let fmp := mload(0x40)
mstore(fmp, hex"19_01")
mstore(add(fmp, 0x02), domainSeparator)
mstore(add(fmp, 0x22), dataHash)
digest := keccak256(fmp, 0x42)
}
}
our gas will be in line with theirs again! Based on this function
| 79585 No newline at end of file | ||
| 79566 |
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.0; | ||
|
|
||
| interface IEIP712 { |
There was a problem hiding this comment.
I think IPositionManager should inherit this?
Related Issue
Replace OZ EIP712.sol with our own because UR uses OZ v4.7 and cannot find the import
Description of changes
Took Permit2's EIP712.sol:
_HASHED_NAMEinto an immutableAdded checks for _CACHED_THIS == address(this) (mirrors OZ's behavior)removed since its a delegate protection, see comment below_HASHED_VERSIONto DOMAIN_SEPARATOR (mirrors OZ's behavior)RYO cryptography at 10pm on a friday, what could possibly go wrong 🤷