diff --git a/src/commands/mod.rs b/src/commands/mod.rs index f2102014..605d4196 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1488,9 +1488,9 @@ mod tests { // rust-bitcoin's serialization of transactions with no input silently affected our fee // calculation. - // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 170 sats fees. + // 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_830); + 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() @@ -1500,7 +1500,7 @@ mod tests { panic!("expect successful spend creation") }; let tx = psbt.unsigned_tx; - assert_eq!(tx.output[1].value.to_sat(), 89_660); + assert_eq!(tx.output[1].value.to_sat(), 89_678); // A feerate of 555 won't trigger the sanity checks (they were previously not taking the // satisfaction size into account and overestimating the feerate). @@ -1563,13 +1563,13 @@ mod tests { warnings, vec![ "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \ - Instead of creating a change of 4830 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_830; + *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() @@ -1599,7 +1599,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_830 + /* 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 }), @@ -1607,7 +1607,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 */ 127 - /* 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() @@ -1628,7 +1628,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 */ 127 - /* 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() @@ -1644,7 +1644,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 */ 127 - /* 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() @@ -1697,12 +1697,8 @@ mod tests { spend_txid: None, spend_block: None, }]); - // Even though 1_000 is the max feerate allowed by our sanity check, we need to - // use 1_003 in order to exceed it and fail this test since coin selection is - // based on a minimum feerate of `feerate_vb / 4.0` sats/wu, which can result in - // the sats/vb feerate being lower than `feerate_vb`. assert_eq!( - control.create_spend(&destinations, &[dummy_op_dup], 1_003, None), + control.create_spend(&destinations, &[dummy_op_dup], 1_001, None), Err(CommandError::SpendCreation(SpendCreationError::InsaneFees( InsaneFeeInfo::TooHighFeerate(1_001) ))) diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index cb611fb5..3cefe9f5 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -6,12 +6,13 @@ use miniscript::{ secp256k1, }, descriptor, + plan::{Assets, CanSign}, psbt::{PsbtInputExt, PsbtOutputExt}, translate_hash_clone, ForEachKey, TranslatePk, Translator, }; use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet, HashMap, HashSet}, convert::TryInto, error, fmt, str::{self, FromStr}, @@ -56,6 +57,10 @@ impl From for LianaDescError { } } +fn varint_len(n: usize) -> usize { + bitcoin::VarInt(n as u64).size() +} + // Whether the key identified by its fingerprint+derivation path was derived from one of the xpubs // for this spending path. fn key_is_for_path( @@ -228,23 +233,64 @@ impl LianaDescriptor { .0 } - // TODO: on Taproot we should use this for recovery but keyspend size if there is a spendable - // internal key. /// Get the maximum size difference of a transaction input spending a Script derived from this /// descriptor before and after satisfaction. The returned value is in weight units. /// Callers are expected to account for the Segwit marker (2 WU). This takes into account the /// size of the witness stack length varint. - pub fn max_sat_weight(&self) -> usize { - // We add one to account for the witness stack size, as the `max_weight_to_satisfy` method - // computes the difference in size for a satisfied input that was *already* in a - // transaction that spent one or more Segwit coins (and thus already have 1 WU accounted - // for the emtpy witness). But this method is used to account between a completely "nude" - // transaction (and therefore no Segwit marker nor empty witness in inputs) and a satisfied - // transaction. - self.multi_desc - .max_weight_to_satisfy() - .expect("Always satisfiable") - + 1 + pub fn max_sat_weight(&self, use_primary_path: bool) -> usize { + if use_primary_path { + // Get the keys from the primary path, to get a satisfaction size estimation only + // considering those. + let keys = self + .policy() + .primary_path + .thresh_origins() + .1 + .into_iter() + .fold(BTreeSet::new(), |mut keys, (fg, der_paths)| { + for der_path in der_paths { + keys.insert(((fg, der_path), CanSign::default())); + } + keys + }); + let assets = Assets { + keys, + ..Default::default() + }; + + // Unfortunately rust-miniscript satisfaction size estimation is inconsistent. For + // Taproot it considers the whole witness (including the control block size + the + // script size) but under P2WSH it does not consider the witscript! Therefore we + // manually add the size of the witscript, but only under P2WSH by the mean of the + // `explicit_script()` helper. + let der_desc = self + .receive_desc + .0 + .at_derivation_index(0) + .expect("unhardened index"); + let witscript_size = der_desc + .explicit_script() + .map(|s| varint_len(s.len()) + s.len()) + .unwrap_or(0); + + // Finally, compute the satisfaction template for the primary path and get its size. + der_desc + .plan(&assets) + .expect("Always satisfiable") + .witness_size() + + witscript_size + } else { + // We add one to account for the witness stack size, as the values above give the + // difference in size for a satisfied input that was *already* in a transaction + // that spent one or more Segwit coins (and thus already have 1 WU accounted for the + // emtpy witness). But this method is used to account between a completely "nude" + // transaction (and therefore no Segwit marker nor empty witness in inputs) and a + // satisfied transaction. + self.multi_desc + .max_weight_to_satisfy() + .expect("Always satisfiable") + + 1 + } } /// Get the maximum size difference of a transaction input spending a Script derived from this @@ -252,8 +298,8 @@ impl LianaDescriptor { /// bytes. /// Callers are expected to account for the Segwit marker (2 WU). This takes into account the /// size of the witness stack length varint. - pub fn max_sat_vbytes(&self) -> usize { - self.max_sat_weight() + pub fn max_sat_vbytes(&self, use_primary_path: bool) -> usize { + self.max_sat_weight(use_primary_path) .checked_add(WITNESS_SCALE_FACTOR - 1) .unwrap() .checked_div(WITNESS_SCALE_FACTOR) @@ -262,9 +308,9 @@ impl LianaDescriptor { /// Get the maximum size in virtual bytes of the whole input in a transaction spending /// a coin with this Script. - pub fn spender_input_size(&self) -> usize { + pub fn spender_input_size(&self, use_primary_path: bool) -> usize { // txid + vout + nSequence + empty scriptSig + witness - 32 + 4 + 4 + 1 + self.max_sat_vbytes() + 32 + 4 + 4 + 1 + self.max_sat_vbytes(use_primary_path) } /// Whether this is a Taproot descriptor. @@ -492,10 +538,10 @@ impl LianaDescriptor { /// Maximum possible size in vbytes of an unsigned transaction, `tx`, /// after satisfaction, assuming all inputs of `tx` are from this /// descriptor. - pub fn unsigned_tx_max_vbytes(&self, tx: &bitcoin::Transaction) -> u64 { + pub fn unsigned_tx_max_vbytes(&self, tx: &bitcoin::Transaction, use_primary_path: bool) -> u64 { let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap(); let num_inputs: u64 = tx.input.len().try_into().unwrap(); - let max_sat_weight: u64 = self.max_sat_weight().try_into().unwrap(); + let max_sat_weight: u64 = self.max_sat_weight(use_primary_path).try_into().unwrap(); // Add weights together before converting to vbytes to avoid rounding up multiple times. let tx_wu = tx .weight() @@ -1055,17 +1101,31 @@ mod tests { #[test] fn inheritance_descriptor_sat_size() { let desc = LianaDescriptor::from_str("wsh(or_d(pk([92162c45]tpubD6NzVbkrYhZ4WzTf9SsD6h7AH7oQEippXK2KP8qvhMMqFoNeN5YFVi7vRyeRSDGtgd2bPyMxUNmHui8t5yCgszxPPxMafu1VVzDpg9aruYW/<0;1>/*),and_v(v:pkh([abcdef01]tpubD6NzVbkrYhZ4Wdgu2yfdmrce5g4fiH1ZLmKhewsnNKupbi4sxjH1ZVAorkBLWSkhsjhg8kiq8C4BrBjMy3SjAKDyDdbuvUa1ToAHbiR98js/<0;1>/*),older(2))))#ravw7jw5").unwrap(); - assert_eq!(desc.max_sat_vbytes(), (1 + 66 + 1 + 34 + 73 + 3) / 4); // See the stack details below. + // See the stack details below. + assert_eq!(desc.max_sat_vbytes(true), (1 + 66 + 73 + 3) / 4); + assert_eq!(desc.max_sat_vbytes(false), (1 + 66 + 1 + 34 + 73 + 3) / 4); // Maximum input size is (txid + vout + scriptsig + nSequence + max_sat). // Where max_sat is: // - Push the witness stack size // - Push the script + // If recovery: // - Push an empty vector for using the recovery path // - Push the recovery key - // - Push a signature for the recovery key + // EndIf + // - Push a signature for the primary/recovery key // NOTE: The specific value is asserted because this was tested against a regtest // transaction. + let stack = vec![vec![0; 65], vec![0; 72]]; + let witness_size = bitcoin::VarInt(stack.len() as u64).size() + + stack + .iter() + .map(|item| bitcoin::VarInt(item.len() as u64).size() + item.len()) + .sum::(); + assert_eq!( + desc.spender_input_size(true), + 32 + 4 + 1 + 4 + wu_to_vb(witness_size), + ); let stack = vec![vec![0; 65], vec![0; 0], vec![0; 33], vec![0; 72]]; let witness_size = bitcoin::VarInt(stack.len() as u64).size() + stack @@ -1073,9 +1133,51 @@ mod tests { .map(|item| bitcoin::VarInt(item.len() as u64).size() + item.len()) .sum::(); assert_eq!( - desc.spender_input_size(), + desc.spender_input_size(false), 32 + 4 + 1 + 4 + wu_to_vb(witness_size), ); + + // Now perform the sanity checks under Taproot. + let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); + let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*").unwrap()); + let timelock = 52560; + let desc = LianaDescriptor::new( + LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key.clone())].iter().cloned().collect(), + ) + .unwrap(), + ); + + // If using the primary path, it's a keypath spend. + assert_eq!(desc.max_sat_vbytes(true), (1 + 65 + 3) / 4); + // If using the recovery path, it's a script path spend. The script is 40 bytes long. The + // control block is just the internal key and parity, so 33 bytes long. + assert_eq!( + desc.max_sat_vbytes(false), + (1 + 65 + 1 + 40 + 1 + 33 + 3) / 4 + ); + + // The same against the spender_input_size() helper, adding the size of the txin and + // checking against a dummy witness stack. + fn wit_size(stack: &[Vec]) -> usize { + varint_len(stack.len()) + + stack + .iter() + .map(|item| varint_len(item.len()) + item.len()) + .sum::() + } + let txin_boilerplate = 32 + 4 + 1 + 4; + let stack = vec![vec![0; 64]]; + assert_eq!( + desc.spender_input_size(true), + txin_boilerplate + wu_to_vb(wit_size(&stack)), + ); + let stack = vec![vec![0; 33], vec![0; 40], vec![0; 64]]; + assert_eq!( + desc.spender_input_size(false), + txin_boilerplate + wu_to_vb(wit_size(&stack)), + ); } #[test] diff --git a/src/spend.rs b/src/spend.rs index dc403e88..ef858801 100644 --- a/src/spend.rs +++ b/src/spend.rs @@ -99,6 +99,7 @@ fn check_output_value(value: bitcoin::Amount) -> Result<(), SpendCreationError> fn sanity_check_psbt( spent_desc: &descriptors::LianaDescriptor, psbt: &Psbt, + use_primary_path: bool, ) -> Result<(), SpendCreationError> { let tx = &psbt.unsigned_tx; @@ -137,7 +138,7 @@ fn sanity_check_psbt( } // Check the feerate isn't insane. - let tx_vb = spent_desc.unsigned_tx_max_vbytes(tx); + let tx_vb = spent_desc.unsigned_tx_max_vbytes(tx, use_primary_path); let feerate_sats_vb = abs_fee .checked_div(tx_vb) .ok_or(SpendCreationError::InsaneFees( @@ -663,6 +664,13 @@ pub fn create_spend( value: bitcoin::Amount::MAX, script_pubkey: change_addr.addr.script_pubkey(), }; + // If no candidates have relative locktime, then we should use the primary spending path. + // Note we set this value before actually selecting the coins, but we expect either all + // candidates or none to have relative locktime sequence so this is fine. + let use_primary_path = !candidate_coins + .iter() + .filter_map(|cand| cand.sequence) + .any(|seq| seq.is_relative_lock_time()); // Now select the coins necessary using the provided candidates and determine whether // there is any leftover to create a change output. let CoinSelectionRes { @@ -684,7 +692,7 @@ pub fn create_spend( } .into(); let max_sat_wu = main_descriptor - .max_sat_weight() + .max_sat_weight(use_primary_path) .try_into() .expect("Weight must fit in a u32"); select_coins_for_spend( @@ -771,7 +779,7 @@ pub fn create_spend( inputs: psbt_ins, outputs: psbt_outs, }; - sanity_check_psbt(main_descriptor, &psbt)?; + sanity_check_psbt(main_descriptor, &psbt, use_primary_path)?; // TODO: maybe check for common standardness rules (max size, ..)? Ok(CreateSpendRes { diff --git a/tests/test_chain.py b/tests/test_chain.py index e4d6a99f..0dcf2b49 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -446,10 +446,10 @@ def test_spend_replacement(lianad, bitcoind): destinations = { bitcoind.rpc.getnewaddress(): 650_000, } - second_res = lianad.rpc.createspend(destinations, second_outpoints, 2) + second_res = lianad.rpc.createspend(destinations, second_outpoints, 3) second_psbt = PSBT.from_base64(second_res["psbt"]) destinations = {} - third_res = lianad.rpc.createspend(destinations, second_outpoints, 4) + third_res = lianad.rpc.createspend(destinations, second_outpoints, 5) third_psbt = PSBT.from_base64(third_res["psbt"]) # Broadcast the first transaction. Make sure it's detected. diff --git a/tests/test_rpc.py b/tests/test_rpc.py index b1ae8815..16ad2744 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -1118,13 +1118,19 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind): for c in lianad.rpc.listcoins([], first_outpoints)["coins"] ) ) + mempool_rbf_1 = bitcoind.rpc.getmempoolentry(rbf_1_txid) + # Note that in the mempool entry, "ancestor" includes rbf_1_txid itself. + rbf_1_feerate = ( + mempool_rbf_1["fees"]["ancestor"] * COIN / mempool_rbf_1["ancestorsize"] + ) + assert 9.75 < rbf_1_feerate < 10.25 # If we try to RBF the first transaction again, it will use the first RBF's - # feerate of 10 sat/vb to set the min feerate, instead of 1 sat/vb of first + # feerate to set the min feerate, instead of 1 sat/vb of first # transaction: - with pytest.raises(RpcError, match=f"Feerate too low: 10."): - lianad.rpc.rbfpsbt(first_txid, False, 10) - # Using 11 for feerate works for P2WSH. For Taproot we need 12. - feerate = 12 if USE_TAPROOT else 11 + with pytest.raises(RpcError, match=f"Feerate too low: {int(rbf_1_feerate)}."): + lianad.rpc.rbfpsbt(first_txid, False, int(rbf_1_feerate)) + # Using 1 more for feerate works. + feerate = int(rbf_1_feerate) + 1 lianad.rpc.rbfpsbt(first_txid, False, feerate) # Add a new transaction spending the change from the first RBF. desc_1_destinations = { diff --git a/tests/test_spend.py b/tests/test_spend.py index dcfdc296..6125dbf3 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -134,7 +134,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 = 840 if USE_TAPROOT else 830 + change_amount = 858 if USE_TAPROOT else 839 assert len(psbt.o) == 1 assert len(res["warnings"]) == 1 assert ( @@ -153,7 +153,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 = 828 if USE_TAPROOT else 818 + change_amount = 846 if USE_TAPROOT else 827 assert len(psbt.o) == 1 assert len(res["warnings"]) == 1 assert ( @@ -252,17 +252,17 @@ def test_send_to_self(lianad, bitcoind): lianad.rpc.broadcastspend(spend_txid) # The only output is the change output so the feerate of the transaction must - # not be lower than the one provided, and only possibly slightly higher (since - # we slightly overestimate the satisfaction size). - # FIXME: a 15% increase is huge. + # not be much lower than the one provided (it could be slightly lower since + # the change amount is determined using feerate in terms of sat/wu which, due + # to rounding, can lead to a slightly lower feerate in terms of sat/vb, + # especially when the number of inputs increases), and only possibly slightly + # higher (since we slightly overestimate the satisfaction size). res = bitcoind.rpc.getmempoolentry(spend_txid) - spend_feerate = int(res["fees"]["base"] * COIN / res["vsize"]) - if not USE_TAPROOT: - assert specified_feerate <= spend_feerate <= int(specified_feerate * 115 / 100) + spend_feerate = res["fees"]["base"] * COIN / res["vsize"] # keep as decimal + if USE_TAPROOT: + assert specified_feerate <= spend_feerate < specified_feerate + 0.5 else: - # FIXME: under Taproot we should not consider the max feerate of all leaves if there - # is a spendable internal key. - assert specified_feerate <= spend_feerate <= int(specified_feerate * 125 / 100) + assert specified_feerate - 0.5 < spend_feerate < specified_feerate + 0.5 # We should by now only have one coin. bitcoind.generate_block(1, wait_for_mempool=spend_txid) @@ -275,7 +275,9 @@ def test_send_to_self(lianad, bitcoind): assert len(lianad.rpc.listaddresses()["addresses"]) == 3 # Create a new spend to the receive address with index 3. recv_addr = lianad.rpc.listaddresses(3, 1)["addresses"][0]["receive"] - res = lianad.rpc.createspend({recv_addr: 11_955_000}, [], 2) + res = lianad.rpc.createspend( + {recv_addr: 11_965_000 if USE_TAPROOT else 11_955_000}, [], 2 + ) assert "psbt" in res # Max(receive_index, change_index) is now 4: assert len(lianad.rpc.listaddresses()["addresses"]) == 4 @@ -323,6 +325,14 @@ def test_coin_selection(lianad, bitcoind): # Sign and broadcast this Spend transaction. spend_txid_1 = sign_and_broadcast_psbt(lianad, spend_psbt_1) + # Check its feerate is approx 2 sat/vb + anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] + anc_fees = int( + bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN + ) + # txid_1's feerate is approx 2 sat/vb as required. + txid_1_feerate = anc_fees / anc_vsize + assert 1.99 < txid_1_feerate < 2.01 wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2) # Check that change output is unconfirmed. assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1 @@ -342,17 +352,14 @@ def test_coin_selection(lianad, bitcoind): assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str( bytes.fromhex(spend_txid_1)[::-1] ) - anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] - anc_fees = int( - bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN - ) - additional_fee = additional_fees(anc_vsize, anc_fees, feerate) + additional_fee_at_10satvb = additional_fees(anc_vsize, anc_fees, feerate) assert len(spend_res_2["warnings"]) == 1 assert ( spend_res_2["warnings"][0] == "CPFP: an unconfirmed input was selected. The current transaction fee " - f"was increased by {additional_fee} sats to make the average feerate of " - "both the input and current transaction equal to the selected feerate." + f"was increased by {additional_fee_at_10satvb} sats to make the average " + "feerate of both the input and current transaction equal to the selected " + "feerate." ) # Try 3 sat/vb: @@ -364,24 +371,39 @@ def test_coin_selection(lianad, bitcoind): assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str( bytes.fromhex(spend_txid_1)[::-1] ) - anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] - anc_fees = int( - bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN - ) - additional_fee = additional_fees(anc_vsize, anc_fees, feerate) + additional_fee_at_3satvb = additional_fees(anc_vsize, anc_fees, feerate) + assert additional_fee_at_10satvb > additional_fee_at_3satvb assert len(spend_res_2["warnings"]) == 1 assert ( spend_res_2["warnings"][0] == "CPFP: an unconfirmed input was selected. The current transaction fee " - f"was increased by {additional_fee} sats to make the average feerate of " - "both the input and current transaction equal to the selected feerate." + f"was increased by {additional_fee_at_3satvb} sats to make the average " + "feerate of both the input and current transaction equal to the selected " + "feerate." ) - # 2 sat/vb is same feerate as ancestor and we have no warnings: - spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 2) + # 2 sat/vb is approx same feerate as ancestor: + feerate = 2 + spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], feerate) assert "psbt" in spend_res_2 - assert len(spend_res_2["warnings"]) == 0 spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) + spend_txid_2 = spend_psbt_2.tx.txid().hex() + if USE_TAPROOT: + assert len(spend_res_2["warnings"]) == 0 + else: + # We still get a warning in the non-taproot case. + assert len(spend_res_2["warnings"]) == 1 + additional_fee_at_2satvb = additional_fees(anc_vsize, anc_fees, feerate) + assert additional_fee_at_3satvb > additional_fee_at_2satvb + assert len(spend_res_2["warnings"]) == 1 + assert ( + spend_res_2["warnings"][0] + == "CPFP: an unconfirmed input was selected. The current transaction fee " + f"was increased by {additional_fee_at_2satvb} sats to make the average " + "feerate of both the input and current transaction equal to the selected " + "feerate." + ) + # The spend is using the unconfirmed change. assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str( bytes.fromhex(spend_txid_1)[::-1] @@ -435,14 +457,18 @@ def test_coin_selection(lianad, bitcoind): # Note that in the mempool entry, "ancestor" includes spend_txid_3 itself. assert ( mempool_txid_3["fees"]["ancestor"] * COIN // mempool_txid_3["ancestorsize"] - == 10 + == feerate ) # The spend_txid_3 transaction itself has a higher feerate. assert (mempool_txid_3["fees"]["base"] * COIN) // mempool_txid_3["vsize"] > 10 - # If we subtract the extra that pays for the ancestor, the feerate is at the target value. - assert ((mempool_txid_3["fees"]["base"] * COIN) - 2770) // mempool_txid_3[ - "vsize" - ] == 10 + # If we subtract the extra that pays for the ancestor, the feerate is approximately + # at the target value. + assert ( + feerate - 0.5 + < ((mempool_txid_3["fees"]["base"] * COIN) - additional_fee) + / mempool_txid_3["vsize"] + < feerate + 0.5 + ) # Now confirm the spend. bitcoind.generate_block(1, wait_for_mempool=spend_txid_3)