From 4b7d30d026815dbe2330cd3e2edc044835a3eaed Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 15 Oct 2022 19:11:39 +0100 Subject: [PATCH 1/8] Adjust `.tx/config` for new Transifex CLI The old Transifex Command-Line Tool is considered deprecated (as of January 2022) and will sunset on Nov 30, 2022. See: https://github.com/transifex/cli/blob/devel/README.md#migrating-from-older-versions-of-the-client An accompanying PR: https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/142 Github-Pull: #26321 Rebased-From: d6adbb7ee1de661ad89879609eecd11129322405 --- .tx/config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.tx/config b/.tx/config index 37c0bca0ab8..20eff98d28b 100644 --- a/.tx/config +++ b/.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_.xlf source_file = src/qt/locale/bitcoin_en.xlf source_lang = en From bbe864a13a2e5ce15674eda5c3760ee851120c63 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 19 Oct 2022 15:13:11 -0400 Subject: [PATCH 2/8] wallet: Correctly check ismine for sendall sendall should be using a bitwise AND for sendall's IsMine check rather than an equality as IsMine will never return ISMINE_ALL. Github-Pull: #26344 Rebased-From: 6bcd7e2a3b52f855db84cd23b5ee70d27be3434f --- src/wallet/rpc/spend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index e38b13624c2..7d105b35b88 100644 --- a/src/wallet/rpc/spend.cpp +++ b/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 || !(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; From 931db785ee6f5c34e0f053314bc8c70b01642b72 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 19 Oct 2022 15:21:23 -0400 Subject: [PATCH 3/8] test: Test that sendall works with watchonly spending specific utxos Github-Pull: #26344 Rebased-From: 708b72b7151c855cb5dac2fb6a81e8f35153c46f --- test/functional/wallet_sendall.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py index db4f32fe161..3b8835e8100 100755 --- a/test/functional/wallet_sendall.py +++ b/test/functional/wallet_sendall.py @@ -276,6 +276,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 +368,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() From dedee6af572471b9beeebca9543934e788484b2e Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 20 Oct 2022 13:24:57 -0400 Subject: [PATCH 4/8] wallet: Check utxo prevout index out of bounds in sendall Github-Pull: #26344 Rebased-From: b132c85650afb2182f2e58e903f3d6f86fd3fb22 --- src/wallet/rpc/spend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 7d105b35b88..bc65cbf7bfe 100644 --- a/src/wallet/rpc/spend.cpp +++ b/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; From b04f5f960893983400e07b96dbe9fe68383a21d2 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 20 Oct 2022 13:25:13 -0400 Subject: [PATCH 5/8] test: Test for out of bounds vout in sendall Github-Pull: #26344 Rebased-From: 315fd4dbabb6b631b755811742a3bdf93e1241bf --- test/functional/wallet_sendall.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/functional/wallet_sendall.py b/test/functional/wallet_sendall.py index 3b8835e8100..4fe11455b13 100755 --- a/test/functional/wallet_sendall.py +++ b/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, From bf2bf73bcbc5277074f1211c20b71995a175c314 Mon Sep 17 00:00:00 2001 From: muxator Date: Thu, 6 Oct 2022 22:17:49 +0200 Subject: [PATCH 6/8] rpc: fix crash in deriveaddresses when derivation index is 2147483647 2147483647 is the maximum positive value of a signed int32, and - currently - the maximum value that the deriveaddresses bitcoin RPC call accepts as derivation index due to its input validation routines. Before this change, when the derivation index (and thus range_end) reached std::numeric_limits::max(), the "i" variable in the for cycle (which is declared as int, and as such 32 bits in size on most platforms) would be incremented at the end of the first iteration and then warp back to -2147483648. This caused SIGABRT in bitcoind and a core dump. This change assigns "i" an explicit size of 64 bits on every platform, sidestepping the problem. Fixes #26274. Github-Pull: #26275 Rebased-From: addf9d6502db12cebcc5976df3111cac1a369b82 --- src/rpc/output_script.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index 744f8098147..a980c609e80 100644 --- a/src/rpc/output_script.cpp +++ b/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 scripts; if (!desc->Expand(i, key_provider, scripts, provider)) { From e4b8c9b2bf2118064e68d33f6b7207e721ae03dd Mon Sep 17 00:00:00 2001 From: muxator Date: Thu, 6 Oct 2022 12:03:36 +0200 Subject: [PATCH 7/8] rpc: add non-regression test about deriveaddresses crash when index is 2147483647 This test would cause a crash in bitcoind (see #26274) if the fix given in the previous commit was not applied. Github-Pull: #26275 Rebased-From: 9153ff3e274953ea0d92d53ddab4c72deeace1b1 --- test/functional/rpc_deriveaddresses.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py index 42d7d59d563..a69326736d7 100755 --- a/test/functional/rpc_deriveaddresses.py +++ b/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) From d5701900fcf70220701a1686588114db165dce1c Mon Sep 17 00:00:00 2001 From: w0xlt Date: Wed, 19 Oct 2022 16:10:04 -0300 Subject: [PATCH 8/8] rpc: make `address` field optional Github-Pull: #26349 Rebased-From: eb679a7896ce00e322972a011b023661766923b9 --- src/wallet/rpc/transactions.cpp | 4 ++-- test/functional/wallet_listsinceblock.py | 14 ++++++++++++++ test/functional/wallet_listtransactions.py | 12 ++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 0e13e4756bc..6b9ec8ce10b 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -447,7 +447,7 @@ RPCHelpMan listtransactions() {RPCResult::Type::OBJ, "", "", Cat(Cat>( { {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>( { {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" diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index f259449bef4..aff408ceb14 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/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() diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 7c16b6328dd..9bb06774a5e 100755 --- a/test/functional/wallet_listtransactions.py +++ b/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()