feat(price): Phase 4 — unify live quote path on cache + enforce staleness

[grunch]

Phase 4 of docs/PRICE_PROVIDERS.md

Unifies the live market-quote path onto the multi-source cache and turns on staleness (TTL) enforcement. Depends on mostro-core 0.13.1 (MostroP2P/mostro-core#153), which adds ServiceError::PriceTooStale and CantDoReason::PriceTooStale.

What changed

Live path → cache

• get_market_quote (util.rs) is now a synchronous cache read — sats = (fiat_amount / get_price(ccy)) × 1e8 with the premium applied — instead of a live Yadio /convert call (with 4 retries + blocking thread::sleep) on every take. No network I/O on the take path.
• Removed the dead live-path code: retries_yadio_request, yadio_base_url / select_yadio_base_url / normalize_base_url, MAX_RETRY, FiatNames, and the Yadio/Request response model (src/models.rs deleted). With the live path gone, the legacy [mostro].bitcoin_price_api_url now feeds only legacy synthesis.
• get_market_amount_and_fee is now sync and returns MostroError so callers can distinguish the stale case.

Staleness enforcement (spec §6.4)

• PriceManager::get_price returns ServiceError::PriceTooStale for a value older than max_price_staleness_seconds (still warning once on the transition), instead of serving stale data.
• Order create (app/order.rs) and market-priced takes (app/take_buy.rs, app/take_sell.rs) map it onto a user-facing CantDoReason::PriceTooStale.

Test evidence

• New unit test stale_price_is_refused_but_fresh_is_served: past-TTL → PriceTooStale, within-TTL → served, never-seen currency → NoAPIResponse.
• Full suite green (488 tests), clippy --all-targets --all-features -D warnings clean, cargo fmt applied.

---

How to test manually

Prereqs: an LND/regtest mostrod setup as usual, with a [price] section enabled in settings.toml.

1. Happy path — market-priced order prices from the cache (no live /convert)

1. Start mostrod. Wait for the first price tick (price: ... ok (N currencies) in the logs).
2. Create a market-priced order (amount 0) in a fiat like USD, and take it.
3. Confirm the sats amount looks right for the current rate, and that no GET .../convert/... request is made during create/take (the old live call is gone). The only price HTTP traffic is the scheduler's periodic provider polls.

2. Staleness refusal — past the TTL

1. Set a tiny window in settings.toml:

   [price]
   update_interval_seconds = 300
   max_price_staleness_seconds = 30

2. Start mostrod, let it fetch once, then block provider refreshes so the cached value ages out — e.g. disable the providers or cut network to the price APIs (firewall/airplane) while keeping the daemon running.
3. After > 30s with no fresh tick, try to create a market-priced order (or take one) in a currency that aggregated earlier.
   • Expected: the request is refused with CantDoReason::PriceTooStale (client shows a "price too stale" cant-do), not a priced order.
   • Logs show price: <CCY> is past the staleness window (<n>s old) — refusing once (one-shot warning), not per read.
4. Fixed-amount (non-market) orders in the same currency are unaffected — they don't price off the cache.

3. Recovery — fresh tick re-arms

1. Restore provider connectivity.
2. After the next successful tick, retry the create/take from step 2 — it should now succeed.
3. A subsequent slide past the TTL warns again (the guard re-armed on the fresh read).

4. Currency with no data

1. Request a market-priced order in a fiat no provider reports.
2. Expected: refused (internal NoAPIResponse → existing behaviour), distinct from the stale path.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Market quotes are now computed from cached exchange-rate data, making pricing faster and more consistent.
  • When pricing is too old, buy/sell flows now explicitly return a “price too stale” response so clients can retry with fresh pricing.

• Bug Fixes

  • Stale pricing is rejected past the allowed TTL instead of being used.

• Documentation

  • Rollout documentation updated for Phase 3/4 status and the new staleness/legacy behavior, including known bonded-takes gaps.

[coderabbitai]

Walkthrough

Phase 4 switches market pricing to cached reads, refuses stale prices past TTL, propagates PriceTooStale through order and take flows, removes legacy Yadio quote code, and updates the rollout documentation.

Changes

Phase 4 Price Staleness Enforcement

Layer / File(s)	Summary
PriceManager::get_price staleness enforcement src/price/manager.rs	Price refusal past TTL now returns ServiceError::PriceTooStale, uses a separate refusal warning guard, clears both guards on fresh reads, and is covered by updated and new tests.
Synchronous cached market quote src/util.rs, src/main.rs	The async Yadio /convert quote path is removed; get_market_quote now computes sats from cached price plus premium, get_market_amount_and_fee is synchronous, and crate::models is no longer publicly exported.
PriceTooStale propagation at call sites src/app/order.rs, src/app/take_buy.rs, src/app/take_sell.rs	Order create and market-price take handlers map stale cached pricing to MostroCantDo(CantDoReason::PriceTooStale) instead of the prior generic error path.
Docs shipped-status update docs/PRICE_PROVIDERS.md	Phase 3 is marked done, and Phase 4 documentation now reflects the shipped cached-quote flow, stale-price refusal, and shipped core error variants.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant OrderFlow
  participant PriceManager
  Client->>OrderFlow: create/take market-priced order
  OrderFlow->>PriceManager: get_price()
  PriceManager-->>OrderFlow: price or PriceTooStale
  OrderFlow-->>Client: accept or CantDoReason::PriceTooStale

Loading

Possibly related PRs

• MostroP2P/mostro#499: Shares the legacy market-quote and Yadio conversion code in src/util.rs.
• MostroP2P/mostro#745: Updates the same phased price-provider rollout documentation.
• MostroP2P/mostro#753: Introduces the price-module groundwork this PR builds on.

Suggested reviewers: codaMW, arkanoider, Catrya, AndreaDiazCorreia

Poem

A nibble of cache, a hop through the rate,
Stale sats are refused at the TTL gate.
This bunny says “fresh” with a twitch of the nose,
And market quotes land where the warm bitcoin grows.
🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: moving quotes to cache-backed reads and enforcing price staleness in Phase 4.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/price-phase4-unify-staleness

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2397684c18

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Recheck market quotes when taker bonds lock

When taker bonds are enabled, this call snapshots order.amount and fees before creating the bond; request_taker_bond stores those values in the bond's taker_* columns, and promote_taker_context_to_order later copies them when the invoice is accepted. If the taker pays the bond after max_price_staleness_seconds has elapsed, the trade still proceeds with that old market quote instead of returning PriceTooStale, so bonded market-priced takes can bypass the Phase 4 staleness guarantee. Revalidate/reprice before promoting the bond, or expire/reject stale bond snapshots.

Useful? React with 👍 / 👎.

[grunch]

Confirmed — and reproduced end-to-end by @codaMW (see his review). Agreed with his assessment that the fix (revalidating or expiring the bond's quote snapshot at lock time) belongs to the bond flow rather than this price PR, so it stays out of scope here and will be tracked as a follow-up issue. In the meantime, 0ab4750 documents in docs/PRICE_PROVIDERS.md that the Phase 4 staleness guarantee currently applies to the direct (bond-less) path only.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/util.rs (1)

1071-1081: ⚡ Quick win

Add /// docs for get_market_amount_and_fee

This public helper returns a tuple with positional semantics; add a short doc comment describing tuple order and error behavior.

As per coding guidelines: "Document non-obvious public APIs with /// doc comments."

🤖 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/util.rs` around lines 1071 - 1081, Add `///` doc comments to the public
function `get_market_amount_and_fee` to document its non-obvious behavior.
Document each parameter (fiat_amount, fiat_code, and premium), clearly explain
the return tuple's positional semantics (first element is satoshi amount, second
element is the fee), and document the error conditions that may result in
MostroError. Place the doc comments immediately before the function signature.

Source: Coding guidelines

🤖 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 401-417: The `warned_stale` state is currently shared between two
different warning paths: the within-TTL stale warning in the `observe_freshness`
method (around line 448) and the past-TTL warning in the current block (around
line 401). This causes the past-TTL transition warning to be suppressed if the
within-TTL stale warning has already been issued for the same key. To fix this,
separate the warning state by creating two distinct warning trackers instead of
sharing one `warned_stale` variable - one specifically for tracking within-TTL
stale warnings and another for tracking past-TTL refused warnings. Update the
`mark_warned` calls to use the appropriate warning state based on the context.

---

Nitpick comments:
In `@src/util.rs`:
- Around line 1071-1081: Add `///` doc comments to the public function
`get_market_amount_and_fee` to document its non-obvious behavior. Document each
parameter (fiat_amount, fiat_code, and premium), clearly explain the return
tuple's positional semantics (first element is satoshi amount, second element is
the fee), and document the error conditions that may result in MostroError.
Place the doc comments immediately before the function signature.

🪄 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: fa0736ae-9e26-4d39-82b2-e0665d10ac42

📥 Commits

Reviewing files that changed from the base of the PR and between 82f1923 and 2397684.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (9)

• Cargo.toml
• docs/PRICE_PROVIDERS.md
• src/app/order.rs
• src/app/take_buy.rs
• src/app/take_sell.rs
• src/main.rs
• src/models.rs
• src/price/manager.rs
• src/util.rs

💤 Files with no reviewable changes (2)

• src/models.rs
• src/main.rs

[codaMW]

Reviewed on regtest against mostro-core 0.13.1. The core Phase 4 change is sound, I verified the mechanics, demonstrated the staleness enforcement live, and dug into the two open bot findings. One (the bonded-take bypass) is real and I was able to reproduce it end-to-end; details below. Not blocking from my side, but worth a decision on scope before merge.
Mechanics
• Builds clean against mostro-core 0.13.1 (Rust 1.93); cargo clippy --all-targets --all-features -- -D warnings clean.
• cargo test: 487 pass. The one failure is test_lnurl_validation_with_test_server (AddrInUse), a pre-existing port-bind flake unrelated to this PR (fails on main too).
• The new stale_price_is_refused_but_fresh_is_served unit test passes.
Staleness enforcement verified live. With a [price] block (max_price_staleness_seconds set low) and the cache aged past the window, the daemon refused market-priced requests with price: is past the staleness window (s old) refusing and returned CantDoReason::PriceTooStale. So the core guarantee works at runtime, not just in the unit test.
Config gate worth flagging. Phase 4 enforcement only engages when a [price] block is present; on a legacy config (no [price]), the daemon runs the legacy synthesis path and market orders proceed regardless of cache age (confirmed at runtime orders on a no-[price] config were not refused). Suggest documenting that Phase 4 protection requires migrating to a [price] block, or consider a sensible default guard for legacy configs.
On the Codex P2 finding (bonded takes bypassing staleness) confirmed by code trace AND reproduced end-to-end. Code path: take_buy runs the staleness check in get_market_amount_and_fee (take_buy.rs:122), then request_taker_bond snapshots the quote into bond.taker_amount (bond/flow.rs:183). When the bond locks, promote_taker_context_to_order copies that snapshot into the order (order.amount = bond.taker_amount, bond/flow.rs ~1069) with no re-pricing or staleness re-check. Reproduced with two funded regtest LND nodes + a mobile-app taker, apply_to = take, max_price_staleness_seconds = 60:

1. Took a market-priced order while the price was fresh -> bond snapshotted taker_amount = 33196.
2. Blocked the price provider so the cache aged past the window (direct market takes were refused with price_too_stale throughout the stale period).
3. ~10 min into the stale window, paid the bond hold invoice -> daemon logged Bond … locked, promoted the order to waiting-payment with amount = 33196, and issued the trade invoice for that amount, no staleness re-check, despite the price being stale the entire time. So a bonded market take completes on a pre-TTL quote that the direct path refuses. Scope: only bites with bonds enabled and a delayed bond payment, and the fix (revalidate/expire the snapshot before promoting) belongs to the bond flow rather than this price PR. I'd treat it as a tracked follow-up, not a blocker but the Phase 4 guarantee should probably be documented as direct path only until the bonded path re-checks.
   On the CodeRabbit warned_stale finding confirmed, minor/logging-only. The past-TTL refuse path (manager.rs:401) and the within-TTL stale path (manager.rs:448) both call mark_warned(&self.warned_stale, key) on the same set, so a within-TTL warning already recorded for a key suppresses the past-TTL refusing log line. The refusal itself still fires (the Err(PriceTooStale) returns regardless) only the operator warning can be swallowed. Non-blocking; separate trackers would fix it.
   Housekeeping: the branch currently shows merge conflicts against main needs a rebase before merge.
   Net: the Phase 4 core is solid and I'm comfortable with it landing after the rebase. The bonded-take staleness gap is real but reasonably a follow-up rather than a blocker for this PR. Nice deletion of the whole live-Yadio retry path.

[arkanoider]

Tested manual path:

• Point 1

  • Created a sell order and taked ( only yadio as provider ) --> no http request anymore
  • Created a buy order and taked ( only yadio as provider ) --> no http request anymore

• Point 2

  • Launched mostro with max_price_staleness_seconds = 30 turned off wifi and waited 30 secs.
  • Created an order with result:

mostrod::price::manager: price: EUR is past the staleness window (244s old) — refusing
mostrod::util: sender key acf3c926f37102b03a5ff8a83fa4480af59452e47d84e15513e5adfa6c2aac83 - receiver key 4b8bc3eee9adda0d47c81fe5152a010f3e1b1f7d007510a3c9f
INFO mostrod::util: Sending message, Event ID: af666058ac36afcbff94c1a3dac48dcda4acfd5133601fc42850d8921df8e3b5 to ea4dddaa1fc4c4b8bc3eee9adda0d47c81fe5152a010f3e1b1f7d007510a3c9f with payload: "{\"cant-do\":{\"version\":2,\"request_id\":10989935784236698400,\"trade_index\":null,\"action\":\"cant-do\",\"payload\":{\"cant_do\":\"price_too_stale\"}}}"

• Point3

  • wifi turned on, waited to be in the not stale window, created an order and works

• Point4

  • Order refused with no valid currency

[arkanoider]

tACK ( after conflicts fixing )

[grunch]

Addressed the outstanding review feedback in 36409bf…0ab4750:

• CodeRabbit — shared warned_stale state: fixed in 36409bf. Split into warned_stale (within-TTL warning) and warned_refused (past-TTL refusal warning), with a regression test. Confirmed it was logging-only, as @codaMW noted.
• CodeRabbit nitpick — missing docs on get_market_amount_and_fee: added in 855ee12 (tuple order + error behaviour).
• Codex P2 — bonded takes bypass the staleness check: agreed with @codaMW's call that the fix belongs to the bond flow (revalidate/expire the quote snapshot at bond lock), so it's out of scope here and goes to a follow-up issue. Documented in docs/PRICE_PROVIDERS.md (0ab4750) that the Phase 4 guarantee is direct-path-only for now.
• @codaMW — config gate on legacy configs: clarified in the docs (0ab4750). At code level enforcement does engage without a [price] block — install_price_manager synthesises the legacy config and PriceManager::get_price enforces the default 1800 s TTL. What a legacy config can't do is tune the TTL (the old [mostro] keys carry no staleness setting), which is likely why a shorter outage wasn't refused in your run. Refusals on a no-[price] config kick in once the cache is > 30 min old.
• Broken tests after the merge from main: the merge reintroduced three unit tests for select_yadio_base_url, which this PR deletes — the branch didn't compile under cargo test. Removed them in 855ee12.

Full suite green locally: 501 passed, clippy clean.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/util.rs (1)

39-70: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Flip the premium sign in get_market_quote The shared helper applies 1 - premium/100, so a positive premium lowers the sats quote. Since both take-buy and take-sell use this path and the docs/tests describe premium as markup, this should be 1 + premium/100 for positive premiums; negative premiums can stay as discounts.

🤖 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/util.rs` around lines 39 - 70, The premium handling in get_market_quote
is using the wrong sign, causing positive premiums to reduce the sats quote
instead of increasing it. Update the premium adjustment in get_market_quote so a
positive premium acts as a markup (and negative values still act as discounts),
and keep the change localized to the existing quote calculation path used by
take-buy and take-sell.

🤖 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.

Outside diff comments:
In `@src/util.rs`:
- Around line 39-70: The premium handling in get_market_quote is using the wrong
sign, causing positive premiums to reduce the sats quote instead of increasing
it. Update the premium adjustment in get_market_quote so a positive premium acts
as a markup (and negative values still act as discounts), and keep the change
localized to the existing quote calculation path used by take-buy and take-sell.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dec73935-6868-48cf-af1f-ce8a1e15a64d

📥 Commits

Reviewing files that changed from the base of the PR and between 935a0db and 0ab4750.

📒 Files selected for processing (3)

• docs/PRICE_PROVIDERS.md
• src/price/manager.rs
• src/util.rs

✅ Files skipped from review due to trivial changes (1)

• docs/PRICE_PROVIDERS.md

🚧 Files skipped from review as they are similar to previous changes (1)

• src/price/manager.rs