-
Notifications
You must be signed in to change notification settings - Fork 3
feat(#337): validate market-price orders against the node's sats limits #343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6c00634
f19a7c1
0f432dc
70c0ab7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import 'package:flutter_riverpod/flutter_riverpod.dart'; | ||
|
|
||
| import 'package:mostro/core/mostro_defaults.dart'; | ||
| import 'package:mostro/features/settings/widgets/mostro_node_selector.dart'; | ||
| import 'package:mostro/src/rust/api/nostr.dart' as nostr_api; | ||
|
|
||
| /// Price of one BTC in [fiatCode], as published by the active Mostro node in | ||
| /// its Kind 30078 (`d` = `mostro-rates`) event. | ||
| /// | ||
| /// Exists so a market-price order can be checked against the node's sats | ||
| /// limits before it is submitted (#337): the daemon prices such an order from | ||
| /// this same rate, so it is the number its range check will use. | ||
| /// | ||
| /// Reads the node pubkey from [mostroPubkeyProvider], like | ||
| /// `mostroNodeProvider`, so the rate always belongs to the node the order will | ||
| /// be sent to. | ||
| /// | ||
| /// Resolves to `null` whenever the node has no usable rate to give — it | ||
| /// publishes none (publishing is optional), the event has expired, or it | ||
| /// quotes no such currency — and an unreachable relay surfaces as an error. | ||
| /// Callers must treat both as "not checkable" and submit anyway, leaving the | ||
| /// daemon as the authority, which is what PR #302 chose for fixed-sats | ||
| /// amounts. | ||
| /// | ||
| /// `autoDispose` and keyed by currency: switching currency starts a fetch for | ||
| /// the new one, which the Rust-side cache usually answers without another | ||
| /// relay query. | ||
| final exchangeRateProvider = | ||
| FutureProvider.autoDispose.family<double?, String>((ref, fiatCode) async { | ||
| final code = fiatCode.trim(); | ||
| if (code.isEmpty) return null; | ||
|
|
||
| final pubkey = ref.watch(mostroPubkeyProvider); | ||
| final resolvedPubkey = | ||
| pubkey.trim().isEmpty ? defaultMostroPubkey : pubkey.trim(); | ||
|
|
||
| return nostr_api.fetchExchangeRate( | ||
| mostroPubkeyHex: resolvedPubkey, | ||
| fiatCode: code, | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import 'dart:math'; | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
|
|
||
| /// Expresses a Mostro node's sats order limits in the fiat currency the user | ||
| /// types in, so a market-price order can be checked before it is submitted | ||
| /// (#337). | ||
| /// | ||
| /// A market-price order carries no sats amount: the daemon derives one from | ||
| /// the fiat amount at its own rate and rejects the order with | ||
| /// `OutOfRangeSatsAmount` when the result falls outside | ||
| /// `min_order_amount`/`max_order_amount`. Everything here mirrors that | ||
| /// derivation so the client reaches the same verdict beforehand. | ||
|
|
||
| /// Sats per BTC. | ||
| const int _satsPerBtc = 100000000; | ||
|
|
||
| /// The sats amount the daemon will price [fiat] at, given [rate] (the price of | ||
| /// one BTC in that fiat). | ||
| /// | ||
| /// Truncates rather than rounds, because that is what the daemon does: | ||
| /// `(fiat_amount / price * 1E8) as i64` (`mostro/src/app/order.rs`). Rounding | ||
| /// up would let the client accept an amount one sat below the node's minimum | ||
| /// and still see it rejected — the exact surprise this check exists to remove. | ||
| int satsFromFiat(double fiat, double rate) => | ||
| (fiat / rate * _satsPerBtc).truncate(); | ||
|
|
||
| /// A node's sats limits converted to whole fiat units. | ||
| @immutable | ||
| class FiatAmountLimits { | ||
| const FiatAmountLimits({required this.minFiat, required this.maxFiat}); | ||
|
|
||
| final int minFiat; | ||
| final int maxFiat; | ||
|
|
||
| /// Whether the range is worth showing. False when the node's whole valid | ||
| /// range collapses below one unit of fiat, leaving no enterable whole | ||
| /// number; callers then fall back to the raw sats bounds. | ||
| bool get isDisplayable => minFiat >= 1 && maxFiat >= minFiat; | ||
| } | ||
|
|
||
| /// Converts the node's sats limits to whole-fiat bounds at [rate]. | ||
| /// | ||
| /// The minimum rounds up and the maximum rounds down, so every whole number | ||
| /// inside the returned range converts back to a sats amount inside the node's | ||
| /// real range — a bound shown to the user is never itself rejected. The | ||
| /// minimum is floored at 1 because the amount field takes whole numbers only. | ||
| FiatAmountLimits fiatAmountLimits({ | ||
| required int minSats, | ||
| required int maxSats, | ||
| required double rate, | ||
| }) { | ||
| if (rate <= 0) return const FiatAmountLimits(minFiat: 0, maxFiat: 0); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🌐 Web query:
💡 Result: Yes, according to the Dart SDK documentation, double.tryParse('Infinity') and double.tryParse('NaN') return the respective non-finite double values (Infinity and NaN) [1][2]. Both truncate and ceil throw an UnsupportedError if called on a non-finite double, such as NaN or Infinity [3][4][5]. This is because these methods are designed to return an integer representation, which is not defined for non-finite values [3][4][5]. Conversely, methods like truncateToDouble and ceilToDouble do not throw for non-finite values and instead return the value unmodified [5][6]. Citations:
🏁 Script executed: printf '%s\n' '--- repository conventions ---'
head -5 /tmp/coderabbit-repo-knowledge/mostrop2p-app-eda38ece/*/*.md 2>/dev/null
printf '%s\n' '--- target file ---'
cat -n lib/shared/utils/order_amount_limits.dart
printf '%s\n' '--- direct references ---'
rg -n --glob '*.dart' 'FiatAmountLimits|satsFromFiat|fiatAmountLimits|_checkValid' libRepository: MostroP2P/app Length of output: 10058 🏁 Script executed: cat -n lib/features/order/screens/add_order_screen.dart | sed -n '120,205p;590,635p'
printf '%s\n' '--- tests for the utility and validation path ---'
rg -n --glob '*_test.dart' 'fiatOutOfNodeRange|satsFromFiat|fiatAmountLimits|_checkValid|Infinity|NaN' test libRepository: MostroP2P/app Length of output: 9557 🏁 Script executed: rg -n -C 4 --glob '*.dart' '_fiatRangeError|fiatOutOfNodeRange|_rangeWarning|_checkValid' lib/features/order/screens/add_order_screen.dartRepository: MostroP2P/app Length of output: 3446 Reject non-finite amounts and rates.
🤖 Prompt for AI Agents |
||
| return FiatAmountLimits( | ||
| minFiat: max(1, (minSats / _satsPerBtc * rate).ceil()), | ||
| maxFiat: (maxSats / _satsPerBtc * rate).floor(), | ||
| ); | ||
| } | ||
|
|
||
| /// Returns the node's accepted range — in sats, and converted to fiat — when | ||
| /// the entered market-price [fiatStr] prices outside it, otherwise null. | ||
| /// | ||
| /// Pure and testable, like `satsOutOfNodeRange` in `add_order_screen.dart`, | ||
| /// its fixed-sats counterpart. Fails open on everything it cannot | ||
| /// judge: no rate ([rate] null or non-positive, i.e. the node publishes none), | ||
| /// a node advertising only one bound, or an amount that is not a positive | ||
| /// number. In those cases the daemon stays the only authority, exactly as it | ||
| /// was before this check existed. | ||
| ({int minSats, int maxSats, FiatAmountLimits limits})? fiatOutOfNodeRange( | ||
| String fiatStr, | ||
| int? minOrder, | ||
| int? maxOrder, | ||
| double? rate, | ||
| ) { | ||
| if (minOrder == null || maxOrder == null) return null; | ||
| if (rate == null || rate <= 0) return null; | ||
| final fiat = double.tryParse(fiatStr.trim()); | ||
| if (fiat == null || fiat <= 0) return null; | ||
|
|
||
| final sats = satsFromFiat(fiat, rate); | ||
|
Comment on lines
+77
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because the amount fields have no input formatter, pasted values such as Useful? React with 👍 / 👎. |
||
| if (sats >= minOrder && sats <= maxOrder) return null; | ||
|
|
||
| return ( | ||
| minSats: minOrder, | ||
| maxSats: maxOrder, | ||
| limits: fiatAmountLimits( | ||
| minSats: minOrder, | ||
| maxSats: maxOrder, | ||
| rate: rate, | ||
| ), | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For decimal inputs, this computes sats from the original
double, butnew_orderconverts fixed and range fiat amounts toi64inrust/src/mostro/actions.rs:48-50before the daemon receives them. For example, with a 30,000 rate and a 3,334-sat minimum,1.1is accepted here as 3,666 sats but is transmitted as1, which the daemon prices at 3,333 sats and rejects. Perform this protocol conversion and validation in Rust, or normalize the fiat value exactly as the wire path does before comparing it.AGENTS.md reference: AGENTS.md:L23-L27
Useful? React with 👍 / 👎.