Merge pull request #194 from propeller-heads/router/dc/ENG-4515-univ4-callback

feat: Remove special handling of the Univ4 callback

foundry/src/Dispatcher.sol

@@ -127,7 +127,9 @@ contract Dispatcher {
             tstore(_CURRENTLY_SWAPPING_EXECUTOR_SLOT, 0)
         }
 
-        // this is necessary because the delegatecall will prepend extra bytes we don't want like the length and prefix
+        // The final callback result should not be ABI encoded. That is why we are decoding here.
+        // ABI encoding is very gas expensive and we want to avoid it if possible.
+        // The result from `handleCallback` is always ABI encoded.
         bytes memory decodedResult = abi.decode(result, (bytes));
         return decodedResult;
     }

foundry/src/TychoRouter.sol

@@ -657,13 +657,8 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard {
     /**
      * @dev We use the fallback function to allow flexibility on callback.
      */
-    fallback() external {
-        bytes memory result = _callHandleCallbackOnExecutor(msg.data);
-        // slither-disable-next-line assembly
-        assembly ("memory-safe") {
-            // Propagate the result
-            return(add(result, 32), mload(result))
-        }
+    fallback(bytes calldata data) external returns (bytes memory) {
+        return _callHandleCallbackOnExecutor(data);
     }
 
     /**
@@ -778,18 +773,6 @@ contract TychoRouter is AccessControl, Dispatcher, Pausable, ReentrancyGuard {
         require(msg.sender.code.length != 0);
     }
 
-    /**
-     * @dev Called by UniswapV4 pool manager after achieving unlock state.
-     */
-    function unlockCallback(bytes calldata data)
-        external
-        returns (bytes memory)
-    {
-        if (data.length < 24) revert TychoRouter__InvalidDataLength();
-        bytes memory result = _callHandleCallbackOnExecutor(data);
-        return result;
-    }
-
     /**
      * @dev Gets balance of a token for a given address. Supports both native ETH and ERC20 tokens.
      */

foundry/src/executors/CurveExecutor.sol

@@ -149,6 +149,11 @@ contract CurveExecutor is IExecutor, TokenTransfer {
         receiver = address(bytes20(data[65:85]));
     }
 
+    /**
+     * @dev Even though this contract is mostly called through delegatecall, we still want to be able to receive ETH.
+     * This is needed when using the executor directly and it makes testing easier.
+     * There are some curve pools that take ETH directly.
+     */
     receive() external payable {
         require(msg.sender.code.length != 0);
     }

foundry/src/executors/UniswapV4Executor.sol

@@ -184,8 +184,11 @@ contract UniswapV4Executor is
         external
         returns (bytes memory)
     {
-        verifyCallback(data);
-        return _unlockCallback(data);
+        bytes calldata stripped = data[68:];
+        verifyCallback(stripped);
+        // Our general callback logic returns a not ABI encoded result.
+        // However, the pool manager expects the result to be ABI encoded. That is why we need to encode it here again.
+        return abi.encode(_unlockCallback(stripped));
     }
 
     function verifyCallback(bytes calldata) public view poolManagerOnly {}