chore: Renamings + comment fixes
- Renamed ExecutorTransferMethods to TokenTransfer to avoid leaking information about how the transfer happens into the executor. The executor shouldn't care if there are multiple methods or one single method that takes care of everything. - Also renamed TransferMethod to TransferType to match the rust encoding
This commit is contained in:
committed by
Diana Carvalho
parent
4a20fa6215
commit
4bc29f283b
@@ -5,14 +5,14 @@ import "@interfaces/IExecutor.sol";
|
||||
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
|
||||
import "@permit2/src/interfaces/IAllowanceTransfer.sol";
|
||||
|
||||
error ExecutorTransferMethods__InvalidPermit2();
|
||||
error TokenTransfer__InvalidPermit2();
|
||||
|
||||
contract ExecutorTransferMethods {
|
||||
contract TokenTransfer {
|
||||
using SafeERC20 for IERC20;
|
||||
|
||||
IAllowanceTransfer public immutable permit2;
|
||||
|
||||
enum TransferMethod {
|
||||
enum TransferType {
|
||||
// Assume funds are in the TychoRouter - transfer into the pool
|
||||
TRANSFER,
|
||||
// Assume funds are in msg.sender's wallet - transferFrom into the pool
|
||||
@@ -25,7 +25,7 @@ contract ExecutorTransferMethods {
|
||||
|
||||
constructor(address _permit2) {
|
||||
if (_permit2 == address(0)) {
|
||||
revert ExecutorTransferMethods__InvalidPermit2();
|
||||
revert TokenTransfer__InvalidPermit2();
|
||||
}
|
||||
permit2 = IAllowanceTransfer(_permit2);
|
||||
}
|
||||
@@ -35,20 +35,18 @@ contract ExecutorTransferMethods {
|
||||
address sender,
|
||||
address receiver,
|
||||
uint256 amount,
|
||||
TransferMethod method
|
||||
TransferType transferType
|
||||
) internal {
|
||||
if (method == TransferMethod.TRANSFER) {
|
||||
if (transferType == TransferType.TRANSFER) {
|
||||
tokenIn.safeTransfer(receiver, amount);
|
||||
} else if (method == TransferMethod.TRANSFERFROM) {
|
||||
} else if (transferType == TransferType.TRANSFERFROM) {
|
||||
// slither-disable-next-line arbitrary-send-erc20
|
||||
tokenIn.safeTransferFrom(sender, receiver, amount);
|
||||
} else if (method == TransferMethod.TRANSFERPERMIT2) {
|
||||
} else if (transferType == TransferType.TRANSFERPERMIT2) {
|
||||
// Permit2.permit is already called from the TychoRouter
|
||||
permit2.transferFrom(
|
||||
sender, receiver, uint160(amount), address(tokenIn)
|
||||
);
|
||||
} else {
|
||||
// Funds are likely already in pool. Do nothing.
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -4,14 +4,14 @@ pragma solidity ^0.8.26;
|
||||
import "@interfaces/IExecutor.sol";
|
||||
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
|
||||
import "@uniswap-v2/contracts/interfaces/IUniswapV2Pair.sol";
|
||||
import "./ExecutorTransferMethods.sol";
|
||||
import "./TokenTransfer.sol";
|
||||
|
||||
error UniswapV2Executor__InvalidDataLength();
|
||||
error UniswapV2Executor__InvalidTarget();
|
||||
error UniswapV2Executor__InvalidFactory();
|
||||
error UniswapV2Executor__InvalidInitCode();
|
||||
|
||||
contract UniswapV2Executor is IExecutor, ExecutorTransferMethods {
|
||||
contract UniswapV2Executor is IExecutor, TokenTransfer {
|
||||
using SafeERC20 for IERC20;
|
||||
|
||||
address public immutable factory;
|
||||
@@ -19,7 +19,7 @@ contract UniswapV2Executor is IExecutor, ExecutorTransferMethods {
|
||||
address private immutable self;
|
||||
|
||||
constructor(address _factory, bytes32 _initCode, address _permit2)
|
||||
ExecutorTransferMethods(_permit2)
|
||||
TokenTransfer(_permit2)
|
||||
{
|
||||
if (_factory == address(0)) {
|
||||
revert UniswapV2Executor__InvalidFactory();
|
||||
@@ -42,14 +42,15 @@ contract UniswapV2Executor is IExecutor, ExecutorTransferMethods {
|
||||
address target;
|
||||
address receiver;
|
||||
bool zeroForOne;
|
||||
TransferMethod method;
|
||||
TransferType transferType;
|
||||
|
||||
(tokenIn, target, receiver, zeroForOne, method) = _decodeData(data);
|
||||
(tokenIn, target, receiver, zeroForOne, transferType) =
|
||||
_decodeData(data);
|
||||
|
||||
_verifyPairAddress(target);
|
||||
|
||||
calculatedAmount = _getAmountOut(target, givenAmount, zeroForOne);
|
||||
_transfer(tokenIn, msg.sender, target, givenAmount, method);
|
||||
_transfer(tokenIn, msg.sender, target, givenAmount, transferType);
|
||||
|
||||
IUniswapV2Pair pool = IUniswapV2Pair(target);
|
||||
if (zeroForOne) {
|
||||
@@ -67,7 +68,7 @@ contract UniswapV2Executor is IExecutor, ExecutorTransferMethods {
|
||||
address target,
|
||||
address receiver,
|
||||
bool zeroForOne,
|
||||
TransferMethod method
|
||||
TransferType transferType
|
||||
)
|
||||
{
|
||||
if (data.length != 62) {
|
||||
@@ -77,7 +78,7 @@ contract UniswapV2Executor is IExecutor, ExecutorTransferMethods {
|
||||
target = address(bytes20(data[20:40]));
|
||||
receiver = address(bytes20(data[40:60]));
|
||||
zeroForOne = uint8(data[60]) > 0;
|
||||
method = TransferMethod(uint8(data[61]));
|
||||
transferType = TransferType(uint8(data[61]));
|
||||
}
|
||||
|
||||
function _getAmountOut(address target, uint256 amountIn, bool zeroForOne)
|
||||
|
||||
@@ -5,14 +5,14 @@ import "@interfaces/IExecutor.sol";
|
||||
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
|
||||
import "@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol";
|
||||
import "@interfaces/ICallback.sol";
|
||||
import {ExecutorTransferMethods} from "./ExecutorTransferMethods.sol";
|
||||
import {TokenTransfer} from "./TokenTransfer.sol";
|
||||
|
||||
error UniswapV3Executor__InvalidDataLength();
|
||||
error UniswapV3Executor__InvalidFactory();
|
||||
error UniswapV3Executor__InvalidTarget();
|
||||
error UniswapV3Executor__InvalidInitCode();
|
||||
|
||||
contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
contract UniswapV3Executor is IExecutor, ICallback, TokenTransfer {
|
||||
using SafeERC20 for IERC20;
|
||||
|
||||
uint160 private constant MIN_SQRT_RATIO = 4295128739;
|
||||
@@ -24,7 +24,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
address private immutable self;
|
||||
|
||||
constructor(address _factory, bytes32 _initCode, address _permit2)
|
||||
ExecutorTransferMethods(_permit2)
|
||||
TokenTransfer(_permit2)
|
||||
{
|
||||
if (_factory == address(0)) {
|
||||
revert UniswapV3Executor__InvalidFactory();
|
||||
@@ -50,7 +50,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
address receiver,
|
||||
address target,
|
||||
bool zeroForOne,
|
||||
TransferMethod method
|
||||
TransferType transferType
|
||||
) = _decodeData(data);
|
||||
|
||||
_verifyPairAddress(tokenIn, tokenOut, fee, target);
|
||||
@@ -60,7 +60,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
IUniswapV3Pool pool = IUniswapV3Pool(target);
|
||||
|
||||
bytes memory callbackData =
|
||||
_makeV3CallbackData(tokenIn, tokenOut, fee, method);
|
||||
_makeV3CallbackData(tokenIn, tokenOut, fee, transferType);
|
||||
|
||||
{
|
||||
(amount0, amount1) = pool.swap(
|
||||
@@ -98,10 +98,10 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
address tokenIn = address(bytes20(msgData[132:152]));
|
||||
|
||||
require(
|
||||
uint8(msgData[171]) <= uint8(TransferMethod.NONE),
|
||||
uint8(msgData[171]) <= uint8(TransferType.NONE),
|
||||
"InvalidTransferMethod"
|
||||
);
|
||||
TransferMethod method = TransferMethod(uint8(msgData[171]));
|
||||
TransferType transferType = TransferType(uint8(msgData[171]));
|
||||
address sender = address(bytes20(msgData[172:192]));
|
||||
|
||||
verifyCallback(msgData[132:]);
|
||||
@@ -109,7 +109,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
uint256 amountOwed =
|
||||
amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta);
|
||||
|
||||
_transfer(IERC20(tokenIn), sender, msg.sender, amountOwed, method);
|
||||
_transfer(IERC20(tokenIn), sender, msg.sender, amountOwed, transferType);
|
||||
|
||||
return abi.encode(amountOwed, tokenIn);
|
||||
}
|
||||
@@ -146,7 +146,7 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
address receiver,
|
||||
address target,
|
||||
bool zeroForOne,
|
||||
TransferMethod method
|
||||
TransferType transferType
|
||||
)
|
||||
{
|
||||
if (data.length != 85) {
|
||||
@@ -158,17 +158,17 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods {
|
||||
receiver = address(bytes20(data[43:63]));
|
||||
target = address(bytes20(data[63:83]));
|
||||
zeroForOne = uint8(data[83]) > 0;
|
||||
method = TransferMethod(uint8(data[84]));
|
||||
transferType = TransferType(uint8(data[84]));
|
||||
}
|
||||
|
||||
function _makeV3CallbackData(
|
||||
address tokenIn,
|
||||
address tokenOut,
|
||||
uint24 fee,
|
||||
TransferMethod method
|
||||
TransferType transferType
|
||||
) internal pure returns (bytes memory) {
|
||||
return abi.encodePacked(
|
||||
tokenIn, tokenOut, fee, uint8(method), msg.sender, self
|
||||
tokenIn, tokenOut, fee, uint8(transferType), msg.sender, self
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -185,7 +185,7 @@ contract TychoRouterTestSetup is Constants, Permit2TestHelper {
|
||||
target,
|
||||
receiver,
|
||||
zero2one,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
}
|
||||
|
||||
@@ -204,7 +204,7 @@ contract TychoRouterTestSetup is Constants, Permit2TestHelper {
|
||||
receiver,
|
||||
target,
|
||||
zero2one,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
pragma solidity ^0.8.26;
|
||||
|
||||
import "@src/executors/UniswapV2Executor.sol";
|
||||
import "@src/executors/ExecutorTransferMethods.sol";
|
||||
import "@src/executors/TokenTransfer.sol";
|
||||
import {Test} from "../../lib/forge-std/src/Test.sol";
|
||||
import {Constants} from "../Constants.sol";
|
||||
import {Permit2TestHelper} from "../Permit2TestHelper.sol";
|
||||
@@ -20,7 +20,7 @@ contract UniswapV2ExecutorExposed is UniswapV2Executor {
|
||||
address target,
|
||||
address receiver,
|
||||
bool zeroForOne,
|
||||
TransferMethod method
|
||||
TransferType transferType
|
||||
)
|
||||
{
|
||||
return _decodeData(data);
|
||||
@@ -84,7 +84,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address(2),
|
||||
address(3),
|
||||
false,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
|
||||
(
|
||||
@@ -92,7 +92,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address target,
|
||||
address receiver,
|
||||
bool zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod method
|
||||
TokenTransfer.TransferType transferType
|
||||
) = uniswapV2Exposed.decodeParams(params);
|
||||
|
||||
assertEq(address(tokenIn), WETH_ADDR);
|
||||
@@ -100,8 +100,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
assertEq(receiver, address(3));
|
||||
assertEq(zeroForOne, false);
|
||||
assertEq(
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFER),
|
||||
uint8(method)
|
||||
uint8(TokenTransfer.TransferType.TRANSFER), uint8(transferType)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -158,7 +157,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
WETH_DAI_POOL,
|
||||
BOB,
|
||||
zeroForOne,
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFER)
|
||||
uint8(TokenTransfer.TransferType.TRANSFER)
|
||||
);
|
||||
|
||||
deal(WETH_ADDR, address(uniswapV2Exposed), amountIn);
|
||||
@@ -177,7 +176,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
WETH_DAI_POOL,
|
||||
BOB,
|
||||
zeroForOne,
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFERFROM)
|
||||
uint8(TokenTransfer.TransferType.TRANSFERFROM)
|
||||
);
|
||||
|
||||
deal(WETH_ADDR, address(this), amountIn);
|
||||
@@ -198,7 +197,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
WETH_DAI_POOL,
|
||||
ALICE,
|
||||
zeroForOne,
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFERPERMIT2)
|
||||
uint8(TokenTransfer.TransferType.TRANSFERPERMIT2)
|
||||
);
|
||||
|
||||
deal(WETH_ADDR, ALICE, amountIn);
|
||||
@@ -211,7 +210,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
);
|
||||
|
||||
// Assume the permit2.approve method will be called from the TychoRouter
|
||||
// Replicate this secnario in this test.
|
||||
// Replicate this scenario in this test.
|
||||
permit2.permit(ALICE, permitSingle, signature);
|
||||
|
||||
uniswapV2Exposed.swap(amountIn, protocolData);
|
||||
@@ -230,7 +229,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
WETH_DAI_POOL,
|
||||
BOB,
|
||||
zeroForOne,
|
||||
uint8(ExecutorTransferMethods.TransferMethod.NONE)
|
||||
uint8(TokenTransfer.TransferType.NONE)
|
||||
);
|
||||
|
||||
deal(WETH_ADDR, address(this), amountIn);
|
||||
@@ -251,7 +250,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address target,
|
||||
address receiver,
|
||||
bool zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod method
|
||||
TokenTransfer.TransferType transferType
|
||||
) = uniswapV2Exposed.decodeParams(protocolData);
|
||||
|
||||
assertEq(address(tokenIn), WETH_ADDR);
|
||||
@@ -259,7 +258,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
assertEq(receiver, 0x0000000000000000000000000000000000000001);
|
||||
assertEq(zeroForOne, false);
|
||||
// TRANSFER = 0
|
||||
assertEq(0, uint8(method));
|
||||
assertEq(0, uint8(transferType));
|
||||
}
|
||||
|
||||
function testSwapIntegration() public {
|
||||
@@ -284,7 +283,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
fakePool,
|
||||
BOB,
|
||||
zeroForOne,
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFER)
|
||||
uint8(TokenTransfer.TransferType.TRANSFER)
|
||||
);
|
||||
|
||||
deal(WETH_ADDR, address(uniswapV2Exposed), amountIn);
|
||||
@@ -304,7 +303,7 @@ contract UniswapV2ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
USDC_MAG7_POOL,
|
||||
BOB,
|
||||
zeroForOne,
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFER)
|
||||
uint8(TokenTransfer.TransferType.TRANSFER)
|
||||
);
|
||||
|
||||
deal(BASE_USDC, address(uniswapV2Exposed), amountIn);
|
||||
|
||||
@@ -22,7 +22,7 @@ contract UniswapV3ExecutorExposed is UniswapV3Executor {
|
||||
address receiver,
|
||||
address target,
|
||||
bool zeroForOne,
|
||||
TransferMethod method
|
||||
TransferType method
|
||||
)
|
||||
{
|
||||
return _decodeData(data);
|
||||
@@ -71,7 +71,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address(2),
|
||||
address(3),
|
||||
false,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
|
||||
(
|
||||
@@ -81,7 +81,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address receiver,
|
||||
address target,
|
||||
bool zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod method
|
||||
TokenTransfer.TransferType method
|
||||
) = uniswapV3Exposed.decodeData(data);
|
||||
|
||||
assertEq(tokenIn, WETH_ADDR);
|
||||
@@ -90,10 +90,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
assertEq(receiver, address(2));
|
||||
assertEq(target, address(3));
|
||||
assertEq(zeroForOne, false);
|
||||
assertEq(
|
||||
uint8(method),
|
||||
uint8(ExecutorTransferMethods.TransferMethod.TRANSFER)
|
||||
);
|
||||
assertEq(uint8(method), uint8(TokenTransfer.TransferType.TRANSFER));
|
||||
}
|
||||
|
||||
function testDecodeParamsInvalidDataLength() public {
|
||||
@@ -124,10 +121,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
|
||||
vm.startPrank(DAI_WETH_USV3);
|
||||
bytes memory protocolData = abi.encodePacked(
|
||||
WETH_ADDR,
|
||||
DAI_ADDR,
|
||||
poolFee,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
WETH_ADDR, DAI_ADDR, poolFee, TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
uint256 dataOffset = 3; // some offset
|
||||
uint256 dataLength = protocolData.length;
|
||||
@@ -161,7 +155,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address(this),
|
||||
DAI_WETH_USV3,
|
||||
zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
|
||||
uint256 amountOut = uniswapV3Exposed.swap(amountIn, data);
|
||||
@@ -185,7 +179,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address(this),
|
||||
DAI_WETH_USV3,
|
||||
zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFERFROM
|
||||
TokenTransfer.TransferType.TRANSFERFROM
|
||||
);
|
||||
|
||||
uint256 amountOut = uniswapV3Exposed.swap(amountIn, data);
|
||||
@@ -207,7 +201,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address(this),
|
||||
DAI_WETH_USV3,
|
||||
zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFERPERMIT2
|
||||
TokenTransfer.TransferType.TRANSFERPERMIT2
|
||||
);
|
||||
|
||||
deal(WETH_ADDR, ALICE, amountIn);
|
||||
@@ -220,7 +214,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
);
|
||||
|
||||
// Assume the permit2.approve method will be called from the TychoRouter
|
||||
// Replicate this secnario in this test.
|
||||
// Replicate this scenario in this test.
|
||||
permit2.permit(ALICE, permitSingle, signature);
|
||||
uint256 amountOut = uniswapV3Exposed.swap(amountIn, data);
|
||||
vm.stopPrank();
|
||||
@@ -243,7 +237,7 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address(this),
|
||||
fakePool,
|
||||
zeroForOne,
|
||||
ExecutorTransferMethods.TransferMethod.TRANSFER
|
||||
TokenTransfer.TransferType.TRANSFER
|
||||
);
|
||||
|
||||
vm.expectRevert(UniswapV3Executor__InvalidTarget.selector);
|
||||
@@ -256,11 +250,17 @@ contract UniswapV3ExecutorTest is Test, Constants, Permit2TestHelper {
|
||||
address receiver,
|
||||
address target,
|
||||
bool zero2one,
|
||||
ExecutorTransferMethods.TransferMethod method
|
||||
TokenTransfer.TransferType transferType
|
||||
) internal view returns (bytes memory) {
|
||||
IUniswapV3Pool pool = IUniswapV3Pool(target);
|
||||
return abi.encodePacked(
|
||||
tokenIn, tokenOut, pool.fee(), receiver, target, zero2one, method
|
||||
tokenIn,
|
||||
tokenOut,
|
||||
pool.fee(),
|
||||
receiver,
|
||||
target,
|
||||
zero2one,
|
||||
transferType
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user