aboutsummaryrefslogtreecommitdiff
path: root/src/net.cpp
AgeCommit message (Collapse)Author
2024-09-11assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed. Github-Pull: bitcoin/bitcoin#30807 Rebased-From: 6d5812e5c852c233bd7ead2ceef051f8567619ed
2024-08-09net: Clarify that m_addr_local is only set onceMarcoFalke
Includes a rename from addrLocal to m_addr_local to match the name of its corresponding Mutex.
2024-08-05Merge bitcoin/bitcoin#30512: net: Log accepted connection after ↵merge-script
m_nodes.push_back; Fix intermittent test issue fa3ea3b83c6a4c9726a17f2a48bbdb77f944bf6c test: Fix intermittent issue in p2p_v2_misbehaving.py (MarcoFalke) 55555574d105f6c529c5c966c3c971c9db5ab786 net: Log accepted connection after m_nodes.push_back (MarcoFalke) Pull request description: Fix the two issues reported in https://github.com/bitcoin/bitcoin/pull/30468/files#r1688444784: * Delay a debug log line for consistency. * Fix an intermittent test issue. They are completely separate fixes, but both `net` related. ACKs for top commit: 0xB10C: Code Review ACK fa3ea3b83c6a4c9726a17f2a48bbdb77f944bf6c stratospher: tested ACK fa3ea3b. Tree-SHA512: cd6b6e164b317058a305a5c3e38c56c9a814a7469039e1143f1d7addfbc91b0a28506873356b373d97448b46cb6fbe94a1309df82e34c855540b241a09489e8b
2024-08-02Merge bitcoin-core/gui#626: Showing Local Addresses in Node WindowHennadii Stepanov
189c987386a0da132d7ef076cdf539f9eb75fc3c Showing local addresses on the Node Window (Jadi) a5d7aff867a3df9ac77664deed03e930e2636db0 net: Providing an interface for mapLocalHost (Jadi) Pull request description: This change adds a new row to the Node Window (debugwindow.ui) under the Network section which shows the LocalAddresses. fixes #564 <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md --> <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. --> <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. --> ACKs for top commit: pablomartin4btc: re-ACK 189c987386a0da132d7ef076cdf539f9eb75fc3c furszy: utACK 189c987 Tree-SHA512: 93f201bc6d21d81b27b87be050a447b841f01e3efb69b9eca2cc7af103023d7cd69eb5e16e2875855573ef51a5bf74a6ee6028636c1b6798cb4bb11567cb4996
2024-08-02net: Providing an interface for mapLocalHostJadi
Contributes to #564 by providing an interface for mapLocalHost through net -> node interface -> clientModel. Later this value can be read by GUI to show the local addresses.
2024-07-23net: Log accepted connection after m_nodes.push_backMarcoFalke
Otherwise, the debug log could read confusingly, when the getpeerinfo() RPC (calling GetNodeStats) happens after the "accepted connection" log line, but returns an empty list. For example, the following timeline in the debug log could correspond to a getpeerinfo reply that is empty: [net] [net.cpp:3764] [CNode] Added connection peer=0 [net] [net.cpp:1814] [CreateNodeFromAcceptedSocket] connection from 127.0.0.1:45154 accepted [http] [httpserver.cpp:305] [http_request_cb] Received a POST request for / from 127.0.0.1:33320 [httpworker.1] [rpc/request.cpp:232] [parse] ThreadRPCServer method=getpeerinfo user=__cookie__ Fix it by moving the log line.
2024-07-16Merge bitcoin/bitcoin#22729: Make it possible to disable Tor binds and abort ↵Ava Chow
startup on bind failure bca346a97056748f1e3fb899f336d56d9fd45a64 net: require P2P binds to succeed (Vasil Dimov) af552534ab83c572d3bc3f93ccaee5c1961ccab5 net: report an error if unable to bind on the Tor listening addr:port (Vasil Dimov) 9a7e5f4d68dadc64a675f32d1e91199d6b1aa095 net: don't extra bind for Tor if binds are restricted (Vasil Dimov) Pull request description: Make it possible to disable the Tor binding on `127.0.0.1:8334` and stop startup if any P2P bind fails instead of "if all P2P binds fail". Fixes https://github.com/bitcoin/bitcoin/issues/22726 Fixes https://github.com/bitcoin/bitcoin/issues/22727 ACKs for top commit: achow101: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 cbergqvist: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 pinheadmz: ACK bca346a97056748f1e3fb899f336d56d9fd45a64 Tree-SHA512: fabef89a957191eea4f3e3b6109d2b8389f27ecc74440a920b0c10f31fff00a85bcfd1eb3c91826c7169c618f4de8a8d0a260e2caf40fd854f07ea9a980d8603
2024-07-09Merge bitcoin/bitcoin#30396: random: add benchmarks and drop unnecessary ↵Ava Chow
Shuffle function 6ecda04fefad980872c72fba89844393f5581120 random: drop ad-hoc Shuffle in favor of std::shuffle (Pieter Wuille) da28a26aae3178fb7663efbe20bb650857ace775 bench random: benchmark more functions, and add InsecureRandomContext (Pieter Wuille) 0a9bbc64c157a314e5472ecd98300e30b12d3fdf random bench refactor: move to new bench/random.cpp (Pieter Wuille) Pull request description: This adds benchmarks for various operations on `FastRandomContext` and `InsecureRandomContext`, and then removes the ad-hoc `Shuffle` functions, now that it appears that standard library `std::shuffle` has comparable performance. The other reason for keeping `Shuffle`, namely the fact that libstdc++ used self-move (which debug mode panics on) has been fixed as well (see https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049). ACKs for top commit: achow101: ACK 6ecda04fefad980872c72fba89844393f5581120 hodlinator: ACK 6ecda04fefad980872c72fba89844393f5581120 dergoegge: Code review ACK 6ecda04fefad980872c72fba89844393f5581120 Tree-SHA512: 2560b7312410581ff2b9bd0716e0f1558d910b5eadb9544785c972384985ac0f11f72d6b2797cfe2e7eb71fa57c30cffd98cc009cb4ee87a18b1524694211417
2024-07-09Merge bitcoin/bitcoin#29431: test/BIP324: disconnection scenarios during v2 ↵Ava Chow
handshake c9dacd958d7c1e98b08a7083c299d981e4c11193 test: Check that non empty version packet is ignored and no disconnection happens (stratospher) 997cc00b950a7d1b7f2a3971282685f4e81d87d2 test: Check that disconnection happens when AAD isn't filled (stratospher) b5e6238fdbba5c777a5adfa4477dac51a82f4448 test: Check that disconnection happens when garbage sent/received are different (stratospher) ad1482d5a20e6b155184a43d0724d2dcd950ce52 test: Check that disconnection happens when wrong garbage terminator is sent (stratospher) e351576862471fc77b1e798a16833439e23ff0b4 test: Check that disconnection happens when >4095 garbage bytes is sent (stratospher) e075fd131d668d9d1ba3c8566624481c4a57032d test: Introduce test types and modify v2 handshake function accordingly (stratospher) 7d07daa62311bdb0e2ce23d0b55f711f5088bd28 log: Add V2 handshake timeout (stratospher) d4a1da8543522a213ac75761131d878eedfd4a5b test: Make global TRANSPORT_VERSION variable an instance variable (stratospher) c642b08c4e45cb3a625a867ebd66c0ae51bde212 test: Log when the garbage is actually sent to transport layer (stratospher) 86cca2cba230c10324c6aedd12ae9655b83b2856 test: Support disconnect waiting for add_p2p_connection (stratospher) bf9669af9ccc33dcade09bceb27d6745e9d9a75a test: Rename early key response test and move random_bitflip to util (stratospher) Pull request description: Add tests for the following v2 handshake scenarios: 1. Disconnection happens when > `MAX_GARBAGE_LEN` bytes garbage is sent 2. Disconnection happens when incorrect garbage terminator is sent 3. Disconnection happens when garbage bytes are tampered with 4. Disconnection happens when AAD of first encrypted packet after the garbage terminator is not filled 5. bitcoind ignores non-empty version packet and no disconnection happens All these tests require a modified v2 P2P class (different from `EncryptedP2PState` used in `v2_p2p.py`) to implement our custom handshake behaviour based on different scenarios and have been kept in a single test file (`test/functional/p2p_v2_misbehaving.py`). Shifted the test in `test/functional/p2p_v2_earlykeyresponse.py` which is of the same pattern to this file too. ACKs for top commit: achow101: ACK c9dacd958d7c1e98b08a7083c299d981e4c11193 mzumsande: ACK c9dacd958d7c1e98b08a7083c299d981e4c11193 theStack: Code-review ACK c9dacd958d7c1e98b08a7083c299d981e4c11193 Tree-SHA512: 90df81f0c7f4ecf0a47762d290a618ded92cde9f83d3ef3cc70e1b005ecb16125ec39a9d80ce95f99e695d29abd63443240cb5490aa57c5bc8fa2e52149a0672
2024-07-06random: drop ad-hoc Shuffle in favor of std::shufflePieter Wuille
Benchmarks show it is no longer faster with modern standard C++ libraries, and the debug-mode failure due to self-move has been fixed as well.
2024-07-02net: require P2P binds to succeedVasil Dimov
In the Tor case, this prevents us from telling the Tor daemon to send our incoming connections from the Tor network to an address where we do not listen (we tried to listen but failed probably because another application is already listening). In the other cases (IPv4/IPv6 binds) this also prevents unpleasant surprises caused by continuing operations even on bind failure. For example, another application may be listening on portX, bitcoind tries to bind on portX and portY, only succeeds with portY and continues operation leaving the user thinking that his bitcoind is listening on portX whereas another application is listening (the error message in the log could easily be missed). Avoid having the functional testing framework start multiple `bitcoind`s that try to listen on the same `127.0.0.1:18445` (Tor listen for regtest) if `bind_to_localhost_only` is set to `False`. Also fix a typo in `test-shell.md` related to `bind_to_localhost_only`. Fixes https://github.com/bitcoin/bitcoin/issues/22727
2024-07-02net: report an error if unable to bind on the Tor listening addr:portVasil Dimov
2024-07-01net, net_processing: use existing RNG objects morePieter Wuille
PeerManagerImpl, as well as several net functions, already have existing FastRandomContext objects. Reuse them instead of constructing new ones.
2024-07-01random: convert GetExponentialRand into rand_exp_durationPieter Wuille
2024-07-01random: convert GetRand{Micros,Millis} into randrangePieter Wuille
There are only a few call sites of these throughout the codebase, so move the functionality into FastRandomContext, and rewrite all call sites. This requires the callers to explicit construct FastRandomContext objects, which do add to the verbosity, but also make potentially apparent locations where the code can be improved by reusing a FastRandomContext object (see further commit).
2024-07-01net: use GetRandMicros for cache expirationPieter Wuille
This matches the data type of m_cache_entry_expiration.
2024-06-14netbase: extend CreateSock() to support creating arbitrary socketsVasil Dimov
Allow the callers of `CreateSock()` to pass all 3 arguments to the `socket(2)` syscall. This makes it possible to create sockets of any domain/type/protocol.
2024-05-28log: Add V2 handshake timeoutstratospher
2024-05-21Merge bitcoin/bitcoin#29421: net: make the list of known message types a ↵Ava Chow
compile time constant b3efb486732f3caf8b8a8e9d744e6d20ae4255ef protocol: make message types constexpr (Vasil Dimov) 2fa9de06c2c8583ee8e2434dc97014b26e218ab5 net: make the list of known message types a compile time constant (Vasil Dimov) Pull request description: Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size. --- This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29418, thus the current standalone PR. ACKs for top commit: achow101: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef jonatack: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef maflcko: utACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef 🎊 willcl-ark: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af
2024-05-16Merge bitcoin/bitcoin#30085: p2p: detect addnode cjdns peers in ↵merge-script
GetAddedNodeInfo() d0b047494c28381942c09d0cca45baa323bfcffc test: add GetAddedNodeInfo() CJDNS regression unit test (Jon Atack) 684da9707040ce25d95b2954eda50b811136d92c p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo() (Jon Atack) Pull request description: Addnode peers connected to us via the cjdns network are currently not detected by `CConnman::GetAddedNodeInfo()`, i.e. `fConnected` is always false. This causes the following issues: - RPC `getaddednodeinfo` incorrectly shows them as not connected - `CConnman::ThreadOpenAddedConnections()` continually retries to connect them Fix the issue and add a unit regression test. Extracted from #28248. Suggest running the test with: `./src/test/test_bitcoin -t net_peer_connection_tests -l test_suite` ACKs for top commit: mzumsande: utACK d0b047494c28381942c09d0cca45baa323bfcffc brunoerg: crACK d0b047494c28381942c09d0cca45baa323bfcffc pinheadmz: ACK d0b047494c28381942c09d0cca45baa323bfcffc Tree-SHA512: a4d81425f79558f5792585611f3fe8ab999b82144daeed5c3ec619861c69add934c2b2afdad24c8488a0ade94f5ce8112f5555d60a1ce913d4f5a1cf5dbba55a
2024-05-15Merge bitcoin/bitcoin#28929: serialization: Support for multiple parametersAva Chow
8d491ae9ecf1948ea29f67b50ca7259123f602aa serialization: Add ParamsStream GetStream() method (Ryan Ofsky) 951203bcc496c4415b7754cd764544659b76067f net: Simplify ParamsStream usage (Ryan Ofsky) e6794e475c84d9edca4a2876e2342cbb1d85f804 serialization: Accept multiple parameters in ParamsStream constructor (Ryan Ofsky) cb28849a88339c1e7ba03ffc7e38998339074e6e serialization: Reverse ParamsStream constructor order (Ryan Ofsky) 83436d14f06551026bcf5529df3b63b4e8a679fb serialization: Drop unnecessary ParamsStream references (Ryan Ofsky) 84502b755bcc35413ad466047893b5edf134c53f serialization: Drop references to GetVersion/GetType (Ryan Ofsky) f3a2b5237688e9f574444e793724664b00fb7f2a serialization: Support for multiple parameters (Ryan Ofsky) Pull request description: Currently it is only possible to attach one serialization parameter to a stream at a time. For example, it is not possible to set a parameter controlling the transaction format and a parameter controlling the address format at the same time because one parameter will override the other. This limitation is inconvenient for multiprocess code since it is not possible to create just one type of stream and serialize any object to it. Instead it is necessary to create different streams for different object types, which requires extra boilerplate and makes using the new parameter fields a lot more awkward than the older version and type fields. Fix this problem by allowing an unlimited number of serialization stream parameters to be set, and allowing them to be requested by type. Later parameters will still override earlier parameters, but only if they have the same type. For an example of different ways multiple parameters can be set, see the new [`with_params_multi`](https://github.com/bitcoin/bitcoin/blob/40f505583f4edeb2859aeb70da20c6374d331a4f/src/test/serialize_tests.cpp#L394-L410) unit test. This change requires replacing the `stream.GetParams()` method with a `stream.GetParams<T>()` method in order for serialization code to retrieve the desired parameters. The change is more verbose, but probably a good thing for readability because previously it could be difficult to know what type the `GetParams()` method would return, and now it is more obvious. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). ACKs for top commit: maflcko: ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa 🔵 sipa: utACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa TheCharlatan: ACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa Tree-SHA512: 40b7041ee01c0372b1f86f7fd6f3b6af56ef24a6383f91ffcedd04d388e63407006457bf7ed056b0e37b4dec9ffd5ca006cb8192e488ea2c64678567e38d4647
2024-05-10p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo()Jon Atack
Addnode (manual) peers connected to us via the cjdns network are currently not detected by CConnman::GetAddedNodeInfo(), i.e. fConnected is always false. This causes the following issues: - RPC `getaddednodeinfo` incorrectly shows them as not connected - CConnman::ThreadOpenAddedConnections() continually retries to connect them
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-07Merge bitcoin/bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in ↵merge-script
Discover a68fed111be393ddbbcd7451f78bc63601253ee0 net: Fix misleading comment for Discover (laanwj) 7766dd280d9a4a7ffdfcec58224d0985cfd4169b net: Replace ifname check with IFF_LOOPBACK in Discover (laanwj) Pull request description: Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently. Check for the `IFF_LOOPBACK` flag to detect loopback interface instead. Also remove a misleading comment in Discover's doc comment. ACKs for top commit: sipa: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 willcl-ark: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 theuni: utACK a68fed111be393ddbbcd7451f78bc63601253ee0. Satoshi-era brittleness :) Tree-SHA512: e2d7fc541f40f6a6af08286e7bcb0873ff55debdcd8b38b03f274897b673a6fb51d84d6c7241a02a9567ddf2645f50231d91bb1f55307ba7c6e68196c29b0edf
2024-05-01scripted-diff: Add IWYU pragma keep to bitcoin-config.h includesMarcoFalke
-BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' ) -END VERIFY SCRIPT-
2024-04-30Merge bitcoin/bitcoin#28016: p2p: gives seednode priority over dnsseed if ↵Ava Chow
both are provided 82f41d76f1c6ad38290917dad5499ffbe6b3974d Added seednode prioritization message to help output (tdb3) 3120a4678ab2a71a381e847688f44068749cfa97 Gives seednode priority over dnsseed if both are provided (Sergi Delgado Segura) Pull request description: This is a follow-up of #27577 If both `seednode` and `dnsseed` are provided, the node will start a race between them in order to fetch data to feed the `addrman`. This PR gives priority to `seednode` over `dnsseed` so if some nodes are provided as seeds, they can be tried before defaulting to the `dnsseeds` ACKs for top commit: davidgumberg: untested reACK https://github.com/bitcoin/bitcoin/commit/82f41d76f1c6ad38290917dad5499ffbe6b3974d itornaza: tested re-ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d achow101: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d cbergqvist: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d Tree-SHA512: 4e39e10a7449af6cd9b8f9f6878f846b94bca11baf89ff2d4fbcd4f28293978a6ed71a3a86cea36d49eca891314c834e32af93f37a09c2cc698a878f84d31c62
2024-04-30Merge bitcoin/bitcoin#29623: Simplify network-adjusted time warning logicAva Chow
c6be144c4b774a03a8bcab5a165768cf81e9b06b Remove timedata (stickies-v) 92e72b5d0d49aa395e626c238bc28aba8e4c3d44 [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge) 7d9c3ec622d73a98d07ab3cee78751718982a5bc [net processing] Introduce PeerManagerInfo (dergoegge) ee178dfcc1175e0af8163216c9c024f4bfc97965 Add TimeOffsets helper class (stickies-v) 55361a15d1aa6984051441bce88112000688fb43 [net processing] Use std::chrono for type-safe time offsets (stickies-v) 038fd979effb54ee76ce1b7cf078e920c652326a [net processing] Move nTimeOffset to net_processing (dergoegge) Pull request description: [An earlier approach](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes. Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are: - Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty. - Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number). - Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to 1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes - no more globals - remove the `-maxtimeadjustment` startup arg Closes #4521 ACKs for top commit: sr-gi: Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b) achow101: reACK c6be144c4b774a03a8bcab5a165768cf81e9b06b dergoegge: utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
2024-04-28net: Replace ifname check with IFF_LOOPBACK in Discoverlaanwj
Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently. Check for the `IFF_LOOPBACK` flag to detect loopback interface instead.
2024-04-25Merge bitcoin/bitcoin#28834: net: attempts to connect to all resolved ↵Ava Chow
addresses when connecting to a node fd81a37239541d0d508402cd4eeb28f38128c1f2 net: attempts to connect to all resolved addresses when connecting to a node (Sergi Delgado Segura) Pull request description: This is a follow-up of #28155 motivated by https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1362677038 ## Rationale Prior to this, when establishing a network connection via `CConnman::ConnectNode`, if the connection needed address resolution, a single address would be picked at random from the resolved addresses and our node would try to connect to it. However, this would lead to the behavior of `ConnectNode` being unpredictable when the address was resolved to various ips (e.g. the address resolving to IPv4 and IPv6, but we only support one of them). This patches the aforementioned behavior by going over all resolved IPs until a valid one is found or until we exhaust them. ACKs for top commit: mzumsande: re-ACK fd81a37239541d0d508402cd4eeb28f38128c1f2 (just looked at diff, only small logging change) achow101: ACK fd81a37239541d0d508402cd4eeb28f38128c1f2 vasild: ACK fd81a37239541d0d508402cd4eeb28f38128c1f2 Tree-SHA512: fa1ebc5c84fe61dd0a7fe1113ae2d594a75ad661c43ed8984a31fc9bc50f166b2759b0d8d84ee5dc247691eff78c8156fac970af797bbcbf67492eec0353fb58
2024-04-22Added seednode prioritization message to help outputtdb3
2024-04-22Gives seednode priority over dnsseed if both are providedSergi Delgado Segura
2024-04-18net: attempts to connect to all resolved addresses when connecting to a nodeSergi Delgado Segura
Prior to this, when establishing a network connection via CConnman::ConnectNode, if the connection needed address resolution, a single address would be picked at random from the resolved addresses and our node will try to connect to it. However, this would lead to the behavior of ConnectNode being unpredictable when the address was resolved to various ips (e.g. the address resolving to IPv4 and IPv6, but we only support one of them). This patches the aforementioned behavior by going over all resolved IPs until we find one we can connect to or until we exhaust them.
2024-04-11net: Decrease nMaxIPs when learning from DNS seedslaanwj
Limit number of IPs learned from a single DNS seed to 32, to prevent the results from one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is bounded to 33 already, but it is possible for it to use TCP where a potentially enormous number of results can be returned. Closes #16070.
2024-04-10[net processing] Move nTimeOffset to net_processingdergoegge
2024-03-13Merge bitcoin/bitcoin#27375: net: support unix domain sockets for -proxy and ↵Ava Chow
-onion 567cec9a05e1261e955535f734826b12341684b6 doc: add release notes and help text for unix sockets (Matthew Zipkin) bfe51928911daf484ae07deb52a7ff0bcb2526ae test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin) c65c0d01630b44fa71321ea7ad68d5f9fbb7aefb init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin) c3bd43142eba77dcf1acd4984e437759f65e237a gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin) a88bf9dedd1d8c1db0a9c8b663dab3e3c2f0f030 i2p: construct Session with Proxy instead of CService (Matthew Zipkin) d9318a37ec09fe0b002815a7e48710e530620ae2 net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin) ac2ecf3182fb5ad9bcd41540b19382376114d6ee proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin) a89c3f59dc44eaf4f59912c1accfc0ce5d61933a netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin) 3a7d6548effa6cd9a4a5413b690c2fd85da4ef65 net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin) 74f568cb6fd5c74b7b9bf0ce69876430746a53b1 netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin) bae86c8d318d06818aa75a9ebe3db864197f0bc6 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin) adb3a3e51de205cc69b1a58647c65c04fa6c6362 configure: test for unix domain sockets (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27252 UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system. There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/pull/9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=` I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example): `tor --SocksPort unix:/Users/matthewzipkin/torsocket/x` `bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x` Prep work for this feature includes: - Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService` - Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path) Future work: - Enable UNIX sockets for ZMQ (https://github.com/bitcoin/bitcoin/pull/27679) - Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet) - Enable UNIX sockets on windows where supported - Update Network Proxies dialog in GUI to support UNIX sockets ACKs for top commit: Sjors: re-ACK 567cec9a05e1261e955535f734826b12341684b6 tdb3: re ACK for 567cec9a05e1261e955535f734826b12341684b6. achow101: ACK 567cec9a05e1261e955535f734826b12341684b6 vasild: ACK 567cec9a05e1261e955535f734826b12341684b6 Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1
2024-03-12Merge bitcoin/bitcoin#27114: p2p: Allow whitelisting manual connectionsAva Chow
0a533613fb44207053796fd01a9f4b523a3153d4 docs: add release notes for #27114 (brunoerg) e6b8f19de9a6d1c477d0bbda18d17794cd81a6f4 test: add coverage for whitelisting manual connections (brunoerg) c985eb854cc86deb747caea5283c17cf51b6a983 test: add option to speed up tx relay/mempool sync (brunoerg) 66bc6e2d1749f43d7b314aa2784a06af78440170 Accept "in" and "out" flags to -whitelist to allow whitelisting manual connections (Luke Dashjr) 8e06be347c5e14cbe75256eba170e0867f95f360 net_processing: Move extra service flag into InitializeNode (Luke Dashjr) 9133fd69a5cc9a0ab1a06a60d09f1b7e1039018e net: Move `NetPermissionFlags::Implicit` verification to `AddWhitelistPermissionFlags` (Luke Dashjr) 2863d7dddb62d987b3e1c3b8bfad7083f0f774b2 net: store `-whitelist{force}relay` values in `CConnman` (brunoerg) Pull request description: Revives #17167. It allows whitelisting manual connections. Fixes #9923 Since there are some PRs/issues around this topic, I'll list some motivations/comments for whitelisting outbound connections from them: - Speed-up tx relay/mempool sync for testing purposes (my personal motivation for this) - In #26970, theStack pointed out that we whitelist peers to speed up tx relay for fast mempool synchronization, however, since it applies only for inbound connections and considering the topology `node0 <--- node1 <---- node2 <--- ... <-- nodeN`, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long. - https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1865155764 - "Before enabling -v2transport by default (which I'd image may happen after https://github.com/bitcoin/bitcoin/pull/24748) we could consider a way to force manual connections to be only-v1 or even only-v2 (disabling reconnect-with-v1). A possibility could be through a net permission flag, if https://github.com/bitcoin/bitcoin/pull/27114 makes it in." - https://github.com/bitcoin/bitcoin/pull/17167#issuecomment-1168606032 - "This would allow us to use https://github.com/bitcoin/bitcoin/pull/25355 when making outgoing connections to all nodes, except to whitelisted ones for which we would use our persistent I2P address." - Force-relay/mempool permissions for a node you intentionally connected to. ACKs for top commit: achow101: ACK 0a533613fb44207053796fd01a9f4b523a3153d4 sr-gi: re-ACK [0a53361](https://github.com/bitcoin/bitcoin/pull/27114/commits/0a533613fb44207053796fd01a9f4b523a3153d4) pinheadmz: ACK 0a533613fb44207053796fd01a9f4b523a3153d4 Tree-SHA512: 97a79bb854110da04540897d2619eda409d829016aafdf1825ab5515334b0b42ef82f33cd41587af235b3af6ddcec3f2905ca038b5ab22e4c8a03d34f27aebe1
2024-03-12Merge bitcoin/bitcoin#29236: log: Nuke error(...)fanquake
fa391513949b7a3b56321436e2015c7e9e6dac2b refactor: Remove unused error() (MarcoFalke) fad0335517096f435d76adce7833e213d3cc23d1 scripted-diff: Replace error() with LogError() (MarcoFalke) fa808fb74972637840675e310f6d4a0f06028d61 refactor: Make error() return type void (MarcoFalke) fa1d62434843866d242bff9f9c55cb838a4f0d83 scripted-diff: return error(...); ==> error(...); return false; (MarcoFalke) fa9a5e80ab86c997102a1c3d4ba017bbe86641d5 refactor: Add missing {} around error() calls (MarcoFalke) Pull request description: `error(...)` has many issues: * It is often used in the context of `return error(...)`, implying that it has a "fancy" type, creating confusion with `util::Result/Error` * `-logsourcelocations` does not work with it, because it will pretend the error happened inside of `logging.h` * The log line contains `ERROR: `, as opposed to `[error]`, like for other errors logged with `LogError`. Fix all issues by removing it. ACKs for top commit: fjahr: re-utACK fa391513949b7a3b56321436e2015c7e9e6dac2b stickies-v: re-ACK fa391513949b7a3b56321436e2015c7e9e6dac2b, no changes since 4a903741b0 ryanofsky: Code review ACK fa391513949b7a3b56321436e2015c7e9e6dac2b. Just rebase since last review Tree-SHA512: ec5bb502ab0d3733fdb14a8a00762638fce0417afd8dd6294ae0d485ce2b7ca5b1efeb50fc2cd7467f6c652e4ed3e99b0f283b08aeca04bbfb7ea4f2c95d283a
2024-03-11Merge bitcoin/bitcoin#29007: test: create deterministic addrman in the ↵Ava Chow
functional tests 2cc8ca19f4185490f30a49516c890b2289fbab71 [test] Use deterministic addrman in addrman info tests (stratospher) a8978661093500df8d8dcf2a9c90837afacd0aab [test] Restart a node with empty addrman (stratospher) 71c19915c0c716d6f8a539dd92b8ad41e8c447ee [test] Use deterministic addrman in addpeeraddress test (stratospher) 7b868e6b678502e86571976d696c0e3cb72c0884 Revert "test: avoid non-determinism in asmap-addrman test" (stratospher) 69e091f3e1f65b12cf1804e7d39651f55a01827a [init] Create deterministic addrman in tests using -test=addrman (stratospher) be25ac3092b7755e26e1ec6c33a27cd0e3dd9eac [init] Remove -addrmantest command line arg (stratospher) 802e6e128bba5ffa6d4ec53ff45acccb7cb28f21 [init] Add new command line arg for use only in functional tests (stratospher) Pull request description: An address is placed in a `[bucket,position]` in the addrman table (new table or tried table) using the `addpeeraddress` RPC. This `[bucket,position]` is calculated using `nKey`(and other metrics) for the addrman which is chosen randomly during every run. Supposing there are 2 addresses to be placed in an addrman table. During every test run, a different `[bucket,position]` would be calculated for each address.These calculated `[bucket,position]` could even be the same for the 2 addresses in some test runs and result in collisions in the addrman. We wouldn't be able to predict when the collisions are going to happen because we can't predict the `nKey` value which is chosen at random. This can cause flaky tests. Because of these non deterministic collisions, we are limited in what we can do to test addrman functionality. Currently in our tests don't add a second address to prevent these collisions from happening - we only keep 1 address in the new table and 1 address in the tried table. See https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1091145647, https://github.com/bitcoin/bitcoin/pull/23084, [#22831(comment)](https://github.com/bitcoin/bitcoin/pull/22831/files#r708302639). This PR lets us create a deterministic addrman with fixed `nKey` so that we can know the `[bucket,position]` collisions beforehand, safely add more addresses in an addrman table and write more extensive tests. ACKs for top commit: amitiuttarwar: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 achow101: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 0xB10C: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 mzumsande: Code Review ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 Tree-SHA512: 8acd9bdfe7de1eb44d22373bf13533d8ecf602df966fdd5b8b78afcd8cc35a286c95d2712f67a89473a0d68dded7d38f5599f6e4bf95a6589475444545bfb189
2024-03-11scripted-diff: Replace error() with LogError()MarcoFalke
This fixes the log output when -logsourcelocations is used. Also, instead of 'ERROR:', the log will now say '[error]', like other errors logged with LogError. -BEGIN VERIFY SCRIPT- sed -i --regexp-extended 's! error\("([^"]+)"! LogError("\1\\n"!g' $( git grep -l ' error(' ./src/ ) -END VERIFY SCRIPT-
2024-03-01i2p: construct Session with Proxy instead of CServiceMatthew Zipkin
2024-03-01netbase: extend Proxy class to wrap UNIX socket as well as TCPMatthew Zipkin
2024-03-01net: move CreateSock() calls from ConnectNode() to netbase methodsMatthew Zipkin
2024-03-01netbase: refactor CreateSock() to accept sa_family_tMatthew Zipkin
Also implement CService::GetSAFamily() to provide sa_family_t
2024-02-28net: make the list of known message types a compile time constantVasil Dimov
Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size.
2024-02-28Accept "in" and "out" flags to -whitelist to allow whitelisting manual ↵Luke Dashjr
connections
2024-02-28net_processing: Move extra service flag into InitializeNodeLuke Dashjr
2024-02-28net: Move `NetPermissionFlags::Implicit` verification to ↵Luke Dashjr
`AddWhitelistPermissionFlags`
2024-02-21net: Simplify ParamsStream usageRyan Ofsky
Simplify ParamsStream usage in ConvertSeeds now that ParamsStream supports rvalue substream arguments.
2024-02-21serialization: Reverse ParamsStream constructor orderRyan Ofsky
Move parameter argument after stream argument so will be possible to accept multiple variadic parameter arguments in the following commit. Also reverse template parameter order for consistency.
2024-01-31Merge bitcoin/bitcoin#28170: p2p: adaptive connections services flagsAva Chow
27f260aa6e04f82dad78e9a06d58927546143a27 net: remove now unused global 'g_initial_block_download_completed' (furszy) aff7d92b1500e2478ce36a7e86ae47df47dda178 test: add coverage for peerman adaptive connections service flags (furszy) 6ed53602ac7c565273b5722de167cb2569a0e381 net: peer manager, dynamically adjust desirable services flag (furszy) 9f36e591c551ec2e58a6496334541bfdae8fdfe5 net: move state dependent peer services flags (furszy) f9ac96b8d6f4eba23c88f302b22a2c676e351263 net: decouple state independent service flags from desirable ones (furszy) 97df4e38879d2644aeec34c1eef241fed627333e net: store best block tip time inside PeerManager (furszy) Pull request description: Derived from #28120 discussion. By relocating the peer desirable services flags into the peer manager, we allow the connections acceptance process to handle post-IBD potential stalling scenarios. The peer manager will be able to dynamically adjust the services flags based on the node's proximity to the tip (back and forth). Allowing the node to recover from the following post-IBD scenario: Suppose the node has successfully synced the chain, but later experienced dropped connections and remained inactive for a duration longer than the limited peers threshold (the timeframe within which limited peers can provide blocks). In such cases, upon reconnecting to the network, the node might only establish connections with limited peers, filling up all available outbound slots. Resulting in an inability to synchronize the chain (because limited peers will not provide blocks older than the `NODE_NETWORK_LIMITED_MIN_BLOCKS` threshold). ACKs for top commit: achow101: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 vasild: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 naumenkogs: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 mzumsande: Light Code Review ACK 27f260aa6e04f82dad78e9a06d58927546143a27 andrewtoth: ACK 27f260aa6e04f82dad78e9a06d58927546143a27 Tree-SHA512: 07befb9bcd0b60a4e7c45e4429c02e7b6c66244f0910f4b2ad97c9b98258b6f46c914660a717b5ed4ef4814d0dbfae6e18e6559fe9bec7d0fbc2034109200953
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-14 21:29:28 +0000