[pull] master from bitcoin:master#1672
Merged
Merged
Conversation
`OpenNetworkConnection()` supports overriding the proxy to use for connecting. However when v2 connection is attempted and it fails a v1 connection is tried without that proxy. Store the override proxy in `CNode` and pass it to `CConnman::m_reconnections` to be used for v1 retries.
Private broadcast connections use either Tor or I2P, which require a proxy intrinsically or IPv4 or IPv6 which must use a proxy in the context of private broadcast to avoid leaking the originator's IP address. Add a safety check to guard against future mistakes. Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
Extract the part of `p2p_private_broadcast.py` that configures and starts the SOCKS5 server into a reusable function and put it into `test_framework/socks5.py`. Use bind port 0 to let the OS pick an available port instead of hackishly assuming that `p2p_port(N)` is available where N is more than the number of the nodes the test uses.
Extract the part of `p2p_private_broadcast.py` that starts listening on a `P2PConnection` object (or its children classes) and put it into `test_framework/p2p.py`.
Extract the part of `p2p_private_broadcast.py` that fills a given node's addrman and put it into `test_framework/test_framework.py`.
This way callers will not forget to set it.
This case invoked bitcoin-cli via raw subprocess.run() without the valgrind wrapper (bypassing valgrind) and without -datadir (so it read the default datadir's bitcoin.conf, e.g. ~/.bitcoin, and failed whenever that real config was unusable). Build the command like the rest of the framework: prepend binaries.valgrind_cmd and pass the node's -datadir so the check runs under valgrind and depends only on the build's IPC support, not the host environment.
zmq seems to be an alias for pyzmq, and the project page, https://pypi.org/project/zmq/, states "You are probably looking for pyzmq.". So switch to pyzmq, which is what we document, and use in all other jobs.
So that interface_zmq.py is run.
Speeds of 1MB/s and 15 minute cached docker image pulls during builds are not uncommon. Warp runners provide a local GitHub Actions cache protocol proxy for Docker layer cache traffic. Point BuildKit's gha cache backend at that proxy on Warp runners so cached image layers do not have to be fetched from GitHub's slower cache service. Add a default for provider so other users (e.g. qa-assets) don't have to update this unless they use custome runners.
8290198 ci: use Warp cache for Docker layers (will) Pull request description: We switched to GH cache recently, but the performance is beyond terrible. We have already switched the main cache actions over to warp, and this completes the transfer with the docker buildkit cache to the Warp endpoint. ACKs for top commit: m3dwards: ACK 8290198 Tree-SHA512: caee053598c5082b1f0cc52a6abfe6314ac2d5d840238480f1706f55a892f08ca533ac3d5d4f9f20a84958df1c9100ae00518686072515a1011be994619b4df9
4a6d145 ci: add pyzmq to msan job (fanquake) c21b58e ci: use pyzmq over zmq (fanquake) Pull request description: `zmq` seems to be an alias for `pyzmq`, and the project page, https://pypi.org/project/zmq/, states "You are probably looking for pyzmq.". So switch to `pyzmq`, which is what we document, and use in all other jobs. Also add `pyzmq` to native MSAN, so that `interface_zmq.py` is run. ACKs for top commit: sedited: ACK 4a6d145 Tree-SHA512: 86b96f6b2dca032bf7460113173472c8384834b1f38d961dff1ee325cce75410cc02062da798484f12fb28f676611df7ea406ee8385f9324b298c036826881f9
7735c13 test: run bitcoin-cli -ipcconnect check under valgrind with -datadir (Michael Dietz) Pull request description: This case invokes bitcoin-cli via raw subprocess.run() without -datadir, so it reads the default datadir's bitcoin.conf (e.g. ~/.bitcoin) and fails whenever that real config is unusable. Pass the node's -datadir so the check reads the test's own bitcoin.conf and depends only on the build's IPC support, not the host environment. ACKs for top commit: maflcko: lgtm ACK 7735c13 sedited: ACK 7735c13 Tree-SHA512: 5264433e2cd1747b9b9b4437b80f1849b5fa01620dd958c34c124925270dfca85c3809186285bf1dd70a8358552e1ebb589ec84093e11bb0d66cfe10f04dde81
…nections bf0d257 net: un-default the OpenNetworkConnection()'s proxy_override argument (Eugene Siegel) 5a3756d test: add a regression test for private broadcast v1 retries (Vasil Dimov) ab35a02 test: make reusable filling of a node's addrman (Vasil Dimov) 2333be9 test: make reusable starting a standalone P2P listener (Vasil Dimov) 2ffa81f test: make reusable SOCKS5 server starting (Vasil Dimov) 32d072a doc: add release notes for #35319 (Vasil Dimov) d01b461 net: ensure no direct private broadcast connections (Vasil Dimov) fd230f9 net: use the proxy if overriden when doing v2->v1 reconnections (Vasil Dimov) Pull request description: This PR includes #35319 and on top of that adds a regression functional test. The functional test exercises the relevant code paths without modifying non-test code. To do that it does: * Add a bunch of IPv4 addresses to the node's addrman (they will be added without P2P_V2 flag). * Get them to report P2P_V2 in their service flags and connect to each one, so that the flags in addrman are updated to contain P2P_V2. * Get one successful connection to a Tor peer (.onion) so that bitcoind assumes the configured Tor proxy works and is indeed a proxy to the Tor network. This will make it open private broadcast connections also to IPv4 addresses via that proxy. * Start some private broadcast connections. * Remember the destination IPv4 address of the first connection and get it to fail the v2 transport. * Wait for a subsequent connection also through the Tor proxy to the same IPv4 and expect it to be v1, i.e. the v2->v1 downgrade retry. The test fails without the fix - the v1 retry never arrives to the Tor proxy. And passes with the fix. The fix is in the first commit here and in #35319, can remove it by `git show fd230f9 | git apply -R`. ACKs for top commit: Crypt-iQ: reACK bf0d257 andrewtoth: ACK bf0d257 instagibbs: ACK bf0d257 sedited: utACK bf0d257 Tree-SHA512: 11e89be36577199e0312e5e63efeac04e295faaba1cf1c13a30e683d35f473c8dbb419d1897b0333c2e993c10637adecafcf90fe08c812065c793cbc903744c9
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.
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 : )