Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

little readability improvement + unchecked on loops #168

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .forge-snapshots/lockdown.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
28435
28447
2 changes: 1 addition & 1 deletion .forge-snapshots/permitBatchCleanWrite.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
91924
91826
2 changes: 1 addition & 1 deletion .forge-snapshots/permitBatchDirtyWrite.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
57724
57626
Original file line number Diff line number Diff line change
@@ -1 +1 @@
143387
143240
2 changes: 1 addition & 1 deletion .forge-snapshots/permitBatchTransferFromSingleToken.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
88867
88818
2 changes: 1 addition & 1 deletion .forge-snapshots/permitTransferFromBatchTypedWitness.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
120325
120227
2 changes: 1 addition & 1 deletion .forge-snapshots/single recipient 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118525
118427
2 changes: 1 addition & 1 deletion .forge-snapshots/single recipient many tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133544
133054
56 changes: 28 additions & 28 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
AllowanceTransferInvariants:invariant_balanceEqualsSpent() (runs: 256, calls: 3840, reverts: 886)
AllowanceTransferInvariants:invariant_permit2NeverHoldsBalance() (runs: 256, calls: 3840, reverts: 886)
AllowanceTransferInvariants:invariant_spendNeverExceedsPermit() (runs: 256, calls: 3840, reverts: 886)
AllowanceTransferInvariants:invariant_balanceEqualsSpent() (runs: 256, calls: 3840, reverts: 885)
AllowanceTransferInvariants:invariant_permit2NeverHoldsBalance() (runs: 256, calls: 3840, reverts: 885)
AllowanceTransferInvariants:invariant_spendNeverExceedsPermit() (runs: 256, calls: 3840, reverts: 885)
AllowanceTransferTest:testApprove() (gas: 47561)
AllowanceTransferTest:testBatchTransferFrom() (gas: 159268)
AllowanceTransferTest:testBatchTransferFromDifferentOwners() (gas: 235508)
AllowanceTransferTest:testBatchTransferFromMultiToken() (gas: 231810)
AllowanceTransferTest:testBatchTransferFromMultiToken() (gas: 231712)
AllowanceTransferTest:testBatchTransferFromWithGasSnapshot() (gas: 159818)
AllowanceTransferTest:testExcessiveInvalidation() (gas: 64136)
AllowanceTransferTest:testInvalidateMultipleNonces() (gas: 83139)
AllowanceTransferTest:testInvalidateNonces() (gas: 62679)
AllowanceTransferTest:testInvalidateNoncesInvalid() (gas: 16261)
AllowanceTransferTest:testLockdown() (gas: 145952)
AllowanceTransferTest:testLockdownEvent() (gas: 117758)
AllowanceTransferTest:testLockdown() (gas: 145866)
AllowanceTransferTest:testLockdownEvent() (gas: 117672)
AllowanceTransferTest:testMaxAllowance() (gas: 134993)
AllowanceTransferTest:testMaxAllowanceDirtyWrite() (gas: 117582)
AllowanceTransferTest:testPartialAllowance() (gas: 105067)
AllowanceTransferTest:testReuseOrderedNonceInvalid() (gas: 69095)
AllowanceTransferTest:testSetAllowance() (gas: 89583)
AllowanceTransferTest:testSetAllowanceBatch() (gas: 133608)
AllowanceTransferTest:testSetAllowanceBatchDifferentNonces() (gas: 118583)
AllowanceTransferTest:testSetAllowanceBatchDirtyWrite() (gas: 99144)
AllowanceTransferTest:testSetAllowanceBatchEvent() (gas: 115892)
AllowanceTransferTest:testSetAllowanceBatch() (gas: 133510)
AllowanceTransferTest:testSetAllowanceBatchDifferentNonces() (gas: 118485)
AllowanceTransferTest:testSetAllowanceBatchDirtyWrite() (gas: 99046)
AllowanceTransferTest:testSetAllowanceBatchEvent() (gas: 115794)
AllowanceTransferTest:testSetAllowanceCompactSig() (gas: 89543)
AllowanceTransferTest:testSetAllowanceDeadlinePassed() (gas: 56500)
AllowanceTransferTest:testSetAllowanceDirtyWrite() (gas: 72175)
Expand Down Expand Up @@ -72,20 +72,20 @@ Permit2LibTest:testTransferFrom2Full() (gas: 53258)
Permit2LibTest:testTransferFrom2InvalidAmount() (gas: 12710)
Permit2LibTest:testTransferFrom2NonPermitToken() (gas: 53104)
SignatureTransferTest:testCorrectWitnessTypehashes() (gas: 3075)
SignatureTransferTest:testGasMultiplePermitBatchTransferFrom() (gas: 270919)
SignatureTransferTest:testGasSinglePermitBatchTransferFrom() (gas: 186316)
SignatureTransferTest:testGasMultiplePermitBatchTransferFrom() (gas: 270772)
SignatureTransferTest:testGasSinglePermitBatchTransferFrom() (gas: 186267)
SignatureTransferTest:testGasSinglePermitTransferFrom() (gas: 123850)
SignatureTransferTest:testInvalidateUnorderedNonces() (gas: 41268)
SignatureTransferTest:testPermitBatchMultiPermitSingleTransfer() (gas: 133644)
SignatureTransferTest:testPermitBatchTransferFrom() (gas: 162010)
SignatureTransferTest:testPermitBatchTransferFromSingleRecipient() (gas: 190319)
SignatureTransferTest:testPermitBatchTransferFromTypedWitness() (gas: 239854)
SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidType() (gas: 84467)
SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidTypeHash() (gas: 85864)
SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidWitness() (gas: 85688)
SignatureTransferTest:testPermitBatchTransferInvalidAmountsLengthMismatch() (gas: 43967)
SignatureTransferTest:testPermitBatchTransferMultiAddr() (gas: 160406)
SignatureTransferTest:testPermitBatchTransferSingleRecipientManyTokens() (gas: 211834)
SignatureTransferTest:testPermitBatchMultiPermitSingleTransfer() (gas: 133546)
SignatureTransferTest:testPermitBatchTransferFrom() (gas: 161912)
SignatureTransferTest:testPermitBatchTransferFromSingleRecipient() (gas: 190221)
SignatureTransferTest:testPermitBatchTransferFromTypedWitness() (gas: 239756)
SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidType() (gas: 84369)
SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidTypeHash() (gas: 85766)
SignatureTransferTest:testPermitBatchTransferFromTypedWitnessInvalidWitness() (gas: 85590)
SignatureTransferTest:testPermitBatchTransferInvalidAmountsLengthMismatch() (gas: 43869)
SignatureTransferTest:testPermitBatchTransferMultiAddr() (gas: 160308)
SignatureTransferTest:testPermitBatchTransferSingleRecipientManyTokens() (gas: 211344)
SignatureTransferTest:testPermitTransferFrom() (gas: 93012)
SignatureTransferTest:testPermitTransferFromCompactSig() (gas: 123927)
SignatureTransferTest:testPermitTransferFromIncorrectSigLength() (gas: 51327)
Expand All @@ -96,14 +96,14 @@ SignatureTransferTest:testPermitTransferFromTypedWitness() (gas: 125271)
SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidType() (gas: 55906)
SignatureTransferTest:testPermitTransferFromTypedWitnessInvalidTypehash() (gas: 56794)
SignatureTransferTest:testPermitTransferSpendLessThanFull(uint256,uint128) (runs: 256, μ: 97989, ~: 99707)
TypehashGeneration:testPermitBatch() (gas: 40493)
TypehashGeneration:testPermitBatchTransferFrom() (gas: 49854)
TypehashGeneration:testPermitBatchTransferFromWithWitness() (gas: 56587)
TypehashGeneration:testPermitBatchTransferFromWithWitnessIncorrectPermitData() (gas: 56744)
TypehashGeneration:testPermitBatchTransferFromWithWitnessIncorrectTypehashStub() (gas: 57229)
TypehashGeneration:testPermitBatch() (gas: 40400)
TypehashGeneration:testPermitBatchTransferFrom() (gas: 49756)
TypehashGeneration:testPermitBatchTransferFromWithWitness() (gas: 56489)
TypehashGeneration:testPermitBatchTransferFromWithWitnessIncorrectPermitData() (gas: 56646)
TypehashGeneration:testPermitBatchTransferFromWithWitnessIncorrectTypehashStub() (gas: 57131)
TypehashGeneration:testPermitSingle() (gas: 28117)
TypehashGeneration:testPermitTransferFrom() (gas: 36520)
TypehashGeneration:testPermitTransferFromWithWitness() (gas: 43369)
TypehashGeneration:testPermitTransferFromWithWitnessIncorrectPermitData() (gas: 43430)
TypehashGeneration:testPermitTransferFromWithWitnessIncorrectTypehashStub() (gas: 43833)
MockPermit2Lib:testPermit2Code(address):(bool) (runs: 256, μ: 3025, ~: 3016)
MockPermit2Lib:testPermit2Code(address):(bool) (runs: 256, μ: 3035, ~: 3016)
37 changes: 21 additions & 16 deletions src/AllowanceTransfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ contract AllowanceTransfer is IAllowanceTransfer, EIP712 {
signature.verify(_hashTypedData(permitBatch.hash()), owner);

address spender = permitBatch.spender;
unchecked {
uint256 length = permitBatch.details.length;
for (uint256 i = 0; i < length; ++i) {
_updateApproval(permitBatch.details[i], owner, spender);

uint256 length = permitBatch.details.length;
for (uint256 i = 0; i < length;) {
_updateApproval(permitBatch.details[i], owner, spender);
unchecked {
++i;
}
}
}
Expand All @@ -62,11 +64,12 @@ contract AllowanceTransfer is IAllowanceTransfer, EIP712 {

/// @inheritdoc IAllowanceTransfer
function transferFrom(AllowanceTransferDetails[] calldata transferDetails) external {
unchecked {
uint256 length = transferDetails.length;
for (uint256 i = 0; i < length; ++i) {
AllowanceTransferDetails memory transferDetail = transferDetails[i];
_transfer(transferDetail.from, transferDetail.to, transferDetail.amount, transferDetail.token);
uint256 length = transferDetails.length;
for (uint256 i = 0; i < length;) {
AllowanceTransferDetails memory transferDetail = transferDetails[i];
_transfer(transferDetail.from, transferDetail.to, transferDetail.amount, transferDetail.token);
unchecked {
++i;
}
}
}
Expand Down Expand Up @@ -97,14 +100,16 @@ contract AllowanceTransfer is IAllowanceTransfer, EIP712 {
function lockdown(TokenSpenderPair[] calldata approvals) external {
address owner = msg.sender;
// Revoke allowances for each pair of spenders and tokens.
unchecked {
uint256 length = approvals.length;
for (uint256 i = 0; i < length; ++i) {
address token = approvals[i].token;
address spender = approvals[i].spender;

allowance[owner][token][spender].amount = 0;
emit Lockdown(owner, token, spender);
uint256 length = approvals.length;
for (uint256 i = 0; i < length;) {
address token = approvals[i].token;
address spender = approvals[i].spender;

allowance[owner][token][spender].amount = 0;
emit Lockdown(owner, token, spender);
unchecked {
++i;
}
}
}
Expand Down
20 changes: 11 additions & 9 deletions src/SignatureTransfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,19 @@ contract SignatureTransfer is ISignatureTransfer, EIP712 {
_useUnorderedNonce(owner, permit.nonce);
signature.verify(_hashTypedData(dataHash), owner);

unchecked {
for (uint256 i = 0; i < numPermitted; ++i) {
TokenPermissions memory permitted = permit.permitted[i];
uint256 requestedAmount = transferDetails[i].requestedAmount;
for (uint256 i = 0; i < numPermitted;) {
TokenPermissions memory permitted = permit.permitted[i];
uint256 requestedAmount = transferDetails[i].requestedAmount;

if (requestedAmount > permitted.amount) revert InvalidAmount(permitted.amount);
if (requestedAmount > permitted.amount) revert InvalidAmount(permitted.amount);

if (requestedAmount != 0) {
// allow spender to specify which of the permitted tokens should be transferred
ERC20(permitted.token).safeTransferFrom(owner, transferDetails[i].to, requestedAmount);
}
if (requestedAmount != 0) {
// allow spender to specify which of the permitted tokens should be transferred
ERC20(permitted.token).safeTransferFrom(owner, transferDetails[i].to, requestedAmount);
}

unchecked {
++i;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Allowance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ library Allowance {
uint48 expiration
) internal {
// If the inputted expiration is 0, the allowance only lasts the duration of the block.
allowed.expiration = expiration == 0 ? uint48(block.timestamp) : expiration;
allowed.expiration = expiration == BLOCK_TIMESTAMP_EXPIRATION ? uint48(block.timestamp) : expiration;
allowed.amount = amount;
}

Expand Down
15 changes: 12 additions & 3 deletions src/libraries/PermitHash.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ library PermitHash {
function hash(IAllowanceTransfer.PermitBatch memory permitBatch) internal pure returns (bytes32) {
uint256 numPermits = permitBatch.details.length;
bytes32[] memory permitHashes = new bytes32[](numPermits);
for (uint256 i = 0; i < numPermits; ++i) {
for (uint256 i = 0; i < numPermits;) {
permitHashes[i] = _hashPermitDetails(permitBatch.details[i]);
unchecked {
++i;
}
}
return keccak256(
abi.encode(
Expand All @@ -67,8 +70,11 @@ library PermitHash {
uint256 numPermitted = permit.permitted.length;
bytes32[] memory tokenPermissionHashes = new bytes32[](numPermitted);

for (uint256 i = 0; i < numPermitted; ++i) {
for (uint256 i = 0; i < numPermitted;) {
tokenPermissionHashes[i] = _hashTokenPermissions(permit.permitted[i]);
unchecked {
++i;
}
}

return keccak256(
Expand Down Expand Up @@ -104,8 +110,11 @@ library PermitHash {
uint256 numPermitted = permit.permitted.length;
bytes32[] memory tokenPermissionHashes = new bytes32[](numPermitted);

for (uint256 i = 0; i < numPermitted; ++i) {
for (uint256 i = 0; i < numPermitted;) {
tokenPermissionHashes[i] = _hashTokenPermissions(permit.permitted[i]);
unchecked {
++i;
}
}

return keccak256(
Expand Down