Merge bitcoin/bitcoin#26616: [24.x] Backports for 24.0.1

8b726bf556e05edf02946d4b1c3356df17fd0d57 test: Coin Selection, duplicated preset inputs selection (furszy)
9d73176d00a013e1383ae18cb5c0f8cbdd186cba test: wallet, coverage for CoinsResult::Erase function (furszy)
195f0dfd0ec7fadfbbb3d86decb3f6d96beae159 wallet: bugfix, 'CoinsResult::Erase' is erasing only one output of the set (furszy)
e5d097b639c7f75b530349b524836804cb753597 [test] Add p2p_tx_privacy.py (dergoegge)
c8426706deda827231715a1e9afd2078026a5e49 [net processing] Assume that TxRelay::m_tx_inventory_to_send is empty pre-verack (dergoegge)
e15b3060179f94962eff82f3ed87a1d26ef65c88 [net processing] Ensure transaction announcements are only queued for fully connected peers (dergoegge)
95fded106979a523431863679107810db81ca4b3 wallet: Explicitly say migratewallet on encrypted wallets is unsupported (Andrew Chow)
d464b2af30f2b02be2ce0b5e45dc6c141529dba5 tests: Test for migrating encrypted wallets (Andrew Chow)
7a97a56ffb22fbf8ccb143a8a7da77e8c7e77069 wallet: Avoid null pointer deref when cleaning up migratewallet (Andrew Chow)

Pull request description:

  Backports remaining changes on the 24.0.1 milestone.

  Currently backports:
  * https://github.com/bitcoin/bitcoin/pull/26594
  * https://github.com/bitcoin/bitcoin/pull/26569
  * https://github.com/bitcoin/bitcoin/pull/26560

ACKs for top commit:
  josibake:
    ACK https://github.com/bitcoin/bitcoin/pull/26616/commits/8b726bf556e05edf02946d4b1c3356df17fd0d57

Tree-SHA512: db77ec1a63a7b6a4412750a0f4c0645681fc346a5df0a7cd38d5d27384e1d0fa95f3953af90042afe131ddbd4b6a6e009527095f13e9f58c0190cd378738a9e5

4 files changed, 134 insertions, 0 deletions

diff --git a/test/functional/p2p_tx_privacy.py b/test/functional/p2p_tx_privacy.py
new file mode 100755
index 0000000000..b885ccdf5d
--- /dev/null
+++ b/test/functional/p2p_tx_privacy.py
@@ -0,0 +1,78 @@
+#!/usr/bin/env python3
+# Copyright (c) 2022 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test that transaction announcements are only queued for peers that have
+successfully completed the version handshake.
+
+Topology:
+
+  tx_originator ----> node[0] <---- spy
+
+We test that a transaction sent by tx_originator is only relayed to spy
+if it was received after spy's version handshake completed.
+
+1. Fully connect tx_originator
+2. Connect spy (no version handshake)
+3. tx_originator sends tx1
+4. spy completes the version handshake
+5. tx_originator sends tx2
+6. We check that only tx2 is announced on the spy interface
+"""
+from test_framework.messages import (
+    msg_wtxidrelay,
+    msg_verack,
+    msg_tx,
+    CInv,
+    MSG_WTX,
+)
+from test_framework.p2p import (
+    P2PInterface,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.wallet import MiniWallet
+
+class P2PTxSpy(P2PInterface):
+    def __init__(self):
+        super().__init__()
+        self.all_invs = []
+
+    def on_version(self, message):
+        self.send_message(msg_wtxidrelay())
+
+    def on_inv(self, message):
+        self.all_invs += message.inv
+
+    def wait_for_inv_match(self, expected_inv):
+        self.wait_until(lambda: len(self.all_invs) == 1 and self.all_invs[0] == expected_inv)
+
+class TxPrivacyTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+
+    def run_test(self):
+        self.wallet = MiniWallet(self.nodes[0])
+        self.wallet.rescan_utxos()
+
+        tx_originator = self.nodes[0].add_p2p_connection(P2PInterface())
+        spy = self.nodes[0].add_p2p_connection(P2PTxSpy(), wait_for_verack=False)
+        spy.wait_for_verack()
+
+        # tx_originator sends tx1
+        tx1 = self.wallet.create_self_transfer()["tx"]
+        tx_originator.send_and_ping(msg_tx(tx1))
+
+        # Spy sends the verack
+        spy.send_and_ping(msg_verack())
+
+        # tx_originator sends tx2
+        tx2 = self.wallet.create_self_transfer()["tx"]
+        tx_originator.send_and_ping(msg_tx(tx2))
+
+        # Spy should only get an inv for the second transaction as the first
+        # one was received pre-verack with the spy
+        spy.wait_for_inv_match(CInv(MSG_WTX, tx2.calc_sha256(True)))
+
+if __name__ == '__main__':
+    TxPrivacyTest().main()
diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py
index a963bb5e2d..9a3a356097 100755
--- a/test/functional/rpc_fundrawtransaction.py
+++ b/test/functional/rpc_fundrawtransaction.py
@@ -107,6 +107,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.generate(self.nodes[0], 121)
 
         self.test_add_inputs_default_value()
+        self.test_preset_inputs_selection()
         self.test_weight_calculation()
         self.test_change_position()
         self.test_simple()
@@ -1189,6 +1190,50 @@ class RawTransactionsTest(BitcoinTestFramework):
 
         self.nodes[2].unloadwallet("test_preset_inputs")
 
+    def test_preset_inputs_selection(self):
+        self.log.info('Test wallet preset inputs are not double-counted or reused in coin selection')
+
+        # Create and fund the wallet with 4 UTXO of 5 BTC each (20 BTC total)
+        self.nodes[2].createwallet("test_preset_inputs_selection")
+        wallet = self.nodes[2].get_wallet_rpc("test_preset_inputs_selection")
+        outputs = {}
+        for _ in range(4):
+            outputs[wallet.getnewaddress(address_type="bech32")] = 5
+        self.nodes[0].sendmany("", outputs)
+        self.generate(self.nodes[0], 1)
+
+        # Select the preset inputs
+        coins = wallet.listunspent()
+        preset_inputs = [coins[0], coins[1], coins[2]]
+
+        # Now let's create the tx creation options
+        options = {
+            "inputs": preset_inputs,
+            "add_inputs": True,  # automatically add coins from the wallet to fulfill the target
+            "subtract_fee_from_outputs": [0],  # deduct fee from first output
+            "add_to_wallet": False
+        }
+
+        # Attempt to send 29 BTC from a wallet that only has 20 BTC. The wallet should exclude
+        # the preset inputs from the pool of available coins, realize that there is not enough
+        # money to fund the 29 BTC payment, and fail with "Insufficient funds".
+        #
+        # Even with SFFO, the wallet can only afford to send 20 BTC.
+        # If the wallet does not properly exclude preset inputs from the pool of available coins
+        # prior to coin selection, it may create a transaction that does not fund the full payment
+        # amount or, through SFFO, incorrectly reduce the recipient's amount by the difference
+        # between the original target and the wrongly counted inputs (in this case 9 BTC)
+        # so that the recipient's amount is no longer equal to the user's selected target of 29 BTC.
+
+        # First case, use 'subtract_fee_from_outputs = true'
+        assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{wallet.getnewaddress(address_type="bech32"): 29}], options=options)
+
+        # Second case, don't use 'subtract_fee_from_outputs'
+        del options["subtract_fee_from_outputs"]
+        assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{wallet.getnewaddress(address_type="bech32"): 29}], options=options)
+
+        self.nodes[2].unloadwallet("test_preset_inputs_selection")
+
     def test_weight_calculation(self):
         self.log.info("Test weight calculation with external inputs")
 
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index d78c1c634f..caa4af957a 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -316,6 +316,7 @@ BASE_SCRIPTS = [
     'rpc_deriveaddresses.py',
     'rpc_deriveaddresses.py --usecli',
     'p2p_ping.py',
+    'p2p_tx_privacy.py',
     'rpc_scantxoutset.py',
     'feature_txindex_compatibility.py',
     'feature_unsupported_utxo_db.py',
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 3c1cb6ac32..4f060f9960 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -393,6 +393,15 @@ class WalletMigrationTest(BitcoinTestFramework):
 
         assert_equal(bals, wallet.getbalances())
 
+    def test_encrypted(self):
+        self.log.info("Test migration of an encrypted wallet")
+        wallet = self.create_legacy_wallet("encrypted")
+
+        wallet.encryptwallet("pass")
+
+        assert_raises_rpc_error(-15, "Error: migratewallet on encrypted wallets is currently unsupported.", wallet.migratewallet)
+        # TODO: Fix migratewallet so that we can actually migrate encrypted wallets
+
     def run_test(self):
         self.generate(self.nodes[0], 101)
 
@@ -402,6 +411,7 @@ class WalletMigrationTest(BitcoinTestFramework):
         self.test_other_watchonly()
         self.test_no_privkeys()
         self.test_pk_coinbases()
+        self.test_encrypted()
 
 if __name__ == '__main__':
     WalletMigrationTest().main()