Skip to content

fix: keep on-chain pegged prices; never use CoinGecko for pegged coins - #175

Merged
harshaalphafi merged 6 commits into
mainfrom
bugfix/pegged-coin-price
Aug 15, 2026
Merged

fix: keep on-chain pegged prices; never use CoinGecko for pegged coins#175
harshaalphafi merged 6 commits into
mainfrom
bugfix/pegged-coin-price

Conversation

@preyam2002

@preyam2002 preyam2002 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Problem

ALKIMI and UP are pegged at 0 on-chain (oracle::set_fixed_price, 2026-08-04). The ws update_prices cron mirrors the peg into coin_info.pyth_price every ~10 s. The SDK gates its price fallback on if (pythPrice), but the GraphQL Float arrives as a JS number at runtime — the declared string | null type is wrong — so a pegged 0 is falsy and silently falls through to coingeckoPrice.

The portfolio math then counts phantom collateral. totalSuppliedUsd, safeBorrowLimit, and the weighted liquidation threshold (ALKIMI threshold 69, UP 60) all inflate, so getUserPortfolio reports a position as safer than the chain enforces. getAllMarketsData prices the wound-down markets at the CoinGecko quote instead of 0.

Fix

New resolveCoinPrice in src/utils/price.ts with a PEGGED_COIN_TYPES allowlist (ALKIMI, UP):

  • A pegged coin takes pythPrice only, zero included. It never uses CoinGecko. A missing mirror value gives null, not a market-quote substitute.
  • Non-pegged coins keep the pyth-then-coingecko rule. Non-positive and non-finite values stay unusable.
  • The helper accepts both string and number runtime shapes.

All five fallback sites now use it: Market.getPrice, Position.getPrice, the rewards USD helper, and both swap-quote USD estimates. The swap guards still skip a 0 price instead of dividing by it.

Keep PEGGED_COIN_TYPES in sync with alphalend-sdk-rust src/blockchain/coin_registry.rs and the liquidator's price_feed_config.rs.

Verification

  • New __tests__/pegged-price.test.ts: pegged zero survives; a pegged coin with no mirror value returns null, never CoinGecko; non-pegged behavior unchanged; number and string shapes both handled.
  • 62/62 tests across 7 suites. npm run build clean.

Companion Rust fix: AlphaFiTech/alphalend-sdk-rust#222 (same allowlist, same rule).

ALKIMI/UP are pegged at 0 via oracle::set_fixed_price and mirrored into
coin_info.pyth_price by the ws cron. The truthiness gate on pythPrice
treats the pegged 0 (a JS number at runtime) as missing and substitutes
the CoinGecko market quote, which inflates totalSuppliedUsd,
safeBorrowLimit, and the weighted liquidation threshold.

resolveCoinPrice (src/utils/price.ts) now carries a PEGGED_COIN_TYPES
allowlist: pegged coins take pythPrice only, zero included, and never
fall back to CoinGecko. Non-pegged coins keep the old rule. All five
fallback sites use the helper.
Same thin caller the other repos use; delegates to
AlphaFiTech/alphalend-workflows claude-review.yml@main.
Review follow-up: a Market.getMarketData fixture test proves the wiring
(pegged zero survives, non-pegged zero still falls back), a positive-peg
assertion covers the accept side of the >= 0 bound, and the
PEGGED_COIN_TYPES doc records that pegged prices carry no freshness
signal in this SDK.
resolveCoinPrice took the metadata and the coin type as two arguments,
so a caller could pass a mismatched pair. The coin type is already on
the metadata object. Read it from there.

The return type stays Decimal | null. A raw pythPrice return would
spread the CoinMetadata type error to every call site. The field is
declared string | null, but the GraphQL schema says Float and the wire
carries a JSON number. TypeScript would then permit string operations
that throw at runtime.

Verified against the live prod API: of the 52 coins it returns, only
ALKIMI and UP change, and the other 50 match main exactly.
11felix
11felix previously approved these changes Aug 13, 2026
@11felix
11felix self-requested a review August 13, 2026 21:13
@11felix

11felix commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Review

The core fix is correct. Verified against prod api.alphalend.xyz/public/graphql: of the 52 coins returned, the two hardcoded coin types match byte-for-byte, pythPrice arrives as a JSON float 0.0 for both (confirming the declared string | null is wrong at runtime), and no other coin has a non-positive pythPrice. Ran the suite on this branch: 67/67 pass, npm run build clean.

One blocking issue.

A zero price produces Infinity reward APRs, then NaN across the whole portfolio

src/models/market.ts:265 and :334:

const marketPrice = this.getPrice(this.market.coinType);
if (!marketPrice) { throw new Error(...) }        // dead: !new Decimal(0) === false
const totalLiquidityValue = totalLiquidity.mul(marketPrice);   // -> 0
...
const rewardApr = rewardValue.div(totalLiquidityValue);        // -> Infinity

getPrice returns a Decimal, and a Decimal instance is always truthy, so the guard meant to catch a missing price never fires. That was harmless before this PR because a zero price could not reach it. Now it can, and decimal.js returns Infinity for x/0 rather than throwing. Same at :294 and :362 (if (!price) continue).

It does not stay contained to the pegged market. In position.ts:82-94:

supplyApr.interestApr.add(stakingApr).add(totalSupplyRewardApr).mul(collateralUsd)
// Infinity * 0 === NaN

so netApr, dailyEarnings, and aggregatedSupplyApr go NaN for any user holding ALKIMI/UP collateral, not just those markets' rows.

Reproduced on this branch with a market fixture carrying an active reward campaign:

supply reward APRs: [ 'Infinity' ]
borrow reward APRs: [ 'Infinity' ]
sum * collateralUsd(0) = NaN

Latent today, not firing. Prod market_data off wss://ws.alphalend.xyz/ws shows markets 17 and 19 both with "rewards": [] — no reward distributor is active on either. It fires the moment one is attached.

What makes this worth fixing here rather than filing separately: the companion Rust PR guards exactly this case, at lending_protocol.rs:1406 and :1532, with the comment "market_price feeds a divisor below; a missing or zero price would panic"Some(price) if !price.is_zero() → skip with a warn. This PR mirrors the price rule but not the divisor guard. Same shape here:

if (marketPrice.lte(0)) return rewardAprs;   // at 265 and 334
if (price.lte(0)) continue;                  // at 294 and 362

A test with a reward campaign on the pegged fixture would pin it — the current Market.getMarketData fixture has empty distributors, so it stays green either way.

Checked, no action needed

  • All five fallback sites are migrated. Remaining pythPrice || hits are commented-out dead code (client.ts:2062-2063), map construction, and the ALPHA override.
  • alphafi-ws/src/data/sdk_manager.rs:200 looks like the same bug (pyth_price.or(coingecko_price)) but is Option::or, so Some(0.0) already wins. It reads the same coinInfo endpoint via alphafi-sdk-rust/src/provider/coin_info_provider.rs:31, so the AlphaFi side already serves the peg. No third fix needed.
  • No other pyth→coingecko fallback exists anywhere in the monorepo (swept all .rs/.ts).
  • Other divisions. position.ts aggregates are all .gt(0)-guarded; swap paths skip on a falsy price by design. On the FE, flashRepayUtils.ts:55 already has if (dPrice.lte(0)) return ... and SwapModal.tsx:173 guards > 0.
  • Release mechanics. dist/ is gitignored and version bumps are separate chore(release) commits, so nothing else is needed in this PR. alphafi-fe consumes the SDK via file:../alphalend-sdk-js, so no npm publish gates the FE fix.

The resolver makes 0 a legal price, and three paths assumed it never
was. flashRepay divides the withdraw value by the withdraw market's
price: at 0 the raw amount is Infinity and the supply cap turns that
into 'withdraw the entire collateral balance', so refuse to build the
transaction instead. The reward-APR paths divide by liquidity value
scaled by the market price: at 0 the APRs go NaN and poison every
portfolio aggregate they touch, so skip reward APRs with a warn. The
old truthiness guards on these paths were dead — a Decimal is always
truthy — and are deleted.

An unpriced reward coin stays in the list at $0 / 0% APR to match the
Rust SDK, whose numbers ws serves. Export resolveCoinPrice and
PEGGED_COIN_TYPES so consumers can adopt the rule instead of copying
the list. Tests fail when any guard is reverted (verified one by one).
README documents that MarketData.price can now be exactly 0.
@11felix

11felix commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Re-review of 0cc2c41

The finding is fixed, and the commit found a third divisor I had missed. Verified on this head: 71/71 tests across 8 suites, npm run build clean.

The original failure mode is gone. Re-ran the fixture that previously produced ['Infinity'] / NaN — a pegged market with an active reward campaign now yields:

supply: []
borrow: []
weighted (sum * collateralUsd=0): 0
no usable price for reward-APR market; skipping reward APRs: 0x1a8f4bc3…::alkimi::ALKIMI

The guards are genuinely pinned. Reverted each one individually and re-ran the suite:

guard reverted result
supply reward-APR (market.ts:268) 1 failed, 70 passed
borrow reward-APR (market.ts:343) 1 failed, 70 passed
flashRepay (flashRepay.ts:124) 2 failed, 69 passed

!gt(0) over lte(0) is the right call, and the reasoning checks out — decimal.js cmp returns NaN for a NaN operand, so NaN.lte(0) === false (the lte guard would let it through and divide) while !NaN.gt(0) === true (skips). Worth having as the documented house rule.

flashRepay was the more serious of the three. withdrawValueWithBuffer.div(withdrawMarket.price) at zero gives Infinity, and Decimal.min(Infinity, suppliedBaseUnits) collapses to the full supplied balance — so the user asks to withdraw ~370 and the built transaction withdraws the entire 21927 collateral position. Rejecting is right; the guard sits after both market lookups, so .price is always defined, and it matches the throw-based validation around it.

Also confirmed dropping if (!price) continue for reward coins matches the Rust SDK: lending_protocol.rs:1455 resolves the reward price through spot_price_for_usd (missing → $0 with a warn) and pushes the entry unconditionally, so an unpriced reward coin reports 0% there too. Behaviorally it was dead code either way — a Decimal is always truthy.

Nothing blocking left. Two notes:

  1. position.ts:170 is now the only price divisor still guarded with lte(0)if (suiPrice.lte(0) || stsuiPrice.lte(0)) before .div(stsuiPrice), which is the pattern the new README section warns against. Safe today, since resolveCoinPrice filters non-finite values to null and getPrice turns that into Decimal(0), so NaN cannot reach it. Worth flipping to !gt(0) anyway so the rule reads consistently.

  2. The third flashRepay test (does not raise the price error when both markets are priced) reaches the live Navi fee API. It passes either way, since any later throw still satisfies rejects.not.toThrow(/usable price/), but it puts an outbound HTTP call in the unit suite.

Full sweep of src/ at this head confirms every remaining price-derived divisor is guarded: market.ts:309/:383 and flashRepay.ts:191 by the new guards, position.ts:149/:153/:210 by .gt(0) on the aggregates, position.ts:175 as above.

@preyam2002

Copy link
Copy Markdown
Contributor Author

Merge/deploy notes: independent of the Rust-side ordering — this SDK is consumed by the FEs via file: dependency, so the fix lands on their next regular build; npm publish is optional. Companion PRs: alphalend-sdk-rust#222 (same bug, server-side SDK — the ws numbers) and alphalend-api#78 (guards market_stats against the pegged zero; must merge before the api rebuilds against #222). Behavior changes on FE pickup: swap-quote USD estimates for a pegged coin show $0 instead of a CoinGecko-derived value, and flash repay against a pegged market refuses with a clear error instead of silently withdrawing the entire collateral balance.

@jangid jangid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operate.md: clean — CI green (test / lint / build), head 0cc2c41. Not a contract PR. Zero review threads open.

Re-derived against the current diff rather than the replies. src/utils/price.ts is the single resolution point: toFinite normalises the runtime-number/declared-string mismatch, the peg branch admits >= 0 and never reaches CoinGecko, the non-pegged branch keeps > 0 with the fallback, and the peg check reads coinMetadata.coinType (not the caller's map key), which is what keeps the long-form SUI alias out of it. Both getPrice implementations (market.ts, position.ts) now route through it. The three divisor guards are present and use !gt(0) rather than lte(0) — correct, since decimal.js cmp returns NaN for a NaN operand, so lte would let a NaN through into the division.

Nothing new to raise; every finding on this PR is closed on the record.

One 🟢 note carried, not a blocker and already stated on the PR: position.ts still has one lte(0) price guard, which is safe today only because resolveCoinPrice filters non-finite values to null upstream. Worth flipping to !gt(0) so the file reads to one rule.

Approvals: 2/2 — merge is a maintainer call. Consumed by the FEs via file: dep, so it lands on their next build; no publish gate.

@harshaalphafi
harshaalphafi merged commit 98465e6 into main Aug 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants