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 : use actions router, remove return vals #214

Merged
merged 16 commits into from
Jul 30, 2024
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_empty.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
50204
47271
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_empty_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
50021
47088
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_nonEmpty.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
135849
130139
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_nonEmpty_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
128771
123060
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
158136
151223
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
149288
142375
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
158136
151223
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_decreaseLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123679
116766
Original file line number Diff line number Diff line change
@@ -1 +1 @@
114905
109375
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_decrease_burnEmpty.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142176
135178
Original file line number Diff line number Diff line change
@@ -1 +1 @@
134916
127917
Original file line number Diff line number Diff line change
@@ -1 +1 @@
136395
129482
Original file line number Diff line number Diff line change
@@ -1 +1 @@
159575
152726
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141458
134609
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142282
135432
Original file line number Diff line number Diff line change
@@ -1 +1 @@
178438
171588
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
379791
372827
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
344574
337610
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_nativeWithSweep.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
351512
344548
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickLower.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
322473
315509
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickUpper.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
323115
316151
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
248697
241733
Original file line number Diff line number Diff line change
@@ -1 +1 @@
328491
321527
Original file line number Diff line number Diff line change
@@ -1 +1 @@
424259
417163
99 changes: 40 additions & 59 deletions src/PositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol"

import {ERC721Permit} from "./base/ERC721Permit.sol";
import {ReentrancyLock} from "./base/ReentrancyLock.sol";
import {IPositionManager, Actions} from "./interfaces/IPositionManager.sol";
import {IPositionManager} from "./interfaces/IPositionManager.sol";
import {SafeCallback} from "./base/SafeCallback.sol";
import {Multicall} from "./base/Multicall.sol";
import {PoolInitializer} from "./base/PoolInitializer.sol";
import {DeltaResolver} from "./base/DeltaResolver.sol";
import {PositionConfig, PositionConfigLibrary} from "./libraries/PositionConfig.sol";
import {BaseActionsRouter} from "./base/BaseActionsRouter.sol";
import {Actions} from "./libraries/Actions.sol";

contract PositionManager is
IPositionManager,
Expand All @@ -29,7 +31,8 @@ contract PositionManager is
Multicall,
SafeCallback,
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove SafeCallback from this list now - thats inherited by BaseActionsRouter

DeltaResolver,
ReentrancyLock
ReentrancyLock,
BaseActionsRouter
{
using SafeTransferLib for *;
using CurrencyLibrary for Currency;
Expand All @@ -48,7 +51,7 @@ contract PositionManager is
IAllowanceTransfer public immutable permit2;

constructor(IPoolManager _poolManager, IAllowanceTransfer _permit2)
SafeCallback(_poolManager)
BaseActionsRouter(_poolManager)
ERC721Permit("Uniswap V4 Positions NFT", "UNI-V4-POSM", "1")
{
permit2 = _permit2;
Expand All @@ -60,80 +63,63 @@ contract PositionManager is
}

/// @param unlockData is an encoding of actions, params, and currencies
/// @return returnData is the endocing of each actions return information
/// @param deadline is the timestamp at which the unlockData will no longer be valid
function modifyLiquidities(bytes calldata unlockData, uint256 deadline)
snreynolds marked this conversation as resolved.
Show resolved Hide resolved
external
payable
isNotLocked
checkDeadline(deadline)
returns (bytes[] memory)
{
// TODO: Edit the encoding/decoding.
return abi.decode(poolManager.unlock(unlockData), (bytes[]));
_executeActions(unlockData);
}

function _unlockCallback(bytes calldata payload) internal override returns (bytes memory) {
(Actions[] memory actions, bytes[] memory params) = abi.decode(payload, (Actions[], bytes[]));

bytes[] memory returnData = _dispatch(actions, params);

return abi.encode(returnData);
function _handleAction(uint256 action, bytes calldata params) internal override {
if (action == Actions.INCREASE_LIQUIDITY) {
_increase(params);
} else if (action == Actions.DECREASE_LIQUIDITY) {
_decrease(params);
} else if (action == Actions.MINT_POSITION) {
_mint(params);
} else if (action == Actions.CLOSE_CURRENCY) {
_close(params);
} else if (action == Actions.BURN_POSITION) {
// Will automatically decrease liquidity to 0 if the position is not already empty.
_burn(params);
Comment on lines +75 to +85
Copy link
Collaborator

Choose a reason for hiding this comment

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

dont need to change it now, and its more a discussion bit --

I think decoding the parameters before making the internal calls makes the internal function signatures much more readable (and easier to reference)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this makes gas go down quite a bit though bc we're just passing calldata

} else {
revert UnsupportedAction(action);
}
saucepoint marked this conversation as resolved.
Show resolved Hide resolved
}

function _dispatch(Actions[] memory actions, bytes[] memory params) internal returns (bytes[] memory returnData) {
uint256 length = actions.length;
if (length != params.length) revert MismatchedLengths();
returnData = new bytes[](length);
for (uint256 i; i < length; i++) {
if (actions[i] == Actions.INCREASE) {
returnData[i] = _increase(params[i]);
} else if (actions[i] == Actions.DECREASE) {
returnData[i] = _decrease(params[i]);
} else if (actions[i] == Actions.MINT) {
// TODO: Mint will be coupled with increase.
returnData[i] = _mint(params[i]);
} else if (actions[i] == Actions.CLOSE_CURRENCY) {
returnData[i] = _close(params[i]);
} else if (actions[i] == Actions.BURN) {
// Will automatically decrease liquidity to 0 if the position is not already empty.
returnData[i] = _burn(params[i]);
} else {
revert UnsupportedAction();
}
}
function _msgSender() internal view override returns (address) {
return _getLocker();
}

/// @param params is an encoding of uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes hookData
/// @return returns an encoding of the BalanceDelta applied by this increase call, including credited fees.
/// @dev Calling increase with 0 liquidity will credit the caller with any underlying fees of the position
function _increase(bytes memory params) internal returns (bytes memory) {
function _increase(bytes memory params) internal {
(uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes memory hookData) =
abi.decode(params, (uint256, PositionConfig, uint256, bytes));

if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
// Note: The tokenId is used as the salt for this position, so every minted position has unique storage in the pool manager.
BalanceDelta delta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
return abi.encode(delta);
BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
}

/// @param params is an encoding of uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes hookData
/// @return returns an encoding of the BalanceDelta applied by this increase call, including credited fees.
/// @dev Calling decrease with 0 liquidity will credit the caller with any underlying fees of the position
function _decrease(bytes memory params) internal returns (bytes memory) {
function _decrease(bytes memory params) internal {
(uint256 tokenId, PositionConfig memory config, uint256 liquidity, bytes memory hookData) =
abi.decode(params, (uint256, PositionConfig, uint256, bytes));

if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker());
if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender());
if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);

// Note: the tokenId is used as the salt.
BalanceDelta delta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
return abi.encode(delta);
BalanceDelta liquidityDelta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
Copy link
Contributor

Choose a reason for hiding this comment

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

this liquidityDelta is unused in all these functions? whats it for?

Copy link
Member Author

Choose a reason for hiding this comment

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

its for slippage checks - sauce is working on that

}

/// @param params is an encoding of PositionConfig memory config, uint256 liquidity, address recipient, bytes hookData where recipient is the receiver / owner of the ERC721
/// @return returns an encoding of the BalanceDelta from the initial increase
function _mint(bytes memory params) internal returns (bytes memory) {
function _mint(bytes memory params) internal {
(PositionConfig memory config, uint256 liquidity, address owner, bytes memory hookData) =
abi.decode(params, (PositionConfig, uint256, address, bytes));

Expand All @@ -146,23 +132,20 @@ contract PositionManager is
_mint(owner, tokenId);

// _beforeModify is not called here because the tokenId is newly minted
BalanceDelta delta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);
BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData);

positionConfigs[tokenId] = config.toId();

return abi.encode(delta);
}

/// @param params is an encoding of the Currency to close
/// @return bytes an encoding of int256 the balance of the currency being settled by this call
function _close(bytes memory params) internal returns (bytes memory) {
function _close(bytes memory params) internal {
(Currency currency) = abi.decode(params, (Currency));
// this address has applied all deltas on behalf of the user/owner
// it is safe to close this entire delta because of slippage checks throughout the batched calls.
int256 currencyDelta = poolManager.currencyDelta(address(this), currency);

// the locker is the payer or receiver
address caller = _getLocker();
address caller = _msgSender();
if (currencyDelta < 0) {
_settle(currency, caller, uint256(-currencyDelta));

Expand All @@ -171,29 +154,27 @@ contract PositionManager is
} else if (currencyDelta > 0) {
_take(currency, caller, uint256(currencyDelta));
}

return abi.encode(currencyDelta);
}

/// @param params is an encoding of uint256 tokenId, PositionConfig memory config, bytes hookData
/// @dev this is overloaded with ERC721Permit._burn
function _burn(bytes memory params) internal returns (bytes memory) {
function _burn(bytes memory params) internal {
(uint256 tokenId, PositionConfig memory config, bytes memory hookData) =
abi.decode(params, (uint256, PositionConfig, bytes));

if (!_isApprovedOrOwner(_getLocker(), tokenId)) revert NotApproved(_getLocker());
if (!_isApprovedOrOwner(_msgSender(), tokenId)) revert NotApproved(_msgSender());
if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId);
uint256 liquidity = uint256(_getPositionLiquidity(config, tokenId));

BalanceDelta liquidityDelta;
// Can only call modify if there is non zero liquidity.
BalanceDelta delta;

if (liquidity > 0) delta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
if (liquidity > 0) {
liquidityDelta = _modifyLiquidity(config, -(liquidity.toInt256()), bytes32(tokenId), hookData);
}

delete positionConfigs[tokenId];
// Burn the token.
_burn(tokenId);
return abi.encode(delta);
}

function _modifyLiquidity(PositionConfig memory config, int256 liquidityChange, bytes32 salt, bytes memory hookData)
Expand Down
5 changes: 2 additions & 3 deletions src/base/BaseActionsRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ abstract contract BaseActionsRouter is SafeCallback {

/// @notice internal function that triggers the execution of a set of actions on v4
/// @dev inheriting contracts should call this function to trigger execution
function _executeActions(bytes calldata params) internal {
poolManager.unlock(params);
function _executeActions(bytes calldata unlockData) internal {
poolManager.unlock(unlockData);
}

/// @notice function that is called by the PoolManager through the SafeCallback.unlockCallback
Expand All @@ -38,7 +38,6 @@ abstract contract BaseActionsRouter is SafeCallback {
_handleAction(action, params[actionIndex]);
}

// TODO do we want to return anything?
return "";
}

Expand Down
14 changes: 1 addition & 13 deletions src/interfaces/IPositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,9 @@ pragma solidity ^0.8.24;

import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";

enum Actions {
MINT,
BURN,
INCREASE,
DECREASE,
// Any positive delta on a currency will be sent to specified address
CLOSE_CURRENCY
}

interface IPositionManager {
error MismatchedLengths();
error NotApproved(address caller);
error DeadlinePassed();
error UnsupportedAction();
error IncorrectPositionConfigForTokenId(uint256 tokenId);

/// @notice Maps the ERC721 tokenId to a configId, which is a keccak256 hash of the position's pool key, and range (tickLower, tickUpper)
Expand All @@ -28,8 +17,7 @@ interface IPositionManager {
/// @notice Batches many liquidity modification calls to pool manager
/// @param payload is an encoding of actions, and parameters for those actions
/// @param deadline is the deadline for the batched actions to be executed
/// @return returnData is the endocing of each actions return information
function modifyLiquidities(bytes calldata payload, uint256 deadline) external payable returns (bytes[] memory);
function modifyLiquidities(bytes calldata payload, uint256 deadline) external payable;

function nextTokenId() external view returns (uint256);
}
4 changes: 4 additions & 0 deletions src/libraries/Actions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ library Actions {
// minting/burning 6909s to close deltas
uint256 constant MINT_6909 = 0x20;
uint256 constant BURN_6909 = 0x21;

// mint + burn ERC721 position
uint256 constant MINT_POSITION = 0x22;
uint256 constant BURN_POSITION = 0x23;
}
Loading
Loading