Skip to content

Commit

Permalink
feat: update the FOT detector to check for external fees (#24)
Browse files Browse the repository at this point in the history
* Add support for checking if token has external fees

* use factoryV2 as sentinel test address

* Add mock fot token and test cases

* flat fee for external fees token and fix tests

* detect external transfer failure

* Rename to feeTakenOnTransfer and add mock token test

* forge fmt

* comment

* remove debug logs

* foundry.toml fix

* fix foundry toml

* remove rpc in foundry toml

* comments

* nit readability

* forge fmt

* custom error and comment

* func name

* comments - readability

* var names

* bubble up reason in unknown transfer error case

* nit readability

---------

Co-authored-by: ConjunctiveNormalForm <[email protected]>
  • Loading branch information
zhongeric and ConjunctiveNormalForm authored Jun 28, 2024
1 parent 6101960 commit 4ce5b58
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 14 deletions.
105 changes: 91 additions & 14 deletions src/FeeOnTransferDetector.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import "./lib/UniswapV2Library.sol";
struct TokenFees {
uint256 buyFeeBps;
uint256 sellFeeBps;
bool feeTakenOnTransfer;
bool externalTransferFailed;
}

/// @notice Detects the buy and sell fee for a fee-on-transfer token
Expand All @@ -18,6 +20,7 @@ contract FeeOnTransferDetector {

error SameToken();
error PairLookupFailed();
error UnknownExternalTransferFailure(string reason);

uint256 constant BPS = 10_000;
address internal immutable factoryV2;
Expand Down Expand Up @@ -69,18 +72,18 @@ contract FeeOnTransferDetector {
(uint256 amount0Out, uint256 amount1Out) =
token == token0Address ? (amountToBorrow, uint256(0)) : (uint256(0), amountToBorrow);

uint256 balanceBeforeLoan = ERC20(token).balanceOf(address(this));
uint256 detectorBalanceBeforeLoan = ERC20(token).balanceOf(address(this));

IUniswapV2Pair pair = IUniswapV2Pair(pairAddress);

try pair.swap(amount0Out, amount1Out, address(this), abi.encode(balanceBeforeLoan, amountToBorrow)) {}
try pair.swap(amount0Out, amount1Out, address(this), abi.encode(detectorBalanceBeforeLoan, amountToBorrow)) {}
catch (bytes memory reason) {
result = parseRevertReason(reason);
}
}

function parseRevertReason(bytes memory reason) private pure returns (TokenFees memory) {
if (reason.length != 64) {
if (reason.length != 128) {
assembly {
revert(add(32, reason), mload(reason))
}
Expand All @@ -95,24 +98,81 @@ contract FeeOnTransferDetector {

ERC20 tokenBorrowed = ERC20(amount0 > 0 ? token0 : token1);

(uint256 balanceBeforeLoan, uint256 amountRequestedToBorrow) = abi.decode(data, (uint256, uint256));
uint256 amountBorrowed = tokenBorrowed.balanceOf(address(this)) - balanceBeforeLoan;
(uint256 detectorBalanceBeforeLoan, uint256 amountRequestedToBorrow) = abi.decode(data, (uint256, uint256));
uint256 amountBorrowed = tokenBorrowed.balanceOf(address(this)) - detectorBalanceBeforeLoan;

uint256 buyFeeBps = (amountRequestedToBorrow - amountBorrowed) * BPS / amountRequestedToBorrow;
balanceBeforeLoan = tokenBorrowed.balanceOf(address(pair));
uint256 sellFeeBps;
uint256 buyFeeBps = _calculateBuyFee(amountRequestedToBorrow, amountBorrowed);

(bool feeTakenOnTransfer, bool externalTransferFailed) =
tryExternalTransferAndRevert(tokenBorrowed, amountBorrowed);

uint256 sellFeeBps = _calculateSellFee(pair, tokenBorrowed, amountBorrowed, buyFeeBps);

bytes memory fees = abi.encode(
TokenFees({
buyFeeBps: buyFeeBps,
sellFeeBps: sellFeeBps,
feeTakenOnTransfer: feeTakenOnTransfer,
externalTransferFailed: externalTransferFailed
})
);

// revert with the abi encoded fees
assembly {
revert(add(32, fees), mload(fees))
}
}

/// @notice simple helper function to calculate the buy fee in bps
function _calculateBuyFee(uint256 amountRequestedToBorrow, uint256 amountBorrowed)
internal
pure
returns (uint256 buyFeeBps)
{
buyFeeBps = (amountRequestedToBorrow - amountBorrowed) * BPS / amountRequestedToBorrow;
}

/// @notice helper function to calculate the sell fee in bps
/// - in the case where the transfer fails, we set the sell fee to be the same as the buy fee
function _calculateSellFee(IUniswapV2Pair pair, ERC20 tokenBorrowed, uint256 amountBorrowed, uint256 buyFeeBps)
internal
returns (uint256 sellFeeBps)
{
uint256 pairBalanceBeforeSell = tokenBorrowed.balanceOf(address(pair));
try this.callTransfer(tokenBorrowed, address(pair), amountBorrowed) {
uint256 sellFee = amountBorrowed - (tokenBorrowed.balanceOf(address(pair)) - balanceBeforeLoan);
uint256 amountSold = tokenBorrowed.balanceOf(address(pair)) - pairBalanceBeforeSell;
uint256 sellFee = amountBorrowed - amountSold;
sellFeeBps = sellFee * BPS / amountBorrowed;
} catch (bytes memory) {
sellFeeBps = buyFeeBps;
}
}

bytes memory fees = abi.encode(TokenFees({buyFeeBps: buyFeeBps, sellFeeBps: sellFeeBps}));

// revert with the abi encoded fees
assembly {
revert(add(32, fees), mload(fees))
/// @notice some tokens take fees even when not buy/selling to the pair,
/// or they fail when transferred within the context of an existing swap
/// @return feeTakenOnTransfer boolean indicating whether or not a fee is taken on transfers not involving the pair
/// @return externalTransferFailed boolean indicating whether or not the external transfer failed
function tryExternalTransferAndRevert(ERC20 tokenBorrowed, uint256 amountBorrowed)
internal
returns (bool feeTakenOnTransfer, bool externalTransferFailed)
{
uint256 balanceBeforeLoan = tokenBorrowed.balanceOf(factoryV2);
try this.callTransfer(tokenBorrowed, factoryV2, amountBorrowed, balanceBeforeLoan + amountBorrowed) {}
catch (bytes memory revertData) {
if (revertData.length > 32) {
// transfer itself failed so we did not return abi-encoded `feeTakenOnTransfer` boolean variable
assembly {
revertData := add(revertData, 0x04)
}
string memory reason = abi.decode(revertData, (string));
if (keccak256(bytes(reason)) == keccak256(bytes("TRANSFER_FAILED"))) {
externalTransferFailed = true;
} else {
revert UnknownExternalTransferFailure(reason);
}
} else {
feeTakenOnTransfer = abi.decode(revertData, (bool));
}
}
}

Expand All @@ -121,4 +181,21 @@ contract FeeOnTransferDetector {
function callTransfer(ERC20 token, address to, uint256 amount) external {
token.safeTransfer(to, amount);
}

// function that reverts with a boolean indicating whether or not a fee is taken on the token transfer
// bubbles up any reverts from the token transfer
function callTransfer(ERC20 token, address to, uint256 amount, uint256 expectedBalance) external {
try this.callTransfer(token, to, amount) {}
catch (bytes memory revertData) {
if (revertData.length < 68) revert();
assembly {
revertData := add(revertData, 0x04)
}
revert(abi.decode(revertData, (string)));
}
bytes memory feeTakenOnTransfer = abi.encode(token.balanceOf(to) != expectedBalance);
assembly {
revert(add(32, feeTakenOnTransfer), mload(feeTakenOnTransfer))
}
}
}
56 changes: 56 additions & 0 deletions test/FeeOnTransferDetector.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {stdError} from "forge-std/StdError.sol";
import {TokenFees, FeeOnTransferDetector} from "../src/FeeOnTransferDetector.sol";
import {MockV2Factory} from "./mock/MockV2Factory.sol";
import {MockFotToken} from "./mock/MockFotToken.sol";
import {MockFotTokenWithExternalFees} from "./mock/MockFotTokenWithExternalFees.sol";
import {MockReentrancyFotToken} from "./mock/MockReentrancyFotToken.sol";
import {MockToken} from "./mock/MockToken.sol";

interface IUniswapV2Pair {
Expand Down Expand Up @@ -33,6 +35,40 @@ contract FeeOnTransferDetectorTest is Test {
TokenFees memory fees = detector.validate(address(fotToken), address(otherToken), 1 ether);
assertEq(fees.buyFeeBps, 200);
assertEq(fees.sellFeeBps, 500);
assertEq(fees.feeTakenOnTransfer, false);
assertEq(fees.externalTransferFailed, false);
}

function testBasicFotTokenWithExternalFees() public {
MockFotTokenWithExternalFees fotToken = new MockFotTokenWithExternalFees(500);
MockToken otherToken = new MockToken();
address pair = factory.deployPair(address(fotToken), address(otherToken));
fotToken.setPair(pair);
fotToken.mint(pair, 100 ether);
otherToken.mint(pair, 100 ether);
IUniswapV2Pair(pair).sync();

TokenFees memory fees = detector.validate(address(fotToken), address(otherToken), 1 ether);
assertEq(fees.buyFeeBps, 500);
assertEq(fees.sellFeeBps, 500);
assertEq(fees.feeTakenOnTransfer, true);
assertEq(fees.externalTransferFailed, false);
}

function testReentrancyFotToken() public {
MockReentrancyFotToken fotToken = new MockReentrancyFotToken(500);
MockToken otherToken = new MockToken();
address pair = factory.deployPair(address(fotToken), address(otherToken));
fotToken.setPair(pair);
fotToken.mint(pair, 100 ether);
otherToken.mint(pair, 100 ether);
IUniswapV2Pair(pair).sync();

TokenFees memory fees = detector.validate(address(fotToken), address(otherToken), 1 ether);
assertEq(fees.buyFeeBps, 500);
assertEq(fees.sellFeeBps, 500);
assertEq(fees.feeTakenOnTransfer, false);
assertEq(fees.externalTransferFailed, true);
}

function testBasicFotTokenFuzz(uint16 buyFee, uint16 sellFee) public {
Expand All @@ -49,6 +85,26 @@ contract FeeOnTransferDetectorTest is Test {
TokenFees memory fees = detector.validate(address(fotToken), address(otherToken), 1 ether);
assertEq(fees.buyFeeBps, buyFee);
assertEq(fees.sellFeeBps, sellFee);
assertEq(fees.feeTakenOnTransfer, false);
assertEq(fees.externalTransferFailed, false);
}

function testBasicFotTokenWithExternalFeesFuzz(uint16 fee) public {
fee = uint16(bound(fee, 0, 10000));
MockFotTokenWithExternalFees fotToken = new MockFotTokenWithExternalFees(fee);
MockToken otherToken = new MockToken();
address pair = factory.deployPair(address(fotToken), address(otherToken));
fotToken.setPair(pair);
fotToken.mint(pair, 100 ether);
otherToken.mint(pair, 100 ether);
IUniswapV2Pair(pair).sync();

TokenFees memory fees = detector.validate(address(fotToken), address(otherToken), 1 ether);
assertEq(fees.buyFeeBps, fee);
assertEq(fees.sellFeeBps, fee);
bool feeTakenOnTransfer = (fee == 0 && fee == 0) ? false : true;
assertEq(fees.feeTakenOnTransfer, feeTakenOnTransfer);
assertEq(fees.externalTransferFailed, false);
}

function testTransferFailsErrorPassthrough() public {
Expand Down
39 changes: 39 additions & 0 deletions test/mock/MockFotTokenWithExternalFees.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity =0.8.19;

import {ERC20} from "solmate/tokens/ERC20.sol";

contract MockFotTokenWithExternalFees is ERC20 {
uint256 public taxBps;
address public pair;

constructor(uint256 _taxBps) ERC20("MockFotToken", "MFOT", 18) {
taxBps = _taxBps;
}

function setPair(address _pair) external {
pair = _pair;
}

function mint(address to, uint256 amount) external {
_mint(to, amount);
}

// this token takes fees on ALL transfers, treating transfers to other addresses as sells
function transfer(address to, uint256 amount) public override returns (bool) {
balanceOf[msg.sender] -= amount;

// Cannot overflow because the sum of all user
// balances can't exceed the max uint256 value.
uint256 feeAmount;
unchecked {
feeAmount = amount * taxBps / 10000;
balanceOf[to] += amount - feeAmount;
balanceOf[address(this)] += feeAmount;
}

emit Transfer(msg.sender, to, amount - feeAmount);

return true;
}
}
47 changes: 47 additions & 0 deletions test/mock/MockReentrancyFotToken.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity =0.8.19;

import {ERC20} from "solmate/tokens/ERC20.sol";
import "v2-core/interfaces/IUniswapV2Pair.sol";

contract MockReentrancyFotToken is ERC20 {
uint256 public taxBps;
address public pair;

constructor(uint256 _taxBps) ERC20("MockReentrancyFotToken", "MFOT", 18) {
taxBps = _taxBps;
}

function setPair(address _pair) external {
pair = _pair;
}

function mint(address to, uint256 amount) external {
_mint(to, amount);
}

// this token takes a fee on all transfers, and swaps the fee on the V2 pair for external transfers (not buy/sells)
function transfer(address to, uint256 amount) public override returns (bool) {
balanceOf[msg.sender] -= amount;

// Cannot overflow because the sum of all user
// balances can't exceed the max uint256 value.
uint256 feeAmount;
unchecked {
feeAmount = amount * taxBps / 10000;
balanceOf[to] += amount - feeAmount;
balanceOf[address(this)] += feeAmount;
}

// Only add in extra swap if is not buy/sell on the pair
if (to != pair && msg.sender != pair) {
IUniswapV2Pair(pair).token0() == address(this)
? IUniswapV2Pair(pair).swap(0, feeAmount, address(this), new bytes(0))
: IUniswapV2Pair(pair).swap(feeAmount, 0, address(this), new bytes(0));
}

emit Transfer(msg.sender, to, amount - feeAmount);

return true;
}
}

0 comments on commit 4ce5b58

Please sign in to comment.