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: owner-level nonce for permit #139

Closed
wants to merge 17 commits into from

Conversation

saucepoint
Copy link
Collaborator

@saucepoint saucepoint commented Jul 7, 2024

in v3, the nonce for permit was at the token level and managed by NonfungiblePositionManager. This was gas-inefficient where a user paid clean / cold-storage writes for each position.

in v4, we're putting the nonce at the owner level in ERC721Permit -- enabling a one-time cold-storage write, regardless of token / pool.

saucepoint and others added 12 commits June 28, 2024 11:23
* consolidate using owner, update burn

* fix: accrue deltas to caller in increase
* rip out vanilla and benchmark

* fix gas benchmark

* check posm is the locker before allowing access to external functions

* restore execute tests

* posm takes as 6909; remove legacy deadcode

* restore tests

* move helpers to the same file
@saucepoint saucepoint marked this pull request as ready for review July 9, 2024 10:04
@@ -96,7 +96,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
// mint receipt token
uint256 tokenId;
_mint(owner, (tokenId = nextTokenId++));
tokenPositions[tokenId] = TokenPosition({owner: owner, range: range});
tokenPositions[tokenId] = TokenPosition({owner: owner, range: range, operator: address(0x0)});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you mint, maybe you should be allowed to specify a default operator? maybe thats wack though lol

@@ -11,8 +11,7 @@ import {IERC1271} from "../interfaces/external/IERC1271.sol";
/// @title ERC721 with permit
/// @notice Nonfungible tokens that support an approve via signature, i.e. permit
abstract contract ERC721Permit is ERC721, IERC721Permit {
/// @dev Gets the current nonce for a token ID and then increments it, returning the original value
function _getAndIncrementNonce(uint256 tokenId) internal virtual returns (uint256);
mapping(address owner => uint256 nonce) public nonce;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO natspec

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we will not be doing nonce per owner no? We are doing the bitmap I thought

Base automatically changed from sauce/posm-batch-execute-transient to sauce/posm-batch-execute July 9, 2024 16:03
@snreynolds
Copy link
Member

Can you open this against dev-posm?

@snreynolds snreynolds added the posm position manager label Jul 9, 2024
@saucepoint saucepoint changed the base branch from sauce/posm-batch-execute to dev-posm July 10, 2024 12:47
@snreynolds
Copy link
Member

Closing in favor of #153

@snreynolds snreynolds closed this Jul 18, 2024
@saucepoint saucepoint deleted the sauce/posm-batch-execute-transient-permit branch July 29, 2024 13:35
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 this pull request may close these issues.

2 participants