From c0d432057df6b2f0bc879f718489c761867fa88c Mon Sep 17 00:00:00 2001 From: jp1ac4 <121959000+jp1ac4@users.noreply.github.com> Date: Thu, 18 Jan 2024 13:42:52 +0000 Subject: [PATCH] spend: add warning about fee for ancestor --- src/spend.rs | 15 ++++- tests/test_spend.py | 136 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 131 insertions(+), 20 deletions(-) diff --git a/src/spend.rs b/src/spend.rs index 001085e6..d545375e 100644 --- a/src/spend.rs +++ b/src/spend.rs @@ -543,6 +543,7 @@ pub enum SpendTxFees { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum CreateSpendWarning { ChangeAddedToFee(u64), + AddtionalFeeForAncestors(u64), } impl fmt::Display for CreateSpendWarning { @@ -554,6 +555,12 @@ impl fmt::Display for CreateSpendWarning { amt, if *amt > 1 {"s"} else {""}, ), + CreateSpendWarning::AddtionalFeeForAncestors(amt) => write!( + f, + "An additional fee of {} sat{} has been added to pay for ancestors at the target feerate.", + amt, + if *amt > 1 {"s"} else {""}, + ), } } } @@ -673,7 +680,7 @@ pub fn create_spend( selected, change_amount, max_change_amount, - .. + fee_for_ancestors, } = { // At this point the transaction still has no input and no change output, as expected // by the coins selection helper function. @@ -735,6 +742,12 @@ pub fn create_spend( )); } + if fee_for_ancestors.to_sat() > 0 { + warnings.push(CreateSpendWarning::AddtionalFeeForAncestors( + fee_for_ancestors.to_sat(), + )); + } + // Iterate through selected coins and add necessary information to the PSBT inputs. let mut psbt_ins = Vec::with_capacity(selected.len()); for cand in &selected { diff --git a/tests/test_spend.py b/tests/test_spend.py index 5a6fe3a0..3e7224a2 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -171,7 +171,16 @@ def test_coin_marked_spent(lianad, bitcoind): psbt = PSBT.from_base64(res["psbt"]) sign_and_broadcast(psbt) assert len(psbt.o) == 4 - assert len(res["warnings"]) == 0 + assert bitcoind.rpc.getmempoolentry(deposit_d)["ancestorsize"] == 165 + assert bitcoind.rpc.getmempoolentry(deposit_d)["fees"]["ancestor"] * COIN == 165 + # ancestor vsize at feerate 2 sat/vb = ancestor_fee / 2 = 165 / 2 = 82 + # extra_weight <= (extra vsize * witness factor) = (165 - 82) * 4 = 332 + # additional fee at 2 sat/vb (0.5 sat/wu) = 332 * 0.5 = 166 + assert len(res["warnings"]) == 1 + assert ( + res["warnings"][0] + == "An additional fee of 166 sats has been added to pay for ancestors at the target feerate." + ) # All the spent coins must have been detected as such all_deposits = (deposit_a, deposit_b, deposit_c, deposit_d) @@ -263,6 +272,7 @@ def test_coin_selection(lianad, bitcoind): # Coin selection now succeeds. spend_res_1 = lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2) assert "psbt" in spend_res_1 + assert len(spend_res_1["warnings"]) == 0 # Increase spend amount and we have insufficient funds again even though we # now have confirmed coins. assert "missing" in lianad.rpc.createspend({dest_addr_1: 200_000}, [], 2) @@ -280,17 +290,61 @@ def test_coin_selection(lianad, bitcoind): assert lianad.rpc.listcoins(["unconfirmed"])["coins"][0]["is_change"] is True assert len(lianad.rpc.listcoins(["spending"])["coins"]) == 1 # We can use unconfirmed change as candidate. + # Depending on the feerate, we'll get a warning about paying extra for the ancestor. dest_addr_2 = bitcoind.rpc.getnewaddress() - spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 2) + # If feerate is higher than ancestor, we'll need to pay extra. + + # Try 10 sat/vb: + spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 10) assert "psbt" in spend_res_2 spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) # 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] ) + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] == 161 + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN == 339 + # ancestor vsize at feerate 10 sat/vb = ancestor_fee / 10 = 339 / 10 = 33 + # extra_weight <= (extra vsize * witness factor) = (161 - 33) * 4 = 512 + # additional fee at 10 sat/vb (2.5 sat/wu) = 512 * 2.5 = 1280 + assert len(spend_res_2["warnings"]) == 1 + assert ( + spend_res_2["warnings"][0] + == "An additional fee of 1280 sats has been added to pay for ancestors at the target feerate." + ) + + # Try 3 sat/vb: + spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 3) + assert "psbt" in spend_res_2 + spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) + # 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] + ) + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] == 161 + assert bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN == 339 + # ancestor vsize at feerate 3 sat/vb = ancestor_fee / 3 = 339 / 3 = 113 + # extra_weight <= (extra vsize * witness factor) = (161 - 113) * 4 = 192 + # additional fee at 3 sat/vb (0.75 sat/wu) = 192 * 0.75 = 144 + assert len(spend_res_2["warnings"]) == 1 + assert ( + spend_res_2["warnings"][0] + == "An additional fee of 144 sats has been added to pay for ancestors at the target 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) + assert "psbt" in spend_res_2 + assert len(spend_res_2["warnings"]) == 0 + spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"]) + # 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] + ) + # Get another coin to check coin selection with more than one candidate. recv_addr_2 = lianad.rpc.getnewaddress()["address"] - deposit_2 = bitcoind.rpc.sendtoaddress(recv_addr_2, 0.0002) # 20_000 sats + deposit_2 = bitcoind.rpc.sendtoaddress(recv_addr_2, 30_000 / COIN) wait_for(lambda: len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 2) assert ( len( @@ -302,33 +356,77 @@ def test_coin_selection(lianad, bitcoind): ) == 1 ) - # As only one unconfirmed coin is change, we have insufficient funds. dest_addr_3 = bitcoind.rpc.getnewaddress() - assert "missing" in lianad.rpc.createspend({dest_addr_3: 30_000}, [], 2) - # Now confirm both coins. - bitcoind.generate_block(1, wait_for_mempool=deposit_2) - wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 2) - spend_res_3 = lianad.rpc.createspend({dest_addr_3: 30_000}, [], 2) + # As only one unconfirmed coin is change, we have insufficient funds. + assert "missing" in lianad.rpc.createspend({dest_addr_3: 20_000}, [], 10) + + # If we include both unconfirmed coins manually, it will succeed. + # We'll need to pay extra for each unconfirmed coin's ancestors. + outpoints = [c["outpoint"] for c in lianad.rpc.listcoins(["unconfirmed"])["coins"]] + + spend_res_3 = lianad.rpc.createspend({dest_addr_3: 20_000}, outpoints, 10) assert "psbt" in spend_res_3 + assert bitcoind.rpc.getmempoolentry(deposit_2)["ancestorsize"] == 165 + assert bitcoind.rpc.getmempoolentry(deposit_2)["fees"]["ancestor"] * COIN == 165 + # From above, extra fee for unconfirmed change at 10 sat/vb = 1280. + # For unconfirmed non-change: + # ancestor vsize at feerate 10 sat/vb = ancestor_fee / 10 = 165 / 10 = 16 + # extra_weight <= (extra vsize * witness factor) = (165 - 16) * 4 = 596 + # additional fee at 10 sat/vb (2.5 sat/wu) = 596 * 2.5 = 1490 + # Sum of extra ancestor fees = 1280 + 1490 = 2770. + assert len(spend_res_3["warnings"]) == 1 + assert ( + spend_res_3["warnings"][0] + == "An additional fee of 2770 sats has been added to pay for ancestors at the target feerate." + ) + spend_psbt_3 = PSBT.from_base64(spend_res_3["psbt"]) + spend_txid_3 = sign_and_broadcast_psbt(lianad, spend_psbt_3) + mempool_txid_3 = bitcoind.rpc.getmempoolentry(spend_txid_3) + # The effective feerate of new transaction plus ancestor matches the target. + # Note that in the mempool entry, "ancestor" includes spend_txid_3 itself. + assert ( + mempool_txid_3["fees"]["ancestor"] * COIN // mempool_txid_3["ancestorsize"] + == 10 + ) + # 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 + + # Now confirm the spend. + bitcoind.generate_block(1, wait_for_mempool=spend_txid_3) + wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1) + + # Now create the same spend with auto and manual selection: + dest_addr_4 = bitcoind.rpc.getnewaddress() + spend_res_4 = lianad.rpc.createspend({dest_addr_4: 15_000}, [], 2) + assert "psbt" in spend_res_4 + assert len(spend_res_4["warnings"]) == 0 # The transaction contains a change output. - spend_psbt_3 = PSBT.from_base64(spend_res_3["psbt"]) - assert len(spend_psbt_3.i) == 2 - assert len(spend_psbt_3.o) == 2 - assert len(spend_psbt_3.tx.vout) == 2 + spend_psbt_4 = PSBT.from_base64(spend_res_4["psbt"]) + assert len(spend_psbt_4.i) == 1 + assert len(spend_psbt_4.o) == 2 + assert len(spend_psbt_4.tx.vout) == 2 # Now create a transaction with manual coin selection using the same outpoints. outpoints = [ - f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in spend_psbt_3.tx.vin + f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in spend_psbt_4.tx.vin ] - res_manual = lianad.rpc.createspend({dest_addr_3: 30_000}, outpoints, 2) + assert len(outpoints) > 0 + res_manual = lianad.rpc.createspend({dest_addr_4: 15_000}, outpoints, 2) + assert len(res_manual["warnings"]) == 0 psbt_manual = PSBT.from_base64(res_manual["psbt"]) # Recipient details are the same for both. - assert spend_psbt_3.tx.vout[0].nValue == psbt_manual.tx.vout[0].nValue - assert spend_psbt_3.tx.vout[0].scriptPubKey == psbt_manual.tx.vout[0].scriptPubKey - # Change amount is the same (change address will be different). - assert spend_psbt_3.tx.vout[1].nValue == psbt_manual.tx.vout[1].nValue + assert spend_psbt_4.tx.vout[0].nValue == psbt_manual.tx.vout[0].nValue + assert spend_psbt_4.tx.vout[0].scriptPubKey == psbt_manual.tx.vout[0].scriptPubKey + # Change details are also the same + # (change address is same as neither transaction has been broadcast) + assert spend_psbt_4.tx.vout[1].nValue == psbt_manual.tx.vout[1].nValue + assert spend_psbt_4.tx.vout[1].scriptPubKey == psbt_manual.tx.vout[1].scriptPubKey def test_coin_selection_changeless(lianad, bitcoind):