fix(curve): derive unpriced legs with get_dy - #41
Conversation
optionalChildren rethrows retryable/budget child failures instead of reporting them as unpriceable, so a flaky RPC no longer switches the pool to a get_dy-derived valuation. Dust-anchor guard now measures the anchor's share of pool USD value, not its share of summed token amounts. Derivation moved to deriveMissingLegs.
|
/review |
|
Review started (review-pr): https://github.com/yearn/yearn-prices/actions/runs/32988773165 |
SummaryPR #41 extends the Curve reserve-NAV adapter so pools with one or more constituents lacking a market price can still be valued. Previously gitconfig-mask: sentinel DependenciesNo Verification
IssuesNone blocking. Suggestions
VerdictCOMMENT — implementation and tests are sound; the two notes above are non-blocking efficiency/hardening suggestions for the author to weigh. How This Was ReviewedThis review was conducted using the review-pr skill, diffed against |
|
/review-workflow |
1 similar comment
|
/review-workflow |
|
Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/32990186700 |
|
Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/32991123354 |
SummaryCurve pool pricer now derives prices for constituents the market can't price directly, by quoting each against the most valuable priced reserve ( tier: light — 4 files, ~147 net source lines, single top-level directory ( IssuesNone found. SuggestionsNone. VerdictAPPROVE How This Was ReviewedReviewed with the review-pr-workflow skill — 2 review lenses (spec-conformance, bugs), each finding independently verified by claude. 0 candidate findings were refuted and dropped. gitconfig-mask: sentinel |
|
@murderteeth 0 issues, ready. |
murderteeth
left a comment
murderteeth
left a comment
There was a problem hiding this comment.
Summary
Curve reserve-NAV now prices pools with unpriceable constituents by quoting each missing coin against the largest priced reserve via get_dy, and correctly distinguishes a transient child failure from an absent price. The transient/budget rethrow in optionalChildren is the right call and is well tested. Lint clean in changed files, tsc clean, 240 tests pass.
Issues
- src/sources/onchain/adapters/curve.ts:242 - Dust-anchor guard never fires (high) — the 1% floor is meant to refuse a pool whose priced reserve is a rounding error, but it measures the anchor's share using prices derived from that same anchor, so draining the anchor raises the computed share instead of lowering it. I simulated a two-coin stableswap (A=100, 0.04% fee) holding 1,000,000 unpriced units: as the anchor falls from 1,000,000 units to 1 unit the computed share never drops below 6.8% and ends at 32%, so the guard passes every step, while the NAV it publishes falls from ~$2.0M to ~$3.10. The existing dust test only passes because its fixture quotes 1,000,000 anchor units per coin — the guard detects over-quoting, not depletion.
- Done when: reducing the anchor reserve across a series of otherwise-identical pools never makes the accept/reject decision more permissive, and a pool holding 1,000,000 unpriced units against 5,000 $1 anchor units quoting
get_dy≈ 0.036 returns no price; a test pins the series. - Provenance: 3c41364
- Done when: reducing the anchor reserve across a series of otherwise-identical pools never makes the accept/reject decision more permissive, and a pool holding 1,000,000 unpriced units against 5,000 $1 anchor units quoting
- src/sources/onchain/adapters/curve.ts:225 - Derived NAV tracks pool imbalance (high) — a derived leg marks the pool's entire reserve of that coin at the marginal one-unit swap rate, so the published LP price follows how skewed the pool is at that block rather than any outside reference. In the same simulation the price moved 6.8× while the anchor still held 2% of its starting balance, with nothing rejecting it. Exposure scales with the derived legs' share of pool value, so a single small unpriced leg is barely affected.
- Done when: a pool published under
valuationRule: 'get-dy-derived-constituents'has a stated bound on how far its price may move as the reserve ratio changes, and a test sweeps one fake pool from balanced to heavily imbalanced and asserts the price stays inside that bound — or that no price is published for such pools. - Provenance: 3c41364
- Done when: a pool published under
Verdict
REQUEST_CHANGES
How This Was Reviewed
Reviewed with the review-pr-workflow skill —
5 review lenses, each finding independently verified by claude. 5 candidate findings were refuted and dropped.
one-unit get_dy says nothing about depth: a drained or skewed pool marks its whole unpriced reserve at a marginal rate it cannot settle, and the anchor-share guard measured that share with prices derived from the same anchor, so draining the anchor made it more permissive. each derived leg now also quotes get_dy for its entire balance and is refused unless the pool pays out at least half the marked value. that is monotone in anchor depletion and bounds published NAV at ~3x the anchor's market value. threshold 0.5 is generous for the pegged/stable pools this targets; a balanced constant-product pool sits at the knife edge. tests: anchor-drain series never flips back to accept, 1M unpriced vs 5,000 anchor at 0.036 returns no price. fake get_dy is now dx-aware.
2 participants
Summary
Curve LP reserve NAV now prices pools where one or more constituent coins have no market price. The adapter resolves the child prices it can, quotes one unit of each missing coin against the largest priced reserve with the pool's
get_dy, and keeps the existing NAV calculation when every leg is priced. Unsafe quotes (revert, zero, non-finite) return no price.A child price that fails transiently is not the same as one that does not exist.
optionalChildrenrethrowsretryableandbudgetchild failures as aRecursiveDependencyError, so a flaky RPC surfaces as UNAVAILABLE instead of quietly switching the pool to a derived valuation. Only genuinely unpriceable coins (unsupported,invalid,cycle,max-depth) feed the derivation path.The anchor carries the only market price behind every derived leg, so a pool whose anchor holds less than 1% of pool USD value (
MIN_ANCHOR_SHARE) gets no price at all. The share is measured on value, not on summed token amounts, which do not compare across coins that trade at different prices.Metadata records each derived coin, its priced anchor, the one-unit input, and the raw quote, under
valuationRule: 'get-dy-derived-constituents'.Test plan
bun installbun run typecheckbun run lintnpx vitest run --exclude test/enso.integration.test.ts(239 passed)npx vitest run(the two live Enso integration tests receive HTTP 401)retryableTwo historical spot checks were run against the first commit, before the anchor guard moved from amount share to value share. The derivation math they exercise is unchanged, but neither has been re-run since.