Cw 649 rbf improvements opt in #1772
Conversation
cw_bitcoin/lib/electrum_wallet.dart
Outdated
| if (utxo.utxo.isP2tr()) { | ||
| return key.signTapRoot(txDigest, sighash: sighash); |
There was a problem hiding this comment.
this was accidentally removed during a merge. I’ve restored it to handle Taproot signing
cw_bitcoin/lib/electrum_wallet.dart
Outdated
| @@ -1543,14 +1543,17 @@ abstract class ElectrumWalletBase extends WalletBase< | |||
| final bundle = await getTransactionExpanded(hash: txId); | |||
| final outputs = bundle.originalTransaction.outputs; | |||
|
|
|||
| final changeAddresses = walletAddresses.allAddresses.where((element) => element.isHidden); | |||
| final receiverAmount = outputs.isNotEmpty ? outputs[0].amount.toInt() : null; | |||
There was a problem hiding this comment.
what if it has multiple outputs?
why not just use outputs.fold
There was a problem hiding this comment.
Receiver amount is what the receiver should get, not just the sum of all outputs. I’ve improved this to sum outputs that aren’t change outputs, as using the first output isn’t reliable
| int deduction = (outputAmount - _dustAmount >= remainingFee) | ||
| ? remainingFee | ||
| : outputAmount - _dustAmount; | ||
|
|
||
| outputs[i] = BitcoinOutput( | ||
| address: outputs[i].address, value: BigInt.from(outputAmount - deduction)); | ||
| address: output.address, value: BigInt.from(outputAmount - deduction)); |
There was a problem hiding this comment.
why do we keep the output with the dust amount?
why not just check if the outputAmount - remainingFee > _dustAmount then simply deduct the remaining fee from the output and break
else, then deduct the whole output amount from remainingFee and remove the whole output, and decrement the variable i
i.e
if (outputAmount - remainingFee > _dustAmount) {
_remainingFee = 0;
outputs[i] = BitcoinOutput(
address: output.address, value: BigInt.from(outputAmount - remainingFee));
break;
} else {
_remainingFee -= outputAmount;
outputs.remove(i);
i--;
}There was a problem hiding this comment.
I’ve double-checked, and if I remove the output completely, I get the error “Sum value of utxo not spending” in some cases because the transaction becomes unbalanced—the inputs no longer equal the outputs plus the fee.
There was a problem hiding this comment.
but like this it might also error out because of the dust amount
my solution is to remove it, and you can add the remaining funds (which would be a dust amount) to the fees, so that the transaction becomes balanced again
Issue Number (if Applicable): Fixes #
Description
Please include a summary of the changes and which issue is fixed / feature is added.
Pull Request - Checklist