Merge bitcoin/bitcoin#29015: kernel: Streamline util library

c7376babd1 doc: Clarify distinction between util and common libraries in libraries.md (Ryan Ofsky)
4f74c59334 util: Move util/string.h functions to util namespace (Ryan Ofsky)
4d05d3f3b4 util: add TransactionError includes and namespace declarations (Ryan Ofsky)
680eafdc74 util: move fees.h and error.h to common/messages.h (Ryan Ofsky)
02e62c6c9a common: Add PSBTError enum (Ryan Ofsky)
0d44c44ae3 util: move error.h TransactionError enum to node/types.h (Ryan Ofsky)
9bcce2608d util: move spanparsing.h to script/parsing.h (Ryan Ofsky)
6dd2ad4792 util: move spanparsing.h Split functions to string.h (Ryan Ofsky)
23cc8ddff4 util: move HexStr and HexDigit from util to crypto (TheCharlatan)
6861f954f8 util: move util/message to common/signmessage (Ryan Ofsky)
cc5f29fbea build: move memory_cleanse from util to crypto (Ryan Ofsky)
5b9309420c build: move chainparamsbase from util to common (Ryan Ofsky)
ffa27af24d test: Add check-deps.sh script to check for unexpected library dependencies (Ryan Ofsky)

Pull request description:

  Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically:

  - Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity.
  - Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing.
  - Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of the util library.

  Motivation for this change is that the util library is a dependency of the kernel, and we should remove functionality from util that shouldn't be called by kernel code or kernel applications. These changes should also improve code organization and make functions easier to discover. Some of these same moves are (or were) part of #28690, but did not help with code organization, or made it worse, so it is better to move them and clean them up in the same PR so code only has to change one time.

ACKs for top commit:
  achow101:
    ACK c7376babd1
  TheCharlatan:
    Re-ACK c7376babd1
  hebasto:
    re-ACK c7376babd1.

Tree-SHA512: 5bcef16c1255463b1b69270548711e7ff78ca0dd34e300b95e3ca1ce52ceb34f83d9ddb2839e83800ba36b200de30396e504bbb04fa02c6d0c24a16d06ae523d
This commit is contained in:
Ava Chow
2024-06-12 17:12:54 -04:00
121 changed files with 1023 additions and 526 deletions

View File

@@ -5,6 +5,7 @@
| *libbitcoin_cli* | RPC client functionality used by *bitcoin-cli* executable |
| *libbitcoin_common* | Home for common functionality shared by different executables and libraries. Similar to *libbitcoin_util*, but higher-level (see [Dependencies](#dependencies)). |
| *libbitcoin_consensus* | Stable, backwards-compatible consensus functionality used by *libbitcoin_node* and *libbitcoin_wallet*. |
| *libbitcoin_crypto* | Hardware-optimized functions for data encryption, hashing, message authentication, and key derivation. |
| *libbitcoin_kernel* | Consensus engine and support library used for validation by *libbitcoin_node*. |
| *libbitcoinqt* | GUI functionality used by *bitcoin-qt* and *bitcoin-gui* executables. |
| *libbitcoin_ipc* | IPC functionality used by *bitcoin-node*, *bitcoin-wallet*, *bitcoin-gui* executables to communicate when [`--enable-multiprocess`](multiprocess.md) is used. |
@@ -53,13 +54,18 @@ bitcoin-wallet[bitcoin-wallet]-->libbitcoin_wallet_tool;
libbitcoin_cli-->libbitcoin_util;
libbitcoin_cli-->libbitcoin_common;
libbitcoin_consensus-->libbitcoin_crypto;
libbitcoin_common-->libbitcoin_consensus;
libbitcoin_common-->libbitcoin_crypto;
libbitcoin_common-->libbitcoin_util;
libbitcoin_kernel-->libbitcoin_consensus;
libbitcoin_kernel-->libbitcoin_crypto;
libbitcoin_kernel-->libbitcoin_util;
libbitcoin_node-->libbitcoin_consensus;
libbitcoin_node-->libbitcoin_crypto;
libbitcoin_node-->libbitcoin_kernel;
libbitcoin_node-->libbitcoin_common;
libbitcoin_node-->libbitcoin_util;
@@ -67,7 +73,10 @@ libbitcoin_node-->libbitcoin_util;
libbitcoinqt-->libbitcoin_common;
libbitcoinqt-->libbitcoin_util;
libbitcoin_util-->libbitcoin_crypto;
libbitcoin_wallet-->libbitcoin_common;
libbitcoin_wallet-->libbitcoin_crypto;
libbitcoin_wallet-->libbitcoin_util;
libbitcoin_wallet_tool-->libbitcoin_wallet;
@@ -78,22 +87,23 @@ class bitcoin-qt,bitcoind,bitcoin-cli,bitcoin-wallet bold
```
</td></tr><tr><td>
**Dependency graph**. Arrows show linker symbol dependencies. *Consensus* lib depends on nothing. *Util* lib is depended on by everything. *Kernel* lib depends only on consensus and util.
**Dependency graph**. Arrows show linker symbol dependencies. *Crypto* lib depends on nothing. *Util* lib is depended on by everything. *Kernel* lib depends only on consensus, crypto, and util.
</td></tr></table>
- The graph shows what _linker symbols_ (functions and variables) from each library other libraries can call and reference directly, but it is not a call graph. For example, there is no arrow connecting *libbitcoin_wallet* and *libbitcoin_node* libraries, because these libraries are intended to be modular and not depend on each other's internal implementation details. But wallet code is still able to call node code indirectly through the `interfaces::Chain` abstract class in [`interfaces/chain.h`](../../src/interfaces/chain.h) and node code calls wallet code through the `interfaces::ChainClient` and `interfaces::Chain::Notifications` abstract classes in the same file. In general, defining abstract classes in [`src/interfaces/`](../../src/interfaces/) can be a convenient way of avoiding unwanted direct dependencies or circular dependencies between libraries.
- *libbitcoin_consensus* should be a standalone dependency that any library can depend on, and it should not depend on any other libraries itself.
- *libbitcoin_crypto* should be a standalone dependency that any library can depend on, and it should not depend on any other libraries itself.
- *libbitcoin_util* should also be a standalone dependency that any library can depend on, and it should not depend on other internal libraries.
- *libbitcoin_consensus* should only depend on *libbitcoin_crypto*, and all other libraries besides *libbitcoin_crypto* should be allowed to depend on it.
- *libbitcoin_common* should serve a similar function as *libbitcoin_util* and be a place for miscellaneous code used by various daemon, GUI, and CLI applications and libraries to live. It should not depend on anything other than *libbitcoin_util* and *libbitcoin_consensus*. The boundary between _util_ and _common_ is a little fuzzy but historically _util_ has been used for more generic, lower-level things like parsing hex, and _common_ has been used for bitcoin-specific, higher-level things like parsing base58. The difference between util and common is mostly important because *libbitcoin_kernel* is not supposed to depend on *libbitcoin_common*, only *libbitcoin_util*. In general, if it is ever unclear whether it is better to add code to *util* or *common*, it is probably better to add it to *common* unless it is very generically useful or useful particularly to include in the kernel.
- *libbitcoin_util* should be a standalone dependency that any library can depend on, and it should not depend on other libraries except *libbitcoin_crypto*. It provides basic utilities that fill in gaps in the C++ standard library and provide lightweight abstractions over platform-specific features. Since the util library is distributed with the kernel and is usable by kernel applications, it shouldn't contain functions that external code shouldn't call, like higher level code targetted at the node or wallet. (*libbitcoin_common* is a better place for higher level code, or code that is meant to be used by internal applications only.)
- *libbitcoin_common* is a home for miscellaneous shared code used by different Bitcoin Core applications. It should not depend on anything other than *libbitcoin_util*, *libbitcoin_consensus*, and *libbitcoin_crypto*.
- *libbitcoin_kernel* should only depend on *libbitcoin_util* and *libbitcoin_consensus*.
- *libbitcoin_kernel* should only depend on *libbitcoin_util*, *libbitcoin_consensus*, and *libbitcoin_crypto*.
- The only thing that should depend on *libbitcoin_kernel* internally should be *libbitcoin_node*. GUI and wallet libraries *libbitcoinqt* and *libbitcoin_wallet* in particular should not depend on *libbitcoin_kernel* and the unneeded functionality it would pull in, like block validation. To the extent that GUI and wallet code need scripting and signing functionality, they should be get able it from *libbitcoin_consensus*, *libbitcoin_common*, and *libbitcoin_util*, instead of *libbitcoin_kernel*.
- The only thing that should depend on *libbitcoin_kernel* internally should be *libbitcoin_node*. GUI and wallet libraries *libbitcoinqt* and *libbitcoin_wallet* in particular should not depend on *libbitcoin_kernel* and the unneeded functionality it would pull in, like block validation. To the extent that GUI and wallet code need scripting and signing functionality, they should be get able it from *libbitcoin_consensus*, *libbitcoin_common*, *libbitcoin_crypto*, and *libbitcoin_util*, instead of *libbitcoin_kernel*.
- GUI, node, and wallet code internal implementations should all be independent of each other, and the *libbitcoinqt*, *libbitcoin_node*, *libbitcoin_wallet* libraries should never reference each other's symbols. They should only call each other through [`src/interfaces/`](../../src/interfaces/) abstract interfaces.