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

[posm]: Add Permit functionality to ERC721 #167

Closed
7 of 8 tasks
snreynolds opened this issue Jul 19, 2024 · 2 comments · Fixed by #210
Closed
7 of 8 tasks

[posm]: Add Permit functionality to ERC721 #167

snreynolds opened this issue Jul 19, 2024 · 2 comments · Fixed by #210
Assignees
Labels
posm position manager

Comments

@snreynolds
Copy link
Member

snreynolds commented Jul 19, 2024

Component

Position Manager

Describe the suggested feature and problem it solves.

Permit should:

  • Use unordered nonces
  • Not be payable (in V3 it is payable)
  • Use relevant libraries and base contracts (ie a PermitHash library) and a EIP1271 base contract that caches the domain separator. (see comments here.
  • Constants above functions
  • Test: Ensure PERMIT_TYPEHASH is equal to expected hash
  • Use custom errors
  • replace magic constant with IERC1271.isValidSignature.selector
  • this comment

Ensure that all comments related to permit from this PR and this PR are resolved in the final feature PR.

Describe the desired implementation.

No response

Describe alternatives.

No response

Additional context.

No response

@snreynolds snreynolds added the posm position manager label Jul 19, 2024
Copy link

linear bot commented Jul 19, 2024

@saucepoint
Copy link
Collaborator

note to self: ensure permit is payable with a test for permit & increase with native tokens multicall{value: x}(permit, increase)

#189 (review)

@saucepoint saucepoint mentioned this issue Jul 26, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
posm position manager
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants