From 7b92635b53f8161941bbbd4004de34f6bdd43b7a Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Mon, 25 Sep 2023 13:45:12 +0200 Subject: [PATCH 1/2] refactor(sig-utils): simplify contract --- test/forge/PermitTest.sol | 27 +++++++++------------------ test/forge/helpers/SigUtils.sol | 28 +++++++++++++++------------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/test/forge/PermitTest.sol b/test/forge/PermitTest.sol index ce7028a0..5c3b4ab1 100644 --- a/test/forge/PermitTest.sol +++ b/test/forge/PermitTest.sol @@ -27,8 +27,7 @@ contract PermitTest is BaseTest { function testPermit() public { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -41,8 +40,7 @@ contract PermitTest is BaseTest { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: vault.nonces(owner), deadline: 1 days}); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vm.warp(1 days + 1 seconds); // fast forward one second past the deadline @@ -55,8 +53,7 @@ contract PermitTest is BaseTest { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: vault.nonces(owner), deadline: 1 days}); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(SPENDER_PK, digest); // spender signs owner's approval vm.expectRevert("ERC20Permit: invalid signature"); @@ -72,8 +69,7 @@ contract PermitTest is BaseTest { deadline: 1 days }); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vm.expectRevert("ERC20Permit: invalid signature"); @@ -83,8 +79,7 @@ contract PermitTest is BaseTest { function testRevertSignatureReplay() public { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -96,8 +91,7 @@ contract PermitTest is BaseTest { function testTransferFromLimitedPermit() public { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -114,8 +108,7 @@ contract PermitTest is BaseTest { Permit memory permit = Permit({owner: owner, spender: spender, value: type(uint256).max, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -137,8 +130,7 @@ contract PermitTest is BaseTest { deadline: 1 days }); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -156,8 +148,7 @@ contract PermitTest is BaseTest { deadline: 1 days }); - bytes32 digest = sigUtils.getTypedDataHash(permit); - + bytes32 digest = sigUtils.toTypedDataHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); diff --git a/test/forge/helpers/SigUtils.sol b/test/forge/helpers/SigUtils.sol index 11d90252..8f265c30 100644 --- a/test/forge/helpers/SigUtils.sol +++ b/test/forge/helpers/SigUtils.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: GPL-2.0-or-later pragma solidity ^0.8.0; +import {ECDSA} from "@openzeppelin/utils/cryptography/ECDSA.sol"; + struct Permit { address owner; address spender; @@ -9,25 +11,25 @@ struct Permit { uint256 deadline; } -// keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); -bytes32 constant PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9; +bytes32 constant PERMIT_TYPEHASH = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); contract SigUtils { - bytes32 internal DOMAIN_SEPARATOR; + bytes32 internal immutable _DOMAIN_SEPARATOR; - constructor(bytes32 _DOMAIN_SEPARATOR) { - DOMAIN_SEPARATOR = _DOMAIN_SEPARATOR; + constructor(bytes32 domainSeparator) { + _DOMAIN_SEPARATOR = domainSeparator; } - // computes the hash of a permit - function getStructHash(Permit memory _permit) internal pure returns (bytes32) { - return keccak256( - abi.encode(PERMIT_TYPEHASH, _permit.owner, _permit.spender, _permit.value, _permit.nonce, _permit.deadline) - ); + function toTypedDataHash(bytes32 digest) public view returns (bytes32) { + return ECDSA.toTypedDataHash(_DOMAIN_SEPARATOR, digest); } - // computes the hash of the fully encoded EIP-712 message for the domain, which can be used to recover the signer - function getTypedDataHash(Permit memory _permit) public view returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, getStructHash(_permit))); + function toTypedDataHash(Permit memory permit) public view returns (bytes32) { + return toTypedDataHash( + keccak256( + abi.encode(PERMIT_TYPEHASH, permit.owner, permit.spender, permit.value, permit.nonce, permit.deadline) + ) + ); } } From 26b7fd6b2cbffe8d81c6de166778ad856799de5a Mon Sep 17 00:00:00 2001 From: Rubilmax Date: Mon, 25 Sep 2023 15:24:21 +0200 Subject: [PATCH 2/2] refactor(sig-utils): remove domain separator storage value --- test/forge/PermitTest.sol | 22 +++++++++------------- test/forge/helpers/SigUtils.sol | 17 ++++------------- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/test/forge/PermitTest.sol b/test/forge/PermitTest.sol index 5c3b4ab1..75a50e87 100644 --- a/test/forge/PermitTest.sol +++ b/test/forge/PermitTest.sol @@ -5,8 +5,6 @@ import "./helpers/SigUtils.sol"; import "./helpers/BaseTest.sol"; contract PermitTest is BaseTest { - SigUtils internal sigUtils; - uint256 internal constant OWNER_PK = 0xA11CE; uint256 internal constant SPENDER_PK = 0xB0B; @@ -16,8 +14,6 @@ contract PermitTest is BaseTest { function setUp() public override { super.setUp(); - sigUtils = new SigUtils(vault.DOMAIN_SEPARATOR()); - owner = vm.addr(OWNER_PK); spender = vm.addr(SPENDER_PK); @@ -27,7 +23,7 @@ contract PermitTest is BaseTest { function testPermit() public { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -40,7 +36,7 @@ contract PermitTest is BaseTest { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: vault.nonces(owner), deadline: 1 days}); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vm.warp(1 days + 1 seconds); // fast forward one second past the deadline @@ -53,7 +49,7 @@ contract PermitTest is BaseTest { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: vault.nonces(owner), deadline: 1 days}); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(SPENDER_PK, digest); // spender signs owner's approval vm.expectRevert("ERC20Permit: invalid signature"); @@ -69,7 +65,7 @@ contract PermitTest is BaseTest { deadline: 1 days }); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vm.expectRevert("ERC20Permit: invalid signature"); @@ -79,7 +75,7 @@ contract PermitTest is BaseTest { function testRevertSignatureReplay() public { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -91,7 +87,7 @@ contract PermitTest is BaseTest { function testTransferFromLimitedPermit() public { Permit memory permit = Permit({owner: owner, spender: spender, value: 1e18, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -108,7 +104,7 @@ contract PermitTest is BaseTest { Permit memory permit = Permit({owner: owner, spender: spender, value: type(uint256).max, nonce: 0, deadline: 1 days}); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -130,7 +126,7 @@ contract PermitTest is BaseTest { deadline: 1 days }); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); @@ -148,7 +144,7 @@ contract PermitTest is BaseTest { deadline: 1 days }); - bytes32 digest = sigUtils.toTypedDataHash(permit); + bytes32 digest = SigUtils.toTypedDataHash(vault.DOMAIN_SEPARATOR(), permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(OWNER_PK, digest); vault.permit(permit.owner, permit.spender, permit.value, permit.deadline, v, r, s); diff --git a/test/forge/helpers/SigUtils.sol b/test/forge/helpers/SigUtils.sol index 8f265c30..9c8ef5e2 100644 --- a/test/forge/helpers/SigUtils.sol +++ b/test/forge/helpers/SigUtils.sol @@ -14,19 +14,10 @@ struct Permit { bytes32 constant PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); -contract SigUtils { - bytes32 internal immutable _DOMAIN_SEPARATOR; - - constructor(bytes32 domainSeparator) { - _DOMAIN_SEPARATOR = domainSeparator; - } - - function toTypedDataHash(bytes32 digest) public view returns (bytes32) { - return ECDSA.toTypedDataHash(_DOMAIN_SEPARATOR, digest); - } - - function toTypedDataHash(Permit memory permit) public view returns (bytes32) { - return toTypedDataHash( +library SigUtils { + function toTypedDataHash(bytes32 domainSeparator, Permit memory permit) internal pure returns (bytes32) { + return ECDSA.toTypedDataHash( + domainSeparator, keccak256( abi.encode(PERMIT_TYPEHASH, permit.owner, permit.spender, permit.value, permit.nonce, permit.deadline) )