RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

RebateFiHook::_isReFiBuy misclassifies direction (buys flagged as sells when ReFi is on the output side)

themilenkov

RebateFiHook::_isReFiBuy returns zeroForOne when ReFi is currency0 and !zeroForOne otherwise. This causes The hook to apply the wrong fee tier and emits the wrong event.

Description

  • Normal behavior

    The hook should classify a swap as buy if the output side is ReFi (user ends the swap holding more ReFi), and sellotherwise. That choice controls which LP fee (buyFee vs sellFee) overrides the pool and which event is emitted.


  • Issue

    Current logic flips that classification for common cases. For example, with a pool ETH (currency0) ↔ ReFi (currency1), a swap zeroForOne == true (ETH→ReFi) is a buy, but the function returns false, treating it as a sell(applies sellFee and emits ReFiSold).


/// @return True if buying ReFi, false if selling
function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
//@> if (IsReFiCurrency0) {
//@> return zeroForOne; // <-- WRONG for "buy = receive ReFi"
} else {
//@> return !zeroForOne; // <-- WRONG for ReFi == currency1, zeroForOne == true
}
}

Risk

Likelihood:

  • Every swap path uses _isReFiBuy; with the default pool in tests (currency0 = ETH, currency1 = ReFi), common buys (ETH→ReFi) are always misclassified


Impact:

  • Buys are charged sellFee (default 3000 pips = 0.30%) instead of buyFee (default 0), changing user pricing and liquidity incentives.


Proof of Concept

Add to RebateFiHookTest.t.sol. It shows that a clear buy (ETH→ReFi with zeroForOne = true in the default setup) emits ReFiSold (misclassification), not ReFiBought. We only check the indexed address to avoid amount encoding nuances.

function test_BuyReFi_Is_Misclassified_As_Sell_Event() public {
// Setup: user has ETH and will buy ReFi via ETH -> ReFi (zeroForOne = true)
uint256 ethAmount = 0.003 ether;
vm.deal(user1, 1 ether);
​
vm.startPrank(user1);
​
// Prepare swap: zeroForOne == true means currency0 -> currency1 (ETH -> ReFi in default setUp)
SwapParams memory params = SwapParams({
zeroForOne: true, // ETH -> ReFi (this is a BUY)
amountSpecified: -int256(ethAmount), // exact-output convention; abs used only for event here
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
});
​
PoolSwapTest.TestSettings memory testSettings = PoolSwapTest.TestSettings({
takeClaims: false,
settleUsingBurn: false
});
​
// EXPECT the WRONG event due to misclassification: ReFiSold instead of ReFiBought.
// Only topic1 (seller/buyer) is indexed; ignore data (amount/fee).
vm.expectEmit(true, false, false, false, address(rebateHook));
emit ReFiSwapRebateHook.ReFiSold(user1, 0, 0);
​
// Execute the swap; send ETH as msg.value for the exact-out path used in this harness
swapRouter.swap{value: ethAmount}(key, params, testSettings, ZERO_BYTES);
​
vm.stopPrank();
}

Recommended Mitigation

Use a predicate that directly encodes “buy ReFi = ReFi is the output side”:

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
- bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
- if (IsReFiCurrency0) {
- return zeroForOne;
- } else {
- return !zeroForOne;
- }
+ // Buy ReFi if swap outputs ReFi:
+ // - zeroForOne true => output is currency1 => buy when currency1 == ReFi
+ // - zeroForOne false => output is currency0 => buy when currency0 == ReFi
+ return (zeroForOne && Currency.unwrap(key.currency1) == ReFi) ||
+ (!zeroForOne && Currency.unwrap(key.currency0) == ReFi);
}
Updates

Lead Judging Commences

chaossr Lead Judge 11 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Inverted buy/sell logic when ReFi is currency0, leading to incorrect fee application.

Support

FAQs

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

Give us feedback!