First Flight #18: T-Swap
First Flight #18
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect fee calculation in `TSwapPool::getInputAmountBasedOnOutput` causes `TSwapPool:swapExactOutput` to take a ~90% fee from users

lulox

Summary

The TSwapPool::getInputAmountBasedOnOutput function is intended to calculate the amount of tokens a user should send in exchange of a specific amount of output tokens. However, the function currently miscalculates the resulting amount. When calculating the fee, it scales the amount by 10000 instead of 1000 required to calculate the 0.3% fee specified in the project's README.md.

It reads: "The TSwap protocol accrues fees from users who make swaps. Every swap has a 0.3 fee, represented in getInputAmountBasedOnOutput and getOutputAmountBasedOnInput. Each applies a 997 out of 1000 multiplier. That fee stays in the protocol."

Impact

Protocol takes more fees than expected from users.

Correct calculation of the fee should be:

997 out of 1000

997 / 1000 = 0.997 | Multiplier

0.997 * 100 = 99.7% | Total after fee

100% - 99.7% = 0.3% | Fee

However, getInputAmountBasedOnOutput currently calculates the fee as:

997 out of 10000

997 / 10000 = 0.0997 | Multiplier

0.0997 * 100 = 9.97% | Total after fee

100% - 9.97% = 90.03% | Fee

Proof of Code

Every time a user swaps with TSwapPool:swapExactOutput, fee is miscalculated.

Include this test in TSwapPool.t.sol:

Proof of Code
function testGetInputAmountBasedOnOutputIncorrectFeeCalculation() public {
// Liquidity provider adds liquidity to the pool
testDeposit();
​
// User starts out with 10e18 pool tokens, but only intends to spend one
uint256 intendedInput = 1e18;
uint256 userPoolTokenBalanceBefore = poolToken.balanceOf(address(user));
console.log("User pool token balance before: ", userPoolTokenBalanceBefore);
console.log("Intended input: ", intendedInput);
// We calculate the expected output amount based on the input amount we're willing to spend
uint256 expectedOutput = pool.getOutputAmountBasedOnInput(
intendedInput, poolToken.balanceOf(address(pool)), weth.balanceOf(address(pool))
);
​
vm.startPrank(user);
// User approves more tokens than its willing to swap
poolToken.approve(address(pool), type(uint256).max);
// swapExactOutput returns the actual input amount incorrectly given by getInputAmountBasedOnOutput
uint256 actualInput = pool.swapExactOutput(poolToken, weth, expectedOutput, uint64(block.timestamp));
uint256 userPoolTokenBalanceAfter = poolToken.balanceOf(address(user));
console.log("Actual input: ", actualInput);
console.log("User pool token balance after: ", userPoolTokenBalanceAfter);
​
// The intended input amount is different from the actual input amount
assert(actualInput > intendedInput);
assert(userPoolTokenBalanceBefore - userPoolTokenBalanceAfter > intendedInput);
// The swap drained more than 90% of the user's pool tokens
assert(userPoolTokenBalanceAfter < 1e18);
}

Tools Used

Foundry and manual review

Recommendations

Don't use magic numbers, declare them as constants instead.

+ uint256 private constant PRECISION = 1000;
+ uint256 private constant FEE = 997;
.
.
.
function getInputAmountBasedOnOutput(
uint256 outputAmount,
uint256 inputReserves,
uint256 outputReserves
)
public
pure
revertIfZero(outputAmount)
revertIfZero(outputReserves)
returns (uint256 inputAmount)
{
- return ((inputReserves * outputAmount) * 10000) / ((outputReserves - outputAmount) * 997);
+ return ((inputReserves * outputAmount) * PRECISION) / ((outputReserves - outputAmount) * FEE);
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect fee calculation in TSwapPool::getInputAmountBasedOnOutput causes protocol to take too many tokens from users, resulting in lost fees

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.