From 4eea7e9beede67b0545bd12ba9b3fb27ce4b1662 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 16:27:45 -0700 Subject: [PATCH 01/28] =?UTF-8?q?=F0=9F=9A=A7=20WIP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 14 +++---- src/Approve2.vy | 96 +++++++++++++++++++++++++++++++++++++++++++++ test/Approve2.t.sol | 16 +++++++- 3 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 src/Approve2.vy diff --git a/.gas-snapshot b/.gas-snapshot index 6c828a3e..ef116a37 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,13 +1,13 @@ Approve2Test:testOZSafePermit() (gas: 23395) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129095) 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:testPermit2() (gas: 21774) +Approve2Test:testPermit2Full() (gas: 32744) +Approve2Test:testPermit2NonPermitToken() (gas: 22687) +Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136623) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47928) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47929) +Approve2Test:testTransferFrom2Full() (gas: 48070) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 48071) diff --git a/src/Approve2.vy b/src/Approve2.vy new file mode 100644 index 00000000..31f30928 --- /dev/null +++ b/src/Approve2.vy @@ -0,0 +1,96 @@ +from vyper.interfaces import ERC20 + +nonces: public(HashMap[address, uint256]) +isOperator: public(HashMap[address, HashMap[address, bool]]) +allowance: public(HashMap[address, HashMap[address, HashMap[address, uint256]]]) + +@external +def invalidateNonces(noncesToInvalidate: uint256): + assert noncesToInvalidate < 2 ** 16 + + self.nonces[msg.sender] += noncesToInvalidate + +@external +def setOperator(operator: address, approved: bool): + self.isOperator[msg.sender][operator] = approved + +@external +def approve(token: address, spender: address, amount: uint256): + self.allowance[msg.sender][token][spender] = amount + +@internal +def computeDomainSeperator(token: address) -> bytes32: + return keccak256( + concat( + keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), + keccak256(convert("Yearn Vault", Bytes[11])), + keccak256(convert("1", Bytes[28])), + convert(chain.id, bytes32), + convert(token, bytes32) + ) + ) + +@external +def DOMAIN_SEPARATOR(token: address) -> bytes32: + return self.computeDomainSeperator(token) + +@external +def permit(token: address, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): + assert expiry >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" + + 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(expiry, bytes32), + ) + ) + ) + ) + + recoveredAddress: address = ecrecover(digest, convert(v, uint256), convert(r, uint256), convert(s, uint256)) + + # assert receiver not in [self, ZERO_ADDRESS] + assert recoveredAddress != ZERO_ADDRESS and recoveredAddress == owner, "INVALID_SIGNER" + + self.allowance[owner][token][spender] = amount + self.nonces[owner] = nonce + 1 + +@internal +def erc20_safe_transferFrom(token: address, sender: address, receiver: address, amount: uint256): + # Used only to send tokens that are not the type managed by this Vault. + # HACK: Used to handle non-compliant tokens like USDT + response: Bytes[32] = raw_call( + token, + concat( + method_id("transferFrom(address,address,uint256)"), + convert(sender, bytes32), + convert(receiver, bytes32), + convert(amount, bytes32), + ), + max_outsize=32, + ) + if len(response) > 0: + assert convert(response, bool), "TRANSFER_FROM_FAILED" + +@external +def transferFrom(token: address, owner: address, to: address, amount: uint256): + allowed: uint256 = self.allowance[owner][token][msg.sender] + + if allowed != MAX_UINT256: + if allowed >= amount: + # todo: vyper has safe math right? + self.allowance[owner][token][msg.sender] = allowed - amount + else: + assert self.isOperator[owner][msg.sender], "APPROVE_ALL_REQUIRED" + + self.erc20_safe_transferFrom(token, owner, to, amount) \ No newline at end of file diff --git a/test/Approve2.t.sol b/test/Approve2.t.sol index c46a8330..eca36240 100644 --- a/test/Approve2.t.sol +++ b/test/Approve2.t.sol @@ -23,12 +23,26 @@ 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); + 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); From 306ba1f153e3c3e313c0901fcd27a9a76406371e Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 16:29:41 -0700 Subject: [PATCH 02/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Inline=20safetransfe?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 30 ++++++++++++------------------ 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ef116a37..afdb0a72 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -5,9 +5,9 @@ Approve2Test:testPermit2() (gas: 21774) Approve2Test:testPermit2Full() (gas: 32744) Approve2Test:testPermit2NonPermitToken() (gas: 22687) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136623) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136539) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 48070) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 48071) +Approve2Test:testTransferFrom2Full() (gas: 47986) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47987) diff --git a/src/Approve2.vy b/src/Approve2.vy index 31f30928..f131fa57 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -65,23 +65,6 @@ def permit(token: address, owner: address, spender: address, amount: uint256, ex self.allowance[owner][token][spender] = amount self.nonces[owner] = nonce + 1 -@internal -def erc20_safe_transferFrom(token: address, sender: address, receiver: address, amount: uint256): - # Used only to send tokens that are not the type managed by this Vault. - # HACK: Used to handle non-compliant tokens like USDT - response: Bytes[32] = raw_call( - token, - concat( - method_id("transferFrom(address,address,uint256)"), - convert(sender, bytes32), - convert(receiver, bytes32), - convert(amount, bytes32), - ), - max_outsize=32, - ) - if len(response) > 0: - assert convert(response, bool), "TRANSFER_FROM_FAILED" - @external def transferFrom(token: address, owner: address, to: address, amount: uint256): allowed: uint256 = self.allowance[owner][token][msg.sender] @@ -93,4 +76,15 @@ def transferFrom(token: address, owner: address, to: address, amount: uint256): else: assert self.isOperator[owner][msg.sender], "APPROVE_ALL_REQUIRED" - self.erc20_safe_transferFrom(token, owner, to, amount) \ No newline at end of file + response: Bytes[32] = raw_call( + token, + concat( + method_id("transferFrom(address,address,uint256)"), + convert(owner, bytes32), + convert(to, bytes32), + convert(amount, bytes32), + ), + max_outsize=32, + ) + + if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" \ No newline at end of file From b7ad2134ba19e55b2ad9d33bda51113074aa42bb Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 16:40:24 -0700 Subject: [PATCH 03/28] =?UTF-8?q?=F0=9F=91=B7=E2=80=8D=E2=99=82=EF=B8=8F?= =?UTF-8?q?=20Update=20vyper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index afdb0a72..69ac4386 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -2,12 +2,12 @@ Approve2Test:testOZSafePermit() (gas: 23395) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129095) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21774) -Approve2Test:testPermit2Full() (gas: 32744) -Approve2Test:testPermit2NonPermitToken() (gas: 22687) +Approve2Test:testPermit2Full() (gas: 32625) +Approve2Test:testPermit2NonPermitToken() (gas: 22568) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136539) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136405) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47986) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47987) +Approve2Test:testTransferFrom2Full() (gas: 47971) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47972) From 44162d028ed8bcce75fc8431a6a8059c7818f2e6 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 16:44:09 -0700 Subject: [PATCH 04/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Reordering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 10 ++++---- src/Approve2.vy | 64 ++++++++++++++++++++++++------------------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 69ac4386..4d201d57 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -2,12 +2,12 @@ Approve2Test:testOZSafePermit() (gas: 23395) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129095) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21774) -Approve2Test:testPermit2Full() (gas: 32625) -Approve2Test:testPermit2NonPermitToken() (gas: 22568) +Approve2Test:testPermit2Full() (gas: 32556) +Approve2Test:testPermit2NonPermitToken() (gas: 22499) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136405) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136221) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47971) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47972) +Approve2Test:testTransferFrom2Full() (gas: 47856) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47857) diff --git a/src/Approve2.vy b/src/Approve2.vy index f131fa57..d060c348 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -5,18 +5,28 @@ isOperator: public(HashMap[address, HashMap[address, bool]]) allowance: public(HashMap[address, HashMap[address, HashMap[address, uint256]]]) @external -def invalidateNonces(noncesToInvalidate: uint256): - assert noncesToInvalidate < 2 ** 16 +def transferFrom(token: address, owner: address, to: address, amount: uint256): + allowed: uint256 = self.allowance[owner][token][msg.sender] - self.nonces[msg.sender] += noncesToInvalidate + if allowed != MAX_UINT256: + if allowed >= amount: + # todo: vyper has safe math right? + self.allowance[owner][token][msg.sender] = allowed - amount + else: + assert self.isOperator[owner][msg.sender], "APPROVE_ALL_REQUIRED" -@external -def setOperator(operator: address, approved: bool): - self.isOperator[msg.sender][operator] = approved + response: Bytes[32] = raw_call( + token, + concat( + method_id("transferFrom(address,address,uint256)"), + convert(owner, bytes32), + convert(to, bytes32), + convert(amount, bytes32), + ), + max_outsize=32, + ) -@external -def approve(token: address, spender: address, amount: uint256): - self.allowance[msg.sender][token][spender] = amount + if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" @internal def computeDomainSeperator(token: address) -> bytes32: @@ -30,10 +40,6 @@ def computeDomainSeperator(token: address) -> bytes32: ) ) -@external -def DOMAIN_SEPARATOR(token: address) -> bytes32: - return self.computeDomainSeperator(token) - @external def permit(token: address, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): assert expiry >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" @@ -66,25 +72,19 @@ def permit(token: address, owner: address, spender: address, amount: uint256, ex self.nonces[owner] = nonce + 1 @external -def transferFrom(token: address, owner: address, to: address, amount: uint256): - allowed: uint256 = self.allowance[owner][token][msg.sender] +def invalidateNonces(noncesToInvalidate: uint256): + assert noncesToInvalidate < 2 ** 16 - if allowed != MAX_UINT256: - if allowed >= amount: - # todo: vyper has safe math right? - self.allowance[owner][token][msg.sender] = allowed - amount - else: - assert self.isOperator[owner][msg.sender], "APPROVE_ALL_REQUIRED" + self.nonces[msg.sender] += noncesToInvalidate - response: Bytes[32] = raw_call( - token, - concat( - method_id("transferFrom(address,address,uint256)"), - convert(owner, bytes32), - convert(to, bytes32), - convert(amount, bytes32), - ), - max_outsize=32, - ) +@external +def setOperator(operator: address, approved: bool): + self.isOperator[msg.sender][operator] = approved - if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" \ No newline at end of file +@external +def approve(token: address, spender: address, amount: uint256): + self.allowance[msg.sender][token][spender] = amount + +@external +def DOMAIN_SEPARATOR(token: address) -> bytes32: + return self.computeDomainSeperator(token) \ No newline at end of file From 8b136b94fd2157bd864a385cce1b579a84315eac Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 17:24:40 -0700 Subject: [PATCH 05/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.vy | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Approve2.vy b/src/Approve2.vy index d060c348..874b0734 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -28,6 +28,7 @@ def transferFrom(token: address, owner: address, to: address, amount: uint256): if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" +@view @internal def computeDomainSeperator(token: address) -> bytes32: return keccak256( @@ -63,9 +64,12 @@ def permit(token: address, owner: address, spender: address, amount: uint256, ex ) ) - recoveredAddress: address = ecrecover(digest, convert(v, uint256), convert(r, uint256), convert(s, uint256)) + recoveredAddress: address = ecrecover(digest, + convert(v, uint256), + convert(r, uint256), + convert(s, uint256) + ) - # assert receiver not in [self, ZERO_ADDRESS] assert recoveredAddress != ZERO_ADDRESS and recoveredAddress == owner, "INVALID_SIGNER" self.allowance[owner][token][spender] = amount @@ -85,6 +89,7 @@ def setOperator(operator: address, approved: bool): def approve(token: address, spender: address, amount: uint256): self.allowance[msg.sender][token][spender] = amount +@view @external def DOMAIN_SEPARATOR(token: address) -> bytes32: return self.computeDomainSeperator(token) \ No newline at end of file From 2b9886e01b6111031c837e7021aa4d73a9399e20 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 21:47:33 -0700 Subject: [PATCH 06/28] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Optimize?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 30 +++++++++++++++--------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 4d201d57..78c83793 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -2,10 +2,10 @@ Approve2Test:testOZSafePermit() (gas: 23395) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129095) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21774) -Approve2Test:testPermit2Full() (gas: 32556) -Approve2Test:testPermit2NonPermitToken() (gas: 22499) +Approve2Test:testPermit2Full() (gas: 32526) +Approve2Test:testPermit2NonPermitToken() (gas: 22469) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136221) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136191) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) diff --git a/src/Approve2.vy b/src/Approve2.vy index 874b0734..deedb754 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -28,19 +28,6 @@ def transferFrom(token: address, owner: address, to: address, amount: uint256): if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" -@view -@internal -def computeDomainSeperator(token: address) -> bytes32: - return keccak256( - concat( - keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), - keccak256(convert("Yearn Vault", Bytes[11])), - keccak256(convert("1", Bytes[28])), - convert(chain.id, bytes32), - convert(token, bytes32) - ) - ) - @external def permit(token: address, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): assert expiry >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" @@ -73,7 +60,7 @@ def permit(token: address, owner: address, spender: address, amount: uint256, ex assert recoveredAddress != ZERO_ADDRESS and recoveredAddress == owner, "INVALID_SIGNER" self.allowance[owner][token][spender] = amount - self.nonces[owner] = nonce + 1 + self.nonces[owner] = unsafe_add(nonce, 1) @external def invalidateNonces(noncesToInvalidate: uint256): @@ -92,4 +79,17 @@ def approve(token: address, spender: address, amount: uint256): @view @external def DOMAIN_SEPARATOR(token: address) -> bytes32: - return self.computeDomainSeperator(token) \ No newline at end of file + return self.computeDomainSeperator(token) + +@view +@internal +def computeDomainSeperator(token: address) -> bytes32: + return keccak256( + concat( + keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), + keccak256(convert("Yearn Vault", Bytes[11])), + keccak256(convert("1", Bytes[28])), + convert(chain.id, bytes32), + convert(token, bytes32) + ) + ) \ No newline at end of file From 7016abab229233d0d52c44d8f87890d7b8f7a94e Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 21:48:13 -0700 Subject: [PATCH 07/28] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Optimize?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 78c83793..dfa73762 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -5,9 +5,9 @@ Approve2Test:testPermit2() (gas: 21774) Approve2Test:testPermit2Full() (gas: 32526) Approve2Test:testPermit2NonPermitToken() (gas: 22469) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136191) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136153) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47856) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47857) +Approve2Test:testTransferFrom2Full() (gas: 47818) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47819) diff --git a/src/Approve2.vy b/src/Approve2.vy index deedb754..a179e52a 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -11,7 +11,7 @@ def transferFrom(token: address, owner: address, to: address, amount: uint256): if allowed != MAX_UINT256: if allowed >= amount: # todo: vyper has safe math right? - self.allowance[owner][token][msg.sender] = allowed - amount + self.allowance[owner][token][msg.sender] = unsafe_sub(allowed, amount) else: assert self.isOperator[owner][msg.sender], "APPROVE_ALL_REQUIRED" From f50353dde4a7fe18c0abe707b07be5a56aa5f895 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 22:19:24 -0700 Subject: [PATCH 08/28] =?UTF-8?q?=F0=9F=94=A5=20Remove=20isOperator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 10 +++++----- src/Approve2.vy | 12 +----------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index dfa73762..ea136318 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -2,12 +2,12 @@ Approve2Test:testOZSafePermit() (gas: 23395) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129095) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21774) -Approve2Test:testPermit2Full() (gas: 32526) -Approve2Test:testPermit2NonPermitToken() (gas: 22469) +Approve2Test:testPermit2Full() (gas: 32503) +Approve2Test:testPermit2NonPermitToken() (gas: 22446) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136153) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136136) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47818) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47819) +Approve2Test:testTransferFrom2Full() (gas: 47824) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47825) diff --git a/src/Approve2.vy b/src/Approve2.vy index a179e52a..f12860dd 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -1,19 +1,13 @@ from vyper.interfaces import ERC20 nonces: public(HashMap[address, uint256]) -isOperator: public(HashMap[address, HashMap[address, bool]]) allowance: public(HashMap[address, HashMap[address, HashMap[address, uint256]]]) @external def transferFrom(token: address, owner: address, to: address, amount: uint256): allowed: uint256 = self.allowance[owner][token][msg.sender] - if allowed != MAX_UINT256: - if allowed >= amount: - # todo: vyper has safe math right? - self.allowance[owner][token][msg.sender] = unsafe_sub(allowed, amount) - else: - assert self.isOperator[owner][msg.sender], "APPROVE_ALL_REQUIRED" + if allowed != MAX_UINT256: self.allowance[owner][token][msg.sender] = allowed - amount response: Bytes[32] = raw_call( token, @@ -68,10 +62,6 @@ def invalidateNonces(noncesToInvalidate: uint256): self.nonces[msg.sender] += noncesToInvalidate -@external -def setOperator(operator: address, approved: bool): - self.isOperator[msg.sender][operator] = approved - @external def approve(token: address, spender: address, amount: uint256): self.allowance[msg.sender][token][spender] = amount From 7967ce4166ac11f5288da9a744835669dba3a187 Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 22:20:42 -0700 Subject: [PATCH 09/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.vy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Approve2.vy b/src/Approve2.vy index f12860dd..a98d96f2 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -7,7 +7,7 @@ allowance: public(HashMap[address, HashMap[address, HashMap[address, uint256]]]) def transferFrom(token: address, owner: address, to: address, amount: uint256): allowed: uint256 = self.allowance[owner][token][msg.sender] - if allowed != MAX_UINT256: self.allowance[owner][token][msg.sender] = allowed - amount + if allowed != max_value(uint256): self.allowance[owner][token][msg.sender] = allowed - amount response: Bytes[32] = raw_call( token, @@ -51,7 +51,7 @@ def permit(token: address, owner: address, spender: address, amount: uint256, ex convert(s, uint256) ) - assert recoveredAddress != ZERO_ADDRESS and recoveredAddress == owner, "INVALID_SIGNER" + assert recoveredAddress != empty(address) and recoveredAddress == owner, "INVALID_SIGNER" self.allowance[owner][token][spender] = amount self.nonces[owner] = unsafe_add(nonce, 1) From e5fe831b70b04dd150a9cc5e0c71938ead92ac7d Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 22:29:16 -0700 Subject: [PATCH 10/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Use=20ERC20=20interf?= =?UTF-8?q?ace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.vy | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Approve2.vy b/src/Approve2.vy index a98d96f2..8b6e3d92 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -1,16 +1,16 @@ from vyper.interfaces import ERC20 nonces: public(HashMap[address, uint256]) -allowance: public(HashMap[address, HashMap[address, HashMap[address, uint256]]]) +allowance: public(HashMap[address, HashMap[ERC20, HashMap[address, uint256]]]) @external -def transferFrom(token: address, owner: address, to: address, amount: uint256): +def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): allowed: uint256 = self.allowance[owner][token][msg.sender] if allowed != max_value(uint256): self.allowance[owner][token][msg.sender] = allowed - amount response: Bytes[32] = raw_call( - token, + token.address, concat( method_id("transferFrom(address,address,uint256)"), convert(owner, bytes32), @@ -23,7 +23,7 @@ def transferFrom(token: address, owner: address, to: address, amount: uint256): if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" @external -def permit(token: address, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): +def permit(token: ERC20, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): assert expiry >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" nonce: uint256 = self.nonces[owner] @@ -63,22 +63,22 @@ def invalidateNonces(noncesToInvalidate: uint256): self.nonces[msg.sender] += noncesToInvalidate @external -def approve(token: address, spender: address, amount: uint256): +def approve(token: ERC20, spender: address, amount: uint256): self.allowance[msg.sender][token][spender] = amount @view @external -def DOMAIN_SEPARATOR(token: address) -> bytes32: +def DOMAIN_SEPARATOR(token: ERC20) -> bytes32: return self.computeDomainSeperator(token) @view @internal -def computeDomainSeperator(token: address) -> bytes32: +def computeDomainSeperator(token: ERC20) -> bytes32: return keccak256( concat( keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), - keccak256(convert("Yearn Vault", Bytes[11])), - keccak256(convert("1", Bytes[28])), + keccak256(convert("Yearn Vault", Bytes[11])), # todo rename + keccak256(convert("1", Bytes[28])), # todo why so many bytes convert(chain.id, bytes32), convert(token, bytes32) ) From fe70c4f334f499fc5759a0815cf7c4cea5b87dab Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 22:30:23 -0700 Subject: [PATCH 11/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Use=20default=5Fretu?= =?UTF-8?q?rn=5Fvalue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 13 +------------ 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ea136318..a1bb2ea0 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -5,9 +5,9 @@ Approve2Test:testPermit2() (gas: 21774) Approve2Test:testPermit2Full() (gas: 32503) Approve2Test:testPermit2NonPermitToken() (gas: 22446) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 136136) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135907) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47824) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47825) +Approve2Test:testTransferFrom2Full() (gas: 47595) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47596) diff --git a/src/Approve2.vy b/src/Approve2.vy index 8b6e3d92..742d262a 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -9,18 +9,7 @@ def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): if allowed != max_value(uint256): self.allowance[owner][token][msg.sender] = allowed - amount - response: Bytes[32] = raw_call( - token.address, - concat( - method_id("transferFrom(address,address,uint256)"), - convert(owner, bytes32), - convert(to, bytes32), - convert(amount, bytes32), - ), - max_outsize=32, - ) - - if len(response) > 0: assert convert(response, bool), "TRANSFER_FROM_FAILED" + token.transferFrom(owner, to, amount, default_return_value=True) # use skip_contract_check? @external def permit(token: ERC20, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): From cea31b729dcf09ce0a03c82914698856bc96bd4e Mon Sep 17 00:00:00 2001 From: t11s Date: Fri, 29 Jul 2022 22:31:31 -0700 Subject: [PATCH 12/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Use=20skip=5Fcontrac?= =?UTF-8?q?t=5Fcheck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index a1bb2ea0..4e15cd81 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -5,9 +5,9 @@ Approve2Test:testPermit2() (gas: 21774) Approve2Test:testPermit2Full() (gas: 32503) Approve2Test:testPermit2NonPermitToken() (gas: 22446) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135907) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135886) Approve2Test:testStandardPermit() (gas: 21427) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38086) -Approve2Test:testTransferFrom2Full() (gas: 47595) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47596) +Approve2Test:testTransferFrom2Full() (gas: 47574) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47575) diff --git a/src/Approve2.vy b/src/Approve2.vy index 742d262a..211fbc74 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -9,7 +9,7 @@ def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): if allowed != max_value(uint256): self.allowance[owner][token][msg.sender] = allowed - amount - token.transferFrom(owner, to, amount, default_return_value=True) # use skip_contract_check? + token.transferFrom(owner, to, amount, default_return_value=True, skip_contract_check=True) @external def permit(token: ERC20, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): From e7b777f113af548a058d703721ec0a0e7cc7e31e Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 00:48:16 -0700 Subject: [PATCH 13/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.sol | 85 ++---------------------------------------------- src/Approve2.vy | 32 ++++++++++++++++-- 2 files changed, 32 insertions(+), 85 deletions(-) diff --git a/src/Approve2.sol b/src/Approve2.sol index 0136f61e..41051db7 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -52,22 +52,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. @@ -141,57 +129,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 //////////////////////////////////////////////////////////////*/ @@ -213,16 +150,7 @@ contract Approve2 { 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 (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); @@ -235,17 +163,15 @@ contract Approve2 { // TODO: Bench if a struct for token-spender pairs is cheaper. - /// @notice Enables performing a "lockdown" of the sender's Approve2 identity - /// by batch revoking approvals, unapproving operators, and invalidating nonces. + /// @notice Enables performing a "lockdown" of the sender's Approve2 + /// identity by batch revoking approvals 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 { unchecked { @@ -260,11 +186,6 @@ contract Approve2 { 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]]; - } } } } diff --git a/src/Approve2.vy b/src/Approve2.vy index 211fbc74..23358358 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -1,8 +1,18 @@ from vyper.interfaces import ERC20 +################################################################ +# STORAGE # +################################################################ + nonces: public(HashMap[address, uint256]) + allowance: public(HashMap[address, HashMap[ERC20, HashMap[address, uint256]]]) + +################################################################ +# TRANSFERFROM LOGIC # +################################################################ + @external def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): allowed: uint256 = self.allowance[owner][token][msg.sender] @@ -11,6 +21,10 @@ def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): 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, expiry: uint256, v: uint8, r: bytes32, s: bytes32): assert expiry >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" @@ -45,16 +59,28 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, expi self.allowance[owner][token][spender] = amount self.nonces[owner] = unsafe_add(nonce, 1) +################################################################ +# NONCE INVALIDATION LOGIC # +################################################################ + @external def invalidateNonces(noncesToInvalidate: uint256): - assert noncesToInvalidate < 2 ** 16 + assert noncesToInvalidate < 2 ** 16 # todo do we need to extract this into a constant? self.nonces[msg.sender] += noncesToInvalidate +################################################################ +# MANUAL APPROVAL LOGIC # +################################################################ + @external def approve(token: ERC20, spender: address, amount: uint256): self.allowance[msg.sender][token][spender] = amount +################################################################ +# DOMAIN SEPERATOR LOGIC # +################################################################ + @view @external def DOMAIN_SEPARATOR(token: ERC20) -> bytes32: @@ -66,8 +92,8 @@ def computeDomainSeperator(token: ERC20) -> bytes32: return keccak256( concat( keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), - keccak256(convert("Yearn Vault", Bytes[11])), # todo rename - keccak256(convert("1", Bytes[28])), # todo why so many bytes + keccak256(convert("Approve2", Bytes[8])), + keccak256(convert("1", Bytes[1])), convert(chain.id, bytes32), convert(token, bytes32) ) From 88fdd6bb902fa18820228419cff3324d3b8209e3 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 01:08:47 -0700 Subject: [PATCH 14/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- foundry.toml | 1 + src/Approve2.sol | 2 -- src/Approve2.vy | 74 ++++++++++++++++++++++++++++++++++++++++++--- test/Approve2.t.sol | 1 + 4 files changed, 71 insertions(+), 7 deletions(-) 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 41051db7..f69dabc0 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -33,8 +33,6 @@ contract Approve2 { /// @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( diff --git a/src/Approve2.vy b/src/Approve2.vy index 23358358..b178f0de 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -1,20 +1,47 @@ +""" +@title Approve2 +@license MIT +@author transmissions11 +@notice + Backwards compatible, low-overhead, + next generation token approval/meta-tx system. +""" + +# TODO Run vyper in CI, make sure to compile docs. +# TODO Add the lockdown feature to the contract. + from vyper.interfaces import ERC20 ################################################################ # 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 @@ -26,8 +53,23 @@ def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): ################################################################ @external -def permit(token: ERC20, owner: address, spender: address, amount: uint256, expiry: uint256, v: uint8, r: bytes32, s: bytes32): - assert expiry >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" +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" nonce: uint256 = self.nonces[owner] @@ -42,7 +84,7 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, expi convert(spender, bytes32), convert(amount, bytes32), convert(nonce, bytes32), - convert(expiry, bytes32), + convert(deadline, bytes32), ) ) ) @@ -65,7 +107,14 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, expi @external def invalidateNonces(noncesToInvalidate: uint256): - assert noncesToInvalidate < 2 ** 16 # todo do we need to extract this into a constant? + """ + @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 @@ -75,6 +124,15 @@ def invalidateNonces(noncesToInvalidate: uint256): @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 ################################################################ @@ -84,6 +142,12 @@ def approve(token: ERC20, spender: address, amount: uint256): @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 diff --git a/test/Approve2.t.sol b/test/Approve2.t.sol index eca36240..328859a8 100644 --- a/test/Approve2.t.sol +++ b/test/Approve2.t.sol @@ -29,6 +29,7 @@ contract Approve2Test is DSTestPlus { 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"; From fd0af787d031350a5a3e7791d14920f28a6b9122 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 01:46:13 -0700 Subject: [PATCH 15/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.vy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Approve2.vy b/src/Approve2.vy index b178f0de..a277135b 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -114,7 +114,7 @@ def invalidateNonces(noncesToInvalidate: uint256): @param noncesToInvalidate The number of nonces to invalidate. """ - assert noncesToInvalidate < 2 ** 16 # TODO do we need to extract this into a constant? + assert noncesToInvalidate < 2 ** 16 # TODO Do we need to extract this into a constant? self.nonces[msg.sender] += noncesToInvalidate From 7b5b4139ebbf1ec4442ad4ba48f5e17462d0e5bb Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 01:48:05 -0700 Subject: [PATCH 16/28] =?UTF-8?q?=F0=9F=91=B7=E2=80=8D=E2=99=82=EF=B8=8F?= =?UTF-8?q?=20Add=20vyper=20to=20CI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/tests.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2a298901..15d24339 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -13,6 +13,14 @@ 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: Install dependencies run: forge install From 249b8f8941dc4f39e47ecd53ce859894ef5b87f2 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 01:54:37 -0700 Subject: [PATCH 17/28] =?UTF-8?q?=F0=9F=91=B7=E2=80=8D=E2=99=82=EF=B8=8F?= =?UTF-8?q?=20Build=20Vyper=20docs=20in=20CI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/tests.yml | 5 ++++- src/Approve2.vy | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 15d24339..9fb6382a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -18,9 +18,12 @@ jobs: with: python-version: 3.8 - - name: Install vyper + - 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/src/Approve2.vy b/src/Approve2.vy index a277135b..a8a5e88a 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -7,7 +7,6 @@ next generation token approval/meta-tx system. """ -# TODO Run vyper in CI, make sure to compile docs. # TODO Add the lockdown feature to the contract. from vyper.interfaces import ERC20 From e0d37eb26fb20ec3ba6fa40064c1bc6ed5351cc8 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 01:57:26 -0700 Subject: [PATCH 18/28] =?UTF-8?q?=F0=9F=93=9D=20TODOs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/Approve2.t.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/Approve2.t.sol b/test/Approve2.t.sol index 328859a8..232f6e21 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. +// 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)"); From 6ad3b2fdc731f86aded7a066f328b19af48f5b07 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 16:34:13 -0700 Subject: [PATCH 19/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.sol | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/Approve2.sol b/src/Approve2.sol index f69dabc0..479a5587 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -92,10 +92,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( @@ -144,15 +146,13 @@ 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) allowance[from][token][msg.sender] = allowed - amount; + // 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); } /*////////////////////////////////////////////////////////////// @@ -172,14 +172,16 @@ contract Approve2 { address[] calldata spenders, uint256 noncesToInvalidate ) external { - unchecked { - // Will revert if trying to invalidate - // more than type(uint16).max nonces. - invalidateNonces(noncesToInvalidate); + // 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"); + // Each index should correspond to an index in the other array. + require(tokens.length == spenders.length, "LENGTH_MISMATCH"); + // Unchecked because counter overflow is impossible + // in any environment with reasonable gas limits. + unchecked { // 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]]; From 4efdfe143f1243871b47ad80ca790fedd9709222 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 16:35:52 -0700 Subject: [PATCH 20/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Approve2.sol b/src/Approve2.sol index 479a5587..3a255dd0 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -27,7 +27,12 @@ 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 From 2ae0cf6b14b7547ded4058ba55ff0b7f98d9c010 Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 17:07:36 -0700 Subject: [PATCH 21/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.vy | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Approve2.vy b/src/Approve2.vy index a8a5e88a..d76ce5f0 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -29,6 +29,7 @@ allowance: public(HashMap[address, HashMap[ERC20, HashMap[address, uint256]]]) @external def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): + """ @notice Transfer approved tokens from one address to another. @@ -53,6 +54,7 @@ def transferFrom(token: ERC20, owner: address, to: address, amount: uint256): @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 @@ -106,6 +108,7 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, dead @external def invalidateNonces(noncesToInvalidate: uint256): + """ @notice Invalidate a specific number of nonces. Can be used @@ -123,6 +126,7 @@ def invalidateNonces(noncesToInvalidate: uint256): @external def approve(token: ERC20, spender: address, amount: uint256): + """ @notice Manually approve a spender to transfer a specific @@ -141,12 +145,14 @@ def approve(token: ERC20, spender: address, amount: uint256): @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 From 49fa8a555bb4657a039d06f5f4a3827c2586362b Mon Sep 17 00:00:00 2001 From: t11s Date: Sat, 30 Jul 2022 17:11:56 -0700 Subject: [PATCH 22/28] =?UTF-8?q?=F0=9F=93=9D=20TODO?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Approve2.vy | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Approve2.vy b/src/Approve2.vy index d76ce5f0..5d9691ec 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -158,6 +158,7 @@ def DOMAIN_SEPARATOR(token: ERC20) -> bytes32: @view @internal def computeDomainSeperator(token: ERC20) -> bytes32: + # TODO: I don't think concat is abi encode compliant. return keccak256( concat( keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), From 048fc00c9c8d865b945f272c3c592a258611908d Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 31 Jul 2022 20:19:48 -0700 Subject: [PATCH 23/28] =?UTF-8?q?=E2=9C=85=20Lockdown=20draft?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 20 +-- src/Approve2.sol | 356 +++++++++++++++++++++++++++++++++++++++++--- src/Approve2.vy | 37 ++++- test/Approve2.t.sol | 16 +- 4 files changed, 395 insertions(+), 34 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 4e15cd81..ef07bff3 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: 21774) -Approve2Test:testPermit2Full() (gas: 32503) -Approve2Test:testPermit2NonPermitToken() (gas: 22446) +Approve2Test:testPermit2() (gas: 21797) +Approve2Test:testPermit2Full() (gas: 32526) +Approve2Test:testPermit2NonPermitToken() (gas: 22514) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135886) -Approve2Test:testStandardPermit() (gas: 21427) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135887) +Approve2Test:testStandardPermit() (gas: 21405) Approve2Test:testStandardTransferFrom() (gas: 38207) -Approve2Test:testTransferFrom2() (gas: 38086) +Approve2Test:testTransferFrom2() (gas: 38064) Approve2Test:testTransferFrom2Full() (gas: 47574) -Approve2Test:testTransferFrom2NonPermitToken() (gas: 47575) +Approve2Test:testTransferFrom2NonPermitToken() (gas: 47531) diff --git a/src/Approve2.sol b/src/Approve2.sol index 3a255dd0..0ffcf1aa 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -1,9 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; -import {ERC20} from "solmate/tokens/ERC20.sol"; -import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; - /// @title Approve2 /// @author transmissions11 /// @notice Backwards compatible, low-overhead, @@ -164,33 +161,350 @@ contract Approve2 { 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 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. - function lockdown( - ERC20[] calldata tokens, - address[] calldata spenders, - uint256 noncesToInvalidate - ) external { + /// @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 { + // Revoke allowances for each pair of spenders and tokens. + 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); + } +} - // Each index should correspond to an index in the other array. - require(tokens.length == spenders.length, "LENGTH_MISMATCH"); +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +abstract contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ - // Unchecked because counter overflow is impossible - // in any environment with reasonable gas limits. + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public immutable decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal immutable INITIAL_CHAIN_ID; + + bytes32 internal immutable INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = block.chainid; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public virtual returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public virtual returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. unchecked { - // 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]]; - } + balanceOf[to] += amount; } + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public virtual returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != type(uint256).max) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + unchecked { + balanceOf[to] += amount; + } + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public virtual { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + unchecked { + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + } + + emit Approval(owner, spender, value); + } + + function DOMAIN_SEPARATOR() public view virtual returns (bytes32) { + return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : computeDomainSeparator(); + } + + function computeDomainSeparator() internal view virtual returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + block.chainid, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal virtual { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + unchecked { + balanceOf[to] += amount; + } + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal virtual { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + unchecked { + totalSupply -= amount; + } + + emit Transfer(from, address(0), amount); + } +} + +/// @notice Safe ETH and ERC20 transfer library that gracefully handles missing return values. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol) +/// @dev Use with caution! Some functions in this library knowingly create dirty bits at the destination of the free memory pointer. +/// @dev Note that none of the functions in this library check that a token has code at all! That responsibility is delegated to the caller. +library SafeTransferLib { + /*////////////////////////////////////////////////////////////// + ETH OPERATIONS + //////////////////////////////////////////////////////////////*/ + + function safeTransferETH(address to, uint256 amount) internal { + bool success; + + assembly { + // Transfer the ETH and store if it succeeded or not. + success := call(gas(), to, amount, 0, 0, 0, 0) + } + + require(success, "ETH_TRANSFER_FAILED"); + } + + /*////////////////////////////////////////////////////////////// + ERC20 OPERATIONS + //////////////////////////////////////////////////////////////*/ + + function safeTransferFrom( + ERC20 token, + address from, + address to, + uint256 amount + ) internal { + bool success; + + assembly { + // Get a pointer to some free memory. + let freeMemoryPointer := mload(0x40) + + // Write the abi-encoded calldata into memory, beginning with the function selector. + mstore(freeMemoryPointer, 0x23b872dd00000000000000000000000000000000000000000000000000000000) + mstore(add(freeMemoryPointer, 4), from) // Append the "from" argument. + mstore(add(freeMemoryPointer, 36), to) // Append the "to" argument. + mstore(add(freeMemoryPointer, 68), amount) // Append the "amount" argument. + + success := and( + // Set success to whether the call reverted, if not we check it either + // returned exactly 1 (can't just be non-zero data), or had no return data. + or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())), + // We use 100 because the length of our calldata totals up like so: 4 + 32 * 3. + // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. + // Counterintuitively, this call must be positioned second to the or() call in the + // surrounding and() call or else returndatasize() will be zero during the computation. + call(gas(), token, 0, freeMemoryPointer, 100, 0, 32) + ) + } + + require(success, "TRANSFER_FROM_FAILED"); + } + + function safeTransfer( + ERC20 token, + address to, + uint256 amount + ) internal { + bool success; + + assembly { + // Get a pointer to some free memory. + let freeMemoryPointer := mload(0x40) + + // Write the abi-encoded calldata into memory, beginning with the function selector. + mstore(freeMemoryPointer, 0xa9059cbb00000000000000000000000000000000000000000000000000000000) + mstore(add(freeMemoryPointer, 4), to) // Append the "to" argument. + mstore(add(freeMemoryPointer, 36), amount) // Append the "amount" argument. + + success := and( + // Set success to whether the call reverted, if not we check it either + // returned exactly 1 (can't just be non-zero data), or had no return data. + or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())), + // We use 68 because the length of our calldata totals up like so: 4 + 32 * 2. + // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. + // Counterintuitively, this call must be positioned second to the or() call in the + // surrounding and() call or else returndatasize() will be zero during the computation. + call(gas(), token, 0, freeMemoryPointer, 68, 0, 32) + ) + } + + require(success, "TRANSFER_FAILED"); + } + + function safeApprove( + ERC20 token, + address to, + uint256 amount + ) internal { + bool success; + + assembly { + // Get a pointer to some free memory. + let freeMemoryPointer := mload(0x40) + + // Write the abi-encoded calldata into memory, beginning with the function selector. + mstore(freeMemoryPointer, 0x095ea7b300000000000000000000000000000000000000000000000000000000) + mstore(add(freeMemoryPointer, 4), to) // Append the "to" argument. + mstore(add(freeMemoryPointer, 36), amount) // Append the "amount" argument. + + success := and( + // Set success to whether the call reverted, if not we check it either + // returned exactly 1 (can't just be non-zero data), or had no return data. + or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())), + // We use 68 because the length of our calldata totals up like so: 4 + 32 * 2. + // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. + // Counterintuitively, this call must be positioned second to the or() call in the + // surrounding and() call or else returndatasize() will be zero during the computation. + call(gas(), token, 0, freeMemoryPointer, 68, 0, 32) + ) + } + + require(success, "APPROVE_FAILED"); } } diff --git a/src/Approve2.vy b/src/Approve2.vy index 5d9691ec..d7503032 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -1,3 +1,5 @@ +# @version 0.3.4 + """ @title Approve2 @license MIT @@ -7,10 +9,11 @@ next generation token approval/meta-tx system. """ -# TODO Add the lockdown feature to the contract. - 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 # ################################################################ @@ -97,11 +100,37 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, dead convert(s, uint256) ) + # TODO: Would a seperate assert be cheaper? assert recoveredAddress != empty(address) and 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 # ################################################################ @@ -118,7 +147,7 @@ def invalidateNonces(noncesToInvalidate: uint256): assert noncesToInvalidate < 2 ** 16 # TODO Do we need to extract this into a constant? - self.nonces[msg.sender] += noncesToInvalidate + self.nonces[msg.sender] += noncesToInvalidate # TODO This can be made unsafe, overflow unlikely. ################################################################ # MANUAL APPROVAL LOGIC # @@ -159,6 +188,8 @@ def DOMAIN_SEPARATOR(token: ERC20) -> bytes32: @internal def computeDomainSeperator(token: ERC20) -> bytes32: # TODO: I don't think concat is abi encode compliant. + # TODO: See https://github.com/curvefi/curve-crypto-contract/blob/d7d04cd9ae038970e40be850df99de8c1ff7241b/contracts/CurveTokenV5.vy#L71 + # TODO: Would making these hashes constant be cheaper? The compiler should hopefully do this for us? return keccak256( concat( keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), diff --git a/test/Approve2.t.sol b/test/Approve2.t.sol index 232f6e21..5557599a 100644 --- a/test/Approve2.t.sol +++ b/test/Approve2.t.sol @@ -11,7 +11,7 @@ import {Approve2Lib} from "../src/Approve2Lib.sol"; import {MockNonPermitERC20} from "./mocks/MockNonPermitERC20.sol"; -// TODO: Fuzzing. +// TODO: Fuzzing and coverage of Approve2.sol and Approve2.vy. // TODO: Test and bench functions like lockdown and invalidateNonces. contract Approve2Test is DSTestPlus { @@ -134,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 //////////////////////////////////////////////////////////////*/ From 75a3a18ad5b574e026c5a80090a4dc2303ebecc3 Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 31 Jul 2022 20:20:56 -0700 Subject: [PATCH 24/28] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Inline?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ef07bff3..cd32cf79 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -4,10 +4,10 @@ Approve2Test:testOZSafePermit() (gas: 23418) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129118) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21797) -Approve2Test:testPermit2Full() (gas: 32526) -Approve2Test:testPermit2NonPermitToken() (gas: 22514) +Approve2Test:testPermit2Full() (gas: 32365) +Approve2Test:testPermit2NonPermitToken() (gas: 22353) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135887) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135726) Approve2Test:testStandardPermit() (gas: 21405) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38064) diff --git a/src/Approve2.vy b/src/Approve2.vy index d7503032..892e5423 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -193,8 +193,8 @@ def computeDomainSeperator(token: ERC20) -> bytes32: return keccak256( concat( keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), - keccak256(convert("Approve2", Bytes[8])), - keccak256(convert("1", Bytes[1])), + keccak256("Approve2"), + keccak256("1"), convert(chain.id, bytes32), convert(token, bytes32) ) From 0c700ab2377fd4348dd36477a808f1c5e04f570e Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 31 Jul 2022 20:21:12 -0700 Subject: [PATCH 25/28] Update Approve2.vy --- src/Approve2.vy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Approve2.vy b/src/Approve2.vy index 892e5423..a1ae0439 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -192,7 +192,7 @@ def computeDomainSeperator(token: ERC20) -> bytes32: # TODO: Would making these hashes constant be cheaper? The compiler should hopefully do this for us? return keccak256( concat( - keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'), + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), keccak256("Approve2"), keccak256("1"), convert(chain.id, bytes32), From 5e18bcf42f932a24651314a3692672262b01f49f Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 31 Jul 2022 20:39:19 -0700 Subject: [PATCH 26/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Compliant=20domain?= =?UTF-8?q?=20seperator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +- src/Approve2.sol | 325 +---------------------------------------------- src/Approve2.vy | 9 +- 3 files changed, 9 insertions(+), 331 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index cd32cf79..40d788dc 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -4,10 +4,10 @@ Approve2Test:testOZSafePermit() (gas: 23418) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129118) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21797) -Approve2Test:testPermit2Full() (gas: 32365) -Approve2Test:testPermit2NonPermitToken() (gas: 22353) +Approve2Test:testPermit2Full() (gas: 32257) +Approve2Test:testPermit2NonPermitToken() (gas: 22245) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135726) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135618) Approve2Test:testStandardPermit() (gas: 21405) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38064) diff --git a/src/Approve2.sol b/src/Approve2.sol index 0ffcf1aa..7dc420d1 100644 --- a/src/Approve2.sol +++ b/src/Approve2.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.15; +import {ERC20} from "solmate/tokens/ERC20.sol"; +import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; + /// @title Approve2 /// @author transmissions11 /// @notice Backwards compatible, low-overhead, @@ -186,325 +189,3 @@ contract Approve2 { invalidateNonces(noncesToInvalidate); } } - -/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. -/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) -/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) -/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. -abstract contract ERC20 { - /*////////////////////////////////////////////////////////////// - EVENTS - //////////////////////////////////////////////////////////////*/ - - event Transfer(address indexed from, address indexed to, uint256 amount); - - event Approval(address indexed owner, address indexed spender, uint256 amount); - - /*////////////////////////////////////////////////////////////// - METADATA STORAGE - //////////////////////////////////////////////////////////////*/ - - string public name; - - string public symbol; - - uint8 public immutable decimals; - - /*////////////////////////////////////////////////////////////// - ERC20 STORAGE - //////////////////////////////////////////////////////////////*/ - - uint256 public totalSupply; - - mapping(address => uint256) public balanceOf; - - mapping(address => mapping(address => uint256)) public allowance; - - /*////////////////////////////////////////////////////////////// - EIP-2612 STORAGE - //////////////////////////////////////////////////////////////*/ - - uint256 internal immutable INITIAL_CHAIN_ID; - - bytes32 internal immutable INITIAL_DOMAIN_SEPARATOR; - - mapping(address => uint256) public nonces; - - /*////////////////////////////////////////////////////////////// - CONSTRUCTOR - //////////////////////////////////////////////////////////////*/ - - constructor( - string memory _name, - string memory _symbol, - uint8 _decimals - ) { - name = _name; - symbol = _symbol; - decimals = _decimals; - - INITIAL_CHAIN_ID = block.chainid; - INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); - } - - /*////////////////////////////////////////////////////////////// - ERC20 LOGIC - //////////////////////////////////////////////////////////////*/ - - function approve(address spender, uint256 amount) public virtual returns (bool) { - allowance[msg.sender][spender] = amount; - - emit Approval(msg.sender, spender, amount); - - return true; - } - - function transfer(address to, uint256 amount) public virtual returns (bool) { - balanceOf[msg.sender] -= amount; - - // Cannot overflow because the sum of all user - // balances can't exceed the max uint256 value. - unchecked { - balanceOf[to] += amount; - } - - emit Transfer(msg.sender, to, amount); - - return true; - } - - function transferFrom( - address from, - address to, - uint256 amount - ) public virtual returns (bool) { - uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. - - if (allowed != type(uint256).max) allowance[from][msg.sender] = allowed - amount; - - balanceOf[from] -= amount; - - // Cannot overflow because the sum of all user - // balances can't exceed the max uint256 value. - unchecked { - balanceOf[to] += amount; - } - - emit Transfer(from, to, amount); - - return true; - } - - /*////////////////////////////////////////////////////////////// - EIP-2612 LOGIC - //////////////////////////////////////////////////////////////*/ - - function permit( - address owner, - address spender, - uint256 value, - uint256 deadline, - uint8 v, - bytes32 r, - bytes32 s - ) public virtual { - require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); - - // Unchecked because the only math done is incrementing - // the owner's nonce which cannot realistically overflow. - unchecked { - address recoveredAddress = ecrecover( - keccak256( - abi.encodePacked( - "\x19\x01", - DOMAIN_SEPARATOR(), - keccak256( - abi.encode( - keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"), - owner, - spender, - value, - nonces[owner]++, - deadline - ) - ) - ) - ), - v, - r, - s - ); - - require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); - - allowance[recoveredAddress][spender] = value; - } - - emit Approval(owner, spender, value); - } - - function DOMAIN_SEPARATOR() public view virtual returns (bytes32) { - return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : computeDomainSeparator(); - } - - function computeDomainSeparator() internal view virtual returns (bytes32) { - return - keccak256( - abi.encode( - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), - keccak256(bytes(name)), - keccak256("1"), - block.chainid, - address(this) - ) - ); - } - - /*////////////////////////////////////////////////////////////// - INTERNAL MINT/BURN LOGIC - //////////////////////////////////////////////////////////////*/ - - function _mint(address to, uint256 amount) internal virtual { - totalSupply += amount; - - // Cannot overflow because the sum of all user - // balances can't exceed the max uint256 value. - unchecked { - balanceOf[to] += amount; - } - - emit Transfer(address(0), to, amount); - } - - function _burn(address from, uint256 amount) internal virtual { - balanceOf[from] -= amount; - - // Cannot underflow because a user's balance - // will never be larger than the total supply. - unchecked { - totalSupply -= amount; - } - - emit Transfer(from, address(0), amount); - } -} - -/// @notice Safe ETH and ERC20 transfer library that gracefully handles missing return values. -/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol) -/// @dev Use with caution! Some functions in this library knowingly create dirty bits at the destination of the free memory pointer. -/// @dev Note that none of the functions in this library check that a token has code at all! That responsibility is delegated to the caller. -library SafeTransferLib { - /*////////////////////////////////////////////////////////////// - ETH OPERATIONS - //////////////////////////////////////////////////////////////*/ - - function safeTransferETH(address to, uint256 amount) internal { - bool success; - - assembly { - // Transfer the ETH and store if it succeeded or not. - success := call(gas(), to, amount, 0, 0, 0, 0) - } - - require(success, "ETH_TRANSFER_FAILED"); - } - - /*////////////////////////////////////////////////////////////// - ERC20 OPERATIONS - //////////////////////////////////////////////////////////////*/ - - function safeTransferFrom( - ERC20 token, - address from, - address to, - uint256 amount - ) internal { - bool success; - - assembly { - // Get a pointer to some free memory. - let freeMemoryPointer := mload(0x40) - - // Write the abi-encoded calldata into memory, beginning with the function selector. - mstore(freeMemoryPointer, 0x23b872dd00000000000000000000000000000000000000000000000000000000) - mstore(add(freeMemoryPointer, 4), from) // Append the "from" argument. - mstore(add(freeMemoryPointer, 36), to) // Append the "to" argument. - mstore(add(freeMemoryPointer, 68), amount) // Append the "amount" argument. - - success := and( - // Set success to whether the call reverted, if not we check it either - // returned exactly 1 (can't just be non-zero data), or had no return data. - or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())), - // We use 100 because the length of our calldata totals up like so: 4 + 32 * 3. - // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. - // Counterintuitively, this call must be positioned second to the or() call in the - // surrounding and() call or else returndatasize() will be zero during the computation. - call(gas(), token, 0, freeMemoryPointer, 100, 0, 32) - ) - } - - require(success, "TRANSFER_FROM_FAILED"); - } - - function safeTransfer( - ERC20 token, - address to, - uint256 amount - ) internal { - bool success; - - assembly { - // Get a pointer to some free memory. - let freeMemoryPointer := mload(0x40) - - // Write the abi-encoded calldata into memory, beginning with the function selector. - mstore(freeMemoryPointer, 0xa9059cbb00000000000000000000000000000000000000000000000000000000) - mstore(add(freeMemoryPointer, 4), to) // Append the "to" argument. - mstore(add(freeMemoryPointer, 36), amount) // Append the "amount" argument. - - success := and( - // Set success to whether the call reverted, if not we check it either - // returned exactly 1 (can't just be non-zero data), or had no return data. - or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())), - // We use 68 because the length of our calldata totals up like so: 4 + 32 * 2. - // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. - // Counterintuitively, this call must be positioned second to the or() call in the - // surrounding and() call or else returndatasize() will be zero during the computation. - call(gas(), token, 0, freeMemoryPointer, 68, 0, 32) - ) - } - - require(success, "TRANSFER_FAILED"); - } - - function safeApprove( - ERC20 token, - address to, - uint256 amount - ) internal { - bool success; - - assembly { - // Get a pointer to some free memory. - let freeMemoryPointer := mload(0x40) - - // Write the abi-encoded calldata into memory, beginning with the function selector. - mstore(freeMemoryPointer, 0x095ea7b300000000000000000000000000000000000000000000000000000000) - mstore(add(freeMemoryPointer, 4), to) // Append the "to" argument. - mstore(add(freeMemoryPointer, 36), amount) // Append the "amount" argument. - - success := and( - // Set success to whether the call reverted, if not we check it either - // returned exactly 1 (can't just be non-zero data), or had no return data. - or(and(eq(mload(0), 1), gt(returndatasize(), 31)), iszero(returndatasize())), - // We use 68 because the length of our calldata totals up like so: 4 + 32 * 2. - // We use 0 and 32 to copy up to 32 bytes of return data into the scratch space. - // Counterintuitively, this call must be positioned second to the or() call in the - // surrounding and() call or else returndatasize() will be zero during the computation. - call(gas(), token, 0, freeMemoryPointer, 68, 0, 32) - ) - } - - require(success, "APPROVE_FAILED"); - } -} diff --git a/src/Approve2.vy b/src/Approve2.vy index a1ae0439..81dec4ef 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -187,15 +187,12 @@ def DOMAIN_SEPARATOR(token: ERC20) -> bytes32: @view @internal def computeDomainSeperator(token: ERC20) -> bytes32: - # TODO: I don't think concat is abi encode compliant. - # TODO: See https://github.com/curvefi/curve-crypto-contract/blob/d7d04cd9ae038970e40be850df99de8c1ff7241b/contracts/CurveTokenV5.vy#L71 - # TODO: Would making these hashes constant be cheaper? The compiler should hopefully do this for us? return keccak256( - concat( + _abi_encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), keccak256("Approve2"), keccak256("1"), - convert(chain.id, bytes32), - convert(token, bytes32) + chain.id, + token ) ) \ No newline at end of file From fd4d378e257230f18848c6c4d1932ff70627d857 Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 31 Jul 2022 20:43:01 -0700 Subject: [PATCH 27/28] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Separate=20assert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 40d788dc..36d948b9 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -4,10 +4,10 @@ Approve2Test:testOZSafePermit() (gas: 23418) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129118) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21797) -Approve2Test:testPermit2Full() (gas: 32257) -Approve2Test:testPermit2NonPermitToken() (gas: 22245) +Approve2Test:testPermit2Full() (gas: 32245) +Approve2Test:testPermit2NonPermitToken() (gas: 22233) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135618) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135606) Approve2Test:testStandardPermit() (gas: 21405) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38064) diff --git a/src/Approve2.vy b/src/Approve2.vy index 81dec4ef..55357af5 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -100,8 +100,9 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, dead convert(s, uint256) ) - # TODO: Would a seperate assert be cheaper? - assert recoveredAddress != empty(address) and recoveredAddress == owner, "INVALID_SIGNER" + + assert recoveredAddress != empty(address) + assert recoveredAddress == owner, "INVALID_SIGNER" self.allowance[owner][token][spender] = amount self.nonces[owner] = unsafe_add(nonce, 1) From 717f9e0b012a262d8ed87cecfe062bff8796c40c Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 31 Jul 2022 21:21:58 -0700 Subject: [PATCH 28/28] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 6 +++--- src/Approve2.vy | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 36d948b9..d04fcbf0 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -4,10 +4,10 @@ Approve2Test:testOZSafePermit() (gas: 23418) Approve2Test:testOZSafePermitPlusOZSafeTransferFrom() (gas: 129118) Approve2Test:testOZSafeTransferFrom() (gas: 39310) Approve2Test:testPermit2() (gas: 21797) -Approve2Test:testPermit2Full() (gas: 32245) -Approve2Test:testPermit2NonPermitToken() (gas: 22233) +Approve2Test:testPermit2Full() (gas: 32243) +Approve2Test:testPermit2NonPermitToken() (gas: 22231) Approve2Test:testPermit2PlusTransferFrom2() (gas: 126273) -Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135606) +Approve2Test:testPermit2PlusTransferFrom2WithNonPermit() (gas: 135604) Approve2Test:testStandardPermit() (gas: 21405) Approve2Test:testStandardTransferFrom() (gas: 38207) Approve2Test:testTransferFrom2() (gas: 38064) diff --git a/src/Approve2.vy b/src/Approve2.vy index 55357af5..5cee7669 100644 --- a/src/Approve2.vy +++ b/src/Approve2.vy @@ -75,6 +75,8 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, dead assert deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED" + assert owner != empty(address), "INVALID_OWNER" + nonce: uint256 = self.nonces[owner] digest: bytes32 = keccak256( @@ -100,8 +102,6 @@ def permit(token: ERC20, owner: address, spender: address, amount: uint256, dead convert(s, uint256) ) - - assert recoveredAddress != empty(address) assert recoveredAddress == owner, "INVALID_SIGNER" self.allowance[owner][token][spender] = amount