Palpitada

Prediction Markets

6.1 Buy-side liquidity parameter estimation uses stale liquidityParameter instead of the current dynamic b

When autoDynamicLiquidity is enabled, the buy path and sell path use different methods to determine the effective b parameter. The sell path correctly derives b from the actual pool size:

function getEffectiveLiquidityParameter() public view override returns (uint256) {
    if (autoDynamicLiquidity) {
        uint256 totalPoolSize = totalCostA + totalCostB;
        return MarketCalculations.calculateDynamicB(totalPoolSize);
    }
    return liquidityParameter;
}

The buy path, however, attempts to project the post-trade pool size to select b. It estimates the cost of the incoming purchase using the stored liquidityParameter rather than the current dynamic b:

function _getEffectiveBForPurchase(uint256 sharesToBuy, bool onSideA) internal view returns (uint256) {
    if (!autoDynamicLiquidity) {
        return liquidityParameter;
    }
    uint256 currentB = liquidityParameter > 0 ? liquidityParameter : DEFAULT_LIQUIDITY_PARAMETER;
    uint256 currentPrice = MarketCalculations.getPrice(sharesA, sharesB, onSideA, currentB);
    uint256 estimatedCost = (sharesToBuy * currentPrice);

    uint256 currentPoolSize = totalCostA + totalCostB;
    uint256 projectedPoolSize = currentPoolSize + estimatedCost;
    return MarketCalculations.calculateDynamicB(projectedPoolSize);
}

The liquidityParameter is the value set at market creation. Once the pool crosses a tier boundary (e.g., past $1,000), getEffectiveLiquidityParameter() returns a larger b (e.g., 10,000), but _getEffectiveBForPurchase still uses the original liquidityParameter (e.g., 1,000) for its price estimate.

A smaller b produces a steeper LMSR price curve, which pushes the favored side's marginal price toward 1.0. This inflates estimatedCost, which inflates projectedPoolSize, which causes calculateDynamicB to return a b from a higher tier than what the actual post-trade pool size would justify. The buy is then priced under this inflated b via calculateAmountRequiredInUSDC(...):

function calculateAmountRequiredInUSDC(uint256 sharesToBuy, bool onSideA)
    public
    view
    override
    returns (uint256, uint256, uint256, uint256)
{
    uint256 effectiveB = _getEffectiveBForPurchase(sharesToBuy, onSideA);

    uint256 initialCost = MarketCalculations.getCostFunction(sharesA, sharesB, effectiveB);
    uint256 newCost;

    if (onSideA) {
        newCost = MarketCalculations.getCostFunction(sharesA + sharesToBuy, sharesB, effectiveB);
    } else {
        newCost = MarketCalculations.getCostFunction(sharesA, sharesB + sharesToBuy, effectiveB);
    }

    uint256 cost = newCost - initialCost;
    // ...
}

A larger b means a flatter cost curve and lower price impact, so the buyer pays less for the same number of shares. After the trade, the actual pool grows by this lower real cost rather than the inflated estimate, so getEffectiveLiquidityParameter() still returns the lower-tier b. When the buyer then sells, the return is computed under this correct, smaller b, which values each share higher.

In an example scenairo, a market with liquidityParameter = 1,000 and autoDynamicLiquidity = true has grown to a pool size of 4,349 through normal 70/30 trading activity. The actual effective b from getEffectiveLiquidityParameter() is 10,000, but the stored liquidityParameter remains 1,000.

An attacker buys 6,000 shares on the favored side:

1. _getEffectiveBForPurchase uses currentB = 1,000 (stale). Under this small b, the price for the favored side is 0.985 (near 1.0). The correct price under b = 10,000 would be 0.54;
2. estimatedCost = 6,000 * 0.985 = 5,910. The correct estimate would be 6,000 * 0.54 = 3,240;
3. projectedPoolSize = 4,349 + 5,910 = 10,259, which crosses the $10K tier boundary, so the buy is priced under b = 100,000. The correct projection (4,349 + 3,240 = 7,589) would stay in the 1K-10K tier at b = 10,000;
4. Under b = 100,000, the LMSR cost curve is very flat and the attacker pays only $3,108 for 6,000 shares;
5. The actual pool grows to $7,458 (still in the $1K-$10K tier, b = 10,000);
6. The attacker sells immediately. The sell path uses the real b = 10,000 and returns $4,029 before fees;
7. After the 1% sell fee, the attacker nets $881 profit, extracted directly from other users' collateral in the pool;

Recommendation

Consider updating the logic to use the getEffectiveLiquidityParameter() in _getEffectiveBForPurchase(...)

6.2 Credit consumption in predict(...) permanently blocks selling

The predict(...) function automatically consumes any available credits from the user's CreditManager balance before processing a prediction. This consumption is greedy it applies as much credit as possible and there is no parameter or mechanism for a user to opt out.

function predict(
    bool onSideA,
    uint256 sharesToBuy,
    uint256 maxPricePerShare,
    PermitData calldata permit
) external override nonReentrant returns (uint256) {
    // ...
    uint256 creditsUsed = 0;
    uint256 userCredits = creditManager.getCreditBalance(msg.sender);

    if (userCredits > 0) {
        creditsUsed = Math.min(totalAmountRequiredInUSDC, userCredits);
    }

    uint256 usdcAmountRequired = totalAmountRequiredInUSDC - creditsUsed;
    // ...
    position.creditsUsed += creditsUsed;
    // ...
}

Once any amount of credits is recorded on a position, sellPosition(...) permanently rejects sells for that side:

function sellPosition(bool fromSideA, uint256 sharesToSell) external override nonReentrant returns (uint256) {
    // ...
    UserPosition storage position = positions[msg.sender][fromSideA];
    if (position.creditsUsed != 0) {
        revert CannotSellCreditsPosition();
    }
    // ...
}

The combination of these two behaviors creates a situation where a user who holds any credit balance no matter how small will have that credit silently consumed on their next prediction, permanently locking that entire side's position from being sold. The user's only exit becomes waiting for settlement (win or draw).

This is particularly problematic in the following scenarios:

• The protocol distributes small promotional credits (e.g., $1) to attract users. A user who then makes a large USDC-funded prediction (e.g., $50) has the $1 credit automatically applied, making the entire $50 position unsellable despite being 98% USDC-funded;
• When a market settles as a draw, withdraw(...) returns the user's previously consumed credits via creditManager.addCredit(...). If the user later participates in a new market, those restored credits are automatically consumed, once again locking their new position from selling;
• The sell restriction is binary (creditsUsed != 0) rather than proportional. There is no threshold or ratio check a single wei of credits on a position worth thousands of dollars in USDC is treated identically to a fully credit-funded position;

Recommendation

Consider scaling the sell restriction proportionally to the credit-funded portion of a position rather than applying a blanket block on any non-zero credit usage.

6.3 Pro-rata usdcSpent deduction on sells enables over-claiming draw refund

On a draw, withdraw(...) refunds users based on position.usdcSpent:

function withdraw() external override nonReentrant {
    // ...
    if (isDraw) {
        uint256 totalUsdcSpent = positions[msg.sender][true].usdcSpent + positions[msg.sender][false].usdcSpent;
        payout = totalUsdcSpent - creditsToBeReturned;
        // ...
    }
    // ...
}

When a user sells before settlement, usdcSpent is reduced pro-rata by shares:

function sellPosition(bool fromSideA, uint256 sharesToSell) external override nonReentrant returns (uint256) {
    // ...
    uint256 usdcToDeduct = (position.usdcSpent * sharesToSell) / userShares;
    position.usdcSpent -= usdcToDeduct;
    // ...
}

This deduction is based on the user's historical average cost per share, not the actual USDC the AMM returned for those shares. When the AMM returns more than the average cost (a profitable sell), usdcSpent is only partially reduced while the full sell payout leaves the contract. This leaves usdcSpent overstated for the remaining position.

Consider the following scenario:

1. Alice buys 50 shares on Side A for 25 USDC. usdcSpent = 25;
2. Bob buys 200 shares on Side A, pushing the price up;
3. Alice sells 25 shares. The AMM returns 12.72 USDC, but the pro-rata deduction is only 25 * 25/50 = 12.50. Her remaining usdcSpent = 12.50;
4. The contract paid out 12.72 but only reduced the draw liability by 12.50, creating a 0.22 USDC gap;
5. Draw is declared. Alice claims 12.50 as a draw refund;
6. Alice received 12.72 (sell) + 12.50 (draw) = 25.22 USDC total, despite depositing 25;

The 0.22 USDC shortfall falls on the last user to call withdraw(...), whose transaction reverts with InsufficientContractFunds. This is not balanced by other users' deposits because each buyer's USDC is fully spoken for by their own usdcSpent draw claim.

Recommendation

Consider tracking the cumulative deficit between AMM sell proceeds and pro-rata usdcSpent deductions in a state variable whenever the actual payout diverges from the deducted amount. This variable should be exposed via a view function so the protocol can monitor the gap and deposit USDC via before draw settlement.

6.4 Discrepancy in event date enforcement allows front-running of settlement

The Market contract contains a logic discrepancy in how the eventDate is enforced between the predict(...) and _setWinner(...) functions.

The predict(...) function only prevents users from placing new predictions when the current block.timestamp is strictly greater than the eventDate. Consequently, if the block.timestamp is exactly equal to the eventDate, a purchase is still permitted.

function predict(...) external override nonReentrant returns (uint256) {
    // ...
    // solhint-disable-next-line not-rely-on-time
    // @audit-issue Strict inequality allows purchases when block.timestamp == eventDate.
    if (block.timestamp > eventDate) {
        // ...
        revert EventClosed();
    }    // ...

Conversely, the _setWinner(...) function allows the market to be transitioned from OPEN to CLOSED and settled as soon as the block.timestamp is greater than or equal to the eventDate.

function _setWinner(bool sideAHasWon_, bool draw) internal {
    // ...
    if (marketStatus == MarketStatus.OPEN) {
        // solhint-disable-next-line not-rely-on-time
        // @audit-issue Settlement is allowed when block.timestamp == eventDate.
        if (block.timestamp >= eventDate) {
            marketStatus = MarketStatus.CLOSED;
            emit MarketStatusChanged(MarketStatus.OPEN, MarketStatus.CLOSED);
        } else {
            revert EventNotClosed();
        }
    }
    // ...
}

This creates a one-block window where a market is both open for trading and eligible for settlement. An attacker can monitor the mempool for a settlement transaction (which reveals the winner in its calldata) and front-run it with a large predict(...) transaction. Because the attacker places the trade at the exact moment the outcome is revealed or settlement is possible, they can acquire a massive portion of the winning side's shares.

Recommendation

Consider synchronizing the time boundary across both functions.

6.5 Dynamic fee in direct trades is computed from the taker's limit price instead of the execution price

The dynamic fee in _executeDirect is meant to reflect the risk profile of a trade. The parabolic fee curve topicRate * price * (1 - price) charges more for trades near 0.50 (uncertain outcomes) and less for trades near 0.00 or 1.00 (near-certain outcomes). This makes sense when the price fed into the curve represents where the trade actually executed. However, the function uses the taker's limit price rather than the execution price:

function _executeDirect(
    IOrderbookMarket market,
    Order calldata takerOrder,
    Order calldata makerOrder,
    uint256 fillAmount,
    uint256 takerDiscountBps
) internal {
    // ...
    uint256 takerPrice = (takerOrder.takerAmount * 1e18) / takerOrder.makerAmount;
    uint256 makerPrice = (makerOrder.takerAmount * 1e18) / makerOrder.makerAmount;

    if (takerOrder.side == Side.BUY) {
        uint256 executionPrice = makerPrice < takerPrice ? makerPrice : takerPrice;
        usdcAmount = (fillAmount * executionPrice) / 1e18;
    } else {
        uint256 executionPrice = makerPrice > takerPrice ? makerPrice : takerPrice;
        usdcAmount = (fillAmount * executionPrice) / 1e18;
    }

    uint256 price = _extractPrice(takerOrder);
    (uint256 takerFee, uint256 baseFeeRate) = _calculateDynamicFee(
        address(market),
        usdcAmount,
        price,
        takerDiscountBps
    );
    // ...
}

_extractPrice returns the taker's own limit ratio, not the execution price:

function _extractPrice(Order calldata order) internal pure returns (uint256) {
    if (order.makerAmount == 0) return 0;
    return (order.takerAmount * PRICE_PRECISION) / order.makerAmount; // ... 

The limit price represents the worst price the taker is willing to accept, not the price at which the trade settles. Due to price improvement, the execution price can differ from the limit. A taker who sets a limit at $0.90 might execute at $0.55 through price improvement. The fee should reflect the $0.55 execution since that is the actual risk position being taken, but instead it reflects the $0.90 limit. At $0.90, the fee curve produces topicRate * 0.90 * 0.10 = 0.09 * topicRate, whereas the correct fee at $0.55 would be topicRate * 0.55 * 0.45 = 0.2475 * topicRate.

This becomes more pronounced at extremes. A limit near $1.00 produces a fee approaching zero from the curve (topicRate * 0.99 * 0.01), while the actual execution could be anywhere. The minimum fee floor does not help because it only applies when fee > 0:

if (fee > 0 && fee < minimumFeeAmount) {
    fee = minimumFeeAmount;
}

When integer arithmetic rounds the parabolic result to exactly zero, the floor is bypassed entirely.

The result is that fees do not correspond to the actual trade that occurred. Two identical trades at the same execution price can produce different fee amounts depending on how each taker set their limit, which is not a meaningful factor in the risk of the position.

Recommendation

Consider computing the fee using the actual execution price rather than the taker's limit price.

6.6 Holding period bypass via stale purchaseTimestamp in predict(...)

The predict(...) function is the primary entry point for users to buy shares in a market. When a user makes their first prediction on a given side, the contract records the current block.timestamp in position.purchaseTimestamp. This timestamp is later used by sellPosition(...) to enforce the minHoldingPeriod a configurable delay (e.g., 1 hour) that prevents users from immediately flipping positions.

However, purchaseTimestamp is only written on the first purchase for a given side, and is never updated on subsequent buys:

function predict(
    bool onSideA,
    uint256 sharesToBuy,
    uint256 maxPricePerShare,
    PermitData calldata permit
) external override nonReentrant returns (uint256) {
    // ...
    UserPosition storage position = positions[msg.sender][onSideA];
    position.shares += sharesToBuy;
    position.usdcSpent += totalAmountRequiredInUSDC;
    position.creditsUsed += creditsUsed;
    if (position.purchaseTimestamp == 0) {
        position.purchaseTimestamp = block.timestamp;
    }
    // ...
}

The holding period check in sellPosition(...) then uses this stale timestamp:

function sellPosition(bool fromSideA, uint256 sharesToSell) external override nonReentrant returns (uint256) {
    // ...
    if (block.timestamp < position.purchaseTimestamp + sellConfig.minHoldingPeriod) {
        revert HoldingPeriodNotMet();
    }
    // ...
}

Any user can exploit this by "pre-aging" a position with a negligible dust buy (e.g., 1 share), waiting for the holding period to elapse, and then purchasing an arbitrarily large number of shares. Because the purchaseTimestamp remains anchored to the original dust buy, the user can immediately sell the newly acquired shares, completely bypassing the intended holding period.

Recommendation

Consider creating a more granular approach for the holding period such as tracking a weighted-average purchase time or maintaining per-batch timestamps.

6.7 Mint/Merge match has no on-chain collateralization enforcement

In _executeMint, the code calculates each party's payment (yesPay, noPay) based on their order prices. If totalPay > fillAmount, surplus is returned to the taker as price improvement. However, if totalPay < fillAmount, the underpayment is silently accepted. The market's mint() function then records totalUsdcReserve += fillAmount (not += totalPay), inflating the reserve beyond actual USDC held:

uint256 totalPay = yesPay + noPay + drawPay;
if (totalPay > fillAmount) {
    uint256 surplus = totalPay - fillAmount;
    // ... reduce taker's payment
}
// No else branch for totalPay < fillAmount

IERC20(usdc).safeTransferFrom(yesRecipient, address(market), yesPay);
IERC20(usdc).safeTransferFrom(noRecipient, address(market), noPay);

market.mint(yesRecipient, noRecipient, drawRecipient, fillAmount);

As an example, Alice BUY 100 YES at $0.30, Bob BUY 100 NO at $0.30. totalPay = 60 USDC, fillAmount = 100. The market receives 60 USDC but records totalUsdcReserve = 100. The 40 USDC shortfall means winners cannot fully redeem.

The same issue exists in _executeMerge in the opposite direction. If both SELL orders demand prices that sum above $1.00 per token set, totalProceeds > fillAmount is not checked:

uint256 totalProceeds = yesProceeds + noProceeds + drawProceeds;
if (totalProceeds < fillAmount) {
    uint256 surplus = fillAmount - totalProceeds;
    // ... increase taker's proceeds
}
// No else branch for totalProceeds > fillAmount

market.burn(yesHolder, noHolder, drawHolder, fillAmount, yesProceeds, noProceeds, drawProceeds, takerFee);

As an example, Alice SELL 100 YES at $0.70, Bob SELL 100 NO at $0.70. totalProceeds = 140 USDC, fillAmount = 100. The market's burn decrements totalUsdcReserve by 100 but transfers 140 USDC out, draining 40 USDC more than the token set is worth.

Both cases lead to the same outcome, the market's actual USDC balance diverges from totalUsdcReserve. A compromised or buggy operator can create undercollateralized markets through mint or drain existing collateral through merge, leaving token holders facing insolvency at redemption.

Recommendation

Consider adding a check to prevent this scenario.

6.8 OrderbookMarket emergency withdrawal does not update market status

_emergencyWithdraw drains all USDC from the market and resets totalUsdcReserve to zero, but does not update status or resolution:

function _emergencyWithdraw(address recipient) internal {
    if (recipient == address(0)) revert ZeroAddress();

    uint256 balance = IERC20(usdc).balanceOf(address(this));
    if (balance == 0) revert ZeroAmount();

    // Reset total reserve
    totalUsdcReserve = 0;

    // Transfer all USDC to recipient
    IERC20(usdc).safeTransfer(recipient, balance);

    emit EmergencyWithdrawal(recipient, balance);
}

Since status remains ACTIVE, user-facing operations continue to function on a market with no backing. When splitMergeRestricted is false, any user can call split() directly to deposit USDC into the drained market. Once fresh USDC enters the contract, old token holders (who received tokens before the drain) can call merge() to extract it. For example, if User B deposits 100 USDC via split(), User A who held 100 YES + 100 NO tokens from before the drain can immediately merge() to claim that 100 USDC. User B is then unable to fully merge their own tokens because the contract no longer holds enough USDC.

Additionally, outcome tokens are standard ERC20 contracts with unrestricted transfers. Old token holders can sell or transfer their unbacked tokens freely to other users who have no on-chain indication that the market's collateral was drained.

Recommendation

Consider setting the status to a terminal state inside _emergencyWithdraw.

6.9 Ternary draw order can be executed against a different market than the one the maker signed

Every execution path in the exchange correctly binds signed orders to their intended market except the ternary draw order path. For binary and 2-order matches, fillOrders(...) enforces that both parties signed for the same market:

if (takerOrder.market != makerOrder.market) revert MarketMismatch();

IOrderbookMarket market = IOrderbookMarket(takerOrder.market);

The single-user operations (executeSplit, executeMerge, executeRedeem) are inherently safe because they use the signed order's own market field for both validation and execution, there is no second order whose market could diverge.

The ternary draw order path diverges from both patterns. It introduces a third independently-signed order, but unlike the taker/maker pair, the draw order's market is never compared against the execution market:

if (hasDrawOrder) {
    if (!OrderbookMarket(address(market)).hasDraw()) revert NotTernaryMarket();

    drawHash = _validateOrder(drawOrder, address(0), drawSignature);
    _updateOrderFilled(drawHash, drawOrder.makerAmount, fillAmount);

    if (takerOrder.tokenId != 0 || makerOrder.tokenId != 1 || drawOrder.tokenId != 2) {
        revert InvalidTokenIds();
    }

    if (takerOrder.side != makerOrder.side || takerOrder.side != drawOrder.side) {
        revert SideMismatch();
    }
}

_validateOrder(...) verifies the draw order's EIP-712 signature against drawOrder.market (confirming the signer authorized that specific market) and checks that drawOrder.market is registered and active. However, these are isolated validations. The draw order passes all its own checks, but is never anchored to the execution context established by the taker/maker pair.

The subsequent execution in _executeMint(...) uses the market variable (derived from takerOrder.market) for all fund transfers and token minting, including the draw maker's:

if (hasDrawOrder && drawPay > 0) {
    IERC20(usdc).safeTransferFrom(drawRecipient, address(market), drawPay);
}

market.mint(yesRecipient, noRecipient, drawRecipient, fillAmount);

Because USDC is fungible, the safeTransferFrom succeeds regardless of which market the draw maker originally intended. The draw maker's USDC is transferred to takerOrder.market (Market B) and DRAW tokens are minted for them in Market B, even though their EIP-712 signature specifically authorized drawOrder.market (Market A).

Recommendation

Consider adding a check such as if (drawOrder.market != takerOrder.market) revert MarketMismatch(); inside the hasDrawOrder block.

6.10 Cancelled delayed operations can be re-queued by anyone using on-chain signatures

When a delayed operation is first submitted via executeWithMultisig(...), the caller provides the required signer signatures as calldata. These signatures become permanently visible on-chain in the transaction's input data. If the signers later cancel the operation, the cancellation clears the queue state but does not invalidate the signatures:

function _cancelOperation(Operation memory operation) internal {
    bytes32 operationHash = _hashOperation(operation);
    if (!operationQueued[operationHash]) revert OperationNotQueued();

    delete pendingOperations[operationHash];
    delete operationQueued[operationHash];
    emit OperationCancelled(operationHash);
}

After cancellation, the original operation remains valid because:

1. The target nonce has not advanced. The cancel operation increments nonces[address(this)] (the self-target nonce), but the market/target nonce is unchanged;
2. The operation is not marked in executedOperations;
3. The original signatures still recover to valid signers for the unchanged operation hash;

Any observer can extract the signatures from the original queuing transaction's calldata and call executeWithMultisig(operation, signatures) again, re-queuing the cancelled operation with a fresh delay timer. The signers must cancel again, and this cycle can repeat indefinitely. Each cancellation requires coordinating threshold signatures for a new cancel operation (consuming a self-target nonce), while re-queuing costs only a single transaction using the already-known signatures.

The only way to permanently stop the re-queuing is to execute a different operation for the same target, which advances the target nonce and invalidates the old operation.

Recommendation

Consider incrementing the target nonce when an operation is cancelled, or marking the cancelled operation hash in executedOperations so it cannot be resubmitted.

6.11 Cancelled orderbook market can be subsequently resolved

_cancel() sets status = CANCELLED but does not set resolution.resolved = true. A subsequent _setWinner() call only checks resolution.resolved, not the status. This allows a cancelled market to be resolved, overwriting the cancellation with status = RESOLVED.

In the Orderbook system, 1 YES token + 1 NO token together represent $1.00 of collateral held in the reserve. The payout per token depends entirely on market state at redemption time:

| Market state | Winning token payout | Losing token payout | | CANCELLED | $0.50 (50% of face value) | $0.50 (50% of face value) | | RESOLVED | $1.00 (100% of face value) | $0.00 |

The cancellation path in redeemFor() explicitly applies a 50% rate:

uint256 yesUsdc = (yesAmount * 5000) / 10000;  // 50%
uint256 noUsdc  = (noAmount  * 5000) / 10000;  // 50%

The resolution path pays 1 token = 1 USDC to the winning side only.

An example vulnerable sequence is as follows:

1. Multisig calls _cancel(). Status becomes CANCELLED, but resolution.resolved stays false;
2. Alice sees CANCELLED and redeems immediately. She holds 100 YES tokens and receives 50 USDC ($0.50 x 100);
3. Bob holds 100 YES tokens and waits;
4. Multisig calls _setWinner(). Because resolution.resolved is still false, the guard at line 440 passes. The market is now RESOLVED with YES as winner, overwriting the cancelled state;
5. Bob calls redeemFor(). He holds 100 YES tokens and receives 100 USDC ($1.00 x 100);

Resulting inequality is shown below:

| User | Tokens | Redeemed when | Received | Per token | | Alice | 100 YES | During CANCELLED | 50 USDC | $0.50 | | Bob | 100 YES | After RESOLVED | 100 USDC | $1.00 |

Same tokens, same market, same side. Alice receives half of what Bob receives purely because she acted in good faith on the state the contract publicly advertised. The bug lets the protocol retroactively change the payout rules after users have already redeemed under the prior state.

Recommendation

Consider adding if (status == OrderbookMarketStatus.CANCELLED) revert MarketCancelled(); in _setWinner().

6.12 Direct trades do not enforce that limit prices cross before execution

In _executeDirect, the contract computes an execution price using a price improvement rule (min for buy taker, max for sell taker) but never verifies that the two orders' limit prices are compatible:

function _executeDirect(
    IOrderbookMarket market,
    Order calldata takerOrder,
    Order calldata makerOrder,
    uint256 fillAmount,
    uint256 takerDiscountBps
) internal {
    // ...
    uint256 takerPrice = (takerOrder.takerAmount * 1e18) / takerOrder.makerAmount;
    uint256 makerPrice = (makerOrder.takerAmount * 1e18) / makerOrder.makerAmount;

    if (takerOrder.side == Side.BUY) {
        // ...
        uint256 executionPrice = makerPrice < takerPrice ? makerPrice : takerPrice;
        usdcAmount = (fillAmount * executionPrice) / 1e18;
    } else {
        // ...
        uint256 executionPrice = makerPrice > takerPrice ? makerPrice : takerPrice;
        usdcAmount = (fillAmount * executionPrice) / 1e18;
    }

    IERC20(usdc).safeTransferFrom(buyer, seller, usdcAmount);
    market.transferFrom(seller, buyer, fillAmount, tokenId);
}

For a buy taker, the execution price is min(makerPrice, takerPrice). If the buyer's limit is $0.40 and the seller's limit is $0.60, the orders do not cross (the buyer won't pay enough to meet the seller's minimum). However, the code does not reject this match. It sets executionPrice = $0.40 and executes, transferring $0.40 from the buyer and forcibly moving the seller's tokens via market.transferFrom. The seller receives less than the $0.60 minimum they signed for.

For a sell taker, the reverse occurs: executionPrice = max(makerPrice, takerPrice) can force the buyer to pay above their signed limit.

Signature validation confirms that both parties signed orders with the given fields (market, amounts, token, side), but the contract never checks that the two orders' prices are mutually satisfiable. The operator is trusted to only match crossing orders, but the on-chain contract provides no enforcement.

Recommendation

Consider adding a crossing price check in _executeDirect before computing the execution price.

6.13 Emergency withdrawal does not update market status or prevent subsequent settlement

The _emergencyWithdraw function drains all USDC to the owner and resets global pool accounting, but does not change marketStatus or set any flag to prevent the market from being settled afterward:

function _emergencyWithdraw() internal {
    if (block.timestamp < eventDate + EMERGENCY_TIMELOCK) {
        revert EmergencyTimelockNotMet();
    }

    uint256 balance = IERC20(usdcToken).balanceOf(address(this));
    IERC20(usdcToken).safeTransfer(owner(), balance);

    predictionsPoolBalance = 0;
    protocolFeesBalance = 0;
    curatorFeesBalance = 0;
    totalCostA = 0;
    totalCostB = 0;
    creditsUsedSideA = 0;
    creditsUsedSideB = 0;
    totalCreditsUsed = 0;

    emit EmergencyWithdrawal(owner(), balance);
}

After this function executes, the contract has zero USDC and zeroed global balances, but marketStatus remains unchanged (e.g., CLOSED), settled is still false, and all per-user state (positions[].usdcSpent, balancesA/B, balanceCreditsAmount) is preserved. This allows the multisig to subsequently call _setWinner, which sets settled = true and assigns an outcome.

Since settled remains false, the multisig can still call _setWinner afterward. If the market is settled with a winner, predictionsPoolBalance is zero, so all payouts compute to zero and revert with NoPayoutAvailable. The winner path has no credit restoration logic, so credits consumed by winning users are permanently lost.

If the market is settled as a draw, withdraw() computes payout = totalUsdcSpent - creditsToBeReturned from the preserved per-user state. This creates an asymmetry in how different user types are treated:

• Pure-credit users (where payout == 0) can successfully call withdraw(). The function restores their credits via creditManager.addCredit(...) and returns early before reaching the USDC transfer. These users are made whole;
• Mixed users (USDC + credits) and pure-USDC users have payout > 0, but the contract holds zero USDC. The transaction reverts with InsufficientContractFunds, and the revert rolls back any credit restoration that occurred earlier in the same call. These users lose both their USDC and their credits;

The result is that only pure-credit users can recover from an emergency-withdrawn market that is later settled as a draw, while all other users are locked out entirely. The credits restored to pure-credit users come at no cost to the owner, who already drained the contract's full USDC balance. The function also zeroes totalCreditsUsed, creditsUsedSideA, and creditsUsedSideB but does not interact with CreditManager to restore per-user credit balances. For non-pure-credit users, the only recovery path is for the CreditManager owner or multisig to manually call addCredit for each affected user using their balanceCreditsAmount values from the market contract.

Recommendation

Consider settling a market within _emergencyWithdraw to make the emergency state terminal and prevent subsequent settlement.

6.14 LMSR math double-scales PRBMath exp() outputs, causing premature overflow in pricing and cost calculations

getPrice() and getCostFunction() unwrap PRBMath exp() results into raw uint256 values (already scaled by 1e18) and then multiply by DECIMALS (1e18) again in intermediate steps:

function getPrice(...) public pure returns (uint256) {
    // ...
    uint256 expA = unwrap(udA.exp()); // already 1e18-scaled
    uint256 expB = unwrap(udB.exp());

    if (onSideA) {
        return (expA * DECIMALS) / (expA + expB); // expA * 1e18 overflows
    } else {
        return (expB * DECIMALS) / (expA + expB);
    }
}

function getCostFunction(...) external pure returns (uint256) {
    // ...
    uint256 expA = unwrap(udA.exp());
    uint256 expB = unwrap(udB.exp());

    UD60x18 udSum = ud((expA + expB) * DECIMALS); // (sum) * 1e18 overflows
    uint256 _logln = unwrap(udSum.ln());
    return (b * _logln);
}

The intermediate expA * 1e18 overflows uint256 when expA > 1.15e59, which occurs at q/b > 94.5. PRBMath's exp() supports inputs up to 133.08, so there is a window between 94.5 and 133.08 where exp() succeeds but the subsequent multiplication overflows and reverts. With a manual b of 1,000, this threshold is reached at roughly q = 94,500 (approximately $94.5K invested on one side). Once triggered, getPrice(), getCostFunction(), and all functions that depend on them (predict(), sellPosition(), getSellReturn(), getOdds()) revert, freezing the market until an admin increases b.

In getPrice, the * DECIMALS is necessary to preserve precision in the division but can be replaced with PRBMath's div() which uses mulDiv with 512-bit intermediate arithmetic. In getCostFunction, the * DECIMALS is mathematically unnecessary — it adds a constant ln(1e18) to every evaluation — but is harmless in practice because LMSR only uses cost *differences* where the constant cancels. Removing it both fixes the overflow and corrects the formula.

Recommendation

In getPrice, replace the raw multiplication with PRBMath's overflow-safe div: return unwrap(ud(expA).div(ud(expA + expB))). In getCostFunction, remove the extra scaling: ud(expA + expB) instead of ud((expA + expB) * DECIMALS). Both changes produce identical pricing results and eliminate the overflow window entirely.

6.15 No signer rotation or removal mechanism after one-time multisig configuration

The configureMultisig(...) function enforces a one-time configuration per target through the targetConfigured flag:

function configureMultisig(
    address target,
    address[] memory _signers,
    uint256 _threshold,
    FunctionConfig[] memory _functionConfigs
) external override onlyOwner {
    // ...
    if (targetConfigured[target]) {
        revert TargetAlreadyConfigured();
    }
    // ...
    config.signers.push(_signers[i]);
    config.isSigner[_signers[i]] = true;
    // ...
    targetConfigured[target] = true;
}

Once configured, there is no function to add, remove, or replace signers, and no function to update the threshold. If a signer's private key is compromised, that key retains full signing authority for the lifetime of the MultisigController instance.

This means a compromised signer cannot be removed from the active signer set, and no target contract can be migrated to a new multisig configuration. A compromised signer retains permanent ability to contribute one valid signature toward any multisig operation.

Recommendation

Consider adding a multisig-governed signer rotation mechanism that allows the existing signer set to vote on adding or removing signers. Alternatively, consider adding a setMultisigController(...) function on target contracts (protected by the existing multisig) to allow migration to a new MultisigController instance with an updated signer set.

6.16 Sell-generated protocol and curator fees are locked when a market resolves as a draw

During active trading, sells generate fees that are split: 50% to protocolFeesBalance and 50% recycled into predictionsPoolBalance:

uint256 protocolShare = fees / 2;
uint256 poolShare = fees - protocolShare;
protocolFeesBalance += protocolShare;
predictionsPoolBalance += poolShare;

When a market resolves as a draw, both _withdrawProtocolFees and withdrawCuratorFees revert:

function _withdrawProtocolFees() internal {
    if (!settled || isDraw) {
        revert EventNotFinishedOrDraw();
    }
    // ...
}

The comment on withdrawCuratorFees explains: "On draw, users get full refunds including fees, so curator cannot withdraw." This is correct for buy-side fees, users' usdcSpent includes the fees they paid on purchase, and draw refunds return usdcSpent. However, sell fees are collected from entirely separate transactions where the seller voluntarily exited their position and already received a reduced payout (after fees). These fees were earned from real completed trades and have no corresponding draw refund claimant.

The sell-fee portion of protocolFeesBalance remains locked in the contract with no withdrawal path. The only recovery mechanism is _emergencyWithdraw, which drains all funds indiscriminately and requires a 7-day timelock past the event date.

Recommendation

Consider tracking sell-generated protocol fees separately from buy-generated protocol fees. On draw, allow withdrawal of sell-generated fees only after verifying the contract's USDC balance still covers all outstanding draw obligations. This ensures the fees are only released when they are genuine surplus rather than needed to partially offset the draw-refund-overclaim deficit.

6.17 Stepwise calculateDynamicB creates price discontinuities at tier boundaries

The calculateDynamicB function determines the LMSR liquidity parameter b using a hardcoded lookup table of discrete tiers:

function calculateDynamicB(uint256 totalPoolSize) external pure returns (uint256) {
    if (totalPoolSize >= POOL_TIER_9) {
        return 1_000_000_000; // $100M+ -> b = 1B
    } else if (totalPoolSize >= POOL_TIER_8) {
        return 500_000_000;   // $50M+  -> b = 500M
    } else if (totalPoolSize >= POOL_TIER_7) {
        return 100_000_000;   // $10M+  -> b = 100M
    // ... more tiers ...
    } else if (totalPoolSize >= POOL_TIER_1) {
        return 100_000;       // $10K+  -> b = 100K
    } else if (totalPoolSize >= POOL_TIER_TINY) {
        return 10_000;        // $1K+   -> b = 10K
    } else if (totalPoolSize >= POOL_TIER_MICRO) {
        return 1_000;         // $100+  -> b = 1K
    } else {
        return 1_000;         // below $100
    }
}

At boundary crossings, b jumps by up to 10x (e.g., 999 to1,001 changes b from 1,000 to 10,000). Since b controls the curvature of the entire LMSR cost function, these discrete jumps break LMSR's fundamental property of continuous pricing. A pool at $9,999 operates under b = 10,000, but a single $2 trade that pushes it to $10,001 switches the entire market to b = 100,000. Under the new b, prices flatten dramatically and the behavior of the market changes abruptly.

Users whose transactions land on different sides of a boundary experience fundamentally different pricing for similar positions. The abrupt 10x changes in market curvature at the most common pool sizes ($1K and $10K boundaries) make market behavior unpredictable.

Recommendation

Consider replacing the stepwise tier lookup with a continuous function for b calculations.

6.18 Switching between manual and dynamic liquidity modes on a live market invalidates pool accounting

The multisig can toggle _setAutoDynamicLiquidity(...) at any time, including while the market is OPEN with active positions:

function _setAutoDynamicLiquidity(bool enabled) internal {
    autoDynamicLiquidity = enabled;
    emit AutoDynamicLiquidityChanged(enabled);
}

There is no restriction on market status. When a market uses a manually set liquidityParameter, gradual updates via _setLiquidityParameter(...) are expected to keep the market healthy as the pool grows. However, toggling from manual to dynamic mode (or vice versa) creates a discontinuity: the effective b jumps from the stored liquidityParameter to calculateDynamicB(totalCostA + totalCostB), which can be an order-of-magnitude change if the pool has grown beyond the initial parameter's tier.

All existing accounting variables (totalCostA, totalCostB, predictionsPoolBalance) reflect LMSR costs accumulated under the previous b values. After a large discontinuity in effective b, subsequent sell operations compute state-dependent LMSR cost differences under the new b, which can diverge significantly from the tracked values. If a sell return exceeds the tracked totalCostA, the clamping logic (else totalCostA = 0;) silently absorbs the excess, leaving totalCostA at zero while shares remain outstanding.

Recommendation

Consider preventing switching between manual and dynamic liquidity modes on markets that have already received predictions. More broadly, consider whether the manual liquidity parameter system is necessary given that the dynamic system exists.

6.19 Ternary market mint/merge operations hardcode all fees and price improvement to the YES position

In ternary (three-outcome) markets, fillOrders enforces a fixed token ID ordering where the taker must always be the YES (tokenId 0) position:

if (takerOrder.tokenId != 0 || makerOrder.tokenId != 1 || drawOrder.tokenId != 2) {
    revert InvalidTokenIds();
}

Because the YES order is always designated as the taker, all fee charges and price improvement logic are hardcoded to the YES position. The condition takerOrder.tokenId == 0 || hasDrawOrder is always true when hasDrawOrder is true, causing:

• Mint surplus is always deducted from yesPay, giving price improvement exclusively to the YES buyer;
• Merge surplus is always added to yesProceeds giving excess proceeds exclusively to the YES seller;
• Merge fees are always deducted from yesProceeds, charging the fee exclusively to the YES seller;
• Fee rates are always computed from the YES order's price via _extractPrice(takerOrder);

In binary markets, the operator can alternate which order is designated as taker, distributing the fee burden fairly. In ternary markets, the protocol has no such flexibility due to the hardcoded token ID ordering.

Recommendation

Consider splitting fees proportionally across all three parties in ternary mint/merge operations based on each party's contribution, rather than concentrating the entire fee burden on the YES position. Alternatively, consider allowing the operator to specify which of the three orders is the taker rather than hardcoding the token ID ordering.

6.20 sellPosition lacks slippage protection for sellers

The predict function provides buy-side slippage protection through the maxPricePerShare parameter. If the average cost per share exceeds the caller's limit, the transaction reverts:

function predict(
    bool onSideA,
    uint256 sharesToBuy,
    uint256 maxPricePerShare,
    PermitData calldata permit
) external override nonReentrant returns (uint256) {
    // ...
    if (maxPricePerShare > 0) {
        uint256 averagePrice = cost / sharesToBuy;
        if (averagePrice > maxPricePerShare) {
            revert SlippageExceeded(maxPricePerShare, averagePrice);
        }
    }
    // ...
}

The sellPosition function has no equivalent protection. It accepts only the side and share count, computes the return internally, and transfers whatever amount results:

function sellPosition(bool fromSideA, uint256 sharesToSell) external override nonReentrant returns (uint256) {
    // ...
    uint256 amountAfterFeesInternal = _processSell(fromSideA, sharesToSell);

    uint256 amountAfterFees = amountAfterFeesInternal / DECIMAL_MULTIPLIER;
    // ...
    IERC20(usdcToken).safeTransfer(msg.sender, amountAfterFees);

    return amountAfterFees;
}

A seller who previews their return via getSellReturn() sees a value computed from the market state at that moment. Between the preview and execution, other trades can change sharesA, sharesB, totalCostA, and totalCostB, which alter the LMSR cost curve and the effective b when autoDynamicLiquidity is enabled. The actual return at execution time can be lower than expected, and the seller has no on-chain mechanism to reject the trade.

Recommendation

Add a minReturnAmount parameter to sellPosition that reverts if the computed amountAfterFees falls below the caller's specified minimum.

6.21 setMultisigController in CreditManager does not emit an event

The setMultisigController function sets the multisigController address, a critical security parameter that controls which address can execute privileged operations. However, it emits no event:

function setMultisigController(address _multisigController) external override onlyOwner {
    if (multisigController != address(0)) revert AlreadyInitialized();
    if (_multisigController == address(0)) revert ZeroAddress();
    multisigController = _multisigController;
}

Every other configuration function in CreditManager emits an event. The multisig controller configuration is the most privileged operation on the contract and is the only one invisible to event-based monitoring.

Recommendation

Consider defining and emitting a MultisigControllerSet event.

6.22 18-to-6 decimal truncation in sellPosition can burn shares for zero USDC

sellPosition() processes the sell and updates all state before converting the internal 18-decimal result to 6-decimal USDC via floor division. _processSell() reduces sharesA/B, balancesA/B, totalCostA/B, and predictionsPoolBalance, then the result is floored:

uint256 amountAfterFeesInternal = _processSell(fromSideA, sharesToSell);
uint256 amountAfterFees = amountAfterFeesInternal / DECIMAL_MULTIPLIER;
// ...
IERC20(usdcToken).safeTransfer(msg.sender, amountAfterFees);

If amountAfterFeesInternal < 1e12, the user's shares are burned and pool accounting is updated, but the transfer sends 0 USDC. There is no zero-check, the transaction succeeds silently.

This is reachable when the liquidity parameter b is very large relative to the position size. With b = MAX_LIQUIDITY_PARAMETER (1e9), selling 1 share returns approximately 1e9 internally — well below the 1e12 threshold. A more natural path: a user buys a small position while b is low, then dynamic b scales up as the pool grows, reducing the per-share cost difference under the new b.

Recommendation

Consider adding a check such as if (amountAfterFees == 0) revert ZeroPayout(); after the 18-to-6 conversion, before the transfer.

6.23 Curator fees are not subject to token-based fee discounts

In _getPredictionCosts, the protocol fee applies the user's discount from governance token holdings, while the curator fee does not:

if (feesEnabled) {
    uint256 discountBps = getFeeDiscountBasisPoints(msg.sender);
    _protocolFee = (cost * protocolFee * (10000 - discountBps)) / 1e8;

    if (curator != address(0) && curatorFee > 0) {
        curatorFeeAmount = (cost * curatorFee) / 10000;
    }
}

The curator fee is calculated without any discount regardless of the user's token holdings. This means users holding governance/discount tokens receive reduced protocol fees but always pay the full curator fee. This is an asymmetry that could be unexpected to users who assume "fee discount" applies to total fees.

Recommendation

Consider documenting this behavior clearly in the UI and contract documentation. If curator fee discounts are desired, apply the same discountBps logic to curatorFeeAmount.

Resolution

By design.

6.24 EIP-2612 permit flow in predict(...) is broken for users with credit balances

The predict(...) function supports EIP-2612 permits, allowing users to approve and transfer USDC in a single transaction. However, the permit is called with usdcAmountRequired the post-credit-deduction amount which the user cannot know at signing time.

The credit deduction happens internally inside predict(...) after the user has already signed the permit:

function predict(
    bool onSideA,
    uint256 sharesToBuy,
    uint256 maxPricePerShare,
    PermitData calldata permit
) external override nonReentrant returns (uint256) {
    // ...
    (
        uint256 cost,
        uint256 actualProtocolFee,
        uint256 totalAmountRequiredInUSDC,
        uint256 curatorFeeAmount
    ) = calculateAmountRequiredInUSDC(sharesToBuy, onSideA);
    // ...
    uint256 creditsUsed = 0;
    uint256 userCredits = creditManager.getCreditBalance(msg.sender);

    if (userCredits > 0) {
        creditsUsed = Math.min(totalAmountRequiredInUSDC, userCredits);
    }

    uint256 usdcAmountRequired = totalAmountRequiredInUSDC - creditsUsed;
    // ...
    if (usdcAmountRequired > 0) {
        if (permit.deadline > 0) {
            IERC20Permit(usdcToken).permit(
                msg.sender,
                address(this),
                usdcAmountRequired,
                permit.deadline,
                permit.v,
                permit.r,
                permit.s
            );
        }

        IERC20(usdcToken).safeTransferFrom(msg.sender, address(this), usdcAmountRequired);
    }
    // ...
}

The user's only available information before calling predict(...) is totalAmountRequiredInUSDC returned by calculateAmountRequiredInUSDC(...). They sign a permit for that full amount. Inside predict(...), the contract deducts credits and calls permit() with the reduced usdcAmountRequired. Because the signed value does not match the value passed to permit(), the call reverts with ERC2612InvalidSigner.

The user's credit balance can also change between signing and execution for example, if another market consumes credits in an earlier transaction within the same block, or if the protocol adds credits between the user signing and the transaction being mined making the post-credit amount fundamentally unpredictable at signing time.

As a result, any user with a non-zero credit balance is forced to fall back to a separate approve() transaction with permit.deadline = 0, which costs extra gas and defeats the purpose of the permit flow.

Recommendation

Consider signing and calling the permit for the full totalAmountRequiredInUSDC amount, or providing a way for users to query the exact post-credit USDC amount before signing.

6.25 Fee discount can be obtained via flash-loaned tokens

The fee discount mechanism checks a user's token balance at the moment of the call via IERC20(token).balanceOf(user):

function getFeeDiscountBasisPoints(
    address user,
    mapping(address => TokenDiscountConfig) storage tokenConfigs,
    address[] memory discountTokens
) external view returns (uint256) {
    // ...
    for (uint256 i = 0; i < discountTokens.length; i++) {
        // ...
        try IERC20(token).balanceOf(user) returns (uint256 balance) {
            tokenBalance = balance;
        } catch {
            continue;
        }
        // ...
    }
    return maxDiscount;
}

This balance check is a point-in-time snapshot. A user can borrow discount tokens via a flash loan, call predict(...) or sellPosition(...) through an intermediary contract to receive the reduced fee, and return the tokens in the same transaction.

The discount applies to both buy-side protocol fees (in calculateAmountRequiredInUSDC) and sell-side dynamic fees (in _calculateSellFees), meaning both entry and exit costs can be reduced this way.

This allows any user to obtain the maximum configured fee discount without actually holding the required tokens, undermining the intended incentive for long-term token holders.

Recommendation

Consider implementing a time-weighted average balance check, a snapshot-based mechanism, or requiring that discount tokens be staked/locked for a minimum period before the discount applies.

Resolution

We consider the practical risk negligible

6.26 Hardcoded magic numbers used throughout the codebase

Multiple contracts across the codebase use hardcoded numeric literals directly in logic rather than referencing named constants. The most common categories are:

• Basis point denominators (e.g., 10000) and fee/ratio thresholds (e.g., 2000, 3000, 5000) used in conditionals and arithmetic in Market.sol, MarketCalculations.sol, OrderbookMarket.sol, and OrderbookFactory.sol;
• Time durations (e.g., 3 days, 7 days, 4 days) used inline for sell lockouts, holding period caps, and fee multiplier brackets in Market.sol and MarketCalculations.sol;
• Fee multiplier values (e.g., 50, 75, 100, 125, 150, 200, 300) for dynamic sell fee calculation in MarketCalculations.sol;
• Function selectors written as raw hex literals (e.g., 0x1aa48bde, 0x1956e2ae) for multisig dispatch routing in Market.sol, MultisigController.sol, OrderbookExchange.sol, OrderbookMarket.sol, and CreditManager.sol;
• Calculation divisors (e.g., 1e8, 1e14, 1e18) embedded directly in fee and price formulas in Market.sol, MarketCalculations.sol, and OrderbookExchange.sol;
• Cancellation refund splits (e.g., 3333, 3334, 5000) used for ternary outcome distributions in OrderbookMarket.sol;

Using named constants improves readability, reduces the risk of inconsistent updates, and makes the intended semantics of each value explicit.

Recommendation

Consider extracting all hardcoded numeric literals into named constants with descriptive names (e.g., BASIS_POINTS, MAX_SELL_FEE_BPS, SELL_LOCKOUT_BEFORE_EVENT, HOLDING_MULTIPLIER_7D). For function selectors, consider computing them from the function signature string (e.g., bytes4(keccak256("setWinner(bool,bool)"))) or referencing interface definitions to prevent silent breakage if function signatures change.

6.27 Implementation contracts can be initialized by anyone

The protocol deploys implementation contracts as templates for EIP-1167 clones. These implementation contracts use empty constructors and an initialized boolean to guard their initialize function:

// Market.sol
constructor() Ownable(msg.sender) {}

function initialize(...) external override {
    if (initialized) {
        revert AlreadyInitialized();
    }
    // ...
}

// OrderbookMarket.sol
constructor() {}

function initialize(...) external override {
    if (initialized) revert AlreadyInitialized();
    // ...
}

// OrderbookExchange.sol
constructor() EIP712("PalpitadaOrderbookExchange", "1") {}

function initialize(...) external {
    if (initialized) revert AlreadyInitialized();
    // ...
}

None of these constructors set initialized = true, so initialize() remains callable on the implementation contract itself. Since initialized defaults to false, anyone can call initialize() on each implementation with arbitrary parameters, setting themselves as owner, configuring a controlled multisig, and pointing to arbitrary token addresses.

This does not affect clones, as each clone has independent storage. However, the implementation addresses are publicly visible on-chain and could be used in social engineering scenarios where a user is tricked into interacting with what appears to be a legitimate contract at a known protocol address.

Recommendation

Consider setting initialized = true in each implementation's constructor.

6.28 Inconsistent msg.sender vs _msgSender() usage in CreditManager's onlyAuthorized modifier

The onlyAuthorized modifier mixes msg.sender and _msgSender() in the same conditional:

modifier onlyAuthorized() {
    if (!isAuthorizedConsumer[msg.sender] && owner() != _msgSender()) {
        revert NotAuthorized();
    }
    _;
}

The onlyMarketFactory modifier consistently uses _msgSender():

modifier onlyMarketFactory() {
    if (_msgSender() != marketFactory) {
        revert NotAuthorized();
    }
    _;
}

In the current deployment, msg.sender and _msgSender() return the same value because no meta-transaction forwarder is configured. However, if ERC2771Context or a similar forwarder is added in the future, msg.sender (the forwarder) and _msgSender() (the original signer) would diverge, causing the authorized consumer check to use the wrong address.

Recommendation

Consider using _msgSender() consistently in both conditions within the onlyAuthorized modifier to avoid inconsistent behavior if meta-transaction support is introduced.

6.29 Inconsistent validation for protocolFee between initialize(...) and _setProtocolFee(...)

During initialization, protocolFee is allowed to be zero:

function initialize(...) external override {
    // ...
    if (_protocolFee >= 10000) {
        revert InvalidFees();
    }
    // ...
}

However, _setProtocolFee(...) (called via multisig to update the fee after deployment) explicitly rejects zero:

function _setProtocolFee(uint256 _protocolFee) internal {
    if (_protocolFee == 0) {
        revert ZeroValue();
    }
    if (_protocolFee >= 10000) {
        revert InvalidFees();
    }
    // ...
}

A market initialized with protocolFee = 0 cannot be restored to that state through _setProtocolFee(...). Conversely, if the intent is that zero fees should never be allowed, the check is missing from initialize(...).

Recommendation

Consider aligning the validation between both functions.

6.30 Market hard-codes a 6-decimal token assumption via DECIMAL_MULTIPLIER

The contract uses a fixed DECIMAL_MULTIPLIER = 1e12 to convert between 18-decimal internal accounting and the payment token's decimals, assuming the token always has 6 decimals. This constant is used throughout the protocol for charging users, computing payouts, and converting pool balances:

uint256 public constant DECIMAL_MULTIPLIER = 1e12;

// Charging (round up):
uint256 totalAmountRequiredInUSDC = (totalAmountRequired + DECIMAL_MULTIPLIER - 1) / DECIMAL_MULTIPLIER;

// Paying out (truncate):
uint256 amountAfterFees = amountAfterFeesInternal / DECIMAL_MULTIPLIER;

If a token with different decimals is configured, internal accounting and actual token transfers would diverge silently. Since initialize() accepts any _usdcToken address without validating its decimals, there is no on-chain safeguard against misconfiguration.

Recommendation

If the protocol plans to support payment tokens beyond 6-decimal USDC, consider accepting the token's decimal count as an initialization parameter and deriving the conversion factor dynamically rather than relying on a hard-coded constant. Note that decimals() is optional in the ERC-20 standard and not all tokens implement it, so querying it on-chain may not be reliable and an explicit parameter validated by the deployer is a safer approach.

6.31 MarketFactory owner storage variable has no setter preventing rotation for market ownership

MarketFactoryStorage declares a plain address public owner variable that is assigned once in the constructor and passed to every cloned Market's initialize(...) as its Ownable owner:

// MarketFactoryStorage.sol
address public owner;

// MarketFactory.sol constructor
owner = msg.sender;

// MarketFactory.sol createMarket
Market(newMarket).initialize(
    // ...
    owner,
    // ...
);

In Market.sol, this address receives Ownable ownership via _transferOwnership(_owner).

MarketFactory uses AccessControl for role management. However, the owner storage variable has no setter function. After an admin rotation, all newly created markets still receive the original deployer as their Ownable owner. The new admin has no way to set itself as the owner of future markets without redeploying the entire factory.

While each Market's ownership can be changed post-deployment via Ownable's transferOwnership(...), this requires the stale owner to cooperate and must be done individually per market.

Recommendation

Consider adding a setter function for owner so that owner rotation is reflected in future market deployments.

6.32 No combined ceiling on protocolFee + curatorFee

Market.initialize validates protocolFee independently (_protocolFee >= 10000 reverts) but does not check the sum of protocolFee + curatorFee. Both fees are in basis points, and each is capped under 10000 individually, so their combined value could reach up to 19998 bps ( 200% of the cost).

In _getPredictionCosts:

uint256 totalAmountRequired = cost + _protocolFee + curatorFeeAmount;

If protocolFee = 9999 (99.99%) and curatorFee = 9999 (99.99%), the total amount required would be approximately three times the LMSR cost. While such extreme fees would be set by the market creator and visible to users before they participate, the protocol has no guardrail preventing accidental or malicious over-fee configuration.

Recommendation

Consider adding a combined fee ceiling check (e.g., protocolFee + curatorFee < MAX_COMBINED_FEE) in Market.initialize to prevent markets from being created with unreasonable total fee rates.

6.33 No on-chain order cancellation for regular orders

Regular Order structs have no on-chain cancellation mechanism. The nonce field is part of the signed hash but is never validated against an on-chain counter. Orders with expiration = 0 exist forever. The only way to cancel is off-chain by asking the operator to stop matching. If the operator is compromised, stale orders can be filled at outdated prices and users have no way to revoke signed orders on-chain.

Recommendation

Consider adding an invalidateNonce(uint256 nonce) function that lets users increment their nonce, invalidating all orders with lower nonces.

6.34 Non-delayed operations invalidate pending delayed operations via shared nonce

The MultisigController uses a single nonce counter per target for all operations. When a delayed operation is queued, the nonce is not incremented until the operation actually executes:

function executeWithMultisig(Operation memory operation, bytes[] memory signatures)
    external
    override
    returns (bool)
{
    _validateOperation(operation);
    // ...
    if (!_checkAndHandleDelay(operation.target, operation.selector, operationHash)) {
        return false; // Operation queued, not executed -- nonce NOT incremented
    }

    executedOperations[operation.target][operationHash] = true;
    nonces[operation.target]++;
    // ...
}

While a delayed operation sits in the queue waiting for its delay to elapse, the target nonce remains unchanged. If during this window the signers execute any other non-delayed operation for the same target, the nonce advances and the pending delayed operation becomes permanently unexecutable:

1. Signers queue setWinner(...) with a 1-day delay (nonce = 5). The operation is queued but the nonce stays at 5;
2. During the delay, signers execute setSellsEnabled(true) (non-delayed, nonce = 5). This succeeds and increments the nonce to 6;
3. After the delay elapses, the queued setWinner operation (nonce = 5) fails validation because 5 != nonces[target] (now 6);
4. The setWinner must be re-signed with nonce 6 and re-queued, restarting the full delay period;

The stale pendingOperations and operationQueued entries for the invalidated operation also remain in storage and are never cleaned up.

Recommendation

Consider documenting this behavior clearly for operators so they understand that executing any operation during a pending delay period will invalidate the queued operation.

6.35 OrderbookMarket emergency withdrawal has no timelock

In Market.sol (LMSR markets), _emergencyWithdraw enforces a 7-day timelock after the event date to give time for normal settlement before funds can be drained:

function _emergencyWithdraw() internal {
    if (block.timestamp < eventDate + EMERGENCY_TIMELOCK) {
        revert EmergencyTimelockNotMet();
    }
    // ...
}

The equivalent function in OrderbookMarket has no such restriction:

function _emergencyWithdraw(address recipient) internal {
    if (recipient == address(0)) revert ZeroAddress();

    uint256 balance = IERC20(usdc).balanceOf(address(this));
    if (balance == 0) revert ZeroAmount();

    totalUsdcReserve = 0;
    IERC20(usdc).safeTransfer(recipient, balance);

    emit EmergencyWithdrawal(recipient, balance);
}

The multisig can execute an emergency withdrawal at any time, including while the market is active and users are holding positions. While MultisigController may impose a time delay on the operation itself, this is a configurable per-function delay set during deployment and is not tied to the market's lifecycle. A market configured with a short or zero delay for emergencyWithdraw would allow immediate fund extraction.

Recommendation

Consider adding a timelock tied to the market's lifecycle.

6.36 The executeMultisigOperation(...) function bypasses initialization checks for marketFactory

The CreditManager contract is designed to have a one-time initialization for the marketFactory address. The setMarketFactory(...) function, which is restricted to the contract owner, explicitly prevents re-initialization by checking if marketFactory is already set.

function setMarketFactory(address _marketFactory) external override onlyOwner {
    // @audit-issue This check prevents re-initialization by the owner.
    if (marketFactory != address(0)) revert AlreadyInitialized();
    _setMarketFactory(_marketFactory);
}

However, the executeMultisigOperation(...) function allows a multisigController to call the internal _setMarketFactory(...) function directly without performing this "already initialized" check.

function executeMultisigOperation(bytes4 selector, bytes calldata data) 
    external override returns (bool) 
{
    if (msg.sender != multisigController) revert NotAuthorized();

    if (selector == 0x4dcc434a) { // setMarketFactory(address)
        address _marketFactory = abi.decode(data, (address));
        // @audit-issue This internal call bypasses the alreadyInitialized check.
        _setMarketFactory(_marketFactory);
    } 
    // ...
}

If the intention of the protocol is to ensure that the marketFactory is an immutable, one-time set parameter for security reasons, the current implementation allows the multisig to bypass this constraint and change the factory at any time. If this bypass is an intended administrative "backdoor" for the multisig, it should be explicitly documented to avoid confusion regarding the contract's initialization state.

Recommendation

Consider adding the if (marketFactory != address(0)) revert AlreadyInitialized(); check inside the executeMultisigOperation(...) block for the 0x4dcc434a selector if the parameter is intended to be strictly one-time. Alternatively, if the multisig is supposed to have the power to rotate the factory, update the documentation to reflect that the initialization check only applies to the standard Ownable caller and not the multisig.

Resolution

By design.

6.37 MAX_SHARES overflow guard is not compatible with the allowed liquidityParameter range

The contract uses a fixed MAX_SHARES limit (13,000,000,000) as an overflow guard, but the actual overflow condition in the LMSR math depends on the ratio shares / b used as the input to UD60x18.exp().

// MarketStorage.sol
uint256 public constant MIN_LIQUIDITY_PARAMETER = 10;
uint256 public constant MAX_SHARES = 13_000_000_000;

Pricing and cost compute exponentials on (shares * 1e18) / b:

// MarketCalculations.sol
UD60x18 udA = ud((qA * DECIMALS) / b);
UD60x18 udB = ud((qB * DECIMALS) / b);
uint256 expA = unwrap(udA.exp());
uint256 expB = unwrap(udB.exp());

UD60x18.exp() reverts when its input exceeds its maximum supported value. For small b values (the contract allows b as low as MIN_LIQUIDITY_PARAMETER = 10), even relatively small sharesA/sharesB values can push (shares * 1e18) / b above the exp() limit, causing getPrice, getCostFunction, predict(), sellPosition(), and getSellReturn() to revert.

Because _setLiquidityParameter(uint256 newB) permits setting b down to 10:

function _setLiquidityParameter(uint256 newB) internal {
    if (newB < MIN_LIQUIDITY_PARAMETER || newB > MAX_LIQUIDITY_PARAMETER) {
        revert InvalidLiquidityParameter();
    }
    liquidityParameter = newB;
}

a misconfiguration (or a later reduction of b after shares have grown) can effectively brick the market’s pricing/trading functions due to math reverts, despite MAX_SHARES checks still passing.

Recommendation

Consider making MAX_SHARES dynamic based on the current b value, or adding a validation that checks the existing share counts against the new b to ensure calculation remains within exp()'s safe input range.

6.38 WinnerSet event is misleading when a draw is declared

The _setWinner(...) function emits a single WinnerSet event for all settlement outcomes, including draws:

function _setWinner(bool sideAHasWon_, bool draw) internal {
    // ...
    if (draw) {
        isDraw = true;
    } else {
        sideAHasWon = sideAHasWon_;
    }
    settled = true;
    marketStatus = MarketStatus.SETTLED;
    emit MarketStatusChanged(MarketStatus.CLOSED, MarketStatus.SETTLED);
    emit WinnerSet(sideAHasWon_, draw);
}

When draw is true, the sideAHasWon_ parameter is not used in state updates, but it is still included in the emitted event. This means a draw can produce WinnerSet(true, true) or WinnerSet(false, true) depending on the caller's input, both representing the same outcome but appearing different to off-chain consumers indexing the event. The event name itself is also semantically incorrect for a draw, as there is no winner.

By comparison, the Orderbook market emits a Resolved(WinningOutcome, uint256) event that clearly distinguishes all three outcomes through the enum value.

Recommendation

Consider emitting a separate DrawDeclared event when a draw is settled, or renaming the event to something like MarketResolved and normalizing the sideAHasWon_ parameter to a consistent value (e.g., false) when draw is true.

6.39 isRedeemOrderValid returns false for cancelled markets despite executeRedeem succeeding

The isRedeemOrderValid view function only checks whether a market is resolved via isResolved():

try IOrderbookMarket(redeemOrder.market).isResolved() returns (bool resolved) {
    if (!resolved) return false;
} catch {
    return false;
}

However, redeemFor(...) in OrderbookMarket successfully processes redemptions for both resolved and cancelled markets:

bool isResolvedMarket = resolution.resolved;
bool isCancelledMarket = status == OrderbookMarketStatus.CANCELLED;

if (!isResolvedMarket && !isCancelledMarket) {
    revert MarketNotResolved(); //..

When a market is cancelled via _cancel(), only status is set to CANCELLED, resolution.resolved remains false. So isResolved() returns false and isRedeemOrderValid returns false, even though executeRedeem would succeed.

Since redeemFor is onlyExchange, users cannot redeem directly. If the backend relies on isRedeemOrderValid as a gate, it will skip all cancelled-market redemptions, preventing users from claiming their refunds through the normal gasless flow.

Recommendation

Consider updating isRedeemOrderValid to also check for the cancelled state, for example by adding an isCancelled() check alongside isResolved().

6.40 minimumOrderAmount stored but never enforced

minimumOrderAmount is initialized in initialize() and settable via multisig (_setMinimumOrder), but never checked in fillOrders() or any execution path. It is a dead state variable — the setter and storage exist but have no effect on protocol behavior.

Recommendation

Either enforce the minimum in fillOrders() or remove the variable entirely.

6.41 Enum types lack a default sentinel value for uninitialized state

Several enum definitions across the codebase assign meaningful values at index 0, which is also the default value for any uninitialized enum in Solidity storage. This makes it impossible to distinguish between an explicitly set value and one that was never assigned.

As an example, WinningOutcome:

enum WinningOutcome {
    SIDE_A, // 0 - YES wins (binary) or HOME wins (ternary)
    SIDE_B, // 1 - NO wins (binary) or AWAY wins (ternary)
    DRAW    // 2 - Only valid for ternary markets
}

An uninitialized WinningOutcome variable silently reads as SIDE_A, which represents a real resolution. Any code path that reads this value before the market is resolved would interpret it as "Side A wins" rather than "no winner has been set."

Other enums follow the same pattern: MarketStatus defaults to OPEN, Side defaults to BUY, MatchType defaults to DIRECT, and MarketType defaults to LMSR. While MarketStatus defaulting to OPEN happens to align with the intended lifecycle of a newly deployed market, the remaining enums carry real semantic meaning at index 0 with no way to represent an unset state.

Recommendation

Consider prepending a NONE or UNSET member at index 0 for enums where an uninitialized default could be confused with a valid state.

6.42 Fee configuration allows setting topicRate to zero after registration

When registering a market, _registerMarket requires a non-zero topicRate:

function _registerMarket(
    address market,
    uint256 topicRate,
    bool dynamicPricingEnabled
) internal {
    if (market == address(0)) revert ZeroAddress();
    if (registeredMarkets[market]) revert MarketAlreadyRegistered();
    if (topicRate == 0) revert MarketFeeConfigRequired();
    // ...
}

However, _setMarketFeeConfig (callable via multisig) does not enforce the same constraint:

function _setMarketFeeConfig(
    address marketAddr,
    uint256 topicRate,
    bool dynamicEnabled
) internal {
    if (!registeredMarkets[marketAddr]) revert MarketNotRegistered();

    marketFeeConfigs[marketAddr] = MarketFeeConfig({ topicRate: topicRate, dynamicPricingEnabled: dynamicEnabled });
    emit MarketFeeConfigSet(marketAddr, topicRate, dynamicEnabled);
}

The multisig can set topicRate = 0 after registration, which disables all fees for that market. This is inconsistent with the registration-time validation.

Recommendation

Consider aligning the validation between both functions.

6.43 No mechanism to deauthorize markets from CreditManager

When MarketFactory creates a market, it authorizes it as a credit consumer in CreditManager:

CreditManager creditManager = CreditManager(CREDIT_MANAGER);
creditManager.setAuthorizedConsumer(newMarket);

The setAuthorizedConsumer function can only set authorization to true:

function setAuthorizedConsumer(address _consumer) external override onlyMarketFactory nonReentrant {
    if (_consumer == address(0)) revert ZeroAddress();
    isAuthorizedConsumer[_consumer] = true;
    emit NewConsumer(_consumer);
}

There is no removeAuthorizedConsumer function, and the executeMultisigOperation dispatcher only handles the setMarketFactory, addCredit, and consumeCredit selectors. Once a market is authorized, it retains the ability to call addCredit and consumeCredit indefinitely.

Authorized markets can both consume user credits (reducing balances) and add credits (inflating balances). If a market needs to be decommissioned, its credit authorization persists with no revocation path. The only recovery would be deploying an entirely new CreditManager, losing all existing credit balances and requiring re-authorization of every legitimate market.

Recommendation

Consider adding a removeAuthorizedConsumer(address) function gated by the multisig controller.

6.44 Non-standard double-hash EIP-712

MultisigController wraps the EIP-712 structured hash inside an additional 19Ethereum Signed Message:32 hash. This creates a non-standard signature scheme that may confuse wallet integrations expecting pure EIP-712. The Orderbook system correctly uses standard EIP-712 via OpenZeppelin's EIP712 base contract. Wallets or signing libraries that implement EIP-712 natively will not produce the correct signature format without additional wrapping logic.

Recommendation

Consider aligning MultisigController with the Orderbook's approach and use OpenZeppelin's EIP712 base contract for standard-compliant structured hashing.

6.45 OrderbookFactory allows 100% protocol fee at market creation

The OrderbookFactory validates the protocol fee with an inclusive upper bound:

// OrderbookFactory.sol
if (protocolFee > 10000 || protocolFee == 0 || topicRate == 0) revert InvalidProtocolFee();

This allows protocolFee = 10000 (100%), meaning the operator could charge 100% of trade value as fees. In contrast, the AMM Market contract uses a strict upper bound:

// Market.sol initialize
if (_protocolFee >= 10000) {
    revert InvalidFees();
}

This inconsistency means AMM markets enforce a strictly-less-than-100% fee, while orderbook markets allow exactly 100%.

Recommendation

Consider aligning the validation to use a strict upper bound consistently across both factories, or document the intentional difference.

6.46 OrderbookMarket transferFrom silently no-ops for invalid token IDs

transferFrom handles token IDs 0 (YES), 1 (NO), and 2 (DRAW), but has no default revert for unrecognized values:

function transferFrom(
    address from,
    address to,
    uint256 amount,
    uint8 tokenId
) external override onlyExchange whenActive whenNotResolved whenNotExpired {
    if (tokenId == 0) {
        // YES token
        // ...
    } else if (tokenId == 1) {
        // NO token
        // ...
    } else if (tokenId == 2) {
        // DRAW token (only for ternary markets)
        if (!hasDraw) revert DrawNotAllowed();
        // ...
    }

    emit Transfer(from, to, amount, tokenId);
}

If tokenId >= 3, none of the branches execute. The function emits a Transfer event and returns without moving any tokens. In a DIRECT trade, the buyer's USDC is transferred to the seller before transferFrom is called. If the token transfer silently fails, the buyer pays USDC but receives no tokens.

This requires a user to sign an order with an invalid tokenId, so it is a self-harm scenario rather than an external exploit. However, the function should fail explicitly rather than succeed silently.

Recommendation

Add a revert for unrecognized token IDs after the last else if branch.

6.47 State changes in the predict(...) function are ineffective due to an immediate revert

The predict(...) function is the primary entry point for users to participate in a market. It includes a check to prevent predictions from being placed after the eventDate has passed.

function predict(...) external override nonReentrant returns (uint256) {
    // ...
    if (block.timestamp > eventDate) {
        // Auto-close market if past event date
        // @audit-issue These state changes and events are reverted by the following line.
        marketStatus = MarketStatus.CLOSED;
        emit MarketStatusChanged(MarketStatus.OPEN, MarketStatus.CLOSED);
        revert EventClosed();
    }
    // ...
}

As shown in the code snippet above, when the current time exceeds the eventDate, the contract attempts to update the marketStatus to MarketStatus.CLOSED and emit a MarketStatusChanged event. However, the function immediately executes revert EventClosed(). In the EVM, a revert undoes all state modifications made during the execution of a transaction. Consequently, the assignment to marketStatus is rolled back, and the event is never stored in the blockchain logs.

This results in the "auto-close" logic being a no-op. The market status will remain in its previous state until a successful transaction, such as an administrator calling _setWinner(...) or _closeMarket(...), transitions the status.

Recommendation

Consider removing the state update and event emission from within the reverting if block to save gas and improve code clarity.

6.48 The depositUSDC(...) function is redundant

The Market contract provides the depositUSDC(...) function, which is restricted to the contract owner, to allow the manual transfer of USDC into the contract's balance.

function depositUSDC(uint256 amount) external override onlyOwner nonReentrant {
    IERC20(usdcToken).safeTransferFrom(msg.sender, address(this), amount);
    emit USDCDeposited(msg.sender, amount);
}

However, the function only executes a safeTransferFrom(...) call and emits an event. It does not update any internal accounting variables. Since the contract can receive USDC through standard token transfers, this function is functionally redundant.

This results in unnecessary gas consumption for the owner due to the onlyOwner and nonReentrant modifiers.

Recommendation

Consider removing the depositUSDC(...) function to simplify the codebase and reduce gas costs.

Resolution

We acknowledge the function is technically redundant — USDC can be transferred directly to the contract. However, we will keep it for operational reasons.

6.49 Wrong error emitted in withdrawCuratorFees

The check msg.sender != curator emits NotOwner() error instead of a more appropriate NotCurator() error. This causes misleading revert messages for off-chain tooling and front-ends attempting to display why a withdrawal failed.

Recommendation

Consider defining and emitting a NotCurator() custom error in place of NotOwner().

6.50 _cancel() in OrderbookMarket does not require ACTIVE status

The _cancel() function only checks that the market has not been resolved, but does not verify the market is in ACTIVE status:

function _cancel() internal {
    if (resolution.resolved) revert MarketResolved();

    OrderbookMarketStatus oldStatus = status;
    status = OrderbookMarketStatus.CANCELLED;

    emit StatusChanged(oldStatus, status);
    emit MarketCancelled();
}

This means a market that is already PAUSED or CANCELLED can be cancelled again. In the case of re-cancellation, the function succeeds silently (changing CANCELLED to CANCELLED), consuming gas and emitting duplicate MarketCancelled events. While this does not affect contract state, it is inconsistent with the expectation that cancel is a one-way state transition from ACTIVE (or PAUSED) to CANCELLED.

Recommendation

Consider adding a status check to ensure the market is in a cancellable state (e.g., ACTIVE or PAUSED) before allowing cancellation.