Improve coments

Uniswap · Oct 17, 2024 · b6f402a · b6f402a
1 parent 5bc8881
commit b6f402a
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 5 deletions.
diff --git a/contracts/base/Dispatcher.sol b/contracts/base/Dispatcher.sol
@@ -278,12 +278,12 @@ abstract contract Dispatcher is Payments, V2SwapRouter, V3SwapRouter, V4SwapRout
                     }
                     (success, output) =
                         address(poolManager).call(abi.encodeCall(IPoolManager.initialize, (poolKey, sqrtPriceX96)));
-                } else if (command == Commands.V4_POSITION_CALL) {
+                } else if (command == Commands.V4_POSITION_MANAGER_CALL) {
                     // should only call modifyLiquidities() to mint
                     _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);
                 }
             }

diff --git a/contracts/libraries/Commands.sol b/contracts/libraries/Commands.sol
@@ -36,7 +36,7 @@ library Commands {
     uint256 constant V3_POSITION_MANAGER_PERMIT = 0x11;
     uint256 constant V3_POSITION_MANAGER_CALL = 0x12;
     uint256 constant V4_INITIALIZE_POOL = 0x13;
-    uint256 constant V4_POSITION_CALL = 0x14;
+    uint256 constant V4_POSITION_MANAGER_CALL = 0x14;
     // COMMAND_PLACEHOLDER = 0x15 -> 0x20
 
     // Command Types where 0x21<=value<=0x3f

diff --git a/contracts/modules/V3ToV4Migrator.sol b/contracts/modules/V3ToV4Migrator.sol
@@ -30,6 +30,9 @@ abstract contract V3ToV4Migrator is MigratorImmutables {
     }
 
     /// @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 {
@@ -39,9 +42,11 @@ abstract contract V3ToV4Migrator is MigratorImmutables {
             revert InvalidAction(selector);
         }
 
+        // slice is `abi.encode(bytes unlockData, uint256 deadline)`
         bytes calldata slice = inputs[4:];
-        // first bytes(0) extracts the unlockData parameter from modifyLiquidities
-        // second bytes(0) extracts the actions parameter from unlockData
+        // 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;