alpaca-finance/bsc-alpaca-contract

Trading Fee Discrepancy Between Alpaca And PancakeSwapV2

Closed this issue · 1 comments

mgjoo commented

In the Alpaca Finance Protocol, a number of situations require the real-time swap of one token to another. For example, the StrategyAddBaseTokenOnly strategy takes only the base token and converts some portion of it to quote token so that their ratio matches the current swap price in the PancakeSwapV2 pool. Note that in PancakeSwapV2, if you make a token swap or trade on the exchange, you will need to pay a 0.25% trading fee, which is broken down into three parts. The first part of 0.17% is returned to liquidity pools in the form of a fee reward for liquidity providers while the second part of 0.03% is sent to the PancakeSwap Treasury, third part of 0.05% is used to CAKE buyback and burn.

To elaborate, i show below the getAmountOut() routine inside the the PancakeLibraryV2. For comparison, i also show the getMktSellAmount() routine in PancakeswapV2Worker. It is interesting to note that PancakeswapV2Worker has implicitly assumed the trading fee is 0.2%, instead of 0.25%. The difference in the built-in trading fee may skew the optimal allocation of assets in the developed strategies, including StrategyAddBaseTokenOnly and StrategyAddTwoSidesOptimal. It also affects the helper contract, i,e., ibTokenRouter.

PancakeLibraryV2.sol:

  // given an input amount of an asset and pair reserves, returns the maximum output amount of the other asset
  function getAmountOut(uint amountIn, uint reserveIn, uint reserveOut) internal pure returns (uint amountOut) {
    require(amountIn > 0, "PancakeLibrary: INSUFFICIENT_INPUT_AMOUNT");
    require(reserveIn > 0 && reserveOut > 0, "PancakeLibrary: INSUFFICIENT_LIQUIDITY");
    uint amountInWithFee = amountIn.mul(9975);
    uint numerator = amountInWithFee.mul(reserveOut);
    uint denominator = reserveIn.mul(10000).add(amountInWithFee);
    amountOut = numerator / denominator;
  }

PancakeswapV2Worker.sol:

  /// @dev Return maximum output given the input amount and the status of Uniswap reserves.
  /// @param aIn The amount of asset to market sell.
  /// @param rIn the amount of asset in reserve for input.
  /// @param rOut The amount of asset in reserve for output.
  function getMktSellAmount(uint256 aIn, uint256 rIn, uint256 rOut) public view returns (uint256) {
    if (aIn == 0) return 0;
    require(rIn > 0 && rOut > 0, "PancakeswapWorker::getMktSellAmount:: bad reserve values");
    /// if fee is not set, 
    /// revert to V1 fee first as this can implied that migrationLp hasn't bee executed
    uint256 _fee = fee;
    uint256 _feeDenom = feeDenom;
    if (_fee == 0) _fee = 998;
    if (_feeDenom == 0) _feeDenom = 1000;
    uint256 aInWithFee = aIn.mul(_fee);
    uint256 numerator = aInWithFee.mul(rOut);
    uint256 denominator = rIn.mul(_feeDenom).add(aInWithFee);
    return numerator / denominator;
  }

Recommendation: Make the built-in trading fee in Alpaca consistent with the actual trading fee in PancakeSwapV2.

spicysquid168 commented

Hi mgjoo,

Thanks for being a part of our community! Here is the reason that we put those conditions there;

We put if(fee == 0) then fee == 998; if(feeDenom == 0) then feeDenom == 1000 is because once we upgrade the workers, fee and feeDenom will be 0, as previous version of the worker doesn't has these two fee variables, and when we upgraded worker to the new version, the LP still point to PCSv1. Hence, trading fee still need to be calculated as PCSv1 trading fee. Then, there is L353 (

bsc-alpaca-contract/contracts/6/protocol/workers/PancakeswapV2Worker.sol

Line 353 in 643ec0c

fee = 9975;
) where we reassign fee = 9975 and feeDenom = 10000. So, after the migration to PCSv2 is done. Those fee variables will be updated to 9975 and 10000 respectively.

  • 👍1