feat(price): Phase 3 — El Toque fiat-cross provider (CUP/MLC)#778
Conversation
…ional wiring
Implements Phase 3 of docs/PRICE_PROVIDERS.md (§9, §11.3): the El Toque
provider, a fiat-cross quoter that contributes CUP and MLC as
`Quote::PerBase { base: "USD" }`, resolved against the aggregated USD/BTC
anchor by Phase 0's existing `resolve_per_base` (no aggregation-core change).
Per §5.4 this is one adapter file + one registry arm + one config block +
one fixture — the aggregation core, store, scheduler and handlers are
untouched.
- src/price/providers/eltoque.rs: ElToqueProvider. Parses El Toque's
CUP-denominated `tasas` payload and emits:
CUP -> PerBase{USD, cup_per_usd}
MLC -> PerBase{USD, cup_per_usd / cup_per_mlc} (§11.3 cross math)
Bearer-token auth (required when enabled -> startup error otherwise,
§7); token redacted from Debug/logs (§10.3). Anchor = USD only (the
§11.3 EUR-fallback question was declined for this phase).
- manager.rs: registry arm builds El Toque (fails fast without a token);
the old "unimplemented" rejection and its test are replaced with
with-token / without-token coverage.
- settings.tpl.toml: real [price.providers.eltoque] block (enabled=false).
- docs/PRICE_PROVIDERS.md: Phase 3 marked in review; §11.3 status note.
PROVISIONAL: the tasas API is token-gated, so a real payload could not be
captured. The parse path is grounded in the confirmed CUP-denominated
`tasas` shape and is fully unit-tested; the request line in `fetch` (path
/v1/trmi, GET, no date params) is best-effort from third-party
reverse-engineering and the shipped fixture is a representative sample,
not a capture. All clearly flagged in comments and the spec — to be
finalised against the official docs in a follow-up. Keep enabled=false in
production until then.
Tests: 8 new El Toque adapter tests + updated registry tests. Full suite
472 passed; cargo clippy + fmt clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughAdds a new ChangesElToque Provider Adapter
Sequence Diagram(s)sequenceDiagram
participant PriceManager
participant ElToqueProvider
participant reqwest as reqwest::Client
participant API as El Toque API
rect rgba(70, 130, 180, 0.5)
note over PriceManager,ElToqueProvider: Startup — build_provider
PriceManager->>ElToqueProvider: new(cfg) — validate token, normalize URL
ElToqueProvider-->>PriceManager: Ok(ElToqueProvider) or Err(missing token)
end
rect rgba(60, 179, 113, 0.5)
note over PriceManager,API: Runtime — fetch cycle
PriceManager->>ElToqueProvider: fetch(http)
ElToqueProvider->>reqwest: GET {url}/v1/trmi + Authorization: Bearer <token>
reqwest->>API: HTTP GET /v1/trmi?date_from=…&date_to=…
API-->>reqwest: 200 {tasas:{USD,MLC,...}}
reqwest-->>ElToqueProvider: response body
ElToqueProvider->>ElToqueProvider: parse — anchor USD→CUP, derive cup_per_usd/cup_per_mlc for MLC
ElToqueProvider-->>PriceManager: ProviderQuotes([CUP, MLC])
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
codaMW
left a comment
There was a problem hiding this comment.
Tested on regtest exercised everything that doesn't depend on the live (token-gated) El Toque endpoint, per the manual-testing notes. The grounded parts check out; ACK on that basis, with the provisional request-wiring caveat below.
Build + tests
• Builds clean.
• The 10 El Toque tests pass: parse → CUP/MLC PerBase emission, the MLC cross-math (cup_per_usd / cup_per_mlc), no-USD-anchor → nothing, non-positive dropped, parse error, token-required, Debug redaction, trailing-slash strip, plus the two registry arms (builds with token / rejects without).
• Full suite: 471 pass. The one failure is test_lnurl_validation_with_test_server (AddrInUse / port bind) Lightning subsystem, untouched here, fails identically on main pre-existing local-env flake, not from this PR.
Live tick against a local mock (Scenario 2 + 5)
Served the shipped fixture at /v1/trmi via python3 -m http.server, with Yadio enabled for the USD anchor:
price: yadio ok (127 currencies)
price: eltoque ok (2 currencies)
price: published exchange rates to Nostr (123 currencies)
So El Toque polls, parses, emits exactly CUP + MLC, and they resolve against the aggregated USD/BTC anchor the PerBase path works end-to-end, stable across ticks. Scoping holds (only the 2 currencies, not USD/ECU/crypto from the tasas payload).
Secret redaction (Scenario 5): I ran with token = "SECRET123" and grepped the full log the token appears 0 times. Debug redaction holds at runtime, matching the debug_redacts_token unit test.
Provisional wiring (acknowledged, not a blocker)
The parse path is grounded and well-tested; the fetch request line (path /v1/trmi, GET, params) is the reverse-engineered part that can't be verified without the token-gated API. That's clearly flagged in the module comment + docs §11.3, the template ships enabled = false, and per the description it was agreed before implementation. So I'm treating it as a known, safely-gated limitation rather than a defect the follow-up to confirm the request against the official docs + drop in a real fixture should land before anyone enables it in production, as the PR states.
Nice touch: the parser being lenient on individual null rates (Option) so one bad entry can't fail the whole poll.
Apply the El Toque API details confirmed with the maintainer, finalising the previously-provisional request wiring: - fetch now sends the required [date_from, date_to] range: GET /v1/trmi?date_from=…&date_to=… (YYYY-MM-DD HH:MM:SS, URL-encoded by reqwest), Bearer auth unchanged. The endpoint returns the most recent rate within the range, so we query a rolling 48h window ending "now" (LOOKBACK_HOURS) — wide enough to absorb the ~daily TRMI update cadence and UTC↔Cuba skew without returning empty tasas. - tests/fixtures/price/eltoque_trmi.json: replaced the representative sample with a real captured response. This also corrects the timestamp shape — hour/minutes/seconds are separate integers, not a "HH:MM:SS" string (the parser ignores them either way). - Dropped the PROVISIONAL/reverse-engineering language from the module doc and docs/PRICE_PROVIDERS.md; documented the confirmed request and response shapes. Tests updated to the captured values (USD=490, MLC=200 → MLC/USD = 490/200). Q1/Q3 (§11.3 maintainer questions) remain open; provider stays enabled=false until a token is provisioned. Tests: full price suite 91 passed; cargo clippy --all-targets + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The /v1/trmi endpoint rejects any [date_from, date_to] range of 24h or more with `400 "El intervalo de tiempo debe ser menor a 24 horas"`, so every poll failed regardless of token. Lower LOOKBACK_HOURS from 48 to 23 (1h margin under the hard cap, still spanning a full daily TRMI cycle) and document the constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NvSncWrjedLiAtAV4DTJR
When `[price]` is configured the multi-source manager drives aggregation and the legacy `[mostro].bitcoin_price_api_url` is not consulted. Emit a startup WARN naming the legacy value and the enabled providers so an operator who still has the legacy key set isn't misled into thinking it takes effect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NvSncWrjedLiAtAV4DTJR
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/main.rs`:
- Around line 223-227: The tracing::warn! call is logging the full
bitcoin_price_api_url which may contain embedded credentials (userinfo, query
tokens, etc.). Parse the bitcoin_price_api_url as a URL object and construct a
sanitized version containing only the scheme, host, and path while stripping
userinfo, query parameters, and fragments. Replace the direct
mostro_settings.bitcoin_price_api_url reference in the log message with the
sanitized URL string to prevent credential leakage in 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
| tracing::warn!( | ||
| "price: legacy `bitcoin_price_api_url` = \"{}\" is ignored for price \ | ||
| aggregation because `[price]` is configured; using enabled providers: {}", | ||
| mostro_settings.bitcoin_price_api_url, | ||
| enabled, |
There was a problem hiding this comment.
Avoid logging bitcoin_price_api_url verbatim.
Logging the full legacy URL can leak credentials if operators embed auth data (userinfo or query tokens) in the URL. Log a redacted/sanitized form instead (e.g., scheme + host + path, strip userinfo/query/fragment).
🔒 Suggested fix
+ let legacy_url_for_log = reqwest::Url::parse(&mostro_settings.bitcoin_price_api_url)
+ .map(|mut u| {
+ let _ = u.set_username("");
+ let _ = u.set_password(None);
+ u.set_query(None);
+ u.set_fragment(None);
+ u.to_string()
+ })
+ .unwrap_or_else(|_| "<invalid_url>".to_string());
tracing::warn!(
"price: legacy `bitcoin_price_api_url` = \"{}\" is ignored for price \
aggregation because `[price]` is configured; using enabled providers: {}",
- mostro_settings.bitcoin_price_api_url,
+ legacy_url_for_log,
enabled,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tracing::warn!( | |
| "price: legacy `bitcoin_price_api_url` = \"{}\" is ignored for price \ | |
| aggregation because `[price]` is configured; using enabled providers: {}", | |
| mostro_settings.bitcoin_price_api_url, | |
| enabled, | |
| let legacy_url_for_log = reqwest::Url::parse(&mostro_settings.bitcoin_price_api_url) | |
| .map(|mut u| { | |
| let _ = u.set_username(""); | |
| let _ = u.set_password(None); | |
| u.set_query(None); | |
| u.set_fragment(None); | |
| u.to_string() | |
| }) | |
| .unwrap_or_else(|_| "<invalid_url>".to_string()); | |
| tracing::warn!( | |
| "price: legacy `bitcoin_price_api_url` = \"{}\" is ignored for price \ | |
| aggregation because `[price]` is configured; using enabled providers: {}", | |
| legacy_url_for_log, | |
| enabled, | |
| ); |
🤖 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/main.rs` around lines 223 - 227, The tracing::warn! call is logging the
full bitcoin_price_api_url which may contain embedded credentials (userinfo,
query tokens, etc.). Parse the bitcoin_price_api_url as a URL object and
construct a sanitized version containing only the scheme, host, and path while
stripping userinfo, query parameters, and fragments. Replace the direct
mostro_settings.bitcoin_price_api_url reference in the log message with the
sanitized URL string to prevent credential leakage in logs.
Phase 3 of
docs/PRICE_PROVIDERS.md— El Toque (fiat-cross CUP/MLC)Phase (spec §9, §11.3): 3 — depends on Phase 2 (#773, merged). Adds the El Toque provider: a fiat-cross quoter that contributes CUP and MLC to the aggregate. Unlike the direct quoters it reports
Quote::PerBase { base: "USD" }, resolved to a per-BTC figure against the aggregated USD/BTC anchor by Phase 0's existingresolve_per_base— no aggregation-core change.Per the §5.4 extension contract this is one adapter file + one registry arm + one config block + one fixture;
aggregate.rs,store.rs, the scheduler and every order handler are untouched.What it does
El Toque publishes the informal Cuban market rate as CUP per foreign unit (it is not a BTC source). The adapter parses its CUP-denominated
tasaspayload and emits, doing the §11.3 cross math internally:CUPPerBase { base: "USD", value: cup_per_usd }tasas.USDMLCPerBase { base: "USD", value: cup_per_usd / cup_per_mlc }MLC_per_USD = (CUP per USD) / (CUP per MLC)Debug/logs (§10.3).only = ["CUP", "MLC"]; the adapter independently emits only CUP/MLC, so the two agree. CUP/MLC are already in the §6.6 fiat allowlist, so they survive to aggregation.Request & response (confirmed against the live API)
The El Toque request line — previously provisional — is now confirmed with the maintainer:
GET {url}/v1/trmi?date_from=…&date_to=…withAuthorization: Bearer <token>. The endpoint requires a[date_from, date_to]range (wire formatYYYY-MM-DD HH:MM:SS, URL-encoded by reqwest) and returns the most recent rate published within it.fetchtherefore queries a rolling 48h window ending "now" (LOOKBACK_HOURS) — wide enough to absorb the ~daily TRMI update cadence plus any UTC↔Cuba (UTC-4/-5) skew, so a quiet day still resolves a recent rate instead of emptytasas.tasasobject plus the timestamp of the returned rate (date/hour/minutes/seconds, ignored by the parser). El Toque usesECUfor the euro:{ "tasas": { "USD": 490.0, "MLC": 200.0, "ECU": 540.0, "TRX": 150.0, "BTC": 490.0, "USDT_TRC20": 525.0 }, "date": "2022-10-27", "hour": 7, "minutes": 59, "seconds": 30 }tests/fixtures/price/eltoque_trmi.json) is a real captured response; the parse path applies the §11.3 cross math and is fully unit-tested.Still open (§11.3 Q1/Q3, maintainer questions): whether the chosen plan exposes CUP/MLC/USD in one call, and confirming El Toque's CUP and Yadio's CUP track the same informal rate. The template ships
enabled = false; keep it off until a token is provisioned and the operator opts in.Files
src/price/providers/eltoque.rs— the adapter (+ 8 unit tests).src/price/providers/mod.rs—pub mod eltoque;.src/price/manager.rs— registry arm (build_provider) builds El Toque, failing fast without a token; the old "unimplemented" rejection test is replaced by with-token/without-token coverage.settings.tpl.toml— real[price.providers.eltoque]block (enabled = false).docs/PRICE_PROVIDERS.md— Phase 3 status + §11.3 confirmed request/response note.tests/fixtures/price/eltoque_trmi.json— captured El Toque/v1/trmiresponse.Test evidence
aggregate_tick_unions_partial_coveragetest.cargo clippy --all-targets --all-features+cargo fmtclean.🧪 Manual testing — step by step
Prereqs: a working
mostroddev setup (LND not required for price checks; the daemon logs price ticks regardless of trading activity). To exercise everything without burning a real token, point El Toque at a tiny local mock serving the shipped fixture. (The live API requires a real Bearer token; once you have one, swapurl/tokenfor the real endpoint and the same checks apply.)1. Startup validation — token is required (spec §7)
settings.toml:mostrod. Expected: it refuses to start withprice provider 'eltoque': enabled provider requires a \token` (Bearer API key) …`.token = "any-string"and restart — startup proceeds. ✔️2. Cross-math + PerBase resolution against a local mock
This exercises the parse path and the USD-anchor resolution without a real token.
/v1/trmipath. From the repo root:mkdir -p /tmp/eltoque/v1 cp tests/fixtures/price/eltoque_trmi.json /tmp/eltoque/v1/trmi (cd /tmp/eltoque && python3 -m http.server 8099)GET {url}/v1/trmi?date_from=…&date_to=…withAuthorization: Bearer <token>; Python'shttp.serverstrips the query string and ignores the header, returning the JSON file.)RUST_LOG=mostrod=info cargo run -- -d <data-dir>and watch one tick. Expected: bothprice: yadio ok (…)andprice: eltoque ok (2 currencies).nak req -k 30078 -a <mostro pubkey> <relay>) and verify with the fixture's numbers (USD=490,MLC=200CUP):CUP/BTC ÷ USD/BTC ≈ 490(informal CUP per USD), andCUP/BTC ÷ MLC/BTC ≈ 200(1 MLC ≈ 200 CUP, matching the source).sourcetag includeseltoquefor CUP/MLC.3. Anchor dependency — no USD source ⇒ CUP/MLC can't resolve (spec §11.3)
eltoque ok (2 currencies)still polls, butget_price("CUP")/("MLC")produce no fresh aggregate — the PerBase quotes are dropped for lack of a USD anchor, and the currencies fall back to last-known-good (or are absent on a cold store). Re-enable Yadio → CUP/MLC resolve again next tick. ✔️4. Scoping — El Toque contributes only CUP/MLC
tasasalso listsUSD,ECU(EUR) and crypto. Confirm the published event'seltoque-sourced currencies are only CUP and MLC: El Toque never contributes USD/EUR (the adapter emits only CUP/MLC, andonly = ["CUP","MLC"]backstops it). USD/EUR come from the direct quoters alone. ✔️5. Secret never leaks (spec §10.3)
RUST_LOG=mostrod=debugand a recognisable token (e.g.token = "SECRET123").Debugprintstoken: "<redacted>". ✔️6. Regression — disabled / legacy untouched
enabled = falseon El Toque (or remove the block). Expected: daemon runs exactly as in Phase 2 — no El Toque poll, CUP served by Yadio's informal rate alone, no behaviour change. ✔️[price]block: legacy single-source Yadio path still works (Phase 1 migration), untouched by this PR. ✔️🤖 Generated with Claude Code
Summary by CodeRabbit