fix: bugs

Author: shahthepro

README.md

@@ -210,6 +210,40 @@ When `includeMarkets: true`, impact results include a `markets` array of `Market
 
 For `fetchVaultApy`, the `marketDetails` array contains `MarketApyDetail` (same fields as above, without the `current`/`simulated` nesting).
 
+## Impact Sign Convention
+
+Impact values are signed as:
+
+```typescript
+impact = newApy - currentApy
+impactBps = round(impact * 10000)
+```
+
+- negative impact means APY decreased
+- positive impact means APY increased
+
+## Numerical Precision Notes
+
+This library converts on-chain `bigint` balances to JavaScript `number` for APY math.
+
+Because IEEE-754 doubles have 53 bits of integer precision, precision loss starts when raw base units exceed `2^53`:
+
+- 18 decimals: `2^53 / 1e18 = 0.009007199254740993` tokens
+- 6 decimals: `2^53 / 1e6 = 9,007,199,254.740992` tokens
+
+This does not automatically imply large APY error; it usually starts as tiny base-unit rounding.
+
+Approximate 18-decimal token magnitudes where floating spacing reaches:
+
+- `1e-12` tokens at ~`4.72e3` tokens
+- `1e-9` tokens at ~`4.84e6` tokens
+- `1e-6` tokens at ~`4.95e9` tokens
+- `1e-3` tokens at ~`5.07e12` tokens
+- `0.1` tokens at ~`6.49e14` tokens
+- `1` token at ~`5.19e15` tokens
+
+At runtime, the math layer emits a one-time warning per market when estimated token spacing reaches `1e-6` tokens or higher.
+
 ## Custom Constraints
 
 The `findMaxDeposit*` and `findMaxWithdrawal*` functions accept an optional `constraint` callback that adds custom criteria beyond APY impact. The binary search finds the largest amount satisfying **both** the APY impact threshold and the custom constraint.

docs/algorithm.md

@@ -11,6 +11,23 @@ This document describes the math behind vault APY computation, deposit impact si
 | WAD | 1e18 | Fixed-point scaling factor (18 decimals) |
 | SECONDS_PER_YEAR | 31,536,000 | 365 * 24 * 3600 (no leap year, matches Morpho convention) |
 
+## 1.1 Numerical Precision
+
+APY computation uses JavaScript `number` values after converting on-chain `bigint` balances by token decimals.
+
+- First precision loss starts when raw base units exceed `2^53`.
+- For 18-decimal tokens, this is `0.009007199254740993` tokens.
+- For 6-decimal tokens, this is `9,007,199,254.740992` tokens.
+
+In practice, rounding is usually tiny at first. For 18-decimal assets, approximate token magnitudes where floating spacing reaches:
+
+- `1e-12` tokens at ~`4.72e3` tokens
+- `1e-9` tokens at ~`4.84e6` tokens
+- `1e-6` tokens at ~`4.95e9` tokens
+- `1e-3` tokens at ~`5.07e12` tokens
+
+The implementation emits a one-time warning per market when estimated spacing reaches `1e-6` tokens or higher.
+
 ## 2. Single Market APY
 
 ### 2.1 Utilization
@@ -142,7 +159,8 @@ The increased supply lowers utilization, which typically lowers both borrow and
 
 ```
 newAPY = weightedVaultApy(markets, depositSim={})
-impactBps = round((currentAPY - newAPY) * 10000)
+impact = newAPY - currentAPY
+impactBps = round(impact * 10000)
 ```
 
 ## 5. Withdrawal Simulation
@@ -396,8 +414,9 @@ function computeDepositImpact(markets, idleAssets, depositAmount):
     currentApy = weightedVaultApy(markets)
     sim = simulateDeposit(markets, depositAmount)
     newApy = weightedVaultApy(markets, depositSim=sim)
-    impactBps = round((currentApy - newApy) * 10000)
-    return { currentApy, newApy, impactBps }
+    impact = newApy - currentApy
+    impactBps = round(impact * 10000)
+    return { currentApy, newApy, impact, impactBps }
 ```
 
 ### 7.6 computeWithdrawalImpact
@@ -414,7 +433,8 @@ function computeWithdrawalImpact(markets, idleAssets, withdrawAmount):
     else:
         newApy = weightedVaultApy(markets, withdrawalSim=sim)
 
-    impactBps = round((currentApy - newApy) * 10000)
+    impact = newApy - currentApy
+    impactBps = round(impact * 10000)
     isPartial = remaining > 0
-    return { currentApy, newApy, impactBps, isPartial, withdrawableAmount: withdrawable }
+    return { currentApy, newApy, impact, impactBps, isPartial, withdrawableAmount: withdrawable }
 ```

src/find-max.ts

@@ -8,7 +8,6 @@ import type {
   FindMaxConstraint,
   FindMaxDepositResult,
   FindMaxWithdrawalResult,
-  MarketApyDetail,
   MarketForApy,
   VaultApyDetailed,
   WithdrawalImpactResult,
@@ -70,6 +69,21 @@ function buildWithdrawalImpact(
   }
 }
 
+function makeFullDrainSnapshot(current: VaultApyDetailed): VaultApyDetailed {
+  return {
+    apy: 0,
+    markets: current.markets.map((m) => ({
+      marketId: m.marketId,
+      name: m.name,
+      supplyApy: 0,
+      borrowApy: 0,
+      utilization: 0,
+      allocationPct: 0,
+      vaultSupplyAssets: 0n,
+    })),
+  }
+}
+
 function zeroDepositImpact(markets: MarketForApy[], includeMarkets: boolean): DepositImpactResult {
   if (markets.length === 0) {
     return { currentApy: 0, newApy: 0, impact: 0, impactBps: 0, markets: [] }
@@ -258,9 +272,7 @@ export async function findMaxWithdrawalAmount(
   async function isAcceptable(amount: bigint): Promise<boolean> {
     const { sim, withdrawable, remaining } = simulateWithdrawal(markets, amount, idleAssets)
     const isFullDrain = withdrawable >= totalVaultAssets
-    const simulated = isFullDrain
-      ? { apy: 0, markets: [] as MarketApyDetail[] }
-      : weightedVaultApyDetailed(markets, {}, sim)
+    const simulated = isFullDrain ? makeFullDrainSnapshot(current) : weightedVaultApyDetailed(markets, {}, sim)
     const impact = Math.abs(simulated.apy - current.apy)
     if (impact > maxImpact) return false
     if (!constraint) return true
@@ -294,9 +306,7 @@ export async function findMaxWithdrawalAmount(
   if (await isAcceptable(effectiveMax)) {
     const { sim, withdrawable, remaining } = simulateWithdrawal(markets, effectiveMax, idleAssets)
     const simulated =
-      withdrawable >= totalVaultAssets
-        ? { apy: 0, markets: [] as MarketApyDetail[] }
-        : weightedVaultApyDetailed(markets, {}, sim)
+      withdrawable >= totalVaultAssets ? makeFullDrainSnapshot(current) : weightedVaultApyDetailed(markets, {}, sim)
     return {
       amount: effectiveMax,
       maxPossibleAmount: maxPossible,
@@ -335,9 +345,7 @@ export async function findMaxWithdrawalAmount(
 
   const { sim, withdrawable, remaining } = simulateWithdrawal(markets, lo, idleAssets)
   const simulated =
-    withdrawable >= totalVaultAssets
-      ? { apy: 0, markets: [] as MarketApyDetail[] }
-      : weightedVaultApyDetailed(markets, {}, sim)
+    withdrawable >= totalVaultAssets ? makeFullDrainSnapshot(current) : weightedVaultApyDetailed(markets, {}, sim)
   return {
     amount: lo,
     maxPossibleAmount: maxPossible,

src/math.ts

@@ -10,6 +10,31 @@ const TARGET_UTIL = 0.9
 const STEEPNESS = 4
 const WAD = 1e18
 const SECONDS_PER_YEAR = 365 * 24 * 3600 // 365-day year, no leap year — matches Morpho convention
+const PRECISION_WARNING_TOKEN_SPACING = 1e-6
+const PRECISION_WARNED_MARKETS = new Set<string>()
+
+function estimateTokenSpacing(rawAmount: bigint, decimals: number): number {
+  const abs = rawAmount < 0n ? -rawAmount : rawAmount
+  if (abs === 0n) return 0
+  const bitLength = abs.toString(2).length
+  if (bitLength <= 53) return 0
+  const spacingRaw = 2 ** (bitLength - 53)
+  return spacingRaw / Math.pow(10, decimals)
+}
+
+function warnIfPrecisionRisk(marketId: string, decimals: number, simSupply: bigint, simVaultSupply: bigint): void {
+  if (PRECISION_WARNED_MARKETS.has(marketId)) return
+  const supplySpacing = estimateTokenSpacing(simSupply, decimals)
+  const vaultSpacing = estimateTokenSpacing(simVaultSupply, decimals)
+  const spacing = Math.max(supplySpacing, vaultSpacing)
+  if (spacing < PRECISION_WARNING_TOKEN_SPACING) return
+
+  PRECISION_WARNED_MARKETS.add(marketId)
+  console.warn(
+    `[morpho-apy] Potential floating-point precision loss for market ${marketId}: ` +
+      `number spacing is ~${spacing.toExponential(2)} tokens at current magnitudes.`,
+  )
+}
 
 /** Normalized distance from target utilization. Range: [-1, +1]. */
 function _error(util: number): number {
@@ -99,6 +124,7 @@ export function weightedVaultApyDetailed(
     const wSim = withdrawalSim[m.marketId] ?? 0n
     simSupply = simSupply - wSim
     const simVaultSupply = m.vaultSupplyAssets - wSim
+    warnIfPrecisionRisk(m.marketId, m.decimals, simSupply, simVaultSupply)
 
     const supply = Number(simSupply) / scale
     const borrows = Number(m.totalBorrowAssets) / scale
@@ -228,8 +254,27 @@ export function simulateWithdrawal(
  * array of MarketImpactDetail with nested `current` and `simulated` snapshots.
  */
 export function mergeMarketDetails(current: MarketApyDetail[], simulated: MarketApyDetail[]): MarketImpactDetail[] {
-  return current.map((c, i) => {
-    const s = simulated[i]
+  const currentIds = new Set<string>()
+  for (const c of current) {
+    if (currentIds.has(c.marketId)) {
+      throw new Error(`[morpho-apy] Duplicate current marketId in mergeMarketDetails: ${c.marketId}`)
+    }
+    currentIds.add(c.marketId)
+  }
+
+  const simulatedById = new Map<string, MarketApyDetail>()
+  for (const s of simulated) {
+    if (simulatedById.has(s.marketId)) {
+      throw new Error(`[morpho-apy] Duplicate simulated marketId in mergeMarketDetails: ${s.marketId}`)
+    }
+    simulatedById.set(s.marketId, s)
+  }
+
+  return current.map((c) => {
+    const s = simulatedById.get(c.marketId)
+    if (!s) {
+      throw new Error(`[morpho-apy] Missing simulated marketId in mergeMarketDetails: ${c.marketId}`)
+    }
     return {
       marketId: c.marketId,
       name: c.name,

src/withdrawal-impact.ts

@@ -5,6 +5,18 @@ import { fetchVaultApyViem } from './fetch.js'
 import { mergeMarketDetails, simulateWithdrawal, weightedVaultApyDetailed } from './math.js'
 import type { WithdrawalImpactResult } from './types.js'
 
+function makeFullDrainMarketDetails(current: ReturnType<typeof weightedVaultApyDetailed>) {
+  return current.markets.map((m) => ({
+    marketId: m.marketId,
+    name: m.name,
+    supplyApy: 0,
+    borrowApy: 0,
+    utilization: 0,
+    allocationPct: 0,
+    vaultSupplyAssets: 0n,
+  }))
+}
+
 /**
  * Compute the APY impact of a withdrawal from a MetaMorpho vault.
  * Makes live on-chain RPC calls via the provided viem PublicClient.
@@ -45,7 +57,9 @@ export async function computeWithdrawalImpact(
   // Edge case: full withdrawal drains all vault positions
   const totalVaultAssets = markets.reduce((acc, m) => acc + m.vaultSupplyAssets, 0n) + idleAssets
   const simulated =
-    withdrawable >= totalVaultAssets ? { apy: 0, markets: [] } : weightedVaultApyDetailed(markets, {}, sim)
+    withdrawable >= totalVaultAssets
+      ? { apy: 0, markets: makeFullDrainMarketDetails(current) }
+      : weightedVaultApyDetailed(markets, {}, sim)
   const impact = simulated.apy - current.apy
 
   return {

test/find-max.test.ts

@@ -378,6 +378,22 @@ describe('findMaxWithdrawalAmount', async () => {
     expect(result.impact.newApy).toBe(0)
   })
 
+  it('full drain still returns includeMarkets details', async () => {
+    const market = makeMarket({
+      totalSupplyAssets: 500n * TOKEN,
+      totalBorrowAssets: 0n,
+      vaultSupplyAssets: 500n * TOKEN,
+      withdrawQueueIndex: 0,
+    })
+    const idle = 100n * TOKEN
+    const result = await findMaxWithdrawalAmount([market], idle, 600n * TOKEN, 10000, { includeMarkets: true })
+
+    expect(result.amount).toBe(600n * TOKEN)
+    expect(result.impact.newApy).toBe(0)
+    expect(result.impact.markets).toHaveLength(1)
+    expect(result.impact.markets[0].simulated.vaultSupplyAssets).toBe(0n)
+  })
+
   it('returns 0 when market is 100% utilized and no idle', async () => {
     const market = makeMarket({
       totalSupplyAssets: 1000n * TOKEN,

test/math.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
 
 import {
   estimateMarketApy,
+  mergeMarketDetails,
   simulateDeposit,
   simulateWithdrawal,
   weightedVaultApy,
@@ -358,3 +359,40 @@ describe('weightedVaultApyDetailed', () => {
     expect(detailed.markets[0].vaultSupplyAssets).toBe(400n * 10n ** 18n)
   })
 })
+
+describe('mergeMarketDetails', () => {
+  it('merges by marketId even when simulated order differs', () => {
+    const current = weightedVaultApyDetailed([
+      makeMarket({ marketId: '0xa' }),
+      makeMarket({ marketId: '0xb', totalBorrowAssets: 700n * 10n ** 18n }),
+    ]).markets
+    const simulated = weightedVaultApyDetailed([
+      makeMarket({ marketId: '0xa', totalSupplyAssets: 1100n * 10n ** 18n }),
+      makeMarket({ marketId: '0xb', totalSupplyAssets: 1200n * 10n ** 18n }),
+    ]).markets
+
+    const reordered = [simulated[1], simulated[0]]
+    const merged = mergeMarketDetails(current, reordered)
+
+    expect(merged).toHaveLength(2)
+    expect(merged[0].marketId).toBe('0xa')
+    expect(merged[0].simulated.vaultSupplyAssets).toBe(simulated[0].vaultSupplyAssets)
+    expect(merged[1].marketId).toBe('0xb')
+    expect(merged[1].simulated.vaultSupplyAssets).toBe(simulated[1].vaultSupplyAssets)
+  })
+
+  it('throws on duplicate simulated market IDs', () => {
+    const current = weightedVaultApyDetailed([makeMarket({ marketId: '0xa' })]).markets
+    const simulated = weightedVaultApyDetailed([makeMarket({ marketId: '0xa' })]).markets
+    const duplicated = [simulated[0], simulated[0]]
+
+    expect(() => mergeMarketDetails(current, duplicated)).toThrow(/Duplicate simulated marketId/)
+  })
+
+  it('throws when a current market is missing from simulated details', () => {
+    const current = weightedVaultApyDetailed([makeMarket({ marketId: '0xa' }), makeMarket({ marketId: '0xb' })]).markets
+    const simulated = weightedVaultApyDetailed([makeMarket({ marketId: '0xa' })]).markets
+
+    expect(() => mergeMarketDetails(current, simulated)).toThrow(/Missing simulated marketId/)
+  })
+})