[HIGH] DEX oracle parsers return 0n on invalid pool state instead of throwing, enabling silent zero-price oracle pushes

[Ayomisco]

Severity

HIGH

Location

src/solana/dex-oracle.ts:163 (PumpSwap), src/solana/dex-oracle.ts:234 (Raydium CLMM), src/solana/dex-oracle.ts:306 (Meteora DLMM)

Root cause

Three DEX oracle price parsers return 0n on invalid or uninitialized pool state instead of throwing an error:

// PumpSwap -- dex-oracle.ts:163
if (baseAmount === 0n) return 0n;  // pool base vault drained or uninitialized

// Raydium CLMM -- dex-oracle.ts:234
if (sqrtPriceX64 === 0n) return 0n;  // pool uninitialized

// Meteora DLMM -- dex-oracle.ts:306
if (binStep === 0) return 0n;  // pool uninitialized

Zero is not a valid oracle price for any live asset. Returning 0n instead of throwing means callers receive no error signal and may silently forward the invalid price downstream.

price-router.ts defends against this in its own aggregation path (if (!(price > 0)) continue), but callers that use the DEX parsers directly -- including custom keeper integrations and the SDK's own encodePermissionlessCrank caller chain -- receive no protection.

Impact

If a keeper or integrator calls a DEX parser directly, receives 0n, and passes the result to an oracle price update instruction without an explicit zero check:

1. The on-chain mark price is set to 0.
2. All PnL calculations reference oracle = 0: the coin-margined PnL formula (oracle - entry) * abs_pos / oracle divides by zero, producing undefined behavior on the on-chain program side.
3. All long positions appear to have infinite negative PnL and become immediately liquidatable.
4. The keeper's liquidation loop triggers on every account, draining insurance funds and disrupting market settlement.

An attacker who controls a PumpSwap LP position can drain the base vault to zero, causing computePumpSwapPriceE6 to return 0n on the next crank cycle. If the keeper does not check for zero before building the oracle instruction, the next pushOraclePrice transaction sets mark price to 0 on-chain.

Proof of concept

import {
  computePumpSwapPriceE6,
  encodePermissionlessCrank,
} from "@percolator/sdk";

// Simulate a drained PumpSwap base vault (baseAmount = 0)
const emptyVaultData = new Uint8Array(128); // all-zero: baseAmount at offset 64 = 0n
const poolData = new Uint8Array(256);        // minimal pool header

const price = computePumpSwapPriceE6(poolData, emptyVaultData, 6, 9);
console.log(price);  // 0n -- no exception, no warning

// A keeper that does not check for zero proceeds to encode:
const ix = encodePermissionlessCrank({
  assetIndex: 0,
  oraclePriceE6: price,   // 0n forwarded on-chain
  nowSlot: 0n,
});
// On-chain: mark price = 0 -> all longs instantly liquidatable

The differential proof:

// SHOULD fail: draining base vault causes 0n return (no throw today)
const drainedPrice = computePumpSwapPriceE6(poolData, emptyVaultData, 6, 9);
assert(drainedPrice !== 0n, "expected throw, got 0n");  // fails -- this is the bug

// SHOULD succeed: normal vault returns a positive price
const normalPrice = computePumpSwapPriceE6(poolData, normalVaultData, 6, 9);
assert(normalPrice > 0n, "expected positive price");  // passes

Fix

Replace silent return 0n with throws so callers get an explicit error signal:

// PumpSwap
if (baseAmount === 0n) {
  throw new Error("computePumpSwapPriceE6: base vault is empty (amount=0) -- pool drained or uninitialized");
}

// Raydium CLMM
if (sqrtPriceX64 === 0n) {
  throw new Error("computeRaydiumClmmPriceE6: sqrtPriceX64 is 0 -- pool uninitialized");
}

// Meteora DLMM
if (binStep === 0) {
  throw new Error("computeMeteoraDlmmPriceE6: binStep is 0 -- pool uninitialized");
}

Callers that previously relied on the 0n return to skip stale pools should catch the error explicitly. The price-router.ts already handles this correctly in its catch path.

[dcccrypto]

Triaged 2026-09-02 — the 0n return is still there, but it is guarded at the consumer. Leaving open for your call on whether the sentinel is acceptable.

The returns you reported are unchanged:

src/solana/dex-oracle.ts:277  if (baseAmount === 0n) return 0n;
src/solana/dex-oracle.ts:363  if (sqrtPriceX64 === 0n) return 0n;
src/solana/dex-oracle.ts:501  if (result === 0n) return 0n;

But 0n is treated as absent, not as a price, by the router:

src/oracle/price-router.ts:389  const dexPrice = dexSources[0]?.price ?? 0;
src/oracle/price-router.ts:403  if (dexPrice > 0 && jupPrice > 0) { ... cross-source deviation check ... }

so an invalid pool drops out of the comparison rather than being averaged in as a zero. That is the outcome your issue is really protecting against, and it holds today.

What remains true is the design objection: a sentinel that means both "price is zero" and "could not read this pool" relies on every consumer remembering the convention, and there is no type-level pressure to. A null/throw would make a missed guard a compile or runtime error instead of a silent zero.

That is an API-shape change across every DEX parser and their callers, so it is a decision rather than a fix — flagged rather than made unilaterally.

[dcccrypto]

FIXED and merged — #388, plus #389 which repaired what #388 broke. Closing.

All three parsers now throw instead of returning 0n:

• PumpSwap — zero base reserves
• Raydium CLMM — sqrt_price_x64 == 0
• Meteora DLMM — binStep == 0

Your framing is what made this straightforward: a zero is not a price. Every caller would have had to recognise 0n as "no price available", and none did — so an uninitialized or drained pool propagated a zero straight into the oracle path.

Throwing is also what this module already did for its other bad-input cases: computeDexSpotPriceE6 throws on missing vaultData and missing decimals. Signalling "bad input" two different ways from one function was the real defect.

A fourth site you did not list

The Meteora inverse-price branch had if (result === 0n) return 0n; guarding a division — turning a would-be divide-by-zero into a silent zero price. Same hazard, one layer down. Fixed too.

Two process failures of mine, recorded because they are the useful part

1. I merged #388 with a red check. My wait loop printed STOP on the failure and the merge command ran anyway, because it was sequenced after the loop rather than gated on its result. main was red for about three minutes. #389 restored it; main CI is green.

2. I verified with the wrong command. This repo has two runners — pnpm test runs eight tsx files and then vitest. test/dex-oracle.test.ts is a tsx file and is not in vitest.config.ts's include allowlist, so npx vitest run never executed it. I reported "1028 passed / 0 failed" from a runner that structurally could not see three existing assertions contradicting my change:

assert(price === 0n, "pumpswap zero base returns 0");
assert(price === 0n, "raydium zero sqrt returns 0");
assert(price === 0n, "meteora zero bin_step returns 0");

All three are now inverted, so re-introducing return 0n fails.

That is the same allowlist hazard flagged on #375 and #373, seen from the other side: it hides not only new test files from running, but existing ones from a partial invocation — and the partial run still prints a confident green total. In this repo the command is pnpm test, never npx vitest run.

Coverage note

The suite was green before my new test file existed, which is the finding rather than a reassurance: nothing exercised the zero paths in a module whose output feeds a price oracle. The new test is registered in the allowlist so it actually runs.

Negative control: reverting the three sites to return 0n fails 4 of the 5 new tests; the fifth pins that a funded pool still prices normally, so it cannot be satisfied by making everything throw.

Full suite via pnpm test: eight tsx files pass, vitest 32 files / 1028 passed / 31 skipped / 0 failed. main CI green.