🥢 ERC20: _givePermit2InfiniteAllowance: true

[z0r0z]

Description

uh, true?

Checklist

Ensure you completed all of the steps below before submitting your pull request:

• Ran forge fmt?
• Ran forge test?

Pull requests with an incomplete checklist will be thrown out.

[github-actions]

Gas Snapshot Comparison Report

Generated at commit : 6f36260, Compared to commit : 513f581

Contract Name	Test Name	Main Gas	PR Gas	Diff
CREATE3Test	testDeployERC20()	781227	815879	34652
	testDoubleDeployDifferentBytecodeReverts()	96898738	96899651	913
	test__codesize()	13860	14174	314
ERC20Invariants	test__codesize()	7278	7447	169
ERC20Test	testApprove()	35730	35881	151
	testBurn()	62424	62496	72
	testInfiniteApproveTransferFrom()	90852	91121	269
	testMint()	58888	58924	36
	testMintOverMaxUintReverts()	56350	56398	48
	testPermit()	89379	89553	174
	testTransfer()	65884	65956	72
	testTransferFrom()	86320	86536	216
	testTransferFromInsufficientAllowanceReverts()	82330	82489	159
	testTransferFromInsufficientBalanceReverts()	62678	62838	160
	testTransferInsufficientBalanceReverts()	56494	56542	48
	test__codesize()	22608	22958	350
ERC20VotesTest	test__codesize()	24044	24251	207
ERC4626Test	testDepositWithNoApprovalReverts()	17076	17145	69
	testDepositWithNotEnoughApprovalReverts()	90874	91118	244
	testMintWithNoApprovalReverts()	17050	17119	69
	testMintZero()	53924	54006	82
	testMultipleMintDepositRedeemWithdraw()	412842	413903	1061
	testRedeemWithNotEnoughShareAmountReverts()	143771	143931	160
	testTryGetAssetDecimals()	30755448	32001810	1246362
	testUseVirtualShares()	2435672	2518612	82940
	testVaultInteractionsForSomeoneElse()	297821	298192	371
	testVirtualSharesMultipleMintDepositRedeemWithdraw()	1618669	1661512	42843
	testWithdrawWithNotEnoughUnderlyingAmountReverts()	144904	145076	172
	testWithdrawZero()	52053	52089	36
	test__codesize()	36923	37299	376
LifebuoyTest	testLockEverything()	960877	960901	24
	testLockRescue()	978964	978924	-40
	testLockRescueETH()	991228	991188	-40
	test__codesize()	45953	46122	169
SafeTransferLibTest	testApproveWithRetry()	789995	822073	32078
	testApproveWithStandardERC20()	32367	32518	151
	testBalanceOfStandardERC20()	7874	7886	12
	testPermit2()	90134	90223	89
	testSimplePermit2AndPermit2TransferFrom()	1139584	1139690	106
	testSimplePermit2AndPermit2TransferFromGas()	141416	141375	-41
	testTotalSupplyQuery()	27280	27316	36
	testTransferAllFromWithStandardERC20()	33696	33863	167
	testTransferAllWithStandardERC20()	31011	31047	36
	testTransferFromWithMissingReturn()	587800	592412	4612
	testTransferFromWithReturnsFalseReverts()	554017	558629	4612
	testTransferFromWithReturnsTooLittleReverts()	553913	558525	4612
	testTransferFromWithReturnsTooMuch()	588432	593065	4633
	testTransferFromWithRevertingReverts()	546619	551231	4612
	testTransferFromWithStandardERC20()	586646	591506	4860
	testTransferWithMissingReturn()	574298	578910	4612
	testTransferWithReturnsFalseReverts()	551075	555708	4633
	testTransferWithReturnsTooLittleReverts()	550671	555262	4591
	testTransferWithReturnsTooMuch()	574873	579485	4612
	testTransferWithRevertingReverts()	550925	555537	4612
	testTransferWithStandardERC20()	575018	579711	4693
	test__codesize()	69219	69544	325
WETHInvariants	test__codesize()	5178	5323	145
WETHTest	testDeposit()	62272	62296	24
	testFallbackDeposit()	61978	62002	24
	testPartialWithdraw()	70422	70446	24
	testWithdraw()	52047	52066	19
	testWithdrawToContractWithoutReceiveReverts()	90164	90183	19
	test__codesize()	9410	9555	145

[Vectorized]

Dang, need to update the tests too.

[Vectorized]

@atarpara help me update the tests

[z0r0z]

based thx @atarpara

[atarpara]

Updated test, but I do not favor this because it will increase gas by default. The Solady ERC20 already has the permit functionality. So very less people use permit2.

[z0r0z]

Most tokens will be used on L2 such that the convenience might be indeed worth the tradeoff. If we are shooting for hyperoptimization, then I would suggest we shuck off permit() and make it into an inheritable. **Additional thought does pose the question, why have both permit and permit2. Maybe Solady should just use permit2.

[Vectorized]

Hmm… good point. @atarpara

This extra gas is very concerning.

Let’s marinate on this for at least a week.

[Vectorized]

We can have both. For max compat.

[Vectorized]

Ok, we shall only consider the gas overhead in the transferFrom function.

With Permit2 enabled, it costs about 20 more gas. An extra 21000 gas for the approve transaction is 1000 times more than this overhead. Hence, on overall, we save more gas. @atarpara

[Vectorized]

I'll merge this when the Spearbit audit has been finalized.