diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 467edce590..4caca5ec5e 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -813,9 +813,12 @@ where } }; - fee_amount += fee_rate.fee_vb(serialize(&drain_output).len()); - - let drain_val = (coin_selection.selected_amount() - outgoing).saturating_sub(fee_amount); + // sum(inputs) - sum(outputs) + let delta = coin_selection.selected_amount() - outgoing; + // Fee to be paid if we add the drain_output + let drain_fee = fee_rate.fee_vb(serialize(&drain_output).len()); + // Value of the potential drain output + let drain_val = delta.saturating_sub(fee_amount).saturating_sub(drain_fee); if tx.output.is_empty() { // Uh oh, our transaction has no outputs. @@ -838,12 +841,13 @@ where } if drain_val.is_dust(&drain_output.script_pubkey) { - fee_amount += drain_val; + fee_amount = delta; } else { drain_output.value = drain_val; if self.is_mine(&drain_output.script_pubkey)? { received += drain_val; } + fee_amount = delta - drain_val; // TODO: We should pay attention when adding a new output: this might increase // the lenght of the "number of vouts" parameter by 2 bytes, potentially making // our feerate too low @@ -3969,6 +3973,45 @@ pub(crate) mod test { builder.finish().unwrap(); } + #[test] + fn test_fee_amount_negative_drain_val() { + // While building the transaction, bdk would calculate the drain_value + // as + // current_delta - fee_amount - drain_fee + // using saturating_sub, meaning that if the result would end up negative, + // it'll remain to zero instead. + // This caused a bug in master where we would calculate the wrong fee + // for a transaction. + let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); + let send_to = Address::from_str("tb1ql7w62elx9ucw4pj5lgw4l028hmuw80sndtntxt").unwrap(); + let incoming_txid = crate::populate_test_db!( + wallet.database.borrow_mut(), + testutils! (@tx ( (@external descriptors, 0) => 8859 ) (@confirmations 1)), + Some(100), + ); + + let mut builder = wallet.build_tx(); + builder + .add_recipient(send_to.script_pubkey(), 8630) + .add_utxo(OutPoint::new(incoming_txid, 0)) + .unwrap() + .enable_rbf() + .fee_rate(FeeRate::from_sat_per_vb(2.01)); + let (psbt, details) = builder.finish().unwrap(); + + assert!(psbt.inputs.len() == 1); + let fee_amount = psbt + .inputs + .iter() + .fold(0, |acc, i| acc + i.witness_utxo.as_ref().unwrap().value) + - psbt + .unsigned_tx + .output + .iter() + .fold(0, |acc, o| acc + o.value); + assert_eq!(fee_amount, details.fee.unwrap()); + } + #[test] fn test_sign_single_xprv() { let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");