[pull] master from bitcoin:master#1588
Merged
pull[bot] merged 15 commits intoAll-Blockchains:masterfrom Apr 9, 2026
Merged
Conversation
This is undocumented and unspecified Boost behavior that happens to work as intended for now, but could break at any point in the future. See the boost discussion here: https://groups.google.com/g/boost-list/c/So4i8JXneJ0 It also complicates a potential replacement of Boost::signals2.
…uilds The current code forward-declares boost::signals2, which avoids the need for these includes. An upcoming commit will (temporarily) include boost headers directly instead. A follow-up commit will then replace boost with an internal signals implementation, which will allow this commit to be reverted.
For now, including btcsignals.h simply includes boost's signals. A follow-up commit will replace the implementation.
This eases the transition to a replacement signals implementation
The next commit will add a real implementation in this namespace.
These tests are compatible with boost::signals2 as well as the replacement implementation that will be introduced in the next commit. This is intended to demonstrate some equivalency between the implementations.
This re-implements the tiny portion of boost::signals2 that we currently use. It is enough to be useful as a generic multicast callback mechanism.
These were necessary to work around unnecessary constraints that have been fixed in the (upcoming) boost::signals2 version 1.91. Our implementation's constraints match those of that version.
…n-node builds" This reverts commit 3df1a14. This was only necessary as an interim workaround.
The real includes were only needed temporarily while supporting btcsignals as an alias for boost::signals2.
In both of these cases, boost was only needed for signals and not for multi_index/test.
The documented example is no longer relevant, so remove it rather than updating it to mention btcsignals.
This simplifies the implementation and eliminates an unnecessary shared_ptr.
Suggested by Marco Falke.
<Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
…ation 242b0eb btcsignals: use a single shared_ptr for liveness and callback (Cory Fields) b12f43a signals: remove boost::signals2 from depends and vcpkg (Cory Fields) a4b1607 signals: remove boost::signals2 mentions in linters and docs (Cory Fields) 375397e signals: remove boost includes where possible (Cory Fields) 091736a signals: re-add forward-declares to interface headers (Cory Fields) 9958f4f Revert "signals: Temporarily add boost headers to bitcoind and bitcoin-node builds" (Cory Fields) 34eabd7 signals: remove boost compatibility guards (Cory Fields) e60a0b9 signals: Add a simplified boost-compatible implementation (Cory Fields) 63c68e2 signals: add signals tests (Cory Fields) edc2978 signals: use an alias for the boost::signals2 namespace (Cory Fields) 9ade392 signals: remove forward-declare for signals (Cory Fields) 037e58b signals: use forwarding header for boost signals (Cory Fields) 2150153 signals: Temporarily add boost headers to bitcoind and bitcoin-node builds (Cory Fields) fd5e9d9 signals: Use a lambda to avoid connecting a signal to another signal (Cory Fields) Pull request description: This drops our dependency on `boost::signals2`, leaving `boost::multi_index` as the only remaining boost dependency for bitcoind. `boost::signals2` is a complex beast, but we only use a small portion of it. Namely: it's a way for multiple subscribers to connect to the same event, and the ability to later disconnect individual subscribers from that event. `btcsignals` adheres to the subset of the `boost::signals2` API that we currently use, and thus is a drop-in replacement. Rather than implementing a complex `slot` tracking class that we never used anyway (and which was much more useful in the days before std::function existed), callbacks are simply wrapped directly in `std::function`s. The new tests work with either `boost::signals2` or the new `btcsignals` implementation. Reviewers can verify functional equivalency by running the tests in the commit that introduces them against `boost::signals2`, then again with `btcsignals`. The majority of the commits in this PR are preparation and cleanup. Once `boost::signals2` is no longer needed, it is removed from depends. Additionally, a few CMake targets no longer need boost includes as they were previously only required for signals. I think this is actually pretty straightforward to review. I kept things simple, including keeping types unmovable/uncopyable where possible rather than trying to define those semantics. In doing so, the new implementation has even fewer type requirements than boost, which I believe is due to a boost bug. I've opened a PR upstream for that to attempt to maintain parity between the implementations. See individual commits for more details. Closes #26442. ACKs for top commit: fjahr: Code review ACK 242b0eb maflcko: re-review ACK 242b0eb 🎯 w0xlt: reACK 242b0eb Tree-SHA512: 9a472afa4f655624fa44493774a63b57509ad30fb61bf1d89b6d0b52000cb9a1409a5b8d515a99c76e0b26b2437c30508206c29a7dd44ea96eb1979d572cd4d4
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )