Skip to content

Commit

Permalink
restrict increase, decrease, and burn and v4 initialize pool call (#416)
Browse files Browse the repository at this point in the history
* restrict increase, decrease, and burn

* prettier

* initialize v4 pool call

* prettier

* refactor logic into function, plus optimise decode

* merge latest periphery

* move v3 logic outside of dispatcher

* initialize pool tests

* sbapshot

* larger refactor

* pull v4 periphery and revert payments

* make checkV4InitializeCall like the others

* Call initialize directly

* undo v3 refactor

* imports

* remove unused typescript type

* remove unnecessary extra immutable

* Bump beta version

* Improve coments

* Add pool intiialization to some tests

---------

Co-authored-by: Alice Henshaw <[email protected]>
  • Loading branch information
dianakocsis and hensha256 authored Oct 17, 2024
1 parent 1889511 commit 5db48b1
Show file tree
Hide file tree
Showing 15 changed files with 178 additions and 723 deletions.
19 changes: 14 additions & 5 deletions contracts/base/Dispatcher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {IAllowanceTransfer} from 'permit2/src/interfaces/IAllowanceTransfer.sol'
import {IERC721Permit} from '@uniswap/v3-periphery/contracts/interfaces/IERC721Permit.sol';
import {ActionConstants} from '@uniswap/v4-periphery/src/libraries/ActionConstants.sol';
import {CalldataDecoder} from '@uniswap/v4-periphery/src/libraries/CalldataDecoder.sol';
import {PoolKey} from '@uniswap/v4-core/src/types/PoolKey.sol';
import {IPoolManager} from '@uniswap/v4-core/src/interfaces/IPoolManager.sol';

/// @title Decodes and Executes Commands
/// @notice Called by the UniversalRouter contract to efficiently decode and execute a singular command
Expand All @@ -24,8 +26,6 @@ abstract contract Dispatcher is Payments, V2SwapRouter, V3SwapRouter, V4SwapRout

error InvalidCommandType(uint256 commandType);
error BalanceTooLow();
error InvalidAction(bytes4 action);
error NotAuthorizedForToken(uint256 tokenId);

/// @notice Executes encoded commands along with provided inputs.
/// @param commands A set of concatenated commands, each 1 byte in length
Expand Down Expand Up @@ -269,12 +269,21 @@ abstract contract Dispatcher is Payments, V2SwapRouter, V3SwapRouter, V4SwapRout
}

(success, output) = address(V3_POSITION_MANAGER).call(inputs);
} else if (command == Commands.V4_POSITION_CALL) {
} else if (command == Commands.V4_INITIALIZE_POOL) {
PoolKey calldata poolKey;
uint160 sqrtPriceX96;
assembly {
poolKey := inputs.offset
sqrtPriceX96 := calldataload(add(inputs.offset, 0xa0))
}
(success, output) =
address(poolManager).call(abi.encodeCall(IPoolManager.initialize, (poolKey, sqrtPriceX96)));
} else if (command == Commands.V4_POSITION_MANAGER_CALL) {
// should only call modifyLiquidities() to mint
// do not permit or approve this contract over a v4 position or someone could use this command to decrease, burn, or transfer your position
_checkV4PositionManagerCall(inputs);
(success, output) = address(V4_POSITION_MANAGER).call{value: address(this).balance}(inputs);
} else {
// placeholder area for commands 0x13-0x20
// placeholder area for commands 0x15-0x20
revert InvalidCommandType(command);
}
}
Expand Down
5 changes: 3 additions & 2 deletions contracts/libraries/Commands.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ library Commands {
uint256 constant V4_SWAP = 0x10;
uint256 constant V3_POSITION_MANAGER_PERMIT = 0x11;
uint256 constant V3_POSITION_MANAGER_CALL = 0x12;
uint256 constant V4_POSITION_CALL = 0x13;
// COMMAND_PLACEHOLDER = 0x14 -> 0x20
uint256 constant V4_INITIALIZE_POOL = 0x13;
uint256 constant V4_POSITION_MANAGER_CALL = 0x14;
// COMMAND_PLACEHOLDER = 0x15 -> 0x20

// Command Types where 0x21<=value<=0x3f
uint256 constant EXECUTE_SUB_PLAN = 0x21;
Expand Down
5 changes: 3 additions & 2 deletions contracts/modules/MigratorImmutables.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity ^0.8.24;

import {INonfungiblePositionManager} from '@uniswap/v3-periphery/contracts/interfaces/INonfungiblePositionManager.sol';
import {IPositionManager} from '@uniswap/v4-periphery/src/interfaces/IPositionManager.sol';
import {IPoolManager} from '@uniswap/v4-core/src/interfaces/IPoolManager.sol';

struct MigratorParameters {
address v3PositionManager;
Expand All @@ -12,9 +13,9 @@ struct MigratorParameters {
/// @title Migrator Immutables
/// @notice Immutable state for liquidity-migration contracts
contract MigratorImmutables {
/// @notice v3PositionManager address
/// @notice v3 PositionManager address
INonfungiblePositionManager public immutable V3_POSITION_MANAGER;
/// @notice v4PositionManager address
/// @notice v4 PositionManager address
IPositionManager public immutable V4_POSITION_MANAGER;

constructor(MigratorParameters memory params) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/modules/Payments.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity ^0.8.24;

import {Constants} from '../libraries/Constants.sol';
import {ActionConstants} from '@uniswap/v4-periphery/src/libraries/ActionConstants.sol';
import {BipsLibrary} from '@uniswap/v4-core/src/libraries/BipsLibrary.sol';
import {BipsLibrary} from '@uniswap/v4-periphery/src/libraries/BipsLibrary.sol';
import {PaymentsImmutables} from '../modules/PaymentsImmutables.sol';
import {SafeTransferLib} from 'solmate/src/utils/SafeTransferLib.sol';
import {ERC20} from 'solmate/src/tokens/ERC20.sol';
Expand Down
42 changes: 42 additions & 0 deletions contracts/modules/V3ToV4Migrator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@ pragma solidity ^0.8.24;

import {MigratorImmutables} from '../modules/MigratorImmutables.sol';
import {INonfungiblePositionManager} from '@uniswap/v3-periphery/contracts/interfaces/INonfungiblePositionManager.sol';
import {Actions} from '@uniswap/v4-periphery/src/libraries/Actions.sol';
import {CalldataDecoder} from '@uniswap/v4-periphery/src/libraries/CalldataDecoder.sol';

/// @title V3 to V4 Migrator
/// @notice A contract that migrates liquidity from Uniswap V3 to V4
abstract contract V3ToV4Migrator is MigratorImmutables {
using CalldataDecoder for bytes;

error InvalidAction(bytes4 action);
error OnlyMintAllowed();
error NotAuthorizedForToken(uint256 tokenId);

/// @dev validate if an action is decreaseLiquidity, collect, or burn
function isValidAction(bytes4 selector) internal pure returns (bool) {
return selector == INonfungiblePositionManager.decreaseLiquidity.selector
Expand All @@ -20,4 +28,38 @@ abstract contract V3ToV4Migrator is MigratorImmutables {
return caller == owner || V3_POSITION_MANAGER.getApproved(tokenId) == caller
|| V3_POSITION_MANAGER.isApprovedForAll(owner, caller);
}

/// @dev check that the v4 position manager call is a safe call
/// of the position-altering Actions, we only allow Actions.MINT
/// this is because, if a user could be tricked into approving the UniversalRouter for
/// their position, an attacker could take their fees, or drain their entire position
function _checkV4PositionManagerCall(bytes calldata inputs) internal view {
bytes4 selector;
assembly {
selector := calldataload(inputs.offset)
}
if (selector != V4_POSITION_MANAGER.modifyLiquidities.selector) {
revert InvalidAction(selector);
}

// slice is `abi.encode(bytes unlockData, uint256 deadline)`
bytes calldata slice = inputs[4:];
// the first bytes(0) extracts the unlockData parameter from modifyLiquidities
// unlockData = `abi.encode(bytes actions, bytes[] params)`
// the second bytes(0) extracts the actions parameter from unlockData
bytes calldata actions = slice.toBytes(0).toBytes(0);

uint256 numActions = actions.length;

for (uint256 actionIndex = 0; actionIndex < numActions; actionIndex++) {
uint256 action = uint8(actions[actionIndex]);

if (
action == Actions.INCREASE_LIQUIDITY || action == Actions.DECREASE_LIQUIDITY
|| action == Actions.BURN_POSITION
) {
revert OnlyMintAllowed();
}
}
}
}
2 changes: 1 addition & 1 deletion lib/v4-periphery
Submodule v4-periphery updated 41 files
+1 −1 .forge-snapshots/PositionManager_burn_nonEmpty_native_withClose.snap
+1 −1 .forge-snapshots/PositionManager_burn_nonEmpty_native_withTakePair.snap
+1 −1 .forge-snapshots/PositionManager_burn_nonEmpty_withClose.snap
+1 −1 .forge-snapshots/PositionManager_burn_nonEmpty_withTakePair.snap
+1 −1 .forge-snapshots/PositionManager_collect_native.snap
+1 −1 .forge-snapshots/PositionManager_collect_sameRange.snap
+1 −1 .forge-snapshots/PositionManager_collect_withClose.snap
+1 −1 .forge-snapshots/PositionManager_collect_withTakePair.snap
+1 −1 .forge-snapshots/PositionManager_decreaseLiquidity_native.snap
+1 −1 .forge-snapshots/PositionManager_decreaseLiquidity_withClose.snap
+1 −1 .forge-snapshots/PositionManager_decreaseLiquidity_withTakePair.snap
+1 −1 .forge-snapshots/PositionManager_decrease_burnEmpty.snap
+1 −1 .forge-snapshots/PositionManager_decrease_burnEmpty_native.snap
+1 −1 .forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap
+1 −1 .forge-snapshots/PositionManager_decrease_take_take.snap
+1 −1 .forge-snapshots/PositionManager_increaseLiquidity_erc20_withClose.snap
+1 −1 .forge-snapshots/PositionManager_increaseLiquidity_erc20_withSettlePair.snap
+1 −1 .forge-snapshots/PositionManager_increaseLiquidity_native.snap
+1 −1 .forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap
+1 −1 .forge-snapshots/PositionManager_increase_autocompound_clearExcess.snap
+1 −1 .forge-snapshots/PositionManager_mint_native.snap
+1 −1 .forge-snapshots/PositionManager_mint_nativeWithSweep_withClose.snap
+1 −1 .forge-snapshots/PositionManager_mint_nativeWithSweep_withSettlePair.snap
+1 −1 .forge-snapshots/PositionManager_mint_onSameTickLower.snap
+1 −1 .forge-snapshots/PositionManager_mint_onSameTickUpper.snap
+1 −1 .forge-snapshots/PositionManager_mint_sameRange.snap
+1 −1 .forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap
+1 −1 .forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap
+1 −1 .forge-snapshots/PositionManager_mint_withClose.snap
+1 −1 .forge-snapshots/PositionManager_mint_withSettlePair.snap
+1 −1 .forge-snapshots/PositionManager_multicall_initialize_mint.snap
+1 −1 .forge-snapshots/PositionManager_permit_twice.snap
+1 −1 lib/v4-core
+1 −1 src/V4Router.sol
+3 −2 src/base/ImmutableState.sol
+10 −0 src/interfaces/IImmutableState.sol
+2 −1 src/interfaces/IPositionManager.sol
+2 −1 src/interfaces/IV4Router.sol
+17 −0 src/libraries/BipsLibrary.sol
+51 −0 test/libraries/BipsLibrary.t.sol
+1 −1 test/router/Payments.t.sol
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"access": "public",
"provenance": true
},
"version": "2.0.0-beta.1",
"version": "2.0.0-beta.2",
"keywords": [
"uniswap",
"router",
Expand Down
2 changes: 1 addition & 1 deletion test/integration-tests/UniswapV2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe('Uniswap V2 Tests:', () => {

beforeEach(async () => {
// cancel the permit on DAI
await permit2.approve(DAI.address, ADDRESS_ZERO, 0, 0)
await permit2.approve(DAI.address, router.address, 0, 0)
})

it('V2 exactIn, permiting the exact amount', async () => {
Expand Down
Loading

0 comments on commit 5db48b1

Please sign in to comment.