Incomplete constructor validation allows zero limits causing all swaps to pay penalty fees
Description
The constructor validates that phase1Duration and phase2Duration are non-zero, but does not apply the same validation to phase1LimitBps and phase2LimitBps. If either limit is deployed as zero, the maxSwapAmount calculation in _beforeSwap during the relevant phase will always result in zero:
uint256 maxSwapAmount = (initialLiquidity * phaseLimitBps) / 10000;
This means every swap will exceed the limit, triggering penalty fees for all users regardless of their swap amount. Since these variables are immutable, this cannot be corrected after deployment and the hook will be permanently broken.
constructor(
IPoolManager _poolManager,
uint256 _phase1Duration,
uint256 _phase2Duration,
uint256 _phase1LimitBps,
uint256 _phase2LimitBps,
uint256 _phase1Cooldown,
uint256 _phase2Cooldown,
uint256 _phase1PenaltyBps,
uint256 _phase2PenaltyBps
) BaseHook(_poolManager) {
if (_phase1Duration == 0 || _phase2Duration == 0) revert InvalidConstructorParams();
if (
@> _phase1LimitBps > 10000 || _phase2LimitBps > 10000 || _phase1PenaltyBps > 10000 || _phase2PenaltyBps > 10000
) revert InvalidConstructorParams();
phase1Duration = _phase1Duration;
phase2Duration = _phase2Duration;
phase1LimitBps = _phase1LimitBps;
phase2LimitBps = _phase2LimitBps;
phase1Cooldown = _phase1Cooldown;
phase2Cooldown = _phase2Cooldown;
phase1PenaltyBps = _phase1PenaltyBps;
phase2PenaltyBps = _phase2PenaltyBps;
currentPhase = 0;
lastPhaseUpdateBlock = 0;
totalPenaltyFeesCollected = 0;
}
Risk
Likelihood:
The contract already validates some constructor parameters (durations), suggesting the developer intended to validate inputs but missed the limit parameters
A deployment script error or misconfiguration could easily pass zero for these values without any revert
Impact:
Every swap during the affected phase will pay penalty fees, regardless of swap size, as maxSwapAmount will be zero
The hook becomes effectively unusable for its intended purpose of protecting legitimate users while penalizing bots - instead it penalizes everyone equally
Since the limit variables are immutable, a new hook contract must be deployed to fix the issue, requiring pool migration
Proof of Concept
As shown in the code below from TokenLaunchHook::_beforeSwap, maxSwapAmount is calculated by (initialLiquidity * phaseLimitBps) / 10000, and phaseLimitBps is derived from uint256 phaseLimitBps = currentPhase == 1 ? phase1LimitBps : phase2LimitBps
This means that if phase1LimitBps or phase2LimitBps are set to zero:
maxSwapAmount = (initialLiquidity * 0) / 10000
maxSwapAmount = 0
This shows that all swaps will enter the if(applyPenalty) block and have to pay penalty fees
function _beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata)
internal
override
returns (bytes4, BeforeSwapDelta, uint24)
{
@> uint256 phaseLimitBps = currentPhase == 1 ? phase1LimitBps : phase2LimitBps;
uint256 phaseCooldown = currentPhase == 1 ? phase1Cooldown : phase2Cooldown;
uint256 phasePenaltyBps = currentPhase == 1 ? phase1PenaltyBps : phase2PenaltyBps;
uint256 swapAmount =
params.amountSpecified < 0 ? uint256(-params.amountSpecified) : uint256(params.amountSpecified);
@> uint256 maxSwapAmount = (initialLiquidity * phaseLimitBps) / 10000;
bool applyPenalty = false;
if (addressLastSwapBlock[sender] > 0) {
uint256 blocksSinceLastSwap = block.number - addressLastSwapBlock[sender];
if (blocksSinceLastSwap < phaseCooldown) {
applyPenalty = true;
}
}
if (!applyPenalty && addressSwappedAmount[sender] + swapAmount > maxSwapAmount) {
applyPenalty = true;
}
addressSwappedAmount[sender] += swapAmount;
addressLastSwapBlock[sender] = block.number;
uint24 feeOverride = 0;
if (applyPenalty) {
feeOverride = uint24((phasePenaltyBps * 100));
}
return (
BaseHook.beforeSwap.selector,
BeforeSwapDeltaLibrary.ZERO_DELTA,
feeOverride | LPFeeLibrary.OVERRIDE_FEE_FLAG
);
}
Recommended Mitigation
Add zero checks for the limit parameters passed into the constructor
constructor(
IPoolManager _poolManager,
uint256 _phase1Duration,
uint256 _phase2Duration,
uint256 _phase1LimitBps,
uint256 _phase2LimitBps,
uint256 _phase1Cooldown,
uint256 _phase2Cooldown,
uint256 _phase1PenaltyBps,
uint256 _phase2PenaltyBps
) BaseHook(_poolManager) {
if (_phase1Duration == 0 || _phase2Duration == 0) revert InvalidConstructorParams();
- if (
- _phase1LimitBps > 10000 || _phase2LimitBps > 10000 || _phase1PenaltyBps > 10000 || _phase2PenaltyBps > 10000
- ) revert InvalidConstructorParams();
+ if (
+ _phase1LimitBps == 0 || _phase2LimitBps == 0 ||_phase1LimitBps > 10000 || _phase2LimitBps > 10000 || _phase1PenaltyBps > 10000 || _phase2PenaltyBps > 10000
+ ) revert InvalidConstructorParams();
phase1Duration = _phase1Duration;
phase2Duration = _phase2Duration;
phase1LimitBps = _phase1LimitBps;
phase2LimitBps = _phase2LimitBps;
phase1Cooldown = _phase1Cooldown;
phase2Cooldown = _phase2Cooldown;
phase1PenaltyBps = _phase1PenaltyBps;
phase2PenaltyBps = _phase2PenaltyBps;
currentPhase = 0;
lastPhaseUpdateBlock = 0;
totalPenaltyFeesCollected = 0;
}
