diff --git a/packages/ethereum-contracts/CHANGELOG.md b/packages/ethereum-contracts/CHANGELOG.md index 80a0de65cc..27bcfda540 100644 --- a/packages/ethereum-contracts/CHANGELOG.md +++ b/packages/ethereum-contracts/CHANGELOG.md @@ -3,6 +3,11 @@ All notable changes to the ethereum-contracts will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [unreleased] + +### Added +- `SuperToken` now implements [EIP-2612](https://eips.ethereum.org/EIPS/eip-2612) (permit extension for EIP-20 signed approvals) + ## [v1.12.1] ### Added diff --git a/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperToken.sol b/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperToken.sol index c1a468f631..6289f227b8 100644 --- a/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperToken.sol +++ b/packages/ethereum-contracts/contracts/interfaces/superfluid/ISuperToken.sol @@ -3,6 +3,8 @@ pragma solidity >= 0.8.11; import { ISuperfluidToken } from "./ISuperfluidToken.sol"; import { IERC20, IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import { IERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Permit.sol"; +import { IERC5267 } from "@openzeppelin/contracts/interfaces/IERC5267.sol"; import { IERC777 } from "@openzeppelin/contracts/token/ERC777/IERC777.sol"; import { IPoolAdminNFT } from "../agreements/gdav1/IPoolAdminNFT.sol"; import { IPoolMemberNFT } from "../agreements/gdav1/IPoolMemberNFT.sol"; @@ -11,25 +13,27 @@ import { IPoolMemberNFT } from "../agreements/gdav1/IPoolMemberNFT.sol"; * @title Super token (Superfluid Token + ERC20 + ERC777) interface * @author Superfluid */ -interface ISuperToken is ISuperfluidToken, IERC20Metadata, IERC777 { +interface ISuperToken is ISuperfluidToken, IERC20Metadata, IERC777, IERC20Permit, IERC5267 { /************************************************************************** * Errors *************************************************************************/ - error SUPER_TOKEN_CALLER_IS_NOT_OPERATOR_FOR_HOLDER(); // 0xf7f02227 - error SUPER_TOKEN_NOT_ERC777_TOKENS_RECIPIENT(); // 0xfe737d05 - error SUPER_TOKEN_INFLATIONARY_DEFLATIONARY_NOT_SUPPORTED(); // 0xe3e13698 - error SUPER_TOKEN_NO_UNDERLYING_TOKEN(); // 0xf79cf656 - error SUPER_TOKEN_ONLY_SELF(); // 0x7ffa6648 - error SUPER_TOKEN_ONLY_ADMIN(); // 0x0484acab - error SUPER_TOKEN_ONLY_GOV_OWNER(); // 0xd9c7ed08 - error SUPER_TOKEN_APPROVE_FROM_ZERO_ADDRESS(); // 0x81638627 - error SUPER_TOKEN_APPROVE_TO_ZERO_ADDRESS(); // 0xdf070274 - error SUPER_TOKEN_BURN_FROM_ZERO_ADDRESS(); // 0xba2ab184 - error SUPER_TOKEN_MINT_TO_ZERO_ADDRESS(); // 0x0d243157 - error SUPER_TOKEN_TRANSFER_FROM_ZERO_ADDRESS(); // 0xeecd6c9b - error SUPER_TOKEN_TRANSFER_TO_ZERO_ADDRESS(); // 0xe219bd39 - error SUPER_TOKEN_NFT_PROXY_ADDRESS_CHANGED(); // 0x6bef249d + error SUPER_TOKEN_CALLER_IS_NOT_OPERATOR_FOR_HOLDER(); // 0xf7f02227 + error SUPER_TOKEN_NOT_ERC777_TOKENS_RECIPIENT(); // 0xfe737d05 + error SUPER_TOKEN_INFLATIONARY_DEFLATIONARY_NOT_SUPPORTED(); // 0xe3e13698 + error SUPER_TOKEN_NO_UNDERLYING_TOKEN(); // 0xf79cf656 + error SUPER_TOKEN_ONLY_SELF(); // 0x7ffa6648 + error SUPER_TOKEN_ONLY_ADMIN(); // 0x0484acab + error SUPER_TOKEN_ONLY_GOV_OWNER(); // 0xd9c7ed08 + error SUPER_TOKEN_APPROVE_FROM_ZERO_ADDRESS(); // 0x81638627 + error SUPER_TOKEN_APPROVE_TO_ZERO_ADDRESS(); // 0xdf070274 + error SUPER_TOKEN_BURN_FROM_ZERO_ADDRESS(); // 0xba2ab184 + error SUPER_TOKEN_MINT_TO_ZERO_ADDRESS(); // 0x0d243157 + error SUPER_TOKEN_TRANSFER_FROM_ZERO_ADDRESS(); // 0xeecd6c9b + error SUPER_TOKEN_TRANSFER_TO_ZERO_ADDRESS(); // 0xe219bd39 + error SUPER_TOKEN_NFT_PROXY_ADDRESS_CHANGED(); // 0xef1b6ddf + error SUPER_TOKEN_PERMIT_EXPIRED_SIGNATURE(uint256 deadline); // 0x6e72b90f + error SUPER_TOKEN_PERMIT_INVALID_SIGNER(address signer, address owner); // 0xb6422105 /** * @dev Initialize the contract diff --git a/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.t.sol b/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.t.sol index 402c753631..e4eb61f788 100644 --- a/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.t.sol +++ b/packages/ethereum-contracts/contracts/mocks/SuperTokenMock.t.sol @@ -67,8 +67,8 @@ contract SuperTokenStorageLayoutTester is SuperToken { require (slot == 18 && offset == 0, "_operators changed location"); // uses 4 slots - assembly { slot:= _reserve22.slot offset := _reserve22.offset } - require (slot == 22 && offset == 0, "_reserve22 changed location"); + assembly { slot:= _reserve23.slot offset := _reserve23.offset } + require (slot == 23 && offset == 0, "_reserve23 changed location"); assembly { slot:= _reserve31.slot offset := _reserve31.offset } require (slot == 31 && offset == 0, "_reserve31 changed location"); diff --git a/packages/ethereum-contracts/contracts/superfluid/SuperToken.sol b/packages/ethereum-contracts/contracts/superfluid/SuperToken.sol index dbb0db043f..f5d15ef539 100644 --- a/packages/ethereum-contracts/contracts/superfluid/SuperToken.sol +++ b/packages/ethereum-contracts/contracts/superfluid/SuperToken.sol @@ -20,6 +20,7 @@ import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { IERC777Recipient } from "@openzeppelin/contracts/token/ERC777/IERC777Recipient.sol"; import { IERC777Sender } from "@openzeppelin/contracts/token/ERC777/IERC777Sender.sol"; import { Address } from "@openzeppelin/contracts/utils/Address.sol"; +import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; // placeholder types needed as an intermediate step before complete removal of FlowNFTs // solhint-disable-next-line no-empty-blocks @@ -37,7 +38,6 @@ contract SuperToken is SuperfluidToken, ISuperToken { - using SafeMath for uint256; using SafeCast for uint256; using Address for address; @@ -49,6 +49,14 @@ contract SuperToken is uint8 constant private _STANDARD_DECIMALS = 18; + // EIP-712 permit typehash + bytes32 constant private _PERMIT_TYPEHASH = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + bytes32 constant private _EIP712_DOMAIN_TYPEHASH = + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + + string constant private _EIP712_VERSION = "1"; + // solhint-disable-next-line var-name-mixedcase IConstantOutflowNFT immutable public CONSTANT_OUTFLOW_NFT; @@ -84,6 +92,9 @@ contract SuperToken is /// @dev ERC777 operators support data ERC777Helper.Operators internal _operators; + /// @dev ERC20 Nonces for EIP-2612 (permit) + mapping(address account => uint256) internal _nonces; + // NOTE: for future compatibility, these are reserved solidity slots // The sub-class of SuperToken solidity slot will start after _reserve22 @@ -91,8 +102,7 @@ contract SuperToken is // function in its respective mock contract to ensure that it doesn't break anything or lead to unexpected // behaviors/layout when upgrading - uint256 internal _reserve22; - uint256 private _reserve23; + uint256 internal _reserve23; uint256 private _reserve24; uint256 private _reserve25; uint256 private _reserve26; @@ -181,14 +191,14 @@ contract SuperToken is UUPSProxiable._updateCodeAddress(newAddress); } - function changeAdmin(address newAdmin) external override onlyAdmin { + function changeAdmin(address newAdmin) external virtual override onlyAdmin { address oldAdmin = _getAdmin(); _setAdmin(newAdmin); emit AdminChanged(oldAdmin, newAdmin); } - function getAdmin() external view override returns (address) { + function getAdmin() external view virtual override returns (address) { return _getAdmin(); } @@ -223,6 +233,101 @@ contract SuperToken is return _STANDARD_DECIMALS; } + /************************************************************************** + * ERC20 Permit (EIP-2612) + *************************************************************************/ + + /// @dev EIP-2612 Permit + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public virtual override { + if (block.timestamp > deadline) revert SUPER_TOKEN_PERMIT_EXPIRED_SIGNATURE(deadline); + + bytes32 structHash = keccak256( + abi.encode( + _PERMIT_TYPEHASH, + owner, + spender, + value, + _nonces[owner]++, + deadline + ) + ); + + bytes32 domainSeparator = DOMAIN_SEPARATOR(); + // Get the keccak256 digest of the EIP-712 typed data (ERC-191 version `0x01`). + // solhint-disable-next-line max-line-length + // Snippet taken from https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v5.2.0/contracts/utils/cryptography/MessageHashUtils.sol + bytes32 hash; + assembly ("memory-safe") { + let ptr := mload(0x40) + mstore(ptr, hex"19_01") + mstore(add(ptr, 0x02), domainSeparator) + mstore(add(ptr, 0x22), structHash) + hash := keccak256(ptr, 0x42) + } + + address signer = ECDSA.recover(hash, v, r, s); + if (signer != owner) revert SUPER_TOKEN_PERMIT_INVALID_SIGNER(signer, owner); + + _approve(owner, spender, value); + } + + /// @dev EIP-712 Domain Separator + // solhint-disable func-name-mixedcase + function DOMAIN_SEPARATOR() public view virtual override returns (bytes32) { + // Here we could squeeze out some gas by using pre-computed hashes + return keccak256( + abi.encode( + _EIP712_DOMAIN_TYPEHASH, + keccak256(bytes(_name)), + keccak256(bytes(_EIP712_VERSION)), + block.chainid, + address(this) + ) + ); + } + + /// @dev EIP-2612 Nonces + function nonces(address owner) public view virtual override returns (uint256) { + return _nonces[owner]; + } + + /// @dev EIP-5267: Retrieval of EIP-712 domain + function eip712Domain() + public + view + virtual + override + returns + ( + bytes1 fields, + /* commented out to avoid warning of name clash with name() */ + string memory /*name*/, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ) + { + return ( + hex"0f", // 01111 - field "salt" not present + _name, + _EIP712_VERSION, + block.chainid, + address(this), // verifyingContract + bytes32(0), // salt + new uint256[](0) // extensions + ); + } + /************************************************************************** * (private) Token Logics *************************************************************************/ @@ -905,5 +1010,4 @@ contract SuperToken is if (msg.sender != admin) revert SUPER_TOKEN_ONLY_ADMIN(); _; } - } diff --git a/packages/ethereum-contracts/foundry.toml b/packages/ethereum-contracts/foundry.toml index 0d39459284..df8b2e2338 100644 --- a/packages/ethereum-contracts/foundry.toml +++ b/packages/ethereum-contracts/foundry.toml @@ -14,7 +14,7 @@ optimizer_runs = 200 remappings = [ '@superfluid-finance/ethereum-contracts/contracts/=packages/ethereum-contracts/contracts/', '@superfluid-finance/solidity-semantic-money/src/=packages/solidity-semantic-money/src/', - '@openzeppelin/=node_modules/@openzeppelin/', + '@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/', 'ds-test/=lib/forge-std/lib/ds-test/src/', 'forge-std/=lib/forge-std/src/'] out = 'packages/ethereum-contracts/build/foundry/default' diff --git a/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol b/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol index 59be7a3a39..f7bc8156e7 100644 --- a/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol +++ b/packages/ethereum-contracts/test/foundry/agreements/gdav1/GeneralDistributionAgreement.t.sol @@ -446,6 +446,7 @@ contract GeneralDistributionAgreementV1IntegrationTest is FoundrySuperfluidTeste vm.assume(distributionFlowRate < minDepositFlowRate); vm.assume(distributionFlowRate > 0); vm.assume(member != address(pool)); + vm.assume(member != address(freePool)); // yes, this also happened vm.assume(member != address(0)); _addAccount(member); diff --git a/packages/ethereum-contracts/test/foundry/superfluid/SuperToken.t.sol b/packages/ethereum-contracts/test/foundry/superfluid/SuperToken.t.sol index e0bf1e7166..cfedd62934 100644 --- a/packages/ethereum-contracts/test/foundry/superfluid/SuperToken.t.sol +++ b/packages/ethereum-contracts/test/foundry/superfluid/SuperToken.t.sol @@ -24,7 +24,7 @@ contract SuperTokenIntegrationTest is FoundrySuperfluidTester { } function testToUnderlyingAmountWithUpgrade(uint8 decimals, uint256 amount) public { - vm.assume(amount < type(uint64).max); + amount = bound(amount, 0, type(uint64).max); // We assume that most underlying tokens will not have more than 32 decimals vm.assume(decimals <= 32); (TestToken localToken, ISuperToken localSuperToken) = @@ -42,10 +42,10 @@ contract SuperTokenIntegrationTest is FoundrySuperfluidTester { function testToUnderlyingAmountWithDowngrade(uint8 decimals, uint256 upgradeAmount, uint256 downgradeAmount) public { - vm.assume(upgradeAmount < type(uint64).max); + upgradeAmount = bound(upgradeAmount, 0, type(uint64).max); // We assume that most underlying tokens will not have more than 32 decimals vm.assume(decimals <= 32); - vm.assume(downgradeAmount < upgradeAmount); + downgradeAmount = bound(downgradeAmount, 0, upgradeAmount); (TestToken localToken, ISuperToken localSuperToken) = sfDeployer.deployWrapperSuperToken("FTT", "FTT", decimals, type(uint256).max, address(0)); (uint256 underlyingAmount, uint256 adjustedAmount) = localSuperToken.toUnderlyingAmount(upgradeAmount); @@ -178,4 +178,81 @@ contract SuperTokenIntegrationTest is FoundrySuperfluidTester { "testOnlyHostCanUpdateCodeWhenNoAdmin: super token logic not updated correctly" ); } + + function testPermit( + address relayer, + uint256 signerPrivKey, + uint256 amount, + address spender, + uint32 deadlineDelta + ) public { + uint256 deadline = bound(deadlineDelta, block.timestamp, block.timestamp + deadlineDelta); + amount = bound(amount, 1, type(uint96).max); + signerPrivKey = bound(signerPrivKey, 1, type(uint128).max); + address permitSigner = vm.addr(signerPrivKey); + // zero address is not a valid signer + vm.assume(permitSigner != address(0)); + // SuperToken doesn't allow approval to zero address + vm.assume(spender != address(0)); + + (ISuperToken localSuperToken) = sfDeployer.deployPureSuperToken("Super MR", "MRx", amount * 2); + localSuperToken.transfer(permitSigner, amount * 2); + uint256 nonce = localSuperToken.nonces(permitSigner); + // check nonce is 0 + assertEq(nonce, 0, "Nonce should be 0"); + + assertEq(localSuperToken.allowance(permitSigner, spender), 0, "Allowance should be 0"); + + bytes32 digest; + // stack too deep avoidance gymnastics + { + // create permit digest + bytes32 PERMIT_TYPEHASH = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + bytes32 structHash = keccak256(abi.encode(PERMIT_TYPEHASH, permitSigner, spender, amount, nonce, deadline)); + digest = keccak256( + abi.encodePacked( + "\x19\x01", + localSuperToken.DOMAIN_SEPARATOR(), + structHash + ) + ); + } + + // create signature + (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivKey, digest); + + vm.startPrank(relayer); + + // expect revert if spender doesn't match + if (spender != relayer) { + vm.expectRevert(); + localSuperToken.permit(permitSigner, relayer, amount, deadline, v, r, s); + } + + // expect revert if amount doesn't match + vm.expectRevert(); + localSuperToken.permit(permitSigner, spender, amount + 1, deadline, v, r, s); + + // expect revert if signature is invalid + vm.expectRevert(); + localSuperToken.permit(permitSigner, spender, amount, deadline, v + 1, r, s); + + // expect revert if deadline is in the past + uint256 prevBlockTS = block.timestamp; + vm.warp(block.timestamp + deadline + 1); + vm.expectRevert(); + localSuperToken.permit(permitSigner, spender, amount, deadline, v, r, s); + // restore block timestamp + vm.warp(prevBlockTS); + + // succeed with correct parameters + localSuperToken.permit(permitSigner, spender, amount, deadline, v, r, s); + + vm.stopPrank(); + + // Verify expected state changes + assertEq(localSuperToken.nonces(permitSigner), 1, "Nonce should be incremented"); + assertEq(localSuperToken.allowance(permitSigner, spender), amount, "Allowance should be set"); + } }