From 3bb64b71ac0e2219333ecfe5559c2fca0883e8ab Mon Sep 17 00:00:00 2001 From: Omar Hatem Date: Sat, 12 Sep 2026 18:59:00 +0200 Subject: [PATCH] Add more guards to amounts being zero or infinity (#3612) * Add more guards to amounts being zero or infinity * Add more guards to amounts being zero or infinity --- .../test/amount/non_finite_amount_test.dart | 127 ++++++++++++++++++ cw_monero/lib/api/transaction_history.dart | 5 + cw_monero/lib/monero_wallet.dart | 5 + lib/entities/calculate_fiat_amount.dart | 3 +- lib/entities/calculate_fiat_amount_raw.dart | 4 + .../widgets/send_page/send_confirm_sheet.dart | 2 +- .../widgets/swap_page/swap_confirm_sheet.dart | 9 +- lib/view_model/send/output.dart | 17 ++- lib/view_model/send/send_view_model.dart | 18 ++- 9 files changed, 179 insertions(+), 11 deletions(-) create mode 100644 cw_core/test/amount/non_finite_amount_test.dart diff --git a/cw_core/test/amount/non_finite_amount_test.dart b/cw_core/test/amount/non_finite_amount_test.dart new file mode 100644 index 000000000..4498e2f9e --- /dev/null +++ b/cw_core/test/amount/non_finite_amount_test.dart @@ -0,0 +1,127 @@ +import "package:cw_core/amount/money.dart"; +import "package:cw_core/amount/money_double.dart"; +import "package:cw_core/crypto_currency.dart"; +import "package:cw_core/parse_fixed.dart"; +import "package:flutter_test/flutter_test.dart"; + +/// Every non-finite spelling that `double.tryParse` accepts. +/// +/// These matter because a `double`-based amount guard cannot reject them: +/// `Infinity <= 0` is false and *every* comparison against `NaN` is false, so a +/// non-finite amount sails through an `if (amount <= 0) throw` check and then +/// collapses to zero once it reaches Money/BigInt -- i.e. a zero-amount +/// transaction that passed validation. The Money layer is the enforcement point +/// for that invariant, so pin it here. +const nonFiniteSpellings = [ + "Infinity", + "+Infinity", + "-Infinity", + "NaN", + "1e999", // overflows to Infinity as a double + "1E400", + " Infinity ", +]; + +void main() { + group("non-finite amounts", () { + test("double.tryParse accepts them (this is why Money must reject them)", () { + for (final spelling in nonFiniteSpellings) { + final parsed = double.tryParse(spelling); + expect(parsed, isNotNull, reason: "double.tryParse rejected $spelling"); + expect(parsed!.isFinite, isFalse, reason: "$spelling parsed as finite"); + // The point of the whole exercise: a `<= 0` guard does not stop these. + if (!parsed.isNegative) { + expect(parsed <= 0, isFalse, reason: "$spelling was caught by a <= 0 guard"); + } + } + }); + + test("Money.tryParse returns null for every non-finite spelling", () { + for (final spelling in nonFiniteSpellings) { + expect( + Money.tryParse(spelling, CryptoCurrency.btc), + isNull, + reason: "Money.tryParse accepted $spelling", + ); + expect( + Money.tryParse(spelling, CryptoCurrency.btc, strictParsing: false), + isNull, + reason: "Money.tryParse(strictParsing: false) accepted $spelling", + ); + } + }); + + test("Money.parse throws for every non-finite spelling", () { + for (final spelling in nonFiniteSpellings) { + expect( + () => Money.parse(spelling, CryptoCurrency.btc), + throwsA(isA()), + reason: "Money.parse accepted $spelling", + ); + } + }); + + test("CryptoCurrency.tryParseAmount returns null, parseAmount throws", () { + for (final spelling in nonFiniteSpellings) { + expect(CryptoCurrency.btc.tryParseAmount(spelling), isNull, reason: spelling); + expect( + () => CryptoCurrency.btc.parseAmount(spelling), + throwsA(isA()), + reason: spelling, + ); + } + }); + + test("tryParseFixed returns null / parseFixed throws (the underlying gate)", () { + for (final spelling in nonFiniteSpellings) { + expect(tryParseFixed(spelling, 8), isNull, reason: spelling); + expect( + () => parseFixed(spelling, 8), + throwsA(isA()), + reason: spelling, + ); + } + }); + + test("double.tryToMoney returns null for non-finite doubles", () { + // Relied on by ExchangeViewModel.changeReceiveAmount/changeDepositAmount, which + // divide by `bestRate` and can produce Infinity when the rate is still 0. + expect(double.infinity.tryToMoney(CryptoCurrency.btc), isNull); + expect(double.negativeInfinity.tryToMoney(CryptoCurrency.btc), isNull); + expect(double.nan.tryToMoney(CryptoCurrency.btc), isNull); + }); + + test("a rejected amount funnels to Money.zero, which is the sweep-all sentinel", () { + // Why the cw_monero/cw_wownero guards exist. MONERO_Wallet_createTransaction: + // Monero::optional optAmount; + // if (amount != 0) { optAmount = amount; } + // so amount == 0 means "no amount given" and wallet2 sweeps the whole account. + // Output.cryptoAmountMoney falls back to Money.zero on any parse failure, and a + // zero Money stringifies to an all-zeros decimal -- exactly the value the native + // layer reads as "send everything". Nothing between them may treat 0 as an amount. + for (final spelling in nonFiniteSpellings) { + final fallback = + CryptoCurrency.btc.tryParseAmount(spelling) ?? Money.zero(CryptoCurrency.xmr); + expect(fallback.amount, BigInt.zero, reason: spelling); + } + + final zeroXmr = Money.zero(CryptoCurrency.xmr); + expect(zeroXmr.amount, BigInt.zero); + expect(double.tryParse(zeroXmr.toString()), 0.0, + reason: "a zero Money must stringify to a numeric zero, i.e. the sweep-all sentinel"); + expect(RegExp(r"^0(\.0+)?$").hasMatch(zeroXmr.toString()), isTrue, + reason: "unexpected zero rendering: ${zeroXmr.toString()}"); + + // One piconero is NOT zero -- the guards must not reject genuine dust. + expect(Money(BigInt.one, CryptoCurrency.xmr).sign, 1); + expect(double.parse(Money(BigInt.one, CryptoCurrency.xmr).toString()) > 0, isTrue); + }); + + test("finite amounts still round-trip (guard did not over-reject)", () { + expect(Money.tryParse("0", CryptoCurrency.btc)!.amount, BigInt.zero); + expect(Money.tryParse("1", CryptoCurrency.btc)!.amount, BigInt.from(100000000)); + expect(Money.tryParse("0.00000001", CryptoCurrency.btc)!.amount, BigInt.one); + expect(1.5.tryToMoney(CryptoCurrency.btc)!.amount, BigInt.from(150000000)); + }); + }); +} diff --git a/cw_monero/lib/api/transaction_history.dart b/cw_monero/lib/api/transaction_history.dart index c6d1c3cf5..16bfa1e96 100644 --- a/cw_monero/lib/api/transaction_history.dart +++ b/cw_monero/lib/api/transaction_history.dart @@ -149,6 +149,11 @@ Future createTransactionSync( List preferredInputs = const []}) async { final amt = amount == null ? 0 : currentWallet!.amountFromString(amount); + if (amount != null && amt == 0) { + throw MoneroTransactionCreationException( + 'Refusing to create a transaction with a zero amount: 0 is the sweep-all '); + } + final waddr = currentWallet!.ffiAddress(); // force reconnection in case the os killed the connection? diff --git a/cw_monero/lib/monero_wallet.dart b/cw_monero/lib/monero_wallet.dart index afd9284be..98d9e9d5b 100644 --- a/cw_monero/lib/monero_wallet.dart +++ b/cw_monero/lib/monero_wallet.dart @@ -476,6 +476,11 @@ abstract class MoneroWalletBase } else { final output = outputs.first; final address = output.isParsedAddress ? output.extractedAddress : output.address; + + if (!output.sendAll && output.cryptoAmount.amount <= BigInt.zero) { + throw MoneroTransactionCreationException('Amount must be greater than 0.'); + } + final amount = output.sendAll ? null : output.cryptoAmount.toString(); // if ((formattedAmount != null && unlockedBalance < formattedAmount) || diff --git a/lib/entities/calculate_fiat_amount.dart b/lib/entities/calculate_fiat_amount.dart index ae1436e5d..17fe60b61 100644 --- a/lib/entities/calculate_fiat_amount.dart +++ b/lib/entities/calculate_fiat_amount.dart @@ -8,8 +8,9 @@ String calculateFiatAmount({double? price, String? cryptoAmount, bool raw = fals cryptoAmount = cryptoAmount.sanitized(); final _amount = double.tryParse(cryptoAmount); - if (_amount == null || _amount.isNaN) return '0.00'; + if (_amount == null || !_amount.isFinite) return '0.00'; final _result = price * _amount; + if (!_result.isFinite) return '0.00'; final result = _result < 0 ? _result * -1 : _result; if (result == 0.0) { diff --git a/lib/entities/calculate_fiat_amount_raw.dart b/lib/entities/calculate_fiat_amount_raw.dart index 0156c7036..9465b55bd 100644 --- a/lib/entities/calculate_fiat_amount_raw.dart +++ b/lib/entities/calculate_fiat_amount_raw.dart @@ -7,6 +7,10 @@ String calculateFiatAmountRaw({required double cryptoAmount, double? price}) { final result = price * cryptoAmount; + if (!result.isFinite) { + return '0.00'; + } + if (result == 0.0) { return '0.00'; } diff --git a/lib/new-ui/widgets/send_page/send_confirm_sheet.dart b/lib/new-ui/widgets/send_page/send_confirm_sheet.dart index 35f710209..2830aa32c 100644 --- a/lib/new-ui/widgets/send_page/send_confirm_sheet.dart +++ b/lib/new-ui/widgets/send_page/send_confirm_sheet.dart @@ -214,7 +214,7 @@ class SendTransactionDetails extends StatelessWidget { sendViewModel.balance, sendViewModel.selectedCryptoCurrency) ?? zero; - return sendViewModel.selectedCryptoCurrency.tryParseAmount(o.cryptoAmount) ?? zero; + return o.cryptoAmountMoney; }, sendViewModel.selectedCryptoCurrency)) : sendViewModel.amountParsingProxy.asDisplayString(transaction.amount); diff --git a/lib/new-ui/widgets/swap_page/swap_confirm_sheet.dart b/lib/new-ui/widgets/swap_page/swap_confirm_sheet.dart index b5626056b..db1cd6d0c 100644 --- a/lib/new-ui/widgets/swap_page/swap_confirm_sheet.dart +++ b/lib/new-ui/widgets/swap_page/swap_confirm_sheet.dart @@ -14,6 +14,7 @@ import 'package:cake_wallet/src/widgets/new_list_row/new_list_section.dart'; import 'package:cake_wallet/view_model/exchange/exchange_trade_view_model.dart'; import 'package:cake_wallet/view_model/exchange/exchange_view_model.dart'; import 'package:cake_wallet/view_model/send/send_view_model_state.dart'; +import 'package:cw_core/amount/amount_sanitizer.dart'; import 'package:cw_core/amount/money.dart'; import 'package:cw_core/crypto_amount_format.dart'; import 'package:cw_core/crypto_currency.dart'; @@ -176,9 +177,11 @@ class SwapTransactionDetails extends StatelessWidget { iconPath: exchangeViewModel.depositCurrency.iconPath ?? "", badgeIconPath: _resolveChainBadgePath(exchangeViewModel.depositCurrency), trailingText: exchangeViewModel.amountParsingProxy - .asDisplayStringWithSymbol(exchangeViewModel.depositCurrency - .tryParseAmount(exchangeTradeViewModel.trade.amount) ?? - Money.zero(exchangeViewModel.depositCurrency)), + .asDisplayStringWithSymbol( + exchangeViewModel.depositCurrency.tryParseAmount( + exchangeTradeViewModel.trade.amount.sanitized(), + ) ?? + Money.zero(exchangeViewModel.depositCurrency)), ), if (exchangeTradeViewModel.sendViewModel.pendingTransaction != null) ListItemRegularRow( diff --git a/lib/view_model/send/output.dart b/lib/view_model/send/output.dart index 106dc7a89..618416b95 100644 --- a/lib/view_model/send/output.dart +++ b/lib/view_model/send/output.dart @@ -338,10 +338,21 @@ abstract class OutputBase with Store { ? CryptoCurrency.btc : cryptoCurrencyHandler(); + final price = _fiatConversationStore.prices[cryptoCurrency]; + if (price == null || !price.isFinite || price <= 0) { + printV('_updateCryptoAmount: invalid fiat rate $price for $cryptoCurrency'); + cryptoAmount = ''; + return; + } + final decimals = min(20, cryptoCurrencyHandler().decimals); - final crypto = (double.parse(fiatAmount.replaceAll(',', '.')) / - _fiatConversationStore.prices[cryptoCurrency]!) - .toStringAsFixed(decimals); + final rawCrypto = double.parse(fiatAmount.replaceAll(',', '.')) / price; + if (!rawCrypto.isFinite) { + cryptoAmount = ''; + return; + } + + final crypto = rawCrypto.toStringAsFixed(decimals); if (cryptoAmount != crypto) cryptoAmount = crypto; } catch (e) { diff --git a/lib/view_model/send/send_view_model.dart b/lib/view_model/send/send_view_model.dart index 187c718cd..3d32febfd 100644 --- a/lib/view_model/send/send_view_model.dart +++ b/lib/view_model/send/send_view_model.dart @@ -54,6 +54,7 @@ import 'package:cake_wallet/view_model/unspent_coins/unspent_coins_list_view_mod import 'package:cake_wallet/wownero/wownero.dart'; import 'package:cake_wallet/zano/zano.dart'; import 'package:cake_wallet/zcash/zcash.dart'; +import 'package:cw_core/amount/amount_sanitizer.dart'; import 'package:cw_core/amount/money.dart'; import 'package:cw_core/crypto_currency.dart'; import 'package:cw_core/currency_for_wallet_type.dart'; @@ -882,7 +883,11 @@ abstract class SendViewModelBase extends WalletChangeListenerViewModel with Stor final fee = actualFee > 0 ? actualFee : 0.0005; final fromCurrency = trade.from ?? CryptoCurrency.sol; - final amount = Money.tryParse(trade.amount, fromCurrency) ?? Money.zero(fromCurrency); + final amount = Money.tryParse( + trade.amount.sanitized(), + fromCurrency, + strictParsing: false, + ) ?? Money.zero(fromCurrency); pendingTransaction = await solana!.signAndPrepareJupiterSwapTransaction( wallet, @@ -920,8 +925,15 @@ abstract class SendViewModelBase extends WalletChangeListenerViewModel with Stor final bool isTradeTx = trade != null && provider != null; if (isTradeTx) { - final tradeAmountDouble = double.tryParse(trade.amount) ?? 0.0; - if (tradeAmountDouble <= 0) throw Exception('Trade amount must be greater than 0'); + final tradeAmountMoney = Money.tryParse( + trade.amount.sanitized(), + trade.from ?? selectedCryptoCurrency, + strictParsing: false, + ); + if (tradeAmountMoney == null || tradeAmountMoney.sign <= 0) { + throw Exception('Trade amount must be greater than 0'); + } + final tradeAmountDouble = double.tryParse(tradeAmountMoney.toString()) ?? 0.0; if (trade.isSendAll == true) { if (provider is NearIntentsExchangeProvider) {