Skip to content

fix: normalize pool depth to ETH for path scoring#175

Merged
tvinagre merged 7 commits intomainfrom
tl/fix-depth-denomination-scoring
Apr 1, 2026
Merged

fix: normalize pool depth to ETH for path scoring#175
tvinagre merged 7 commits intomainfrom
tl/fix-depth-denomination-scoring

Conversation

@tvinagre
Copy link
Copy Markdown
Collaborator

@tvinagre tvinagre commented Apr 1, 2026

Summary

  • Normalize pool depth from raw token_in units to ETH-equivalent in DepthAndPrice::from_sim_and_derived, using TokenGasPrices already available in DerivedData
  • Fix min(depths) comparison across hops — previously meaningless when comparing different token denominations (e.g., WETH 18 decimals vs USDC 6 decimals)
  • Guard against zero numerator/denominator in token price

Problem

MostLiquidAlgorithm::try_score_path computes score = product(spot_prices) × min(depths) to prioritize which paths to simulate. Depth was stored in raw token_in units, so a WETH edge depth of 1e18 (1 WETH) appeared 10^12x larger than a USDC depth of 1e9 (1000 USDC). This caused misordered simulation priority, and with max_routes truncation or timeout, genuinely good paths could be permanently excluded.

Changes

Single file change: fynd-core/src/algorithm/most_liquid.rs

Production code:

  • from_sim_and_derived: after reading raw BigUint depth, converts to ETH using depth_eth = raw_depth * denominator / numerator from TokenGasPrices
  • Returns None (skips edge) if token price is missing or has zero numerator/denominator
  • Updated depth field doc comment and computation_requirements() comment

Tests (6 new/updated):

  • 3 existing tests updated to provide token prices in DerivedData
  • test_from_sim_and_derived_missing_token_price_returns_none
  • test_from_sim_and_derived_normalizes_depth_to_eth (integer price ratio)
  • test_from_sim_and_derived_normalizes_depth_fractional_price (fractional price ratio)

Benchmark Results (500 requests, fynd-benchmark compare)

Coverage:
  Both found route:          371
  Only main found route:       0
  Only normalized-depth:       0
  Neither found route:       129

Head-to-head (amount out before gas):
  normalized-depth wins:  65  (13.0%)
  main wins:              29  ( 5.8%)
  Ties:                  406  (81.2%)
  Avg diff:           +0.50 bps

Head-to-head (net of gas, server-side):
  normalized-depth wins:  96  (19.2%)
  main wins:              26  ( 5.2%)
  Ties:                  378  (75.6%)
  Avg diff:           +0.67 bps

Solve Time:
  main:             avg=34ms  p50=35  p95=87   p99=103
  normalized-depth: avg=34ms  p50=34  p95=88   p99=102

Route Depth:
  main avg swaps: 1.6  (max 3)
  normalized-depth avg swaps: 1.7  (max 3)
  Identical routes: 415/500

Key takeaways:

  • Zero coverage regression — no routes lost from skipping edges without token prices
  • 2:1 win ratio pre-gas (65 vs 29), improving to ~4:1 net-of-gas (96 vs 26)
  • Top outliers show +18-20 bps on multi-hop routes (WETH → small tokens) where the fix matters most
  • Solve time unchanged — normalization adds negligible overhead
  • Worst single trade: -11.9 bps; best: +19.9 bps; average consistently positive

Test plan

  • All 324 fynd-core tests pass
  • Clippy clean (-D warnings)
  • Formatting clean
  • Benchmark comparison against main (500 requests)

🤖 Generated with Claude Code

@tvinagre tvinagre force-pushed the tl/fix-depth-denomination-scoring branch from 653a7d4 to 2982e49 Compare April 1, 2026 13:11
// Look up pre-computed depth; skip edge if unavailable.
let depth = match derived
let raw_depth = match derived
.pool_depths()
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Please note that I chose to not change how pool_depth was calculated. It's still denominated in sell_token, we just normalize here for this algorithm usage.

Fixing the calculation would change a lot, as it would create another dependency layer (pool_depth needs token_prices). IMO this is fine to be done here - we can move to the derived_data layer if preferred.

Copy link
Copy Markdown
Contributor

@louise-poole louise-poole left a comment

Choose a reason for hiding this comment

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

Two minor comments, otherwise looks good! Interesting that pre-normalising results had some better paths it found... I expected with normalising we'd find the same or better always. What max_routes value did you run these benchmarks with?

@tvinagre tvinagre force-pushed the tl/fix-depth-denomination-scoring branch from 4181b4a to e5c03f6 Compare April 1, 2026 14:36
@tvinagre tvinagre enabled auto-merge April 1, 2026 14:37
tvinagre and others added 7 commits April 1, 2026 11:37
Depth values were stored in raw token_in units, making min(depths)
across hops meaningless when comparing different token denominations.
Now converts depth to ETH-equivalent using TokenGasPrices.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Existing tests need token prices in DerivedData now that depth
normalization requires them. Tests still verify the same conditions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… paths

- Replace "ETH" with "gas token" / "native token" in comments and docs
- Separate to_f64 overflow from zero-value in token price conversion
  so each failure case has a distinct trace message

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@tvinagre tvinagre force-pushed the tl/fix-depth-denomination-scoring branch from e5c03f6 to 0968040 Compare April 1, 2026 14:37
@tvinagre tvinagre merged commit 0785dcd into main Apr 1, 2026
13 checks passed
@tvinagre tvinagre deleted the tl/fix-depth-denomination-scoring branch April 1, 2026 14:38
@propellerci
Copy link
Copy Markdown

propellerci bot commented Apr 1, 2026

This PR is included in version 0.44.1 🎉

@propellerci propellerci bot added the true label Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants