Internal audit part 1

.forge-snapshots/BinNativePancakeSwapV4Test#test_v4BinSwap_ExactInSingle_NativeIn.snap

@@ -1 +1 @@
-145951
+145907

.forge-snapshots/BinNativePancakeSwapV4Test#test_v4BinSwap_ExactInSingle_NativeOut_RouterRecipient.snap

@@ -1 +1 @@
-122846
+122802

.forge-snapshots/BinPancakeSwapV4Test#test_v4BinSwap_ExactInSingle.snap

@@ -1 +1 @@
-146913
+146869

.forge-snapshots/BinPancakeSwapV4Test#test_v4BinSwap_ExactIn_MultiHop.snap

@@ -1 +1 @@
-178322
+178278

.forge-snapshots/BinPancakeSwapV4Test#test_v4BinSwap_ExactIn_SingleHop.snap

@@ -1 +1 @@
-148707
+148663

.forge-snapshots/BinPancakeSwapV4Test#test_v4BinSwap_ExactOut_MultiHop.snap

@@ -1 +1 @@
-182133
+182089

.forge-snapshots/BinPancakeSwapV4Test#test_v4BinSwap_ExactOut_SingleHop.snap

@@ -1 +1 @@
-153050
+153006

.forge-snapshots/BinPancakeSwapV4Test#test_v4ClSwap_ExactOutSingle.snap

@@ -1 +1 @@
-151246
+151202

.forge-snapshots/CLNativePancakeSwapV4Test#test_v4ClSwap_ExactInSingle_NativeIn.snap

@@ -1 +1 @@
-172551
+172507

.forge-snapshots/CLNativePancakeSwapV4Test#test_v4ClSwap_ExactInSingle_NativeOut.snap

@@ -1 +1 @@
-174555
+174511

.forge-snapshots/CLPancakeSwapV4Test#test_v4ClSwap_ExactInSingle.snap

@@ -1 +1 @@
-181917
+181873

.forge-snapshots/CLPancakeSwapV4Test#test_v4ClSwap_ExactIn_MultiHop.snap

@@ -1 +1 @@
-247787
+247743

.forge-snapshots/CLPancakeSwapV4Test#test_v4ClSwap_ExactIn_SingleHop.snap

@@ -1 +1 @@
-183422
+183378

.forge-snapshots/CLPancakeSwapV4Test#test_v4ClSwap_ExactOutSingle.snap

@@ -1 +1 @@
-185719
+185675

.forge-snapshots/CLPancakeSwapV4Test#test_v4ClSwap_ExactOut_MultiHop.snap

@@ -1 +1 @@
-250586
+250542

.forge-snapshots/CLPancakeSwapV4Test#test_v4ClSwap_ExactOut_SingleHop.snap

@@ -1 +1 @@
-187265
+187221

.forge-snapshots/PancakeSwapV2Test#test_v2Swap_exactInput0For1.snap

@@ -1 +1 @@
-100169
+100147

.forge-snapshots/PancakeSwapV2Test#test_v2Swap_exactOutput0For1.snap

@@ -1 +1 @@
-100797
+100775

.forge-snapshots/PancakeSwapV3Test#test_v3Swap_ExactInput0For1.snap

@@ -1 +1 @@
-151950
+151906

.forge-snapshots/PancakeSwapV3Test#test_v3Swap_exactOutput0For1.snap

@@ -1 +1 @@
-152828
+152784

.forge-snapshots/StableSwapTest#test_stableSwap_ExactInput0For1.snap

@@ -1 +1 @@
-193254
+193236

.forge-snapshots/StableSwapTest#test_stableSwap_ExactInput1For0.snap

@@ -1 +1 @@
-192710
+192692

.forge-snapshots/UniversalRouterBytecodeSize.snap

@@ -1 +1 @@
-23514
+23397

.forge-snapshots/UniversalRouterTest#test_sweep_token.snap

@@ -1 +1 @@
-55464
+55442

.forge-snapshots/V3ToV4MigrationNativeTest#test_v4CLPositionmanager_Mint_Native.snap

@@ -1 +1 @@
-607648
+607626

.forge-snapshots/V3ToV4MigrationTest#test_v3PositionManager_burn.snap

@@ -1 +1 @@
-291686
+291669

.forge-snapshots/V3ToV4MigrationTest#test_v4BinPositionmanager_BinAddLiquidity.snap

@@ -1 +1 @@
-594516
+594494

.forge-snapshots/V3ToV4MigrationTest#test_v4BinPositionmanager_BinAddLiquidity_Native.snap

@@ -1 +1 @@
-570271
+570249

.forge-snapshots/V3ToV4MigrationTest#test_v4CLPositionmanager_Mint.snap

@@ -1 +1 @@
-631905
+631883

src/UniversalRouter.sol

@@ -12,17 +12,22 @@ import {StableSwapRouter} from "./modules/pancakeswap/StableSwapRouter.sol";
 import {Pausable} from "@openzeppelin/contracts/utils/Pausable.sol";
 
 contract UniversalRouter is RouterImmutables, IUniversalRouter, Dispatcher, Pausable {
-    modifier checkDeadline(uint256 deadline) {
-        if (block.timestamp > deadline) revert TransactionDeadlinePassed();
-        _;
-    }
-
     constructor(RouterParameters memory params)
         RouterImmutables(params)
         StableSwapRouter(params.stableFactory, params.stableInfo)
         V4SwapRouter(params.v4Vault, params.v4ClPoolManager, params.v4BinPoolManager)
     {}
 
+    modifier checkDeadline(uint256 deadline) {
+        if (block.timestamp > deadline) revert TransactionDeadlinePassed();
+        _;
+    }
+
+    /// @notice To receive ETH from WETH
+    receive() external payable {
+        if (msg.sender != address(WETH9) && msg.sender != address(vault)) revert InvalidEthSender();
+    }
+
     /// @inheritdoc IUniversalRouter
     function execute(bytes calldata commands, bytes[] calldata inputs, uint256 deadline)
         external
@@ -76,9 +81,4 @@ contract UniversalRouter is RouterImmutables, IUniversalRouter, Dispatcher, Paus
     function unpause() external onlyOwner whenPaused {
         _unpause();
     }
-
-    /// @notice To receive ETH from WETH
-    receive() external payable {
-        if (msg.sender != address(WETH9) && msg.sender != address(vault)) revert InvalidEthSender();
-    }
 }

src/base/Dispatcher.sol

@@ -8,7 +8,6 @@ import {StableSwapRouter} from "../modules/pancakeswap/StableSwapRouter.sol";
 import {Payments} from "../modules/Payments.sol";
 import {RouterImmutables} from "../base/RouterImmutables.sol";
 import {V3ToV4Migrator} from "../modules/V3ToV4Migrator.sol";
-import {Callbacks} from "../base/Callbacks.sol";
 import {BytesLib} from "../libraries/BytesLib.sol";
 import {Commands} from "../libraries/Commands.sol";
 import {Lock} from "./Lock.sol";
@@ -27,7 +26,6 @@ abstract contract Dispatcher is
     StableSwapRouter,
     V4SwapRouter,
     V3ToV4Migrator,
-    Callbacks,
     Lock
 {
     using BytesLib for bytes;
@@ -37,6 +35,17 @@ abstract contract Dispatcher is
     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
+    /// @param inputs An array of byte strings containing abi encoded inputs for each command
+    function execute(bytes calldata commands, bytes[] calldata inputs) external payable virtual;
+
+    /// @notice Public view function to be used instead of msg.sender, as the contract performs self-reentrancy and at
+    /// times msg.sender == address(this). Instead msgSender() returns the initiator of the lock
+    function msgSender() public view override(BaseActionsRouter) returns (address) {
+        return _getLocker();
+    }
+
     /// @notice Decodes and executes the given command with the given inputs
     /// @param commandType The command type to execute
     /// @param inputs The inputs to execute the command with
@@ -48,10 +57,12 @@ abstract contract Dispatcher is
 
         success = true;
 
-        if (command < Commands.FOURTH_IF_BOUNDARY) {
-            if (command < Commands.SECOND_IF_BOUNDARY) {
+        // 0x00 <= command < 0x21
+        if (command < Commands.EXECUTE_SUB_PLAN) {
+            // 0x00 <= command < 0x10
+            if (command < Commands.V4_SWAP) {
                 // 0x00 <= command < 0x08
-                if (command < Commands.FIRST_IF_BOUNDARY) {
+                if (command < Commands.V2_SWAP_EXACT_IN) {
                     if (command == Commands.V3_SWAP_EXACT_IN) {
                         // equivalent: abi.decode(inputs, (address, uint256, uint256, bytes, bool))
                         address recipient;
@@ -141,8 +152,8 @@ abstract contract Dispatcher is
                         // placeholder area for command 0x07
                         revert InvalidCommandType(command);
                     }
-                    // 0x08 <= command < 0x10
                 } else {
+                    // 0x08 <= command < 0x10
                     if (command == Commands.V2_SWAP_EXACT_IN) {
                         // equivalent: abi.decode(inputs, (address, uint256, uint256, bytes, bool))
                         address recipient;
@@ -226,8 +237,8 @@ abstract contract Dispatcher is
                         revert InvalidCommandType(command);
                     }
                 }
-                // 0x10 <= command < 0x18
             } else {
+                // 0x10 <= command < 0x21
                 if (command == Commands.V4_SWAP) {
                     // pass the calldata provided to V4SwapRouter._executeActions (defined in BaseActionsRouter)
                     _executeActions(inputs);
@@ -244,16 +255,19 @@ abstract contract Dispatcher is
                     (success, output) = address(V3_POSITION_MANAGER).call(inputs);
                 } else if (command == Commands.V3_POSITION_MANAGER_CALL) {
                     bytes4 selector;
-                    uint256 tokenId;
                     assembly {
                         selector := calldataload(inputs.offset)
-                        // tokenId is always the first parameter in the valid actions
-                        tokenId := calldataload(add(inputs.offset, 0x04))
                     }
-
                     if (!isValidAction(selector)) {
                         revert InvalidAction(selector);
                     }
+
+                    uint256 tokenId;
+                    assembly {
+                        // tokenId is always the first parameter in the valid actions
+                        tokenId := calldataload(add(inputs.offset, 0x04))
+                    }
+
                     // If any other address that is not the owner wants to call this function, it also needs to be approved (in addition to this contract)
                     // This can be done in 2 ways:
                     //    1. This contract is permitted for the specific token and the caller is approved for ALL of the owner's tokens
@@ -265,19 +279,19 @@ abstract contract Dispatcher is
                     (success, output) = address(V3_POSITION_MANAGER).call(inputs);
                 } else if (command == Commands.V4_CL_POSITION_CALL) {
                     // should only call modifyLiquidities() with Actions.CL_MINT_POSITION
-                    // do not permit or approve this contract over a v4 position or someone could use this command to decrease/burn your position
+                    // do not permit or approve this contract over a v4 position or someone could use this command to decrease, burn, or transfer your position
                     (success, output) = address(V4_CL_POSITION_MANAGER).call{value: address(this).balance}(inputs);
                 } else if (command == Commands.V4_BIN_POSITION_CALL) {
                     // should only call modifyLiquidities() with Actions.BIN_ADD_LIQUIDITY
-                    // do not permit or approve this contract over a v4 position or someone could use this command to decrease/burn your position
+                    // do not permit or approve this contract over a v4 position or someone could use this command to decrease, burn, or transfer your position
                     (success, output) = address(V4_BIN_POSITION_MANAGER).call{value: address(this).balance}(inputs);
                 } else {
-                    // placeholder area for command
+                    // placeholder area for commands 0x15-0x20
                     revert InvalidCommandType(command);
                 }
             }
-            // 0x20 <= command
         } else {
+            // 0x21 <= command
             if (command == Commands.EXECUTE_SUB_PLAN) {
                 bytes calldata _commands = inputs.toBytes(0);
                 bytes[] calldata _inputs = inputs.toBytesArray(1);
@@ -320,7 +334,7 @@ abstract contract Dispatcher is
                 uint256 amountIn = stableSwapExactOutputAmountIn(amountOut, amountInMax, path, flag);
                 stableSwapExactOutput(map(recipient), amountIn, amountOut, path, flag, payer);
             } else {
-                // placeholder area for commands 0x23-0x3f
+                // placeholder area for commands 0x24-0x3f
                 revert InvalidCommandType(command);
             }
         }
@@ -338,15 +352,4 @@ abstract contract Dispatcher is
             return recipient;
         }
     }
-
-    /// @notice Public view function to be used instead of msg.sender, as the contract performs self-reentrancy and at
-    /// times msg.sender == address(this). Instead msgSender() returns the initiator of the lock
-    function msgSender() public view override(BaseActionsRouter) returns (address) {
-        return _getLocker();
-    }
-
-    /// @notice Executes encoded commands along with provided inputs.
-    /// @param commands A set of concatenated commands, each 1 byte in length
-    /// @param inputs An array of byte strings containing abi encoded inputs for each command
-    function execute(bytes calldata commands, bytes[] calldata inputs) external payable virtual;
 }

src/base/Lock.sol

@@ -3,10 +3,14 @@ pragma solidity ^0.8.24;
 
 import {Locker} from "../libraries/Locker.sol";
 
+/// @title Lock
+/// @notice A contract that provides a reentrancy lock for external calls
 contract Lock {
+    /// @notice Thrown when attempting to reenter a locked function from an external caller
     error ContractLocked();
 
     /// @notice Modifier enforcing a reentrancy lock that allows self-reentrancy
+    /// @dev If the contract is not locked, use msg.sender as the locker
     modifier isNotLocked() {
         // Apply a reentrancy lock for all external callers
         if (msg.sender != address(this)) {