From ad050b615b1ffa928a351e177228f305de9314a1 Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 23 Oct 2022 01:12:58 -0700 Subject: [PATCH 1/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Style?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/AllowanceTransfer.sol | 36 +++++++----------------------------- src/DomainSeparator.sol | 22 +++++++++------------- src/SignatureTransfer.sol | 12 ++++++------ 3 files changed, 22 insertions(+), 48 deletions(-) diff --git a/src/AllowanceTransfer.sol b/src/AllowanceTransfer.sol index ed9cbe7a..cf6c0246 100644 --- a/src/AllowanceTransfer.sol +++ b/src/AllowanceTransfer.sol @@ -29,10 +29,6 @@ contract AllowanceTransfer is DomainSeparator { event InvalidateNonces(address indexed owner, uint32 indexed toNonce, address token, address spender); - /*////////////////////////////////////////////////////////////// - ALLOWANCE STORAGE - //////////////////////////////////////////////////////////////*/ - /// @notice Maps users to tokens to spender addresses and information about the approval on the token. /// @dev The saved packed word saves the allowed amount, expiration, and nonce. mapping(address => mapping(address => mapping(address => PackedAllowance))) public allowance; @@ -49,18 +45,12 @@ contract AllowanceTransfer is DomainSeparator { allowed.expiration = expiration; } - /*/////////////////////////////////////////////////////f///////// - PERMIT LOGIC - //////////////////////////////////////////////////////////////*/ - /// @notice Permit a user to spend a given amount of another user's /// approved amount of the given token via the owner's EIP-712 signature. /// @dev May fail if the owner's nonce was invalidated in-flight by invalidateNonce. function permit(Permit calldata permitData, address owner, bytes calldata signature) external { // Ensure the signature's deadline has not already passed. - if (block.timestamp > permitData.sigDeadline) { - revert SignatureExpired(); - } + if (block.timestamp > permitData.sigDeadline) revert SignatureExpired(); // Check current nonce (incremented below). if (permitData.nonce != allowance[owner][permitData.token][permitData.spender].nonce) { @@ -97,10 +87,6 @@ contract AllowanceTransfer is DomainSeparator { PackedAllowance({amount: permitData.amount, expiration: expiration, nonce: permitData.nonce + 1}); } - /*////////////////////////////////////////////////////////////// - TRANSFER LOGIC - //////////////////////////////////////////////////////////////*/ - /// @notice Transfer approved tokens from one address to another. /// @param token The token to transfer. /// @param from The address to transfer from. @@ -115,9 +101,8 @@ contract AllowanceTransfer is DomainSeparator { function batchTransferFrom(address[] calldata token, address from, address[] calldata to, uint160[] calldata amount) external { - if (amount.length != to.length || token.length != to.length) { - revert LengthMismatch(); - } + if (amount.length != to.length || token.length != to.length) revert LengthMismatch(); + unchecked { for (uint256 i = 0; i < token.length; ++i) { _transfer(token[i], from, to[i], amount[i]); @@ -128,11 +113,9 @@ contract AllowanceTransfer is DomainSeparator { function _transfer(address token, address from, address to, uint160 amount) private { PackedAllowance storage allowed = allowance[from][token][msg.sender]; - if (block.timestamp > allowed.expiration) { - revert AllowanceExpired(); - } + if (block.timestamp > allowed.expiration) revert AllowanceExpired(); - uint160 maxAmount = allowed.amount; + uint256 maxAmount = allowed.amount; if (maxAmount != type(uint160).max) { if (amount > maxAmount) { revert InsufficientAllowance(); @@ -142,14 +125,11 @@ contract AllowanceTransfer is DomainSeparator { } } } + // Transfer the tokens from the from address to the recipient. ERC20(token).safeTransferFrom(from, to, amount); } - /*////////////////////////////////////////////////////////////// - LOCKDOWN LOGIC - //////////////////////////////////////////////////////////////*/ - // TODO: Bench if a struct for token-spender pairs is cheaper. // TODO test /// @notice Enables performing a "lockdown" of the sender's Permit2 identity @@ -160,9 +140,7 @@ contract AllowanceTransfer is DomainSeparator { /// Each index should correspond to an index in the tokens array. function lockdown(address[] calldata tokens, address[] calldata spenders) external { // Each index should correspond to an index in the other array. - if (tokens.length != spenders.length) { - revert LengthMismatch(); - } + if (tokens.length != spenders.length) revert LengthMismatch(); // Revoke allowances for each pair of spenders and tokens. unchecked { diff --git a/src/DomainSeparator.sol b/src/DomainSeparator.sol index 2b3cde3e..c6dddd27 100644 --- a/src/DomainSeparator.sol +++ b/src/DomainSeparator.sol @@ -5,12 +5,10 @@ pragma solidity 0.8.17; /// @dev maintains cross-chain replay protection in the event of a fork /// @dev reference: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/EIP712.sol contract DomainSeparator { - /* solhint-disable var-name-mixedcase */ - // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to - // invalidate the cached domain separator if the chain id changes. + // Cache the domain separator as an immutable value, but also store the chain id that it + // corresponds to, in order to invalidate the cached domain separator if the chain id changes. bytes32 private immutable _CACHED_DOMAIN_SEPARATOR; uint256 private immutable _CACHED_CHAIN_ID; - address private immutable _CACHED_THIS; bytes32 private constant _HASHED_NAME = keccak256("Permit2"); bytes32 private constant _TYPE_HASH = @@ -18,21 +16,19 @@ contract DomainSeparator { constructor() { _CACHED_CHAIN_ID = block.chainid; - _CACHED_THIS = address(this); _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME); } - /// @notice returns the domain separator for the current chain - /// @dev uses cached version if chainid and address are unchanged from construction + /// @notice Returns the domain separator for the current chain. + /// @dev Uses cached version if chainid and address are unchanged from construction. function DOMAIN_SEPARATOR() public view returns (bytes32) { - if (address(this) == _CACHED_THIS && block.chainid == _CACHED_CHAIN_ID) { - return _CACHED_DOMAIN_SEPARATOR; - } else { - return _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME); - } + return + block.chainid == _CACHED_CHAIN_ID + ? _CACHED_DOMAIN_SEPARATOR + : _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME); } - /// @notice builds a domain separator using the current chainId and contract address + /// @notice Builds a domain separator using the current chainId and contract address. function _buildDomainSeparator(bytes32 typeHash, bytes32 nameHash) private view returns (bytes32) { return keccak256(abi.encode(typeHash, nameHash, block.chainid, address(this))); } diff --git a/src/SignatureTransfer.sol b/src/SignatureTransfer.sol index a905ea8c..309dd5d4 100644 --- a/src/SignatureTransfer.sol +++ b/src/SignatureTransfer.sol @@ -143,11 +143,10 @@ contract SignatureTransfer is DomainSeparator { ) external { _validatePermit(permit.spender, permit.deadline); _validateInputLengths(permit.tokens.length, to.length, permit.signedAmounts.length, requestedAmounts.length); + unchecked { for (uint256 i = 0; i < permit.tokens.length; ++i) { - if (requestedAmounts[i] > permit.signedAmounts[i]) { - revert InvalidAmount(); - } + if (requestedAmounts[i] > permit.signedAmounts[i]) revert InvalidAmount(); } } @@ -191,6 +190,7 @@ contract SignatureTransfer is DomainSeparator { /// @notice Invalidates the bits specified in `mask` for the bitmap at `wordPos`. function invalidateUnorderedNonces(uint256 wordPos, uint256 mask) external { nonceBitmap[msg.sender][wordPos] |= mask; + emit InvalidateUnorderedNonces(msg.sender, wordPos, mask); } @@ -198,9 +198,9 @@ contract SignatureTransfer is DomainSeparator { function _useUnorderedNonce(address from, uint256 nonce) private { (uint248 wordPos, uint8 bitPos) = bitmapPositions(nonce); uint256 bitmap = nonceBitmap[from][wordPos]; - if ((bitmap >> bitPos) & 1 == 1) { - revert InvalidNonce(); - } + + if ((bitmap >> bitPos) & 1 == 1) revert InvalidNonce(); + nonceBitmap[from][wordPos] = bitmap | (1 << bitPos); } From e55710a24acad13f99b84fab7d5f6395f99a006c Mon Sep 17 00:00:00 2001 From: t11s Date: Sun, 23 Oct 2022 01:15:25 -0700 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Gasolina?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...permitBatchTransferFromMultipleTokens.snap | 2 +- .../permitBatchTransferFromSingleToken.snap | 2 +- .forge-snapshots/permitCleanWrite.snap | 2 +- .forge-snapshots/permitDirtyWrite.snap | 2 +- .forge-snapshots/permitInvalidSigner.snap | 2 +- .../permitSetMaxAllowanceCleanWrite.snap | 2 +- .../permitSetMaxAllowanceDirtyWrite.snap | 2 +- .../permitTransferFromSingleToken.snap | 2 +- .../single recipient 2 tokens.snap | 2 +- .../single recipient many tokens.snap | 2 +- .gas-snapshot | 60 +++++++++---------- 11 files changed, 40 insertions(+), 40 deletions(-) diff --git a/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap b/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap index f143db7c..e0a7712d 100644 --- a/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap +++ b/.forge-snapshots/permitBatchTransferFromMultipleTokens.snap @@ -1 +1 @@ -145408 \ No newline at end of file +145360 \ No newline at end of file diff --git a/.forge-snapshots/permitBatchTransferFromSingleToken.snap b/.forge-snapshots/permitBatchTransferFromSingleToken.snap index 98589ac1..648d244b 100644 --- a/.forge-snapshots/permitBatchTransferFromSingleToken.snap +++ b/.forge-snapshots/permitBatchTransferFromSingleToken.snap @@ -1 +1 @@ -90616 \ No newline at end of file +90568 \ No newline at end of file diff --git a/.forge-snapshots/permitCleanWrite.snap b/.forge-snapshots/permitCleanWrite.snap index 7396783c..c29e2af4 100644 --- a/.forge-snapshots/permitCleanWrite.snap +++ b/.forge-snapshots/permitCleanWrite.snap @@ -1 +1 @@ -62486 \ No newline at end of file +62438 \ No newline at end of file diff --git a/.forge-snapshots/permitDirtyWrite.snap b/.forge-snapshots/permitDirtyWrite.snap index f6143b53..d0e273bb 100644 --- a/.forge-snapshots/permitDirtyWrite.snap +++ b/.forge-snapshots/permitDirtyWrite.snap @@ -1 +1 @@ -45386 \ No newline at end of file +45338 \ No newline at end of file diff --git a/.forge-snapshots/permitInvalidSigner.snap b/.forge-snapshots/permitInvalidSigner.snap index b5a0c543..8efa533a 100644 --- a/.forge-snapshots/permitInvalidSigner.snap +++ b/.forge-snapshots/permitInvalidSigner.snap @@ -1 +1 @@ -43267 \ No newline at end of file +43219 \ No newline at end of file diff --git a/.forge-snapshots/permitSetMaxAllowanceCleanWrite.snap b/.forge-snapshots/permitSetMaxAllowanceCleanWrite.snap index 7fb604ad..f4e24d8c 100644 --- a/.forge-snapshots/permitSetMaxAllowanceCleanWrite.snap +++ b/.forge-snapshots/permitSetMaxAllowanceCleanWrite.snap @@ -1 +1 @@ -60481 \ No newline at end of file +60433 \ No newline at end of file diff --git a/.forge-snapshots/permitSetMaxAllowanceDirtyWrite.snap b/.forge-snapshots/permitSetMaxAllowanceDirtyWrite.snap index 79a9f481..9daa5d41 100644 --- a/.forge-snapshots/permitSetMaxAllowanceDirtyWrite.snap +++ b/.forge-snapshots/permitSetMaxAllowanceDirtyWrite.snap @@ -1 +1 @@ -43381 \ No newline at end of file +43333 \ No newline at end of file diff --git a/.forge-snapshots/permitTransferFromSingleToken.snap b/.forge-snapshots/permitTransferFromSingleToken.snap index eb1130a9..31a57ae6 100644 --- a/.forge-snapshots/permitTransferFromSingleToken.snap +++ b/.forge-snapshots/permitTransferFromSingleToken.snap @@ -1 +1 @@ -88884 \ No newline at end of file +88836 \ No newline at end of file diff --git a/.forge-snapshots/single recipient 2 tokens.snap b/.forge-snapshots/single recipient 2 tokens.snap index 5a7c0ede..124fe801 100644 --- a/.forge-snapshots/single recipient 2 tokens.snap +++ b/.forge-snapshots/single recipient 2 tokens.snap @@ -1 +1 @@ -120425 \ No newline at end of file +120377 \ No newline at end of file diff --git a/.forge-snapshots/single recipient many tokens.snap b/.forge-snapshots/single recipient many tokens.snap index c97646a8..04479446 100644 --- a/.forge-snapshots/single recipient many tokens.snap +++ b/.forge-snapshots/single recipient many tokens.snap @@ -1 +1 @@ -136398 \ No newline at end of file +136350 \ No newline at end of file diff --git a/.gas-snapshot b/.gas-snapshot index 6e466b76..f9cb8284 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,37 +1,37 @@ AllowanceTransferTest:testApprove() (gas: 40056) -AllowanceTransferTest:testBatchTransferFrom() (gas: 124856) -AllowanceTransferTest:testBatchTransferFromLengthMismatch() (gas: 74485) -AllowanceTransferTest:testBatchTransferFromWithGasSnapshot() (gas: 155331) -AllowanceTransferTest:testExcessiveInvalidation() (gas: 58217) +AllowanceTransferTest:testBatchTransferFrom() (gas: 124808) +AllowanceTransferTest:testBatchTransferFromLengthMismatch() (gas: 74437) +AllowanceTransferTest:testBatchTransferFromWithGasSnapshot() (gas: 155283) +AllowanceTransferTest:testExcessiveInvalidation() (gas: 58169) AllowanceTransferTest:testInvalidateNonces() (gas: 54185) -AllowanceTransferTest:testMaxAllowance() (gas: 131556) -AllowanceTransferTest:testMaxAllowanceDirtyWrite() (gas: 114294) -AllowanceTransferTest:testPartialAllowance() (gas: 101778) -AllowanceTransferTest:testReuseOrderedNonceInvalid() (gas: 61038) -AllowanceTransferTest:testSetAllowance() (gas: 85906) +AllowanceTransferTest:testMaxAllowance() (gas: 131508) +AllowanceTransferTest:testMaxAllowanceDirtyWrite() (gas: 114246) +AllowanceTransferTest:testPartialAllowance() (gas: 101730) +AllowanceTransferTest:testReuseOrderedNonceInvalid() (gas: 60990) +AllowanceTransferTest:testSetAllowance() (gas: 85858) AllowanceTransferTest:testSetAllowanceDeadlinePassed() (gas: 53693) -AllowanceTransferTest:testSetAllowanceDirtyWrite() (gas: 68539) -AllowanceTransferTest:testSetAllowanceInvalidSignature() (gas: 64336) -AllowanceTransferTest:testSetAllowanceTransfer() (gas: 99965) -AllowanceTransferTest:testSetAllowanceTransferDirtyNonceDirtyTransfer() (gas: 66051) -AllowanceTransferTest:testTransferFromWithGasSnapshot() (gas: 129868) -DomainSeparatorTest:testDomainSeparator() (gas: 5807) -DomainSeparatorTest:testDomainSeparatorAfterFork() (gas: 10744) -SignatureTransferTest:testGasMultiplePermitBatchTransferFrom() (gas: 266835) -SignatureTransferTest:testGasSinglePermitBatchTransferFrom() (gas: 183692) -SignatureTransferTest:testGasSinglePermitTransferFrom() (gas: 123674) +AllowanceTransferTest:testSetAllowanceDirtyWrite() (gas: 68491) +AllowanceTransferTest:testSetAllowanceInvalidSignature() (gas: 64288) +AllowanceTransferTest:testSetAllowanceTransfer() (gas: 99917) +AllowanceTransferTest:testSetAllowanceTransferDirtyNonceDirtyTransfer() (gas: 66003) +AllowanceTransferTest:testTransferFromWithGasSnapshot() (gas: 129820) +DomainSeparatorTest:testDomainSeparator() (gas: 5759) +DomainSeparatorTest:testDomainSeparatorAfterFork() (gas: 10600) +SignatureTransferTest:testGasMultiplePermitBatchTransferFrom() (gas: 266787) +SignatureTransferTest:testGasSinglePermitBatchTransferFrom() (gas: 183644) +SignatureTransferTest:testGasSinglePermitTransferFrom() (gas: 123626) SignatureTransferTest:testInvalidateUnorderedNonces() (gas: 55723) -SignatureTransferTest:testPermitBatchTransferFrom() (gas: 159739) -SignatureTransferTest:testPermitBatchTransferFromSingleRecipient() (gas: 186048) +SignatureTransferTest:testPermitBatchTransferFrom() (gas: 159691) +SignatureTransferTest:testPermitBatchTransferFromSingleRecipient() (gas: 186000) SignatureTransferTest:testPermitBatchTransferInvalidAmountsLength() (gas: 42167) SignatureTransferTest:testPermitBatchTransferInvalidRecipientsLength() (gas: 41632) SignatureTransferTest:testPermitBatchTransferInvalidSingleAddr() (gas: 40312) -SignatureTransferTest:testPermitBatchTransferMultiAddr() (gas: 157899) -SignatureTransferTest:testPermitBatchTransferSingleRecipientManyTokens() (gas: 196007) -SignatureTransferTest:testPermitTransferFrom() (gas: 92966) -SignatureTransferTest:testPermitTransferFromInvalidNonce() (gas: 92455) -SignatureTransferTest:testPermitTransferFromToSpender() (gas: 97334) -SignatureTransferTest:testPermitTransferFromTypedWitness() (gas: 96716) -SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidType() (gas: 57585) -SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypeName() (gas: 58392) -SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypehash() (gas: 56866) +SignatureTransferTest:testPermitBatchTransferMultiAddr() (gas: 157851) +SignatureTransferTest:testPermitBatchTransferSingleRecipientManyTokens() (gas: 195959) +SignatureTransferTest:testPermitTransferFrom() (gas: 92918) +SignatureTransferTest:testPermitTransferFromInvalidNonce() (gas: 92407) +SignatureTransferTest:testPermitTransferFromToSpender() (gas: 97286) +SignatureTransferTest:testPermitTransferFromTypedWitness() (gas: 96668) +SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidType() (gas: 57537) +SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypeName() (gas: 58344) +SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypehash() (gas: 56818)