Vanguard

First Flight #56
Beginner FriendlyDeFiFoundry
0 EXP
Submission Details
Impact: low
Likelihood: low

Fee Calculation Edge Case at Maximum Penalty Value

Author Revealed upon completion

Description

  • The penalty fee is converted from basis points (1/10000) to Uniswap fee pips (1/1000000) by multiplying by 100.

  • For maximum penalty of 10000 bps (100%), this results in 1,000,000 fee pips.

  • While this fits in uint24 (max ~16.7 million), it represents a 100% fee which may have unintended effects.

function _beforeSwap(...) internal override returns (...) {
// ...
if (applyPenalty) {
feeOverride = uint24((phasePenaltyBps * 100));
// @> If phasePenaltyBps = 10000 (100%), feeOverride = 1,000,000
// @> This is the maximum valid fee in Uniswap (100%)
}
return (
BaseHook.beforeSwap.selector,
BeforeSwapDeltaLibrary.ZERO_DELTA,
feeOverride | LPFeeLibrary.OVERRIDE_FEE_FLAG
);
}

Risk

Likelihood:

  • Owner would need to set penalty to exactly 10000 bps (100%)

  • Edge case that may not be intentional

Impact:

  • 100% fee means the entire swap output goes to LPs

  • User receives nothing from the swap

  • While technically valid, may not be intended behavior

Proof of Concept

Max penalty 10000 bps converts to 1,000,000 pips - a 100% fee where user receives nothing.

function test_MaxPenaltyFee() public {
// Constructor allows 10000 bps (100%) penalty
uint256 maxPenalty = 10000;
uint24 feeResult = uint24(maxPenalty * 100);
assertEq(feeResult, 1000000, "100% fee in pips");
// This is at the boundary of uint24 capacity
assertTrue(feeResult <= type(uint24).max, "Fits in uint24");
// But represents 100% fee - user gets nothing
}

Recommended Mitigation

Cap penalty at a reasonable max (e.g., 50%) to prevent 100% fee edge case.

constructor(...) BaseHook(_poolManager) {
// ...
if (
- _phase1LimitBps > 10000 || _phase2LimitBps > 10000 ||
- _phase1PenaltyBps > 10000 || _phase2PenaltyBps > 10000
+ _phase1LimitBps > 10000 || _phase2LimitBps > 10000 ||
+ _phase1PenaltyBps > 5000 || _phase2PenaltyBps > 5000 // Cap at 50%
) revert InvalidConstructorParams();
// ...
}

Support

FAQs

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

Give us feedback!