Uh oh!
There was an error while loading. Please reload this page.
[pull] master from bitcoin:master - #1832
Merged
Merged
Conversation
BIP324 specifies the 13-byte long-form message type encoding as "an ASCII message type (as in the v1 P2P protocol)", but V2Transport::GetMessageType() accepted bytes up to 0x7F, while for V1 it only accepts printable ASCII (0x20-0x7E). This changes V2 to match V1 on it and add test coverage.
The default ephemeral port range on OpenBSD (1024-49151) overlaps with the test framework's static port range starting at TEST_RUNNER_PORT_MIN, the same way FreeBSD's does (see #34346). Extend `set_ephemeral_port_range()` to OpenBSD. The socket option and its values are identical to FreeBSD's, so only the platform check changes.
This clarifies the initial index sync thread is blocked.
Without the drain, a BlockConnected event may execute during shutdown and lead to memory races.
Without the drain, a BlockConnected event may execute during shutdown and lead to memory races.
Fixes#35632 by allowing both outcomes of a race condition. The server behavior is unchanged: in response to a malformed request we send an error code and disconnect. The issue is that sometimes on Windows the RST is caught by the platform and the receive buffer is discarded before the Python client can process it with recv(). We can also be much more polite to misbehaving clients by implementing SO_LINGER as suggested in #35780 but that will require more review.
…tdown fab80e8 test: Avoid unsafe memory race in baseindex_no_commit_ahead_of_flush (MarcoFalke) fa0f14e test: Avoid unsafe memory race in index_reorg_crash shutdown (MarcoFalke) faf9c8e test: Clarify index.GetSummary().synced state in index_reorg_crash (MarcoFalke) Pull request description: Currently, the `index_reorg_crash` test may rarely crash due to UB in sanitizers like TSan or ASan. This is perfectly fine, because it is just a rare test-only issue. However, fix it nonetheless by adding a missing drain of the unused in-flight events. Also, add a small check about the synced state while touching this test. ACKs for top commit: arejula27: ACK fab80e8 furszy: ACK fab80e8 Tree-SHA512: 4423e420421aa37d8b59e053f44c455fafb676102866bdf23988cf72f3d3f265b996bd953583ea8208f1534defb0e16b13ef08644be97e61959dc777a2918e5a
cc577de net: align v2 message type validation with v1 range (Bruno Garcia) Pull request description: BIP324 specifies the 13-byte long-form message type encoding as "an ASCII message type (as in the v1 P2P protocol)", but V2Transport::GetMessageType() accepted bytes up to 0x7F, while for V1 it only accepts printable ASCII (0x20-0x7E). This changes V2 to match V1 on it and add test coverage. ACKs for top commit: nervana21: tACK cc577de ajtowns: utACK cc577de w0xlt: ACK cc577de sedited: ACK cc577de Tree-SHA512: 8c97ee20df2311949bbe9655c7e04507c4b47d3b18766aa6ae51691d0870f8a5c25ea54d74c9afb797754572d057b4240533da6bf3c2e0435f3cb32c5fb1c3af
…llocation 59ebf55 qa: Use IP_PORTRANGE_HIGH on OpenBSD for dynamic port allocation (Hennadii Stepanov) Pull request description: The default ephemeral port range on OpenBSD (1024-49151) overlaps with the test framework's static port range starting at `TEST_RUNNER_PORT_MIN`, the same way FreeBSD's does (see #34346). Extend `set_ephemeral_port_range()` to OpenBSD. The socket option and its values are identical to FreeBSD's, so only the platform check changes. ACKs for top commit: maflcko: lgtm ACK 59ebf55 theStack: utACK 59ebf55 Tree-SHA512: 680235cf3e1799361796c0ff36d5f19bf74f79393057dbd7b38b0e92a7df3af669873c66ca1e82f1c78999c20351663b8960990f3f209af89ee18ce0773eb7de
e85e279 rpc: detail x-bitcoin-unit in openrpc help (will) Pull request description: Addresses review comment about clarifying this field: #36131 (comment) ACKs for top commit: sedited: ACK e85e279 Tree-SHA512: 7fd0bef8a5d37cd9d2778463b2193c58ec7cced1aa790a0a5807ef093bd51e729b6c8880e1c12af78293eb68abe791f4ae6e96e0457ce85be717e3e776f5406d
a51df9b test: tolerate race condition in interface_http.py (Matthew Zipkin) Pull request description: Fixes#35632 by allowing both outcomes of a race condition. The server behavior is unchanged: in response to a malformed request we send an error code and disconnect. The issue is that sometimes on Windows the RST is caught by the platform and the receive buffer is discarded before the Python client can process it with recv(). We can also be much more polite to misbehaving clients by implementing a lingering close using SO_LINGER as suggested in #35780 but that will require more review. The exact error in #35632 is hard to produce reliably but there are a few close options for reviewers. I tested this on windows native building with MSVC. In both of these cases the patch from this PR caught the error and passed the test. **RemoteDisconnected: Remote end closed connection without response** ```diff diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 9bb8986..62324d3fea 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -1072,7 +1072,7 @@ std::unique_ptr<HTTPRequest> HTTPRemoteClient::TryReadRequest(const std::shared_ e.what()); // We failed to read a complete request from the buffer - WriteNoStoreErrorReply(*client->m_req, HTTP_BAD_REQUEST); + // WriteNoStoreErrorReply(*client->m_req, HTTP_BAD_REQUEST); client->m_disconnect = true; return nullptr; } ``` **ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host** ```diff diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 9bb8986..be52acb874 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -1154,6 +1154,11 @@ bool HTTPRemoteClient::MaybeDisconnect(std::chrono::time_point<SteadyClock> now, "Disconnecting HTTP client %s (id=%llu)", m_origin, m_id); + auto sock{GetSock()}; + linger opt{}; + opt.l_onoff = 1; // enable SO_LINGER + opt.l_linger = 0; // zero timeout + sock->SetSockOpt(SOL_SOCKET, SO_LINGER, &opt, sizeof(opt)); return true; } ``` ACKs for top commit: jeanpablojp: re-ACK a51df9b winterrdog: tACK a51df9b janb84: re ACK a51df9b hodlinator: re-ACK a51df9b sedited: ACK a51df9b Tree-SHA512: a6244581b2b51af647452e0dc8cd09cdc8d975dee6a0dc8b8064cad136023dad68b4af987303bced91a662bf5fae22871ea718a6a8e68024158a9aef6c5855ef
5ba9af6 ci: pass LIBCXX_INCLUDE_TESTS=OFF to LLVM build (fanquake) feb3bd4 clang-tidy: remove some performance-* options (fanquake) b4bd12d ci: use LLVM 23 in *san, fuzz, *cross jobs (fanquake) Pull request description: LLVM 23.1.0 was recently released, switch to using it across sanitizer, fuzzer and cross-compilation jobs. ACKs for top commit: hebasto: ACK 5ba9af6, I have reviewed the code and it looks OK. willcl-ark: ACK 5ba9af6 Tree-SHA512: 4d203bf1ec6100a21d9a185a37365d358859bbde79f44f93d2e4f5e3c9686f57ca06d6c73da7423eb234dad5b9501d7a09029a430d23b2bcf8aba95f2d88e66d
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 freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 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 : )