From 6e74a96e12feb00fcf0f6974f56002c0356e0960 Mon Sep 17 00:00:00 2001 From: Michael Mallan Date: Fri, 8 Nov 2024 15:09:47 +0000 Subject: [PATCH] Revert "spend: add 10 sats to fee for 1 sat/vb txs" This reverts commit 74a53baa39c409d8a39a27affc5ebac34128cb24. I also removed the extra sats being added in the functional test. --- src/commands/mod.rs | 18 +++++++++--------- src/spend.rs | 15 +-------------- tests/test_spend.py | 10 +++------- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 3eca39ac..ae17e0ea 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -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() @@ -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() @@ -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() @@ -1606,7 +1606,7 @@ 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 }), @@ -1614,7 +1614,7 @@ mod tests { // 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() @@ -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() @@ -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() diff --git a/src/spend.rs b/src/spend.rs index 2c7aad3b..9465b775 100644 --- a/src/spend.rs +++ b/src/spend.rs @@ -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}, @@ -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, diff --git a/tests/test_spend.py b/tests/test_spend.py index 3f11874e..19081dd8 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -136,7 +136,7 @@ def test_coin_marked_spent(lianad, bitcoind): 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 ( @@ -155,7 +155,7 @@ def test_coin_marked_spent(lianad, bitcoind): 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 ( @@ -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)