diff --git a/.gas-snapshot b/.gas-snapshot index 6c828a3e..d04fcbf0 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,13 +1,15 @@ -Approve2Test:testOZSafePermit() (gas: 23395) -Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129095) +Approve2Test:testInvalidateNonces() (gas: 27184) +Approve2Test:testLockdown() (gas: 432733) +Approve2Test:testOZSafePermit() (gas: 23418) +Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129118) Approve2Test:testOZSafeTransferFrom() (gas: 39310) -Approve2Test:testPermit2() (gas: 21763) -Approve2Test:testPermit2Full() (gas: 32804) -Approve2Test:testPermit2NonPermitToken() (gas: 22758) -Approve2Test:testPermit2PlusTransferFrom2() (gas: 126262) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136552) -Approve2Test:testStandardPermit() (gas: 21427) +Approve2Test:testPermit2() (gas: 21797) +Approve2Test:testPermit2Full() (gas: 32243) +Approve2Test:testPermit2NonPermitToken() (gas: 22231) +Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135604) +Approve2Test:testStandardPermit() (gas: 21405) Approve2Test:testStandardTransferFrom() (gas: 38207) -Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47928) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47929) +Approve2Test:testTransferFrom2() (gas: 38064) +Approve2Test:testTransferFrom2Full() (gas: 47574) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47531) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2a298901..9fb6382a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -13,6 +13,17 @@ jobs: with: version: nightly + - name: Set up python 3.8 + uses: actions/setup-python@v2 + with: + python-version: 3.8 + + - name: Install Vyper + run: pip install vyper + + - name: Build Vyper docs + run: vyper -f devdoc src/*.vy + - name: Install dependencies run: forge install diff --git a/foundry.toml b/foundry.toml index da461b91..6b46bda5 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,4 +1,5 @@ [profile.default] +ffi = true solc = "0.8.15" bytecode_hash = "none" optimizer_runs = 1000000 diff --git a/src/Approve2.sol b/src/Approve2.sol index 0136f61e..7dc420d1 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -27,14 +27,17 @@ contract Approve2 { // ensure no one accidentally invalidates all their nonces. require(noncesToInvalidate <= type(uint16).max); - nonces[msg.sender] += noncesToInvalidate; + // Unchecked because counter overflow should + // be impossible on any reasonable timescale + // given the cap on noncesToInvalidate above. + unchecked { + nonces[msg.sender] += noncesToInvalidate; + } } /// @notice The EIP-712 "domain separator" the contract /// will use when validating signatures for a given token. /// @param token The token to get the domain separator for. - /// @dev For calls to permitAll, the address of - /// the Approve2 contract will be used the token. function DOMAIN_SEPARATOR(address token) public view returns (bytes32) { return keccak256( @@ -52,22 +55,10 @@ contract Approve2 { ALLOWANCE STORAGE //////////////////////////////////////////////////////////////*/ - /// @notice Maps user addresses to "operator" addresses and whether they are - /// are approved to spend any amount of any token the user has approved. - mapping(address => mapping(address => bool)) public isOperator; - /// @notice Maps users to tokens to spender addresses and how much they /// are approved to spend the amount of that token the user has approved. mapping(address => mapping(ERC20 => mapping(address => uint256))) public allowance; - /// @notice Set whether an spender address is approved - /// to transfer any one of the sender's approved tokens. - /// @param operator The operator address to approve or unapprove. - /// @param approved Whether the operator is approved. - function setOperator(address operator, bool approved) external { - isOperator[msg.sender][operator] = approved; - } - /// @notice Approve a spender to transfer a specific /// amount of a specific ERC20 token from the sender. /// @param token The token to approve. @@ -106,10 +97,12 @@ contract Approve2 { bytes32 r, bytes32 s ) external { - unchecked { - // Ensure the signature's deadline has not already passed. - require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + // Ensure the signature's deadline has not already passed. + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + unchecked { // Recover the signer address from the signature. address recoveredAddress = ecrecover( keccak256( @@ -141,57 +134,6 @@ contract Approve2 { } } - /// @notice Permit a user to spend any amount of any of another - /// user's approved tokens via the owner's EIP-712 signature. - /// @param owner The user to permit spending from. - /// @param spender The user to permit spending to. - /// @param deadline The timestamp after which the signature is no longer valid. - /// @param v Must produce valid secp256k1 signature from the owner along with r and s. - /// @param r Must produce valid secp256k1 signature from the owner along with v and s. - /// @param s Must produce valid secp256k1 signature from the owner along with r and v. - /// @dev May fail if the owner's nonce was invalidated in-flight by invalidateNonce. - function permitAll( - address owner, - address spender, - uint256 deadline, - uint8 v, - bytes32 r, - bytes32 s - ) external { - unchecked { - // Ensure the signature's deadline has not already passed. - require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); - - // Recover the signer address from the signature. - address recoveredAddress = ecrecover( - keccak256( - abi.encodePacked( - "\x19\x01", - DOMAIN_SEPARATOR(address(this)), - keccak256( - abi.encode( - keccak256("PermitAll(address owner,address spender,uint256 nonce,uint256 deadline)"), - owner, - spender, - nonces[owner]++, - deadline - ) - ) - ) - ), - v, - r, - s - ); - - // Ensure the signature is valid and the signer is the owner. - require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); - - // Set isOperator for the spender to true. - isOperator[owner][spender] = true; - } - } - /*////////////////////////////////////////////////////////////// TRANSFER LOGIC //////////////////////////////////////////////////////////////*/ @@ -209,62 +151,41 @@ contract Approve2 { address to, uint256 amount ) external { - unchecked { - uint256 allowed = allowance[from][token][msg.sender]; // Saves gas for limited approvals. + uint256 allowed = allowance[from][token][msg.sender]; // Saves gas for limited approvals. - // If the from address has set an unlimited approval, we'll go straight to the transfer. - if (allowed != type(uint256).max) { - if (allowed >= amount) { - // If msg.sender has enough approved to them, decrement their allowance. - allowance[from][token][msg.sender] = allowed - amount; - } else { - // Otherwise, check if msg.sender is an operator for the - // from address, otherwise we'll revert and block the transfer. - require(isOperator[from][msg.sender], "APPROVE_ALL_REQUIRED"); - } - } + // If the from address has set an unlimited approval, we'll go straight to the transfer. + if (allowed != type(uint256).max) allowance[from][token][msg.sender] = allowed - amount; - // Transfer the tokens from the from address to the recipient. - token.safeTransferFrom(from, to, amount); - } + // Transfer the tokens from the from address to the recipient. + token.safeTransferFrom(from, to, amount); } /*////////////////////////////////////////////////////////////// LOCKDOWN LOGIC //////////////////////////////////////////////////////////////*/ - // TODO: Bench if a struct for token-spender pairs is cheaper. + struct Approval { + ERC20 token; + address spender; + } - /// @notice Enables performing a "lockdown" of the sender's Approve2 identity - /// by batch revoking approvals, unapproving operators, and invalidating nonces. - /// @param tokens An array of tokens who's corresponding spenders should have their - /// approvals revoked. Each index should correspond to an index in the spenders array. - /// @param spenders An array of addresses to revoke approvals from. - /// Each index should correspond to an index in the tokens array. - /// @param operators An array of addresses to revoke operator approval from. - function lockdown( - ERC20[] calldata tokens, - address[] calldata spenders, - address[] calldata operators, - uint256 noncesToInvalidate - ) external { + /// @notice Enables performing a "lockdown" of the sender's Approve2 + /// identity by batch revoking approvals and invalidating nonces. + /// @param approvalsToRevoke An array of approvals to revoke. + /// @param noncesToInvalidate The number of nonces to invalidate. + function lockdown(Approval[] calldata approvalsToRevoke, uint256 noncesToInvalidate) external { + // Unchecked because counter overflow is impossible + // in any environment with reasonable gas limits. unchecked { - // Will revert if trying to invalidate - // more than type(uint16).max nonces. - invalidateNonces(noncesToInvalidate); - - // Each index should correspond to an index in the other array. - require(tokens.length == spenders.length, "LENGTH_MISMATCH"); - // Revoke allowances for each pair of spenders and tokens. - for (uint256 i = 0; i < spenders.length; ++i) { - delete allowance[msg.sender][tokens[i]][spenders[i]]; - } - - // Revoke each of the sender's provided operator's powers. - for (uint256 i = 0; i < operators.length; ++i) { - delete isOperator[msg.sender][operators[i]]; + for (uint256 i = 0; i < approvalsToRevoke.length; ++i) { + // TODO: Can this be optimized? + delete allowance[msg.sender][approvalsToRevoke[i].token][approvalsToRevoke[i].spender]; } } + + // Will revert if trying to invalidate + // more than type(uint16).max nonces. + invalidateNonces(noncesToInvalidate); } } diff --git a/src/Approve2.vy b/src/Approve2.vy new file mode 100644 index 00000000..5cee7669 --- /dev/null +++ b/src/Approve2.vy @@ -0,0 +1,199 @@ +# @version 0.3.4 + +""" +@title Approve2 +@license MIT +@author transmissions11 +@notice + Backwards compatible, low-overhead, + next generation token approval/meta-tx system. +""" + +from vyper.interfaces import ERC20 + +# TODO: Hevm equivalence seems to not be working...? Is it storage layout or what? +# For reference, can test individual funcs with --sig "invalidateNonces(uint256)" + +################################################################ +# STORAGE # +################################################################ + +# Maps addresses to their current nonces. Used to prevent replay +# attacks and allow invalidating active permits via invalidateNonce. +nonces: public(HashMap[address, uint256]) + +# Maps users to tokens to spender addresses and how much they are +# approved to spend the amount of that token the user has approved. +allowance: public(HashMap[address, HashMap[ERC20, HashMap[address, uint256]]]) + +################################################################ +# TRANSFERFROM LOGIC # +################################################################ + +@external +def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): + + """ + @notice + Transfer approved tokens from one address to another. + @dev + Requires either the from address to have approved at least the desired amount of + tokens or msg.sender to be approved to manage all of the from addresses's tokens. + @param token The token to transfer. + @param owner The address to transfer from. + @param to The address to transfer to. + @param amount The amount of tokens to transfer. + """ + + allowed: uint256 = self.allowance[owner][token][msg.sender] + + if allowed != max_value(uint256): self.allowance[owner][token][msg.sender] = allowed - amount + + token.transferFrom(owner, to, amount, default_return_value=True, skip_contract_check=True) + +################################################################ +# PERMIT LOGIC # +################################################################ + +@external +def permit(token: ERC20, owner: address, spender: address, amount: uint256, deadline: uint256, v: uint8, r: bytes32, s: bytes32): + + """ + @notice + Permit a user to spend an amount of another user's approved + amount of the given token via the owner's EIP-712 signature. + @dev May fail if the nonce was invalidated by invalidateNonce. + @param token The token to permit spending. + @param owner The user to permit spending from. + @param spender The user to permit spending to. + @param amount The amount to permit spending. + @param deadline The timestamp after which the signature is no longer valid. + @param v Must produce valid secp256k1 signature from the owner along with r and s. + @param r Must produce valid secp256k1 signature from the owner along with v and s. + @param s Must produce valid secp256k1 signature from the owner along with r and v. + """ + + assert deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" + + assert owner != empty(address), "INVALID_OWNER" + + nonce: uint256 = self.nonces[owner] + + digest: bytes32 = keccak256( + concat( + b'\x19\x01', + self.computeDomainSeperator(token), + keccak256( + concat( + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"), + convert(owner, bytes32), + convert(spender, bytes32), + convert(amount, bytes32), + convert(nonce, bytes32), + convert(deadline, bytes32), + ) + ) + ) + ) + + recoveredAddress: address = ecrecover(digest, + convert(v, uint256), + convert(r, uint256), + convert(s, uint256) + ) + + assert recoveredAddress == owner, "INVALID_SIGNER" + + self.allowance[owner][token][spender] = amount + self.nonces[owner] = unsafe_add(nonce, 1) + +################################################################ +# LOCKDOWN LOGIC # +################################################################ + +struct Approval: + token: ERC20 + spender: address + +# TODO Test gas and if it works with non dyamic arrays. +@external +def lockdown(approvalsToRevoke: DynArray[Approval, 500], noncesToInvalidate: uint256): + + """ + @notice + Enables performing a "lockdown" of the sender's Approve2 + identity by batch revoking approvals and invalidating nonces. + @param approvalsToRevoke An array of approvals to revoke. + @param noncesToInvalidate The number of nonces to invalidate. + """ + # TODO Can we optimize the loop? + for approval in approvalsToRevoke: self.allowance[msg.sender][approval.token][approval.spender] = 0 + + # TODO Needs a 2**16 check. + self.nonces[msg.sender] += noncesToInvalidate # TODO This can be made unsafe, overflow unlikely. + +################################################################ +# NONCE INVALIDATION LOGIC # +################################################################ + +@external +def invalidateNonces(noncesToInvalidate: uint256): + + """ + @notice + Invalidate a specific number of nonces. Can be used + to invalidate in-flight permits before they are executed. + @param noncesToInvalidate The number of nonces to invalidate. + """ + + assert noncesToInvalidate < 2 ** 16 # TODO Do we need to extract this into a constant? + + self.nonces[msg.sender] += noncesToInvalidate # TODO This can be made unsafe, overflow unlikely. + +################################################################ +# MANUAL APPROVAL LOGIC # +################################################################ + +@external +def approve(token: ERC20, spender: address, amount: uint256): + + """ + @notice + Manually approve a spender to transfer a specific + amount of a specific ERC20 token from the sender. + @param token The token to approve. + @param spender The spender address to approve. + @param amount The amount of the token to approve. + """ + + self.allowance[msg.sender][token][spender] = amount + +################################################################ +# DOMAIN SEPERATOR LOGIC # +################################################################ + +@view +@external +def DOMAIN_SEPARATOR(token: ERC20) -> bytes32: + + """ + @notice + The EIP-712 "domain separator" the contract + will use when validating signatures for a given token. + @param token The token to get the domain separator for. + """ + + return self.computeDomainSeperator(token) + +@view +@internal +def computeDomainSeperator(token: ERC20) -> bytes32: + return keccak256( + _abi_encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256("Approve2"), + keccak256("1"), + chain.id, + token + ) + ) \ No newline at end of file diff --git a/test/Approve2.t.sol b/test/Approve2.t.sol index c46a8330..5557599a 100644 --- a/test/Approve2.t.sol +++ b/test/Approve2.t.sol @@ -11,6 +11,9 @@ import {Approve2Lib} from "../src/Approve2Lib.sol"; import {MockNonPermitERC20} from "./mocks/MockNonPermitERC20.sol"; +// TODO: Fuzzing and coverage of Approve2.sol and Approve2.vy. +// TODO: Test and bench functions like lockdown and invalidateNonces. + contract Approve2Test is DSTestPlus { bytes32 constant PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); @@ -23,12 +26,27 @@ contract Approve2Test is DSTestPlus { uint256 immutable PK; address immutable PK_OWNER; - Approve2 immutable approve2 = new Approve2(); + Approve2 immutable approve2 = Approve2(deployContract("Approve2")); MockERC20 immutable token = new MockERC20("Mock Token", "MOCK", 18); MockNonPermitERC20 immutable nonPermitToken = new MockNonPermitERC20("Mock NonPermit Token", "MOCK", 18); + // TODO: Make this a lib in solmate? + function deployContract(string memory fileName) public returns (address deployedAddress) { + string[] memory cmds = new string[](2); + cmds[0] = "vyper"; + cmds[1] = string.concat("src/", fileName, ".vy"); + + bytes memory bytecode = hevm.ffi(cmds); + + assembly { + deployedAddress := create(0, add(bytecode, 32), mload(bytecode)) + } + + require(deployedAddress != address(0), "DEPLOYMENT_FAILED"); + } + constructor() { PK = 0xBEEF; PK_OWNER = hevm.addr(PK); @@ -116,6 +134,20 @@ contract Approve2Test is DSTestPlus { Approve2Lib.permit2(token, PK_OWNER, address(0xB00B), 1e18, block.timestamp, v, r, s); } + function testInvalidateNonces() public { + assertEq(approve2.nonces(address(this)), 0); + + approve2.invalidateNonces(10); + + assertEq(approve2.nonces(address(this)), 10); + } + + function testLockdown() public { + Approve2.Approval[] memory approvals = new Approve2.Approval[](500); + + approve2.lockdown(approvals, 10); + } + /*////////////////////////////////////////////////////////////// BASIC TRANSFERFROM2 BENCHMARKS //////////////////////////////////////////////////////////////*/