From 9e4a7f4331d0ed9a26f5d1c73135a7ae06b4d8c8 Mon Sep 17 00:00:00 2001 From: Omar Hatem Date: Fri, 26 Apr 2024 22:29:31 +0300 Subject: [PATCH] Enhance bitcoin error message (#1399) * Enhance bitcoin error message * fix: unconfirmed spends, spend confirmed first, wrong balance exception * Minor fixes --------- Co-authored-by: Rafael Saes --- cw_bitcoin/lib/electrum_wallet.dart | 44 +++++++++++++++++-- cw_bitcoin/lib/exceptions.dart | 4 +- .../lib/pending_bitcoin_transaction.dart | 2 + cw_bitcoin/pubspec.lock | 34 +++++++------- .../src/pending_bitcoin_cash_transaction.dart | 2 + cw_core/lib/exceptions.dart | 6 ++- cw_core/lib/unspent_transaction_output.dart | 1 + lib/src/screens/send/send_page.dart | 2 +- lib/view_model/send/send_view_model.dart | 2 +- 9 files changed, 72 insertions(+), 25 deletions(-) diff --git a/cw_bitcoin/lib/electrum_wallet.dart b/cw_bitcoin/lib/electrum_wallet.dart index 6b305722e..4a76ee5dd 100644 --- a/cw_bitcoin/lib/electrum_wallet.dart +++ b/cw_bitcoin/lib/electrum_wallet.dart @@ -203,10 +203,14 @@ abstract class ElectrumWalletBase List privateKeys = []; int allInputsAmount = 0; + bool spendsUnconfirmedTX = false; + for (int i = 0; i < unspentCoins.length; i++) { final utx = unspentCoins[i]; - if (utx.isSending) { + if (utx.isSending && !utx.isFrozen) { + if (!spendsUnconfirmedTX) spendsUnconfirmedTX = utx.confirmations == 0; + allInputsAmount += utx.value; final address = addressTypeFromStr(utx.address, network); @@ -264,6 +268,10 @@ abstract class ElectrumWalletBase // Here, when sending all, the output amount equals to the input value - fee to fully spend every input on the transaction and have no amount left for change int amount = allInputsAmount - fee; + if (amount <= 0) { + throw BitcoinTransactionWrongBalanceException(); + } + // Attempting to send less than the dust limit if (_isBelowDust(amount)) { throw BitcoinTransactionNoDustException(); @@ -288,6 +296,7 @@ abstract class ElectrumWalletBase isSendAll: true, hasChange: false, memo: memo, + spendsUnconfirmedTX: spendsUnconfirmedTX, ); } @@ -297,17 +306,25 @@ abstract class ElectrumWalletBase int feeRate, { int? inputsCount, String? memo, + bool? useUnconfirmed, }) async { final utxos = []; List privateKeys = []; int allInputsAmount = 0; + bool spendsUnconfirmedTX = false; int leftAmount = credentialsAmount; - final sendingCoins = unspentCoins.where((utx) => utx.isSending).toList(); + final sendingCoins = unspentCoins.where((utx) => utx.isSending && !utx.isFrozen).toList(); + final unconfirmedCoins = sendingCoins.where((utx) => utx.confirmations == 0).toList(); for (int i = 0; i < sendingCoins.length; i++) { final utx = sendingCoins[i]; + final isUncormirmed = utx.confirmations == 0; + if (useUnconfirmed != true && isUncormirmed) continue; + + if (!spendsUnconfirmedTX) spendsUnconfirmedTX = isUncormirmed; + allInputsAmount += utx.value; leftAmount = leftAmount - utx.value; @@ -345,11 +362,23 @@ abstract class ElectrumWalletBase } final spendingAllCoins = sendingCoins.length == utxos.length; + final spendingAllConfirmedCoins = + !spendsUnconfirmedTX && utxos.length == sendingCoins.length - unconfirmedCoins.length; // How much is being spent - how much is being sent int amountLeftForChangeAndFee = allInputsAmount - credentialsAmount; if (amountLeftForChangeAndFee <= 0) { + if (!spendingAllCoins) { + return estimateTxForAmount( + credentialsAmount, + outputs, + feeRate, + inputsCount: utxos.length + 1, + memo: memo, + useUnconfirmed: useUnconfirmed ?? spendingAllConfirmedCoins, + ); + } throw BitcoinTransactionWrongBalanceException(); } @@ -403,6 +432,7 @@ abstract class ElectrumWalletBase feeRate, inputsCount: utxos.length + 1, memo: memo, + useUnconfirmed: useUnconfirmed ?? spendingAllConfirmedCoins, ); } @@ -449,6 +479,7 @@ abstract class ElectrumWalletBase feeRate, inputsCount: utxos.length + 1, memo: memo, + useUnconfirmed: useUnconfirmed ?? spendingAllConfirmedCoins, ); } } @@ -461,6 +492,7 @@ abstract class ElectrumWalletBase hasChange: true, isSendAll: false, memo: memo, + spendsUnconfirmedTX: spendsUnconfirmedTX, ); } @@ -531,7 +563,7 @@ abstract class ElectrumWalletBase network: network, memo: estimatedTx.memo, outputOrdering: BitcoinOrdering.none, - enableRBF: true, + enableRBF: !estimatedTx.spendsUnconfirmedTX, ); } else { txb = BitcoinTransactionBuilder( @@ -541,7 +573,7 @@ abstract class ElectrumWalletBase network: network, memo: estimatedTx.memo, outputOrdering: BitcoinOrdering.none, - enableRBF: true, + enableRBF: !estimatedTx.spendsUnconfirmedTX, ); } @@ -721,6 +753,7 @@ abstract class ElectrumWalletBase final tx = await fetchTransactionInfo( hash: coin.hash, height: 0, myAddresses: addressesSet); coin.isChange = tx?.direction == TransactionDirection.outgoing; + coin.confirmations = tx?.confirmations; updatedUnspentCoins.add(coin); } catch (_) {} })))); @@ -745,6 +778,7 @@ abstract class ElectrumWalletBase coin.isFrozen = coinInfo.isFrozen; coin.isSending = coinInfo.isSending; coin.note = coinInfo.note; + coin.bitcoinAddressRecord.balance += coinInfo.value; } else { _addCoinInfo(coin); } @@ -1272,6 +1306,7 @@ class EstimatedTxResult { required this.hasChange, required this.isSendAll, this.memo, + required this.spendsUnconfirmedTX, }); final List utxos; @@ -1281,6 +1316,7 @@ class EstimatedTxResult { final bool hasChange; final bool isSendAll; final String? memo; + final bool spendsUnconfirmedTX; } BitcoinBaseAddress addressTypeFromStr(String address, BasedUtxoNetwork network) { diff --git a/cw_bitcoin/lib/exceptions.dart b/cw_bitcoin/lib/exceptions.dart index 4b03eb922..979c1a433 100644 --- a/cw_bitcoin/lib/exceptions.dart +++ b/cw_bitcoin/lib/exceptions.dart @@ -15,7 +15,9 @@ class BitcoinTransactionNoDustOnChangeException extends TransactionNoDustOnChang BitcoinTransactionNoDustOnChangeException(super.max, super.min); } -class BitcoinTransactionCommitFailed extends TransactionCommitFailed {} +class BitcoinTransactionCommitFailed extends TransactionCommitFailed { + BitcoinTransactionCommitFailed({super.errorMessage}); +} class BitcoinTransactionCommitFailedDustChange extends TransactionCommitFailedDustChange {} diff --git a/cw_bitcoin/lib/pending_bitcoin_transaction.dart b/cw_bitcoin/lib/pending_bitcoin_transaction.dart index 529ac61da..a59b4f429 100644 --- a/cw_bitcoin/lib/pending_bitcoin_transaction.dart +++ b/cw_bitcoin/lib/pending_bitcoin_transaction.dart @@ -73,7 +73,9 @@ class PendingBitcoinTransaction with PendingTransaction { if (error.contains("bad-txns-vout-negative")) { throw BitcoinTransactionCommitFailedVoutNegative(); } + throw BitcoinTransactionCommitFailed(errorMessage: error); } + throw BitcoinTransactionCommitFailed(); } diff --git a/cw_bitcoin/pubspec.lock b/cw_bitcoin/pubspec.lock index 53cf550c8..50cd432c0 100644 --- a/cw_bitcoin/pubspec.lock +++ b/cw_bitcoin/pubspec.lock @@ -21,10 +21,10 @@ packages: dependency: transitive description: name: args - sha256: eef6c46b622e0494a36c5a12d10d77fb4e855501a91c1b9ef9339326e58f0596 + sha256: "7cf60b9f0cc88203c5a190b4cd62a99feea42759a7fa695010eb5de1c0b2252a" url: "https://pub.dev" source: hosted - version: "2.4.2" + version: "2.5.0" asn1lib: dependency: transitive description: @@ -80,7 +80,7 @@ packages: description: path: "." ref: cake-update-v2 - resolved-ref: "3fd81d238b990bb767fc7a4fdd5053a22a142e2e" + resolved-ref: "01d844a5f5a520a31df5254e34169af4664aa769" url: "https://github.com/cake-tech/bitcoin_base.git" source: git version: "4.2.0" @@ -153,10 +153,10 @@ packages: dependency: "direct dev" description: name: build_runner - sha256: "581bacf68f89ec8792f5e5a0b2c4decd1c948e97ce659dc783688c8a88fbec21" + sha256: "3ac61a79bfb6f6cc11f693591063a7f19a7af628dc52f141743edac5c16e8c22" url: "https://pub.dev" source: hosted - version: "2.4.8" + version: "2.4.9" build_runner_core: dependency: transitive description: @@ -177,10 +177,10 @@ packages: dependency: transitive description: name: built_value - sha256: a3ec2e0f967bc47f69f95009bb93db936288d61d5343b9436e378b28a2f830c6 + sha256: c7913a9737ee4007efedaffc968c049fd0f3d0e49109e778edc10de9426005cb url: "https://pub.dev" source: hosted - version: "8.9.0" + version: "8.9.2" characters: dependency: transitive description: @@ -309,10 +309,10 @@ packages: dependency: "direct main" description: name: flutter_mobx - sha256: "4a5d062ff85ed3759f4aac6410ff0ffae32e324b2e71ca722ae1b37b32e865f4" + sha256: "859fbf452fa9c2519d2700b125dd7fb14c508bbdd7fb65e26ca8ff6c92280e2e" url: "https://pub.dev" source: hosted - version: "2.2.0+2" + version: "2.2.1+1" flutter_test: dependency: "direct dev" description: flutter @@ -322,10 +322,10 @@ packages: dependency: transitive description: name: frontend_server_client - sha256: "408e3ca148b31c20282ad6f37ebfa6f4bdc8fede5b74bc2f08d9d92b55db3612" + sha256: f64a0333a82f30b0cca061bc3d143813a486dc086b574bfb233b7c1372427694 url: "https://pub.dev" source: hosted - version: "3.2.0" + version: "4.0.0" glob: dependency: transitive description: @@ -466,10 +466,10 @@ packages: dependency: "direct main" description: name: mobx - sha256: "74ee54012dc7c1b3276eaa960a600a7418ef5f9997565deb8fca1fd88fb36b78" + sha256: "63920b27b32ad1910adfe767ab1750e4c212e8923232a1f891597b362074ea5e" url: "https://pub.dev" source: hosted - version: "2.3.0+1" + version: "2.3.3+2" mobx_codegen: dependency: "direct dev" description: @@ -570,10 +570,10 @@ packages: dependency: transitive description: name: pointycastle - sha256: "43ac87de6e10afabc85c445745a7b799e04de84cebaa4fd7bf55a5e1e9604d29" + sha256: "70fe966348fe08c34bf929582f1d8247d9d9408130723206472b4687227e4333" url: "https://pub.dev" source: hosted - version: "3.7.4" + version: "3.8.0" pool: dependency: transitive description: @@ -586,10 +586,10 @@ packages: dependency: transitive description: name: provider - sha256: "9a96a0a19b594dbc5bf0f1f27d2bc67d5f95957359b461cd9feb44ed6ae75096" + sha256: c8a055ee5ce3fd98d6fc872478b03823ffdb448699c6ebdbbc71d59b596fd48c url: "https://pub.dev" source: hosted - version: "6.1.1" + version: "6.1.2" pub_semver: dependency: transitive description: diff --git a/cw_bitcoin_cash/lib/src/pending_bitcoin_cash_transaction.dart b/cw_bitcoin_cash/lib/src/pending_bitcoin_cash_transaction.dart index da4710a8b..6d2ab4696 100644 --- a/cw_bitcoin_cash/lib/src/pending_bitcoin_cash_transaction.dart +++ b/cw_bitcoin_cash/lib/src/pending_bitcoin_cash_transaction.dart @@ -62,7 +62,9 @@ class PendingBitcoinCashTransaction with PendingTransaction { if (error.contains("bad-txns-vout-negative")) { throw BitcoinTransactionCommitFailedVoutNegative(); } + throw BitcoinTransactionCommitFailed(errorMessage: error); } + throw BitcoinTransactionCommitFailed(); } diff --git a/cw_core/lib/exceptions.dart b/cw_core/lib/exceptions.dart index 848ac40e6..d07da8109 100644 --- a/cw_core/lib/exceptions.dart +++ b/cw_core/lib/exceptions.dart @@ -19,7 +19,11 @@ class TransactionNoDustOnChangeException implements Exception { final String min; } -class TransactionCommitFailed implements Exception {} +class TransactionCommitFailed implements Exception { + final String? errorMessage; + + TransactionCommitFailed({this.errorMessage}); +} class TransactionCommitFailedDustChange implements Exception {} diff --git a/cw_core/lib/unspent_transaction_output.dart b/cw_core/lib/unspent_transaction_output.dart index b52daf43c..595df18f4 100644 --- a/cw_core/lib/unspent_transaction_output.dart +++ b/cw_core/lib/unspent_transaction_output.dart @@ -14,6 +14,7 @@ class Unspent { bool isChange; bool isSending; bool isFrozen; + int? confirmations; String note; bool get isP2wpkh => address.startsWith('bc') || address.startsWith('ltc'); diff --git a/lib/src/screens/send/send_page.dart b/lib/src/screens/send/send_page.dart index 970bb31f2..93fadea72 100644 --- a/lib/src/screens/send/send_page.dart +++ b/lib/src/screens/send/send_page.dart @@ -100,7 +100,7 @@ class SendPage extends BasePage { AppBarStyle get appBarStyle => AppBarStyle.transparent; double _sendCardHeight(BuildContext context) { - double initialHeight = 450; + double initialHeight = 480; if (sendViewModel.hasCoinControl) { initialHeight += 35; } diff --git a/lib/view_model/send/send_view_model.dart b/lib/view_model/send/send_view_model.dart index 2e00d1f0b..cabb723e1 100644 --- a/lib/view_model/send/send_view_model.dart +++ b/lib/view_model/send/send_view_model.dart @@ -562,7 +562,7 @@ abstract class SendViewModelBase extends WalletChangeListenerViewModel with Stor return S.current.tx_no_dust_exception; } if (error is TransactionCommitFailed) { - return S.current.tx_commit_failed; + return "${S.current.tx_commit_failed}${error.errorMessage != null ? "\n\n${error.errorMessage}" : ""}"; } if (error is TransactionCommitFailedDustChange) { return S.current.tx_rejected_dust_change;