Skip to content

Commit

Permalink
check posm is the locker before allowing access to external functions
Browse files Browse the repository at this point in the history
  • Loading branch information
saucepoint committed Jul 1, 2024
1 parent 6db1674 commit 4224c5b
Show file tree
Hide file tree
Showing 12 changed files with 43 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/autocompound_exactUnclaimedFees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
299380
299532
Original file line number Diff line number Diff line change
@@ -1 +1 @@
238844
238996
2 changes: 1 addition & 1 deletion .forge-snapshots/autocompound_excessFeesCredit.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
319919
320071
2 changes: 1 addition & 1 deletion .forge-snapshots/decreaseLiquidity_erc20.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
214439
214591
2 changes: 1 addition & 1 deletion .forge-snapshots/decreaseLiquidity_erc6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
214451
214603
2 changes: 1 addition & 1 deletion .forge-snapshots/increaseLiquidity_erc20.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
194596
194748
2 changes: 1 addition & 1 deletion .forge-snapshots/increaseLiquidity_erc6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
194608
194760
2 changes: 1 addition & 1 deletion .forge-snapshots/mintWithLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
513548
513700
28 changes: 16 additions & 12 deletions contracts/NonfungiblePositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
// maps the ERC721 tokenId to the keys that uniquely identify a liquidity position (owner, range)
mapping(uint256 tokenId => TokenPosition position) public tokenPositions;

// TODO: TSTORE this jawn
// TODO: TSTORE these jawns
address internal msgSender;
bool internal unlockedByThis;

// TODO: Context is inherited through ERC721 and will be not useful to use _msgSender() which will be address(this) with our current mutlicall.
function _msgSenderInternal() internal override returns (address) {
Expand All @@ -53,6 +54,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit

function unlockAndExecute(bytes[] memory data, Currency[] memory currencies) public returns (int128[] memory) {
msgSender = msg.sender;
unlockedByThis = true;
return abi.decode(manager.unlock(abi.encode(data, currencies)), (int128[]));
}

Expand Down Expand Up @@ -88,8 +90,7 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
uint256 deadline,
address owner,
bytes calldata hookData
) external payable {
// TODO: security -- what happens if PoolManager is already unlocked by UR???
) external payable onlyIfUnlocked {
_increaseLiquidity(owner, range, liquidity, hookData);

// mint receipt token
Expand Down Expand Up @@ -121,20 +122,20 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
function increaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
external
isAuthorizedForToken(tokenId)
onlyIfUnlocked
{
TokenPosition memory tokenPos = tokenPositions[tokenId];

// TODO: security -- what happens if PoolManager is already unlocked by UR???
_increaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
}

function decreaseLiquidity(uint256 tokenId, uint256 liquidity, bytes calldata hookData, bool claims)
external
isAuthorizedForToken(tokenId)
onlyIfUnlocked
{
TokenPosition memory tokenPos = tokenPositions[tokenId];

// TODO: security -- what happens if PoolManager is already unlocked by UR???
_decreaseLiquidity(tokenPos.owner, tokenPos.range, liquidity, hookData);
}

Expand All @@ -151,15 +152,13 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
}

// TODO: in v3, we can partially collect fees, but what was the usecase here?
function collect(uint256 tokenId, address recipient, bytes calldata hookData, bool claims) external {
function collect(uint256 tokenId, address recipient, bytes calldata hookData, bool claims)
external
onlyIfUnlocked
{
TokenPosition memory tokenPos = tokenPositions[tokenId];

// TODO: security -- what happens if PoolManager is already unlocked by UR???
_collect(recipient, tokenPos.range, hookData);
if (recipient != _msgSenderInternal()) {
tokenPos.range.poolKey.currency0.close(manager, recipient);
tokenPos.range.poolKey.currency1.close(manager, recipient);
}
_collect(recipient, tokenPos.owner, tokenPos.range, hookData);
}

function feesOwed(uint256 tokenId) external view returns (uint256 token0Owed, uint256 token1Owed) {
Expand Down Expand Up @@ -190,4 +189,9 @@ contract NonfungiblePositionManager is INonfungiblePositionManager, BaseLiquidit
require(msg.sender == address(this) || _isApprovedOrOwner(msg.sender, tokenId), "Not approved");
_;
}

modifier onlyIfUnlocked() {
if (!unlockedByThis) revert MustBeUnlockedByThisContract();
_;
}
}
8 changes: 6 additions & 2 deletions contracts/base/BaseLiquidityManagement.sol
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
}

// The recipient may not be the original owner.
function _collect(address owner, LiquidityRange memory range, bytes memory hookData) internal {
function _collect(address recipient, address owner, LiquidityRange memory range, bytes memory hookData) internal {
BalanceDelta callerDelta;
BalanceDelta thisDelta;
Position storage position = positions[owner][range.toId()];
Expand All @@ -233,7 +233,11 @@ abstract contract BaseLiquidityManagement is IBaseLiquidityManagement, SafeCallb
callerDelta = callerDelta + tokensOwed;
thisDelta = thisDelta - tokensOwed;

callerDelta.flush(owner, range.poolKey.currency0, range.poolKey.currency1);
if (recipient == _msgSenderInternal()) {
callerDelta.flush(recipient, range.poolKey.currency0, range.poolKey.currency1);
} else {
callerDelta.closeDelta(manager, recipient, range.poolKey.currency0, range.poolKey.currency1);
}
thisDelta.flush(address(this), range.poolKey.currency0, range.poolKey.currency1);

position.clearTokensOwed();
Expand Down
2 changes: 2 additions & 0 deletions contracts/interfaces/INonfungiblePositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ interface INonfungiblePositionManager {
LiquidityRange range;
}

error MustBeUnlockedByThisContract();

// NOTE: more gas efficient as LiquidityAmounts is used offchain
function mint(
LiquidityRange calldata position,
Expand Down
11 changes: 11 additions & 0 deletions contracts/libraries/TransientLiquidityDelta.sol
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ library TransientLiquidityDelta {
}
}

function closeDelta(
BalanceDelta delta,
IPoolManager manager,
address holder,
Currency currency0,
Currency currency1
) internal {
close(currency0, manager, holder);
close(currency1, manager, holder);
}

function getBalanceDelta(address holder, Currency currency0, Currency currency1)
internal
view
Expand Down

0 comments on commit 4224c5b

Please sign in to comment.