feat(price): Phase 1 — Yadio provider + PriceManager wiring#753
Merged
Conversation
…ource parity)
Second atomic PR of the multi-source price rollout
(docs/PRICE_PROVIDERS.md §9, Phase 1). Wires Phase 0's foundation into the
daemon at single-source parity: Yadio behind the new abstraction, the rest
of the daemon untouched outside the call sites.
Provider:
- src/price/providers/yadio.rs: YadioProvider via GET {url}/exrates/BTC,
lenient Option<f64> parse (drops `null` / non-finite rates, preserving
the db99f94 fix), parse() helper unit-tested against a captured fixture.
- tests/fixtures/price/yadio_btc.json: captured payload exercising
USD/EUR/ARS/CUP plus the `BGN: null` regression case.
Manager:
- src/price/manager.rs: PriceManager owning Vec<Box<dyn PriceProvider>> +
PriceStore + reqwest::Client. update_all() polls each provider with its
own tokio::time::timeout, runs aggregate_tick (applying per-provider
only/except scoping at the boundary so aggregate.rs stays generic), and
writes the store; failed providers contribute nothing so prior values
survive as last-known-good (§6.4). get_price() logs a single warn! when
a value ages past one update interval but still returns it — Phase 1
never refuses an order that would have priced today (§9, enforcement
lands in Phase 4). build_provider() is the single designated
extension point (§5.4 Step 3); unknown ids in config are warn-and-skip
(forward-compat); an enabled-but-unimplemented adapter fails startup.
- Nostr publishing preserved with the legacy {"BTC": {ccy: value}}
wrapper; the `source` tag becomes the contributing provider list
(deterministically sorted), so today it's "yadio" and Phase 2 widens
it without changing the schema.
- Process-wide PriceManager::global() singleton via OnceLock; installed
in main right after settings_init().
Wiring:
- scheduler::job_update_bitcoin_prices now drives PriceManager::update_all;
interval comes from [price].update_interval_seconds, MIN_INTERVAL guard
preserved.
- util::get_bitcoin_price reads through PriceManager.
- src/bitcoin_price.rs shrunk to the shim required by §9 Phase 1
(BitcoinPriceManager::get_price delegates to PriceManager); the rest
retires in Phase 5.
Config migration (§10.1):
- When [price] is absent, synthesise_legacy_price_settings() builds a
single yadio provider from bitcoin_price_api_url +
exchange_rates_update_interval_seconds + publish_exchange_rates_to_nostr,
so existing settings.toml files keep working byte-for-byte.
- settings.tpl.toml documents the new [price] block as opt-in and marks
bitcoin_price_api_url deprecated.
Tests (+11, 365 total green):
- single-yadio tick matches today, yadio-down keeps prior values, no
providers → NoAPIResponse (§9 Phase 1 acceptance criteria).
- only-scoping enforced at the manager boundary (§6.6).
- from_settings rejects enabled-but-not-implemented (CoinGecko etc.),
ignores unknown ids, skips disabled.
- legacy migration validates; deterministic source-tag ordering.
- Yadio fixture parse + null/non-finite drop + trailing-slash URL +
parse-error surfacing.
cargo fmt, cargo clippy --all-targets --all-features, cargo test all
green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughPhase 1 price subsystem: adds a process-global PriceManager with multi-provider aggregation and optional Nostr publishing, a Yadio provider adapter and fixture, startup installation with legacy synthesis, aggregation contributor tracking, and replaces the old BitcoinPriceManager with a compatibility shim. ChangesPrice Subsystem Phase 1 Implementation
Sequence DiagramsequenceDiagram
participant Scheduler
participant PriceManager
participant YadioProvider
participant PriceStore
Scheduler->>PriceManager: update_all()
PriceManager->>YadioProvider: fetch (with timeout)
YadioProvider-->>PriceManager: ProviderQuotes
PriceManager->>PriceManager: aggregate_tick (compute contributors)
PriceManager->>PriceStore: upsert aggregated prices
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/scheduler.rs (1)
565-565: ⚡ Quick winUse the update report to surface provider failures.
At Line 565,
_reportis dropped. Please emit at least a warn/error from the returned report when no providers succeed (or when failures spike), so price-source outages are visible from scheduler logs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/scheduler.rs` at line 565, The update report returned by manager.update_all() is currently ignored (assigned to _report); change this so you capture the returned report (e.g., report) and inspect its success/failure metrics and per-provider results, then emit a log.warn or log.error from the scheduler when no providers succeeded or when failure counts spike; locate the call to manager.update_all() and the binding of _report, read fields on the returned Report (or its methods) to determine total successes/failures and provider-level failures, and add conditional logging that surfaces which providers failed and summary counts so price-source outages are visible in scheduler logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/price/manager.rs`:
- Around line 211-213: The Nostr `source` tag is being built from
report.successes (used in publish_rates_to_nostr and sources_to_tag) which can
include providers filtered out or excluded by aggregation; change the pipeline
so provider identities are those that actually contributed to the final
aggregates: modify aggregate_tick (or the aggregation code) to return
contributor metadata (e.g., a Vec of provider IDs or embed a contributors field
in the aggregate type), update callers (publish_rates_to_nostr and any place
building sources_to_tag) to use that returned contributor list instead of
report.successes, and adjust sources_to_tag to accept and format the actual
contributors; ensure function signatures (aggregate_tick,
publish_rates_to_nostr, sources_to_tag) are updated consistently and tests
updated accordingly.
- Around line 273-285: The TooStale branch currently logs on every call
(PriceError::TooStale in the match), which can spam logs; change this to a
one-shot warning by tracking whether we've already warned for that currency
(e.g., add a per-manager or per-store tracking structure like a HashSet/flag for
warned currencies), check that flag before calling warn! in the
Err(PriceError::TooStale) branch (use self.store.snapshot(currency) as now),
emit warn! only if not already warned, and set the flag after warning; also
clear/unset that flag when a fresh entry is served (where
self.store.snapshot(currency) returns a fresh entry) so future staleness can
warn again.
---
Nitpick comments:
In `@src/scheduler.rs`:
- Line 565: The update report returned by manager.update_all() is currently
ignored (assigned to _report); change this so you capture the returned report
(e.g., report) and inspect its success/failure metrics and per-provider results,
then emit a log.warn or log.error from the scheduler when no providers succeeded
or when failure counts spike; locate the call to manager.update_all() and the
binding of _report, read fields on the returned Report (or its methods) to
determine total successes/failures and provider-level failures, and add
conditional logging that surfaces which providers failed and summary counts so
price-source outages are visible in scheduler logs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c9b5471-d76c-4688-9fa5-ec90d7c76aa6
📒 Files selected for processing (10)
settings.tpl.tomlsrc/bitcoin_price.rssrc/main.rssrc/price/manager.rssrc/price/mod.rssrc/price/providers/mod.rssrc/price/providers/yadio.rssrc/scheduler.rssrc/util.rstests/fixtures/price/yadio_btc.json
…r outage log Addresses the three review findings on #753. 1. Nostr `source` tag now reflects **actual contributors**, not the broader "polled successfully" list. A provider scoped out by `only`/`except` lands in `report.successes` (it did poll OK, so the Phase 2 circuit breaker stays happy) but **not** in the new `report.contributors`, so the kind-30078 `source` tag never names a provider that didn't move the aggregate. Contributors are computed at the manager boundary from the post-scope quote maps; tracking outlier-rejected individual quotes would require pairing every `Quote` with a `ProviderId` through the pure `aggregate_tick`, which is out of scope for Phase 1 and noted as a Phase 2 follow-up. 2. `Err(PriceError::TooStale)` in `get_price` no longer logs on every call. The single `warned_currencies` HashMap (which also collided `Stale` and `SingleSource` flags for the same currency) is split into two independent `HashSet`s — `warned_stale` and `warned_single_source` — so neither flag clobbers the other. The TooStale branch now warns at most once between fresh reads; a fresh `Ok` read clears `warned_stale` so a later regression past the TTL warns again. 3. The scheduler no longer drops the `TickReport`. PriceManager already logs each provider's outcome per tick, so the scheduler only surfaces the outage condition that ops cares about: an `error!` when **every** provider failed (the store is reading last-known-good across the board), and a `warn!` summary on partial outages. Tests (+2, 367 total): - `scoped_out_provider_is_success_but_not_contributor`: a provider whose only quote is filtered by `only` appears in `report.successes` but not in `report.contributors`. - `stale_warning_is_one_shot_then_re_arms_on_fresh_read`: 10 reads against a stale value populate `warned_stale` with one entry; a fresh tick + read clears it so future regressions warn again. cargo fmt, cargo clippy --all-targets --all-features, cargo test all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Actionable comments posted: 0 |
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes: the source metadata is built from providers that merely returned non-empty quotes before aggregation. That means a provider whose quotes are later discarded as outliers can still be advertised as a contributor in the Nostr source tag. Please derive contributors from the quotes that actually survive aggregation, or have aggregate_tick return provenance so the tag reflects reality.
Addresses the Hermes review on #753: the prior fix computed contributors at the manager boundary as "post-scope non-empty quotes", which still claimed a provider as contributor when `combine`'s outlier filter dropped its value. The Nostr `source` tag advertised providers that had no effect on the aggregate. Threads provenance through the pure-function aggregation core instead: - `aggregate_tick` now takes `&[(ProviderId, ProviderQuotes)]`. Direct (PerBtc) quotes and resolved PerBase quotes carry their source id through the pipeline. - `AggregateResult` gains `contributors: Vec<ProviderId>` — sorted, deduplicated, and populated by the new `kept_contributors` helper, which mirrors `combine`'s "kept" predicate exactly: - n≤2: every clean provider contributes, - n≥3: only providers whose value is within `outlier_pct` percent of the median contribute, - bimodal even-length fallback: every clean provider stays (no single source is demonstrably the outlier; `combine` falls back to the median itself). - Fiat-cross resolution attributes the resolved candidate to the fiat-cross provider only. Anchor contributors (the direct quoters whose values built the USD/BTC anchor, say) are an intermediate of the cross math, not upstreams of the cross currency. - Manager derives the tick-wide Nostr `source` tag as the union of every per-currency contributor list — using a BTreeSet so the result is deterministic without re-sorting in `sources_to_tag`. The manager-boundary "non-empty post-scope" heuristic is gone. - `ProviderId` gains `Ord` / `PartialOrd` so the sorted contributor lists are stable. Tests (+3, 370 total): - `aggregate_tick_outlier_drops_provider_from_contributors`: three providers, one outlier (75_000 against a 50_000/50_200 median) — the outlier appears in `sources=3` but NOT in `contributors`. - `aggregate_tick_bimodal_fallback_keeps_all_clean_contributors`: four values across two clusters land in the bimodal fallback; all four providers stay contributors. - `aggregate_tick_non_finite_value_drops_provider_from_contributors`: a NaN-emitting provider is dropped from the cleaned set, not advertised as a contributor. Existing aggregate_tick tests assert on the new contributors field; the unions-partial-coverage test pins the CUP contributor list to [Yadio (direct), ElToque (fiat-cross)] — confirming the anchor's own contributors are NOT propagated into the resolved-currency tag. cargo fmt, cargo clippy --all-targets --all-features, cargo test all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/price/aggregate.rs (1)
470-493: ⚡ Quick winAssert the exact contributor list in the bimodal fallback test.
len() == 4won't catch a regression where this path returns the right set in nondeterministic order. Sincecontributorsis part of the contract now, it's worth pinning the full sorted vector here too.🧪 Tighten the assertion
approx(out["USD"].value, 51.0); // (2+100)/2 - // All four clean providers survive the bimodal fallback. - assert_eq!(out["USD"].contributors.len(), 4); + // All four clean providers survive the bimodal fallback, in deterministic order. + let mut expected = vec![ + ProviderId::Yadio, + ProviderId::CoinGecko, + ProviderId::CurrencyApi, + ProviderId::Blockchain, + ]; + expected.sort(); + assert_eq!(out["USD"].contributors, expected);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/price/aggregate.rs` around lines 470 - 493, The test aggregate_tick_bimodal_fallback_keeps_all_clean_contributors should assert the exact contributor list rather than just contributors.len(); replace the loose length check with a deterministic equality check of the sorted contributors vector so nondeterministic ordering can't hide regressions: extract the provider identifiers from out["USD"].contributors, sort them deterministically (e.g. by provider enum/name), and assert they equal the expected list [ProviderId::Yadio, ProviderId::CoinGecko, ProviderId::CurrencyApi, ProviderId::Blockchain]; update the test around aggregate_tick and the contributors field to perform that exact sorted equality check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/price/aggregate.rs`:
- Around line 470-493: The test
aggregate_tick_bimodal_fallback_keeps_all_clean_contributors should assert the
exact contributor list rather than just contributors.len(); replace the loose
length check with a deterministic equality check of the sorted contributors
vector so nondeterministic ordering can't hide regressions: extract the provider
identifiers from out["USD"].contributors, sort them deterministically (e.g. by
provider enum/name), and assert they equal the expected list [ProviderId::Yadio,
ProviderId::CoinGecko, ProviderId::CurrencyApi, ProviderId::Blockchain]; update
the test around aggregate_tick and the contributors field to perform that exact
sorted equality check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cffa729e-e165-4367-99f6-43c14cec6b3f
📒 Files selected for processing (4)
src/price/aggregate.rssrc/price/manager.rssrc/price/provider.rssrc/price/store.rs
✅ Files skipped from review due to trivial changes (2)
- src/price/provider.rs
- src/price/store.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/price/manager.rs
Replace the loose `.len() == 4` check with the deterministic sorted-list assertion. `kept_contributors` -> `dedup_sort` orders by the derived `Ord` on `ProviderId` (which follows enum-variant declaration order: Yadio, CoinGecko, CurrencyApi, Blockchain, ElToque), so the expected result is fully determined. A future refactor that perturbs ordering or silently drops a contributor will now be caught here rather than slipping past the count. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
|
tACK for me! One detail, maybe it's intended, but from specs seems not. Now we still have to leave the legacy |
Contributor
|
Checked out the branch locally and ran all four smoke tests from the test plan against a live stack (bitcoind regtest + LND + nostr-rs-relay + local Mostro daemon). 55 price tests pass, zero failures. Smoke test 1: legacy config (no
|
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed on current head c391414. I re-checked the previously raised review points and the ones that were still relevant do look addressed on this head: the Nostr source tag now uses actual aggregate contributors instead of all successful providers, and stale-price warnings are guarded as one-shot transitions instead of spamming on every read. I also read through the PR discussion / comments to make sure there were no unresolved change requests left behind. Local validation on this head passed: cargo test price -- --nocapture and cargo clippy --all-targets --all-features -- -D warnings both completed successfully.
…adio URL Human-review follow-ups on Phase 1 (spec §10.1 backward compatibility): - types.rs: add #[serde(default)] to bitcoin_price_api_url so a settings.toml that has migrated to a [price] block may omit the deprecated key instead of failing deserialization and aborting startup (arkanoider, codaMW). Field stays — util.rs and install_price_manager() still read it until Phase 4/5. - settings.tpl.toml: document the key's full lifecycle (now optional, still read by the live /convert path until Phase 4, removed in Phase 5). - util.rs: route the live /convert + /currencies path through a yadio_base_url() helper that prefers [price.providers.yadio].url when a [price] block is present, falling back to the legacy key. Stops the live and cached paths silently hitting different Yadio bases when only the new key is customised. - config.rs / manager.rs: point the three [price] validation errors at docs/PRICE_PROVIDERS.md §7 and make the unimplemented-provider message actionable. - tests: prove a [mostro] block without bitcoin_price_api_url deserializes to the default URL, and that legacy synthesis is identical whether the key is present-at-default or omitted. Does not touch price/aggregate.rs, price/store.rs, or the scheduler tick (§5.4 extension-contract invariant holds). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Strict review after reading the discussion and prior commits:
- The legacy/live Yadio URL helper can emit broken URLs. It prefers the
[price]Yadio URL whenever that section exists, even if the provider is disabled or blank, and it only callstrim(), nottrim_end_matches('/'). That means a configured trailing slash turns into//convert///currencies, while a disabled or empty Yadio config can suppress the legacy fallback entirely. Please gate on a usable URL and normalize it the same wayYadioProvider::newdoes.
ermeme review on PR #753: the live-path Yadio URL helper could emit broken URLs. It only `trim()`med (not `trim_end_matches('/')` like `YadioProvider::new`), so a configured trailing slash produced `//convert` / `//currencies` and diverged from the aggregate path the helper exists to match. It also used the `[price.providers.yadio]` URL even when the provider was disabled, overriding the legacy `bitcoin_price_api_url` fallback. - Normalize the chosen URL with `trim_end_matches('/')`, matching `YadioProvider::new` (applied to the legacy fallback too). - Use the provider URL only when *usable*: enabled and non-empty after normalization; otherwise fall back to the legacy key. Split the selection out into a pure `select_yadio_base_url` so it is unit testable without the write-once global `Settings`; add tests for the prefer-enabled, strip-trailing-slash, and fall-back-when-unusable cases. (The review's "empty config suppresses the fallback" point did not hold — the existing `!url.is_empty()` guard already fell through — but the trailing-slash and disabled-provider issues were real.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 11, 2026
grunch
added a commit
that referenced
this pull request
Jun 12, 2026
…skew (#774) PR #753 removed the BITCOIN_PRICES static when it migrated price reads to the PriceManager, while PR #770 (merged independently) added BitcoinPriceManager::set_price_for_test writing to that static — the combination doesn't compile under cargo test on main. Re-point the test seam at a small cfg(test) override map consulted by price::get_bitcoin_price before the global manager, so unit tests keep seeding deterministic prices without installing the global PriceManager (whose OnceLock would leak one test's configuration into the rest of the binary). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
grunch
added a commit
that referenced
this pull request
Jun 15, 2026
* fix(price): repair test-only price seeding broken by #753/#770 merge skew PR #753 removed the BITCOIN_PRICES static when it migrated price reads to the PriceManager, while PR #770 (merged independently) added BitcoinPriceManager::set_price_for_test writing to that static — the combination doesn't compile under cargo test on main. Re-point the test seam at a small cfg(test) override map consulted by price::get_bitcoin_price before the global manager, so unit tests keep seeding deterministic prices without installing the global PriceManager (whose OnceLock would leak one test's configuration into the rest of the binary). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(bond): Phase 7 — maker timeout slash Fill the maker-responsible rows of the §9.2 responsibility table: a waiting-state timeout now slashes the maker's bond when the maker is the responsible party, gated per posting role (apply_to must cover the responsible side — taker since Phase 4, maker since Phase 7). - slash_or_release_on_timeout resolves the responsible bond through the range-aware resolve_slash_target (the maker bond of a range order lives on the range root), and gates the slash on applies_to_maker() / applies_to_taker() per the responsible side. - A maker-responsible timeout on a range order goes through the Phase 6 partial-slash path: record_maker_slice_slash inserts a proportional child row with reason=Timeout and the parent HTLC stays Locked; the single settle happens at range close. The helper now returns whether it inserted, so the BondSlashed notice fires exactly once (a scheduler retry that finds the child already recorded reports None). - The scheduler's terminal cancel branch now runs resolve_range_maker_bond_at_close_or_warn after the Canceled status persists, so a slashed range settles and distributes promptly instead of waiting for the 5-minute reconciliation sweep (which remains the backstop). The republish branch never closes — the maker stays committed there. - The timeout release loop retains a range maker parent bond (resolved only at range close), alongside the existing republish carve-out. Tests mirror Phase 4 from the maker side: non-range sell/buy slashes, the per-role apply_to gate, and the range path (proportional child, parent stays Locked, root resolution from a descendant slice, per-slice idempotency without re-notification). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(bond): mark Phase 7 shipped + implementation notes; fix stale Phase 4.5 row Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second atomic PR of the multi-source price rollout (
docs/PRICE_PROVIDERS.md§9, Phase 1). Builds on Phase 0 (#747).PriceManagerowns the provider registry + store + HTTP client; the scheduler'sjob_update_bitcoin_pricesnow drivesPriceManager::update_all, andutil::get_bitcoin_pricereads through it.src/price/providers/yadio.rs(GET{url}/exrates/BTC, lenientOption<f64>parse sonullrates are dropped — preserves the d3f5bfa fix). Captured-payload fixture + parse tests.[price]is absent we synthesise a single-yadio config from[mostro].bitcoin_price_api_url+exchange_rates_update_interval_seconds+publish_exchange_rates_to_nostr, so existingsettings.tomlfiles keep working byte-for-byte.src/bitcoin_price.rsshrunk to the §9 Phase 1 shim (BitcoinPriceManager::get_pricedelegates toPriceManager); 423 lines removed, the rest retires in Phase 5.get_pricelogs a singlewarn!when a value ages past one update interval but still returns it — Phase 1 never refuses an order that would have priced today. Enforcement turns on in Phase 4.What is NOT in this PR
get_market_quoteunification + staleness enforcement — Phase 4.bitcoin_price.rsremoval — Phase 5.Single designated extension point (§5.4 Step 3)
build_provideris the single match arm a new provider touches; the aggregation core, the store, and the scheduler stay untouched. An enabled-but-unimplemented provider fails startup; an unknown id is warn-and-skipped (forward-compat with newer config from an older binary).Test evidence
NoAPIResponsematching today's "no data yet".only/exceptscoping (§6.6) enforced at the manager boundary.BGN: nullregression case.source-tag ordering across ticks.cargo fmt,cargo clippy --all-targets --all-features,cargo test(365 passing) all green.Test plan
cargo fmt --check && cargo clippy --all-targets --all-features -- -D warnings && cargo testgreen on CIsettings.toml(no[price]block) — confirm logs showPriceManager installed,price: yadio ok (N currencies)each tick, and orders that exerciseget_bitcoin_price(market-priced create) succeed[price.providers.yadio]block — same behaviouriptablesblock on api.yadio.io) — confirm the store keeps serving last-known-good values,warn!fires once the value ages past one update interval, and orders still succeed (Phase 1 logs but does not refuse)sourcetagyadio(now derived from the contributing-providers list, not hard-coded)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Bug Fixes / Improvements
Documentation
Tests