From 98e57c729f817ba629ba0dc1db62e84dc38260b6 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 23 Nov 2023 17:35:18 +0000 Subject: [PATCH 1/4] Merge bitcoin/bitcoin#18919: test: Add gettransaction test for "coin-join" tx fa20f8919c4fe616c24325ed79a46f3b6cafcf58 test: Add gettransaction test for "coin-join" tx (MarcoFalke) Pull request description: ACKs for top commit: furszy: utACK fa20f8919c4f Tree-SHA512: 6e92455ef478d6bf2c544910402be9046698ded66f1f68d5fe5b989aafc20ba0537d362331e0e653595bca553166f6197fe548bdd0bcf295743775b8afb663a1 --- test/functional/wallet_listtransactions.py | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index ea66a1f2f084..334191cf3fdf 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -106,6 +106,7 @@ def run_test(self): {"txid": txid, "label": "watchonly"}) self.run_externally_generated_address_test() + self.run_coinjoin_test() self.run_invalid_parameters_test() self.test_op_return() @@ -163,6 +164,34 @@ def normalize_list(txs): assert_equal(['pizza2'], self.nodes[0].getaddressinfo(addr2)['labels']) assert_equal(['pizza3'], self.nodes[0].getaddressinfo(addr3)['labels']) + def run_coinjoin_test(self): + self.log.info('Check "coin-join" transaction') + input_0 = next(i for i in self.nodes[0].listunspent(query_options={"minimumAmount": 0.2}, include_unsafe=False)) + input_1 = next(i for i in self.nodes[1].listunspent(query_options={"minimumAmount": 0.2}, include_unsafe=False)) + raw_hex = self.nodes[0].createrawtransaction( + inputs=[ + { + "txid": input_0["txid"], + "vout": input_0["vout"], + }, + { + "txid": input_1["txid"], + "vout": input_1["vout"], + }, + ], + outputs={ + self.nodes[0].getnewaddress(): 0.123, + self.nodes[1].getnewaddress(): 0.123, + }, + ) + raw_hex = self.nodes[0].signrawtransactionwithwallet(raw_hex)["hex"] + raw_hex = self.nodes[1].signrawtransactionwithwallet(raw_hex)["hex"] + txid_join = self.nodes[0].sendrawtransaction(hexstring=raw_hex, maxfeerate=0) + fee_join = self.nodes[0].getmempoolentry(txid_join)["fees"]["base"] + # Fee should be correct: assert_equal(fee_join, self.nodes[0].gettransaction(txid_join)['fee']) + # But it is not, see for example https://github.com/bitcoin/bitcoin/issues/14136: + assert fee_join != self.nodes[0].gettransaction(txid_join)["fee"] + def run_invalid_parameters_test(self): self.log.info("Test listtransactions RPC parameter validity") assert_raises_rpc_error(-8, 'Label argument must be a valid label name or "*".', self.nodes[0].listtransactions, label="") From 8eeff954353eb023ace3d8909b65d732a6d630b1 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 6 Dec 2023 11:16:03 -0500 Subject: [PATCH 2/4] Merge bitcoin/bitcoin#27581: net: Continuous ASMap health check 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 net: Add continuous ASMap health check logging (Fabian Jahr) 28d7e55dff826a69f3f8e58139dbffb611cc5947 test: Add tests for unfiltered GetAddr usage (Fabian Jahr) b8843d37aed1276ff8527328c956c70c6e02ee13 fuzz: Let fuzzers use filter options in GetAddr/GetAddresses (Fabian Jahr) e16f420547fc72a5a2902927aa7138e43c0fb7c8 net: Optionally include terrible addresses in GetAddr results (Fabian Jahr) Pull request description: There are certain statistics we can collect by running all our known clearnet addresses against the ASMap file. This could show issues with a maliciously manipulated file or with an old file that has decayed with time. This is just a proof of concept for now. My idea currently is to run the analysis once per day and print the results to logs if an ASMap file is used. ACKs for top commit: achow101: ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 mzumsande: ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 brunoerg: crACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 Tree-SHA512: 777acbfac43cc43ce4a0a3612434e4ddbc65f59ae8ffc9e24f21de09011bccb297f0599cbaa82bcf40ef68e5af582c4e98556379db7ceff7d9f97574a1cf8e09 --- src/addrman.cpp | 12 ++++++------ src/addrman.h | 3 ++- src/addrman_impl.h | 4 ++-- src/net.cpp | 23 +++++++++++++++++++++-- src/net.h | 6 +++++- src/netgroup.cpp | 21 +++++++++++++++++++++ src/netgroup.h | 10 ++++++++++ src/test/addrman_tests.cpp | 18 ++++++++++++++++++ src/test/fuzz/addrman.cpp | 3 ++- src/test/fuzz/connman.cpp | 3 ++- test/functional/feature_asmap.py | 9 +++++++++ 11 files changed, 98 insertions(+), 14 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index e353d457264e..608d116a5e3a 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -808,7 +808,7 @@ int AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) const return -1; } -std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { AssertLockHeld(cs); @@ -838,7 +838,7 @@ std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct if (network != std::nullopt && ai.GetNetClass() != network) continue; // Filter for quality - if (ai.IsTerrible(now)) continue; + if (ai.IsTerrible(now) && filtered) continue; addresses.push_back(ai); } @@ -1209,11 +1209,11 @@ std::pair AddrManImpl::Select(bool new_only, std::optiona return addrRet; } -std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { LOCK(cs); Check(); - auto addresses = GetAddr_(max_addresses, max_pct, network); + auto addresses = GetAddr_(max_addresses, max_pct, network, filtered); Check(); return addresses; } @@ -1315,9 +1315,9 @@ std::pair AddrMan::Select(bool new_only, std::optionalSelect(new_only, network); } -std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { - return m_impl->GetAddr(max_addresses, max_pct, network); + return m_impl->GetAddr(max_addresses, max_pct, network, filtered); } void AddrMan::Connected(const CService& addr, NodeSeconds time) diff --git a/src/addrman.h b/src/addrman.h index c31fb321f82c..61693baf4fc8 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -177,10 +177,11 @@ class AddrMan * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). + * @param[in] filtered Select only addresses that are considered good quality (false = all). * * @return A vector of randomly selected addresses from vRandom. */ - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const; + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; /** We have successfully connected to this peer. Calling this function * updates the CAddress's nTime, which is used in our IsTerrible() diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 08c597e66765..890685260eeb 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -130,7 +130,7 @@ class AddrManImpl std::pair Select(bool new_only, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs); void Connected(const CService& addr, NodeSeconds time) @@ -262,7 +262,7 @@ class AddrManImpl * */ int GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); void Connected_(const CService& addr, NodeSeconds time) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/net.cpp b/src/net.cpp index 3b70ee45fe00..c3c644bad911 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4089,6 +4089,12 @@ bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_met // Dump network addresses scheduler.scheduleEvery([this] { DumpAddresses(); }, DUMP_PEERS_INTERVAL); + // Run the ASMap Health check once and then schedule it to run every 24h. + if (m_netgroupman.UsingASMap()) { + ASMapHealthCheck(); + scheduler.scheduleEvery([this] { ASMapHealthCheck(); }, ASMAP_HEALTH_CHECK_INTERVAL); + } + return true; } @@ -4224,9 +4230,9 @@ CConnman::~CConnman() Stop(); } -std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { - std::vector addresses = addrman.GetAddr(max_addresses, max_pct, network); + std::vector addresses = addrman.GetAddr(max_addresses, max_pct, network, filtered); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), @@ -4951,6 +4957,19 @@ void CConnman::PerformReconnections() } } +void CConnman::ASMapHealthCheck() +{ + const std::vector v4_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV4, /*filtered=*/ false)}; + const std::vector v6_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV6, /*filtered=*/ false)}; + std::vector clearnet_addrs; + clearnet_addrs.reserve(v4_addrs.size() + v6_addrs.size()); + std::transform(v4_addrs.begin(), v4_addrs.end(), std::back_inserter(clearnet_addrs), + [](const CAddress& addr) { return static_cast(addr); }); + std::transform(v6_addrs.begin(), v6_addrs.end(), std::back_inserter(clearnet_addrs), + [](const CAddress& addr) { return static_cast(addr); }); + m_netgroupman.ASMapHealthCheck(clearnet_addrs); +} + // Dump binary message to file, with timestamp. static void CaptureMessageToFile(const CAddress& addr, const std::string& msg_type, diff --git a/src/net.h b/src/net.h index 8bdda43e9e75..7c5e9c15b207 100644 --- a/src/net.h +++ b/src/net.h @@ -111,6 +111,8 @@ static const bool DEFAULT_BLOCKSONLY = false; static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60; /** Number of file descriptors required for message capture **/ static const int NUM_FDS_MESSAGE_CAPTURE = 1; +/** Interval for ASMap Health Check **/ +static constexpr std::chrono::hours ASMAP_HEALTH_CHECK_INTERVAL{24}; static constexpr bool DEFAULT_FORCEDNSSEED{false}; static constexpr bool DEFAULT_DNSSEED{true}; @@ -1295,6 +1297,7 @@ friend class CNode; void OpenMasternodeConnection(const CAddress& addrConnect, bool use_v2transport, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_unused_i2p_sessions_mutex, !mutexMsgProc, !cs_mapSocketToNode); bool CheckIncomingNonce(uint64_t nonce) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + void ASMapHealthCheck(); // alias for thread safety annotations only, not defined SharedMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); @@ -1405,8 +1408,9 @@ friend class CNode; * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). + * @param[in] filtered Select only addresses that are considered high quality (false = all). */ - std::vector GetAddresses(size_t max_addresses, size_t max_pct, std::optional network) const; + std::vector GetAddresses(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; /** * Cache is used to minimize topology leaks, so it should diff --git a/src/netgroup.cpp b/src/netgroup.cpp index 491017bc15b1..ff04a9a9f994 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -5,6 +5,7 @@ #include #include +#include #include uint256 NetGroupManager::GetAsmapChecksum() const @@ -109,3 +110,23 @@ uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const uint32_t mapped_as = Interpret(m_asmap, ip_bits); return mapped_as; } + +void NetGroupManager::ASMapHealthCheck(const std::vector& clearnet_addrs) const { + std::set clearnet_asns{}; + int unmapped_count{0}; + + for (const auto& addr : clearnet_addrs) { + uint32_t asn = GetMappedAS(addr); + if (asn == 0) { + ++unmapped_count; + continue; + } + clearnet_asns.insert(asn); + } + + LogPrintf("ASMap Health Check: %i clearnet peers are mapped to %i ASNs with %i peers being unmapped\n", clearnet_addrs.size(), clearnet_asns.size(), unmapped_count); +} + +bool NetGroupManager::UsingASMap() const { + return m_asmap.size() > 0; +} diff --git a/src/netgroup.h b/src/netgroup.h index 2dd63ec66b05..5aa6ef774253 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -41,6 +41,16 @@ class NetGroupManager { */ uint32_t GetMappedAS(const CNetAddr& address) const; + /** + * Analyze and log current health of ASMap based buckets. + */ + void ASMapHealthCheck(const std::vector& clearnet_addrs) const; + + /** + * Indicates whether ASMap is being used for clearnet bucketing. + */ + bool UsingASMap() const; + private: /** Compressed IP->ASN mapping, loaded from a file when a node starts. * diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 8022dd8c2643..2efb321266bb 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -428,6 +428,24 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) BOOST_CHECK_EQUAL(addrman->Size(), 2006U); } +BOOST_AUTO_TEST_CASE(getaddr_unfiltered) +{ + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); + + // Set time on this addr so isTerrible = false + CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); + addr1.nTime = Now(); + // Not setting time so this addr should be isTerrible = true + CAddress addr2 = CAddress(ResolveService("250.251.2.2", 9999), NODE_NONE); + + CNetAddr source = ResolveIP("250.1.2.1"); + BOOST_CHECK(addrman->Add({addr1, addr2}, source)); + + // Filtered GetAddr should only return addr1 + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt).size(), 1U); + // Unfiltered GetAddr should return addr1 and addr2 + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false).size(), 2U); +} BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) { diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 1a79febb8a70..4847ce01581e 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -283,7 +283,8 @@ FUZZ_TARGET(addrman, .init = initialize_addrman) (void)const_addr_man.GetAddr( /* max_addresses */ fuzzed_data_provider.ConsumeIntegralInRange(0, 4096), /* max_pct */ fuzzed_data_provider.ConsumeIntegralInRange(0, 4096), - /* network */ std::nullopt); + /* network */ std::nullopt, + /* filtered =*/ fuzzed_data_provider.ConsumeBool()); (void)const_addr_man.Select(fuzzed_data_provider.ConsumeBool()); (void)const_addr_man.Size(); CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 54c0edbbd6ea..2ba8773ab1ea 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -94,7 +94,8 @@ FUZZ_TARGET(connman, .init = initialize_connman) (void)connman.GetAddresses( /*max_addresses=*/fuzzed_data_provider.ConsumeIntegral(), /*max_pct=*/fuzzed_data_provider.ConsumeIntegral(), - /*network=*/std::nullopt); + /*network=*/std::nullopt, + /*filtered=*/fuzzed_data_provider.ConsumeBool()); }, [&] { (void)connman.GetAddresses( diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py index a357ee8cbbcd..dfcc0e70b357 100755 --- a/test/functional/feature_asmap.py +++ b/test/functional/feature_asmap.py @@ -111,6 +111,14 @@ def test_empty_asmap(self): self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg) os.remove(self.default_asmap) + def test_asmap_health_check(self): + self.log.info('Test bitcoind -asmap logs ASMap Health Check with basic stats') + shutil.copyfile(self.asmap_raw, self.default_asmap) + msg = "ASMap Health Check: 2 clearnet peers are mapped to 1 ASNs with 0 peers being unmapped" + with self.node.assert_debug_log(expected_msgs=[msg]): + self.start_node(0, extra_args=['-asmap']) + os.remove(self.default_asmap) + def run_test(self): self.node = self.nodes[0] self.datadir = os.path.join(self.node.datadir, self.chain) @@ -124,6 +132,7 @@ def run_test(self): self.test_asmap_interaction_with_addrman_containing_entries() self.test_default_asmap_with_missing_file() self.test_empty_asmap() + self.test_asmap_health_check() if __name__ == '__main__': From 8e95d2aa8aabca9138cf8b109c09fc0eb104aa2f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 7 Nov 2023 14:13:17 -0500 Subject: [PATCH 3/4] Merge bitcoin/bitcoin#28136: refactor: move GetServicesNames from rpc/util.{h,cpp} to rpc/net.cpp bbb68ffdbdafb6717dcadac074f6098750b8aa77 refactor: drop protocol.h include header in rpc/util.h (Jon Atack) 1dd62c5295ca319c94f7233bcb2e11f9d37a33f1 refactor: move GetServicesNames from rpc/util.{h,cpp} to rpc/net.cpp (Jon Atack) Pull request description: Move `GetServicesNames()` from `rpc/util` to `rpc/net.cpp`, as it is only called from that compilation unit and there is no reason for other ones to need it. Remove the `protocol.h` include in `rpc/util.h`, as it was only needed for `GetServicesNames()`, drop an unneeded forward declaration (the other IWYU suggestions would require more extensive changes in other files), and add 3 already-missing include headers in other translation units that are needed to compile without `protocol.h` in `rpc/util.h`, as `protocol.h` includes `netaddress.h`, which in turn includes `util/strencodings.h`. ACKs for top commit: kevkevinpal: lgtm ACK [bbb68ff](https://github.com/bitcoin/bitcoin/pull/28136/commits/bbb68ffdbdafb6717dcadac074f6098750b8aa77) ns-xvrn: ACK bbb68ff achow101: ACK bbb68ffdbdafb6717dcadac074f6098750b8aa77 Tree-SHA512: fcbe195874dd4aa9e86548685b6b28595a2c46f9869b79b6e2b3835f76b49cab4bef6a59c8ad6428063a41b7bb6f687229b06ea614fbd103e0531104af7de55d --- src/httprpc.cpp | 1 + src/rest.cpp | 1 + src/rpc/mempool.cpp | 1 + src/rpc/net.cpp | 13 +++++++++++++ src/rpc/util.cpp | 11 ----------- src/rpc/util.h | 4 ---- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 46cb1041c23e..f09ca024fb6b 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include diff --git a/src/rest.cpp b/src/rest.cpp index 207a2f247400..6ebe50b4411e 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 206c38d71507..c064d683e91e 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index a5e8cb5fdc34..2e75a19f3bcc 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -95,6 +96,18 @@ static RPCHelpMan ping() }; } +/** Returns, given services flags, a list of humanly readable (known) network services */ +static UniValue GetServicesNames(ServiceFlags services) +{ + UniValue servicesNames(UniValue::VARR); + + for (const auto& flag : serviceFlagsToStr(services)) { + servicesNames.push_back(flag); + } + + return servicesNames; +} + static RPCHelpMan getpeerinfo() { return RPCHelpMan{ diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 5e83622f93c1..fe560e52a1f5 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -990,17 +990,6 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s } } -UniValue GetServicesNames(ServiceFlags services) -{ - UniValue servicesNames(UniValue::VARR); - - for (const auto& flag : serviceFlagsToStr(services)) { - servicesNames.push_back(flag); - } - - return servicesNames; -} - std::vector EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider) { std::string desc_str; diff --git a/src/rpc/util.h b/src/rpc/util.h index 6a62ca00e3e2..47acd28f0936 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -6,7 +6,6 @@ #define BITCOIN_RPC_UTIL_H #include -#include #include #include #include @@ -44,7 +43,6 @@ extern const std::string EXAMPLE_ADDRESS[2]; class FillableSigningProvider; class FillableSigningProvider; -class CPubKey; class CScript; struct Sections; @@ -122,8 +120,6 @@ UniValue DescribeAddress(const CTxDestination& dest); //! Parse a confirm target option and raise an RPC error if it is invalid. unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target); -/** Returns, given services flags, a list of humanly readable (known) network services */ -UniValue GetServicesNames(ServiceFlags services); //! Parse a JSON range specified as int64, or [int64, int64] std::pair ParseDescriptorRange(const UniValue& value); From b39ec50f582e871048927dc3c37d1d92ace12396 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 23 Aug 2023 16:27:27 -0400 Subject: [PATCH 4/4] Merge bitcoin/bitcoin#27829: rpc: fix data optionality for RPC calls. 27b168b81f3959f5376a4176673187a71c7b25e1 Update help text for spend and rawtransaction rpcs (Michael Tidwell) Pull request description: The "data" field without outputs was marked as "required" in the help docs when using bitcoin-cli. This field when left off worked as an intended optional OP_RETURN. closes #27828. Motivation: It is hard to understand that "data" is actually optional for commands like `createpsbt` and `walletcreatefundedpsbt`. ACKs for top commit: achow101: ACK 27b168b81f3959f5376a4176673187a71c7b25e1 Sjors: tACK 27b168b81f3959f5376a4176673187a71c7b25e1 Tree-SHA512: 235e7ed4af69880880c04015b3f7de72c8f31ae035485c4c64c483e282948f3ea3f1eef16f15e260a1aaf21114150713516ba6a99967ccad9ecd91ff67cb0450 --- src/rpc/rawtransaction.cpp | 5 +++-- src/wallet/rpc/spend.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0df5620594f2..070495623111 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -199,8 +199,9 @@ static std::vector CreateTxDoc() }, }, }, - {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs (key-value pairs), where none of the keys are duplicated.\n" - "That is, each address can only appear once and there can only be one 'data' object.\n" + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs specified as key-value pairs.\n" + "Each key may only appear once, i.e. there can only be one 'data' output, and no address may be duplicated.\n" + "At least one output of either type must be specified.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" " accepted as second parameter.", { diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index fdff11c349db..a54b88d3d064 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -803,8 +803,9 @@ RPCHelpMan send() "\nEXPERIMENTAL warning: this call may be changed in future releases.\n" "\nSend a transaction.\n", { - {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs (key-value pairs), where none of the keys are duplicated.\n" - "That is, each address can only appear once and there can only be one 'data' object.\n" + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs specified as key-value pairs.\n" + "Each key may only appear once, i.e. there can only be one 'data' output, and no address may be duplicated.\n" + "At least one output of either type must be specified.\n" "For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.", { {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "",