From 070e6a32d5ff4be2f4f1f6a8200fba2f61df16e3 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 24 Jul 2024 17:09:38 -0400 Subject: [PATCH 1/8] depends: Update libmultiprocess library for cmake headers target This update brings in the following changes: https://github.com/chaincodelabs/libmultiprocess/pull/107 example: Remove manual client adding https://github.com/chaincodelabs/libmultiprocess/pull/108 doc: Add comments for socket descriptor handling when forking https://github.com/chaincodelabs/libmultiprocess/pull/109 example: Add missing thread.join() call so example can exit cleanly https://github.com/chaincodelabs/libmultiprocess/pull/110 cmake: add target_capnp_sources headers target --- depends/packages/native_libmultiprocess.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/native_libmultiprocess.mk b/depends/packages/native_libmultiprocess.mk index 185ef9489e7..3fa5faa4ba1 100644 --- a/depends/packages/native_libmultiprocess.mk +++ b/depends/packages/native_libmultiprocess.mk @@ -1,8 +1,8 @@ package=native_libmultiprocess -$(package)_version=c1b4ab4eb897d3af09bc9b3cc30e2e6fff87f3e2 +$(package)_version=015e95f7ebaa47619a213a19801e7fffafc56864 $(package)_download_path=https://github.com/chaincodelabs/libmultiprocess/archive $(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=6edf5ad239ca9963c78f7878486fb41411efc9927c6073928a7d6edf947cac4a +$(package)_sha256_hash=4b1266b121337f3f6f37e1863fba91c1a5ee9ad126bcffc6fe6b9ca47ad050a1 $(package)_dependencies=native_capnp define $(package)_config_cmds From 206c6e78eec7c71d317666a1af07acf357ba001e Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 6 Sep 2024 18:23:54 -0400 Subject: [PATCH 2/8] build: Make bitcoin_ipc_test depend on bitcoin_ipc This change is needed to allow generated capnp code in src/ipc/capnp/ to be used in unit tests for better test coverage in upcoming commits. --- src/CMakeLists.txt | 16 ++++++++++++++++ src/test/CMakeLists.txt | 8 -------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e652d43d58d..36d76fbb27d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -325,6 +325,22 @@ if(WITH_MULTIPROCESS) $ ) list(APPEND installable_targets bitcoin-node) + + if(BUILD_TESTS) + # bitcoin_ipc_test library target is defined here in src/CMakeLists.txt + # instead of src/test/CMakeLists.txt so capnp files in src/test/ are able to + # reference capnp files in src/ipc/capnp/ by relative path. The Cap'n Proto + # compiler only allows importing by relative path when the importing and + # imported files are underneath the same compilation source prefix, so the + # source prefix must be src/, not src/test/ + add_library(bitcoin_ipc_test STATIC EXCLUDE_FROM_ALL + test/ipc_test.cpp + ) + target_capnp_sources(bitcoin_ipc_test ${PROJECT_SOURCE_DIR} + test/ipc_test.capnp + ) + add_dependencies(bitcoin_ipc_test bitcoin_ipc_headers) + endif() endif() diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 2c9957117c0..c23fbae92fe 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -160,14 +160,6 @@ if(ENABLE_WALLET) endif() if(WITH_MULTIPROCESS) - add_library(bitcoin_ipc_test STATIC EXCLUDE_FROM_ALL - ipc_test.cpp - ) - - target_capnp_sources(bitcoin_ipc_test ${PROJECT_SOURCE_DIR} - ipc_test.capnp - ) - target_link_libraries(bitcoin_ipc_test PRIVATE core_interface From 69dfeb18761cfd933b8a9a6e69ce9d3c516ba949 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 12 Sep 2024 17:12:52 -0400 Subject: [PATCH 3/8] multiprocess: update common-types.h to use C++20 concepts Idea came from review comment by ion- https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757372497 --- src/ipc/capnp/common-types.h | 55 +++++++++--------------------------- 1 file changed, 14 insertions(+), 41 deletions(-) diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h index 39e368491b4..3e238648cfc 100644 --- a/src/ipc/capnp/common-types.h +++ b/src/ipc/capnp/common-types.h @@ -6,6 +6,7 @@ #define BITCOIN_IPC_CAPNP_COMMON_TYPES_H #include +#include #include #include @@ -16,33 +17,6 @@ namespace ipc { namespace capnp { -//! Use SFINAE to define Serializeable trait which is true if type T has a -//! Serialize(stream) method, false otherwise. -template -struct Serializable { -private: - template - static std::true_type test(decltype(std::declval().Serialize(std::declval()))*); - template - static std::false_type test(...); - -public: - static constexpr bool value = decltype(test(nullptr))::value; -}; - -//! Use SFINAE to define Unserializeable trait which is true if type T has -//! an Unserialize(stream) method, false otherwise. -template -struct Unserializable { -private: - template - static std::true_type test(decltype(std::declval().Unserialize(std::declval()))*); - template - static std::false_type test(...); - -public: - static constexpr bool value = decltype(test(nullptr))::value; -}; } // namespace capnp } // namespace ipc @@ -50,18 +24,16 @@ public: namespace mp { //! Overload multiprocess library's CustomBuildField hook to allow any //! serializable object to be stored in a capnproto Data field or passed to a -//! canproto interface. Use Priority<1> so this hook has medium priority, and +//! capnproto interface. Use Priority<1> so this hook has medium priority, and //! higher priority hooks could take precedence over this one. template -void CustomBuildField( - TypeList, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output, - // Enable if serializeable and if LocalType is not cv or reference - // qualified. If LocalType is cv or reference qualified, it is important to - // fall back to lower-priority Priority<0> implementation of this function - // that strips cv references, to prevent this CustomBuildField overload from - // taking precedence over more narrow overloads for specific LocalTypes. - std::enable_if_t::value && - std::is_same_v>>>* enable = nullptr) +void CustomBuildField(TypeList, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output) +// Enable if serializeable and if LocalType is not cv or reference qualified. If +// LocalType is cv or reference qualified, it is important to fall back to +// lower-priority Priority<0> implementation of this function that strips cv +// references, to prevent this CustomBuildField overload from taking precedence +// over more narrow overloads for specific LocalTypes. +requires Serializable && std::is_same_v>> { DataStream stream; value.Serialize(stream); @@ -71,12 +43,11 @@ void CustomBuildField( //! Overload multiprocess library's CustomReadField hook to allow any object //! with an Unserialize method to be read from a capnproto Data field or -//! returned from canproto interface. Use Priority<1> so this hook has medium +//! returned from capnproto interface. Use Priority<1> so this hook has medium //! priority, and higher priority hooks could take precedence over this one. template -decltype(auto) -CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest, - std::enable_if_t::value>* enable = nullptr) +decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest) +requires Unserializable { return read_dest.update([&](auto& value) { if (!input.has()) return; @@ -86,6 +57,8 @@ CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, }); } +//! Overload CustomBuildField and CustomReadField to serialize UniValue +//! parameters and return values as JSON strings. template void CustomBuildField(TypeList, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output) { From 095286f790acda4a32f04c77aa86106007e2a0d8 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 5 Dec 2017 15:57:12 -0500 Subject: [PATCH 4/8] multiprocess: Add serialization code for CTransaction Add support for passing CTransaction and CTransactionRef types to IPC functions. These types can't be passed currently because IPC serialization code currently only supports deserializing types that have an Unserialize() method, which CTransaction does not, because it is supposed to represent immutable transactions. Work around this by adding a CustomReadField overload that will call CTransaction's deserialize_type constructor. These types also can't be passed currently because serializing transactions requires TransactionSerParams to be set. Fix this by setting TX_WITH_WITNESS as default serialization parameters for IPC code. --- src/ipc/capnp/common-types.h | 42 +++++++++++++++++++++++++++++++++--- src/test/ipc_test.capnp | 1 + src/test/ipc_test.cpp | 9 ++++++++ src/test/ipc_test.h | 1 + 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h index 3e238648cfc..302da76c055 100644 --- a/src/ipc/capnp/common-types.h +++ b/src/ipc/capnp/common-types.h @@ -6,6 +6,7 @@ #define BITCOIN_IPC_CAPNP_COMMON_TYPES_H #include +#include #include #include #include @@ -17,6 +18,24 @@ namespace ipc { namespace capnp { +//! Construct a ParamStream wrapping a data stream with serialization parameters +//! needed to pass transaction objects between bitcoin processes. +//! In the future, more params may be added here to serialize other objects that +//! require serialization parameters. Params should just be chosen to serialize +//! objects completely and ensure that serializing and deserializing objects +//! with the specified parameters produces equivalent objects. It's also +//! harmless to specify serialization parameters here that are not used. +template +auto Wrap(S& s) +{ + return ParamsStream{s, TX_WITH_WITNESS}; +} + +//! Detect if type has a deserialize_type constructor, which is +//! used to deserialize types like CTransaction that can't be unserialized into +//! existing objects because they are immutable. +template +concept Deserializable = std::is_constructible_v; } // namespace capnp } // namespace ipc @@ -36,7 +55,8 @@ void CustomBuildField(TypeList, Priority<1>, InvokeContext& invoke_co requires Serializable && std::is_same_v>> { DataStream stream; - value.Serialize(stream); + auto wrapper{ipc::capnp::Wrap(stream)}; + value.Serialize(wrapper); auto result = output.init(stream.size()); memcpy(result.begin(), stream.data(), stream.size()); } @@ -47,16 +67,32 @@ requires Serializable && std::is_same_v decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest) -requires Unserializable +requires Unserializable && (!ipc::capnp::Deserializable) { return read_dest.update([&](auto& value) { if (!input.has()) return; auto data = input.get(); SpanReader stream({data.begin(), data.end()}); - value.Unserialize(stream); + auto wrapper{ipc::capnp::Wrap(stream)}; + value.Unserialize(wrapper); }); } +//! Overload multiprocess library's CustomReadField hook to allow any object +//! with a deserialize constructor to be read from a capnproto Data field or +//! returned from capnproto interface. Use Priority<1> so this hook has medium +//! priority, and higher priority hooks could take precedence over this one. +template +decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest) +requires ipc::capnp::Deserializable +{ + assert(input.has()); + auto data = input.get(); + SpanReader stream({data.begin(), data.end()}); + auto wrapper{ipc::capnp::Wrap(stream)}; + return read_dest.construct(::deserialize, wrapper); +} + //! Overload CustomBuildField and CustomReadField to serialize UniValue //! parameters and return values as JSON strings. template diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp index 55a3dc26839..00cfeb53215 100644 --- a/src/test/ipc_test.capnp +++ b/src/test/ipc_test.capnp @@ -15,4 +15,5 @@ interface FooInterface $Proxy.wrap("FooImplementation") { add @0 (a :Int32, b :Int32) -> (result :Int32); passOutPoint @1 (arg :Data) -> (result :Data); passUniValue @2 (arg :Text) -> (result :Text); + passTransaction @3 (arg :Data) -> (result :Data); } diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp index e6de6e3e477..59083beabc6 100644 --- a/src/test/ipc_test.cpp +++ b/src/test/ipc_test.cpp @@ -88,6 +88,15 @@ void IpcPipeTest() UniValue uni2{foo->passUniValue(uni1)}; BOOST_CHECK_EQUAL(uni1.write(), uni2.write()); + CMutableTransaction mtx; + mtx.version = 2; + mtx.nLockTime = 3; + mtx.vin.emplace_back(txout1); + mtx.vout.emplace_back(COIN, CScript()); + CTransactionRef tx1{MakeTransactionRef(mtx)}; + CTransactionRef tx2{foo->passTransaction(tx1)}; + BOOST_CHECK(*Assert(tx1) == *Assert(tx2)); + // Test cleanup: disconnect pipe and join thread disconnect_client(); thread.join(); diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h index 2453bfa23c7..22fe96b6bac 100644 --- a/src/test/ipc_test.h +++ b/src/test/ipc_test.h @@ -15,6 +15,7 @@ public: int add(int a, int b) { return a + b; } COutPoint passOutPoint(COutPoint o) { return o; } UniValue passUniValue(UniValue v) { return v; } + CTransactionRef passTransaction(CTransactionRef t) { return t; } }; void IpcPipeTest(); From 06882f84017f6b569b46a644f39b6d3c120ec6cf Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 5 Dec 2017 15:57:12 -0500 Subject: [PATCH 5/8] multiprocess: Add serialization code for vector --- src/ipc/capnp/common-types.h | 27 +++++++++++++++++++++++++++ src/test/ipc_test.capnp | 1 + src/test/ipc_test.cpp | 4 ++++ src/test/ipc_test.h | 1 + 4 files changed, 33 insertions(+) diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h index 302da76c055..f1e2d18f1ce 100644 --- a/src/ipc/capnp/common-types.h +++ b/src/ipc/capnp/common-types.h @@ -112,6 +112,33 @@ decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& i value.read(std::string_view{data.begin(), data.size()}); }); } + +//! Generic ::capnp::Data field builder for any C++ type that can be converted +//! to a span of bytes, like std::vector or std::array, or custom +//! blob types like uint256 or PKHash with data() and size() methods pointing to +//! bytes. +//! +//! Note: it might make sense to move this function into libmultiprocess, since +//! it is fairly generic. However this would require decreasing its priority so +//! it can be overridden, which would require more changes inside +//! libmultiprocess to avoid conflicting with the Priority<1> CustomBuildField +//! function it already provides for std::vector. Also, it might make sense to +//! provide a CustomReadField counterpart to this function, which could be +//! called to read C++ types that can be constructed from spans of bytes from +//! ::capnp::Data fields. But so far there hasn't been a need for this. +template +void CustomBuildField(TypeList, Priority<2>, InvokeContext& invoke_context, Value&& value, Output&& output) +requires + (std::is_same_v) && + (std::convertible_to> || + std::convertible_to> || + std::convertible_to> || + std::convertible_to>) +{ + auto data = std::span{value}; + auto result = output.init(data.size()); + memcpy(result.begin(), data.data(), data.size()); +} } // namespace mp #endif // BITCOIN_IPC_CAPNP_COMMON_TYPES_H diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp index 00cfeb53215..32c01dba463 100644 --- a/src/test/ipc_test.capnp +++ b/src/test/ipc_test.capnp @@ -16,4 +16,5 @@ interface FooInterface $Proxy.wrap("FooImplementation") { passOutPoint @1 (arg :Data) -> (result :Data); passUniValue @2 (arg :Text) -> (result :Text); passTransaction @3 (arg :Data) -> (result :Data); + passVectorChar @4 (arg :Data) -> (result :Data); } diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp index 59083beabc6..91afce77786 100644 --- a/src/test/ipc_test.cpp +++ b/src/test/ipc_test.cpp @@ -97,6 +97,10 @@ void IpcPipeTest() CTransactionRef tx2{foo->passTransaction(tx1)}; BOOST_CHECK(*Assert(tx1) == *Assert(tx2)); + std::vector vec1{'H', 'e', 'l', 'l', 'o'}; + std::vector vec2{foo->passVectorChar(vec1)}; + BOOST_CHECK_EQUAL(std::string_view(vec1.begin(), vec1.end()), std::string_view(vec2.begin(), vec2.end())); + // Test cleanup: disconnect pipe and join thread disconnect_client(); thread.join(); diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h index 22fe96b6bac..ca8a5f61b51 100644 --- a/src/test/ipc_test.h +++ b/src/test/ipc_test.h @@ -16,6 +16,7 @@ public: COutPoint passOutPoint(COutPoint o) { return o; } UniValue passUniValue(UniValue v) { return v; } CTransactionRef passTransaction(CTransactionRef t) { return t; } + std::vector passVectorChar(std::vector v) { return v; } }; void IpcPipeTest(); From 33c2eee285e35db50b33efb6e782ff002f9d18ec Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 17 Jul 2024 11:36:34 -0400 Subject: [PATCH 6/8] multiprocess: Add IPC wrapper for Mining interface --- src/ipc/CMakeLists.txt | 6 ++++- src/ipc/capnp/common-types.h | 21 +++++++++++++++++ src/ipc/capnp/common.capnp | 16 +++++++++++++ src/ipc/capnp/init-types.h | 1 + src/ipc/capnp/init.capnp | 3 +++ src/ipc/capnp/mining-types.h | 16 +++++++++++++ src/ipc/capnp/mining.capnp | 44 ++++++++++++++++++++++++++++++++++++ 7 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 src/ipc/capnp/common.capnp create mode 100644 src/ipc/capnp/mining-types.h create mode 100644 src/ipc/capnp/mining.capnp diff --git a/src/ipc/CMakeLists.txt b/src/ipc/CMakeLists.txt index 94b1ceb54e4..427cd6ea3fe 100644 --- a/src/ipc/CMakeLists.txt +++ b/src/ipc/CMakeLists.txt @@ -9,10 +9,14 @@ add_library(bitcoin_ipc STATIC EXCLUDE_FROM_ALL ) target_capnp_sources(bitcoin_ipc ${PROJECT_SOURCE_DIR} - capnp/echo.capnp capnp/init.capnp + capnp/common.capnp + capnp/echo.capnp + capnp/init.capnp + capnp/mining.capnp ) target_link_libraries(bitcoin_ipc PRIVATE core_interface + univalue ) diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h index f1e2d18f1ce..51af6a5f0aa 100644 --- a/src/ipc/capnp/common-types.h +++ b/src/ipc/capnp/common-types.h @@ -6,6 +6,7 @@ #define BITCOIN_IPC_CAPNP_COMMON_TYPES_H #include +#include #include #include #include @@ -93,6 +94,26 @@ requires ipc::capnp::Deserializable return read_dest.construct(::deserialize, wrapper); } +//! Overload CustomBuildField and CustomReadField to serialize std::chrono +//! parameters and return values as numbers. +template +void CustomBuildField(TypeList>, Priority<1>, InvokeContext& invoke_context, Value&& value, + Output&& output) +{ + static_assert(std::numeric_limits::lowest() <= std::numeric_limits::lowest(), + "capnp type does not have enough range to hold lowest std::chrono::duration value"); + static_assert(std::numeric_limits::max() >= std::numeric_limits::max(), + "capnp type does not have enough range to hold highest std::chrono::duration value"); + output.set(value.count()); +} + +template +decltype(auto) CustomReadField(TypeList>, Priority<1>, InvokeContext& invoke_context, + Input&& input, ReadDest&& read_dest) +{ + return read_dest.construct(input.get()); +} + //! Overload CustomBuildField and CustomReadField to serialize UniValue //! parameters and return values as JSON strings. template diff --git a/src/ipc/capnp/common.capnp b/src/ipc/capnp/common.capnp new file mode 100644 index 00000000000..b3359f3f07b --- /dev/null +++ b/src/ipc/capnp/common.capnp @@ -0,0 +1,16 @@ +# Copyright (c) 2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +@0xcd2c6232cb484a28; + +using Cxx = import "/capnp/c++.capnp"; +$Cxx.namespace("ipc::capnp::messages"); + +using Proxy = import "/mp/proxy.capnp"; +$Proxy.includeTypes("ipc/capnp/common-types.h"); + +struct BlockRef $Proxy.wrap("interfaces::BlockRef") { + hash @0 :Data; + height @1 :Int32; +} diff --git a/src/ipc/capnp/init-types.h b/src/ipc/capnp/init-types.h index 42031441b59..c3ddca27c04 100644 --- a/src/ipc/capnp/init-types.h +++ b/src/ipc/capnp/init-types.h @@ -6,5 +6,6 @@ #define BITCOIN_IPC_CAPNP_INIT_TYPES_H #include +#include #endif // BITCOIN_IPC_CAPNP_INIT_TYPES_H diff --git a/src/ipc/capnp/init.capnp b/src/ipc/capnp/init.capnp index e6d358c6655..1001ee53368 100644 --- a/src/ipc/capnp/init.capnp +++ b/src/ipc/capnp/init.capnp @@ -10,11 +10,14 @@ $Cxx.namespace("ipc::capnp::messages"); using Proxy = import "/mp/proxy.capnp"; $Proxy.include("interfaces/echo.h"); $Proxy.include("interfaces/init.h"); +$Proxy.include("interfaces/mining.h"); $Proxy.includeTypes("ipc/capnp/init-types.h"); using Echo = import "echo.capnp"; +using Mining = import "mining.capnp"; interface Init $Proxy.wrap("interfaces::Init") { construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap); makeEcho @1 (context :Proxy.Context) -> (result :Echo.Echo); + makeMining @2 (context :Proxy.Context) -> (result :Mining.Mining); } diff --git a/src/ipc/capnp/mining-types.h b/src/ipc/capnp/mining-types.h new file mode 100644 index 00000000000..3204a0b6617 --- /dev/null +++ b/src/ipc/capnp/mining-types.h @@ -0,0 +1,16 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_IPC_CAPNP_MINING_TYPES_H +#define BITCOIN_IPC_CAPNP_MINING_TYPES_H + +#include +#include +#include +#include +#include +#include +#include + +#endif // BITCOIN_IPC_CAPNP_MINING_TYPES_H diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp new file mode 100644 index 00000000000..13d9e2ffb03 --- /dev/null +++ b/src/ipc/capnp/mining.capnp @@ -0,0 +1,44 @@ +# Copyright (c) 2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +@0xc77d03df6a41b505; + +using Cxx = import "/capnp/c++.capnp"; +$Cxx.namespace("ipc::capnp::messages"); + +using Common = import "common.capnp"; +using Proxy = import "/mp/proxy.capnp"; +$Proxy.include("interfaces/mining.h"); +$Proxy.includeTypes("ipc/capnp/mining-types.h"); + +interface Mining $Proxy.wrap("interfaces::Mining") { + isTestChain @0 (context :Proxy.Context) -> (result: Bool); + isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool); + getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool); + waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef); + createNewBlock @4 (scriptPubKey: Data, options: BlockCreateOptions) -> (result: BlockTemplate); + processNewBlock @5 (context :Proxy.Context, block: Data) -> (newBlock: Bool, result: Bool); + getTransactionsUpdated @6 (context :Proxy.Context) -> (result: UInt32); + testBlockValidity @7 (context :Proxy.Context, block: Data, checkMerkleRoot: Bool) -> (state: BlockValidationState, result: Bool); +} + +interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { + getBlockHeader @0 (context: Proxy.Context) -> (result: Data); + getBlock @1 (context: Proxy.Context) -> (result: Data); + getTxFees @2 (context: Proxy.Context) -> (result: List(Int64)); + getTxSigops @3 (context: Proxy.Context) -> (result: List(Int64)); + getCoinbaseTx @4 (context: Proxy.Context) -> (result: Data); + getCoinbaseCommitment @5 (context: Proxy.Context) -> (result: Data); + getWitnessCommitmentIndex @6 (context: Proxy.Context) -> (result: Int32); +} + +struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { + useMempool @0 :Bool $Proxy.name("use_mempool"); + coinbaseMaxAdditionalWeight @1 :UInt64 $Proxy.name("coinbase_max_additional_weight"); + coinbaseOutputMaxAdditionalSigops @2 :UInt64 $Proxy.name("coinbase_output_max_additional_sigops"); +} + +# TODO add fields to this struct +struct BlockValidationState $Proxy.wrap("BlockValidationState") { +} From d043950ba245cdcd09dc389a71d43b0a58e41a8b Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 16 Jul 2024 12:38:12 -0400 Subject: [PATCH 7/8] multiprocess: Add serialization code for BlockValidationState Co-authored-by: TheCharlatan --- src/ipc/CMakeLists.txt | 1 + src/ipc/capnp/mining-types.h | 10 ++++++++ src/ipc/capnp/mining.capnp | 10 ++++++-- src/ipc/capnp/mining.cpp | 47 ++++++++++++++++++++++++++++++++++++ src/test/ipc_test.capnp | 5 +++- src/test/ipc_test.cpp | 20 +++++++++++++++ src/test/ipc_test.h | 2 ++ src/test/ipc_test_types.h | 12 +++++++++ 8 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 src/ipc/capnp/mining.cpp create mode 100644 src/test/ipc_test_types.h diff --git a/src/ipc/CMakeLists.txt b/src/ipc/CMakeLists.txt index 427cd6ea3fe..904d72f56e1 100644 --- a/src/ipc/CMakeLists.txt +++ b/src/ipc/CMakeLists.txt @@ -3,6 +3,7 @@ # file COPYING or https://opensource.org/license/mit/. add_library(bitcoin_ipc STATIC EXCLUDE_FROM_ALL + capnp/mining.cpp capnp/protocol.cpp interfaces.cpp process.cpp diff --git a/src/ipc/capnp/mining-types.h b/src/ipc/capnp/mining-types.h index 3204a0b6617..2e60b43fcf3 100644 --- a/src/ipc/capnp/mining-types.h +++ b/src/ipc/capnp/mining-types.h @@ -13,4 +13,14 @@ #include #include +namespace mp { +// Custom serialization for BlockValidationState. +void CustomBuildMessage(InvokeContext& invoke_context, + const BlockValidationState& src, + ipc::capnp::messages::BlockValidationState::Builder&& builder); +void CustomReadMessage(InvokeContext& invoke_context, + const ipc::capnp::messages::BlockValidationState::Reader& reader, + BlockValidationState& dest); +} // namespace mp + #endif // BITCOIN_IPC_CAPNP_MINING_TYPES_H diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 13d9e2ffb03..4ea69d16c98 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -39,6 +39,12 @@ struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { coinbaseOutputMaxAdditionalSigops @2 :UInt64 $Proxy.name("coinbase_output_max_additional_sigops"); } -# TODO add fields to this struct -struct BlockValidationState $Proxy.wrap("BlockValidationState") { +# Note: serialization of the BlockValidationState C++ type is somewhat fragile +# and using the struct can be awkward. It would be good if testBlockValidity +# method were changed to return validity information in a simpler format. +struct BlockValidationState { + mode @0 :Int32; + result @1 :Int32; + rejectReason @2 :Text; + debugMessage @3 :Text; } diff --git a/src/ipc/capnp/mining.cpp b/src/ipc/capnp/mining.cpp new file mode 100644 index 00000000000..0f9533c1c73 --- /dev/null +++ b/src/ipc/capnp/mining.cpp @@ -0,0 +1,47 @@ +// Copyright (c) 2024 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 + +namespace mp { +void CustomBuildMessage(InvokeContext& invoke_context, + const BlockValidationState& src, + ipc::capnp::messages::BlockValidationState::Builder&& builder) +{ + if (src.IsValid()) { + builder.setMode(0); + } else if (src.IsInvalid()) { + builder.setMode(1); + } else if (src.IsError()) { + builder.setMode(2); + } else { + assert(false); + } + builder.setResult(static_cast(src.GetResult())); + builder.setRejectReason(src.GetRejectReason()); + builder.setDebugMessage(src.GetDebugMessage()); +} + +void CustomReadMessage(InvokeContext& invoke_context, + const ipc::capnp::messages::BlockValidationState::Reader& reader, + BlockValidationState& dest) +{ + if (reader.getMode() == 0) { + assert(reader.getResult() == 0); + assert(reader.getRejectReason().size() == 0); + assert(reader.getDebugMessage().size() == 0); + } else if (reader.getMode() == 1) { + dest.Invalid(static_cast(reader.getResult()), reader.getRejectReason(), reader.getDebugMessage()); + } else if (reader.getMode() == 2) { + assert(reader.getResult() == 0); + dest.Error(reader.getRejectReason()); + assert(reader.getDebugMessage().size() == 0); + } else { + assert(false); + } +} +} // namespace mp diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp index 32c01dba463..46cd08b94a2 100644 --- a/src/test/ipc_test.capnp +++ b/src/test/ipc_test.capnp @@ -9,7 +9,9 @@ $Cxx.namespace("gen"); using Proxy = import "/mp/proxy.capnp"; $Proxy.include("test/ipc_test.h"); -$Proxy.includeTypes("ipc/capnp/common-types.h"); +$Proxy.includeTypes("test/ipc_test_types.h"); + +using Mining = import "../ipc/capnp/mining.capnp"; interface FooInterface $Proxy.wrap("FooImplementation") { add @0 (a :Int32, b :Int32) -> (result :Int32); @@ -17,4 +19,5 @@ interface FooInterface $Proxy.wrap("FooImplementation") { passUniValue @2 (arg :Text) -> (result :Text); passTransaction @3 (arg :Data) -> (result :Data); passVectorChar @4 (arg :Data) -> (result :Data); + passBlockState @5 (arg :Mining.BlockValidationState) -> (result :Mining.BlockValidationState); } diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp index 91afce77786..91eba9214f0 100644 --- a/src/test/ipc_test.cpp +++ b/src/test/ipc_test.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -101,6 +102,25 @@ void IpcPipeTest() std::vector vec2{foo->passVectorChar(vec1)}; BOOST_CHECK_EQUAL(std::string_view(vec1.begin(), vec1.end()), std::string_view(vec2.begin(), vec2.end())); + BlockValidationState bs1; + bs1.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, "reject reason", "debug message"); + BlockValidationState bs2{foo->passBlockState(bs1)}; + BOOST_CHECK_EQUAL(bs1.IsValid(), bs2.IsValid()); + BOOST_CHECK_EQUAL(bs1.IsError(), bs2.IsError()); + BOOST_CHECK_EQUAL(bs1.IsInvalid(), bs2.IsInvalid()); + BOOST_CHECK_EQUAL(static_cast(bs1.GetResult()), static_cast(bs2.GetResult())); + BOOST_CHECK_EQUAL(bs1.GetRejectReason(), bs2.GetRejectReason()); + BOOST_CHECK_EQUAL(bs1.GetDebugMessage(), bs2.GetDebugMessage()); + + BlockValidationState bs3; + BlockValidationState bs4{foo->passBlockState(bs3)}; + BOOST_CHECK_EQUAL(bs3.IsValid(), bs4.IsValid()); + BOOST_CHECK_EQUAL(bs3.IsError(), bs4.IsError()); + BOOST_CHECK_EQUAL(bs3.IsInvalid(), bs4.IsInvalid()); + BOOST_CHECK_EQUAL(static_cast(bs3.GetResult()), static_cast(bs4.GetResult())); + BOOST_CHECK_EQUAL(bs3.GetRejectReason(), bs4.GetRejectReason()); + BOOST_CHECK_EQUAL(bs3.GetDebugMessage(), bs4.GetDebugMessage()); + // Test cleanup: disconnect pipe and join thread disconnect_client(); thread.join(); diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h index ca8a5f61b51..2d215a20f1b 100644 --- a/src/test/ipc_test.h +++ b/src/test/ipc_test.h @@ -8,6 +8,7 @@ #include #include #include +#include class FooImplementation { @@ -17,6 +18,7 @@ public: UniValue passUniValue(UniValue v) { return v; } CTransactionRef passTransaction(CTransactionRef t) { return t; } std::vector passVectorChar(std::vector v) { return v; } + BlockValidationState passBlockState(BlockValidationState s) { return s; } }; void IpcPipeTest(); diff --git a/src/test/ipc_test_types.h b/src/test/ipc_test_types.h new file mode 100644 index 00000000000..b1d4829aa71 --- /dev/null +++ b/src/test/ipc_test_types.h @@ -0,0 +1,12 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_IPC_TEST_TYPES_H +#define BITCOIN_TEST_IPC_TEST_TYPES_H + +#include +#include +#include + +#endif // BITCOIN_TEST_IPC_TEST_TYPES_H From 1a332817665f77f55090fa166930fec0e5500727 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 11 Jan 2024 18:07:20 -0500 Subject: [PATCH 8/8] doc: multiprocess documentation improvements Most improvements suggested by stickies-v https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1800375604 Omission of CustomReadMessage and CustomBuildMessage was noticed by TheCharlatan in https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1708996040 and is fixed here as well. Co-authored-by: stickies-v --- doc/design/multiprocess.md | 8 ++++---- doc/multiprocess.md | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/design/multiprocess.md b/doc/design/multiprocess.md index 49410a4213d..a781da8d1ba 100644 --- a/doc/design/multiprocess.md +++ b/doc/design/multiprocess.md @@ -81,7 +81,7 @@ This section describes the major components of the Inter-Process Communication ( - In the generated code, we have C++ client subclasses that inherit from the abstract classes in [`src/interfaces/`](../../src/interfaces/). These subclasses are the workhorses of the IPC mechanism. - They implement all the methods of the interface, marshalling arguments into a structured format, sending them as requests to the IPC server via a UNIX socket, and handling the responses. - These subclasses effectively mask the complexity of IPC, presenting a familiar C++ interface to developers. -- Internally, the client subclasses generated by the `mpgen` tool wrap [client classes generated by Cap'n Proto](https://capnproto.org/cxxrpc.html#clients), and use them to send IPC requests. +- Internally, the client subclasses generated by the `mpgen` tool wrap [client classes generated by Cap'n Proto](https://capnproto.org/cxxrpc.html#clients), and use them to send IPC requests. The Cap'n Proto client classes are low-level, with non-blocking methods that use asynchronous I/O and pass request and response objects, while mpgen client subclasses provide normal C++ methods that block while executing and convert between request/response objects and arguments/return values. ### C++ Server Classes in Generated Code - On the server side, corresponding generated C++ classes receive IPC requests. These server classes are responsible for unmarshalling method arguments, invoking the corresponding methods in the local [`src/interfaces/`](../../src/interfaces/) objects, and creating the IPC response. @@ -94,7 +94,7 @@ This section describes the major components of the Inter-Process Communication ( - **Asynchronous I/O and Thread Management**: The library is also responsible for managing I/O and threading. Particularly, it ensures that IPC requests never block each other and that new threads on either side of a connection can always make client calls. It also manages worker threads on the server side of calls, ensuring that calls from the same client thread always execute on the same server thread (to avoid locking issues and support nested callbacks). ### Type Hooks in [`src/ipc/capnp/*-types.h`](../../src/ipc/capnp/) -- **Custom Type Conversions**: In [`src/ipc/capnp/*-types.h`](../../src/ipc/capnp/), function overloads of two `libmultiprocess` C++ functions, `mp::CustomReadField` and `mp::CustomBuildFields`, are defined. These overloads are used for customizing the conversion of specific C++ types to and from Cap’n Proto types. +- **Custom Type Conversions**: In [`src/ipc/capnp/*-types.h`](../../src/ipc/capnp/), function overloads of `libmultiprocess` C++ functions, `mp::CustomReadField`, `mp::CustomBuildField`, `mp::CustomReadMessage` and `mp::CustomBuildMessage`, are defined. These overloads are used for customizing the conversion of specific C++ types to and from Cap’n Proto types. - **Handling Special Cases**: The `mpgen` tool and `libmultiprocess` library can convert most C++ types to and from Cap’n Proto types automatically, including interface types, primitive C++ types, standard C++ types like `std::vector`, `std::set`, `std::map`, `std::tuple`, and `std::function`, as well as simple C++ structs that consist of aforementioned types and whose fields correspond 1:1 with Cap’n Proto struct fields. For other types, `*-types.h` files provide custom code to convert between C++ and Cap’n Proto data representations. ### Protocol-Agnostic IPC Code in [`src/ipc/`](../../src/ipc/) @@ -197,7 +197,7 @@ sequenceDiagram - Upon receiving the request, the Cap'n Proto dispatching code in the `bitcoin-node` process calls the `getBlockHash` method of the `Chain` [server class](#c-server-classes-in-generated-code). - The server class is automatically generated by the `mpgen` tool from the [`chain.capnp`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc/src/ipc/capnp/chain.capnp) file in [`src/ipc/capnp/`](../../src/ipc/capnp/). - The `getBlockHash` method of the generated `Chain` server subclass in `bitcoin-wallet` receives a Cap’n Proto request object with the `height` parameter, and calls the `getBlockHash` method on its local `Chain` object with the provided `height`. - - When the call returns, it encapsulates the return value in a Cap’n Proto response, which it sends back to the `bitcoin-wallet` process, + - When the call returns, it encapsulates the return value in a Cap’n Proto response, which it sends back to the `bitcoin-wallet` process. 5. **Response and Return** - The `getBlockHash` method of the generated `Chain` client subclass in `bitcoin-wallet` which sent the request now receives the response. @@ -232,7 +232,7 @@ This modularization represents an advancement in Bitcoin Core's architecture, of - **Cap’n Proto struct**: A structured data format used in Cap’n Proto, similar to structs in C++, for organizing and transporting data across different processes. -- **client class (in generated code)**: A C++ class generated from a Cap’n Proto interface which inherits from a Bitcoin core abstract class, and implements each virtual method to send IPC requests to another process. (see also [components section](#c-client-subclasses-in-generated-code)) +- **client class (in generated code)**: A C++ class generated from a Cap’n Proto interface which inherits from a Bitcoin Core abstract class, and implements each virtual method to send IPC requests to another process. (see also [components section](#c-client-subclasses-in-generated-code)) - **IPC (inter-process communication)**: Mechanisms that enable processes to exchange requests and data. diff --git a/doc/multiprocess.md b/doc/multiprocess.md index fee50142d7b..1757296eed6 100644 --- a/doc/multiprocess.md +++ b/doc/multiprocess.md @@ -17,7 +17,9 @@ The multiprocess feature requires [Cap'n Proto](https://capnproto.org/) and [lib ``` cd make -C depends NO_QT=1 MULTIPROCESS=1 -cmake -B build --toolchain=depends/x86_64-pc-linux-gnu/toolchain.cmake +# Set host platform to output of gcc -dumpmachine or clang -dumpmachine or check the depends/ directory for the generated subdirectory name +HOST_PLATFORM="x86_64-pc-linux-gnu" +cmake -B build --toolchain=depends/$HOST_PLATFORM/toolchain.cmake cmake --build build build/src/bitcoin-node -regtest -printtoconsole -debug=ipc BITCOIND=$(pwd)/build/src/bitcoin-node build/test/functional/test_runner.py