diff --git a/doc/design/multiprocess.md b/doc/design/multiprocess.md index e6a77dbbc0d..45681d12dea 100644 --- a/doc/design/multiprocess.md +++ b/doc/design/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 @@ BITCOIND=bitcoin-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 58c1da697dc..e9d3bff1dec 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -146,6 +146,7 @@ BITCOIN_TESTS =\ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ test/sock_tests.cpp \ + test/span_tests.cpp \ test/streams_tests.cpp \ test/sync_tests.cpp \ test/system_tests.cpp \ diff --git a/src/common/args.cpp b/src/common/args.cpp index ca04175696a..cfaa4de72d3 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -720,6 +720,13 @@ fs::path ArgsManager::GetConfigFilePath() const return *Assert(m_config_path); } +void ArgsManager::SetConfigFilePath(fs::path path) +{ + LOCK(cs_args); + assert(!m_config_path); + m_config_path = path; +} + ChainType ArgsManager::GetChainType() const { std::variant arg = GetChainArg(); diff --git a/src/common/args.h b/src/common/args.h index ae3ed02bc7d..1c5db718f4d 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -180,6 +180,7 @@ class ArgsManager * Return config file path (read-only) */ fs::path GetConfigFilePath() const; + void SetConfigFilePath(fs::path); [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); /** diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index dea868f844d..b6ea91aaa7c 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -244,7 +244,7 @@ class Chain // 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; + 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(…). @@ -252,7 +252,7 @@ class Chain // 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; + virtual std::optional calculateCombinedBumpFee(const std::vector& outpoints, const CFeeRate& target_feerate) = 0; //! Get the node's package limits. //! Currently only returns the ancestor and descendant count limits, but could be enhanced to @@ -395,6 +395,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 3f8df571249..aeb2612c078 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -204,8 +204,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, std::string& err_string) = 0; diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 4b896c11a37..61142366234 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -118,7 +118,7 @@ class Wallet wallet::AddressPurpose* 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 4c83df7eca3..5ec7e17b263 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -328,10 +328,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, std::string& err_string) override { @@ -670,7 +672,7 @@ class ChainImpl : public Chain m_node.mempool->GetTransactionAncestry(txid, ancestors, descendants, ancestorsize, ancestorfees); } - std::map CalculateIndividualBumpFees(const std::vector& outpoints, const CFeeRate& target_feerate) override + std::map calculateIndividualBumpFees(const std::vector& outpoints, const CFeeRate& target_feerate) override { if (!m_node.mempool) { std::map bump_fees; @@ -682,7 +684,7 @@ class ChainImpl : public Chain return MiniMiner(*m_node.mempool, outpoints).CalculateBumpFees(target_feerate); } - std::optional CalculateCombinedBumpFee(const std::vector& outpoints, const CFeeRate& target_feerate) override + std::optional calculateCombinedBumpFee(const std::vector& outpoints, const CFeeRate& target_feerate) override { if (!m_node.mempool) { return 0; diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 51f6f449231..a916e4ead6f 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -360,12 +360,10 @@ 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 += "
  • "; - const CTxOut &vout = prev.out; + const CTxOut& vout = prev->out; CTxDestination address; if (ExtractDestination(vout.scriptPubKey, address)) { diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index 6b3662996c4..b085828215d 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -91,6 +91,9 @@ static RPCHelpMan mockscheduler() 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)); + } return UniValue::VNULL; }, diff --git a/src/span.h b/src/span.h index 7209d21a585..2e8da27cde2 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 d58de5233bc..ae1434cc1c6 100644 --- a/src/streams.h +++ b/src/streams.h @@ -182,6 +182,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 00000000000..f6cac10b09b --- /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/wallet/context.h b/src/wallet/context.h index 57a6ed77f70..58b9924fb4c 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -13,6 +13,7 @@ #include class ArgsManager; +class CScheduler; namespace interfaces { class Chain; class Wallet; @@ -34,6 +35,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/feebumper.cpp b/src/wallet/feebumper.cpp index f4cb4bbd66c..f0fd789c963 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -86,7 +86,7 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CMutableTrans reused_inputs.push_back(txin.prevout); } - std::optional combined_bump_fee = wallet.chain().CalculateCombinedBumpFee(reused_inputs, newFeerate); + std::optional combined_bump_fee = wallet.chain().calculateCombinedBumpFee(reused_inputs, newFeerate); if (!combined_bump_fee.has_value()) { errors.push_back(strprintf(Untranslated("Failed to calculate bump fees, because unconfirmed UTXOs depend on enormous cluster of unconfirmed transactions."))); } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index c9419be0ab5..4ca5e292294 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -212,7 +213,7 @@ class WalletImpl : public Wallet } return true; } - std::vector getAddresses() const override + std::vector getAddresses() override { LOCK(m_wallet->cs_wallet); std::vector result; @@ -585,10 +586,15 @@ class WalletLoaderImpl : public WalletLoader } bool verify() override { return VerifyWallets(m_context); } bool load() override { return LoadWallets(m_context); } - void start(CScheduler& scheduler) override { return StartWallets(m_context, scheduler); } + void start(CScheduler& scheduler) override + { + m_context.scheduler = &scheduler; + return StartWallets(m_context); + } void flush() override { return FlushWallets(m_context); } void stop() override { return StopWallets(m_context); } void setMockTime(int64_t time) override { return SetMockTime(time); } + void schedulerMockForward(std::chrono::seconds delta) override { Assert(m_context.scheduler)->MockForward(delta); } //! WalletLoader methods util::Result> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector& warnings) override diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 4cdfadbee27..8b78a670e49 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -141,7 +141,7 @@ bool LoadWallets(WalletContext& context) } } -void StartWallets(WalletContext& context, CScheduler& scheduler) +void StartWallets(WalletContext& context) { for (const std::shared_ptr& pwallet : GetWallets(context)) { pwallet->postInitProcess(); @@ -149,9 +149,9 @@ void StartWallets(WalletContext& context, CScheduler& scheduler) // Schedule periodic wallet flushes and tx rebroadcasts if (context.args->GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { - scheduler.scheduleEvery([&context] { MaybeCompactWalletDB(context); }, std::chrono::milliseconds{500}); + context.scheduler->scheduleEvery([&context] { MaybeCompactWalletDB(context); }, 500ms); } - scheduler.scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min); + context.scheduler->scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min); } void FlushWallets(WalletContext& context) diff --git a/src/wallet/load.h b/src/wallet/load.h index 0882f7f8ad7..c079cad955b 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -26,7 +26,7 @@ bool VerifyWallets(WalletContext& context); bool LoadWallets(WalletContext& context); //! Complete startup of wallets. -void StartWallets(WalletContext& context, CScheduler& scheduler); +void StartWallets(WalletContext& context); //! Flush all wallets in preparation for shutdown. void FlushWallets(WalletContext& context); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 8314a2ddfab..ed0134375c4 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -259,7 +259,7 @@ util::Result FetchSelectedInputs(const CWallet& wallet, const { PreSelectedInputs result; const bool can_grind_r = wallet.CanGrindR(); - std::map map_of_bump_fees = wallet.chain().CalculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate); + std::map map_of_bump_fees = wallet.chain().calculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate); for (const COutPoint& outpoint : coin_control.ListSelected()) { int input_bytes = -1; CTxOut txout; @@ -453,7 +453,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, } if (feerate.has_value()) { - std::map map_of_bump_fees = wallet.chain().CalculateIndividualBumpFees(outpoints, feerate.value()); + std::map map_of_bump_fees = wallet.chain().calculateIndividualBumpFees(outpoints, feerate.value()); for (auto& [_, outputs] : result.coins) { for (auto& output : outputs) { @@ -725,7 +725,7 @@ util::Result ChooseSelectionResult(interfaces::Chain& chain, co outpoints.push_back(coin->outpoint); summed_bump_fees += coin->ancestor_bump_fees; } - std::optional combined_bump_fee = chain.CalculateCombinedBumpFee(outpoints, coin_selection_params.m_effective_feerate); + std::optional combined_bump_fee = chain.calculateCombinedBumpFee(outpoints, coin_selection_params.m_effective_feerate); if (!combined_bump_fee.has_value()) { return util::Error{_("Failed to calculate bump fees, because unconfirmed UTXOs depend on enormous cluster of unconfirmed transactions.")}; }