diff --git a/doc/design/multiprocess.md b/doc/design/multiprocess.md index b7e7e3c8d78f7..636d78d905c45 100644 --- a/doc/design/multiprocess.md +++ b/doc/design/multiprocess.md @@ -1,72 +1,264 @@ -# Multiprocess Bitcoin +# Multiprocess Bitcoin Design Document -On unix systems, the `--enable-multiprocess` build option can be passed to `./configure` to build new `bitcoin-node`, `navcoin-wallet`, and `navcoin-gui` executables alongside existing `navcoind` and `navcoin-qt` executables. +Guide to the design and architecture of the Bitcoin Core multiprocess feature -`bitcoin-node` is a drop-in replacement for `navcoind`, and `navcoin-gui` is a drop-in replacement for `navcoin-qt`, and there are no differences in use or external behavior between the new and old executables. But internally (after [#10102](https://github.com/bitcoin/bitcoin/pull/10102)), `navcoin-gui` will spawn a `bitcoin-node` process to run P2P and RPC code, communicating with it across a socket pair, and `bitcoin-node` will spawn `navcoin-wallet` to run wallet code, also communicating over a socket pair. This will let node, wallet, and GUI code run in separate address spaces for better isolation, and allow future improvements like being able to start and stop components independently on different machines and environments. +_This document describes the design of the multiprocess feature. For usage information, see the top-level [multiprocess.md](../multiprocess.md) file._ -## Next steps +## Table of contents -Specific next steps after [#10102](https://github.com/bitcoin/bitcoin/pull/10102) will be: +- [Introduction](#introduction) +- [Current Architecture](#current-architecture) +- [Proposed Architecture](#proposed-architecture) +- [Component Overview: Navigating the IPC Framework](#component-overview-navigating-the-ipc-framework) +- [Design Considerations](#design-considerations) + - [Selection of Cap’n Proto](#selection-of-capn-proto) + - [Hiding IPC](#hiding-ipc) + - [Interface Definition Maintenance](#interface-definition-maintenance) + - [Interface Stability](#interface-stability) +- [Security Considerations](#security-considerations) +- [Example Use Cases and Flows](#example-use-cases-and-flows) + - [Retrieving a Block Hash](#retrieving-a-block-hash) +- [Future Enhancements](#future-enhancements) +- [Conclusion](#conclusion) +- [Appendices](#appendices) + - [Glossary of Terms](#glossary-of-terms) + - [References](#references) +- [Acknowledgements](#acknowledgements) -- [ ] Adding `-ipcbind` and `-ipcconnect` options to `bitcoin-node`, `navcoin-wallet`, and `navcoin-gui` executables so they can listen and connect to TCP ports and unix socket paths. This will allow separate processes to be started and stopped any time and connect to each other. -- [ ] Adding `-server` and `-rpcbind` options to the `navcoin-wallet` executable so wallet processes can handle RPC requests directly without going through the node. -- [ ] Supporting windows, not just unix systems. The existing socket code is already cross-platform, so the only windows-specific code that needs to be written is code spawning a process and passing a socket descriptor. This can be implemented with `CreateProcess` and `WSADuplicateSocket`. Example: https://memset.wordpress.com/2010/10/13/win32-api-passing-socket-with-ipc-method/. -- [ ] Adding sandbox features, restricting subprocess access to resources and data. See [https://eklitzke.org/multiprocess-bitcoin](https://eklitzke.org/multiprocess-bitcoin). +## Introduction -## Debugging +The Bitcoin Core software has historically employed a monolithic architecture. The existing design has integrated functionality like P2P network operations, wallet management, and a GUI into a single executable. While effective, it has limitations in flexibility, security, and scalability. This project introduces changes that transition Bitcoin Core to a more modular architecture. It aims to enhance security, improve usability, and facilitate maintenance and development of the software in the long run. -The `-debug=ipc` command line option can be used to see requests and responses between processes. +## Current Architecture -## Installation +The current system features two primary executables: `bitcoind` and `bitcoin-qt`. `bitcoind` combines a Bitcoin P2P node with an integrated JSON-RPC server, wallet, and indexes. `bitcoin-qt` extends this by incorporating a Qt-based GUI. This monolithic structure, although robust, presents challenges such as limited operational flexibility and increased security risks due to the tight integration of components. -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: +## Proposed Architecture +The new architecture divides the existing code into three specialized executables: + +- `bitcoin-node`: Manages the P2P node, indexes, and JSON-RPC server. +- `bitcoin-wallet`: Handles all wallet functionality. +- `bitcoin-gui`: Provides a standalone Qt-based GUI. + +This modular approach is designed to enhance security through component isolation and improve usability by allowing independent operation of each module. This allows for new use-cases, such as running the node on a dedicated machine and operating wallets and GUIs on separate machines with the flexibility to start and stop them as needed. + +This subdivision could be extended in the future. For example, indexes could be removed from the `bitcoin-node` executable and run in separate executables. And JSON-RPC servers could be added to wallet and index executables, so they can listen and respond to RPC requests on their own ports, without needing to forward RPC requests through `bitcoin-node`. + +
+ +```mermaid +flowchart LR + node[bitcoin-node] -- listens on --> socket["<datadir>/node.sock"] + wallet[bitcoin-wallet] -- connects to --> socket + gui[bitcoin-gui] -- connects to --> socket +``` + +
+Processes and socket connection. +
+ +## Component Overview: Navigating the IPC Framework + +This section describes the major components of the Inter-Process Communication (IPC) framework covering the relevant source files, generated files, tools, and libraries. + +### Abstract C++ Classes in [`src/interfaces/`](../../src/interfaces/) +- The foundation of the IPC implementation lies in the abstract C++ classes within the [`src/interfaces/`](../../src/interfaces/) directory. These classes define pure virtual methods that code in [`src/node/`](../../src/node/), [`src/wallet/`](../../src/wallet/), and [`src/qt/`](../../src/qt/) directories call to interact with each other. +- Each abstract class in this directory represents a distinct interface that the different modules (node, wallet, GUI) implement and use for cross-process communication. +- The classes are written following conventions described in [Internal Interface + Guidelines](../developer-notes.md#internal-interface-guidelines) to ensure + compatibility with Cap'n Proto. + +### Cap’n Proto Files in [`src/ipc/capnp/`](../../src/ipc/capnp/) +- Corresponding to each abstract class, there are `.capnp` files within the [`src/ipc/capnp/`](../../src/ipc/capnp/) directory. These files are used as input to the `mpgen` tool (described below) to generate C++ code. +- These Cap’n Proto files ([learn more about Cap'n Proto RPC](https://capnproto.org/rpc.html)) define the structure and format of messages that are exchanged over IPC. They serve as blueprints for generating C++ code that bridges the gap between high-level C++ interfaces and low-level socket communication. + +### The `mpgen` Code Generation Tool +- A central component of the IPC framework is the `mpgen` tool which is part the [`libmultiprocess` project](https://github.com/chaincodelabs/libmultiprocess). This tool takes the `.capnp` files as input and generates C++ code. +- The generated code handles IPC communication, translating interface calls into socket reads and writes. + +### C++ Client Subclasses in Generated Code +- 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. + +### 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. +- The server classes ensure that return values (including output argument values and thrown exceptions) are marshalled and sent back to the client, completing the communication cycle. +- Internally, the server subclasses generated by the `mpgen` tool inherit from [server classes generated by Cap'n Proto](https://capnproto.org/cxxrpc.html#servers), and use them to process IPC requests. + +### The `libmultiprocess` Runtime Library +- **Core Functionality**: The `libmultiprocess` runtime library's primary function is to instantiate the generated client and server classes as needed. +- **Bootstrapping IPC Connections**: It provides functions for starting new IPC connections, specifically binding generated client and server classes for an initial `interfaces::Init` interface (defined in [`src/interfaces/init.h`](../../src/interfaces/init.h)) to a UNIX socket. This initial interface has methods returning other interfaces that different Bitcoin Core modules use to communicate after the bootstrapping phase. +- **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. +- **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/) +- **Broad Applicability**: Unlike the Cap’n Proto-specific code in [`src/ipc/capnp/`](../../src/ipc/capnp/), the code in the [`src/ipc/`](../../src/ipc/) directory is protocol-agnostic. This enables potential support for other protocols, such as gRPC or a custom protocol in the future. +- **Process Management and Socket Operations**: The main purpose of this component is to provide functions for spawning new processes and creating and connecting to UNIX sockets. +- **ipc::Exception Class**: This code also defines an `ipc::Exception` class which is thrown from the generated C++ client class methods when there is an unexpected IPC error, such as a disconnection. + +
+ +```mermaid +flowchart TD + capnpFile[ipc/capnp/chain.capnp] -->|Input to| mpgenTool([mpgen Tool]) + mpgenTool -->|Generates| proxyTypesH[ipc/capnp/chain.capnp.proxy-types.h] + mpgenTool --> proxyClientCpp[ipc/capnp/chain.capnp.proxy-client.c++] + mpgenTool --> proxyServerCpp[ipc/capnp/chain.capnp.proxy-server.c++] + proxyTypesH -.->|Includes| interfaces/chain.h + proxyClientCpp -.-> interfaces/chain.h + proxyServerCpp -.-> interfaces/chain.h ``` -cd -make -C depends MULTIPROCESS=1 -CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure -make -src/bitcoin-node -regtest -printtoconsole -debug=ipc -BITCOIND=bitcoin-node test/functional/test_runner.py + +
+Diagram showing generated source files and includes. +
+ +## Design Considerations + +### Selection of Cap’n Proto +The choice to use [Cap’n Proto](https://capnproto.org/) for IPC was primarily influenced by its support for passing object references and managing object lifetimes, which would have to be implemented manually with a framework that only supported plain requests and responses like [gRPC](https://grpc.io/). The support is especially helpful for passing callback objects like `std::function` and enabling bidirectional calls between processes. + +The choice to use an RPC framework at all instead of a custom protocol was necessitated by the size of Bitcoin Core internal interfaces which consist of around 150 methods that pass complex data structures and are called in complicated ways (in parallel, and from callbacks that can be nested and stored). Writing a custom protocol to wrap these complicated interfaces would be a lot more work, akin to writing a new RPC framework. + +### Hiding IPC + +The IPC mechanism is deliberately isolated from the rest of the codebase so less code has to be concerned with IPC. + +Building Bitcoin Core with IPC support is optional, and node, wallet, and GUI code can be compiled to either run in the same process or separate processes. The build system also ensures Cap’n Proto library headers can only be used within the [`src/ipc/capnp/`](../../src/ipc/capnp/) directory, not in other parts of the codebase. + +The libmultiprocess runtime is designed to place as few constraints as possible on IPC interfaces and to make IPC calls act like normal function calls. Method arguments, return values, and exceptions are automatically serialized and sent between processes. Object references and `std::function` arguments are tracked to allow invoked code to call back into invoking code at any time. And there is a 1:1 threading model where every client thread has a corresponding server thread responsible for executing incoming calls from that thread (there can be multiple calls from the same thread due to callbacks) without blocking, and holding the same thread-local variables and locks so behavior is the same whether IPC is used or not. + +### Interface Definition Maintenance + +The choice to maintain interface definitions and C++ type mappings as `.capnp` files in the [`src/ipc/capnp/`](../../src/ipc/capnp/) was mostly done for convenience, and probably something that could be improved in the future. + +In the current design, class names, method names, and parameter names are duplicated between C++ interfaces in [`src/interfaces/`](../../src/interfaces/) and Cap’n Proto files in [`src/ipc/capnp/`](../../src/ipc/capnp/). While this keeps C++ interface headers simple and free of references to IPC, it is a maintenance burden because it means inconsistencies between C++ declarations and Cap’n Proto declarations will result in compile errors. (Static type checking ensures these are not runtime errors.) + +An alternate approach could use custom [C++ Attributes](https://en.cppreference.com/w/cpp/language/attributes) embedded in interface declarations to automatically generate `.capnp` files from C++ headers. This has not been pursued because parsing C++ headers is more complicated than parsing Cap’n Proto interface definitions, especially portably on multiple platforms. + +In the meantime, the developer guide [Internal interface guidelines](developer-notes.md#internal-interface-guidelines) can provide guidance on keeping interfaces consistent and functional and avoiding compile errors. + +### Interface Stability + +The currently defined IPC interfaces are unstable, and can change freely with no backwards compatibility. The decision to allow this stems from the recognition that our current interfaces are still evolving and not yet ideal for external use. As these interfaces mature and become more refined, there may be an opportunity to declare them stable and use Cap’n Proto's support for protocol evolution ([Cap'n Proto - Evolving Your Protocol](https://capnproto.org/language.html#evolving-your-protocol)) to allow them to be extended while remaining backwards compatible. This could allow different versions of node, GUI, and wallet binaries to interoperate, and potentially open doors for external tools to utilize these interfaces, such as creating custom indexes through a stable indexing interface. However, for now, the priority is to improve the interfaces internally. Given their current state and the advantages of using JSON-RPC for most common tasks, it's more practical to focus on internal development rather than external applicability. + +## Security Considerations + +The integration of [Cap’n Proto](https://capnproto.org/) and [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) into the Bitcoin Core architecture increases its potential attack surface. Cap’n Proto, being a complex and substantial new dependency, introduces potential sources of vulnerability, particularly through the creation of new UNIX sockets. The inclusion of libmultiprocess, while a smaller external dependency, also contributes to this risk. However, plans are underway to incorporate libmultiprocess as a git subtree, aligning it more closely with the project's well-reviewed internal libraries. While adopting these multiprocess features does introduce some risk, it's worth noting that they can be disabled, allowing builds without these new dependencies. This flexibility ensures that users can balance functionality with security considerations as needed. + +## Example Use Cases and Flows + +### Retrieving a Block Hash + +Let’s walk through an example where the `bitcoin-wallet` process requests the hash of a block at a specific height from the `bitcoin-node` process. This example demonstrates the practical application of the IPC mechanism, specifically the interplay between C++ method calls and Cap’n Proto-generated RPC calls. + +
+ +```mermaid +sequenceDiagram + box "bitcoin-wallet process" + participant WalletCode as Wallet code + participant ChainClient as Generated Chain client class
ProxyClient + end + box "bitcoin-node process" + participant ChainServer as Generated Chain server class
ProxyServer + participant LocalChain as Chain object
node::ChainImpl + end + + WalletCode->>ChainClient: getBlockHash(height) + ChainClient->>ChainServer: Send RPC getBlockHash request + ChainServer->>LocalChain: getBlockHash(height) + LocalChain->>ChainServer: Return block hash + ChainServer->>ChainClient: Send response with block hash + ChainClient->>WalletCode: Return block hash ``` -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/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 -[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 -calls between processes. - -When Wallet, Node, and Chain code is running in the same process, calling any -interface method invokes the implementation directly. When code is running in -different processes, calling an interface method invokes a proxy interface -implementation that communicates with a remote process and invokes the real -implementation in the remote process. The -[libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) code -generation tool internally generates proxy client classes and proxy server -classes for this purpose that are thin wrappers around Cap'n Proto -[client](https://capnproto.org/cxxrpc.html#clients) and -[server](https://capnproto.org/cxxrpc.html#servers) classes, which handle the -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 -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 -a 1:1 threading model where any thread invoking a method in another process has -a corresponding thread in the invoked process responsible for executing all -method calls from the source thread, without blocking I/O or holding up another -call, and using the same thread local variables, locks, and callbacks between -calls. The forwarding, tracking, and threading is implemented inside the -[libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library -which has the design goal of making calls between processes look like calls in -the same process to the extent possible. +
+Chain::getBlockHash call diagram +
+ +1. **Initiation in bitcoin-wallet** + - The wallet process calls the `getBlockHash` method on a `Chain` object. This method is defined as a virtual method in [`src/interfaces/chain.h`](../../src/interfaces/chain.h). + +2. **Translation to Cap’n Proto RPC** + - The `Chain::getBlockHash` virtual method is overridden by the `Chain` [client subclass](#c-client-subclasses-in-generated-code) to translate the method call into a Cap’n Proto RPC call. + - The client subclass 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/). + +3. **Request Preparation and Dispatch** + - The `getBlockHash` method of the generated `Chain` client subclass in `bitcoin-wallet` populates a Cap’n Proto request with the `height` parameter, sends it to `bitcoin-node` process, and waits for a response. + +4. **Handling in bitcoin-node** + - 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, + +5. **Response and Return** + - The `getBlockHash` method of the generated `Chain` client subclass in `bitcoin-wallet` which sent the request now receives the response. + - It extracts the block hash value from the response, and returns it to the original caller. + +## Future Enhancements + +Further improvements are possible such as: + +- Separating indexes from `bitcoin-node`, and running indexing code in separate processes (see [indexes: Stop using node internal types #24230](https://github.com/bitcoin/bitcoin/pull/24230)). +- Enabling wallet processes to listen for JSON-RPC requests on their own ports instead of needing the node process to listen and forward requests to them. +- Automatically generating `.capnp` files from C++ interface definitions (see [Interface Definition Maintenance](#interface-definition-maintenance)). +- Simplifying and stabilizing interfaces (see [Interface Stability](#interface-stability)). +- Adding sandbox features, restricting subprocess access to resources and data (see [https://eklitzke.org/multiprocess-bitcoin](https://eklitzke.org/multiprocess-bitcoin)). +- Using Cap'n Proto's support for [other languages](https://capnproto.org/otherlang.html), such as [Rust](https://github.com/capnproto/capnproto-rust), to allow code written in other languages to call Bitcoin Core C++ code, and vice versa (see [How to rustify libmultiprocess? #56](https://github.com/chaincodelabs/libmultiprocess/issues/56)). + +## Conclusion + +This modularization represents an advancement in Bitcoin Core's architecture, offering enhanced security, flexibility, and maintainability. The project invites collaboration and feedback from the community. + +## Appendices + +### Glossary of Terms + +- **abstract class**: A class in C++ that consists of virtual functions. In the Bitcoin Core project, they define interfaces for inter-component communication. + +- **asynchronous I/O**: A form of input/output processing that allows a program to continue other operations while a transmission is in progress. + +- **Cap’n Proto**: A high-performance data serialization and RPC library, chosen for its support for object references and bidirectional communication. + +- **Cap’n Proto interface**: A set of methods defined in Cap’n Proto to facilitate structured communication between different software components. + +- **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)) + +- **IPC (inter-process communication)**: Mechanisms that enable processes to exchange requests and data. + +- **ipc::Exception class**: A class within Bitcoin Core's protocol-agnostic IPC code that is thrown by client class methods when there is an IPC error. + +- **libmultiprocess**: A custom library and code generation tool used for creating IPC interfaces and managing IPC connections. + +- **marshalling**: Transforming an object’s memory representation for transmission. + +- **mpgen tool**: A tool within the `libmultiprocess` suite that generates C++ code from Cap’n Proto files, facilitating IPC. + +- **protocol-agnostic code**: Generic IPC code in [`src/ipc/`](../../src/ipc/) that does not rely on Cap’n Proto and could be used with other protocols. Distinct from code in [`src/ipc/capnp/`](../../src/ipc/capnp/) which relies on Cap’n Proto. + +- **RPC (remote procedure call)**: A protocol that enables a program to request a service from another program in a different address space or network. Bitcoin Core uses [JSON-RPC](https://en.wikipedia.org/wiki/JSON-RPC) for RPC. + +- **server class (in generated code)**: A C++ class generated from a Cap’n Proto interface which handles requests sent by a _client class_ in another process. The request handled by calling a local Bitcoin Core interface method, and the return values (if any) are sent back in a response. (see also: [components section](#c-server-classes-in-generated-code)) + +- **unix socket**: Communication endpoint which is a filesystem path, used for exchanging data between processes running on the same host. + +- **virtual method**: A function or method whose behavior can be overridden within an inheriting class by a function with the same signature. + +## References + +- **Cap’n Proto RPC protocol description**: https://capnproto.org/rpc.html +- **libmultiprocess project page**: https://github.com/chaincodelabs/libmultiprocess + +## Acknowledgements + +This design doc was written by @ryanofsky, who is grateful to all the reviewers who gave feedback and tested [multiprocess PRs](https://github.com/bitcoin/bitcoin/pull/28722), and everyone else who's helped with this project. Particular thanks to @ariard who deeply reviewed IPC code and improved the design of the IPC library and initialization process. @jnewbery who championed the early refactoring PRs and helped guide them through development and review. @sjors who has reviewed and repeatedly tested multiprocess code, reporting many issues and helping debug them. @hebasto, @fanquake, and @maflcko who made significant improvements to the build system and fixed countless build issues. @vasild and @jamesob who were brave contributors to the libmultiprocess library. And Chaincode Labs for making this work possible. Also thanks to ChatGPT, who actually wrote most of this document (not @ryanofsky). diff --git a/doc/multiprocess.md b/doc/multiprocess.md new file mode 100644 index 0000000000000..7ba89b3ff56a2 --- /dev/null +++ b/doc/multiprocess.md @@ -0,0 +1,34 @@ +# Multiprocess Bitcoin + +_This document describes usage of the multiprocess feature. For design information, see the [design/multiprocess.md](design/multiprocess.md) file._ + +## Build Option + +On unix systems, the `--enable-multiprocess` build option can be passed to `./configure` to build new `bitcoin-node`, `bitcoin-wallet`, and `bitcoin-gui` executables alongside existing `bitcoind` and `bitcoin-qt` executables. + +## Debugging + +The `-debug=ipc` command line option can be used to see requests and responses between processes. + +## 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 started 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 +make -C depends NO_QT=1 MULTIPROCESS=1 +CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure +make +src/bitcoin-node -regtest -printtoconsole -debug=ipc +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/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. + +## Usage + +`bitcoin-node` is a drop-in replacement for `bitcoind`, and `bitcoin-gui` is a drop-in replacement for `bitcoin-qt`, and there are no differences in use or external behavior between the new and old executables. But internally after [#10102](https://github.com/bitcoin/bitcoin/pull/10102), `bitcoin-gui` will spawn a `bitcoin-node` process to run P2P and RPC code, communicating with it across a socket pair, and `bitcoin-node` will spawn `bitcoin-wallet` to run wallet code, also communicating over a socket pair. This will let node, wallet, and GUI code run in separate address spaces for better isolation, and allow future improvements like being able to start and stop components independently on different machines and environments. +[#19460](https://github.com/bitcoin/bitcoin/pull/19460) also adds a new `bitcoin-node` `-ipcbind` option and an `bitcoind-wallet` `-ipcconnect` option to allow new wallet processes to connect to an existing node process. +And [#19461](https://github.com/bitcoin/bitcoin/pull/19461) adds a new `bitcoin-gui` `-ipcconnect` option to allow new GUI processes to connect to an existing node process. diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index b402a8be05322..3c4cedc0db601 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -47,6 +47,7 @@ bench_bench_navcoin_SOURCES = \ bench/poly1305.cpp \ bench/pool.cpp \ bench/prevector.cpp \ + bench/readblock.cpp \ bench/rollingbloom.cpp \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ diff --git a/src/bench/ellswift.cpp b/src/bench/ellswift.cpp index f0348421b38ee..82e8dec9826fc 100644 --- a/src/bench/ellswift.cpp +++ b/src/bench/ellswift.cpp @@ -11,9 +11,7 @@ static void EllSwiftCreate(benchmark::Bench& bench) { ECC_Start(); - CKey key; - key.MakeNewKey(true); - + CKey key = GenerateRandomKey(); uint256 entropy = GetRandHash(); bench.batch(1).unit("pubkey").run([&] { diff --git a/src/bench/readblock.cpp b/src/bench/readblock.cpp new file mode 100644 index 0000000000000..0545c6b0170fa --- /dev/null +++ b/src/bench/readblock.cpp @@ -0,0 +1,53 @@ +// 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 +#include +#include +#include + +static FlatFilePos WriteBlockToDisk(ChainstateManager& chainman) +{ + DataStream stream{benchmark::data::block413567}; + CBlock block; + stream >> TX_WITH_WITNESS(block); + + return chainman.m_blockman.SaveBlockToDisk(block, 0, nullptr); +} + +static void ReadBlockFromDiskTest(benchmark::Bench& bench) +{ + const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; + ChainstateManager& chainman{*testing_setup->m_node.chainman}; + + CBlock block; + const auto pos{WriteBlockToDisk(chainman)}; + + bench.run([&] { + const auto success{chainman.m_blockman.ReadBlockFromDisk(block, pos)}; + assert(success); + }); +} + +static void ReadRawBlockFromDiskTest(benchmark::Bench& bench) +{ + const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; + ChainstateManager& chainman{*testing_setup->m_node.chainman}; + + std::vector block_data; + const auto pos{WriteBlockToDisk(chainman)}; + + bench.run([&] { + const auto success{chainman.m_blockman.ReadRawBlockFromDisk(block_data, pos)}; + assert(success); + }); +} + +BENCHMARK(ReadBlockFromDiskTest, benchmark::PriorityLevel::HIGH); +BENCHMARK(ReadRawBlockFromDiskTest, benchmark::PriorityLevel::HIGH); diff --git a/src/init.cpp b/src/init.cpp index f24101a98cbda..c82565e89c461 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -313,7 +313,7 @@ void Shutdown(NodeContext& node) DumpMempool(*node.mempool, MempoolPath(*node.args)); } - // Drop transactions we were still watching, record fee estimations and Unregister + // Drop transactions we were still watching, record fee estimations and unregister // fee estimator from validation interface. if (node.fee_estimator) { node.fee_estimator->Flush(); diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index aa2b6a5fcf2f2..fe8e12af9602e 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -229,7 +229,7 @@ struct NewMempoolTransactionInfo { * This boolean indicates whether the transaction was added * without enforcing mempool fee limits. */ - const bool m_from_disconnected_block; + const bool m_mempool_limit_bypassed; /* This boolean indicates whether the transaction is part of a package. */ const bool m_submitted_in_package; /* @@ -242,11 +242,11 @@ struct NewMempoolTransactionInfo { explicit NewMempoolTransactionInfo(const CTransactionRef& tx, const CAmount& fee, const int64_t vsize, const unsigned int height, - const bool from_disconnected_block, const bool submitted_in_package, + const bool mempool_limit_bypassed, const bool submitted_in_package, const bool chainstate_is_current, const bool has_no_mempool_parents) : info{tx, fee, vsize, height}, - m_from_disconnected_block{from_disconnected_block}, + m_mempool_limit_bypassed{mempool_limit_bypassed}, m_submitted_in_package{submitted_in_package}, m_chainstate_is_current{chainstate_is_current}, m_has_no_mempool_parents{has_no_mempool_parents} {} diff --git a/src/key.cpp b/src/key.cpp index 0f283ca3e3839..512790252af74 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -369,6 +369,13 @@ ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, c return output; } +CKey GenerateRandomKey(bool compressed) noexcept +{ + CKey key; + key.MakeNewKey(/*fCompressed=*/compressed); + return key; +} + bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const { if (nDepth == std::numeric_limits::max()) return false; out.nDepth = nDepth + 1; @@ -420,8 +427,7 @@ void CExtKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { } bool ECC_InitSanityCheck() { - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); CPubKey pubkey = key.GetPubKey(); return key.VerifyPubKey(pubkey); } diff --git a/src/key.h b/src/key.h index 785059da0218c..9cac716ec86b1 100644 --- a/src/key.h +++ b/src/key.h @@ -205,6 +205,8 @@ class CKey bool initiating) const; }; +CKey GenerateRandomKey(bool compressed = true) noexcept; + struct CExtKey { unsigned char nDepth; unsigned char vchFingerprint[4]; diff --git a/src/net.cpp b/src/net.cpp index 6aa64d195464e..4a689d287400a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -943,13 +944,6 @@ class V2MessageMap const V2MessageMap V2_MESSAGE_MAP; -CKey GenerateRandomKey() noexcept -{ - CKey key; - key.MakeNewKey(/*fCompressed=*/true); - return key; -} - std::vector GenerateRandomGarbage() noexcept { std::vector ret; diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 74c688060d26d..544054863609c 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -612,7 +612,7 @@ void CBlockPolicyEstimator::processTransaction(const NewMempoolTransactionInfo& // - the node is not behind // - the transaction is not dependent on any other transactions in the mempool // - it's not part of a package. - const bool validForFeeEstimation = !tx.m_from_disconnected_block && !tx.m_submitted_in_package && tx.m_chainstate_is_current && tx.m_has_no_mempool_parents; + const bool validForFeeEstimation = !tx.m_mempool_limit_bypassed && !tx.m_submitted_in_package && tx.m_chainstate_is_current && tx.m_has_no_mempool_parents; // Only want to be updating estimates when our blockchain is synced, // otherwise we'll miscalculate how many blocks its taking to get included. diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index a9009948eee41..d44d84af93239 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -162,9 +162,8 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) LOCK(cs_main); tip = m_node.chainman->ActiveChain().Tip(); } - CKey coinbase_key_A, coinbase_key_B; - coinbase_key_A.MakeNewKey(true); - coinbase_key_B.MakeNewKey(true); + CKey coinbase_key_A = GenerateRandomKey(); + CKey coinbase_key_B = GenerateRandomKey(); CScript coinbase_script_pub_key_A = GetScriptForDestination(PKHash(coinbase_key_A.GetPubKey())); CScript coinbase_script_pub_key_B = GetScriptForDestination(PKHash(coinbase_key_B.GetPubKey())); std::vector> chainA, chainB; diff --git a/src/test/compress_tests.cpp b/src/test/compress_tests.cpp index b56629ae40d70..264b47b07cd56 100644 --- a/src/test/compress_tests.cpp +++ b/src/test/compress_tests.cpp @@ -65,8 +65,7 @@ BOOST_AUTO_TEST_CASE(compress_amounts) BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id) { // case CKeyID - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); CPubKey pubkey = key.GetPubKey(); CScript script = CScript() << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG; @@ -101,8 +100,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id) BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id) { - CKey key; - key.MakeNewKey(true); // case compressed PubKeyID + CKey key = GenerateRandomKey(); // case compressed PubKeyID CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // COMPRESSED_PUBLIC_KEY_SIZE (33) BOOST_CHECK_EQUAL(script.size(), 35U); @@ -119,8 +117,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id) BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id) { - CKey key; - key.MakeNewKey(false); // case uncompressed PubKeyID + CKey key = GenerateRandomKey(/*compressed=*/false); // case uncompressed PubKeyID CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // PUBLIC_KEY_SIZE (65) BOOST_CHECK_EQUAL(script.size(), 67U); // 1 char code + 65 char pubkey + OP_CHECKSIG diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 40a1fc80f07df..a4e1947b9f6e3 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -45,12 +45,14 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) } const CTransaction tx{*mtx}; const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx); + const auto tx_submitted_in_package = fuzzed_data_provider.ConsumeBool(); + const auto tx_has_mempool_parents = fuzzed_data_provider.ConsumeBool(); const auto tx_info = NewMempoolTransactionInfo(entry.GetSharedTx(), entry.GetFee(), entry.GetTxSize(), entry.GetHeight(), - /* m_from_disconnected_block */ false, - /* m_submitted_in_package */ false, - /* m_chainstate_is_current */ true, - /* m_has_no_mempool_parents */ fuzzed_data_provider.ConsumeBool()); + /*mempool_limit_bypassed=*/false, + tx_submitted_in_package, + /*chainstate_is_current=*/true, + tx_has_mempool_parents); block_policy_estimator.processTransaction(tx_info); if (fuzzed_data_provider.ConsumeBool()) { (void)block_policy_estimator.removeTx(tx.GetHash()); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 13ec89663ae11..ede73c6895761 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -70,10 +70,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) feeV[j], virtual_size, entry.nHeight, - /* m_from_disconnected_block */ false, - /* m_submitted_in_package */ false, - /* m_chainstate_is_current */ true, - /* m_has_no_mempool_parents */ true)}; + /*mempool_limit_bypassed=*/false, + /*submitted_in_package=*/false, + /*chainstate_is_current=*/true, + /*has_no_mempool_parents=*/true)}; GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); } uint256 hash = tx.GetHash(); @@ -112,6 +112,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } } + // Wait for fee estimator to catch up + SyncWithValidationInterfaceQueue(); + std::vector origFeeEst; // Highest feerate is 10*baseRate and gets in all blocks, // second highest feerate is 9*baseRate and gets in 9/10 blocks = 90%, @@ -168,10 +171,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) feeV[j], virtual_size, entry.nHeight, - /* m_from_disconnected_block */ false, - /* m_submitted_in_package */ false, - /* m_chainstate_is_current */ true, - /* m_has_no_mempool_parents */ true)}; + /*mempool_limit_bypassed=*/false, + /*submitted_in_package=*/false, + /*chainstate_is_current=*/true, + /*has_no_mempool_parents=*/true)}; GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); } uint256 hash = tx.GetHash(); @@ -232,10 +235,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) feeV[j], virtual_size, entry.nHeight, - /* m_from_disconnected_block */ false, - /* m_submitted_in_package */ false, - /* m_chainstate_is_current */ true, - /* m_has_no_mempool_parents */ true)}; + /*mempool_limit_bypassed=*/false, + /*submitted_in_package=*/false, + /*chainstate_is_current=*/true, + /*has_no_mempool_parents=*/true)}; GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); } uint256 hash = tx.GetHash(); diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 58bdb37b7c8c7..29e2d4a569325 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -136,10 +136,8 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success) BOOST_AUTO_TEST_CASE(script_standard_Solver_failure) { - CKey key; - CPubKey pubkey; - key.MakeNewKey(true); - pubkey = key.GetPubKey(); + CKey key = GenerateRandomKey(); + CPubKey pubkey = key.GetPubKey(); CScript s; std::vector > solutions; @@ -192,10 +190,8 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_failure) BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) { - CKey key; - CPubKey pubkey; - key.MakeNewKey(true); - pubkey = key.GetPubKey(); + CKey key = GenerateRandomKey(); + CPubKey pubkey = key.GetPubKey(); CScript s; CTxDestination address; diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 9be5cf0f20837..90abfb036c9a2 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1414,10 +1414,9 @@ sign_multisig(const CScript& scriptPubKey, const CKey& key, const CTransaction& BOOST_FIXTURE_TEST_CASE(script_CHECKMULTISIG12, BasicTestingSetup) { ScriptError err; - CKey key1, key2, key3; - key1.MakeNewKey(true); - key2.MakeNewKey(false); - key3.MakeNewKey(true); + CKey key1 = GenerateRandomKey(); + CKey key2 = GenerateRandomKey(/*compressed=*/false); + CKey key3 = GenerateRandomKey(); CScript scriptPubKey12; scriptPubKey12 << OP_1 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << OP_2 << OP_CHECKMULTISIG; @@ -1444,11 +1443,10 @@ BOOST_FIXTURE_TEST_CASE(script_CHECKMULTISIG12, BasicTestingSetup) BOOST_FIXTURE_TEST_CASE(script_CHECKMULTISIG23, BasicTestingSetup) { ScriptError err; - CKey key1, key2, key3, key4; - key1.MakeNewKey(true); - key2.MakeNewKey(false); - key3.MakeNewKey(true); - key4.MakeNewKey(false); + CKey key1 = GenerateRandomKey(); + CKey key2 = GenerateRandomKey(/*compressed=*/false); + CKey key3 = GenerateRandomKey(); + CKey key4 = GenerateRandomKey(/*compressed=*/false); CScript scriptPubKey23; scriptPubKey23 << OP_2 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << ToByteVector(key3.GetPubKey()) << OP_3 << OP_CHECKMULTISIG; @@ -1533,9 +1531,9 @@ BOOST_FIXTURE_TEST_CASE(script_combineSigs, BasicTestingSetup) FillableSigningProvider keystore; std::vector keys; std::vector pubkeys; - for (int i = 0; i < 3; i++) { - CKey key; - key.MakeNewKey(i % 2 == 1); + for (int i = 0; i < 3; i++) + { + CKey key = GenerateRandomKey(/*compressed=*/i%2 == 1); keys.push_back(key); pubkeys.push_back(key.GetPubKey()); BOOST_CHECK(keystore.AddKey(key)); diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index c0bed50e1d261..2081acdf4da85 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -50,8 +50,7 @@ BOOST_AUTO_TEST_CASE(GetSigOpCount) std::vector keys; for (int i = 0; i < 3; i++) { - CKey k; - k.MakeNewKey(true); + CKey k = GenerateRandomKey(); keys.push_back(k.GetPubKey()); } CScript s2 = GetScriptForMultisig(1, keys); @@ -120,8 +119,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CCoinsView coinsDummy; CCoinsViewCache coins(&coinsDummy); // Create key - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); CPubKey pubkey = key.GetPubKey(); // Default flags const uint32_t flags{SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH}; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 7065f1bcb512d..6fe475390f5b1 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -487,8 +487,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) CMutableTransaction mtx; mtx.nVersion = 1; - CKey key; - key.MakeNewKey(true); // Need to use compressed keys in segwit or the signing will fail + CKey key = GenerateRandomKey(); // Need to use compressed keys in segwit or the signing will fail FillableSigningProvider keystore; BOOST_CHECK(keystore.AddKeyPubKey(key, key.GetPubKey())); CKeyID hash = key.GetPubKey().GetID(); @@ -564,18 +563,16 @@ SignatureData CombineSignatures(const CMutableTransaction& input1, const CMutabl BOOST_AUTO_TEST_CASE(test_witness) { FillableSigningProvider keystore, keystore2; - CKey key1, key2, key3, key1L, key2L; - CPubKey pubkey1, pubkey2, pubkey3, pubkey1L, pubkey2L; - key1.MakeNewKey(true); - key2.MakeNewKey(true); - key3.MakeNewKey(true); - key1L.MakeNewKey(false); - key2L.MakeNewKey(false); - pubkey1 = key1.GetPubKey(); - pubkey2 = key2.GetPubKey(); - pubkey3 = key3.GetPubKey(); - pubkey1L = key1L.GetPubKey(); - pubkey2L = key2L.GetPubKey(); + CKey key1 = GenerateRandomKey(); + CKey key2 = GenerateRandomKey(); + CKey key3 = GenerateRandomKey(); + CKey key1L = GenerateRandomKey(/*compressed=*/false); + CKey key2L = GenerateRandomKey(/*compressed=*/false); + CPubKey pubkey1 = key1.GetPubKey(); + CPubKey pubkey2 = key2.GetPubKey(); + CPubKey pubkey3 = key3.GetPubKey(); + CPubKey pubkey1L = key1L.GetPubKey(); + CPubKey pubkey2L = key2L.GetPubKey(); BOOST_CHECK(keystore.AddKeyPubKey(key1, pubkey1)); BOOST_CHECK(keystore.AddKeyPubKey(key2, pubkey2)); BOOST_CHECK(keystore.AddKeyPubKey(key1L, pubkey1L)); @@ -756,8 +753,7 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) t.vin[0].scriptSig << std::vector(65, 0); t.vout.resize(1); t.vout[0].nValue = 90*CENT; - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); t.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey())); constexpr auto CheckIsStandard = [](const auto& t) { diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 637f92bd0fcdf..f6456526bb054 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -116,8 +116,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) unsigned int initialPoolSize = m_node.mempool->size(); // Parent and Child Package - CKey parent_key; - parent_key.MakeNewKey(true); + CKey parent_key = GenerateRandomKey(); CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey())); auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0, /*input_height=*/0, /*input_signing_key=*/coinbaseKey, @@ -125,8 +124,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) /*output_amount=*/CAmount(49 * COIN), /*submit=*/false); CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); - CKey child_key; - child_key.MakeNewKey(true); + CKey child_key = GenerateRandomKey(); CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey())); auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0, /*input_height=*/101, /*input_signing_key=*/parent_key, @@ -170,11 +168,9 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup) { // The signatures won't be verified so we can just use a placeholder - CKey placeholder_key; - placeholder_key.MakeNewKey(true); + CKey placeholder_key = GenerateRandomKey(); CScript spk = GetScriptForDestination(PKHash(placeholder_key.GetPubKey())); - CKey placeholder_key_2; - placeholder_key_2.MakeNewKey(true); + CKey placeholder_key_2 = GenerateRandomKey(); CScript spk2 = GetScriptForDestination(PKHash(placeholder_key_2.GetPubKey())); // Parent and Child Package @@ -266,8 +262,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) { LOCK(cs_main); unsigned int expected_pool_size = m_node.mempool->size(); - CKey parent_key; - parent_key.MakeNewKey(true); + CKey parent_key = GenerateRandomKey(); CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey())); // Unrelated transactions are not allowed in package submission. @@ -298,8 +293,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) package_parent_child.push_back(tx_parent); package_3gen.push_back(tx_parent); - CKey child_key; - child_key.MakeNewKey(true); + CKey child_key = GenerateRandomKey(); CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey())); auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0, /*input_height=*/101, /*input_signing_key=*/parent_key, @@ -309,8 +303,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) package_parent_child.push_back(tx_child); package_3gen.push_back(tx_child); - CKey grandchild_key; - grandchild_key.MakeNewKey(true); + CKey grandchild_key = GenerateRandomKey(); CScript grandchild_locking_script = GetScriptForDestination(PKHash(grandchild_key.GetPubKey())); auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/tx_child, /*input_vout=*/0, /*input_height=*/101, /*input_signing_key=*/child_key, @@ -434,8 +427,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) witness2.stack.emplace_back(2); witness2.stack.emplace_back(witnessScript.begin(), witnessScript.end()); - CKey child_key; - child_key.MakeNewKey(true); + CKey child_key = GenerateRandomKey(); CScript child_locking_script = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey())); CMutableTransaction mtx_child1; mtx_child1.nVersion = 1; @@ -504,8 +496,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // This tests a potential censorship vector in which an attacker broadcasts a competing package // where a parent's witness is mutated. The honest package should be accepted despite the fact // that we don't allow witness replacement. - CKey grandchild_key; - grandchild_key.MakeNewKey(true); + CKey grandchild_key = GenerateRandomKey(); CScript grandchild_locking_script = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey())); auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/ptx_child2, /*input_vout=*/0, /*input_height=*/0, /*input_signing_key=*/child_key, @@ -595,8 +586,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->m_min_relay_feerate.GetFee(GetVirtualTransactionSize(*ptx_parent3)) <= low_fee_amt); // child spends parent1, parent2, and parent3 - CKey mixed_grandchild_key; - mixed_grandchild_key.MakeNewKey(true); + CKey mixed_grandchild_key = GenerateRandomKey(); CScript mixed_child_spk = GetScriptForDestination(WitnessV0KeyHash(mixed_grandchild_key.GetPubKey())); CMutableTransaction mtx_mixed_child; @@ -648,11 +638,9 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) MockMempoolMinFee(CFeeRate(5000)); LOCK(::cs_main); size_t expected_pool_size = m_node.mempool->size(); - CKey child_key; - child_key.MakeNewKey(true); + CKey child_key = GenerateRandomKey(); CScript parent_spk = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey())); - CKey grandchild_key; - grandchild_key.MakeNewKey(true); + CKey grandchild_key = GenerateRandomKey(); CScript child_spk = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey())); // low-fee parent and high-fee child package diff --git a/src/validationinterface.h b/src/validationinterface.h index e1d6869fab4ea..d9292ae2c9626 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -150,7 +150,7 @@ class CValidationInterface { virtual void BlockConnected(ChainstateRole role, const std::shared_ptr &block, const CBlockIndex *pindex) {} /** * Notifies listeners of a block being disconnected - * Provides the block that was connected. + * Provides the block that was disconnected. * * Called on a background thread. Only called for the active chainstate, since * background chainstates should never disconnect blocks. diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 64e92a44813f7..602302e13f9cb 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1171,8 +1171,7 @@ bool LegacyScriptPubKeyMan::CanGenerateKeys() const CPubKey LegacyScriptPubKeyMan::GenerateNewSeed() { assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); return DeriveNewSeed(key); } diff --git a/src/wallet/test/fuzz/fees.cpp b/src/wallet/test/fuzz/fees.cpp index 2f7892dc0ac9c..c2e785651ad4a 100644 --- a/src/wallet/test/fuzz/fees.cpp +++ b/src/wallet/test/fuzz/fees.cpp @@ -36,6 +36,10 @@ FUZZ_TARGET(wallet_fees, .init = initialize_setup) wallet.SetLastBlockProcessed(chainstate->m_chain.Height(), chainstate->m_chain.Tip()->GetBlockHash()); } + if (fuzzed_data_provider.ConsumeBool()) { + wallet.m_fallback_fee = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; + } + if (fuzzed_data_provider.ConsumeBool()) { wallet.m_discard_rate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; } @@ -58,6 +62,9 @@ FUZZ_TARGET(wallet_fees, .init = initialize_setup) if (fuzzed_data_provider.ConsumeBool()) { coin_control.m_confirm_target = fuzzed_data_provider.ConsumeIntegralInRange(0, 999'000); } + if (fuzzed_data_provider.ConsumeBool()) { + coin_control.m_fee_mode = fuzzed_data_provider.ConsumeBool() ? FeeEstimateMode::CONSERVATIVE : FeeEstimateMode::ECONOMICAL; + } FeeCalculation fee_calculation; FeeCalculation* maybe_fee_calculation{fuzzed_data_provider.ConsumeBool() ? nullptr : &fee_calculation}; diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 95d5c1b9ce855..dfad0e2126973 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -47,8 +47,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) pubkeys[i] = keys[i].GetPubKey(); } - CKey uncompressedKey; - uncompressedKey.MakeNewKey(false); + CKey uncompressedKey = GenerateRandomKey(/*compressed=*/false); CPubKey uncompressedPubkey = uncompressedKey.GetPubKey(); std::unique_ptr& chain = m_node.chain; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index c5bc5c7af7855..bac00c6fade0c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -231,8 +231,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) keys.push_back(key); key.clear(); key.setObject(); - CKey futureKey; - futureKey.MakeNewKey(true); + CKey futureKey = GenerateRandomKey(); key.pushKV("scriptPubKey", HexStr(GetScriptForRawPubKey(futureKey.GetPubKey()))); key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1); key.pushKV("internal", UniValue(true)); @@ -711,8 +710,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) static size_t CalculateNestedKeyhashInputSize(bool use_max_sig) { // Generate ephemeral valid pubkey - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); CPubKey pubkey = key.GetPubKey(); // Generate pubkey hash @@ -796,8 +794,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) context.args = &m_args; context.chain = m_node.chain.get(); auto wallet = TestLoadWallet(context); - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); AddKey(*wallet, key); TestUnloadWallet(std::move(wallet)); @@ -905,8 +902,7 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) context.args = &m_args; context.chain = m_node.chain.get(); auto wallet = TestLoadWallet(context); - CKey key; - key.MakeNewKey(true); + CKey key = GenerateRandomKey(); AddKey(*wallet, key); std::string error; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b6af20103105f..18f0f075ceb3d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3678,8 +3678,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { // Make a seed - CKey seed_key; - seed_key.MakeNewKey(true); + CKey seed_key = GenerateRandomKey(); CPubKey seed = seed_key.GetPubKey(); assert(seed_key.VerifyPubKey(seed));