diff options
-rw-r--r-- | doc/release-notes-25504.md | 6 | ||||
-rw-r--r-- | src/rpc/client.cpp | 1 | ||||
-rw-r--r-- | src/wallet/receive.cpp | 6 | ||||
-rw-r--r-- | src/wallet/receive.h | 3 | ||||
-rw-r--r-- | src/wallet/rpc/coins.cpp | 4 | ||||
-rw-r--r-- | src/wallet/rpc/transactions.cpp | 23 | ||||
-rw-r--r-- | src/wallet/rpc/util.cpp | 9 | ||||
-rw-r--r-- | src/wallet/rpc/util.h | 4 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 12 | ||||
-rw-r--r-- | src/wallet/wallet.h | 3 | ||||
-rwxr-xr-x | test/functional/wallet_basic.py | 33 | ||||
-rwxr-xr-x | test/functional/wallet_listsinceblock.py | 63 |
12 files changed, 158 insertions, 9 deletions
diff --git a/doc/release-notes-25504.md b/doc/release-notes-25504.md new file mode 100644 index 0000000000..bf80f18031 --- /dev/null +++ b/doc/release-notes-25504.md @@ -0,0 +1,6 @@ +Updated RPCs +------------ + +- The `listsinceblock`, `listtransactions` and `gettransaction` output now contain a new + `parent_descs` field for every "receive" entry. +- A new optional `include_change` parameter was added to the `listsinceblock` command. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index ee287656b6..612dbbdacf 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -74,6 +74,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "listsinceblock", 1, "target_confirmations" }, { "listsinceblock", 2, "include_watchonly" }, { "listsinceblock", 3, "include_removed" }, + { "listsinceblock", 4, "include_change" }, { "sendmany", 1, "amounts" }, { "sendmany", 2, "minconf" }, { "sendmany", 4, "subtractfeefrom" }, diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 944925d600..db169aad12 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -193,7 +193,8 @@ CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx, std::list<COutputEntry>& listReceived, - std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter) + std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter, + bool include_change) { nFee = 0; listReceived.clear(); @@ -218,8 +219,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx, // 2) the output is to us (received) if (nDebit > 0) { - // Don't report 'change' txouts - if (OutputIsChange(wallet, txout)) + if (!include_change && OutputIsChange(wallet, txout)) continue; } else if (!(fIsMine & filter)) diff --git a/src/wallet/receive.h b/src/wallet/receive.h index 41a70b089a..9125b1e9aa 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -42,7 +42,8 @@ struct COutputEntry void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx, std::list<COutputEntry>& listReceived, std::list<COutputEntry>& listSent, - CAmount& nFee, const isminefilter& filter); + CAmount& nFee, const isminefilter& filter, + bool include_change); bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter); bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uint256>& trusted_parents) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index d40d800f28..c01ca28290 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -543,6 +543,9 @@ RPCHelpMan listunspent() {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"}, {RPCResult::Type::BOOL, "reused", /*optional=*/true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, {RPCResult::Type::STR, "desc", /*optional=*/true, "(only when solvable) A descriptor for spending this output"}, + {RPCResult::Type::ARR, "parent_descs", /*optional=*/false, "List of parent descriptors for the scriptPubKey of this coin.", { + {RPCResult::Type::STR, "desc", "The descriptor string."}, + }}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions\n" "from outside keys and unconfirmed replacement transactions are considered unsafe\n" "and are not eligible for spending by fundrawtransaction and sendtoaddress."}, @@ -722,6 +725,7 @@ RPCHelpMan listunspent() entry.pushKV("desc", descriptor->ToString()); } } + PushParentDescriptors(*pwallet, scriptPubKey, entry); if (avoid_reuse) entry.pushKV("reused", reused); entry.pushKV("safe", out.safe); results.push_back(entry); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 9c8003d6d7..a0769d3ecb 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -315,13 +315,16 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) * @param filter_label Optional label string to filter incoming transactions. */ template <class Vec> -static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, + Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label, + bool include_change = false) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { CAmount nFee; std::list<COutputEntry> listReceived; std::list<COutputEntry> listSent; - CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, filter_ismine); + CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, filter_ismine, include_change); bool involvesWatchonly = CachedTxIsFromMe(wallet, wtx, ISMINE_WATCH_ONLY); @@ -367,6 +370,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM entry.pushKV("involvesWatchonly", true); } MaybePushAddress(entry, r.destination); + PushParentDescriptors(wallet, wtx.tx->vout.at(r.vout).scriptPubKey, entry); if (wtx.IsCoinBase()) { if (wallet.GetTxDepthInMainChain(wtx) < 1) @@ -418,7 +422,11 @@ static const std::vector<RPCResult> TransactionDescriptionString() {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR, "comment", /*optional=*/true, "If a comment is associated with the transaction, only present if not empty."}, {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - "may be unknown for unconfirmed transactions not in the mempool."}}; + "may be unknown for unconfirmed transactions not in the mempool."}, + {RPCResult::Type::ARR, "parent_descs", /*optional=*/true, "Only if 'category' is 'received'. List of parent descriptors for the scriptPubKey of this coin.", { + {RPCResult::Type::STR, "desc", "The descriptor string."}, + }}, + }; } RPCHelpMan listtransactions() @@ -543,6 +551,7 @@ RPCHelpMan listsinceblock() {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Include transactions to watch-only addresses (see 'importaddress')"}, {"include_removed", RPCArg::Type::BOOL, RPCArg::Default{true}, "Show transactions that were removed due to a reorg in the \"removed\" array\n" "(not guaranteed to work on pruned nodes)"}, + {"include_change", RPCArg::Type::BOOL, RPCArg::Default{false}, "Also add entries for change outputs.\n"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -623,6 +632,7 @@ RPCHelpMan listsinceblock() } bool include_removed = (request.params[3].isNull() || request.params[3].get_bool()); + bool include_change = (!request.params[4].isNull() && request.params[4].get_bool()); int depth = height ? wallet.GetLastBlockHeight() + 1 - *height : -1; @@ -632,7 +642,7 @@ RPCHelpMan listsinceblock() const CWalletTx& tx = pairWtx.second; if (depth == -1 || abs(wallet.GetTxDepthInMainChain(tx)) < depth) { - ListTransactions(wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); + ListTransactions(wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */, /*include_change=*/include_change); } } @@ -649,7 +659,7 @@ RPCHelpMan listsinceblock() if (it != wallet.mapWallet.end()) { // We want all transactions regardless of confirmation count to appear here, // even negative confirmation ones, hence the big negative. - ListTransactions(wallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */); + ListTransactions(wallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */, /*include_change=*/include_change); } } blockId = block.hashPrevBlock; @@ -709,6 +719,9 @@ RPCHelpMan gettransaction() "'send' category of transactions."}, {RPCResult::Type::BOOL, "abandoned", /*optional=*/true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, + {RPCResult::Type::ARR, "parent_descs", /*optional=*/true, "Only if 'category' is 'received'. List of parent descriptors for the scriptPubKey of this coin.", { + {RPCResult::Type::STR, "desc", "The descriptor string."}, + }}, }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 4fcb393226..16305d4465 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -123,6 +123,15 @@ std::string LabelFromValue(const UniValue& value) return label; } +void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry) +{ + UniValue parent_descs(UniValue::VARR); + for (const auto& desc: wallet.GetWalletDescriptors(script_pubkey)) { + parent_descs.push_back(desc.descriptor->ToString()); + } + entry.pushKV("parent_descs", parent_descs); +} + void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error) { if (!wallet) { diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h index 7b810eb06e..2ca28914e7 100644 --- a/src/wallet/rpc/util.h +++ b/src/wallet/rpc/util.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_WALLET_RPC_UTIL_H #define BITCOIN_WALLET_RPC_UTIL_H +#include <script/script.h> + #include <any> #include <memory> #include <string> @@ -39,6 +41,8 @@ const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wal bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param); bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet); std::string LabelFromValue(const UniValue& value); +//! Fetch parent descriptors of this scriptPubKey. +void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry); void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error); } // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index de1078e646..e0a794dbe6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3336,6 +3336,18 @@ std::unique_ptr<SigningProvider> CWallet::GetSolvingProvider(const CScript& scri return nullptr; } +std::vector<WalletDescriptor> CWallet::GetWalletDescriptors(const CScript& script) const +{ + std::vector<WalletDescriptor> descs; + for (const auto spk_man: GetScriptPubKeyMans(script)) { + if (const auto desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man)) { + LOCK(desc_spk_man->cs_desc_man); + descs.push_back(desc_spk_man->GetWalletDescriptor()); + } + } + return descs; +} + LegacyScriptPubKeyMan* CWallet::GetLegacyScriptPubKeyMan() const { if (IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cf33ea21f2..247c172da3 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -845,6 +845,9 @@ public: std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const; std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script, SignatureData& sigdata) const; + //! Get the wallet descriptors for a script. + std::vector<WalletDescriptor> GetWalletDescriptors(const CScript& script) const; + //! Get the LegacyScriptPubKeyMan which is used for all types, internal, and external. LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const; LegacyScriptPubKeyMan* GetOrCreateLegacyScriptPubKeyMan(); diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 9cf1b3d2c4..adeb551895 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -7,6 +7,7 @@ from decimal import Decimal from itertools import product from test_framework.blocktools import COINBASE_MATURITY +from test_framework.descriptors import descsum_create from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_array_result, @@ -700,6 +701,38 @@ class WalletTest(BitcoinTestFramework): txid_feeReason_four = self.nodes[2].sendmany(dummy='', amounts={address: 5}, verbose=False) assert_equal(self.nodes[2].gettransaction(txid_feeReason_four)['txid'], txid_feeReason_four) + self.log.info("Testing 'listunspent' outputs the parent descriptor(s) of coins") + # Create two multisig descriptors, and send a UTxO each. + multi_a = descsum_create("wsh(multi(1,tpubD6NzVbkrYhZ4YBNjUo96Jxd1u4XKWgnoc7LsA1jz3Yc2NiDbhtfBhaBtemB73n9V5vtJHwU6FVXwggTbeoJWQ1rzdz8ysDuQkpnaHyvnvzR/*,tpubD6NzVbkrYhZ4YHdDGMAYGaWxMSC1B6tPRTHuU5t3BcfcS3nrF523iFm5waFd1pP3ZvJt4Jr8XmCmsTBNx5suhcSgtzpGjGMASR3tau1hJz4/*))") + multi_b = descsum_create("wsh(multi(1,tpubD6NzVbkrYhZ4YHdDGMAYGaWxMSC1B6tPRTHuU5t3BcfcS3nrF523iFm5waFd1pP3ZvJt4Jr8XmCmsTBNx5suhcSgtzpGjGMASR3tau1hJz4/*,tpubD6NzVbkrYhZ4Y2RLiuEzNQkntjmsLpPYDm3LTRBYynUQtDtpzeUKAcb9sYthSFL3YR74cdFgF5mW8yKxv2W2CWuZDFR2dUpE5PF9kbrVXNZ/*))") + addr_a = self.nodes[0].deriveaddresses(multi_a, 0)[0] + addr_b = self.nodes[0].deriveaddresses(multi_b, 0)[0] + txid_a = self.nodes[0].sendtoaddress(addr_a, 0.01) + txid_b = self.nodes[0].sendtoaddress(addr_b, 0.01) + self.generate(self.nodes[0], 1, sync_fun=self.no_op) + # Now import the descriptors, make sure we can identify on which descriptor each coin was received. + self.nodes[0].createwallet(wallet_name="wo", descriptors=True, disable_private_keys=True) + wo_wallet = self.nodes[0].get_wallet_rpc("wo") + wo_wallet.importdescriptors([ + { + "desc": multi_a, + "active": False, + "timestamp": "now", + }, + { + "desc": multi_b, + "active": False, + "timestamp": "now", + }, + ]) + coins = wo_wallet.listunspent(minconf=0) + assert_equal(len(coins), 2) + coin_a = next(c for c in coins if c["txid"] == txid_a) + assert_equal(coin_a["parent_descs"][0], multi_a) + coin_b = next(c for c in coins if c["txid"] == txid_b) + assert_equal(coin_b["parent_descs"][0], multi_b) + self.nodes[0].unloadwallet("wo") + if __name__ == '__main__': WalletTest().main() diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 1f3a276d9c..36dbec467b 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -6,6 +6,7 @@ from test_framework.address import key_to_p2wpkh from test_framework.blocktools import COINBASE_MATURITY +from test_framework.descriptors import descsum_create from test_framework.key import ECKey from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import MAX_BIP125_RBF_SEQUENCE @@ -39,6 +40,8 @@ class ListSinceBlockTest(BitcoinTestFramework): self.test_double_send() self.double_spends_filtered() self.test_targetconfirmations() + self.test_desc() + self.test_send_to_self() def test_no_blockhash(self): self.log.info("Test no blockhash") @@ -383,5 +386,65 @@ class ListSinceBlockTest(BitcoinTestFramework): assert_equal(original_found, False) assert_equal(double_found, False) + def test_desc(self): + """Make sure we can track coins by descriptor.""" + self.log.info("Test descriptor lookup by scriptPubKey.") + + # Create a watchonly wallet tracking two multisig descriptors. + multi_a = descsum_create("wsh(multi(1,tpubD6NzVbkrYhZ4YBNjUo96Jxd1u4XKWgnoc7LsA1jz3Yc2NiDbhtfBhaBtemB73n9V5vtJHwU6FVXwggTbeoJWQ1rzdz8ysDuQkpnaHyvnvzR/*,tpubD6NzVbkrYhZ4YHdDGMAYGaWxMSC1B6tPRTHuU5t3BcfcS3nrF523iFm5waFd1pP3ZvJt4Jr8XmCmsTBNx5suhcSgtzpGjGMASR3tau1hJz4/*))") + multi_b = descsum_create("wsh(multi(1,tpubD6NzVbkrYhZ4YHdDGMAYGaWxMSC1B6tPRTHuU5t3BcfcS3nrF523iFm5waFd1pP3ZvJt4Jr8XmCmsTBNx5suhcSgtzpGjGMASR3tau1hJz4/*,tpubD6NzVbkrYhZ4Y2RLiuEzNQkntjmsLpPYDm3LTRBYynUQtDtpzeUKAcb9sYthSFL3YR74cdFgF5mW8yKxv2W2CWuZDFR2dUpE5PF9kbrVXNZ/*))") + self.nodes[0].createwallet(wallet_name="wo", descriptors=True, disable_private_keys=True) + wo_wallet = self.nodes[0].get_wallet_rpc("wo") + wo_wallet.importdescriptors([ + { + "desc": multi_a, + "active": False, + "timestamp": "now", + }, + { + "desc": multi_b, + "active": False, + "timestamp": "now", + }, + ]) + + # Send a coin to each descriptor. + assert_equal(len(wo_wallet.listsinceblock()["transactions"]), 0) + addr_a = self.nodes[0].deriveaddresses(multi_a, 0)[0] + addr_b = self.nodes[0].deriveaddresses(multi_b, 0)[0] + self.nodes[2].sendtoaddress(addr_a, 1) + self.nodes[2].sendtoaddress(addr_b, 2) + self.generate(self.nodes[2], 1) + + # We can identify on which descriptor each coin was received. + coins = wo_wallet.listsinceblock()["transactions"] + assert_equal(len(coins), 2) + coin_a = next(c for c in coins if c["amount"] == 1) + assert_equal(coin_a["parent_descs"][0], multi_a) + coin_b = next(c for c in coins if c["amount"] == 2) + assert_equal(coin_b["parent_descs"][0], multi_b) + + def test_send_to_self(self): + """We can make listsinceblock output our change outputs.""" + self.log.info("Test the inclusion of change outputs in the output.") + + # Create a UTxO paying to one of our change addresses. + block_hash = self.nodes[2].getbestblockhash() + addr = self.nodes[2].getrawchangeaddress() + self.nodes[2].sendtoaddress(addr, 1) + + # If we don't list change, we won't have an entry for it. + coins = self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] + assert not any(c["address"] == addr for c in coins) + + # Now if we list change, we'll get both the send (to a change address) and + # the actual change. + res = self.nodes[2].listsinceblock(blockhash=block_hash, include_change=True) + coins = [entry for entry in res["transactions"] if entry["category"] == "receive"] + assert_equal(len(coins), 2) + assert any(c["address"] == addr for c in coins) + assert all(self.nodes[2].getaddressinfo(c["address"])["ischange"] for c in coins) + + if __name__ == '__main__': ListSinceBlockTest().main() |