IPC plus #100
Closed
IPC plus #100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This causes IPC binaries (bitcoin-node, bitcoin-gui) to be included in releases. The effect on CI is that this causes more depends builds to build IPC binaries, but still the only build running functional tests with them is the i686_multiprocess one. Except for Windows.
The bitcoin-node binary is built on all platforms which have multiprocess enabled, but for functional tests it's only used in CentOS native (depends) job. The next commit will also add a non-depends job.
Additionally update wallet instructions to reflect that sqlite is expected (but can be opted out). The minimum version for Cap'n Proto is the oldest version tested in CI. OpenBSD does not have this package, so recommend building from source for now.
c090cc8619 build: require CapnProto 1.0.1 or better b4120d34ba Merge bitcoin-core/libmultiprocess#192: doc: fix typos 6ecbdcd35a doc: fix typos a11e6905c2 Merge bitcoin-core/libmultiprocess#186: Fix mptest failures in bitcoin CI 6f340a583f doc: fix DrahtBot LLM Linter error c6f7fdf173 type-context: revert client disconnect workaround e09143d2ea proxy-types: fix UndefinedBehaviorSanitizer: null-pointer-use 84b292fcc4 mptest: fix MemorySanitizer: use-of-uninitialized-value fe4a188803 proxy-io: fix race conditions in disconnect callback code d8011c8360 proxy-io: fix race conditions in ProxyClientBase cleanup handler 97e82ce19c doc: Add note about Waiter::m_mutex and interaction with the EventLoop::m_mutex 81d58f5580 refactor: Rename ProxyClient cleanup_it variable 07230f259f refactor: rename ProxyClient<Thread>::m_cleanup_it c0efaa5e8c Merge bitcoin-core/libmultiprocess#187: ci: have bash scripts explicitly opt out of locale dependence. 0d986ff144 mptest: fix race condition in TestSetup constructor d2f6aa2e84 ci: add thread sanitizer job 3a6db38e56 ci: rename configs to .bash 401e0ce1d9 ci: add copyright to bash scripts e956467ae4 ci: export LC_ALL 8954cc0377 Merge bitcoin-core/libmultiprocess#184: Add CI jobs and fix clang-tidy and iwyu errors 757e13a755 ci: add gnu32 cross-compiled 32-bit build 15bf349000 doc: fix typo found by DrahtBot 1a598d5905 clang-tidy: drop 'bitcoin-*' check cbb1e43fdc ci: test libc++ instead of libstdc++ in one job 76313450c2 type-context: disable clang-tidy UndefinedBinaryOperatorResult error 4896e7fe51 proxy-types: fix clang-tidy EnumCastOutOfRange error 060a739269 proxy-types: fix clang-tidy StackAddressEscape error 977d721020 ci: add github actions jobs testing gcc, clang-20, clang-tidy, and iwyu 0d5f1faae5 iwyu: fix add/remove include errors 753d2b10cc util: fix clang-tidy modernize-use-equals-default error ae4f1dc2bb type-number: fix clang-tidy modernize-use-nullptr error 07a741bf69 proxy-types: fix clang-tidy bugprone-use-after-move error 3673114bc9 proxy-types: fix clang-tidy bugprone-use-after-move error 422923f384 proxy-types: fix clang-tidy bugprone-use-after-move error c6784c6ade mpgen: disable clang-tidy misc-no-recursion error c5498aa11b tidy: copy clang-tidy file from bitcoin core 258a617c1e Merge bitcoin-core/libmultiprocess#160: refactor: EventLoop locking cleanups + client disconnect exception 84cf56a0b5 test: Test disconnects during IPC calls 949573da84 Prevent IPC server crash if disconnected during IPC call 0198397580 Merge bitcoin-core/libmultiprocess#179: scripted-diff: Remove copyright year (ranges) ea38392960 Prevent EventLoop async cleanup thread early exit during shutdown 616d9a75d2 doc: Document ProxyClientBase destroy_connection option 56fff76f94 Improve IPC client disconnected exceptions 9b8ed3dc5f refactor: Add clang thread safety annotations to EventLoop 52256e730f refactor: Remove DestructorCatcher and AsyncCallable f24894794a refactor: Drop addClient/removeClient methods 2b830e558e refactor: Use EventLoopRef instead of addClient/removeClient 315ff537fb refactor: Add ProxyContext EventLoop* member 9aaeec3678 proxy-io.h: Add EventLoopRef RAII class handle addClient/removeClient refcounting f58c8d8ba2 proxy-io.h: Add more detailed EventLoop comment 5108445e5d test: Add test coverage for client & server disconnections 59030c68cb Merge bitcoin-core/libmultiprocess#181: type-function.h: Fix CustomBuildField overload 688140b1df test: Add coverage for type-function.h 8b96229da5 type-function.h: Fix CustomBuildField overload fa2ff9a668 scripted-diff: Remove copyright year (ranges) git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: c090cc8619d6c003b86cbf63c1d43e64ff167d78
Use EventLoopRef to avoid reference counting bugs and be more exception safe and deal with removal of addClient/removeClient methods in bitcoin-core/libmultiprocess#160 A test update is also required due to bitcoin-core/libmultiprocess#160 to deal with changed reference count semantics. In IpcPipeTest(), it is now necessary to destroy the client Proxy object instead of just the client Connection object to decrease the event loop reference count and allow the loop to exit so the test does not hang on shutdown.
Currently this code is not called in unit tests. Calling should make it possible to write tests for things like IPC exceptions being thrown during shutdown.
This fixes behavior reported by Antoine Poinsot <darosior@protonmail.com> bitcoin#29409 (comment) where if an IPC client is connected, the node will wait forever for it to disconnect before exiting.
Fix some comments that were referring to previous versions of these methods and did not make sense.
This fixes an error reported by Antoine Poinsot <darosior@protonmail.com> in bitcoin-core/libmultiprocess#123 that does not happen in master, but does happen with bitcoin#10102 applied, where if Ctrl-C is pressed when `bitcoin-node` is started, it is handled by both `bitcoin-node` and `bitcoin-wallet` processes, causing the wallet to shutdown abruptly instead of waiting for the node and shutting down cleanly. This change fixes the problem by having the wallet process print to stdout when it receives a Ctrl-C signal but not otherwise react, letting the node shut everything down cleanly.
This fixes an error reported by Antoine Poinsot <darosior@protonmail.com> in bitcoin-core/libmultiprocess#123 that does not happen in master, but does happen with bitcoin#10102 applied, where if the child bitcoin-wallet process is killed (either by an external signal or by Ctrl-C as reported in the issue) the bitcoin-node process will not shutdown cleanly after that because chain client stop() calls will fail. This change fixes the problem by handling ipc::Exception errors thrown during the stop() calls, and it relies on the fixes to disconnect detection implemented in bitcoin-core/libmultiprocess#160 to work effectively.
c99c8b3 to
1364b3c
Compare
Fixes: ```bash /distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: symbol __divmodti4 from unsupported version GCC_7.0.0(14) /distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: failed IMPORTED_SYMBOLS ``` which is occuring after bitcoin#32750. I can't see any supported distro that is shipping a new enough glibc (2.31), but a GCC older than 7.0.
|
I'll let this CI run finish with 1.0.1 as the minimum. |
ryanofsky
added a commit
to bitcoin-core/libmultiprocess
that referenced
this pull request
Aug 13, 2025
30930df build: require CapnProto 0.7.0 or better (Sjors Provoost) Pull request description: Although 1.0.1. is the oldest version currently covered by Bitcoin Core's extensive CI, Debian Bookwork ships 0.9.2 and #194 introduces test coverage for even older versions. 0.7 has been required since #88. The CI run of Sjors/bitcoin#100 @ [3d55222](https://github.com/Sjors/bitcoin/pull/100/checks?sha=3d552223712eed88d17e5ead1ef7d1ba6fd7e89e) previously checked Bitcoin Core CI against 1.0.1 as the minimum. Lowering the minimum further should not be a problem for that CI. ACKs for top commit: ryanofsky: Code review ACK 30930df. Planning to follow up in #194 to actually test minimum version and error if capnproto version detected is affected by CVE-2022-46149 Tree-SHA512: bed5843973c8ff1f0b2bd93efe7169824c2306097efefaace1752efeb06606df765b68b7ef50c07f5d703010c4d1b324099d6780fa0363e126d34ac1307fba1a
a91a1af to
a12733e
Compare
Sjors
pushed a commit
that referenced
this pull request
Aug 21, 2025
The `-assumevalid` option skips script verification for a specified block and all its ancestors during Initial Block Download. Many new users are surprised when this suddenly slows their node to a halt. This commit adds a log message to clearly indicate when this optimization ends and full validation begins (and vice versa). When using `-assumeutxo`, logging is suppressed for the active assumed-valid chainstate and for the background validation chainstate to avoid the confusing toggles. ------- > cmake -B build && cmake --build build && mkdir -p demo && build/bin/bitcoind -datadir=demo -stopatheight=500 | grep 'signature validation' ``` 2025-08-08T20:59:21Z Disabling signature validations at block #1 (00000000839a8e6886ab5951d76f411475428afc90947ee320161bbf18eb6048). 2025-08-08T20:59:21Z Enabling signature validations at block #100 (000000007bc154e0fa7ea32218a72fe2c1bb9f86cf8c9ebf9a715ed27fdb229a). 2025-08-08T20:59:21Z Disabling signature validations at block bitcoin#200 (000000008f1a7008320c16b8402b7f11e82951f44ca2663caf6860ab2eeef320). 2025-08-08T20:59:21Z Enabling signature validations at block bitcoin#300 (0000000062b69e4a2c3312a5782d7798b0711e9ebac065cd5d19f946439f8609). ```
Sjors
pushed a commit
that referenced
this pull request
Aug 21, 2025
…hange fab2980 assumevalid: log every script validation state change (Lőrinc) Pull request description: The `-assumevalid` option skips script verification for a specified block and all its ancestors during Initial Block Download. Many new [users are surprised](bitcoin#32832) when this suddenly slows their node to a halt. This commit adds a log message to clearly indicate when this optimization ends and full validation begins (and vice versa). <details> <summary>Testing instructions</summary> The behavior can easily be tested by adding this before the new log: ```C++ // TODO hack to enable/disable script checks based on block height for testing purposes if (pindex->nHeight < 100) fScriptChecks = false; else if (pindex->nHeight < 200) fScriptChecks = true; else if (pindex->nHeight < 300) fScriptChecks = false; else if (pindex->nHeight < 400) fScriptChecks = true; ``` and exercise the new code with: ```bash cmake -B build && cmake --build build && mkdir -p demo && build/bin/bitcoind -datadir=demo -stopatheight=500 | grep 'signature validation' ``` showing something like: * Disabling signature validations at block #1 (00000000839a8e6886ab5951d76f411475428afc90947ee320161bbf18eb6048). * Enabling signature validations at block #100 (000000007bc154e0fa7ea32218a72fe2c1bb9f86cf8c9ebf9a715ed27fdb229a). * Disabling signature validations at block bitcoin#200 (000000008f1a7008320c16b8402b7f11e82951f44ca2663caf6860ab2eeef320). * Enabling signature validations at block bitcoin#300 (0000000062b69e4a2c3312a5782d7798b0711e9ebac065cd5d19f946439f8609). </details> ACKs for top commit: achow101: ACK fab2980 ajtowns: crACK fab2980 davidgumberg: untested crACK bitcoin@fab2980 Tree-SHA512: e90b66f7423b639356daace476942ce83e65e70466544394cbe2f15738bdbf716163eaf590c64c5448f9b41aeeaafe3342c48c6a7a478678a70b0310ca94e11d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This tests bitcoin#31802 plus: