Off-by-one error in TokenLaunchHook::_beforeSwap causes phases to last one block longer than configured
Description
The TokenLaunchHook::_beforeSwap function uses <= (less than or equal) comparisons for phase boundaries, which incorrectly extends each phase by one block beyond the configured duration.
function _beforeSwap(address sender, PoolKey calldata key, SwapParams calldata params, bytes calldata)
internal
override
returns (bytes4, BeforeSwapDelta, uint24)
{
if (launchStartBlock == 0) revert PoolNotInitialized();
if (initialLiquidity == 0) {
uint128 liquidity = StateLibrary.getLiquidity(poolManager, key.toId());
initialLiquidity = uint256(liquidity);
}
@> uint256 blocksSinceLaunch = block.number - launchStartBlock;
@> uint256 newPhase;
@> if (blocksSinceLaunch <= phase1Duration) {
@> newPhase = 1;
@> } else if (blocksSinceLaunch <= phase1Duration + phase2Duration) {
@> newPhase = 2;
@> } else {
@> newPhase = 3;
@> }
@> if (newPhase != currentPhase) {
@> _resetPerAddressTracking();
@> currentPhase = newPhase;
@> lastPhaseUpdateBlock = block.number;
@> }
@> if (currentPhase == 3) {
@> return (BaseHook.beforeSwap.selector, BeforeSwapDeltaLibrary.ZERO_DELTA, LPFeeLibrary.OVERRIDE_FEE_FLAG);
@> }
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
);
}
When blocksSinceLaunch == phase1Duration, all phase1Duration blocks have elapsed - the phase is complete. However, the <= check still evaluates to true, keeping the system in Phase 1 for one additional block.
For example, if phase1Duration = 100:
Block 0-99: Phase 1 (100 blocks - correct)
Block 100: Should be Phase 2, but 100 <= 100 is true, so still Phase 1 (incorrect)
Block 101: Finally Phase 2
This same issue affects the Phase 2 to Phase 3 transition.
The getCurrentPhase() view function correctly uses < instead of <=, creating an inconsistency where the view function reports a different phase than what _beforeSwap actually enforces.
Risk
Likelihood:
This occurs deterministically at the exact block boundaries between phases
Any swap executed at block launchStartBlock + phase1Duration or launchStartBlock + phase1Duration + phase2Duration will be affected
Impact:
Phase 1 and Phase 2 each last one block longer than the configured duration
Users swapping at phase boundaries are subject to the wrong phase's limits, cooldowns, and penalty fees
The getCurrentPhase() view function returns a different result than what _beforeSwap enforces, causing inconsistencies
Proof of Concept
A user executes a swap, right as exactly the phase1Duration has passed. Calling TokenLaunchHook::currentPhase() still returns a value of 1
Add the following test to TokenLaunchHookUnit.t.sol:
function test_OffByOnePhaseTransition() public {
vm.roll(block.number + phase1Duration);
SwapParams memory params = SwapParams({
zeroForOne: true,
amountSpecified: -0.01 ether,
sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE + 1
});
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});
swapRouter.swap{value: 0.01 ether}(key, params, testSettings, ZERO_BYTES);
assertEq(antiBotHook.currentPhase(), 1, "Phase should have transitioned to 2");
}
Recommended Mitigation
Change _beforeSwap to use strictly less than (<) comparisons:
uint256 blocksSinceLaunch = block.number - launchStartBlock;
uint256 newPhase;
- if (blocksSinceLaunch <= phase1Duration) {
+ if (blocksSinceLaunch < phase1Duration) {
newPhase = 1;
- } else if (blocksSinceLaunch <= phase1Duration + phase2Duration) {
+ } else if (blocksSinceLaunch < phase1Duration + phase2Duration) {
newPhase = 2;
} else {
newPhase = 3;
}
