[rpc] fundrawtransaction, walletcreatefundedpsbt lock manually select…

…ed coins

Summary:
Previously only automatically selected coins were locked when lockUnspents is set.
It now also locks selected coins.

This is a backport of [[bitcoin/bitcoin#18244 | core#18244]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10144

doc/release-notes.md

@@ -9,4 +9,8 @@ This release includes the following features and fixes:
    (previously was `Bitcoin Signed Message:`).
  - A new `getindexinfo` RPC returns the actively running indices of the node,
    including their current sync status and height. It also accepts an `index_name`
-   to specify returning only the status of that index.
+   to specify returning only the status of that index.
+ - `fundrawtransaction` and `walletcreatefundedpsbt` when used with the `lockUnspents`
+   argument now lock manually selected coins, in addition to automatically selected
+   coins. Note that locked coins are never used in automatic coin selection, but
+   can still be manually selected.

src/wallet/rpcwallet.cpp

@@ -2536,6 +2536,7 @@ static UniValue lockunspent(const Config &config,
         "current locked transaction outputs are unlocked.\n"
         "A locked transaction output will not be chosen by automatic coin "
         "selection, when spending bitcoins.\n"
+        "Manually selected coins are automatically unlocked.\n"
         "Locks are stored in memory only. Nodes start with zero locked "
         "outputs, and the locked output list\n"
         "is always cleared (by virtue of process exit) when a node stops or "

src/wallet/wallet.cpp

@@ -2923,10 +2923,9 @@ bool CWallet::FundTransaction(CMutableTransaction &tx, Amount &nFeeRet,
     for (const CTxIn &txin : tx_new->vin) {
         if (!coinControl.IsSelected(txin.prevout)) {
             tx.vin.push_back(txin);
-
-            if (lockUnspents) {
-                LockCoin(txin.prevout);
-            }
+        }
+        if (lockUnspents) {
+            LockCoin(txin.prevout);
         }
     }
 

test/functional/rpc_psbt.py

@@ -58,6 +58,20 @@ def run_test(self):
         final_tx = self.nodes[0].finalizepsbt(signed_tx)['hex']
         self.nodes[0].sendrawtransaction(final_tx)
 
+        # Manually selected inputs can be locked:
+        assert_equal(len(self.nodes[0].listlockunspent()), 0)
+        utxo1 = self.nodes[0].listunspent()[0]
+        psbtx1 = self.nodes[0].walletcreatefundedpsbt(
+            [{"txid": utxo1['txid'], "vout": utxo1['vout']}],
+            {self.nodes[2].getnewaddress(): 1_000_000}, 0,
+            {"lockUnspents": True})["psbt"]
+        assert_equal(len(self.nodes[0].listlockunspent()), 1)
+
+        # Locks are ignored for manually selected inputs
+        self.nodes[0].walletcreatefundedpsbt(
+            [{"txid": utxo1['txid'], "vout": utxo1['vout']}],
+            {self.nodes[2].getnewaddress(): 1_000_000}, 0)
+
         # Create p2sh, p2pkh addresses
         pubkey0 = self.nodes[0].getaddressinfo(
             self.nodes[0].getnewaddress())['pubkey']

test/functional/wallet_basic.py

@@ -140,12 +140,22 @@ def run_test(self):
         assert_raises_rpc_error(-8, "Invalid parameter, vout index out of bounds",
                                 self.nodes[2].lockunspent, False, [{"txid": unspent_0["txid"], "vout": 999}])
 
-        # An output should be unlocked when spent
+        # The lock on a manually selected output is ignored
         unspent_0 = self.nodes[1].listunspent()[0]
         self.nodes[1].lockunspent(False, [unspent_0])
         tx = self.nodes[1].createrawtransaction(
             [unspent_0], {self.nodes[1].getnewaddress(): 1000000})
         tx = self.nodes[1].fundrawtransaction(tx)['hex']
+        self.nodes[1].fundrawtransaction(tx, {"lockUnspents": True})
+
+        # fundrawtransaction can lock an input
+        self.nodes[1].lockunspent(True, [unspent_0])
+        assert_equal(len(self.nodes[1].listlockunspent()), 0)
+        tx = self.nodes[1].fundrawtransaction(
+            tx, {"lockUnspents": True})['hex']
+        assert_equal(len(self.nodes[1].listlockunspent()), 1)
+
+        # Send transaction
         tx = self.nodes[1].signrawtransactionwithwallet(tx)["hex"]
         self.nodes[1].sendrawtransaction(tx)
         assert_equal(len(self.nodes[1].listlockunspent()), 0)