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

OptimismMintableERC20: Add permit support + native permit2 support #11880

Closed
tynes opened this issue Sep 12, 2024 · 3 comments · Fixed by #11868
Closed

OptimismMintableERC20: Add permit support + native permit2 support #11880

tynes opened this issue Sep 12, 2024 · 3 comments · Fixed by #11868

Comments

@tynes
Copy link
Contributor

tynes commented Sep 12, 2024

Add permit support to the OptimismMintableERC20 token implementation. This can easily be done using the ERC20Permit contract in openzeppelin. Also add native support for permit2 by enforcing that the allowance for permit2 always returns type(uint256).max.

@marktoda
Copy link
Contributor

  • clear requirements, why they need permit + permit2

    • Default support for ERC2612 permits is a pretty clear UX improvement, enabling single tx actions and gas abstraction without native token in wallet to pay for an initial approve
    • Permit2 has become the primary approval mechanism for all Uniswap interface interactions and is integrated by many other DeFi protocols that I'm aware of. Directly integrating tokens with it saves an entire approval step for users, simplifying the user experience of interacting with these protocols. It also synergizes well with intents-based trading systems like UniswapX, where users could submit a trade with a single signature without ever requiring any native token to set up for the trade
  • is there precendent for baking permit2 support directly into a token? why/why not? related to prior bullet what does this give vs. normal out-of-token support

@mds1
Copy link
Contributor

mds1 commented Sep 12, 2024

Thanks @marktoda! One follow up: What's the value of standard ERC2612 permit if permit2 is directly integrated? It seems to me permit2 is a superset of permit. I'm ok with adding both—I see POL has both as well—just curious if there's a different use case for permit vs. permit2, or if it's just to give users/devs more options?

@maurelian
Copy link
Contributor

@marktoda can you confirm that we should enable this setting to _givePermit2InfiniteAllowance in the Solady ERC20? Is that a basic requirement to make the ERC20 Permit2 compatible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants