Merge bitcoin/bitcoin#26410: [24.x] rc3 backports

d5701900fc rpc: make `address` field optional (w0xlt)
e4b8c9b2bf rpc: add non-regression test about deriveaddresses crash when index is 2147483647 (muxator)
bf2bf73bcb rpc: fix crash in deriveaddresses when derivation index is 2147483647 (muxator)
b04f5f9608 test: Test for out of bounds vout in sendall (Andrew Chow)
dedee6af57 wallet: Check utxo prevout index out of bounds in sendall (Andrew Chow)
931db785ee test: Test that sendall works with watchonly spending specific utxos (Andrew Chow)
bbe864a13a wallet: Correctly check ismine for sendall (Andrew Chow)
4b7d30d026 Adjust `.tx/config` for new Transifex CLI (Hennadii Stepanov)

Pull request description:

  Backports:
  * https://github.com/bitcoin/bitcoin/pull/26321
  * https://github.com/bitcoin/bitcoin/pull/26344
  * https://github.com/bitcoin/bitcoin/pull/26275
  * https://github.com/bitcoin/bitcoin/pull/26349

ACKs for top commit:
  instagibbs:
    ACK d5701900fc
  hebasto:
    ACK d5701900fc, I've cherry-picked commits manually and got zero diff with this PR branch.

Tree-SHA512: dad64f4074b4f06d666c0f2d804eda92df241bcce0a49c28486311a151f2e9d46b75e1bce02de570dcc85957c9ce936debb2a4faa045800c9757c6c495115d7c

.tx/config

@@ -1,7 +1,7 @@
 [main]
 host = https://www.transifex.com
 
-[bitcoin.qt-translation-024x]
+[o:bitcoin:p:bitcoin:r:qt-translation-024x]
 file_filter = src/qt/locale/bitcoin_<lang>.xlf
 source_file = src/qt/locale/bitcoin_en.xlf
 source_lang = en

src/rpc/output_script.cpp

@@ -273,7 +273,7 @@ static RPCHelpMan deriveaddresses()
 
             UniValue addresses(UniValue::VARR);
 
-            for (int i = range_begin; i <= range_end; ++i) {
+            for (int64_t i = range_begin; i <= range_end; ++i) {
                 FlatSigningProvider provider;
                 std::vector<CScript> scripts;
                 if (!desc->Expand(i, key_provider, scripts, provider)) {

src/wallet/rpc/spend.cpp

@@ -1380,7 +1380,7 @@ RPCHelpMan sendall()
                         throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not available. UTXO (%s:%d) was already spent.", input.prevout.hash.ToString(), input.prevout.n));
                     }
                     const CWalletTx* tx{pwallet->GetWalletTx(input.prevout.hash)};
-                    if (!tx || pwallet->IsMine(tx->tx->vout[input.prevout.n]) != (coin_control.fAllowWatchOnly ? ISMINE_ALL : ISMINE_SPENDABLE)) {
+                    if (!tx || input.prevout.n >= tx->tx->vout.size() || !(pwallet->IsMine(tx->tx->vout[input.prevout.n]) & (coin_control.fAllowWatchOnly ? ISMINE_ALL : ISMINE_SPENDABLE))) {
                         throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not found. UTXO (%s:%d) is not part of wallet.", input.prevout.hash.ToString(), input.prevout.n));
                     }
                     total_input_value += tx->tx->vout[input.prevout.n].nValue;

src/wallet/rpc/transactions.cpp

@@ -447,7 +447,7 @@ RPCHelpMan listtransactions()
                         {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>(
                         {
                             {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."},
-                            {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."},
+                            {RPCResult::Type::STR, "address",  /*optional=*/true, "The bitcoin address of the transaction (not returned if the output does not have an address, e.g. OP_RETURN null data)."},
                             {RPCResult::Type::STR, "category", "The transaction category.\n"
                                 "\"send\"                  Transactions sent.\n"
                                 "\"receive\"               Non-coinbase transactions received.\n"
@@ -561,7 +561,7 @@ RPCHelpMan listsinceblock()
                             {RPCResult::Type::OBJ, "", "", Cat(Cat<std::vector<RPCResult>>(
                             {
                                 {RPCResult::Type::BOOL, "involvesWatchonly", /*optional=*/true, "Only returns true if imported addresses were involved in transaction."},
-                                {RPCResult::Type::STR, "address", "The bitcoin address of the transaction."},
+                                {RPCResult::Type::STR, "address",  /*optional=*/true, "The bitcoin address of the transaction (not returned if the output does not have an address, e.g. OP_RETURN null data)."},
                                 {RPCResult::Type::STR, "category", "The transaction category.\n"
                                     "\"send\"                  Transactions sent.\n"
                                     "\"receive\"               Non-coinbase transactions received.\n"

test/functional/rpc_deriveaddresses.py

@@ -44,6 +44,13 @@ class DeriveaddressesTest(BitcoinTestFramework):
         combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)")
         assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", "mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"])
 
+        # Before #26275, bitcoind would crash when deriveaddresses was
+        # called with derivation index 2147483647, which is the maximum
+        # positive value of a signed int32, and - currently - the
+        # maximum value that the deriveaddresses bitcoin RPC call
+        # accepts as derivation index.
+        assert_equal(self.nodes[0].deriveaddresses(descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [2147483647, 2147483647]), ["bcrt1qtzs23vgzpreks5gtygwxf8tv5rldxvvsyfpdkg"])
+
         hardened_without_privkey_descriptor = descsum_create("wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1'/1/0)")
         assert_raises_rpc_error(-5, "Cannot derive script without private keys", self.nodes[0].deriveaddresses, hardened_without_privkey_descriptor)
 

test/functional/wallet_listsinceblock.py

@@ -45,6 +45,7 @@ class ListSinceBlockTest(BitcoinTestFramework):
         if self.options.descriptors:
             self.test_desc()
         self.test_send_to_self()
+        self.test_op_return()
 
     def test_no_blockhash(self):
         self.log.info("Test no blockhash")
@@ -448,6 +449,19 @@ class ListSinceBlockTest(BitcoinTestFramework):
         assert any(c["address"] == addr for c in coins)
         assert all(self.nodes[2].getaddressinfo(c["address"])["ischange"] for c in coins)
 
+    def test_op_return(self):
+        """Test if OP_RETURN outputs will be displayed correctly."""
+        block_hash = self.nodes[2].getbestblockhash()
+
+        raw_tx = self.nodes[2].createrawtransaction([], [{'data': 'aa'}])
+        funded_tx = self.nodes[2].fundrawtransaction(raw_tx)
+        signed_tx = self.nodes[2].signrawtransactionwithwallet(funded_tx['hex'])
+        tx_id = self.nodes[2].sendrawtransaction(signed_tx['hex'])
+
+        op_ret_tx = [tx for tx in self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] if tx['txid'] == tx_id][0]
+
+        assert 'address' not in op_ret_tx
+
 
 if __name__ == '__main__':
     ListSinceBlockTest().main()

test/functional/wallet_listtransactions.py

@@ -109,6 +109,7 @@ class ListTransactionsTest(BitcoinTestFramework):
         self.run_rbf_opt_in_test()
         self.run_externally_generated_address_test()
         self.run_invalid_parameters_test()
+        self.test_op_return()
 
     def run_rbf_opt_in_test(self):
         """Test the opt-in-rbf flag for sent and received transactions."""
@@ -284,6 +285,17 @@ class ListTransactionsTest(BitcoinTestFramework):
         assert_raises_rpc_error(-8, "Negative count", self.nodes[0].listtransactions, count=-1)
         assert_raises_rpc_error(-8, "Negative from", self.nodes[0].listtransactions, skip=-1)
 
+    def test_op_return(self):
+        """Test if OP_RETURN outputs will be displayed correctly."""
+        raw_tx = self.nodes[0].createrawtransaction([], [{'data': 'aa'}])
+        funded_tx = self.nodes[0].fundrawtransaction(raw_tx)
+        signed_tx = self.nodes[0].signrawtransactionwithwallet(funded_tx['hex'])
+        tx_id = self.nodes[0].sendrawtransaction(signed_tx['hex'])
+
+        op_ret_tx = [tx for tx in self.nodes[0].listtransactions() if tx['txid'] == tx_id][0]
+
+        assert 'address' not in op_ret_tx
+
 
 if __name__ == '__main__':
     ListTransactionsTest().main()

test/functional/wallet_sendall.py

@@ -221,6 +221,11 @@ class SendallTest(BitcoinTestFramework):
         self.add_utxos([16, 5])
         spent_utxo = self.wallet.listunspent()[0]
 
+        # fails on out of bounds vout
+        assert_raises_rpc_error(-8,
+                "Input not found. UTXO ({}:{}) is not part of wallet.".format(spent_utxo["txid"], 1000),
+                self.wallet.sendall, recipients=[self.remainder_target], options={"inputs": [{"txid": spent_utxo["txid"], "vout": 1000}]})
+
         # fails on unconfirmed spent UTXO
         self.wallet.sendall(recipients=[self.remainder_target])
         assert_raises_rpc_error(-8,
@@ -276,6 +281,33 @@ class SendallTest(BitcoinTestFramework):
                 recipients=[self.remainder_target],
                 fee_rate=100000)
 
+    @cleanup
+    def sendall_watchonly_specific_inputs(self):
+        self.log.info("Test sendall with a subset of UTXO pool in a watchonly wallet")
+        self.add_utxos([17, 4])
+        utxo = self.wallet.listunspent()[0]
+
+        self.nodes[0].createwallet(wallet_name="watching", disable_private_keys=True)
+        watchonly = self.nodes[0].get_wallet_rpc("watching")
+
+        import_req = [{
+            "desc": utxo["desc"],
+            "timestamp": 0,
+        }]
+        if self.options.descriptors:
+            watchonly.importdescriptors(import_req)
+        else:
+            watchonly.importmulti(import_req)
+
+        sendall_tx_receipt = watchonly.sendall(recipients=[self.remainder_target], options={"inputs": [utxo]})
+        psbt = sendall_tx_receipt["psbt"]
+        decoded = self.nodes[0].decodepsbt(psbt)
+        assert_equal(len(decoded["inputs"]), 1)
+        assert_equal(len(decoded["outputs"]), 1)
+        assert_equal(decoded["tx"]["vin"][0]["txid"], utxo["txid"])
+        assert_equal(decoded["tx"]["vin"][0]["vout"], utxo["vout"])
+        assert_equal(decoded["tx"]["vout"][0]["scriptPubKey"]["address"], self.remainder_target)
+
     # This tests needs to be the last one otherwise @cleanup will fail with "Transaction too large" error
     def sendall_fails_with_transaction_too_large(self):
         self.log.info("Test that sendall fails if resulting transaction is too large")
@@ -341,6 +373,9 @@ class SendallTest(BitcoinTestFramework):
         # Sendall fails when providing a fee that is too high
         self.sendall_fails_on_high_fee()
 
+        # Sendall succeeds with watchonly wallets spending specific UTXOs
+        self.sendall_watchonly_specific_inputs()
+
         # Sendall fails when many inputs result to too large transaction
         self.sendall_fails_with_transaction_too_large()