Impact
The user will receive poolTokenAmount of Weth rather than actually sell the poolTokenAmount of Pool Tokens.
Description
The function TSwapPool::sellPoolTokens incorrectly calls TSwapPool::swapExactOutput where it should call
TSwapPool::swapExactInput as the user has specified the poolTokenAmount as input tokens to sell.
Recommendation
The following is indicative of possible solution.
/**
* @notice wrapper function to facilitate users
* selling pool tokens in exchange of WETH
* @param poolTokenAmount amount of pool tokens to sell
* @return wethAmount amount of WETH received by caller
*/
function sellPoolTokens(
uint256 poolTokenAmount
) external returns (uint256 wethAmount) {
return
- swapExactOutput(
- i_poolToken,
- i_wethToken,
- poolTokenAmount,
- uint64(block.timestamp)
- );
+ swapExactInput(
+ i_poolToken,
+ poolTokenAmount,
+ i_wethToken,
+ poolTokenAmount - 1, // we'll loose a little in fees
+ uint64(block.timestamp)
+ );
}
The NatSpec is as follows:
* @notice figures out how much you need to input based on how much
* output you want to receive.
*
* Example: You say "I want 10 output WETH, and my input is DAI"
* The function will figure out how much DAI you need to input to get 10 WETH
* And then execute the swap
* @param inputToken ERC20 token to pull from caller
* @param outputToken ERC20 token to send to caller
* @param outputAmount The exact amount of tokens to send to caller
*/
function swapExactOutput(
IERC20 inputToken,
IERC20 outputToken,
uint256 outputAmount,
uint64 deadline
)
pragma solidity 0.8.20;
import { Test, console } from "forge-std/Test.sol";
import { PoolFactory } from "../../src/PoolFactory.sol";
import { TSwapPool } from "../../src/TSwapPool.sol";
import { ERC20Mock } from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol";
import {USDCMock} from "./USDCMock.sol";
import {WETHMock} from "./WETHMock.sol";
contract ExperimentTest is Test {
TSwapPool pool;
USDCMock poolToken;
WETHMock weth;
PoolFactory poolFactory;
address liquidityProvider = makeAddr("liquidityProvider");
address user = makeAddr("user");
address poolAddress;
function setUp() public {
poolToken = new USDCMock();
weth = new WETHMock();
poolFactory = new PoolFactory( address(weth) );
poolAddress = poolFactory.createPool( address(poolToken) );
pool = TSwapPool(poolFactory.getPool( address(poolToken) ));
}
function testSellPoolTokens() public {
uint256 hundred = 1_000_000_000;
weth.mint(liquidityProvider, hundred*100);
weth.mint(liquidityProvider, hundred*100);
poolToken.mint(liquidityProvider, hundred);
poolToken.mint(liquidityProvider, hundred);
vm.startPrank(liquidityProvider);
weth.approve(address(pool), hundred);
poolToken.approve(address(pool), hundred);
pool.deposit(hundred, hundred, hundred, uint64(block.timestamp));
vm.stopPrank();
uint256 userBal = poolToken.balanceOf(address(user));
assertEq(0, userBal);
poolToken.mint(user, 100);
vm.startPrank(user);
weth.approve(address(pool), hundred*100);
poolToken.approve(address(pool), hundred*100);
uint256 ptBalBefore = poolToken.balanceOf(address(user));
console.log("Sell 9 pool tokens");
pool.sellPoolTokens(9);
vm.stopPrank();
uint256 ptBalAfter = poolToken.balanceOf(address(user));
console.log("pt before sell ", ptBalBefore);
console.log("pt after sell ", ptBalAfter);
}
}
Result
forge test --mt testSellPoolTokens -vvv
Ran 1 test for test/unit/ExperimentTest.t.sol:ExperimentTest
[PASS] testSellPoolTokens() (gas: 363232)
Logs:
Sell 9 pool tokens
pt before sell 100
pt after sell 10
The result of attempting to sell 9 (nine) pool tokens resulted in the user selling 90 (ninety pool tokens).
Running the test with the correct implementation as shown in the diff above yields the following result, which looks correct.
Ran 1 test for test/unit/ExperimentTest.t.sol:ExperimentTest
[PASS] testSellPoolTokens() (gas: 363362)
Logs:
Sell 9 pool tokens
pt before sell 100
pt after sell 91
Note: the bug might be easier to see if you use a Pool Token to Weth ratio that's not 1:1, such as the following:
pool.deposit(hundred, hundred, hundred/2, uint64(block.timestamp));
Furthermore, the implementation of the getInputAmountBasedOnOutput which is called by swapExactOutput incorrectly calculates the inputAmount return value by multiplying by 10_000 rather than 1_000 as described in the documentation.
References
Tools Used
