[pull] master from bitcoin:master#149
Merged
pull[bot] merged 10 commits intoiAMX11:masterfrom Apr 15, 2025
Merged
Conversation
This works around a valgrind false-positive.
This was preventing the (hidden) waitfornewblock, waitforblock and waitforblockheight methods from being used in the GUI. The check was added in d6a5dc4 when these RPC methods were first introduced. They could have been dropped when dca9231 refactored these methods to use waitTipChanged(), which already checks for shutdown. Making this change now simplifies the next commit.
The waitTipChanged() now returns nullopt if the node is shutting down. Previously it would return the last known tip during shutdown, but this creates an ambiguous circumstance in the scenario where the node is started and quickly shutdown, before notifications().TipBlock() is set. The getblocktemplate, waitfornewblock and waitforblockheight RPC are updated to handle this. Existing behavior is preserved. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
- return null on shutdown instead of the last tip - ignore timeout value node initialization This allows consumers of BlockTemplate to safely assume that a tip is connected, instead of having to account for startup and early shutdown scenarios.
Move the comparison to hashWatchedChain inside the while loop. Although this early return prevents the GetTransactionsUpdated() call in cases where the tip updates, it's only done to improve readability. The check itself is very cheap (although a more useful check might not be). Also add code comments.
fa21f83 ci: Use G++ in valgrind tasks (MarcoFalke) fabd05b refactor: Fix net_processing iwyu includes (MarcoFalke) fa1622d refactor: Make node_id a const& in RemoveBlockRequest (MarcoFalke) Pull request description: Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7 It is possible to work around this bug by pulling at least one value from the stack. For example, by making `from_peer` a `const` reference. Alternatively, by replacing `auto [node_id, list_it]` with `const auto& [node_id, list_it]`, which is done here. I think this workaround is acceptable, because it does not look like valgrind can trivially fix this. The alternative would be to add a (temporary?) suppression. Fixes #27741 Also, fix iwyu includes, while touching this module. Also, switch the CI valgrind scripts to use G++. ACKs for top commit: achow101: ACK fa21f83 TheCharlatan: ACK fa21f83 darosior: utACK fa21f83 ryanofsky: Code review ACK fa21f83. Code changes all look good but I'm a little confused about purpose of the third commit, so left a question about that Tree-SHA512: 7b92cdafd525a5ac53ae2c1a7a92e599bc9b5fd5d315a694b493cd5079ac323d884393b57aa18581b7789247a588c9a27d47698de25b340bc76fc9f1dd1850b4
…tdown during long poll and wait methods 05117e6 rpc: clarify longpoll behavior (Sjors Provoost) 5315278 Have createNewBlock() wait for a tip (Sjors Provoost) 64a2795 rpc: handle shutdown during long poll and wait methods (Sjors Provoost) a3bf433 rpc: drop unneeded IsRPCRunning() guards (Sjors Provoost) f9cf8bd Handle negative timeout for waitTipChanged() (Sjors Provoost) Pull request description: This PR prevents Mining interface methods from sometimes crashing when called during startup before a tip is connected. It also makes other improvements like making more RPC methods usable from the GUI. Specifically this PR: - Adds an `Assume` check to disallow passing negative timeout values to `Mining::waitTipChanged` - Makes `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods usable from the GUI when `-server=1` is not set. - Changes `Mining::waitTipChanged` to return `optional<BlockRef>` instead of `BlockRef` and return `nullopt` instead of crashing if there is a timeout or if the node is shut down before a tip is connected. - Changes `Mining::waitTipChanged` to not time out before a tip is connected, so it is convenient and safe to call during startup, and only returns `nullopt` on early shutdowns. - Changes `Mining::createNewBlock` to block and wait for a tip to be connected if it is called on startup instead of crashing. Also documents that it will return null on early shutdowns. This allows `waitNext()` (added in #31283) to safely assume `TipBlock()` isn't `null`, not even during a scenario of early shutdown. Finally this PR clarifies long poll behaviour, mostly by adding code comments, but also through an early `break`. ACKs for top commit: achow101: ACK 05117e6 ryanofsky: Code review ACK 05117e6, just updated a commit message since last review TheCharlatan: ACK 05117e6 vasild: ACK 05117e6 Tree-SHA512: 277c285a6e73dfff88fd379298190b264254996f98b93c91c062986ab35c2aa5e1fbfec4cd71d7b29dc2d68e33f252b5cfc501345f54939d6bd78599b71fec04
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.1)
Can you help keep this open source service alive? 💖 Please sponsor : )