From da201d48ea2eb355ecb8ccc1c137366ca4c9c630 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Tue, 29 Jul 2025 02:09:13 -0500 Subject: [PATCH] Merge bitcoin/bitcoin#28721: multiprocess compatibility updates --- doc/multiprocess.md | 8 +- src/Makefile.test.include | 1 + src/interfaces/chain.h | 43 ++++++++++ src/interfaces/node.h | 4 +- src/interfaces/wallet.h | 2 +- src/node/interfaces.cpp | 29 ++++++- src/qt/transactiondesc.cpp | 8 +- src/rpc/node.cpp | 9 +++ src/span.h | 21 ++++- src/streams.h | 5 ++ src/test/span_tests.cpp | 73 +++++++++++++++++ src/util/system.h | 9 +++ src/wallet/context.h | 2 + src/wallet/interfaces.cpp | 13 ++- src/wallet/load.cpp | 6 +- src/wallet/load.h | 2 +- src/wallet/spend.cpp | 160 +++++++++++++++++++++++++++++++++++++ 17 files changed, 375 insertions(+), 20 deletions(-) create mode 100644 src/test/span_tests.cpp diff --git a/doc/multiprocess.md b/doc/multiprocess.md index 3463130110dd..9866a31a5f97 100644 --- a/doc/multiprocess.md +++ b/doc/multiprocess.md @@ -19,7 +19,7 @@ The `-debug=ipc` command line option can be used to see requests and responses b ## Installation -The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) as dependencies. A simple way to get starting using it without installing these dependencies manually is to use the [depends system](../depends) with the `MULTIPROCESS=1` [dependency option](../depends#dependency-options) passed to make: +The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) as dependencies. A simple way to get starting using it without installing these dependencies manually is to use the [depends system](../../depends) with the `MULTIPROCESS=1` [dependency option](../../depends#dependency-options) passed to make: ``` cd @@ -32,12 +32,12 @@ DASHD=dash-node test/functional/test_runner.py The configure script will pick up settings and library locations from the depends directory, so there is no need to pass `--enable-multiprocess` as a separate flag when using the depends system (it's controlled by the `MULTIPROCESS=1` option). -Alternately, you can install [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) packages on your system, and just run `./configure --enable-multiprocess` without using the depends system. The configure script will be able to locate the installed packages via [pkg-config](https://www.freedesktop.org/wiki/Software/pkg-config/). See [Installation](https://github.com/chaincodelabs/libmultiprocess#installation) section of the libmultiprocess readme for install steps. See [build-unix.md](build-unix.md) and [build-osx.md](build-osx.md) for information about installing dependencies in general. +Alternately, you can install [Cap'n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) packages on your system, and just run `./configure --enable-multiprocess` without using the depends system. The configure script will be able to locate the installed packages via [pkg-config](https://www.freedesktop.org/wiki/Software/pkg-config/). See [Installation](https://github.com/chaincodelabs/libmultiprocess/blob/master/doc/install.md) section of the libmultiprocess readme for install steps. See [build-unix.md](../build-unix.md) and [build-osx.md](../build-osx.md) for information about installing dependencies in general. ## IPC implementation details Cross process Node, Wallet, and Chain interfaces are defined in -[`src/interfaces/`](../src/interfaces/). These are C++ classes which follow +[`src/interfaces/`](../../src/interfaces/). These are C++ classes which follow [conventions](../developer-notes.md#internal-interface-guidelines), like passing serializable arguments so they can be called from different processes, and making methods pure virtual so they can have proxy implementations that forward @@ -58,7 +58,7 @@ actual serialization and socket communication. As much as possible, calls between processes are meant to work the same as calls within a single process without adding limitations or requiring extra implementation effort. Processes communicate with each other by calling regular -[C++ interface methods](../src/interfaces/README.md). Method arguments and +[C++ interface methods](../../src/interfaces/README.md). Method arguments and return values are automatically serialized and sent between processes. Object references and `std::function` arguments are automatically tracked and mapped to allow invoked code to call back into invoking code at any time, and there is diff --git a/src/Makefile.test.include b/src/Makefile.test.include index bed7081dd6d4..56e56a09bcb0 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -174,6 +174,7 @@ BITCOIN_TESTS =\ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ test/sock_tests.cpp \ + test/span_tests.cpp \ test/streams_tests.cpp \ test/subsidy_tests.cpp \ test/sync_tests.cpp \ diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 5dc214fa54a7..c3fb81804115 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -206,6 +206,46 @@ class Chain //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) = 0; +<<<<<<< HEAD +======= + //! For each outpoint, calculate the fee-bumping cost to spend this outpoint at the specified + // feerate, including bumping its ancestors. For example, if the target feerate is 10sat/vbyte + // and this outpoint refers to a mempool transaction at 3sat/vbyte, the bump fee includes the + // cost to bump the mempool transaction to 10sat/vbyte (i.e. 7 * mempooltx.vsize). If that + // transaction also has, say, an unconfirmed parent with a feerate of 1sat/vbyte, the bump fee + // includes the cost to bump the parent (i.e. 9 * parentmempooltx.vsize). + // + // If the outpoint comes from an unconfirmed transaction that is already above the target + // feerate or bumped by its descendant(s) already, it does not need to be bumped. Its bump fee + // is 0. Likewise, if any of the transaction's ancestors are already bumped by a transaction + // in our mempool, they are not included in the transaction's bump fee. + // + // Also supported is bump-fee calculation in the case of replacements. If an outpoint + // conflicts with another transaction in the mempool, it is assumed that the goal is to replace + // that transaction. As such, the calculation will exclude the to-be-replaced transaction, but + // will include the fee-bumping cost. If bump fees of descendants of the to-be-replaced + // transaction are requested, the value will be 0. Fee-related RBF rules are not included as + // they are logically distinct. + // + // Any outpoints that are otherwise unavailable from the mempool (e.g. UTXOs from confirmed + // transactions or transactions not yet broadcast by the wallet) are given a bump fee of 0. + // + // If multiple outpoints come from the same transaction (which would be very rare because + // it means that one transaction has multiple change outputs or paid the same wallet using multiple + // outputs in the same transaction) or have shared ancestry, the bump fees are calculated + // independently, i.e. as if only one of them is spent. This may result in double-fee-bumping. This + // caveat can be rectified per use of the sister-function CalculateCombinedBumpFee(…). + virtual std::map calculateIndividualBumpFees(const std::vector& outpoints, const CFeeRate& target_feerate) = 0; + + //! Calculate the combined bump fee for an input set per the same strategy + // as in CalculateIndividualBumpFees(…). + // Unlike CalculateIndividualBumpFees(…), this does not return individual + // bump fees per outpoint, but a single bump fee for the shared ancestry. + // The combined bump fee may be used to correct overestimation due to + // shared ancestry by multiple UTXOs after coin selection. + virtual std::optional calculateCombinedBumpFee(const std::vector& outpoints, const CFeeRate& target_feerate) = 0; + +>>>>>>> 29c2c90362 (Merge bitcoin/bitcoin#28721: multiprocess compatibility updates) //! Get the node's package limits. //! Currently only returns the ancestor and descendant count limits, but could be enhanced to //! return more policy settings. @@ -342,6 +382,9 @@ class ChainClient //! Set mock time. virtual void setMockTime(int64_t time) = 0; + + //! Mock the scheduler to fast forward in time. + virtual void schedulerMockForward(std::chrono::seconds delta_seconds) = 0; }; //! Return implementation of Chain interface. diff --git a/src/interfaces/node.h b/src/interfaces/node.h index f3d4fc8e046d..5926e7247219 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -264,8 +264,8 @@ class Node //! Unset RPC timer interface. virtual void rpcUnsetTimerInterface(RPCTimerInterface* iface) = 0; - //! Get unspent outputs associated with a transaction. - virtual bool getUnspentOutput(const COutPoint& output, Coin& coin) = 0; + //! Get unspent output associated with a transaction. + virtual std::optional getUnspentOutput(const COutPoint& output) = 0; //! Broadcast transaction. virtual TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, bilingual_str& err_string) = 0; diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 85e054b3cde2..064cdd1fbab2 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -143,7 +143,7 @@ class Wallet std::string* purpose) = 0; //! Get wallet address list. - virtual std::vector getAddresses() const = 0; + virtual std::vector getAddresses() = 0; //! Get receive requests. virtual std::vector getAddressReceiveRequests() = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 493c8606c86c..273e61dc985f 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -504,10 +504,12 @@ class NodeImpl : public Node std::vector listRpcCommands() override { return ::tableRPC.listCommands(); } void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); } void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); } - bool getUnspentOutput(const COutPoint& output, Coin& coin) override + std::optional getUnspentOutput(const COutPoint& output) override { LOCK(::cs_main); - return chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin); + Coin coin; + if (chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin)) return coin; + return {}; } TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, bilingual_str& err_string) override { @@ -916,6 +918,29 @@ class ChainImpl : public Chain if (!m_node.mempool) return; m_node.mempool->GetTransactionAncestry(txid, ancestors, descendants, ancestorsize, ancestorfees); } +<<<<<<< HEAD +======= + + std::map calculateIndividualBumpFees(const std::vector& outpoints, const CFeeRate& target_feerate) override + { + if (!m_node.mempool) { + std::map bump_fees; + for (const auto& outpoint : outpoints) { + bump_fees.emplace(outpoint, 0); + } + return bump_fees; + } + return MiniMiner(*m_node.mempool, outpoints).CalculateBumpFees(target_feerate); + } + + std::optional calculateCombinedBumpFee(const std::vector& outpoints, const CFeeRate& target_feerate) override + { + if (!m_node.mempool) { + return 0; + } + return MiniMiner(*m_node.mempool, outpoints).CalculateTotalBumpFees(target_feerate); + } +>>>>>>> 29c2c90362 (Merge bitcoin/bitcoin#28721: multiprocess compatibility updates) void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) override { limit_ancestor_count = gArgs.GetIntArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index bb6ac6992289..6b02946a815e 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -303,12 +303,14 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall { COutPoint prevout = txin.prevout; - Coin prev; - if(node.getUnspentOutput(prevout, prev)) - { + if (auto prev{node.getUnspentOutput(prevout)}) { { strHTML += "
  • "; +<<<<<<< HEAD const CTxOut& txout = prev.out; +======= + const CTxOut& vout = prev->out; +>>>>>>> 29c2c90362 (Merge bitcoin/bitcoin#28721: multiprocess compatibility updates) CTxDestination address; if (ExtractDestination(txout.scriptPubKey, address)) { diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index 0e9d6dd023ee..16885bed6c07 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -840,10 +840,19 @@ static RPCHelpMan mockscheduler() throw std::runtime_error("delta_time must be between 1 and 3600 seconds (1 hr)"); } +<<<<<<< HEAD auto* node_context = CHECK_NONFATAL(GetContext(request.context)); // protect against null pointer dereference CHECK_NONFATAL(node_context->scheduler); node_context->scheduler->MockForward(std::chrono::seconds(delta_seconds)); +======= + const NodeContext& node_context{EnsureAnyNodeContext(request.context)}; + CHECK_NONFATAL(node_context.scheduler)->MockForward(std::chrono::seconds{delta_seconds}); + SyncWithValidationInterfaceQueue(); + for (const auto& chain_client : node_context.chain_clients) { + chain_client->schedulerMockForward(std::chrono::seconds(delta_seconds)); + } +>>>>>>> 29c2c90362 (Merge bitcoin/bitcoin#28721: multiprocess compatibility updates) return NullUniValue; }, diff --git a/src/span.h b/src/span.h index 6e6b58dbaa0b..1f170498c5a4 100644 --- a/src/span.h +++ b/src/span.h @@ -222,15 +222,32 @@ class Span template friend class Span; }; +// Return result of calling .data() method on type T. This is used to be able to +// write template deduction guides for the single-parameter Span constructor +// below that will work if the value that is passed has a .data() method, and if +// the data method does not return a void pointer. +// +// It is important to check for the void type specifically below, so the +// deduction guides can be used in SFINAE contexts to check whether objects can +// be converted to spans. If the deduction guides did not explicitly check for +// void, and an object was passed that returned void* from data (like +// std::vector), the template deduction would succeed, but the Span +// object instantiation would fail, resulting in a hard error, rather than a +// SFINAE error. +// https://stackoverflow.com/questions/68759148/sfinae-to-detect-the-explicitness-of-a-ctad-deduction-guide +// https://stackoverflow.com/questions/16568986/what-happens-when-you-call-data-on-a-stdvectorbool +template +using DataResult = std::remove_pointer_t().data())>; + // Deduction guides for Span // For the pointer/size based and iterator based constructor: template Span(T*, EndOrSize) -> Span; // For the array constructor: template Span(T (&)[N]) -> Span; // For the temporaries/rvalue references constructor, only supporting const output. -template Span(T&&) -> Span, const std::remove_pointer_t().data())>>>; +template Span(T&&) -> Span && !std::is_void_v>, const DataResult>>; // For (lvalue) references, supporting mutable output. -template Span(T&) -> Span().data())>>; +template Span(T&) -> Span>, DataResult>>; /** Pop the last element off a span, and return a reference to that element. */ template diff --git a/src/streams.h b/src/streams.h index 22972ffb25e8..a977769942b1 100644 --- a/src/streams.h +++ b/src/streams.h @@ -179,6 +179,11 @@ class SpanReader memcpy(dst.data(), m_data.data(), dst.size()); m_data = m_data.subspan(dst.size()); } + + void ignore(size_t n) + { + m_data = m_data.subspan(n); + } }; /** Double ended buffer combining vector and stream-like interfaces. diff --git a/src/test/span_tests.cpp b/src/test/span_tests.cpp new file mode 100644 index 000000000000..f6cac10b09bc --- /dev/null +++ b/src/test/span_tests.cpp @@ -0,0 +1,73 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include + +namespace { +struct Ignore +{ + template Ignore(T&&) {} +}; +template +bool Spannable(T&& value, decltype(Span{value})* enable = nullptr) +{ + return true; +} +bool Spannable(Ignore) +{ + return false; +} + +#if defined(__clang__) +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wunneeded-member-function" +# pragma clang diagnostic ignored "-Wunused-member-function" +#endif +struct SpannableYes +{ + int* data(); + size_t size(); +}; +struct SpannableNo +{ + void* data(); + size_t size(); +}; +#if defined(__clang__) +# pragma clang diagnostic pop +#endif +} // namespace + +BOOST_AUTO_TEST_SUITE(span_tests) + +// Make sure template Span template deduction guides accurately enable calls to +// Span constructor overloads that work, and disable calls to constructor overloads that +// don't work. This makes it is possible to use the Span constructor in a SFINAE +// contexts like in the Spannable function above to detect whether types are or +// aren't compatible with Spans at compile time. +// +// Previously there was a bug where writing a SFINAE check for vector was +// not possible, because in libstdc++ vector has a data() memeber +// returning void*, and the Span template guide ignored the data() return value, +// so the template substitution would succeed, but the constructor would fail, +// resulting in a fatal compile error, rather than a SFINAE error that could be +// handled. +BOOST_AUTO_TEST_CASE(span_constructor_sfinae) +{ + BOOST_CHECK(Spannable(std::vector{})); + BOOST_CHECK(!Spannable(std::set{})); + BOOST_CHECK(!Spannable(std::vector{})); + BOOST_CHECK(Spannable(std::array{})); + BOOST_CHECK(Spannable(Span{})); + BOOST_CHECK(Spannable("char array")); + BOOST_CHECK(Spannable(SpannableYes{})); + BOOST_CHECK(!Spannable(SpannableNo{})); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/system.h b/src/util/system.h index 7bf10098315c..f3aa7fd560a6 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -257,6 +257,15 @@ class ArgsManager void SelectConfigNetwork(const std::string& network); [[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error); +<<<<<<< HEAD:src/util/system.h +======= + + /** + * Return config file path (read-only) + */ + fs::path GetConfigFilePath() const; + void SetConfigFilePath(fs::path); +>>>>>>> 29c2c90362 (Merge bitcoin/bitcoin#28721: multiprocess compatibility updates):src/common/args.h [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); /** diff --git a/src/wallet/context.h b/src/wallet/context.h index 269b24e82cda..d9d104245a86 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -13,6 +13,7 @@ #include class ArgsManager; +class CScheduler; namespace interfaces { class Chain; namespace CoinJoin { @@ -41,6 +42,7 @@ using LoadWalletFn = std::function wall //! behavior. struct WalletContext { interfaces::Chain* chain{nullptr}; + CScheduler* scheduler{nullptr}; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct // It is unsafe to lock this after locking a CWallet::cs_wallet mutex because // this could introduce inconsistent lock ordering and cause deadlocks. diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index e1a3970cd738..092b315dbc2e 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -12,7 +12,11 @@ #include #include #include +<<<<<<< HEAD #include