Repository navigation
fix(rates): keep on-chain BTC quoted at the peg - #908
Conversation
A node that quotes bitcoin on other networks publishes on-chain BTC as the inner "BTC" entry of its rates event at 100000000 (10^8 sats per BTC), next to LBTC and RBTC. parse_rates_content drops every inner "BTC" key, so that quote is lost and the app never prices BTC. The tests expect an inner "BTC" of 100000000 to be kept, and one that is neither the self-rate nor the peg to be dropped in any case: a lower-case "btc" currently slips past the filter and is upper-cased into the table. Both fail on main. Refs #907 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wLvLzCu94YTaE15zLR2Cn
parse_rates_content now keeps an inner "BTC" entry whose value is BTC_PEGGED_RATE (10^8 sats per BTC): that is a node quoting on-chain bitcoin as a BTC-pegged order currency (mostro docs/PRICE_PROVIDERS.md §11.9). Any other inner "BTC", including a source's self-rate of 1 and a lower-case key, is still dropped. Without it lookup_exchange_rate answered NotQuoted for BTC on every node, so Market stayed locked for on-chain BTC and the range pre-check, quick chips and sats estimates had no rate. Refs #907 (item 1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wLvLzCu94YTaE15zLR2Cn
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 12 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Define and apply the BTC peg filter rust/src/mostro/rates.rs |
The parser uses the public BTC_PEGGED_RATE constant and compares inner BTC codes without case sensitivity. It retains entries that match the peg value and filters other inner BTC values. Tests cover integer and floating-point peg values, plus a non-peg value. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- MostroP2P/app#907: The issue reports that the parser drops the inner BTC quote at 100,000,000; this change retains that quote.
- MostroP2P/mostro#1088: The issue defines BTC-pegged order pricing that requires an inner BTC rate of 100,000,000; this change retains that rate.
Possibly related PRs
- MostroP2P/app#894: It adds the exchange-rate lookup contract that uses the rates table affected by this change.
Suggested reviewers: andreadiazcorreia, catrya
Merge Risk: 
7ccba
-
The BTC peg parsing change is ready to merge; no actionable current-head risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: retaining on-chain BTC rates at the peg. Description check The description follows the required template, explains the behavior change and scope, documents testing and limitations, and reports the automated test results. Manual testing was not performed, but … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description follows the required template, explains the behavior change and scope, documents testing and limitations, and reports the automated test results. Manual testing was not performed, but the omission is clearly documented with reproducible steps and a reason.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the rates at dawn,
The BTC peg is kept, not gone.
A hundred million marks the way,
While other BTC quotes stray.
The tests hop through, both whole and float,
And leave the right rate in the quote.
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ccba68668
ℹ️ 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".
The rates contract now accepts an inner "BTC" entry of 100000000: a node quoting on-chain bitcoin as a BTC-pegged order currency, at 10^8 sats per BTC, as LBTC and RBTC. .specify/NOSTR_EXCHANGE_RATES.md says how pegged codes are carried and that any other inner "BTC", such as the self-rate of 1, is dropped; the fetch_exchange_rate contract says what it returns for these codes. Refs #907 (item 1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wLvLzCu94YTaE15zLR2Cn
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Linked issue
Refs #907 (item 1). The other items land in follow-up PRs.
Type
fix
What changes and why
A node that quotes bitcoin on other networks (MostroP2P/mostro#1088) publishes on-chain BTC as the inner
"BTC"entry of itsmostro-ratesevent, at100000000(10⁸ sats per BTC).parse_rates_contentdropped every inner"BTC"key to skip a source's self-rate of 1, so it dropped that quote too, andlookup_exchange_rateansweredNotQuotedfor BTC on every node. It now keeps an inner"BTC"whose value isBTC_PEGGED_RATEand drops any other, in any case. A lower-case"btc"used to slip past the filter.Affected flow
Exchange-rate lookups for market-priced orders (create order, take order): the Market lock, the range pre-check, the quick chips and the sats estimates all read
lookup_exchange_rate. Rust core only:rust/src/mostro/rates.rs.Blast radius
"BTC"key changes. A value of100000000is kept;1and any other value are dropped as before. Every other code parses as before.rust/src/api/change, so no regenerated bindings. Nothing is persisted: the snapshot is in memory and node-scoped.Platforms tested
None by hand (see Manual testing).
cargo teston Linux.Manual testing
Not run. I could not run the app against a Mostro node that quotes on-chain BTC.
Setup
App built from this branch, against a mostrod with MostroP2P/mostro#1091 and
[price.providers.pegged] enabled = true,only = ["BTC"].Steps
mostro-ratesevent carries"BTC": 100000000under"BTC". Expected: it does.lookup_exchange_rate("BTC")for that node (a debug print, until BTC is in the catalogue). Expected: the rate100000000. (fails onmain):NotQuoted.USDon the same node. Expected: its rate, as before.Not covered
The UI: BTC is not in the currency catalogue until the follow-up PR for items 2–8 of #907.
Screenshots
No visible change: BTC cannot be picked yet.
Automated tests
The first commit adds the tests and fails on
main:on_chain_btc_quoted_at_the_peg_is_kept: an inner"BTC"of100000000(integer or float) is kept next toRBTCandUSD.an_inner_btc_that_is_neither_the_self_rate_nor_the_peg_is_dropped: a lower-case"btc": 2is dropped.the_btc_self_rate_is_droppedstill passes.Checklist
cd rust && cargo fmt --check && cargo clippy -- -D warnings && cargo testpass locally:cargo clippy -- -D warningsandcargo testpass. My local rustfmt (1.10.0) reports formatting differences in 55 files thatmainalready has, including one line ofrates.rsthis PR does not touch; the lines this PR adds are formatted.dart format --set-exit-if-changed .,flutter analyzeandflutter testpass locally: no Dart change.rust/src/api/or an.arb: noAI assistance
Written with Claude Code from #907.
🤖 Generated with Claude Code
https://claude.ai/code/session_016wLvLzCu94YTaE15zLR2Cn
Summary by CodeRabbit