aboutsummaryrefslogtreecommitdiff
path: root/src/net.cpp
AgeCommit message (Collapse)Author
2021-04-01[net] Changes to RunInactivityChecksJohn Newbery
- rename to ShouldRunInactivityChecks (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r576394790) - take optional time now (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r575895661) - call from within InactivityChecks (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r575894665) - update comment (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r575894343) - change ordering of inequality (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r574925129)
2021-03-31Merge #21486: build: link against -lsocket if required for *ifaddrsfanquake
4783115fd4cccb46a7f8c592b34fa7c094c29410 net: add ifaddrs.h include (fanquake) 879215e665a9f348c8d3fa92701c34065bc86a69 build: check if -lsocket is required with *ifaddrs (fanquake) 87deac66aa747481e6f34fc80599e1e490de3ea0 rand: only try and use freeifaddrs if available (fanquake) Pull request description: Fixes #21485 by linking against `-lsocket` when it's required for using `*ifaddrs` functions. ACKs for top commit: laanwj: Code review ACK 4783115fd4cccb46a7f8c592b34fa7c094c29410 hebasto: ACK 4783115fd4cccb46a7f8c592b34fa7c094c29410, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 4542e036e9b029de970eff8a9230fe45d9204bb22313d075f474295d49bdaf1f1cbb36c0c6e2fa8dbbcdba518d8d3a68a6116ce304b82414315f333baf9af0e4
2021-03-30Merge #21387: p2p: Refactor sock to add I2P fuzz and unit testsWladimir J. van der Laan
40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 test: add I2P test for a runaway SAM proxy (Vasil Dimov) 2d8ac779708322e1235e823edfc9c8f6e2dd65e4 fuzz: add tests for the I2P Session public interface (Vasil Dimov) 9947e44de0cbd79e99d883443a9ac441d8c69713 i2p: use pointers to Sock to accommodate mocking (Vasil Dimov) 82d360b5a88d9057b6c09b61cd69e426c7a2412d net: change ConnectSocketDirectly() to take a Sock argument (Vasil Dimov) b5861100f85fef77b00f55dcdf01ffb4a2a112d8 net: add connect() and getsockopt() wrappers to Sock (Vasil Dimov) 5a887d49b2b39e59d7cce8e9d5b89c21ad694f8b fuzz: avoid FuzzedSock::Recv() repeated errors with EAGAIN (Vasil Dimov) 3088f83d016e7ebb6e6aa559e6326fa0ef0d6282 fuzz: extend FuzzedSock::Recv() to support MSG_PEEK (Vasil Dimov) 9b05c49ade729311a0f4388a109530ff8d0ed1f9 fuzz: implement unimplemented FuzzedSock methods (Vasil Dimov) Pull request description: Change the networking code and the I2P code to be fully mockable and use `FuzzedSocket` to fuzz the I2P methods `Listen()`, `Accept()` and `Connect()`. Add a mocked `Sock` implementation that returns a predefined data on reads and use it for a regression unit test for the bug fixed in https://github.com/bitcoin/bitcoin/pull/21407. ACKs for top commit: practicalswift: Tested ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 MarcoFalke: Concept ACK 40316a37cb jonatack: re-ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 reviewed `git range-diff 01bb3afb 23c861d 40316a3` and the new unit test commit, debug built, ran unit tests, ran bitcoind with an I2P service and network operation with seven I2P peers (2 in, 5 out) is looking nominal laanwj: Code review ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 Tree-SHA512: 7fc4f129849e16e0c7e16662d9f4d35dfcc369bb31450ee369a2b97bdca95285533bee7787983e881e5a3d248f912afb42b4a2299d5860ace7129b0b19623cc8
2021-03-30Merge #20197: p2p: protect onions in AttemptToEvictConnection(), add ↵Wladimir J. van der Laan
eviction protection test coverage 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Add unit test coverage for our onion peer eviction protection (Jon Atack) caa21f586f951d626a67f391050c3644f1057f57 Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() (Jon Atack) 8f1a53eb027727a4c0eaac6d82f0a8279549f638 Use EraseLastKElements() throughout SelectNodeToEvict() (Jon Atack) 8b1e156143740a5548dc7b601d40fb141e6aae1c Add m_inbound_onion to AttemptToEvictConnection() (Jon Atack) 72e30e8e03f880eba4bd1c3fc18b5558d8cef680 Add unit tests for ProtectEvictionCandidatesByRatio() (Jon Atack) ca63b53ecdf377ce777fd959d400748912266748 Use std::unordered_set instead of std::vector in IsEvicted() (Jon Atack) 41f84d5eccd4c2620bf6fee616f2f8f717dbd6f6 Move peer eviction tests to a separate test file (Jon Atack) f126cbd6de6e1a8fee0e900ecfbc14a88e362541 Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvict (Jon Atack) Pull request description: Now that #19991 and #20210 have been merged, we can determine inbound onion peers using `CNode::m_inbound_onion` and add it to the localhost peers protection in `AttemptToEvictConnection`, which was added in #19670 to address issue #19500. Update 28 February 2021: I've updated this to follow gmaxwell's suggestion in https://github.com/bitcoin/bitcoin/pull/20197#issuecomment-713865992. This branch now protects up to 1/4 onion peers (connected via our tor control service), if any, sorted by longest uptime. If any (or all) onion slots remain after that operation, they are then allocated to protect localhost peers, or a minimum of 2 localhost peers in the case that no onion slots remain and 2 or more onion peers were protected, sorted as before by longest uptime. This patch also adds test coverage for the longest uptime, localhost, and onion peer eviction protection logic to build on the welcome initial unit testing of #20477. Suggest reviewing the commits that move code with `colorMoved = dimmed-zebra` and `colorMovedWs = allow-indentation-change`. Closes #11537. ACKs for top commit: laanwj: Code review ACK 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea vasild: ACK 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Tree-SHA512: 2f5a63f942acaae7882920fc61f0185dcd51da85e5b736df9d1fc72343726dd17da740e02f30fa5dc5eb3b2d8345707aed96031bec143d48a2497a610aa19abd
2021-03-29net: add ifaddrs.h includefanquake
2021-03-22[net] remove fUpdateConnectionTime from FinalizeNodeJohn Newbery
PeerManager can just call directly into CAddrMan::Connected() now.
2021-03-20[net] remove CConnman::AddNewAddressesJohn Newbery
It just forwards calls to CAddrMan::Add.
2021-03-20[net] remove CConnman::MarkAddressGoodJohn Newbery
It just forwards calls to CAddrMan::Good.
2021-03-20[net] remove CConnman::SetServicesJohn Newbery
It just forwards calls to CAddrMan::SetServices.
2021-03-20[net] Construct addrman outside connmanJohn Newbery
node.context owns the CAddrMan. CConnman holds a reference to the CAddrMan.
2021-03-19Merge #21328: net, refactor: pass uint16 CService::port as uint16MarcoFalke
52dd40a9febec1f4e70d777821b6764830bdec61 test: add missing netaddress include headers (Jon Atack) 6f09c0f6b57ac01a473c587a3e51e9d477866bb0 util: add missing braces and apply clang format to SplitHostPort() (Jon Atack) 2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack) 6423c8175fad3163c10ffdb49e0df48e4e4931f1 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack) Pull request description: As noticed during review today in https://github.com/bitcoin/bitcoin/pull/20685#discussion_r584873708 of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch. ACKs for top commit: practicalswift: cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61: patch looks correct MarcoFalke: cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61 vasild: ACK 52dd40a9febec1f4e70d777821b6764830bdec61 Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432
2021-03-19Protect onion+localhost peers in ProtectEvictionCandidatesByRatio()Jon Atack
Now that we have a reliable way to detect inbound onion peers, this commit updates our existing eviction protection of 1/4 localhost peers to instead protect up to 1/4 onion peers (connected via our tor control service), sorted by longest uptime. Any remaining slots of the 1/4 are then allocated to protect localhost peers, or 2 localhost peers if no slots remain and 2 or more onion peers are protected, sorted by longest uptime. The goal is to avoid penalizing onion peers, due to their higher min ping times relative to IPv4 and IPv6 peers, and improve our diversity of peer connections. Thank you to Gregory Maxwell, Suhas Daftuar, Vasil Dimov and Pieter Wuille for valuable review feedback that shaped the direction.
2021-03-19Use EraseLastKElements() throughout SelectNodeToEvict()Jon Atack
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2021-03-19Add m_inbound_onion to AttemptToEvictConnection()Jon Atack
and an `m_is_onion` struct member to NodeEvictionCandidate and tests. We'll use these in the peer eviction logic to protect inbound onion peers in addition to the existing protection of localhost peers.
2021-03-19Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvictJon Atack
to allow deterministic unit testing of the ratio-based peer eviction protection logic, which protects peers having longer connection times and those connected via higher-latency networks. Add documentation.
2021-03-17refactor: post Optional<> removal cleanupsfanquake
2021-03-16p2p, refactor: pass and use uint16_t CService::port as uint16_tJon Atack
2021-03-16i2p: use pointers to Sock to accommodate mockingVasil Dimov
Change the types of `i2p::Connection::sock` and `i2p::sam::Session::m_control_sock` from `Sock` to `std::unique_ptr<Sock>`. Using pointers would allow us to sneak `FuzzedSock` instead of `Sock` and have the methods of the former called. After this change a test only needs to replace `CreateSock()` with a function that returns `FuzzedSock`.
2021-03-16net: change ConnectSocketDirectly() to take a Sock argumentVasil Dimov
Change `ConnectSocketDirectly()` to take a `Sock` argument instead of a bare `SOCKET`. With this, use the `Sock`'s (possibly mocked) methods `Connect()`, `Wait()` and `GetSockOpt()` instead of calling the OS functions directly.
2021-03-15scripted-diff: remove Optional & nulloptfanquake
-BEGIN VERIFY SCRIPT- git rm src/optional.h sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src) sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src) sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src) sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src) sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src) sed -i -e '/optional.h \\/d' src/Makefile.am sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src) -END VERIFY SCRIPT-
2021-03-11scripted-diff: remove MakeUnique<T>()fanquake
-BEGIN VERIFY SCRIPT- git rm src/util/memory.h sed -i -e 's/MakeUnique/std::make_unique/g' $(git grep -l MakeUnique src) sed -i -e '/#include <util\/memory.h>/d' $(git grep -l '#include <util/memory.h>' src) sed -i -e '/util\/memory.h \\/d' src/Makefile.am -END VERIFY SCRIPT-
2021-03-04net: Replace enum CConnMan::NumConnections with enum class ConnectionDirectionLuke Dashjr
2021-03-03Make all Poisson delays use std::chrono typesPieter Wuille
2021-03-03Change all ping times to std::chrono typesPieter Wuille
2021-03-01net: accept incoming I2P connections from CConnmanVasil Dimov
2021-03-01net: make outgoing I2P connections from CConnmanVasil Dimov
2021-03-01init: introduce I2P connectivity optionsVasil Dimov
Introduce two new options to reach the I2P network: * `-i2psam=<ip:port>` point to the I2P SAM proxy. If this is set then the I2P network is considered reachable and we can make outgoing connections to I2P peers via that proxy. We listen for and accept incoming connections from I2P peers if the below is set in addition to `-i2psam=<ip:port>` * `-i2pacceptincoming` if this is set together with `-i2psam=<ip:port>` then we accept incoming I2P connections via the I2P SAM proxy.
2021-03-01net: dedup MSG_NOSIGNAL and MSG_DONTWAIT definitionsVasil Dimov
Deduplicate `MSG_NOSIGNAL` and `MSG_DONTWAIT` definitions from `net.cpp` and `netbase.cpp` to `compat.h` where they can also be reused by other code.
2021-03-01net: avoid unnecessary GetBindAddress() callVasil Dimov
Our local (bind) address is already saved in `CNode::addrBind` and there is no need to re-retrieve it again with `GetBindAddress()`. Also, for I2P connections `CNode::addrBind` would contain our I2P address, but `GetBindAddress()` would return something like `127.0.0.1:RANDOM_PORT`.
2021-03-01net: isolate the protocol-agnostic part of CConnman::AcceptConnection()Vasil Dimov
Isolate the second half of `CConnman::AcceptConnection()` into a new separate method, which could be reused if we accept incoming connections by other means than `accept()` (first half of `CConnman::AcceptConnection()`).
2021-03-01net: get the bind address earlier in CConnman::AcceptConnection()Vasil Dimov
Call `GetBindAddress()` earlier in `CConnman::AcceptConnection()`. That is specific to the TCP protocol and makes the code below it reusable for other protocols, if the caller provides `addr_bind`, retrieved by other means.
2021-03-01net: check for invalid socket earlier in CConnman::AcceptConnection()Vasil Dimov
This check is related to an `accept()` failure. So do the check earlier, closer to the `accept()` call. This will allow to isolate the `accept()`-specific code at the beginning of `CConnman::AcceptConnection()` and reuse the code that follows it.
2021-02-23Merge #21222: log: Clarify log message when file does not existMarcoFalke
faf48f20f196e418b2eea390a0140db3604cfa15 log: Clarify log message when file does not exist (MarcoFalke) Pull request description: Shorter and broader alternative to #21181 Rendered diff: ```diff @@ -1,4 +1,4 @@ -Bitcoin Core version v21.99.0-db656db2ed5a (release build) +Bitcoin Core version v21.99.0-faf48f20f196 (release build) Qt 5.15.2 (dynamic), plugin=wayland (dynamic) No static plugins. Style: adwaita / Adwaita::Style @@ -24,8 +24,8 @@ scheduler thread start Using wallet directory /tmp/test_001/regtest/wallets init message: Verifying wallet(s)... init message: Loading banlist... -ERROR: DeserializeFileDB: Failed to open file /tmp/test_001/regtest/banlist.dat -Invalid or missing banlist.dat; recreating +Missing or invalid file /tmp/test_001/regtest/banlist.dat +Recreating banlist.dat SetNetworkActive: true Failed to read fee estimates from /tmp/test_001/regtest/fee_estimates.dat. Continue anyway. Using /16 prefix for IP bucketing @@ -63,9 +63,9 @@ Bound to [::]:18444 Bound to 0.0.0.0:18444 Bound to 127.0.0.1:18445 init message: Loading P2P addresses... -ERROR: DeserializeFileDB: Failed to open file /tmp/test_001/regtest/peers.dat -Invalid or missing peers.dat; recreating -ERROR: DeserializeFileDB: Failed to open file /tmp/test_001/regtest/anchors.dat +Missing or invalid file /tmp/test_001/regtest/peers.dat +Recreating peers.dat +Missing or invalid file /tmp/test_001/regtest/anchors.dat 0 block-relay-only anchors will be tried for connections. init message: Starting network threads... net thread start ACKs for top commit: jnewbery: utACK faf48f20f196e418b2eea390a0140db3604cfa15 amitiuttarwar: utACK faf48f20f1, 👍 for consistency. also checked where we create / load other `.dat` files, looks good to me. practicalswift: cr ACK faf48f20f196e418b2eea390a0140db3604cfa15 Tree-SHA512: 697a728ef2b9f203363ac00b03eaf23ddf80bee043ecd3719265a0d884e8cfe88cd39afe946c86ab849edd1c836f05ec51125f052bdc14fe184b84447567756f
2021-02-18log: Clarify log message when file does not existMarcoFalke
Also, run clang-format on the function
2021-02-18[net] Move checks from GetLocalAddrForPeer to callerJohn Newbery
GetLocalAddrForPeer() is only called in one place. The checks inside that function make more sense to be carried out be the caller: - fSuccessfullyConnected is already checked at the top of SendMessages(), so must be true when we call GetLocalAddrForPeer() - fListen can go into the conditional before GetLocalAddrForPeer() is called.
2021-02-18[net] Change AdvertiseLocal to GetLocalAddrForPeerJohn Newbery
Gossiping addresses to peers is the responsibility of net processing. Change AdvertiseLocal() in net to just return an (optional) address for net processing to advertise. Update function name to reflect new responsibility.
2021-02-15scripted-diff: rename ping membersJohn Newbery
-BEGIN VERIFY SCRIPT- sed -i 's/fPingQueued/m_ping_queued/g' src/net_processing.cpp sed -i 's/nMinPingUsecTime/m_min_ping_time/g' src/net.* src/net_processing.cpp src/test/net_tests.cpp sed -i 's/nPingNonceSent/m_ping_nonce_sent/g' src/net_processing.cpp sed -i 's/nPingUsecTime/m_last_ping_time/g' src/net.* -END VERIFY SCRIPT-
2021-02-15[net processing] Move ping data fields to net processingJohn Newbery
2021-02-15[net processing] Move ping timeout logic to net processingJohn Newbery
Ping messages are an application-level mechanism. Move timeout logic from net to net processing.
2021-02-15[net] Add RunInactivityChecks()John Newbery
Moves the logic to prevent running inactivity checks until the peer has been connected for -peertimeout time into its own function. This will be reused by net_processing later.
2021-02-12net: remove CNode::m_inbound_onion defaults for explicitnessJon Atack
and to allow the compiler to warn if uninitialized in the ctor or omitted in the caller.
2021-02-12net: make CNode::m_inbound_onion public, drop getter, update testsJon Atack
2021-02-12[refactor] Correct log message in net.cppDhruv Mehta
2021-02-12Merge #19884: p2p: No delay in adding fixed seeds if -dnsseed=0 and ↵Wladimir J. van der Laan
peers.dat is empty fe3e993968d6b46777d5a16a662cd22790ddf5bb [p2p] No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty. Add -fixedseeds arg. (Dhruv Mehta) Pull request description: Closes #19795 Before PR: If `peers.dat` is empty and `-dnsseed=0`, bitcoind will fallback on to fixed seeds but only after a 60 seconds delay. After PR: There's no 60 second delay. To reproduce: `rm ~/.bitcoin/peers.dat && src/bitcoind -dnsseed=0` without and with patch code Other changes in the PR: - `-fixedseeds` command line argument added: `-dnsseed=0 -fixedseeds=0 -addnode=X` provides a trusted peer only setup. `-dnsseed=0 -fixedseeds=0` allows for a `addnode` RPC to add a trusted peer without falling back to hardcoded seeds. ACKs for top commit: LarryRuane: re-ACK fe3e993968d6b46777d5a16a662cd22790ddf5bb laanwj: re-ACK fe3e993968d6b46777d5a16a662cd22790ddf5bb Tree-SHA512: 79449bf4e83a315be6dbac9bdd226de89d2a3f7f76d9c5640a2cb3572866e6b0e8ed67e65674c9824054cf13119dc01c7e1a33848daac6b6c34dbc158b6dba8f
2021-02-11[p2p] No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty. ↵Dhruv Mehta
Add -fixedseeds arg.
2021-02-11Merge #20788: net: add RAII socket and use it instead of bare SOCKETWladimir J. van der Laan
615ba0eb96cf131364c1ceca9d3dedf006fa1e1c test: add Sock unit tests (Vasil Dimov) 7bd21ce1efc363b3e8ea1d51dd1410ccd66820cb style: rename hSocket to sock (Vasil Dimov) 04ae8469049e1f14585aabfb618ae522150240a7 net: use Sock in InterruptibleRecv() and Socks5() (Vasil Dimov) ba9d73268f9585d4b9254adcf54708f88222798b net: add RAII socket and use it instead of bare SOCKET (Vasil Dimov) dec9b5e850c6aad989e814aea5b630b36f55d580 net: move CloseSocket() from netbase to util/sock (Vasil Dimov) aa17a44551c03b00a47854438afe9f2f89b6ea74 net: move MillisToTimeval() from netbase to util/time (Vasil Dimov) Pull request description: Introduce a class to manage the lifetime of a socket - when the object that contains the socket goes out of scope, the underlying socket will be closed. In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()` methods that can be overridden by unit tests to mock the socket operations. The `Wait()` method also hides the `#ifdef USE_POLL poll() #else select() #endif` technique from higher level code. ACKs for top commit: laanwj: Re-ACK 615ba0eb96cf131364c1ceca9d3dedf006fa1e1c jonatack: re-ACK 615ba0eb96cf131364c1ceca9d3dedf006fa1e1c Tree-SHA512: 3003e6bc0259295ca0265ccdeb1522ee25b4abe66d32e6ceaa51b55e0a999df7ddee765f86ce558a788c1953ee2009bfa149b09d494593f7d799c0d7d930bee8
2021-02-10net: use Sock in InterruptibleRecv() and Socks5()Vasil Dimov
Use the `Sock` class instead of `SOCKET` for `InterruptibleRecv()` and `Socks5()`. This way the `Socks5()` function can be tested by giving it a mocked instance of a socket. Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
2021-02-10net: add RAII socket and use it instead of bare SOCKETVasil Dimov
Introduce a class to manage the lifetime of a socket - when the object that contains the socket goes out of scope, the underlying socket will be closed. In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()` methods that can be overridden by unit tests to mock the socket operations. The `Wait()` method also hides the `#ifdef USE_POLL poll() #else select() #endif` technique from higher level code.
2021-02-10net: move CloseSocket() from netbase to util/sockVasil Dimov
Move `CloseSocket()` (and `NetworkErrorString()` which it uses) from `netbase.{h,cpp}` to newly added `src/util/sock.{h,cpp}`. This is necessary in order to use `CloseSocket()` from a newly introduced Sock class (which will live in `src/util/sock.{h,cpp}`). `sock.{h,cpp}` cannot depend on netbase because netbase will depend on it.
2021-02-02Merge #19509: Per-Peer Message CaptureMarcoFalke
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev) 381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev) e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev) 4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev) f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev) dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev) Pull request description: This PR introduces per-peer message capture into Bitcoin Core. 📓 ## Purpose The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?". ## Functionality When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue. The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so: ``` message_capture/203.0.113.7:56072/msgs_recv.dat message_capture/203.0.113.7:56072/msgs_sent.dat ``` Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option. ## Future Maintenance I sympathize greatly with anyone who says "the best code is no code". The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool. Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small. ## FAQ "Why not just use Wireshark" Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark. Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done. Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable. ACKs for top commit: MarcoFalke: re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚 jnewbery: utACK bff7c66e67aa2f18ef70139338643656a54444fe theStack: re-ACK bff7c66e67aa2f18ef70139338643656a54444fe Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47