Skip to content

Commit

Permalink
Revert "spend: add 10 sats to fee for 1 sat/vb txs"
Browse files Browse the repository at this point in the history
This reverts commit 74a53ba.

I also removed the extra sats being added in the functional test.
  • Loading branch information
jp1ac4 committed Nov 8, 2024
1 parent cf88aca commit 6e74a96
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 30 deletions.
18 changes: 9 additions & 9 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,7 @@ mod tests {

// Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 161 sats fees.
// At 2sats/vb, it's twice that.
assert_eq!(tx.output[1].value.to_sat(), 89_829);
assert_eq!(tx.output[1].value.to_sat(), 89_839);
let psbt = if let CreateSpendResult::Success { psbt, .. } = control
.create_spend(&destinations, &[dummy_op], 2, None)
.unwrap()
Expand Down Expand Up @@ -1565,18 +1565,18 @@ mod tests {
dummy_addr.payload().script_pubkey()
);
assert_eq!(tx.output[0].value.to_sat(), 95_000);
// change = 100_000 - 95_000 - /* fee without change */ 128 - /* extra fee for change output */ 43 = 4829
// change = 100_000 - 95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 = 4830
assert_eq!(
warnings,
vec![
"Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \
Instead of creating a change of 4829 sats, it was added to the \
Instead of creating a change of 4839 sats, it was added to the \
transaction fee. Select a larger input to avoid this from happening."
]
);

// Increase the target value by the change amount and the warning will disappear.
*destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_829;
*destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_839;
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap()
Expand All @@ -1591,7 +1591,7 @@ mod tests {

// Now increase target also by the extra fee that was paying for change and we can still create the spend.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 + 4_829 + /* fee for change output */ 43;
95_000 + 4_830 + /* fee for change output */ 43;
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap()
Expand All @@ -1606,15 +1606,15 @@ mod tests {

// Now increase the target by 1 more sat and we will have insufficient funds.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 + 4_829 + /* fee for change output */ 43 + 1;
95_000 + 4_839 + /* fee for change output */ 43 + 1;
assert_eq!(
control.create_spend(&destinations, &[dummy_op], 1, None),
Ok(CreateSpendResult::InsufficientFunds { missing: 1 }),
);

// Now decrease the target so that the lost change is just 1 sat.
*destinations.get_mut(&dummy_addr).unwrap() =
100_000 - /* fee without change */ 128 - /* extra fee for change output */ 43 - 1;
100_000 - /* fee without change */ 118 - /* extra fee for change output */ 43 - 1;
let warnings = if let CreateSpendResult::Success { warnings, .. } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap()
Expand All @@ -1635,7 +1635,7 @@ mod tests {

// Now decrease the target value so that we have enough for a change output.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 - /* fee without change */ 128 - /* extra fee for change output */ 43;
95_000 - /* fee without change */ 118 - /* extra fee for change output */ 43;
let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap()
Expand All @@ -1651,7 +1651,7 @@ mod tests {

// Now increase the target by 1 and we'll get a warning again, this time for 1 less than the dust threshold.
*destinations.get_mut(&dummy_addr).unwrap() =
95_000 - /* fee without change */ 128 - /* extra fee for change output */ 43 + 1;
95_000 - /* fee without change */ 118 - /* extra fee for change output */ 43 + 1;
let warnings = if let CreateSpendResult::Success { warnings, .. } = control
.create_spend(&destinations, &[dummy_op], 1, None)
.unwrap()
Expand Down
15 changes: 1 addition & 14 deletions src/spend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use bdk_coin_select::{
metrics::LowestFee, Candidate, ChangePolicy, CoinSelector, DrainWeights, FeeRate, Replace,
Target, TargetFee, TargetOutputs, TXIN_BASE_WEIGHT,
};
use log::info;
use miniscript::bitcoin::{
self,
absolute::{Height, LockTime},
Expand Down Expand Up @@ -385,21 +384,9 @@ fn select_coins_for_spend(
long_term_feerate,
);

// FIXME: This is a quick change to avoid going below the min relay fee:
// If the required feerate is 1 sat/vb and this is not a replacement tx,
// use the replaced_fee parameter to ensure we pay at least 10 sats more
// than the size of the tx.
// E.g. if vsize = 186, the fee will be pay >= 196 sats.
let replaced_fee_modified = if replaced_fee.is_none() && feerate_vb_u32 == 1 {
info!("setting replaced fee to 10");
Some(10)
} else {
replaced_fee
};

// Finally, run the coin selection algorithm. We use an opportunistic BnB and if it couldn't
// find any solution we fall back to selecting coins by descending value.
let replace = replaced_fee_modified.map(Replace::new);
let replace = replaced_fee.map(Replace::new);
let target_fee = TargetFee {
rate: feerate,
replace,
Expand Down
10 changes: 3 additions & 7 deletions tests/test_spend.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def sign_and_broadcast(psbt):
res = lianad.rpc.createspend(destinations, [outpoint], 1)
psbt = PSBT.from_base64(res["psbt"])
sign_and_broadcast(psbt)
change_amount = 848 if USE_TAPROOT else 829
change_amount = 858 if USE_TAPROOT else 839
assert len(psbt.o) == 1
assert len(res["warnings"]) == 1
assert (
Expand All @@ -155,7 +155,7 @@ def sign_and_broadcast(psbt):
res = lianad.rpc.createspend(destinations, [outpoint_3], 1)
psbt = PSBT.from_base64(res["psbt"])
sign_and_broadcast(psbt)
change_amount = 836 if USE_TAPROOT else 817
change_amount = 846 if USE_TAPROOT else 827
assert len(psbt.o) == 1
assert len(res["warnings"]) == 1
assert (
Expand Down Expand Up @@ -623,10 +623,6 @@ def test_tr_multisig_2_of_2_feerate_is_met(feerate, lianad_multisig_2_of_2, bitc
spend_weight = res["weight"]
assert spend_weight == 646

# Due to https://github.com/wizardsardine/liana/pull/1323 we currently
# add 10 sats if feerate is 1.
extra = 10 if feerate == 1 else 0

# Note that due to https://github.com/wizardsardine/liana/issues/1132
# we do not round up vbytes before multiplying by feerate.
assert spend_fee == math.ceil((646.0 / 4.0) * feerate) + extra
assert spend_fee == math.ceil((646.0 / 4.0) * feerate)

0 comments on commit 6e74a96

Please sign in to comment.