ffff4a293ad878494e12f8f00108cc99ee2b713e bench: Update span-serialize comment (MarcoFalke)
fa4d6ec97bcb1790a7cd4363a13fda7c80c3dd90 refactor: Avoid false-positive gcc warning (MarcoFalke)
fa942332b40c97375af0722f32f7575bca3af819 scripted-diff: Bump copyright headers after std::span changes (MarcoFalke)
fa0c6b7179c062b7ca92d120455ce02a9f4e9e19 refactor: Remove unused Span alias (MarcoFalke)
fade0b5e5e6e80e3da1ab6448b6212244bafa5d3 scripted-diff: Use std::span over Span (MarcoFalke)
fadccc26c03db00a2be3f703aa7e5eec4312bd2e refactor: Make Span an alias of std::span (MarcoFalke)
fa27e36717ec18d64b7ff7bba71b8f0c202ba31d test: Fix broken span_tests (MarcoFalke)
fadf02ef8bf96ad5b3b8e34fd425b31b555f4371 refactor: Return std::span from MakeUCharSpan (MarcoFalke)
fa720b94be17fa9e7c91188710e6a04939ceab11 refactor: Return std::span from MakeByteSpan (MarcoFalke)
Pull request description:
`Span` has some issues:
* It does not support fixed-size spans, which are available through `std::span`.
* It is confusing to have it available and in use at the same time with `std::span`.
* It does not obey the standard library iterator build hardening flags. See https://github.com/bitcoin/bitcoin/issues/31272 for a discussion. For example, this allows to catch issues like the one fixed in commit fabeca3458b38a3d8930cb0cbc866388c3f120f1.
Both types are type-safe and can even implicitly convert into each other in most contexts.
However, exclusively using `std::span` seems less confusing, so do it here with a scripted-diff.
ACKs for top commit:
l0rinc:
reACK ffff4a293ad878494e12f8f00108cc99ee2b713e
theuni:
ACK ffff4a293ad878494e12f8f00108cc99ee2b713e.
Tree-SHA512: 9cc2f1f43551e2c07cc09f38b1f27d11e57e9e9bc0c6138c8fddd0cef54b91acd8b14711205ff949be874294a121910d0aceffe0e8914c4cff07f1e0e87ad5b8
Historically, the headers have been bumped some time after a file has
been touched. Do it now to avoid having to touch them again in the
future for that reason.
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's;( 20[0-2][0-9])(-20[0-2][0-9])? The Bitcoin Core developers;\1-present The Bitcoin Core developers;g' $( git show --pretty="" --name-only HEAD~1 )
-END VERIFY SCRIPT-
The use of e.g. `std::underlying_type_t<T>` replaces the older `typename std::underlying_type<T>::type`.
The `_t` helper alias template (such as `std::underlying_type_t<T>`) introduced in C++14 offers a cleaner and more concise way to extract the type directly.
See https://en.cppreference.com/w/cpp/types/underlying_type for details.
-BEGIN VERIFY SCRIPT-
sed -i -E 's/(typename )?(std::[a-z_]+)(<[^<>]+>)::type\b/\2_t\3/g' $(git grep -l '::type' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
-END VERIFY SCRIPT-
`OBJECT` libraries have historically exhibited poor support for various
features, both in the past and now. For example, see one of the latest
issues:
- https://gitlab.kitware.com/cmake/cmake/-/issues/24058
Furthermore, CMake maintainers have acknowledged:
> In general, however, where there is a choice, static libraries will
> typically be the more convenient choice in CMake projects.
This change:
1. Converts the `bitcoin_clientversion` library from an `OBJECT` library
to a `STATIC` library.
2. Removes an obsolete workaround.
12fa9511b5fba18e83f88b7b831906595bcf2116 build: simplify dependency graph (Cory Fields)
c4e498300c7e6b23dc464eca75a2bc9f86270dab build: avoid unnecessary dependencies on generated headers (Cory Fields)
Pull request description:
These changes speed up my build (default config/options/targets) by roughly 10%. I suspect the difference may be more significant in other build configs.
Before:
> $ time cmake --build build -j24
> real3m26.932s
After:
> $ time cmake --build build -j24
> real3m7.556s
Generally they allow for jobservers (either `make -jX` or `ninja`) to be better utilized. This can be verified using `top` while building and looking at the number of compiles running at any given time before/after these changes. Before, it's easy to observe periods of stalling when only one or two compiles are happening. After these changes, the compiler process count should mostly match the number of jobs given (`-jX`) until it falls off at the end.
---
The first commit sets [DEPENDS_EXPLICIT_ONLY](https://cmake.org/cmake/help/latest/command/add_custom_command.html#command:add_custom_command) for commands which generate our test header files. Without this option, `test_bitcoin`'s generated headers won't be built until all of its other dependencies have been built. This introduces a significant stall in the build, though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.
Example from a generated `build.ninja`:
Before:
> \# Custom command for src/test/data/base58_encode_decode.json.h
>
> build src/test/data/base58_encode_decode.json.h | ${cmake_ninja_workdir}src/test/data/base58_encode_decode.json.h: CUSTOM_COMMAND /home/cory/dev/bitcoin/src/test/data/base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake || libcrc32c.a libcrc32c_sse42.a libleveldb.a libminisketch.a minisketch_clmul src/bitcoin_clientversion src/crypto/libbitcoin_crypto.a src/crypto/libbitcoin_crypto_avx2.a src/crypto/libbitcoin_crypto_sse41.a src/crypto/libbitcoin_crypto_x86_shani.a src/generate_build_info src/libbitcoin_cli.a src/libbitcoin_common.a src/libbitcoin_consensus.a src/libbitcoin_node.a src/secp256k1/src/libsecp256k1.a src/secp256k1/src/secp256k1_precomputed src/test/util/libtest_util.a src/univalue/libunivalue.a src/util/libbitcoin_util.a src/wallet/libbitcoin_wallet.a src/zmq/libbitcoin_zmq.a
After:
> \# Custom command for src/test/data/base58_encode_decode.json.h
>
> build src/test/data/base58_encode_decode.json.h | ${cmake_ninja_workdir}src/test/data/base58_encode_decode.json.h: CUSTOM_COMMAND /home/cory/dev/bitcoin/src/test/data/base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake
---
The second commit is more significant. It sets [CMAKE_OPTIMIZE_DEPENDENCIES](https://cmake.org/cmake/help/latest/prop_tgt/OPTIMIZE_DEPENDENCIES.html) globally, which allows the objects of static libs to be built in parallel when one lib depends on the other. This can be set as a per-lib property, ~but I don't see any need for that as we don't currently have any edge-cases where this wouldn't be ok. If those should arise, we could always disable on a per-lib basis~.
Edit: turns out this triggers an [upstream bug](https://gitlab.kitware.com/cmake/cmake/-/issues/24058), which I guess can be considered an edge-case until fixed in CMake. I've added 2 per-lib opt-outs as a result.
Example:
Before:
> \# Link the static library src/libbitcoin_cli.a
>
> build src/libbitcoin_cli.a: CXX_STATIC_LIBRARY_LINKER__bitcoin_cli_RelWithDebInfo src/CMakeFiles/bitcoin_cli.dir/compat/stdin.cpp.o src/CMakeFiles/bitcoin_cli.dir/rpc/client.cpp.o || src/univalue/libunivalue.a
After:
> \# Link the static library src/libbitcoin_cli.a
>
> build src/libbitcoin_cli.a: CXX_STATIC_LIBRARY_LINKER__bitcoin_cli_RelWithDebInfo src/CMakeFiles/bitcoin_cli.dir/compat/stdin.cpp.o src/CMakeFiles/bitcoin_cli.dir/rpc/client.cpp.o
>
ACKs for top commit:
l0rinc:
utACK 12fa9511b5fba18e83f88b7b831906595bcf2116
hebasto:
ACK 12fa9511b5fba18e83f88b7b831906595bcf2116.
Tree-SHA512: f85f507e70cdc06acd07542161d9f9b8edf9ba866f08c8ef17aaaed770fa11530a27521c4413456d863463a6e77d4d6983fa623a64e17bbd602c2bc70aacc112
0f716f28896c6edfcd4e2a2b25c88f478a029c7b qa: cover PROTOCOL_ERROR variant in PCP unit tests (Antoine Poinsot)
fc700bb47fd8b6ac58f612b932aef0e361686cc3 test: Add tests for PCP and NATPMP implementations (laanwj)
caf952103317a7fa8bd2bceb35d4e8ace5968906 net: Use mockable steady clock in PCP implementation (laanwj)
03648321ecb704b69e47eed7e3df6a779aee8f11 util: Add mockable steady_clock (laanwj)
ab1d3ece026844e682676673b8a461964a5b3ce4 net: Add optional length checking to CService::SetSockAddr (laanwj)
Pull request description:
Add a NodeSteadyClock, a steady_clock that can be mocked with millisecond precision. Use this in the PCP implementation.
Then add a mock for a simple scriptable UDP server,, which is used to test various code paths (including successful mappings, timeouts and errors) in the PCP and NATPMP implementations.
Includes "net: Add optional length checking to CService::SetSockAddr" from #31014 as a prerequisite.
ACKs for top commit:
darosior:
re-ACK 0f716f28896c6edfcd4e2a2b25c88f478a029c7b
i-am-yuvi:
Concept ACK 0f716f28896c6edfcd4e2a2b25c88f478a029c7b
achow101:
ACK 0f716f28896c6edfcd4e2a2b25c88f478a029c7b
Tree-SHA512: 6f91b24e6fe46a3fded7a13972efd77c98e6ef235f8898e4ae44068c5df32d1cdabb22cb66c351b338dc98cb2073b624e43607a28107f4999302bfbe7a138229
2a92702bafca5c78b270a9502a22cb9deac02cfc init: Use size_t consistently for cache sizes (TheCharlatan)
65cde3621dbb9ac7d210d4926e7601c4adf5f498 kernel: Move default cache constants to caches (TheCharlatan)
8826cae285490439dc1f19b25fa70b2b9e62dfe8 kernel: Move non-kernel db cache size constants (TheCharlatan)
e758b26b85da27ef44f3d2c924f3f08e8c1f4fdf kernel: Move kernel-specific cache size options to kernel (TheCharlatan)
d5e2c4a4097c799433cfc5367c61568fad2c784e fuzz: Add fuzz test for checked and saturating add and left shift (TheCharlatan)
c03a2795a8e044d17835bbf03de0c64dc7b41da8 util: Add integer left shift helpers (TheCharlatan)
8bd5f8a38ce903c05606841ebed1902398cb0b14 [refactor] init: Simplify coinsdb cache calculation (TheCharlatan)
5db7d4d3d28bd1269a09955b4695135c86c4827d doc: Correct docstring describing max block tree db cache (TheCharlatan)
Pull request description:
Carrying non-kernel related fields in the cache sizes for the indexes is confusing for kernel library users. The cache sizes are set currently with magic numbers in bitcoin-chainstate. The comments for the cache size calculations are not completely clear. The constants for the cache sizes are also currently in `txdb.h`, which is not an ideal place for holding all cache size related constants.
Solve these things by moving the kernel-specific cache size fields to their own struct and moving the constants to either the node or the kernel cache sizes.
This slightly changes the way the cache is allocated if (and only if) the txindex and/or blockfilterindex is used. Since they are now given precedence over the block tree db cache, this results in a bit less cache being allocated to the block tree db, coinsdb and coins caches. The effect is negligible though, i.e. cache sizes with default dbcache reported through the logs are:
master:
```
Cache configuration:
* Using 2.0 MiB for block index database
* Using 56.0 MiB for transaction index database
* Using 49.0 MiB for basic block filter index database
* Using 8.0 MiB for chain state database
* Using 335.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
this PR:
```
Cache configuration:
* Using 2.0 MiB for block index database
* Using 56.2 MiB for transaction index database
* Using 49.2 MiB for basic block filter index database
* Using 8.0 MiB for chain state database
* Using 334.5 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
---
This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587).
ACKs for top commit:
stickies-v:
re-ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc
ryanofsky:
Code review ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc. Changes since last review are fixing size options to use size_t instead of int64_t again, simplifying CheckedLeftShift more, and making other minor suggested cleanups
hodlinator:
re-ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc
Tree-SHA512: 98376eaa0660b1b8c096a5ce1f3e7c8c30e7cd6644de36856c2d3e573108cfc9473c93ebb3952b7881047b5ae6c85c5b096e6726f30f35be58b98eca07c8c785
The helpers are used in the following commits to increase the safety of
conversions during cache size calculations.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: stickies-v <stickies-v@protonmail.com>
This brings the format types closer to the standard library types:
* FormatStringCheck corresponds to std::basic_format_string, with
compile-time checks done via ConstevalFormatString
* RuntimeFormat corresponds to std::runtime_format, with no compile-time
checks done.
Also, it documents where no compile-time checks are done.
a96b84cb1b76e65a639e62f0224f534f89858c18 fuzz: Abort when calling system time without setting mock time (marcofleon)
ff21870e20b2391b684cc50fdd6879805055d6a1 fuzz: Add SetMockTime() to necessary targets (marcofleon)
Pull request description:
This PR expands the `CheckGlobals` utility that was introduced in https://github.com/bitcoin/bitcoin/pull/31486 and should help with fuzz stability (https://github.com/bitcoin/bitcoin/issues/29018).
System time shouldn't be used when running a fuzz test, as it is likely to introduce instability (non-determinism). This PR identifies and fixes the targets that were calling system time without setting mock time at the start of an iteration.
Removing`SetMockTime()` from any one of these targets should result in a crash and a message describing the issue.
ACKs for top commit:
achow101:
ACK a96b84cb1b76e65a639e62f0224f534f89858c18
dergoegge:
Code review ACK a96b84cb1b76e65a639e62f0224f534f89858c18
brunoerg:
crACK a96b84cb1b76e65a639e62f0224f534f89858c18
Tree-SHA512: e093a9feb8a397954f7b1416dfa8790b2733f09d5ac51fda5a9d225a55ebd8f99135aa52bdf5ab531653ad1a3739c4ca2b5349c1d989bb4b009ec8eaad684f7d
The current code does not have a bug, but is implicitly casting -1 to
65535 and the sanitizer has no way to know whether we intend that or
not.
```
FUZZ=bitset src/test/fuzz/fuzz /tmp/fuz
error: implicit conversion from type 'int' of value -1 (32-bit, signed)
to type 'value_type' (aka 'unsigned short') changed the value to 65535
(16-bit, unsigned)
Base64: Qv7bX/8=
```
fa3e074304780549b1e7972217930e34fa55f59a refactor: Tidy fixups (MarcoFalke)
fa72646f2b197810a324cb0544d9a1fac37d3f9c move-only: Detail_CheckNumFormatSpecifiers and G_TRANSLATION_FUN (MarcoFalke)
faff8403f0aac3b5ec26d3c7fc98240f879f9906 refactor: Pick translated string after format (MarcoFalke)
Pull request description:
The changes are required for https://github.com/bitcoin/bitcoin/pull/31061, however they also make sense on their own. For example, they are fixing up an `inline namespace`, which lead to compile errors otherwise (can be tested by observing the compile error after reverting the changes to `src/util/strencodings.h`). Also, a unit test comment is fixed.
ACKs for top commit:
ryanofsky:
Code review ACK fa3e074304780549b1e7972217930e34fa55f59a. Nice changes! These should allow related PRs to be simpler.
l0rinc:
ACK fa3e074304780549b1e7972217930e34fa55f59a
hodlinator:
cr-ACK fa3e074304780549b1e7972217930e34fa55f59a
Tree-SHA512: 37371181a348610442186b5fbb7a6032d0caf70aae566002ad60be329a3131a2b89f28f6c51e10872079f987986925dc8c0611bde639057bee4f572d2b9ba92a
This is required for a future commit. Can be reviewed via the git
options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
Also move util::detail::Hex to a proper namespace instead of an inline
namespace so it doesn't conflict with the new util::detail namespace, and
won't create other problems for callers trying to use the inline namespaces.
Also fix a misleading comment in util_string_tests.cpp.
Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
fe39acf88ff552bfc4a502c99774375b91824bb1 tinyformat: Add compile-time checking for literal format strings (Ryan Ofsky)
184f34f2d0fa2e56ad594966b2b99ff4cf840d95 util: Support dynamic width & precision in ConstevalFormatString (Ryan Ofsky)
Pull request description:
Add compile-time checking for literal format strings passed to `strprintf` and `tfm::format` to make sure the right number of format arguments are passed.
There is still no compile-time checking if non-literal `std::string` or `bilingual_str` format strings are passed, but this is improved in other PRs:
- [#31061](https://github.com/bitcoin/bitcoin/pull/31061) implements compile-time checking for translated strings
- [#31072](https://github.com/bitcoin/bitcoin/pull/31072) increases compile-time checking by using literal strings as format strings, instead of `std::string` and `bilingual_str`
- [#31149](https://github.com/bitcoin/bitcoin/pull/31149) may drop the `std::string` overload for `strprintf` to [require](https://github.com/bitcoin/bitcoin/pull/31149#issuecomment-2444579999) compile-time checking
ACKs for top commit:
maflcko:
re-ACK fe39acf88ff552bfc4a502c99774375b91824bb1 🕐
l0rinc:
ACK fe39acf88ff552bfc4a502c99774375b91824bb1
hodlinator:
re-ACK fe39acf88ff552bfc4a502c99774375b91824bb1
Tree-SHA512: f1ddef0c96b9468c5ffe31b42dc19f1922583dd14f2e180b618d992c98614c5cc7db9f9cd917ef503f833bbc7dbec78e4489d0035416dce6840837e1d66d87cb
0de3e96e333090548a43e5e870c4cb8941d6baf1 tracing: use bitcoind pid in bcc tracing examples (0xb10c)
411c6cfc6c2e488e407f057b646730e63806ed8a tracing: only prepare tracepoint args if attached (0xb10c)
d524c1ec06643208c189089089e84f6e1cd0abad tracing: dedup TRACE macros & rename to TRACEPOINT (0xb10c)
Pull request description:
Currently, if the tracepoints are compiled (e.g. in depends and release builds), we always prepare the tracepoint arguments regardless of the tracepoints being used or not. We made sure that the argument preparation is as cheap as possible, but we can almost completely eliminate any overhead for users not interested in the tracepoints (the vast majority), by gating the tracepoint argument preparation with an `if(something is attached to this tracepoint)`. To achieve this, we use the optional semaphore feature provided by SystemTap.
The first commit simplifies and deduplicates our tracepoint macros from 13 TRACEx macros to a single TRACEPOINT macro. This makes them easier to use and also avoids more duplicate macro definitions in the second commit.
The Linux tracing tools I'm aware of (bcc, bpftrace, libbpf, and systemtap) all support the semaphore gating feature. Thus, all existing tracepoints and their argument preparation is gated in the second commit. For details, please refer to the commit messages and the updated documentation in `doc/tracing.md`.
Also adding unit tests that include all tracepoint macros to make sure there are no compiler problems with them (e.g. some varadiac extension not supported).
Reviewers might want to check:
- Do the tracepoints still work for you? Do the examples in `contrib/tracing/` run on your system (as bpftrace frequently breaks on every new version, please test master too if it should't work for you)? Do the CI interface tests still pass?
- Is the new documentation clear?
- The `TRACEPOINT_SEMAPHORE(event, context)` macros places global variables in our global namespace. Is this something we strictly want to avoid or maybe move to all `TRACEPOINT_SEMAPHORE`s to a separate .cpp file or even namespace? I like having the `TRACEPOINT_SEMAPHORE()` in same file as the `TRACEPOINT()`, but open for suggestion on alternative approaches.
- Are newly added tracepoints in the unit tests visible when using `readelf -n build/src/test/test_bitcoin`? You can run the new unit tests with `./build/src/test/test_bitcoin --run_test=util_trace_tests* --log_level=all`.
<details><summary>Two of the added unit tests demonstrate that we are only processing the tracepoint arguments when attached by having a test-failure condition in the tracepoint argument preparation. The following bpftrace script can be used to demonstrate that the tests do indeed fail when attached to the tracepoints.</summary>
`fail_tests.bt`:
```c
#!/usr/bin/env bpftrace
usdt:./build/src/test/test_bitcoin:test:check_if_attached {
printf("the 'check_if_attached' test should have failed\n");
}
usdt:./build/src/test/test_bitcoin:test:expensive_section {
printf("the 'expensive_section' test should have failed\n");
}
```
Run the unit tests with `./build/src/test/test_bitcoin` and start `bpftrace fail_tests.bt -p $(pidof test_bitcoin)` in a separate terminal. The unit tests should fail with:
```
Running 594 test cases...
test/util_trace_tests.cpp(31): error: in "util_trace_tests/test_tracepoint_check_if_attached": check false has failed
test/util_trace_tests.cpp(51): error: in "util_trace_tests/test_tracepoint_manual_tracepoint_active_check": check false has failed
*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
```
</details>
These links might provide more contextual information for reviewers:
- [How SystemTap Userspace Probes Work by eklitzke](https://eklitzke.org/how-sytemtap-userspace-probes-work) (actually an example on Bitcoin Core; mentions that with semaphores "the overhead for an untraced process is effectively zero.")
- [libbpf comment on USDT semaphore handling](1596a09b5d/src/usdt.c (L83-L92)) (can recommend the whole comment for background on how the tracepoints and tracing tools work together)
- https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation#Semaphore_Handling
ACKs for top commit:
willcl-ark:
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
laanwj:
re-ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
jb55:
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
vasild:
ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1
Tree-SHA512: 0e5e0dc5e0353beaf5c446e4be03d447e64228b1be71ee9972fde1d6fac3fac71a9d73c6ce4fa68975f87db2b2bf6eee2009921a2a145e24d83a475d007a559b
This is needed in the next commit to add compile-time checking to strprintf
calls, because bitcoin-cli.cpp uses dynamic width in many format strings.
This change is easiest to review ignoring whitespace.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Before this commit, we would always prepare tracepoint arguments
regardless of the tracepoint being used or not. While we already made
sure not to include expensive arguments in our tracepoints, this
commit introduces gating to make sure the arguments are only prepared
if the tracepoints are actually used. This is a win-win improvement
to our tracing framework. For users not interested in tracing, the
overhead is reduced to a cheap 'greater than 0' compare. As the
semaphore-gating technique used here is available in bpftrace, bcc,
and libbpf, users interested in tracing don't have to change their
tracing scripts while profiting from potential future tracepoints
passing slightly more expensive arguments. An example are mempool
tracepoints that pass serialized transactions. We've avoided the
serialization in the past as it was too expensive.
Under the hood, the semaphore-gating works by placing a 2-byte
semaphore in the '.probes' ELF section. The address of the semaphore
is contained in the ELF note providing the tracepoint information
(`readelf -n ./src/bitcoind | grep NT_STAPSDT`). Tracing toolkits
like bpftrace, bcc, and libbpf increase the semaphore at the address
upon attaching to the tracepoint. We only prepare the arguments and
reach the tracepoint if the semaphore is greater than zero. The
semaphore is decreased when detaching from the tracepoint.
This also extends the "Adding a new tracepoint" documentation to
include information about the semaphores and updated step-by-step
instructions on how to add a new tracepoint.
fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd util: Treat Assume as Assert when evaluating at compile-time (MarcoFalke)
Pull request description:
There is no downside or cost of treating an `Assume` at compile-time as an `Assert` and it may even help to find bugs while compiling without `ABORT_ON_FAILED_ASSUME`.
This is also required for https://github.com/bitcoin/bitcoin/pull/31093
ACKs for top commit:
dergoegge:
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
brunoerg:
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
marcofleon:
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
Tree-SHA512: 17604403f841343a6d5b6e5d777e1760d38e0c27dc1fd4479e3741894fba40cdb1fb659cf24519a51d051bd5884a75992d1227ec9fa40fbf53bc619fbfb304ad
0b3ec8c59b2b6d598531cd4e688eb276e597c825 clusterlin: remove Cluster type (Pieter Wuille)
1c24c625105cd62518d2eee7e95c3b1c74ea9923 clusterlin: merge two DepGraph fuzz tests into simulation test (Pieter Wuille)
0606e66fdbb914f984433d8950f0c32b5fb871f3 clusterlin: add DepGraph::RemoveTransactions and support for holes in DepGraph (Pieter Wuille)
75b5d42419ed1286fc9557bc97ec5bac3f9be837 clusterlin: make DepGraph::AddDependency support multiple dependencies at once (Pieter Wuille)
abf50649d13018bf40c5803730a03053737efeee clusterlin: simplify DepGraphFormatter::Ser (Pieter Wuille)
eaab55ffc8102140086297877747b7fa07b419eb clusterlin: rework DepGraphFormatter::Unser (Pieter Wuille)
5901cf7100a75c8131223e23b6c90e0e93611eae clusterlin: abstract out DepGraph::GetReduced{Parents,Children} (Pieter Wuille)
Pull request description:
Part of cluster mempool: #30289
This adds:
* `DepGraph::AddDependencies` to add 0 or more dependencies to a single transaction at once (identical to calling `DepGraph::AddDependency` once for each, but more efficient).
* `DepGraph::RemoveTransactions` to remove 0 or more transactions from a depgraph.
* `DepGraph::GetReducedParents` (and `DepGraph::GetReducedChildren`) to get the (reduced) direct parents and children of a transaction in a depgraph.
After which, the `Cluster` type is removed.
This is the result of fleshing out the design for the "intermediate layer" ("TxGraph", no PR yet) between the cluster linearization layer and the mempool layer. My earlier thinking was that TxGraph would store `Cluster` objects (vectors of pairs of `FeeFrac`s and sets of parents), and convert them to `DepGraph` on the fly whenever needed. However, after more consideration, it seems better to have TxGraph store `DepGraph` objects, and manipulate them directly without constantly re-creating them. This requires `DepGraph` to have some additional functionality.
The bulk of the complexity here is the addition of `DepGraph::RemoveTransactions`, which leaves the remaining transactions' positions within the `DepGraph` untouched (we want existing identifiers to remain valid), so this implies that graphs can now have "holes" (positions that are unused, but followed by positions that are used). To enable that, an extension of the fuzz/test serialization format `DepGraphFormatter` is included to deal with such holes.
ACKs for top commit:
sdaftuar:
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825
instagibbs:
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825
ismaelsadeeq:
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825
glozow:
ACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825, reviewed range-diff from aab53ddcd8fcbc3c0be0da9383f8e06abe5badda and `clusterlin_depgraph_sim`
Tree-SHA512: a804b7f26d544c5cb0847322e235c810525cb0607737be6116c3156d582da3ba3352af8ea48e74eed5268f9c3eca63b30181d01b23a6dd0be1b99191f81cceb0
This commits introduces support in DepGraph for the transaction positions to be
non-continuous. Specifically, it adds:
* DepGraph::RemoveTransactions which removes 0 or more positions from a DepGraph.
* DepGraph::Positions() to get a set of which positions are in use.
* DepGraph::PositionRange() to get the highest used position in a DepGraph + 1.
In addition, it extends the DepGraphFormatter format to support holes in a
compatible way (it serializes non-holey DepGraphs identically to the old code,
and deserializes them the same way)
This adds an utility header with two functions that will be needed for
PCP, `QueryDefaultGateway` and `GetLocalAddresses`.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
a9964c04447745435747d9cc557165c43902783b doc: Updating docs from autotools to cmake (kevkevinpal)
Pull request description:
A bit of a followup from https://github.com/bitcoin/bitcoin/pull/30840
- In this change the documentation where we refer to the `./configure` script which is now gone and have converted the configure params to use the `cmake` equivalent.
ACKs for top commit:
maflcko:
ACK a9964c04447745435747d9cc557165c43902783b
jonatack:
utACK a9964c04447745435747d9cc557165c43902783b
jarolrod:
ACK a9964c04447745435747d9cc557165c43902783b
tdb3:
ACK a9964c04447745435747d9cc557165c43902783b
pablomartin4btc:
re-ACK a9964c04447745435747d9cc557165c43902783b
Tree-SHA512: f7ed20b8ad61f028c0d242b9cc70650d8da63057d4a8f7da88f0117a8d3241c5fe8fcf19d56ec82088160b9fee9b175fe9f64e5a845260d3696dc7e94bfdd0bd
replaced --enable-debug with -DCMAKE_BUILD_TYPE=Debug in developer-notes
replaced --enable-multiprocess with -DWITH_MULTIPROCESS=ON
replaced --disable-zmq with -DWITH_ZMQ=OFF
a240e150e837b5a95ed19765a2e8b7c5b6013f35 streams: remove AutoFile::Get() entirely (Pieter Wuille)
e624a9bef16b6335fd119c10698352b59bf2930a streams: cache file position within AutoFile (Pieter Wuille)
Pull request description:
Fixes#30833.
Instead of relying on frequent `ftell` calls (which appear to cause a significant slowdown on some systems) in XOR-enabled `AutoFile`s, cache the file position within `AutoFile` itself.
ACKs for top commit:
achow101:
ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35
davidgumberg:
untested reACK a240e150e8
theStack:
Code-review ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35
Tree-SHA512: fd3681edc018afaf955dc7a41a0c953ca80d46c1129e3c5b306c87c95aae93b2fe7b900794eb8b6f10491f9211645e7939918a28838295e6873eb226fca7006f
41051290ab3b6c36312cec26a27f787cea9961b4 cmake: Ignore build subdirectories within source directory (Hennadii Stepanov)
6ce50fd9d0ae6850d54bf883e7a7c1bcb6912c5c doc: Update for CMake-based build system (Hennadii Stepanov)
9730288a0cd3f33021ef00fb2d95e5216d10ab61 ci: Migrate CI scripts to CMake (Hennadii Stepanov)
c360837ca5c91c9878ae8088bb5482e96fd87c96 cmake, lint: Adjust `lint_includes_build_config` (Hennadii Stepanov)
3885441ee0d35a40904995ede68120fea471dde7 cmake: Add presets for native Windows builds (Hennadii Stepanov)
7681746b20dd58e7d3e6d2852f07fb876383a133 cmake: Add vcpkg manifest file (Hennadii Stepanov)
8b6f1c4353836bae6aa683cbc65251165bd031ba cmake: Add `Coverage` and `CoverageFuzz` scripts (Hennadii Stepanov)
65bdbc1ff23b0a817f4d9a4682e6f630c9bbdd59 cmake: Add `docs` build target (Hennadii Stepanov)
fb75ebbc33557ddd56f505100ad3631a0028eb86 cmake: Add compiler diagnostic flags (Hennadii Stepanov)
e821f0a37a026fa0480c7f6f6c938da7c77e0d52 cmake: Migrate Guix build scripts to CMake (Hennadii Stepanov)
747adb6ffe9b06d476fc5eaebbaf9a62b91a78c5 cmake: Add `Maintenance` module (Hennadii Stepanov)
1f60b30df0cb58a7381a1bfbd6d34f002232e862 cmake: Add `APPEND_{CPP,C,CXX,LD}FLAGS` cache variables (Hennadii Stepanov)
2b43c45b13ad00cfd9928a03da8a480977724cb1 cmake: Add `AddWindowsResources` module (Hennadii Stepanov)
973a3b0c5dcbf6b3fd155b2dda4c2e94a0b0ee5f cmake: Implement `install` build target (Hennadii Stepanov)
84ac35cfd4dfa6f235f6e5a00b571846358f45ce cmake: Add cross-compiling support (Hennadii Stepanov)
0d01c228a7d39bb4918b5cb9f6db25cb8c30567a build: Generate `toolchain.cmake` in depends (Hennadii Stepanov)
91a799247dc5e4627e6b2f221669c8ff9238bc8d depends: Add host-specific `cmake_system_version` variables (Hennadii Stepanov)
9b31209b4caaa02b3044acd2375a7f595cdbd520 depends: Rename `cmake_system` -> `cmake_system_name` (Hennadii Stepanov)
4a5208a81d5bfeef270c64d48dce3444d6d03511 Revert "build, qt: Do not install *.prl files" (Hennadii Stepanov)
6522af62af1c3a6e2525bfffdb2295751b6fa49b depends: Amend handling flags environment variables (Hennadii Stepanov)
90cec4d251a541adfc5953e24dc01840a8cb4af2 cmake: Add `MULTIPROCESS` option (Hennadii Stepanov)
bb1a450dcb111746869547c8b538b5d2472cf8e6 cmake: Build `bitcoin-chainstate` executable (Hennadii Stepanov)
aed38ea58cbde068fe12b5299b246b4e3649a09c cmake: Build `bitcoinkernel` library (Hennadii Stepanov)
975d67369b8f3a33a21fd7618c299c0ec138292c cmake: Build `test_bitcoin-qt` executable (Hennadii Stepanov)
10fcc668a3430b72eaf7effc83f00cedeb27c7dc cmake: Add `WITH_DBUS` option (Hennadii Stepanov)
5bb5a4bc75a523e30eab561763927252ce105c4d cmake: Add `libqrencode` optional package support (Hennadii Stepanov)
57a6e2ef4abbfd2b12ee6489366bc6609bead263 cmake: Build `bitcoin-qt` executable (Hennadii Stepanov)
30f642952cb5bf39479bdbe467b3950f0d09324a cmake: Add `WERROR` option (Hennadii Stepanov)
c98d4a4c341e524348d0342e145d439816a44c83 cmake: Add `REDUCE_EXPORTS` option (Hennadii Stepanov)
a01cb6e63ff3940f0773b37e2fe1e148f17acad9 cmake: Add `HARDENING` option (Hennadii Stepanov)
a8a2e364acf55bbe18404ab21f852d52257bcb6d cmake: Add Python-based tests (Hennadii Stepanov)
3d853795707c5a1828dcd09c1f68bb07dee472cd cmake: Add fuzzing options (Hennadii Stepanov)
908530e312a3d4561f9c1feeb2a76ce899f21c68 cmake: Add `SANITIZERS` option (Hennadii Stepanov)
8bb0e85631e7c1bee16e136454b2466776be1d65 cmake: Build `bench_bitcoin` executable (Hennadii Stepanov)
801735163a81650619a6c9587e8f1df9ee182694 cmake: Add external signer support (Hennadii Stepanov)
353e0c9e9679864a777e17c1bb7c6ba8b6eac96d cmake: Add `systemtap-sdt` optional package support (Hennadii Stepanov)
d2fda82b4954f4af7e7d340cf42b9cb34d96cde1 cmake: Add `libzmq` optional package support (Hennadii Stepanov)
ae7b39a0e106d798b6e9cc03ee783d9081e41480 cmake: Add `libminiupnpc` optional package support (Hennadii Stepanov)
6480e1dcdb03f43ce3d0aad96b8668d017d11750 cmake: Add `libnatpmp` optional package support (Hennadii Stepanov)
e73e9304a11af65f9b086460ff349f9f700709ce cmake: Build `bitcoin-util` executable (Hennadii Stepanov)
027c6d7caa0355c35b00f2689eddccc3d1227aef cmake: Build `bitcoin-tx` executable (Hennadii Stepanov)
d10c5c34c3d899db8bcff47ac8c6ba396a6da4b6 cmake: Add wallet functionality (Hennadii Stepanov)
ab2e99b0d95714e16a7d1a1313d7da938b0485cb cmake: Create test suite for `ctest` (Hennadii Stepanov)
959370bd76d30ced34208db45fb4fd097fbad31b cmake: Build `test_bitcoin` executable (Hennadii Stepanov)
b27bf9700dbbfa9a0243815f78c8b62abe78d8bc cmake: Build `bitcoin-cli` executable (Hennadii Stepanov)
a9813df826c885b1609e55a83d87cd9cbc90adfd cmake: Build `bitcoind` executable (Hennadii Stepanov)
97829ce2d5a8dc3b0307b5d57c6686b96b7cf850 cmake: Add `FindLibevent` module (Hennadii Stepanov)
3118e40c6157c8ab9e264518d1065d2b0fc07795 cmake: Build `bitcoin_consensus` library (Hennadii Stepanov)
809a2f192903145f88f30bc10d3cf1ab9ed06881 cmake: Build `bitcoin_util` static library (Hennadii Stepanov)
0a9a521a704ca8a27124c1498a86e87ad46d4c34 cmake: Build `bitcoin_crypto` library (Hennadii Stepanov)
958971f476a29cb5bb76f3ae80ae968317ca1930 cmake: Build `univalue` static library (Hennadii Stepanov)
752747fda801f2c0ecce91c96bcc9ef93e27462b cmake: Generate `obj/build.h` header (Hennadii Stepanov)
1f0a78edf3cd2c24236ac512acf420eb9ed86ab3 cmake: Build `minisketch` static library (Hennadii Stepanov)
12bfbc81540f037c95e7796ae0b9f05ce3fb1b4a cmake: Build `leveldb` static library (Hennadii Stepanov)
51985c5304dfc52bd45f505b3115989637d79ff5 cmake: Build `crc32c` static library (Hennadii Stepanov)
db7a198f29c62c5f762eaa25d1d83c57e2f1e211 cmake: Build `secp256k1` subtree (Hennadii Stepanov)
dbb7ed14e8562439238eec70b202c50f172e3def cmake: Add `ccache` support (Hennadii Stepanov)
cedfdf6c72535d0797a271c6bb9d84c4b406a8ea cmake: Redefine/adjust per-configuration flags (Hennadii Stepanov)
b6b5e732c8b49a2cc14f34ac72b2189389c6b27d cmake: Add global compiler and linker flags (Hennadii Stepanov)
f98327931bd0b5d90678ddd1770e9862266b396e cmake: Add `TryAppendLinkerFlag` module (Hennadii Stepanov)
4a0af29697b62d32af6f60d3ec70cd2ed4d7243c cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)
35cffc497d8db3cf3eee35c1513e3435558f056b cmake: Add POSIX threads support (Hennadii Stepanov)
fd72d00ffe34c84e292b305f6797201040d31a72 cmake: Add position independent code support (Hennadii Stepanov)
07069e2bb0bbdacf16cf34efd3a33390de030217 cmake: Add introspection module (Hennadii Stepanov)
27d687fc1f6aceaed7725e1e904a093ead68d6e6 cmake: Add `config/bitcoin-config.h` support (Hennadii Stepanov)
fe5cdace5ffba46fb7981efb816621962d3873e3 cmake: Print compiler and linker flags in summary (Hennadii Stepanov)
70683884c5fd78dbf7816434464e6511b9d4e486 cmake: Introduce interface libraries to encapsulate common flags (Hennadii Stepanov)
a2317e27b7fb86df4e32cd1674c06e09cb808248 cmake: Add root `CMakeLists.txt` file (Hennadii Stepanov)
Pull request description:
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
ML announcement: https://groups.google.com/g/bitcoindev/c/hgKkfQWzrTo
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of contributors, including (in alphabetical order):
- [**achow101**](https://github.com/achow101)
- [**fanquake**](https://github.com/fanquake)
- [**maflcko**](https://github.com/maflcko)
- [**m3dwards**](https://github.com/m3dwards)
- [**pablomartin4btc**](https://github.com/pablomartin4btc)
- [**real-or-random**](https://github.com/real-or-random)
- [**ryanofsky**](https://github.com/ryanofsky)
- [**sipsorcery**](https://github.com/sipsorcery)
- [**TheCharlatan**](https://github.com/TheCharlatan)
- [**theStack**](https://github.com/theStack)
- [**theuni**](https://github.com/theuni)
- [**vasild**](https://github.com/vasild)
Reviewing in a separate staging repo was suggested in https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431798320.
The accompanying changes to the OSS-Fuzz project are available in https://github.com/hebasto/oss-fuzz/pull/8.
Please refer to the [build options parity table](https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e). The "auto" value is no longer available; non-default values must be specified explicitly. Additionally, the new default values have been chosen to suit the everyday build experience for the majority of developers.
System requirements for using the CMake-based build system:
- CMake >= 3.22 (if not available in your system's repository, it can be downloaded from https://cmake.org/download/)
- a build tool of your choice:
- any Make (GNU Make is no longer a requirement); GNU Make is still required to build depends
- Ninja (https://ninja-build.org/)
- MSBuild
- Xcode
A note for Windows users: The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager).
---
We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR's goal may be ignored.
Thank you all for your understanding.
ACKs for top commit:
maflcko:
review ACK 41051290ab3b6c36312cec26a27f787cea9961b4 🐥
sipsorcery:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4.
vasild:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4
TheCharlatan:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4
pablomartin4btc:
tACK 41051290ab3b6c36312cec26a27f787cea9961b4
i-am-yuvi:
tACK [`4105129`](41051290ab)
theuni:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4.
fanquake:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4
Tree-SHA512: 6c1445054436c6c00ad63bfa0f19d64091a2b25c9bd694f85bf2218ac358ffb774d6c000685b3ca1e9b50401babed989fa2a0694b774c211d226bfd1944c9b39
