Loophole

In NftMarketplace.performSwap, the marketplace sells bToken through bSwap.sellTokensExactIn and receives amountOut in offerToken. The function then calculates recipient shares from amountOut, but transfers bToken to afterburner and blvModule:

if (toAfterburner > 0) IERC20(bToken).safeTransfer(recipients.afterburner, toAfterburner);
if (toBLV > 0) IERC20(bToken).safeTransfer(recipients.blvModule, toBLV);

High. Fee distribution from NFT-sale proceeds is broken.

Transfer offerToken to recipients after the swap:

if (toAfterburner > 0) offerToken.safeTransfer(recipients.afterburner, toAfterburner);
if (toBLV > 0) offerToken.safeTransfer(recipients.blvModule, toBLV);

Add a unit test with a mock IBSwap that pulls bToken, sends offerToken, and verifies the afterburner and BLV recipient balances are updated in offerToken.

ProjectFeeRouterUpgradeable is the single fee router that every pool forwards its creator-fee stream to. Per the protocol's documentation, every presale raises LOOP, so PresaleImplementation.finalizeSale always creates the pool with createParams.reserve = address(presaleToken) = LOOP and createParams.feeRecipient = params.feeRouter. As a result, every non-LOOP bToken issued by PresaleFactory produces creator fees denominated in LOOP, all accruing into the same ProjectFeeRouterUpgradeable instance.

The router tracks fees with per-bToken lastBalance entries but computes the sweep delta against the contract's full reserve-token balance:

function sweep(address bToken) external nonReentrant {
    address r = reserve[bToken];
    if (r == address(0)) revert BTokenNotRegistered();

    IERC20 token = IERC20(r);
    uint256 bal = token.balanceOf(address(this));
    uint256 delta = bal - lastBalance[bToken];
    if (delta == 0) revert NothingToSweep();
    ...
}

Because multiple bTokens share LOOP as their reserve, the pot accumulated from all of them sits in a single balanceOf(address(this)) slot with no per-bToken segregation. The first caller to invoke sweep(X) attributes every reserve token sitting in the router, regardless of which pool emitted it, to X's recipients per X's FeeConfig.

This creates a permissionless race in which any actor can monitor the router's public LOOP balance and call sweep on their preferred bToken immediately after fees from competing bTokens arrive. The losing bTokens either revert with NothingToSweep (if their lastBalance baseline is zero) or revert with an arithmetic underflow on the bal - lastBalance[bToken] subtraction, in the latter case remaining bricked until new inflows restore the stale watermark.

High. Creator-fee streams belonging to one bToken are redirected to the recipients of another bToken sharing the same reserve, and the losing bToken's sweep is denial-of-serviced. Because every non-LOOP bToken issued through PresaleFactory shares LOOP as its reserve, the scope of the issue grows linearly with the number of presales that go live.

Remove the reliance on balanceOf(address(this)) for per-bToken accounting. The cleanest options are:

1. Deploy a distinct ProjectFeeRouterUpgradeable instance per bToken, so each contract's balanceOf is unambiguously attributable.
2. Retain a shared router but switch to push-based accounting: have the pool (or an intermediate hook) deliver fees through a dedicated entry point such as receiveFees(address bToken, uint256 amount) that increments a per-bToken accrued[bToken] accumulator. sweep then distributes accrued[bToken] and zeroes it, severing the dependency on the contract-wide balance.

Option 2 preserves the existing single-router topology and is compatible with multiple bTokens sharing LOOP as reserve.

The following proof of concept, placed in test/ProjectFeeRouter.t.sol, reuses the existing setup in which lstBToken and loopBToken are both registered against the same reserveToken and demonstrates the full drain in a single transaction.

Add the test case to the mentioned test file and execute it with forge t --mt test_PoC_CrossBTokenFeeTheft:

function test_PoC_CrossBTokenFeeTheft() public {

    // Verify initial balances
    assertEq(reserveToken.balanceOf(treasury), 0, "!bal LST treasury");
    assertEq(reserveToken.balanceOf(royalties), 0, "!bal LST royalties");
    assertEq(reserveToken.balanceOf(team), 0, "!bal LOOP team");

    // Assert tokens have the same reserve asset
    assertEq(
        router.reserve(loopBToken),
        router.reserve(lstBToken),
        "!reserve token mismatch"
    );

    // Fees accrue for both bTokens into the shared reserve balance
    uint256 lstFees = 1 ether;
    uint256 loopFees = 2 ether;
    reserveToken.mint(address(router), lstFees + loopFees);

    assertEq(reserveToken.balanceOf(address(router)), 3 ether);

    // LST swept first: takes the entire 3 ether
    router.sweep(lstBToken);

    // LST's recipients received the full 3 ether
    assertEq(
        reserveToken.balanceOf(treasury) + reserveToken.balanceOf(royalties),
        3 ether,
        "!LST receive"
    );

    // LOOP's team got nothing
    assertEq(reserveToken.balanceOf(team), 0, "!LOOP starved");

    // LOOP can't be swept
    vm.expectRevert(ProjectFeeRouterUpgradeable.NothingToSweep.selector);
    router.sweep(loopBToken);
}

The credit-sale finalization is explicitly two-phased:

1. finalizeSale: sets poolCreated = true, but leaves finalized = false.
2. claimCreditBatch is called one or more times by the admin.
3. completeFinalization: sets finalized = true.

Between steps 1 and 3 the contract is in an intermediate state where every user-visible pathway is disabled:

• deposit, setPhaseMerkleRoot, cancelSale: blocked by if (poolCreated) revert PoolAlreadyCreated();.
• refund: requires whenCancelled, which never becomes true without a cancelSale call.
• claimSpot: saleType != Spot reverts.

If the admin never calls completeFinalization or never calls claimCreditBatch for some depositors, such depositors have:

1. No refund path, because cancelSale is blocked.
2. No bToken claim, because credit sales don't expose one.
3. No credit position, because the admin failed to include them in any batch.

Unlike the spot branch, the credit branch requires two additional admin transactions to exit the limbo state. In the unlikely case of an admin error, the funds are lost.

Single point of failure for user funds post-finalization of a credit sale.

Introduce an escape hatch functionality to allow depositors to be refunded in case the admin did not include them in its claimCreditBatch calls.

TODO

_modifyMinAuctionPrice() does not fully prevent a sudden price decrease after the auction has already reached the old floor. In that state, lowering minAuctionPrice can immediately reduce nftCost() below the previous floor price, potentially all the way to the new floor depending on how much time has elapsed.

Examples:

Initial state: Starting Price: 100, MinAuctionPrice: 40

| New Floor | Duration | Elapsed at Change | Adjusted Elapsed | New Price After Change | Result |
|---:|---:|---:|---:|---:|---|
| 20 | 100 | 100 | 100 * 60 / 80 = 75 | 100 - 80 * 75 / 100 = 40 | Preserves old floor |
| 20 | 100 | 110 | 110 * 60 / 80 = 82.5 | 100 - 80 * 82.5 / 100 = 34 | Drops below old floor |
| 20 | 100 | 150 | 150 * 60 / 80 = 112.5 | 20 | Hits new floor |

Low.

The function should not use elapsed time beyond the auctionDuration.

_finalizeCredit gates the transfer of circulating supply on a non-zero recipient:

// src/PresaleImplementation.sol:364-372
function _finalizeCredit(address bToken, uint256 initialCirculatingSupply, address circulatingSupplyRecipient)
    internal
{
    if (circulatingSupplyRecipient != address(0) && initialCirculatingSupply > 0) {
        IERC20(bToken).safeTransfer(circulatingSupplyRecipient, initialCirculatingSupply);
    }

    emit PoolCreatedAndPendingClaims(bToken, createdPoolId);
}

PresaleFactory.createBTokenAndPool always returns the circulating supply bTokens back to the presale contract. So when circulatingSupplyRecipient == 0, the presale holds the circulating bTokens and they are never moved.

The credit-sale branch has no user-facing claim function equivalent to claimSpot: depositors receive credit positions via IBCredit.claimCredit, not bTokens. So there is no code path that can later transfer these bTokens out of the contract.

For every credit sale finalized with circulatingSupplyRecipient = address(0), initialCirculatingSupply bTokens are stranded in the presale proxy forever.

It also mis-prices the pool: the AMM thinks the circulating float is initialCirculatingSupply but the tokens are not actually circulating. Anyone valuing the pool against totalSupply will over-value it.

Either require circulatingSupplyRecipient != address(0) when saleType == Credit (revert at the top of finalizeSale) or burn the circulating supply outright when circulatingSupplyRecipient == 0, so the bTokens no longer inflate totalSupply.

TODO

PresaleImplementation and NftMarketplace use the OpenZeppelin upgradeable primitives but lack the standard implementation-side initializer lock.

Low. Initializers aren't disabled in the implementation contracts.

Add to the top of the contracts:

constructor() {
    _disableInitializers();
}

TODO

The ProjectFeeRouterUpgradeable.sweep() function distributes creator-fee tokens for a registered bToken by computing integer-division slices for each recipient. When the total distributed amount does not equal the delta due to rounding, the remainder is sent to the acquisitionTreasury if configured, otherwise to the team recipient. However, when acquisitionTreasury is the NftMarketplace contract, the remainder tokens are transferred without informing the marketplace's accounting system.

In ProjectFeeRouterUpgradeable.sweep, the router computes toTreasury = (delta * cfg.bpsToAcquisitionTreasury) / 10_000 and transfers it to the acquisition treasury while calling informOfFeeDistribution(bToken, toTreasury).
The marketplace then increments checkpointBalance[nftCollection] by toTreasury via _performCheckpoint().
Separately, any remainder = delta - distributed is transferred to the same acquisition treasury address without a corresponding callback. As a result, the marketplace receives more offerToken than its internal checkpointBalance reflects.

The NftMarketplace.offerPrice() calculation caps the bid at checkpointBalance[nftCollection], meaning the unaccounted remainder tokens effectively become stranded from bidding use. This discrepancy accumulates over multiple sweeps where integer division produces nonzero remainders.

Low. The marketplace holds slightly more reserve tokens than its internal bid capacity indicates.

When remainderRecipient == recips.acquisitionTreasury and the acquisition treasury is the marketplace, include the remainder in the fee distribution callback. This can be achieved by calling informOfFeeDistribution(bToken, toTreasury + remainder) instead of only toTreasury, or by issuing a second callback for the remainder amount. This keeps marketplace checkpointBalance aligned with the actual tokens received and prevents stranded dust from accumulating over time.

The team's stated expectation is that PresaleImplementation.finalizeSale should succeed if and only if one of the following holds:

• All phases have ended AND the soft cap has been reached, or
• Not all phases have ended AND the hard cap has been reached (early finalization is acceptable because no further deposits are possible).

In its current form, finalizeSale only enforces the soft cap check, it does not verify that all phases have ended, nor does it restrict early finalization to the hard cap case. As a consequence, the admin can finalize a presale at any moment after the soft cap is reached, regardless of whether subsequent phases are still active or scheduled.

Low. The path is gated by the onlyAdmin modifier, so exploitation requires admin cooperation. A malicious or careless admin can however prematurely close the presale and deny future-phase depositors their allocation, deviating from the protocol's documented behavior.

Modify finalizeSale so that finalization is allowed if and only if (!allPhasesEnded() && totalRaised >= config.hardCap) || (allPhasesEnded() && totalRaised >= config.softCap). The existing SoftCapNotReached check can then be removed, since both branches above already enforce a stricter condition. For example:

bool phasesEnded = allPhasesEnded();
bool hardCapMet = totalRaised >= config.hardCap;
bool softCapMet = totalRaised >= config.softCap;

if (!((phasesEnded && softCapMet) || (!phasesEnded && hardCapMet))) {
    revert PresaleNotFinalizable();
}

After calling bFactory.createPool, PresaleFactory.createBTokenAndPool sweeps any leftover bToken balance back to the calling presale:

An equivalent sweep is not performed for createParams.reserve. According to Baseline documentation, depending on the parameters passed to bFactory.createPool, a portion of the reserve tokens pulled from the presale may also be left behind in the caller after pool creation.

Prior to the createPool call, the factory pulls the full poolReserves amount from the presale via safeTransferFrom and grants an equivalent allowance to bFactory. If bFactory.createPool consumes less than poolReserves, the unused portion will remain in PresaleFactory with no path out, since the contract exposes no rescue or withdrawal functionality. These stranded tokens represent value that would otherwise have been available to the presale and are effectively lost.

Low.

Sweep any leftover createParams.reserve balance back to the calling presale in PresaleFactory.createBTokenAndPool, and in PresaleImplementation.finalizeSale forward the returned amount to an accountable recipient (for instance params.acquisitionTreasury) immediately after createBTokenAndPool returns, so the funds cannot become stuck in the presale.

In alternative, provide PresaleFactory.createBTokenAndPool with an additional address parameter in order to inform it what account to send such refund to.

NftMarketplace.setCollectionForBToken writes both sides of the bToken/NFT collection relationship:

collectionForBToken[bToken] = nftCollection;
bTokenForCollection[nftCollection] = bToken;

However, it does not check whether either side was already mapped. If a collection is first mapped to bTokenA and later mapped to bTokenB, the old reverse mapping remains:

collectionForBToken[bTokenA] = collection; // stale but still live
collectionForBToken[bTokenB] = collection;
bTokenForCollection[collection] = bTokenB;

This means fees for the old bToken can still be credited to the collection, while NFT purchases for that same collection now use the new bToken. The collection-level auction/checkpoint state is also reused across the new token relationship.

Informational.

Make sure the collectionForBToken[bToken] is zero before setting a new value.

NftMarketplace.startAuction is public and only checks whether the marketplace holds at least one NFT from the supplied collection:

function startAuction(address nftCollection) public whenNotPaused {
    if (auctionStartTimestamp[nftCollection] == 0) {
        require(IERC721(nftCollection).balanceOf(address(this)) != 0, NoNftToAuction());

        auctionStartTimestamp[nftCollection] = block.timestamp;
        emit AuctionStarted(nftCollection);
    }
}

It does not check whether nftCollection is registered through setCollectionForBToken.

As a result, if any NFT from an unregistered collection is transferred to the marketplace, anyone can call startAuction(nftCollection) and set auctionStartTimestamp for that collection. This can also happen through sellNftToVault with an unregistered collection, because offerPrice is zero and a seller can pass minPrice = 0; the function then transfers the NFT and calls startAuction.

uint256 _currentOffer = offerPrice(nftCollection); // zero for unregistered collections
require(minPrice <= _currentOffer, InvalidSalePriceInput());

IERC721(nftCollection).transferFrom(msg.sender, address(this), tokenId);

startAuction(nftCollection);

For an unregistered collection, bTokenForCollection[nftCollection] remains address(0), so downstream pricing and purchase logic are not valid for that collection.

The same missing registration check exists in buyNftFromVault:

function buyNftFromVault(address nftCollection, uint256 tokenId, uint256 maxPrice) external whenNotPaused {
    uint256 _currentPrice = nftCost(nftCollection);
    require(_currentPrice <= maxPrice, InvalidPurchasePriceInput());

    ...

    IERC20(bTokenForCollection[nftCollection]).safeTransferFrom(msg.sender, address(this), _currentPrice);

    IERC721(nftCollection).transferFrom(address(this), msg.sender, tokenId);
}

Informational.

Require collections to be registered before they can be sold into the vault, bought from the vault, or started in auction.

PresaleImplementation.setPhaseMerkleRoot allows the admin to overwrite the merkle root of any configured phase at any point before the pool is created, including while the target phase is already open for deposits:

Since PresaleImplementation.deposit validates callers against phases[phaseId].merkleRoot at execution time, the admin retains the ability to reshape the whitelist for a phase that is currently live. Whitelisted users therefore have no on-chain guarantee that their eligibility, as communicated at phase configuration, will remain valid when they attempt to deposit.

Informational.

Prevent the admin from mutating the merkle root of a phase once that phase has started. For instance:

 function setPhaseMerkleRoot(uint8 phaseId, bytes32 merkleRoot)
     external
     onlyAdmin
     whenNotFinalized
     whenNotCancelled
 {
     if (poolCreated) revert PoolAlreadyCreated();
     if (phaseId >= phases.length) revert InvalidPhaseId();
+    if (block.timestamp >= phases[phaseId].startTime) revert PhaseAlreadyStarted();

     bytes32 oldRoot = phases[phaseId].merkleRoot;
     phases[phaseId].merkleRoot = merkleRoot;

     emit PhaseMerkleRootUpdated(phaseId, oldRoot, merkleRoot);
 }

TODO

The Solidity compiler automatically generates a public getter for every state variable declared public. Contracts in scope define explicit view functions that duplicate the auto-generated getters, providing no additional functionality while increasing bytecode size.

The following instances of redundant getters were identified:

• PresaleImplementation.getUserDepositedAmount
• PresaleImplementation.getCreatedToken
• PresaleImplementation.getCreatedPool
• PresaleImplementation.getTotalRaised
• PresaleImplementation.isFinalized
• PresaleImplementation.isCancelled
• PresaleImplementation.getSaleType
• PresaleImplementation.getTotalClaimableTokens
• PresaleFactory.getBeacon
• PresaleFactory.getBFactoryAddress

Informational.

Remove the redundant getter functions listed above and have external consumers rely on the compiler-generated getters.

PresaleImplementation.initialize validates that phases are ordered in time through the following check:

if (i > 0 && _phases[i].startTime < _phases[i - 1].endTime) {
    revert InvalidPhaseConfiguration();
}

The comparison uses a strict inequality, which means that two adjacent phases are allowed to share a boundary timestamp (i.e. _phases[i].startTime == _phases[i - 1].endTime).

Within PresaleImplementation.deposit, the active phase check treats both bounds as inclusive:

if (block.timestamp < phase.startTime || block.timestamp > phase.endTime) {
    revert PhaseNotActive();
}

As a consequence, at the exact block timestamp equal to the shared boundary, two consecutive phases are simultaneously considered active. A depositor can issue two separate deposit calls in the same block, one targeting each phase, and have both calls succeed. Because userAllocationCap is tracked per-phase via the userDeposits[user][phaseId] mapping, the user may fill their allocation in both phases at once, effectively receiving double the per-phase exposure the sale was configured to grant to a single participant during any given phase.

Informational.

Tighten the validation in PresaleImplementation.initialize to reject adjacent phases sharing a boundary timestamp:

-            if (i > 0 && _phases[i].startTime < _phases[i - 1].endTime) {
+            if (i > 0 && _phases[i].startTime <= _phases[i - 1].endTime) {
                 revert InvalidPhaseConfiguration();
             }

This guarantees that, at any given block timestamp, at most one phase is active within PresaleImplementation.deposit.

A series of code quality issues were identified across the in-scope contracts. None carry security impact in isolation; collectively they reduce gas efficiency, off-chain observability and code clarity.

Informational.

Apply the changes itemized above. In particular:

• Remove the redundant SSTOREs identified in ProjectFeeRouterUpgradeable.sweep and NftMarketplace.setCollectionForBToken.
• Either remove the deprecated bFactory slot in PresaleImplementation or, if it must be kept for upgrade compatibility, drop the assignment in initialize.
• Drop the duplicate bFactoryParams.bToken = bToken; write in PresaleImplementation.finalizeSale and rely on createdToken for the same information.
• Delete the unused BeaconUpgraded event in PresaleFactory.
• Either drop the parameter from PresaleCancelled or replace address(this) with a value that carries information.
• Standardize naming across ProjectFeeRouterUpgradeable and NftMarketplace, pick a single verb for NftMarketplace admin setters, and rename or document createdPoolId.
• Resolve the leftover TODO in NftMarketplace, fix or remove the orphan natspec in ProjectFeeRouterUpgradeable, and add the missing __Pausable_init() call in NftMarketplace.initialize.

• ProjectFeeRouterUpgradeable abbreviates fee config and recipient identifiers to _cfg, _recips, cfg and recips. The closely related NftMarketplace uses _feeConfig and _recipients for the same concept. The protocol should standardize on a single naming convention across both contracts.
• NftMarketplace mixes set* and modify* prefixes for admin setters that perform conceptually identical operations (writing a new configuration value for a given nftCollection). Compare setSwapper, setConfig and setCollectionForBToken against modifyAuctionDuration, modifyMinAuctionPrice and modifyMaxOfferIncreaseRate. A single verb across all setters improves readability.
• ProjectFeeRouterUpgradeable.sweep declares a local variable address r = reserve[bToken]. Selecting a more descriptive variable name would improve the method's readability.

• NftMarketplace still carries the comment // TODO: verify which recipients are needed above the BTokenRecipients struct definition. The TODO should be resolved before deployment.
• ProjectFeeRouterUpgradeable carries the orphan comment /// there is a custom function to call to send funds here to the treasury directly above the Swept event declaration. The triple slash prefix makes it look like natspec for the event, but it does not document any of the event's fields and reads as a stray developer note. It should be removed or rewritten as proper natspec.
• NftMarketplace.initialize does not call __Pausable_init() despite inheriting from PausableUpgradeable. The omitted call is currently a no-op in OpenZeppelin's implementation, but skipping it leaves the contract relying on an implementation detail of the parent and breaks the convention that every parent initializer is invoked from initialize.

• PresaleFactory.BeaconUpgraded is declared but never emitted. The contract uses PresaleBeaconModified instead. The unused event definition should be removed.
• PresaleImplementation.cancelSale emits PresaleCancelled(address(this)). The single data field carries no information beyond the event emitter address, which every log already records via its address field. The event should either drop the parameter or carry a more informative payload (for example the canceller's address, or the cancellation timestamp).

• ProjectFeeRouterUpgradeable.sweep writes lastBalance[bToken] twice. The first assignment near the top of the function is unconditionally overwritten by the final assignment after all transfers. The first SSTORE is wasted on every call to sweep.
• NftMarketplace.setCollectionForBToken writes lastCheckpointTimestamp[nftCollection] = block.timestamp; directly, then immediately calls _modifyMaxOfferIncreaseRate, which calls _performCheckpoint, which writes the same slot to block.timestamp a second time. The first write is redundant.
• PresaleImplementation.initialize assigns the bFactory storage variable (bFactory = BFactory(_presaleFactory);). The source code itself comments that this is a deprecated slot kept only for storage layout compatibility, and the variable is never read by the contract. Either the assignment is unnecessary, or the deprecated slot should be replaced with an explicit reserved gap and the assignment removed.
• PresaleImplementation.finalizeSale writes bFactoryParams.bToken = bToken; immediately after writing the same value to the dedicated createdToken storage variable. Internal logic never reads bFactoryParams.bToken, and external callers can already retrieve the value via the compiler generated createdToken getter.