Ai review for drain liquidity

[koo-virtuals]

Note

High Risk
Adds privileged functions that can transfer out all pool/LP liquidity with minimal slippage safeguards, so misconfiguration or compromised roles could cause irreversible fund loss. Changes touch on critical liquidity custody paths (private pools and graduated UniV2 liquidity).

Overview
Adds privileged liquidity drain flows for Project60days tokens.

FRouterV2 now stores a bondingV2 address (set via new setBondingV2) and introduces drainPrivatePool (drains all balances from an FPairV2 to a recipient) and drainUniV2Pool (drains the founder’s full veToken balance by calling AgentFactoryV6.removeLpLiquidity with amountAMin/amountBMin = 0), emitting new drain events and gating both paths behind EXECUTOR_ROLE plus bondingV2.isProject60days.

FPairV2/IFPairV2 add syncAfterDrain and a Sync event so drained private pools can update internal reserves/k without relying on balanceOf (try/catch in router keeps backward compatibility). Interfaces are extended (IAgentFactoryV6.removeLpLiquidity, IAgentVeTokenV2.founder), and a large new test suite (test/project60days/drainLiquidity.js) covers success paths and expected reverts/role checks for both drain functions.

^{Written by Cursor Bugbot for commit 638e098. This will update automatically on new commits. Configure here.}

[AI-Reviewer-QS]

Branch: AI-review → main
Head Commit: db51643

3 findings reported by AI Reviewer

Issue	Severity	File
Zero Slippage Protection in drainUniV2Pool Enables MEV Sandwich Attacks	High	contracts/launchpadv2/FRouterV2.sol L440-441
Stale Reserve Data After drainPrivatePool	Medium	contracts/launchpadv2/FPairV2.sol L123-136
Maturity Lock Bypass in Drain Operation	Medium	contracts/virtualPersona/AgentVeTokenV2.sol L174-235

[AI-Reviewer-QS]

💥 HIGH: MEV Sandwich Attack via Zero Slippage

IAgentFactoryV6(agentFactory).removeLpLiquidity(
    veToken, recipient, veTokenAmount,
    0,  // ❌ No slippage protection
    0,  // ❌ No slippage protection
    deadline
);

Problem: Hardcoded amountAMin=0, amountBMin=0 disables slippage protection. The inline comment (L433-434) states "No slippage protection needed since EXECUTOR_ROLE is trusted", but this misunderstands MEV mechanics.

Why this matters: Once the transaction enters the public mempool, MEV bots can front-run with price manipulation → your drain executes at manipulated price → bot back-runs for profit. The entire founder LP balance (L428 drains balanceOf(founder)) is at risk. Trusted caller status doesn't prevent mempool-based attacks.

Attack scenario:

1. MEV bot detects drain tx in mempool
2. Front-runs with large swap to manipulate pool price
3. Drain executes at bad price (accepts any amount due to 0 slippage bounds)
4. Bot back-runs to restore price and capture 10-30% of LP value

Fix: Accept slippage params from caller

function drainUniV2Pool(
    address agentToken,
    address veToken,
    address recipient,
    uint256 amountAMin,  // ✅ Add
    uint256 amountBMin,  // ✅ Add
    uint256 deadline
) public onlyRole(EXECUTOR_ROLE) nonReentrant {
    // ...
    IAgentFactoryV6(agentFactory).removeLpLiquidity(
        veToken, recipient, veTokenAmount,
        amountAMin, amountBMin, deadline  // ✅ Use params
    );
}

Until fixed: Use Flashbots Protect for all drain operations.

[koo-virtuals]

This is intentional as the whole project is rugged, drainLiquidity is a MUST though users very likely will dump it.
But usually our BE is the first one knows this project is rugged cuz we define the time threshold of RUGGED.

Conclusion: won't fixed

[AI-Reviewer-QS]

⚠️ MEDIUM: Maturity Lock Bypass

AgentVeTokenV2.sol:174-235

Problem: Inconsistent maturity enforcement. The withdraw() function (L148-152) enforces a matureAt time-lock preventing founders from withdrawing LP tokens before maturity (e.g., 10 years per AgentFactoryV6.sol:351). However, removeLpLiquidity() has no such check.

Bypass path:

• FRouterV2.drainUniV2Pool() → AgentFactoryV6.removeLpLiquidity() → AgentVeTokenV2.removeLpLiquidity()
• None of these functions check matureAt, allowing complete bypass of the time-lock

Inconsistency:

// withdraw() - ENFORCES lock
require(block.timestamp >= matureAt ||
        balanceOf(founder) - veTokenAmount >= initialLock, ...);

// removeLpLiquidity() - NO CHECK
_burn(founder, veTokenAmount); // Bypasses lock

Design intent unclear: This may be intentional to allow protocol-controlled recovery of LP from rugged/failed projects before maturity. The attack surface is limited since only EXECUTOR_ROLE (granted to BE_OPS_WALLET) can trigger this path. However, if the bypass is unintentional, it violates the documented 10-year lock commitment and allows premature loss of founder voting power and rewards.

If bypass is unintentional, add check

function removeLpLiquidity(...) external onlyOwnerOrFactory {
    // ... existing requires ...

    require(  // ✅ Add maturity check
        block.timestamp >= matureAt ||
        balanceOf(founder) - veTokenAmount >= initialLock,
        "Cannot reduce balance below initial lock before maturity"
    );

    // ... rest of function ...
}

If intentional, document

/// @dev BYPASSES matureAt for Project60days emergency drains
function removeLpLiquidity(...) external onlyOwnerOrFactory {
    // ... existing implementation ...
}

[koo-virtuals]

Added document for this

[AI-Reviewer-QS]

✅ Fixed

[AI-Reviewer-QS]

⚠️ MEDIUM: Stale Reserves After Drain

FPairV2.sol:123-130 & FPairV2.sol:132-136

Problem: transferAsset() and transferTo() transfer tokens out of the pair but don't update internal _pool.reserve0/reserve1 storage. FPairV2 has no sync() mechanism like Uniswap V2.

Note: Rated Medium (not Critical) because no direct fund loss is possible - on-chain swaps correctly revert when balances are insufficient.

Impact after drain:

• getReserves() returns stale pre-drain values (reads from storage)
• assetBalance() correctly returns 0 (reads actual ERC20 balance)
• Off-chain systems reading getReserves() get incorrect liquidity data
• On-chain swaps correctly revert (no fund loss), but state divergence creates confusion

Counterargument: If drained pools are never reused (as intended for Project60days), stale reserve values may not matter in practice. However, maintaining correct state prevents operational confusion and ensures contract invariants hold.

Fix: Sync reserves after transfer

function transferAsset(address recipient, uint256 amount) external onlyFactory {
    // ... existing checks ...
    IERC20(tokenB).safeTransfer(recipient, amount);

    _pool.reserve1 = uint112(IERC20(tokenB).balanceOf(address(this)));  // ✅ Add
    _pool.k = uint256(_pool.reserve0) * uint256(_pool.reserve1);
    emit Sync(_pool.reserve0, _pool.reserve1);
}

Apply same fix to transferTo() for reserve0.

Alternative: Add sync() function

function sync() external {
    _pool.reserve0 = uint112(IERC20(tokenA).balanceOf(address(this)));
    _pool.reserve1 = uint112(IERC20(tokenB).balanceOf(address(this)));
    _pool.k = uint256(_pool.reserve0) * uint256(_pool.reserve1);
    emit Sync(_pool.reserve0, _pool.reserve1);
}

[koo-virtuals]

Fixed, and your fix solution is wrong cuz the buy() & sell() in FRouterV2 will call transferAsset() & transferTo() before call swap(), if we update reserve0 & reserve1 in transferAsset() & transferTo() then swap() will twicely deduct the reserve0 & reserve1.

Conclusion: fixed by adding a new syncAfterDrain() function

[AI-Reviewer-QS]

✅ Fixed

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[cursor]

MEV sandwich attack via zero slippage protection

High Severity

The drainUniV2Pool function sets amountAMin=0 and amountBMin=0 when calling removeLpLiquidity, completely disabling slippage protection. The inline comment claims "No slippage protection needed since EXECUTOR_ROLE is trusted" but this misunderstands MEV mechanics. MEV bots can sandwich the transaction in the mempool regardless of who initiated it, extracting significant value from the drain operation. The trust in EXECUTOR_ROLE only prevents unauthorized calls, not front-running attacks.

Additional Locations (1)

• contracts/launchpadv2/FRouterV2.sol#L438-L440