From 4c2d0613635a1457c2a63ba788f15dae170fcae9 Mon Sep 17 00:00:00 2001 From: Serhii Date: Fri, 23 Aug 2024 16:19:42 +0300 Subject: [PATCH] rbf fixes issues sum utxo and fee calculation (#1625) * total out amount issue * fix empty inputs and outputs addresses for new tx * fix sum value of utxo not spending * Update configure.dart * Update electrum_wallet.dart * receiving address * review fixes --- cw_bitcoin/lib/electrum_transaction_info.dart | 2 +- cw_bitcoin/lib/electrum_wallet.dart | 133 +++++++++++------- lib/bitcoin/cw_bitcoin.dart | 5 +- lib/view_model/send/send_view_model.dart | 44 ++++-- .../transaction_details_view_model.dart | 23 +-- tool/configure.dart | 3 +- 6 files changed, 132 insertions(+), 78 deletions(-) diff --git a/cw_bitcoin/lib/electrum_transaction_info.dart b/cw_bitcoin/lib/electrum_transaction_info.dart index ea4a3de33..ee3daa0e0 100644 --- a/cw_bitcoin/lib/electrum_transaction_info.dart +++ b/cw_bitcoin/lib/electrum_transaction_info.dart @@ -235,6 +235,6 @@ class ElectrumTransactionInfo extends TransactionInfo { } String toString() { - return 'ElectrumTransactionInfo(id: $id, height: $height, amount: $amount, fee: $fee, direction: $direction, date: $date, isPending: $isPending, confirmations: $confirmations, to: $to, unspent: $unspents)'; + return 'ElectrumTransactionInfo(id: $id, height: $height, amount: $amount, fee: $fee, direction: $direction, date: $date, isPending: $isPending, confirmations: $confirmations, to: $to, unspent: $unspents, inputAddresses: $inputAddresses, outputAddresses: $outputAddresses)'; } } diff --git a/cw_bitcoin/lib/electrum_wallet.dart b/cw_bitcoin/lib/electrum_wallet.dart index da91a29e4..2a57c8d5c 100644 --- a/cw_bitcoin/lib/electrum_wallet.dart +++ b/cw_bitcoin/lib/electrum_wallet.dart @@ -132,6 +132,7 @@ abstract class ElectrumWalletBase final String? _mnemonic; Bip32Slip10Secp256k1 get hd => accountHD.childKey(Bip32KeyIndex(0)); + Bip32Slip10Secp256k1 get sideHd => accountHD.childKey(Bip32KeyIndex(1)); final EncryptionFileUtils encryptionFileUtils; @@ -1363,26 +1364,15 @@ abstract class ElectrumWalletBase } } - Future canReplaceByFee(String hash) async { - final verboseTransaction = await electrumClient.getTransactionVerbose(hash: hash); - - final String? transactionHex; - int confirmations = 0; - - if (verboseTransaction.isEmpty) { - transactionHex = await electrumClient.getTransactionHex(hash: hash); - } else { - confirmations = verboseTransaction['confirmations'] as int? ?? 0; - transactionHex = verboseTransaction['hex'] as String?; - } - - if (confirmations > 0) return false; - - if (transactionHex == null || transactionHex.isEmpty) { + Future canReplaceByFee(ElectrumTransactionInfo tx) async { + try { + final bundle = await getTransactionExpanded(hash: tx.txHash); + _updateInputsAndOutputs(tx, bundle); + if (bundle.confirmations > 0) return false; + return bundle.originalTransaction.canReplaceByFee; + } catch (e) { return false; } - - return BtcTransaction.fromRaw(transactionHex).canReplaceByFee; } Future isChangeSufficientForFee(String txId, int newFee) async { @@ -1458,47 +1448,59 @@ abstract class ElectrumWalletBase ); } - int totalOutAmount = bundle.originalTransaction.outputs - .fold(0, (previousValue, element) => previousValue + element.amount.toInt()); - - var currentFee = allInputsAmount - totalOutAmount; - int remainingFee = newFee - currentFee; - + // Create a list of available outputs final outputs = []; - - // Add outputs and deduct the fees from it - for (int i = bundle.originalTransaction.outputs.length - 1; i >= 0; i--) { - final out = bundle.originalTransaction.outputs[i]; + for (final out in bundle.originalTransaction.outputs) { final address = addressFromOutputScript(out.scriptPubKey, network); final btcAddress = addressTypeFromStr(address, network); + outputs.add(BitcoinOutput(address: btcAddress, value: BigInt.from(out.amount.toInt()))); + } - int newAmount; - if (out.amount.toInt() >= remainingFee) { - newAmount = out.amount.toInt() - remainingFee; - remainingFee = 0; + // Calculate the total amount and fees + int totalOutAmount = + outputs.fold(0, (previousValue, output) => previousValue + output.value.toInt()); + int currentFee = allInputsAmount - totalOutAmount; + int remainingFee = newFee - currentFee; - // if new amount of output is less than dust amount, then don't add this output as well - if (newAmount <= _dustAmount) { - continue; + if (remainingFee <= 0) { + throw Exception("New fee must be higher than the current fee."); + } + + // Deduct Remaining Fee from Main Outputs + if (remainingFee > 0) { + for (int i = outputs.length - 1; i >= 0; i--) { + int outputAmount = outputs[i].value.toInt(); + + if (outputAmount > _dustAmount) { + int deduction = (outputAmount - _dustAmount >= remainingFee) + ? remainingFee + : outputAmount - _dustAmount; + outputs[i] = BitcoinOutput( + address: outputs[i].address, value: BigInt.from(outputAmount - deduction)); + remainingFee -= deduction; + + if (remainingFee <= 0) break; } - } else { - remainingFee -= out.amount.toInt(); - continue; } - - outputs.add(BitcoinOutput(address: btcAddress, value: BigInt.from(newAmount))); } + // Final check if the remaining fee couldn't be deducted + if (remainingFee > 0) { + throw Exception("Not enough funds to cover the fee."); + } + + // Identify all change outputs final changeAddresses = walletAddresses.allAddresses.where((element) => element.isHidden); + final List changeOutputs = outputs + .where((output) => changeAddresses + .any((element) => element.address == output.address.toAddress(network))) + .toList(); - // look for a change address in the outputs - final changeOutput = outputs.firstWhereOrNull((output) => - changeAddresses.any((element) => element.address == output.address.toAddress(network))); + int totalChangeAmount = + changeOutputs.fold(0, (sum, output) => sum + output.value.toInt()); - // deduct the change amount from the output amount - if (changeOutput != null) { - totalOutAmount -= changeOutput.value.toInt(); - } + // The final amount that the receiver will receive + int sendingAmount = allInputsAmount - newFee - totalChangeAmount; final txb = BitcoinTransactionBuilder( utxos: utxos, @@ -1527,10 +1529,10 @@ abstract class ElectrumWalletBase transaction, type, electrumClient: electrumClient, - amount: totalOutAmount, + amount: sendingAmount, fee: newFee, network: network, - hasChange: changeOutput != null, + hasChange: changeOutputs.isNotEmpty, feeRate: newFee.toString(), )..addListener((transaction) async { transactionHistory.addOne(transaction); @@ -2026,6 +2028,39 @@ abstract class ElectrumWalletBase }); } } + + void _updateInputsAndOutputs(ElectrumTransactionInfo tx, ElectrumTransactionBundle bundle) { + tx.inputAddresses = tx.inputAddresses?.where((address) => address.isNotEmpty).toList(); + + if (tx.inputAddresses == null || + tx.inputAddresses!.isEmpty || + tx.outputAddresses == null || + tx.outputAddresses!.isEmpty) { + List inputAddresses = []; + List outputAddresses = []; + + for (int i = 0; i < bundle.originalTransaction.inputs.length; i++) { + final input = bundle.originalTransaction.inputs[i]; + final inputTransaction = bundle.ins[i]; + final vout = input.txIndex; + final outTransaction = inputTransaction.outputs[vout]; + final address = addressFromOutputScript(outTransaction.scriptPubKey, network); + + if (address.isNotEmpty) inputAddresses.add(address); + } + + for (int i = 0; i < bundle.originalTransaction.outputs.length; i++) { + final out = bundle.originalTransaction.outputs[i]; + final address = addressFromOutputScript(out.scriptPubKey, network); + + if (address.isNotEmpty) outputAddresses.add(address); + } + tx.inputAddresses = inputAddresses; + tx.outputAddresses = outputAddresses; + + transactionHistory.addOne(tx); + } + } } class ScanNode { diff --git a/lib/bitcoin/cw_bitcoin.dart b/lib/bitcoin/cw_bitcoin.dart index 5a71e3549..d979f9281 100644 --- a/lib/bitcoin/cw_bitcoin.dart +++ b/lib/bitcoin/cw_bitcoin.dart @@ -398,9 +398,10 @@ class CWBitcoin extends Bitcoin { } @override - Future canReplaceByFee(Object wallet, String transactionHash) async { + Future canReplaceByFee(Object wallet, Object transactionInfo) async { final bitcoinWallet = wallet as ElectrumWallet; - return bitcoinWallet.canReplaceByFee(transactionHash); + final tx = transactionInfo as ElectrumTransactionInfo; + return bitcoinWallet.canReplaceByFee(tx); } @override diff --git a/lib/view_model/send/send_view_model.dart b/lib/view_model/send/send_view_model.dart index 863c83957..22c083455 100644 --- a/lib/view_model/send/send_view_model.dart +++ b/lib/view_model/send/send_view_model.dart @@ -18,6 +18,7 @@ import 'package:cake_wallet/view_model/dashboard/balance_view_model.dart'; import 'package:cake_wallet/view_model/hardware_wallet/ledger_view_model.dart'; import 'package:cake_wallet/wownero/wownero.dart'; import 'package:cw_core/exceptions.dart'; +import 'package:cw_core/transaction_info.dart'; import 'package:cw_core/transaction_priority.dart'; import 'package:cake_wallet/view_model/send/output.dart'; import 'package:cake_wallet/view_model/send/send_template_view_model.dart'; @@ -392,25 +393,38 @@ abstract class SendViewModelBase extends WalletChangeListenerViewModel with Stor } @action - Future replaceByFee(String txId, String newFee) async { + Future replaceByFee(TransactionInfo tx, String newFee) async { state = IsExecutingState(); - final isSufficient = await bitcoin!.isChangeSufficientForFee(wallet, txId, newFee); + try { + final isSufficient = await bitcoin!.isChangeSufficientForFee(wallet, tx.id, newFee); - if (!isSufficient) { - state = AwaitingConfirmationState( - title: S.current.confirm_fee_deduction, - message: S.current.confirm_fee_deduction_content, - onConfirm: () async { - pendingTransaction = await bitcoin!.replaceByFee(wallet, txId, newFee); - state = ExecutedSuccessfullyState(); - }, - onCancel: () { - state = FailureState('Insufficient change for fee'); - }); - } else { - pendingTransaction = await bitcoin!.replaceByFee(wallet, txId, newFee); + if (!isSufficient) { + state = AwaitingConfirmationState( + title: S.current.confirm_fee_deduction, + message: S.current.confirm_fee_deduction_content, + onConfirm: () async => await _executeReplaceByFee(tx, newFee), + onCancel: () => state = FailureState('Insufficient change for fee')); + } else { + await _executeReplaceByFee(tx, newFee); + } + } catch (e) { + state = FailureState(e.toString()); + } + } + + Future _executeReplaceByFee(TransactionInfo tx, String newFee) async { + + + clearOutputs(); + final output = outputs.first; + output.address = tx.outputAddresses?.first ?? ''; + + try { + pendingTransaction = await bitcoin!.replaceByFee(wallet, tx.id, newFee); state = ExecutedSuccessfullyState(); + } catch (e) { + state = FailureState(e.toString()); } } diff --git a/lib/view_model/transaction_details_view_model.dart b/lib/view_model/transaction_details_view_model.dart index 18715e508..aa63ce860 100644 --- a/lib/view_model/transaction_details_view_model.dart +++ b/lib/view_model/transaction_details_view_model.dart @@ -52,7 +52,7 @@ abstract class TransactionDetailsViewModelBase with Store { case WalletType.bitcoin: _addElectrumListItems(tx, dateFormat); _addBumpFeesListItems(tx); - _checkForRBF(); + _checkForRBF(tx); break; case WalletType.litecoin: case WalletType.bitcoinCash: @@ -349,12 +349,15 @@ abstract class TransactionDetailsViewModelBase with Store { void _addBumpFeesListItems(TransactionInfo tx) { transactionPriority = bitcoin!.getBitcoinTransactionPriorityMedium(); + final inputsCount = (transactionInfo.inputAddresses?.isEmpty ?? true) + ? 1 + : transactionInfo.inputAddresses!.length; + final outputsCount = (transactionInfo.outputAddresses?.isEmpty ?? true) + ? 1 + : transactionInfo.outputAddresses!.length; newFee = bitcoin!.getFeeAmountForPriority( - wallet, - bitcoin!.getBitcoinTransactionPriorityMedium(), - transactionInfo.inputAddresses?.length ?? 1, - transactionInfo.outputAddresses?.length ?? 1); + wallet, bitcoin!.getBitcoinTransactionPriorityMedium(), inputsCount, outputsCount); RBFListItems.add(StandartListItem(title: S.current.old_fee, value: tx.feeFormatted() ?? '0.0')); @@ -383,12 +386,12 @@ abstract class TransactionDetailsViewModelBase with Store { return setNewFee(value: sliderValue, priority: transactionPriority!); })); - if (transactionInfo.inputAddresses != null) { + if (transactionInfo.inputAddresses != null && transactionInfo.inputAddresses!.isNotEmpty) { RBFListItems.add(StandardExpandableListItem( title: S.current.inputs, expandableItems: transactionInfo.inputAddresses!)); } - if (transactionInfo.outputAddresses != null) { + if (transactionInfo.outputAddresses != null && transactionInfo.outputAddresses!.isNotEmpty) { RBFListItems.add(StandardExpandableListItem( title: S.current.outputs, expandableItems: transactionInfo.outputAddresses!)); } @@ -416,10 +419,10 @@ abstract class TransactionDetailsViewModelBase with Store { } @action - Future _checkForRBF() async { + Future _checkForRBF(TransactionInfo tx) async { if (wallet.type == WalletType.bitcoin && transactionInfo.direction == TransactionDirection.outgoing) { - if (await bitcoin!.canReplaceByFee(wallet, transactionInfo.id)) { + if (await bitcoin!.canReplaceByFee(wallet, tx)) { _canReplaceByFee = true; } } @@ -441,7 +444,7 @@ abstract class TransactionDetailsViewModelBase with Store { return bitcoin!.formatterBitcoinAmountToString(amount: newFee); } - void replaceByFee(String newFee) => sendViewModel.replaceByFee(transactionInfo.id, newFee); + void replaceByFee(String newFee) => sendViewModel.replaceByFee(transactionInfo, newFee,); @computed String get pendingTransactionFiatAmountValueFormatted => sendViewModel.isFiatDisabled diff --git a/tool/configure.dart b/tool/configure.dart index a0104c34e..c9b6bbdda 100644 --- a/tool/configure.dart +++ b/tool/configure.dart @@ -79,6 +79,7 @@ import 'dart:typed_data'; import 'package:bitcoin_base/bitcoin_base.dart'; import 'package:cake_wallet/view_model/hardware_wallet/ledger_view_model.dart'; import 'package:cake_wallet/view_model/send/output.dart'; +import 'package:cw_bitcoin/electrum_transaction_info.dart'; import 'package:cw_core/hardware/hardware_account_data.dart'; import 'package:cw_core/node.dart'; import 'package:cw_core/output_info.dart'; @@ -204,7 +205,7 @@ abstract class Bitcoin { bool isTestnet(Object wallet); Future replaceByFee(Object wallet, String transactionHash, String fee); - Future canReplaceByFee(Object wallet, String transactionHash); + Future canReplaceByFee(Object wallet, Object tx); Future isChangeSufficientForFee(Object wallet, String txId, String newFee); int getFeeAmountForPriority(Object wallet, TransactionPriority priority, int inputsCount, int outputsCount, {int? size}); int getEstimatedFeeWithFeeRate(Object wallet, int feeRate, int? amount,