From 9df96ac6667d791270651702b4dbe3fca6694f80 Mon Sep 17 00:00:00 2001 From: Karl Kempe Date: Thu, 1 Sep 2022 21:02:40 +0000 Subject: [PATCH] Fix domain separator and add more tests --- .../bridge/token/TokenImplementation.sol | 103 ++- .../contracts/bridge/token/TokenState.sol | 17 +- ethereum/forge-test/TokenImplementation.t.sol | 834 +++++++++++++++--- 3 files changed, 797 insertions(+), 157 deletions(-) diff --git a/ethereum/contracts/bridge/token/TokenImplementation.sol b/ethereum/contracts/bridge/token/TokenImplementation.sol index f952ed7b7..ec153f54b 100644 --- a/ethereum/contracts/bridge/token/TokenImplementation.sol +++ b/ethereum/contracts/bridge/token/TokenImplementation.sol @@ -58,22 +58,27 @@ contract TokenImplementation is TokenState, Context { } function _initializePermitStateIfNeeded() internal { - if (!permitInitialized()) { - _state.hashedTokenChain = _hashedTokenChain(); - _state.hashedNativeContract = _hashedNativeContract(); - _state.hashedVersion = _hashedDomainVersion(); - _state.typeHash = _hashedDomainType(); + // If someone were to change the implementation of name(), we + // need to make sure we recache. + bytes32 hashedName = _eip712DomainNameHashed(); + + // If for some reason the salt generation changes with newer + // token implementations, we need to make sure the state reflects + // the new salt. + bytes32 salt = _eip712DomainSalt(); + + // check cached values + if (_state.cachedHashedName != hashedName || _state.cachedSalt != salt) { _state.cachedChainId = block.chainid; - _state.cachedDomainSeparator = _buildNativeDomainSeparator( - _state.typeHash, _state.hashedTokenChain, _state.hashedNativeContract, _state.hashedVersion - ); _state.cachedThis = address(this); - _state.permitInitialized = true; + _state.cachedDomainSeparator = _buildDomainSeparator(hashedName, salt); + _state.cachedSalt = salt; + _state.cachedHashedName = hashedName; } } function name() public view returns (string memory) { - return string(abi.encodePacked(_state.name)); + return _state.name; } function symbol() public view returns (string memory) { @@ -194,6 +199,10 @@ contract TokenImplementation is TokenState, Context { _state.name = name_; _state.symbol = symbol_; _state.metaLastUpdatedSequence = sequence_; + + // Because the name is updated, we need to recache the domain separator. + // For old implementations, none of the caches may have been written to yet. + _initializePermitStateIfNeeded(); } modifier onlyOwner() { @@ -219,22 +228,25 @@ contract TokenImplementation is TokenState, Context { if (address(this) == _state.cachedThis && block.chainid == _state.cachedChainId) { return _state.cachedDomainSeparator; } else { - return _buildNativeDomainSeparator( - _hashedDomainType(), - _hashedTokenChain(), - _hashedNativeContract(), - _hashedDomainVersion() + return _buildDomainSeparator( + _eip712DomainNameHashed(), _eip712DomainSalt() ); } } - function _buildNativeDomainSeparator( - bytes32 typeHash, - bytes32 tokenChainHash, - bytes32 nativeContractHash, - bytes32 versionHash - ) internal view returns (bytes32) { - return keccak256(abi.encode(typeHash, tokenChainHash, nativeContractHash, versionHash, block.chainid, address(this))); + function _buildDomainSeparator(bytes32 hashedName, bytes32 salt) internal view returns (bytes32) { + return keccak256( + abi.encode( + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)" + ), + hashedName, + keccak256(abi.encodePacked(_eip712DomainVersion())), + block.chainid, + address(this), + salt + ) + ); } /** @@ -276,7 +288,16 @@ contract TokenImplementation is TokenState, Context { require(block.timestamp <= deadline_, "ERC20Permit: expired deadline"); bytes32 structHash = keccak256( - abi.encode(_hashedPermitType(), owner_, spender_, value_, _useNonce(owner_), deadline_) + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner_, + spender_, + value_, + _useNonce(owner_), + deadline_ + ) ); bytes32 message = _hashTypedDataV4(structHash); @@ -296,23 +317,35 @@ contract TokenImplementation is TokenState, Context { return _domainSeparatorV4(); } - function _hashedTokenChain() internal view returns (bytes32) { - return keccak256(abi.encodePacked(_state.chainId)); + function eip712Domain() public view returns ( + bytes1 domainFields, + string memory domainName, + string memory domainVersion, + uint256 domainChainId, + address domainVerifyingContract, + bytes32 domainSalt, + uint256[] memory domainExtensions + ) { + return ( + hex"1F", // 11111 + name(), + _eip712DomainVersion(), + block.chainid, + address(this), + _eip712DomainSalt(), + new uint256[](0) + ); } - function _hashedNativeContract() internal view returns (bytes32) { - return keccak256(abi.encodePacked(_state.nativeContract)); + function _eip712DomainVersion() internal pure returns (string memory) { + return "1"; } - function _hashedDomainVersion() internal pure returns (bytes32) { - return keccak256(bytes("1")); + function _eip712DomainNameHashed() internal view returns (bytes32) { + return keccak256(abi.encodePacked(name())); } - function _hashedDomainType() internal pure returns (bytes32) { - return keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); - } - - function _hashedPermitType() internal pure returns (bytes32) { - return keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + function _eip712DomainSalt() internal view returns (bytes32) { + return keccak256(abi.encodePacked(_state.chainId, _state.nativeContract)); } } diff --git a/ethereum/contracts/bridge/token/TokenState.sol b/ethereum/contracts/bridge/token/TokenState.sol index 93526a90d..ef875bc63 100644 --- a/ethereum/contracts/bridge/token/TokenState.sol +++ b/ethereum/contracts/bridge/token/TokenState.sol @@ -27,17 +27,14 @@ contract TokenStorage { bytes32 nativeContract; // EIP712 - // 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. - bool permitInitialized; + // Cache the domain separator and salt, but also store the chain id that + // it corresponds to, in order to invalidate the cached domain separator + // if the chain id changes. bytes32 cachedDomainSeparator; uint256 cachedChainId; address cachedThis; - - bytes32 hashedTokenChain; - bytes32 hashedNativeContract; - bytes32 hashedVersion; - bytes32 typeHash; + bytes32 cachedSalt; + bytes32 cachedHashedName; // ERC20Permit draft mapping(address => Counters.Counter) nonces; @@ -64,8 +61,4 @@ contract TokenState { current = nonce.current(); nonce.increment(); } - - function permitInitialized() public view returns (bool) { - return _state.permitInitialized; - } } \ No newline at end of file diff --git a/ethereum/forge-test/TokenImplementation.t.sol b/ethereum/forge-test/TokenImplementation.t.sol index 3395dedbf..fb089aafa 100644 --- a/ethereum/forge-test/TokenImplementation.t.sol +++ b/ethereum/forge-test/TokenImplementation.t.sol @@ -22,14 +22,21 @@ contract TestTokenImplementation is TokenImplementation, Test { bytes32 nativeContract; } + struct SignatureSetup { + address allower; + bytes32 r; + bytes32 s; + uint8 v; + } + function setupTestEnvironmentWithInitialize() public { InitiateParameters memory init; init.name = "Valuable Token"; init.symbol = "VALU"; init.decimals = 8; init.sequence = 1; - init.owner = 0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF; - init.chainId = 1; + init.owner = _msgSender(); + init.chainId = 5; init .nativeContract = 0x1337133713371337133713371337133713371337133713371337133713371337; @@ -50,8 +57,8 @@ contract TestTokenImplementation is TokenImplementation, Test { init.symbol = "OLD"; init.decimals = 8; init.sequence = 1; - init.owner = 0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF; - init.chainId = 1; + init.owner = _msgSender(); + init.chainId = 5; init .nativeContract = 0x1337133713371337133713371337133713371337133713371337133713371337; @@ -71,18 +78,10 @@ contract TestTokenImplementation is TokenImplementation, Test { address spender, uint256 amount, uint256 deadline - ) - public - returns ( - address allower, - uint8 v, - bytes32 r, - bytes32 s - ) - { + ) public returns (SignatureSetup memory output) { // prepare signer allowing for tokens to be spent uint256 sk = uint256(walletPrivateKey); - allower = vm.addr(sk); + output.allower = vm.addr(sk); bytes32 PERMIT_TYPEHASH = keccak256( "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" @@ -90,16 +89,269 @@ contract TestTokenImplementation is TokenImplementation, Test { bytes32 structHash = keccak256( abi.encode( PERMIT_TYPEHASH, - allower, + output.allower, spender, amount, - nonces(allower), + nonces(output.allower), deadline ) ); bytes32 message = ECDSA.toTypedDataHash(DOMAIN_SEPARATOR(), structHash); - (v, r, s) = vm.sign(sk, message); + (output.v, output.r, output.s) = vm.sign(sk, message); + } + + // if any of these tests fail, you may have messed around with the + // existing storage slots + function testCheckStorageSlots() public { + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // mint some so we can check totalSupply and balances + uint256 mintedAmount = 42069; + _mint(_msgSender(), mintedAmount); + + // also set allowances + uint256 allowanceAmount = 69420; + address spender = address(0x1); + _approve(_msgSender(), spender, allowanceAmount); + + // slot 0: name (string) + { + bytes32 data = vm.load(address(this), bytes32(0)); + // length 14, name = "Valuable Token" + // data <= 31 bytes long, so length is stored at end as length * 2 + bytes memory expectedName = bytes("Valuable Token"); + require( + uint256(data) & uint256(255) == expectedName.length * 2, + "incorrect name length" + ); + require( + uint256(data) & uint256(255) == bytes(_state.name).length * 2, + "incorrect name length" + ); + for (uint256 i = 0; i < expectedName.length; ++i) { + // I don't care to save this variable to storage + require( + data[i] == expectedName[i], + "data[i] != expectedName[i]" + ); + require( + data[i] == bytes(_state.name)[i], + "data[i] != _state.name[i]" + ); + } + } + + // slot 1: symbol (string) + { + bytes32 data = vm.load(address(this), bytes32(uint256(1))); + // length 14, name = "Valuable Token" + // data <= 31 bytes long, so length is stored at end as length * 2 + bytes memory expectedSymbol = bytes("VALU"); + require( + uint256(data) & uint256(255) == expectedSymbol.length * 2, + "incorrect symbol length" + ); + require( + uint256(data) & uint256(255) == bytes(_state.symbol).length * 2, + "incorrect symbol length" + ); + for (uint256 i = 0; i < expectedSymbol.length; ++i) { + // I don't care to save this variable to storage + require( + data[i] == expectedSymbol[i], + "data[i] != expectedSymbol[i]" + ); + require( + data[i] == bytes(_state.symbol)[i], + "data[i] != _state.symbol[i]" + ); + } + } + + // slot 2: metaLastUpdatedSequence (uint64) + { + bytes32 data = vm.load(address(this), bytes32(uint256(2))); + require( + uint256(data) == uint256(1), + "data != expected metaLastUpdatedSequence" + ); + require( + uint256(data) == uint256(_state.metaLastUpdatedSequence), + "data != _state.metaLastUpdatedSequence" + ); + } + + // slot 3: totalSupply (uint256) + { + // now verify + bytes32 data = vm.load(address(this), bytes32(uint256(3))); + require( + uint256(data) == mintedAmount, + "data != expected totalSupply" + ); + require( + uint256(data) == uint256(_state.totalSupply), + "data != _state.totalSupply" + ); + } + + // slot 4: decimals (uint8) + { + bytes32 data = vm.load(address(this), bytes32(uint256(4))); + require(uint256(data) == uint256(8), "data != expected decimals"); + require( + uint256(data) == uint256(_state.decimals), + "data != _state.decimals" + ); + } + + // slot 5: balances (mapping(address) => uint256) + { + bytes32 data = vm.load(address(this), bytes32(uint256(5))); + require(uint256(data) == uint256(0), "data != 0"); + + bytes32 mappedData = vm.load( + address(this), + keccak256(abi.encode(_msgSender(), 5)) + ); + require( + uint256(mappedData) == mintedAmount, + "data != expected balance for account" + ); + require( + uint256(mappedData) == _state.balances[_msgSender()], + "data != _state.balances[_msgSender()]" + ); + } + + // slot 6: allowances (mapping(address) => uint256) + { + bytes32 data = vm.load(address(this), bytes32(uint256(6))); + require(uint256(data) == uint256(0), "data != 0"); + + bytes32 mappedData = vm.load( + address(this), + keccak256( + abi.encode(spender, keccak256(abi.encode(_msgSender(), 6))) + ) + ); + require( + uint256(mappedData) == allowanceAmount, + "data != expected allowance for account" + ); + require( + uint256(mappedData) == _state.allowances[_msgSender()][spender], + "data != _state.allowances[_msgSender()][spender]" + ); + } + + // slot 7: owner (address), initialized (bool), chainId (uint16) + { + bytes32 data = vm.load(address(this), bytes32(uint256(7))); + require( + (uint256(data) >> (21 * 8)) == uint256(5), + "data[9:11] != expected chainId" + ); + require( + (uint256(data) >> (21 * 8)) == uint256(_state.chainId), + "data[9:11] != _state.chainId" + ); + require( + uint8(data[11]) == uint8(1), + "data[11] != expected initialized" + ); + require( + uint8(data[11]) == uint8(_state.initialized ? 1 : 0), + "data[11] != _state.initialized" + ); + require( + uint256(data) & uint256(2**160 - 1) == + uint256(uint160(_msgSender())), + "data[12:32] != expected owner" + ); + require( + uint256(data) & uint256(2**160 - 1) == + uint256(uint160(_state.owner)), + "data[12:32] != _state.owner" + ); + } + + // slot 8: nativeContract (bytes32) + { + bytes32 data = vm.load(address(this), bytes32(uint256(8))); + require( + data == + 0x1337133713371337133713371337133713371337133713371337133713371337, + "data != expected nativeContract" + ); + require( + data == _state.nativeContract, + "data != _state.nativeContract" + ); + } + + // slot 9: cachedDomainSeparator (bytes32) + { + bytes32 data = vm.load(address(this), bytes32(uint256(9))); + require( + data == + _buildDomainSeparator( + _eip712DomainNameHashed(), + _eip712DomainSalt() + ), + "data != expected domain separator" + ); + require(data == DOMAIN_SEPARATOR(), "data != DOMAIN_SEPARATOR()"); + require( + data == _state.cachedDomainSeparator, + "data != _state.cachedDomainSeparator" + ); + } + + // slot 10: cachedChainId (uint256) + { + bytes32 data = vm.load(address(this), bytes32(uint256(10))); + require(uint256(data) == block.chainid, "data != block.chainid"); + require( + uint256(data) == _state.cachedChainId, + "data != _state.cachedChainId" + ); + } + + // slot 11: cachedThis (address) + { + bytes32 data = vm.load(address(this), bytes32(uint256(11))); + require( + uint256(data) == uint256(uint160(address(this))), + "data != address(this)" + ); + require( + uint256(data) == uint256(uint160(_state.cachedThis)), + "data != _state.cachedThis" + ); + } + + // slot 12: cachedSalt (bytes32) + { + bytes32 data = vm.load(address(this), bytes32(uint256(12))); + require(data == _eip712DomainSalt(), "data != expected salt"); + require(data == _state.cachedSalt, "data != _state.cachedSalt"); + } + + // slot 13: cachedHashedName (bytes32) + { + bytes32 data = vm.load(address(this), bytes32(uint256(13))); + require( + data == _eip712DomainNameHashed(), + "data != _eip712DomainNameHashed()" + ); + require( + data == _state.cachedHashedName, + "data != _state.cachedHashedName" + ); + } } function testPermit( @@ -116,27 +368,26 @@ contract TestTokenImplementation is TokenImplementation, Test { // prepare signer allowing for tokens to be spent uint256 deadline = 10; - ( - address allower, - uint8 v, - bytes32 r, - bytes32 s - ) = simulatePermitSignature( - walletPrivateKey, - spender, - amount, - deadline - ); - - // get allowance before calling permit - uint256 allowanceBefore = allowance(allower, spender); + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); // set allowance with permit - permit(allower, spender, amount, deadline, v, r, s); - uint256 allowanceAfter = allowance(allower, spender); + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); require( - allowanceAfter - allowanceBefore == amount, + allowance(signature.allower, spender) == amount, "allowance incorrect" ); } @@ -155,25 +406,36 @@ contract TestTokenImplementation is TokenImplementation, Test { // prepare signer allowing for tokens to be spent uint256 deadline = 10; - ( - address allower, - uint8 v, - bytes32 r, - bytes32 s - ) = simulatePermitSignature( - walletPrivateKey, - spender, - amount, - deadline - ); + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); // set allowance with permit - permit(allower, spender, amount, deadline, v, r, s); + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); // try again... you shall not pass // NOTE: using "testFail" instead of "test" because // vm.expectRevert("ERC20Permit: invalid signature") does not work - permit(allower, spender, amount, deadline, v, r, s); + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); } function testFailPermitWithBadSignature( @@ -196,22 +458,25 @@ contract TestTokenImplementation is TokenImplementation, Test { // prepare signer allowing for tokens to be spent uint256 deadline = 10; - ( - address allower, - uint8 v, - bytes32 r, - bytes32 s - ) = simulatePermitSignature( - walletPrivateKey, - spender, - wrongAmount, - deadline - ); + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + wrongAmount, + deadline + ); // you shall not pass! // NOTE: using "testFail" instead of "test" because // vm.expectRevert("ERC20Permit: invalid signature") does not work - permit(allower, spender, amount, deadline, v, r, s); + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); } function testPermitWithSignatureUsedAfterDeadline( @@ -228,57 +493,47 @@ contract TestTokenImplementation is TokenImplementation, Test { // prepare signer allowing for tokens to be spent uint256 deadline = 10; - ( - address allower, - uint8 v, - bytes32 r, - bytes32 s - ) = simulatePermitSignature( - walletPrivateKey, - spender, - amount, - deadline - ); + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); // waited too long vm.warp(deadline + 1); // and fail vm.expectRevert("ERC20Permit: expired deadline"); - permit(allower, spender, amount, deadline, v, r, s); + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); } function testInitializePermitState() public { // initialize TokenImplementation as if it were the old implementation setupTestEnvironmentWithOldInitialize(); - require(!permitInitialized(), "permit state should not be initialized"); + require( + _state.cachedHashedName == bytes32(0), + "cachedHashedName is set" + ); + require(_state.cachedSalt == bytes32(0), "cachedSalt is set"); // explicity call private method _initializePermitStateIfNeeded(); - require(permitInitialized(), "permit state should be initialized"); + require( + _state.cachedHashedName == _eip712DomainNameHashed(), + "hasnedName not cached" + ); + require(_state.cachedSalt == _eip712DomainSalt(), "salt not cached"); // check permit state variables - require( - _state.hashedTokenChain == - keccak256(abi.encodePacked(_state.chainId)), - "_state.hashedTokenChain != expected" - ); - require( - _state.hashedNativeContract == - keccak256(abi.encodePacked(_state.nativeContract)), - "_state.hashedNativeContract != expected" - ); - require( - _state.hashedVersion == keccak256(bytes("1")), - "_state.hashedVersion != expected" - ); - require( - _state.typeHash == - keccak256( - "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" - ), - "_state.typeHash != expected" - ); require( _state.cachedChainId == block.chainid, "_state.cachedChainId != expected" @@ -287,16 +542,37 @@ contract TestTokenImplementation is TokenImplementation, Test { _state.cachedDomainSeparator == keccak256( abi.encode( - _hashedDomainType(), - _hashedTokenChain(), - _hashedNativeContract(), - _hashedDomainVersion(), + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)" + ), + keccak256(abi.encodePacked(name())), + keccak256(abi.encodePacked(_eip712DomainVersion())), block.chainid, - address(this) + address(this), + keccak256(abi.encodePacked(chainId(), nativeContract())) ) ), "_state.cachedDomainSeparator != expected" ); + require( + _buildDomainSeparator( + _eip712DomainNameHashed(), + _eip712DomainSalt() + ) == + keccak256( + abi.encode( + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)" + ), + keccak256(abi.encodePacked(name())), + keccak256(abi.encodePacked(_eip712DomainVersion())), + block.chainid, + address(this), + keccak256(abi.encodePacked(chainId(), nativeContract())) + ) + ), + "_buildDomainSeparator() != expected" + ); require( _state.cachedThis == address(this), "_state.cachedThis != expected" @@ -314,31 +590,369 @@ contract TestTokenImplementation is TokenImplementation, Test { // initialize TokenImplementation as if it were the old implementation setupTestEnvironmentWithOldInitialize(); + require(_state.cachedSalt == bytes32(0), "cachedSalt is set"); // prepare signer allowing for tokens to be spent uint256 deadline = 10; + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // set allowance with permit + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); + + require( + allowance(signature.allower, spender) == amount, + "allowance incorrect" + ); + } + + // used to prevent stack too deep in test + struct Eip712DomainOutput { + bytes1 fields; + string name; + string version; + uint256 chainId; + address verifyingContract; + bytes32 salt; + uint256[] extensions; + } + + function testPermitUsingEip712DomainValues( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + Eip712DomainOutput memory domain; ( - address allower, - uint8 v, - bytes32 r, - bytes32 s - ) = simulatePermitSignature( + domain.fields, + domain.name, + domain.version, + domain.chainId, + domain.verifyingContract, + domain.salt, + domain.extensions + ) = eip712Domain(); + require(domain.fields == hex"1F", "domainFields != expected"); + require( + keccak256(abi.encodePacked(domain.name)) == + keccak256(abi.encodePacked(name())), + "domainName != expected" + ); + require( + keccak256(abi.encodePacked(domain.name)) == + _eip712DomainNameHashed(), + "domainName != _eip712DomainNameHashed()" + ); + require( + keccak256(abi.encodePacked(domain.version)) == + keccak256(abi.encodePacked("1")), + "domainVersion != expected" + ); + require( + keccak256(abi.encodePacked(domain.version)) == + keccak256(abi.encodePacked(_eip712DomainVersion())), + "domainVersion != _eip712DomainVersion()" + ); + require(domain.chainId == block.chainid, "domainFields != expected"); + require( + domain.chainId == _state.cachedChainId, + "domainFields != _state.cachedChainId" + ); + require( + domain.verifyingContract == address(this), + "domainVerifyingContract != expected" + ); + require( + domain.verifyingContract == _state.cachedThis, + "domainVerifyingContract != _state.cachedThis" + ); + require( + domain.salt == + keccak256(abi.encodePacked(chainId(), nativeContract())), + "domainFields != expected" + ); + require( + domain.salt == _eip712DomainSalt(), + "domainFields != _eip712DomainSalt()" + ); + require( + domain.salt == _state.cachedSalt, + "domainFields != _state.cachedSalt" + ); + require(domain.extensions.length == 0, "domainExtensions.length != 0"); + + // prepare signer allowing for tokens to be spent + SignatureSetup memory signature; + uint256 sk = uint256(walletPrivateKey); + signature.allower = vm.addr(sk); + + uint256 deadline = 10; + + bytes32 structHash = keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + signature.allower, + spender, + amount, + nonces(signature.allower), + deadline + ) + ); + + // build domain separator by hand using eip712Domain() output + bytes32 domainSeparator = keccak256( + abi.encode( + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)" + ), + keccak256(abi.encodePacked(domain.name)), + keccak256(abi.encodePacked(domain.version)), + domain.chainId, + domain.verifyingContract, + domain.salt + ) + ); + + // sign and set allowance with permit + (signature.v, signature.r, signature.s) = vm.sign( + sk, + ECDSA.toTypedDataHash(domainSeparator, structHash) + ); + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); + + require( + allowance(signature.allower, spender) == amount, + "allowance incorrect" + ); + } + + function testPermitAfterUpdateDetails( + bytes32 walletPrivateKey, + uint256 amount, + address spender, + string calldata newName + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + vm.assume(bytes(newName).length <= 32); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + string memory oldName = name(); + bytes32 oldDomainSeparator = _state.cachedDomainSeparator; + + // permit before updateDetails + { + uint256 deadline = 10; + SignatureSetup memory signature = simulatePermitSignature( walletPrivateKey, spender, amount, deadline ); - // get allowance before calling permit - uint256 allowanceBefore = allowance(allower, spender); + // set allowance with permit + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); - // set allowance with permit - permit(allower, spender, amount, deadline, v, r, s); + require( + allowance(signature.allower, spender) == amount, + "allowance incorrect" + ); - uint256 allowanceAfter = allowance(allower, spender); + // revoke allowance to prep for next test + _approve(signature.allower, spender, 0); + } + + // asset metadata updated here + updateDetails( + newName, + "NEW", // new symbol + _state.metaLastUpdatedSequence + 1 // new sequence + ); require( - allowanceAfter - allowanceBefore == amount, + keccak256(abi.encodePacked(newName)) != + keccak256(abi.encodePacked(oldName)), + "newName == oldName" + ); + require( + _domainSeparatorV4() != oldDomainSeparator, + "_domainSeparatorV4() == oldDomainSeparator" + ); + require( + _state.cachedDomainSeparator != oldDomainSeparator, + "_state.cachedDomainSeparator == oldDomainSeparator" + ); + require( + _state.cachedDomainSeparator == _domainSeparatorV4(), + "_state.cachedDomainSeparator != _domainSeparatorV4()" + ); + + // permit after updateDetails + { + uint256 deadline = 10; + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // set allowance with permit + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); + + require( + allowance(signature.allower, spender) == amount, + "allowance incorrect" + ); + } + } + + function testPermitForOldSalt( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // hijack salt + _state.cachedSalt = keccak256(abi.encodePacked("definitely not right")); + require( + _state.cachedSalt != _eip712DomainSalt(), + "_state.cachedSalt == _eip712DomainSalt()" + ); + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // set allowance with permit + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); + + // verify salt is correct + require( + _state.cachedSalt == _eip712DomainSalt(), + "_state.cachedSalt != _eip712DomainSalt()" + ); + // then allowance + require( + allowance(signature.allower, spender) == amount, + "allowance incorrect" + ); + } + + function testPermitForOldName( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // hijack name + _state.cachedHashedName = keccak256("definitely not right"); + require( + _state.cachedHashedName != _eip712DomainNameHashed(), + "_state.cachedHashedName == _eip712DomainNameHashed()" + ); + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + SignatureSetup memory signature = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // set allowance with permit + permit( + signature.allower, + spender, + amount, + deadline, + signature.v, + signature.r, + signature.s + ); + + // verify name is correct + require( + _state.cachedHashedName == _eip712DomainNameHashed(), + "_state.cachedHashedName != _eip712DomainNameHashed()" + ); + // then allowance + require( + allowance(signature.allower, spender) == amount, "allowance incorrect" ); }