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

Position Manager w/ UR-invokable delta resolving #124

Closed
wants to merge 59 commits into from
Closed

Conversation

saucepoint
Copy link
Collaborator

@saucepoint saucepoint commented Jun 19, 2024

wippity wip


Outstanding items

  • Base functionality: mint/burn/increaseLiquidity/decreaseLiquidity/collect
  • proper fee accounting
  • exclusive zeroOut lock -- prevent hooks from zeroing-out
  • batch interfaces on nftposm w/ testing
  • recipient behavior (specify destination when withdrawing tokens)
  • operator/delegate functionality w/ tests
  • NFT transfer tests (transfer and burn, transfer and increase/decrease liquidity, transfer and collect)
  • ERC721Permit -- nonincrementing nonce
  • deadline checks + tests
  • slippage checks + tests
  • initialize pool if it doesnt exist
  • add salt to LiquidityRange
  • store PoolId in LiquidityRange instead of PoolKey

saucepoint and others added 22 commits April 5, 2024 17:39
* chore: update v4-core:latest (#105)

* update core

* rename lockAcquired to unlockCallback

* update core; temporary path hack in remappings

* update v4-core; remove remapping

* wip: fix compatibility

* update core; fix renaming of swap fee to lp fee

* update core; fix events

* update core; address liquidity salt and modify liquidity return values

* fix incorrect delta accounting when modifying liquidity

* fix todo, use CurrencySettleTake

* remove deadcode

* update core; use StateLibrary; update sqrtRatio to sqrtPrice

* fix beforeSwap return signatures

* forge fmt; remove commented out code

* update core (wow gas savings)

* update core

* update core

* update core; hook flags LSB

* update core

* update core

* chore: update v4 core (#115)

* Update v4-core

* CurrencySettleTake -> CurrencySettler

* Snapshots

* compiling but very broken

* replace PoolStateLibrary

* update currency settle take

* compiling

* wip

* use v4-core's forge-std

* test liquidity increase

* additional fixes for collection and liquidity decrease

* test migration

* replace old implementation with new

---------

Signed-off-by: saucepoint <saucepoint@protonmail.com>
Co-authored-by: 0x57 <qi.wu@coinbase.com>
* wip: consolidation

* further consolidation

* consolidate to single file

* yay no more stack too deep

* some code comments
Co-authored-by: 0x57 <wqi@umich.edu>
Comment on lines 111 to 118
// Update the tokensOwed0 and tokensOwed1 values for the caller.
// if callerDelta < 0, existing fees were re-invested AND net new tokens are required for the liquidity increase
// if callerDelta == 0, existing fees were reinvested (autocompounded)
// if callerDelta > 0, some but not all existing fees were used to increase liquidity. Any remainder is added to the position's owed tokens
if (callerDelta.amount0() > 0) {
position.tokensOwed0 += uint128(callerDelta.amount0());
range.key.currency0.take(manager, address(this), uint128(callerDelta.amount0()), true);
callerDelta = toBalanceDelta(0, callerDelta.amount1());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

possibly contentious...

for cases where some - but not all - fees are used to increase liquidity, any remainder fees are credited to the position's tokensOwed. it feels natural in the vanilla case, but in the batching world i wonder if the credit could be used towards another operation..

@saucepoint saucepoint mentioned this pull request Jun 20, 2024
11 tasks
@hensha256 hensha256 added the posm position manager label Jun 26, 2024
snreynolds and others added 2 commits June 26, 2024 15:04
* change add liq accounting

* remove rand comments

* fix exact fees

* use closeAllDeltas

* comments cleanup

* additional liquidity tests (#129)

* additional increase liquidity tests

* edge case of using cached fees for autocompound

* wip

* fix autocompound bug, use custodied and unclaimed fees in the autocompound

* fix tests and use BalanceDeltas (#130)

* fix some assertions

* use BalanceDeltas for arithmetic

* cleanest code in the game???

* additional cleaning

* typo lol

* autocompound gas benchmarks

* autocompound excess credit gas benchmark

* save 600 gas, cleaner code when moving caller delta to tokensOwed

---------

Co-authored-by: saucepoint <98790946+saucepoint@users.noreply.github.com>
* update decrease

* update collect

* update decrease/collect

* remove delta function

* update burn
@snreynolds
Copy link
Member

Closing in favor of #141

@snreynolds snreynolds closed this Jul 9, 2024
@saucepoint saucepoint deleted the posm-batch 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.

3 participants