uniswap / permit2 Goto Github PK
View Code? Open in Web Editor NEW๐๐๐ next generation token approvals mechanism
License: MIT License
๐๐๐ next generation token approvals mechanism
License: MIT License
every function seems to have a batched version, maybe approve should as well? thoughts?
check that the lengths of tokens, amounts, and expirations are equal
should benchmark typehashes against eth_signTypedData_v4
AllowanceTransferTest.t.sol
testBatchTransferFromMultiToken
startBalanceTo1
is token0.balanceOf(address0);
- should be: token1.balanceOf(address0);
assertEq(token0.balanceOf(address0), startBalanceTo1 + 1 ** 18);
should be assertEq(token1.balanceOf(address0), startBalanceTo1 + 1 ** 18);
tests still pass when corrected
permit2
and transferFrom2
take uint256 amount
as an argument but permit accepts uint160 amount, so breaks for inputs greater than 2**160
we should include overflow checks
We don't care about injections so should just be combined
i was wondering in _transfer whether the bypass of updating the allowed.amount when approved amount is 2**160-1 is intended to save on gas, as i have tried to test in 2 cases, for any value under the max, it did deduct from the allowance accordingly. and also I was looking into the test cases and there is a lack of checking for allowance when permit2.transferFrom takes place.
[EDIT]: only withdraw and redeem methods in ERC-4626 use the underlying ERC-20 approval, so I edited the below to reflect that. This also simplifies the permit2 integration.
Similar to Uniswap/universal-router#176, I believe ERC-4626 integration directly into permit2 is highly beneficial. The main reasons are:
Given that permit2 is intended to be deployed only once to save user approvals on gas, I'd lobby for ERC-4626 inclusion in the canonical deployment, whereas on universal router it can be added in a later deployment.
The simplest integration would be to only replicate the single-use transferFrom logic in AllowanceTransfer.sol for both of the withdraw
and redeem
functions. Batch methods and SignatureTransfer could also be added for each, although I'm not sure if the extra complexity is warranted
Can I get a sense for the intended freeze date for the permit2 contract? If the timelines are possible, I would be able to have a tested PR ready before the end of next week.
some tokens don't revert but return false on transferFrom calls in which case allowances may still be updated
Currently this is treated as success but should not be
permit2/src/interfaces/IAllowanceTransfer.sol
Lines 35 to 43 in 5919a0f
permit2/src/interfaces/ISignatureTransfer.sol
Lines 39 to 43 in 5919a0f
I think it could be better instead of check the DOMAIN_SEPARATOR
which some tokens that implement permit
do not implement like UNI.
Check instead the PERMIT_TYPEHASH
in a very similar behaviour.
In this way you can support native permit to the tokens that do not support DOMAIN_SEPARATOR
like UNI
( so overall will be more optimal) and also support other DAI like tokens that support permit, that you see in this list
EDIT: Also the DOMAIN_SEPARATOR
will change in every network, so you should update the DAI_DOMAIN_SEPARATOR
constant in case of a deployment in different networks, but the PERMIT_TYPEHASH
won't change ^^
In the library Allowance
, neither updateAll
nor updateAmountAndExpiration
check that the expiration
passed is not in the past.
That means it's possible to set an approval expiration timestamp that ends up being lower than block.timestamp
(and greater than 0, which is the special case), and have the call succeed.
Seems like a minor oversight - I don't think it has worrying consequences. As long as user sets a reasonable expiration for the allowance, things should be fine. Worst case scenario the expired allowance is stored, the user notices the problem, and submits another tx to update the allowance.
Here's a simple test case to reproduce:
function testApproveExpired() public {
vm.warp(block.timestamp + 1000);
uint48 expiredTimestamp = uint48(block.timestamp - 1);
vm.prank(from);
vm.expectEmit(true, true, true, true);
emit Approval(from, address(token0), address(this), defaultAmount, expiredTimestamp);
permit2.approve(address(token0), address(this), defaultAmount, expiredTimestamp);
(uint160 amount, uint48 expiration, uint48 nonce) = permit2.allowance(from, address(token0), address(this));
assertEq(amount, defaultAmount);
assertEq(expiration, expiredTimestamp);
assertEq(nonce, 0);
}
compare gas overhead between permit single and permitBatch of len 1
Lines 4 to 10 in fc18098
otherwise it'll just be invalid signature error
might be kind of odd and annoying from an integrator's pov to reveal this internal storage detail to the outside world. could encapsulate and just do a safe cast if outside the limit
On line 90 of Permit2Lib there is an attempt to do a staticcall to get the DOMAIN_SEPARATOR
value. If the token does not have a DOMAIN_SEPARATOR
, the call fails and the function falls back to using permit2. However, if the token has a fallback function (such as WETH), all gas sent in the static call gets burned.
This can have negative effects if a user sets a high gasLimit in their transaction. For example, while testing this function my test environment had a default gas limit equal to the block limit, so the transaction used over 29 million gas.
You can see a similar issue come up in this StackExchange post: https://ethereum.stackexchange.com/questions/96547/high-gas-consumption-when-using-staticcall-on-a-non-view-pure-function
permit2/src/DomainSeparator.sol
Lines 15 to 18 in fc18098
Bench if a struct for token-spender pairs is cheaper
Include nonce as indexed
in event
I am wondering if the transfer by signature logic introduced here in combination with EIP 1271 removes the need for the approve + transferFrom flow and the allowance storage in the ERC20 standard if we were to start from scratch? Or is there some special cases when allowance still has extended functionality?
In a slightly tangential yet related note, is there any thought about introducing a transfer by signature scheme for custom defined data such as a UniswapV3Pool position. The benefit of this would be that custody of the position could be separated from the helper functions that allow you to enter and exit a position. I think you could then still go back and wrap this in the ERC721 standard after the fact too.
permit2/src/interfaces/IAllowanceTransfer.sol
Lines 36 to 43 in be76f11
for readability, this might better be represented as array of structs, something like:
struct Permit {
address token;
uint32 nonce;
uint160 amount;
uint64 expiry;
}
struct PermitSingle {
Permit permit;
address spender;
}
struct PermitBatch {
Permit[] permits;
address spender;
}
let's leave this until the very end incase the interface changes at all
Lines 7 to 8 in fc18098
this isn't stored anywhere so larger types cheaper
remove spender param and pass in msg.sender in the sig reconstruction
the readme could use some love
given the results we've seen in #62, might be a good idea to update all the similar batch functions to have similar interfaces?
Should send funds to spender if 0 addr is passed in as recipient
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.