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

Test lockless posm via hooks #266

Merged
merged 6 commits into from
Aug 4, 2024

Conversation

saucepoint
Copy link
Collaborator

Description of changes

  • Created a test hook where hookData is passed to modifyLiquidities (lockless posm)
  • tested the major operations, the hook can
    • increase liquidity (without permission)
    • decrease liquidity
    • collect fees
    • burn a position
  • also tested fail scenarios if the hook does not have approval
    • decrease, collect, burn

Follow up tests:

#265

Copy link
Contributor

@hensha256 hensha256 left a comment

Choose a reason for hiding this comment

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

looks great!

@@ -24,12 +25,26 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations {
HookSavesDelta hook;
address hookAddr = address(uint160(Hooks.AFTER_ADD_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG));

HookModifyLiquidities hookModifyLiquidities;
address hookModifyLiquiditiesAddr =
address(uint160(Hooks.BEFORE_SWAP_FLAG | Hooks.AFTER_ADD_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG));
Copy link
Contributor

Choose a reason for hiding this comment

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

it only has a before swap hook?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it inherits HookSavesDelta

i wrote a test where if a hook burns a position, it should have the principal capital of the original position. so needed the hook-delta-saving functionality

test/shared/HookModifyLiquidities.sol Show resolved Hide resolved
/// @dev hook can burn liquidity with approval
function test_hook_burn() public {
// mint some liquidity that is NOT burned in beforeSwap
mint(config, 100e18, address(this), ZERO_BYTES);
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the point in this mint that doesnt get burned?

Copy link
Collaborator Author

@saucepoint saucepoint Aug 4, 2024

Choose a reason for hiding this comment

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

because we're burning a position in beforeSwap, if there is only 1 position the swap will fail haha (all liquidity was burned JIT)

need some "idle" liquidity thats untouched for the swap to succeed

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL good catch

@hensha256 hensha256 merged commit fa247e3 into actions-no-unlock Aug 4, 2024
3 checks passed
@hensha256 hensha256 deleted the sauce/actions-no-unlock-hook branch August 4, 2024 22:55
saucepoint added a commit that referenced this pull request Aug 5, 2024
* actions with no unlock

* Test lockless posm via hooks (#266)

* test hook which modifiesLiquidities in beforeSwap

* test hook modifying liquidity

* minor cleanups

* test that hooks cannot re-enter modifyLiquidities

* hook mints liquidity with modifyLiquidities

* PR cmments

* rename

* rename

* Update src/interfaces/IPositionManager.sol

Co-authored-by: Sara Reynolds <[email protected]>

* misc code comments

---------

Co-authored-by: saucepoint <[email protected]>
Co-authored-by: saucepoint <[email protected]>
Co-authored-by: Sara Reynolds <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants