Skip to content

Commit 12635e6

Browse files
afiliniafilini
committed
[wallet] Refactor Wallet::bump_fee() 
1 parent a5713a8 commit 12635e6

File tree

1 file changed

+168
-141
lines changed

1 file changed

+168
-141
lines changed

‎src/wallet/mod.rs‎

Lines changed: 168 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ where
393393
};
394394

395395
let mut fee_amount = fee_amount.ceil() as u64;
396-
let change_val = selected_amount - outgoing - fee_amount;
396+
let change_val = (selected_amount - outgoing).saturating_sub(fee_amount);
397397
if !builder.send_all && !change_val.is_dust() {
398398
let mut change_output = change_output.unwrap();
399399
change_output.value = change_val;
@@ -410,7 +410,7 @@ where
410410
}
411411
} else if !builder.send_all && change_val.is_dust() {
412412
// skip the change output because it's dust, this adds up to the fees
413-
fee_amount += change_val;
413+
fee_amount += selected_amount - outgoing;
414414
} else if builder.send_all {
415415
// send_all but the only output would be below dust limit
416416
return Err(Error::InsufficientFunds); // TODO: or OutputBelowDustLimit?
@@ -443,6 +443,9 @@ where
443443
/// option must be enabled when bumping its fees to correctly reduce the only output's value to
444444
/// increase the fees.
445445
///
446+
/// If the `builder` specifies some `utxos` that must be spent, they will be added to the
447+
/// transaction regardless of whether they are necessary or not to cover additional fees.
448+
///
446449
/// ## Example
447450
///
448451
/// ```no_run
@@ -489,153 +492,194 @@ where
489492
required: required_feerate,
490493
});
491494
}
492-
let mut fee_difference =
493-
(new_feerate.as_sat_vb() * tx.get_weight() as f32 / 4.0).ceil() as u64 - details.fees;
494495

495496
if builder.send_all && tx.output.len() > 1 {
496497
return Err(Error::SendAllMultipleOutputs);
497498
}
498499

499500
// find the index of the output that we can update. either the change or the only one if
500501
// it's `send_all`
501-
let updatable_output = if builder.send_all {
502-
0
503-
} else {
504-
let mut change_output = None;
505-
for (index, txout) in tx.output.iter().enumerate() {
506-
// look for an output that we know and that has the right ScriptType. We use
507-
// `get_descriptor_for` to find what's the ScriptType for `Internal`
508-
// addresses really is, because if there's no change_descriptor it's actually equal
509-
// to "External"
510-
let (_, change_type) = self.get_descriptor_for_script_type(ScriptType::Internal);
511-
match self
512-
.database
513-
.borrow()
514-
.get_path_from_script_pubkey(&txout.script_pubkey)?
515-
{
516-
Some((script_type, _)) if script_type == change_type => {
517-
change_output = Some(index);
518-
break;
502+
let updatable_output = match builder.send_all {
503+
true => Some(0),
504+
false => {
505+
let mut change_output = None;
506+
for (index, txout) in tx.output.iter().enumerate() {
507+
// look for an output that we know and that has the right ScriptType. We use
508+
// `get_descriptor_for` to find what's the ScriptType for `Internal`
509+
// addresses really is, because if there's no change_descriptor it's actually equal
510+
// to "External"
511+
let (_, change_type) =
512+
self.get_descriptor_for_script_type(ScriptType::Internal);
513+
match self
514+
.database
515+
.borrow()
516+
.get_path_from_script_pubkey(&txout.script_pubkey)?
517+
{
518+
Some((script_type, _)) if script_type == change_type => {
519+
change_output = Some(index);
520+
break;
521+
}
522+
_ => {}
519523
}
520-
_ => {}
521524
}
522-
}
523525

524-
// we need a change output, add one here and take into account the extra fees for it
525-
let change_script = self.get_change_address()?;
526-
change_output.unwrap_or_else(|| {
526+
change_output
527+
}
528+
};
529+
let updatable_output = match updatable_output {
530+
Some(updatable_output) => updatable_output,
531+
None => {
532+
// we need a change output, add one here and take into account the extra fees for it
533+
let change_script = self.get_change_address()?;
527534
let change_txout = TxOut {
528535
script_pubkey: change_script,
529536
value: 0,
530537
};
531-
fee_difference +=
532-
(serialize(&change_txout).len() as f32 * new_feerate.as_sat_vb()).ceil() as u64;
533538
tx.output.push(change_txout);
534539

535540
tx.output.len() - 1
536-
})
541+
}
537542
};
538543

539-
// if `builder.utxos` is Some(_) we have to add inputs and we skip down to the last branch
540-
match tx.output[updatable_output]
541-
.value
542-
.checked_sub(fee_difference)
543-
{
544-
Some(new_value) if !new_value.is_dust() && builder.utxos.is_none() => {
545-
// try to reduce the "updatable output" amount
546-
tx.output[updatable_output].value = new_value;
547-
if self.is_mine(&tx.output[updatable_output].script_pubkey)? {
548-
details.received -= fee_difference;
549-
}
544+
// initially always remove the output we can change
545+
let mut removed_updatable_output = tx.output.remove(updatable_output);
546+
if self.is_mine(&removed_updatable_output.script_pubkey)? {
547+
details.received -= removed_updatable_output.value;
548+
}
550549

551-
details.fees += fee_difference;
552-
}
553-
_ if builder.send_all && builder.utxos.is_none() => {
554-
// if the tx is "send_all" it doesn't make sense to either remove the only output
555-
// or add more inputs
556-
return Err(Error::InsufficientFunds);
557-
}
558-
_ => {
559-
// initially always remove the change output
560-
let mut removed_change_output = tx.output.remove(updatable_output);
561-
if self.is_mine(&removed_change_output.script_pubkey)? {
562-
details.received -= removed_change_output.value;
563-
}
550+
let external_weight = self
551+
.get_descriptor_for_script_type(ScriptType::External)
552+
.0
553+
.max_satisfaction_weight();
554+
let internal_weight = self
555+
.get_descriptor_for_script_type(ScriptType::Internal)
556+
.0
557+
.max_satisfaction_weight();
564558

565-
// we want to add more inputs if:
566-
// - builder.utxos tells us to do so
567-
// - the removed change value is lower than the fee_difference we want to add
568-
let needs_more_inputs =
569-
builder.utxos.is_some() || removed_change_output.value <= fee_difference;
570-
let added_amount = if needs_more_inputs {
571-
let (available_utxos, use_all_utxos) = self.get_available_utxos(
572-
builder.change_policy,
573-
&builder.utxos,
574-
&builder.unspendable,
575-
false,
576-
)?;
577-
let available_utxos = rbf::filter_available(
578-
self.database.borrow().deref(),
579-
available_utxos.into_iter(),
580-
)?;
581-
582-
let (must_use_utxos, may_use_utxos) = match use_all_utxos {
583-
true => (available_utxos, vec![]),
584-
false => (vec![], available_utxos),
585-
};
586-
587-
let coin_selection::CoinSelectionResult {
588-
txin,
589-
selected_amount,
590-
fee_amount,
591-
} = builder.coin_selection.coin_select(
592-
self.database.borrow().deref(),
593-
must_use_utxos,
594-
may_use_utxos,
595-
new_feerate,
596-
fee_difference.saturating_sub(removed_change_output.value),
597-
0.0,
598-
)?;
599-
fee_difference += fee_amount.ceil() as u64;
600-
601-
// add the new inputs
602-
let (mut txin, _): (Vec<_>, Vec<_>) = txin.into_iter().unzip();
603-
604-
// TODO: use tx_builder.sequence ??
605-
// copy the n_sequence from the inputs that were already in the transaction
606-
txin.iter_mut()
607-
.for_each(|i| i.sequence = tx.input[0].sequence);
608-
tx.input.extend_from_slice(&txin);
609-
610-
details.sent += selected_amount;
611-
selected_amount
612-
} else {
613-
// otherwise just remove the output and add 0 new coins
614-
0
615-
};
559+
let original_sequence = tx.input[0].sequence;
616560

617-
match (removed_change_output.value + added_amount).checked_sub(fee_difference) {
618-
None => return Err(Error::InsufficientFunds),
619-
Some(new_value) if new_value.is_dust() => {
620-
// the change would be dust, add that to fees
621-
details.fees += fee_difference + new_value;
622-
}
623-
Some(new_value) => {
624-
// add the change back
625-
removed_change_output.value = new_value;
626-
tx.output.push(removed_change_output);
561+
// remove the inputs from the tx and process them
562+
let original_txin = tx.input.drain(..).collect::<Vec<_>>();
563+
let mut original_utxos = original_txin
564+
.iter()
565+
.map(|txin| -> Result<(UTXO, usize), Error> {
566+
let txout = self
567+
.database
568+
.borrow()
569+
.get_previous_output(&txin.previous_output)?
570+
.ok_or(Error::UnknownUTXO)?;
627571

628-
details.received += new_value;
629-
details.fees += fee_difference;
572+
let (weight, is_internal) = match self
573+
.database
574+
.borrow()
575+
.get_path_from_script_pubkey(&txout.script_pubkey)?
576+
{
577+
Some((ScriptType::Internal, _)) => (internal_weight, true),
578+
Some((ScriptType::External, _)) => (external_weight, false),
579+
None => {
580+
// estimate the weight based on the scriptsig/witness size present in the
581+
// original transaction
582+
let weight =
583+
serialize(&txin.script_sig).len() * 4 + serialize(&txin.witness).len();
584+
(weight, false)
630585
}
631-
}
632-
}
586+
};
587+
588+
let utxo = UTXO {
589+
outpoint: txin.previous_output,
590+
txout,
591+
is_internal,
592+
};
593+
594+
Ok((utxo, weight))
595+
})
596+
.collect::<Result<Vec<_>, _>>()?;
597+
598+
let builder_extra_utxos = builder.utxos.as_ref().map(|utxos| {
599+
utxos
600+
.iter()
601+
.filter(|utxo| {
602+
!original_txin
603+
.iter()
604+
.any(|txin| &&txin.previous_output == utxo)
605+
})
606+
.cloned()
607+
.collect()
608+
});
609+
let (available_utxos, use_all_utxos) = self.get_available_utxos(
610+
builder.change_policy,
611+
&builder_extra_utxos,
612+
&builder.unspendable,
613+
false,
614+
)?;
615+
let available_utxos =
616+
rbf::filter_available(self.database.borrow().deref(), available_utxos.into_iter())?;
617+
618+
let (mut must_use_utxos, may_use_utxos) = match use_all_utxos {
619+
true => (available_utxos, vec![]),
620+
false => (vec![], available_utxos),
633621
};
622+
must_use_utxos.append(&mut original_utxos);
623+
624+
let amount_needed = tx.output.iter().fold(0, |acc, out| acc + out.value);
625+
let initial_fee = tx.get_weight() as f32 / 4.0 * new_feerate.as_sat_vb();
626+
let coin_selection::CoinSelectionResult {
627+
txin,
628+
selected_amount,
629+
fee_amount,
630+
} = builder.coin_selection.coin_select(
631+
self.database.borrow().deref(),
632+
must_use_utxos,
633+
may_use_utxos,
634+
new_feerate,
635+
amount_needed,
636+
initial_fee,
637+
)?;
638+
639+
let (mut txin, prev_script_pubkeys): (Vec<_>, Vec<_>) = txin.into_iter().unzip();
640+
// map that allows us to lookup the prev_script_pubkey for a given previous_output
641+
let prev_script_pubkeys = txin
642+
.iter()
643+
.zip(prev_script_pubkeys.into_iter())
644+
.map(|(txin, script)| (txin.previous_output, script))
645+
.collect::<HashMap<_, _>>();
646+
647+
// TODO: use builder.n_sequence??
648+
// use the same n_sequence
649+
txin.iter_mut().for_each(|i| i.sequence = original_sequence);
650+
tx.input = txin;
651+
652+
details.sent = selected_amount;
653+
654+
let mut fee_amount = fee_amount.ceil() as u64;
655+
let removed_output_fee_cost = (serialize(&removed_updatable_output).len() as f32
656+
* new_feerate.as_sat_vb())
657+
.ceil() as u64;
658+
659+
let change_val = selected_amount - amount_needed - fee_amount;
660+
let change_val_after_add = change_val.saturating_sub(removed_output_fee_cost);
661+
if !builder.send_all && !change_val_after_add.is_dust() {
662+
removed_updatable_output.value = change_val_after_add;
663+
fee_amount += removed_output_fee_cost;
664+
details.received += change_val_after_add;
665+
666+
tx.output.push(removed_updatable_output);
667+
} else if builder.send_all && !change_val_after_add.is_dust() {
668+
removed_updatable_output.value = change_val_after_add;
669+
fee_amount += removed_output_fee_cost;
670+
671+
// send_all to our address
672+
if self.is_mine(&removed_updatable_output.script_pubkey)? {
673+
details.received = change_val_after_add;
674+
}
634675

635-
// clear witnesses
636-
for input in &mut tx.input {
637-
input.script_sig = Script::default();
638-
input.witness = vec![];
676+
tx.output.push(removed_updatable_output);
677+
} else if !builder.send_all && change_val_after_add.is_dust() {
678+
// skip the change output because it's dust, this adds up to the fees
679+
fee_amount += change_val;
680+
} else if builder.send_all {
681+
// send_all but the only output would be below dust limit
682+
return Err(Error::InsufficientFunds); // TODO: or OutputBelowDustLimit?
639683
}
640684

641685
// sort input/outputs according to the chosen algorithm
@@ -644,26 +688,9 @@ where
644688
// TODO: check that we are not replacing more than 100 txs from mempool
645689

646690
details.txid = tx.txid();
691+
details.fees = fee_amount;
647692
details.timestamp = time::get_timestamp();
648693

649-
let prev_script_pubkeys = tx
650-
.input
651-
.iter()
652-
.map(|txin| {
653-
Ok((
654-
txin.previous_output,
655-
self.database
656-
.borrow()
657-
.get_previous_output(&txin.previous_output)?,
658-
))
659-
})
660-
.collect::<Result<Vec<_>, Error>>()?
661-
.into_iter()
662-
.filter_map(|(outpoint, txout)| match txout {
663-
Some(txout) => Some((outpoint, txout.script_pubkey)),
664-
None => None,
665-
})
666-
.collect();
667694
let psbt = self.complete_transaction(tx, prev_script_pubkeys, builder)?;
668695

669696
Ok((psbt, details))

0 commit comments

Comments
 (0)