aboutsummaryrefslogtreecommitdiff
path: root/src/net.h
AgeCommit message (Collapse)Author
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-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-28net: Fix misleading comment for Discoverlaanwj
All network addresses are being iterated over and added, not just the first one per interface.
2024-04-22Gives seednode priority over dnsseed if both are providedSergi Delgado Segura
2024-04-10[net processing] Move nTimeOffset to net_processingdergoegge
2024-02-28Accept "in" and "out" flags to -whitelist to allow whitelisting manual ↵Luke Dashjr
connections
2024-02-28net: Move `NetPermissionFlags::Implicit` verification to ↵Luke Dashjr
`AddWhitelistPermissionFlags`
2024-02-28net: store `-whitelist{force}relay` values in `CConnman`brunoerg
2024-01-31Merge bitcoin/bitcoin#29347: net: enable v2transport by defaultAva Chow
0bef1042ce6c459acb1de965cbccd98867a417f1 net: enable v2transport by default (Pieter Wuille) Pull request description: This enables BIP324's v2 transport by default (see #27634): * Inbound connections will auto-sense whether v1 or v2 is in use. * Automatic outbound connections will use v2 if `NODE_P2P_V2` was set in addr gossip, but retry with v1 if met with immediate failure. * Manual outbound connections will default to v2, but retry with v1 if met with immediate failure. It remains possible to run with `-v2transport=0` to disable all of these, and make all outbound and inbound connections v1. It also remains possible to specify the `v2transport` argument to the `addnode` RPC as `false`, to disable attempting a v2 connection for that particular added node. ACKs for top commit: stratospher: ACK 0bef104. josibake: reACK https://github.com/bitcoin/bitcoin/pull/29347/commits/0bef1042ce6c459acb1de965cbccd98867a417f1 achow101: ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 naumenkogs: ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 theStack: ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 willcl-ark: crACK 0bef1042ce6c459acb1de965cbccd98867a417f1 BrandonOdiwuor: utACK 0bef1042ce6c459acb1de965cbccd98867a417f1 pablomartin4btc: re ACK 0bef1042ce6c459acb1de965cbccd98867a417f1 kristapsk: utACK 0bef1042ce6c459acb1de965cbccd98867a417f1 Tree-SHA512: 3f17a91e318b9304c40c74a7a5b231149f664ae684d13e9739a05be6c05ba9720f3c3c62da6a73ace0ae8ce733f1c8410b211f9fa15694e6a8d28999ab9882d8
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
2024-01-29net: enable v2transport by defaultPieter Wuille
2024-01-29Merge bitcoin/bitcoin#24748: test/BIP324: functional tests for v2 P2P encryptionAva Chow
bc9283c4415a932ec1eeb70ca2aa4399c80437b3 [test] Add functional test to test early key response behaviour in BIP 324 (stratospher) ffe6a56d75c0b47d0729e4e0b7225a827b43ad89 [test] Check whether v2 TestNode performs downgrading (stratospher) ba737358a37438c18f0fba723eab10ccfd9aae9b [test] Add functional tests to test v2 P2P behaviour (stratospher) 4115cf995647d1a513caecb54a4ff3f51927aa8e [test] Ignore BIP324 decoy messages (stratospher) 8c054aa04d33b247744b3747cd5bf3005a013e90 [test] Allow inbound and outbound connections supporting v2 P2P protocol (stratospher) 382894c3acd2dbf3e4198814f547c75b6fb17706 [test] Reconnect using v1 P2P when v2 P2P terminates due to magic byte mismatch (stratospher) a94e350ac0e5b65ef23a84b05fb10d1204c98c97 [test] Build v2 P2P messages (stratospher) bb7bffed799dc5ad8b606768164fce46d4cbf9d0 [test] Use lock for sending P2P messages in test framework (stratospher) 5b91fb14aba7d7fe45c9ac364526815bec742356 [test] Read v2 P2P messages (stratospher) 05bddb20f5cc9036fd680500bde8ece70dbf0646 [test] Perform initial v2 handshake (stratospher) a049d1bd08c8cdb3b693520f24f8a82572dcaab1 [test] Introduce EncryptedP2PState object in P2PConnection (stratospher) b89fa59e715a185d9fa7fce089dad4273d3b1532 [test] Construct class to handle v2 P2P protocol functions (stratospher) 8d6c848a48530893ca40be5c1285541b3e7a94f3 [test] Move MAGIC_BYTES to messages.py (stratospher) 595ad4b16880ae1f23463ca9985381c8eae945d8 [test/crypto] Add ECDH (stratospher) 4487b8051797173c7ab432e75efa370afb03b529 [rpc/net] Allow v2 p2p support in addconnection (stratospher) Pull request description: This PR introduces support for v2 P2P encryption(BIP 324) in the existing functional test framework and adds functional tests for the same. ### commits overview 1. introduces a new class `EncryptedP2PState` to store the keys, functions for performing the initial v2 handshake and encryption/decryption. 3. this class is used by `P2PConnection` in inbound/outbound connections to perform the initial v2 handshake before the v1 version handshake. Only after the initial v2 handshake is performed do application layer P2P messages(version, verack etc..) get exchanged. (in a v2 connection) - `v2_state` is the object of class `EncryptedP2PState` in `P2PConnection` used to store its keys, session-id etc. - a node [advertising](https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md#advertising-to-support-v2-p2p) support for v2 P2P is different from a node actually [supporting v2 P2P](https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md#supporting-v2-p2p) (differ when false advertisement of services occur) - introduce a boolean variable `supports_v2_p2p` in `P2PConnection` to denote if it supports v2 P2P. - introduce a boolean variable `advertises_v2_p2p` to denote whether `P2PConnection` which mimics peer behaviour advertises V2 P2P support. Default option is `False`. - In the test framework, you can create Inbound and Outbound connections to `TestNode` 1. During **Inbound Connections**, `P2PConnection` is the initiator [`TestNode` <--------- `P2PConnection`] - Case 1: - if the `TestNode` advertises/signals v2 P2P support (means `self.nodes[i]` set up with `"-v2transport=1"`), different behaviour will be exhibited based on whether: 1. `P2PConnection` supports v2 P2P 2. `P2PConnection` does not support v2 P2P - In a real world scenario, the initiator node would intrinsically know if they support v2 P2P based on whatever code they choose to run. However, in the test scenario where we mimic peer behaviour, we have no way of knowing if `P2PConnection` should support v2 P2P or not. So `supports_v2_p2p` boolean variable is used as an option to enable support for v2 P2P in `P2PConnection`. - Since the `TestNode` advertises v2 P2P support (using "-v2transport=1"), our initiator `P2PConnection` would send: 1. (if the `P2PConnection` supports v2 P2P) ellswift + garbage bytes to initiate the connection 2. (if the `P2PConnection` does not support v2 P2P) version message to initiate the connection - Case 2: - if the `TestNode` doesn't signal v2 P2P support; `P2PConnection` being the initiator would send version message to initiate a connection. 2. During **Outbound Connections** [TestNode --------> P2PConnection] - initiator `TestNode` would send: - (if the `P2PConnection` advertises v2 P2P) ellswift + garbage bytes to initiate the connection - (if the `P2PConnection` advertises v2 P2P) version message to initiate the connection - Suppose `P2PConnection` advertises v2 P2P support when it actually doesn't support v2 P2P (false advertisement scenario) - `TestNode` sends ellswift + garbage bytes - `P2PConnection` receives but can't process it and disconnects. - `TestNode` then tries using v1 P2P and sends version message - `P2PConnection` receives/processes this successfully and they communicate on v1 P2P 4. the encrypted P2P messages follow a different format - 3 byte length + 1-13 byte message_type + payload + 16 byte MAC 5. includes support for testing decoy messages and v2 connection downgrade(using false advertisement - when a v2 node makes an outbound connection to a node which doesn't support v2 but is advertised as v2 by some malicious intermediary) ### run the tests * functional test - `test/functional/p2p_v2_encrypted.py` `test/functional/p2p_v2_earlykeyresponse.py` I'm also super grateful to @ dhruv for his really valuable feedback on this branch. Also written a more elaborate explanation here - https://github.com/stratospher/blogosphere/blob/main/integration_test_bip324.md ACKs for top commit: naumenkogs: ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 mzumsande: Code Review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 theStack: Code-review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 glozow: ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3 Tree-SHA512: 9b54ed27e925e1775e0e0d35e959cdbf2a9a1aab7bcf5d027e66f8b59780bdd0458a7a4311ddc7dd67657a4a2a2cd5034ead75524420d58a83f642a8304c9811
2024-01-23[rpc/net] Allow v2 p2p support in addconnectionstratospher
This test-only RPC is required when a TestNode initiates an outbound v2 p2p connection. Add a new arg `v2transport` so that the node can attempt v2 connections.
2024-01-15net: move state dependent peer services flagsfurszy
No behavior change. Just an intermediate refactoring. By relocating the peer desirable services flags into the peer manager, we allow the connections acceptance process to handle post-IBD potential stalling scenarios. In the follow-up commit(s), the desirable service flags will be dynamically adjusted to detect post-IBD stalling scenarios (such as a +48-hour inactive node that must prefer full node connections instead of limited peer connections because they cannot provide historical blocks). Additionally, this encapsulation enable us to customize the connections decision-making process based on new user's configurations in the future.
2023-12-27net: attempt v2 transport for manual connections if we support itMartin Zumsande
This affects manual connections made either with -connect, or with -addnode provided as a bitcoind config arg (the addnode RPC has an extra option for v2). We don't necessarily know if our peer supports v2, but will reconnect with v1 if they don't. In order to do that, improve the reconnection behavior such that we will reconnect after a sleep of 500ms (which usually should be enough for our peer to send us their version message).
2023-12-06Merge bitcoin/bitcoin#27581: net: Continuous ASMap health checkAndrew Chow
3ea54e5db7d53da5afa321e1800c29aa269dd3b3 net: Add continuous ASMap health check logging (Fabian Jahr) 28d7e55dff826a69f3f8e58139dbffb611cc5947 test: Add tests for unfiltered GetAddr usage (Fabian Jahr) b8843d37aed1276ff8527328c956c70c6e02ee13 fuzz: Let fuzzers use filter options in GetAddr/GetAddresses (Fabian Jahr) e16f420547fc72a5a2902927aa7138e43c0fb7c8 net: Optionally include terrible addresses in GetAddr results (Fabian Jahr) Pull request description: There are certain statistics we can collect by running all our known clearnet addresses against the ASMap file. This could show issues with a maliciously manipulated file or with an old file that has decayed with time. This is just a proof of concept for now. My idea currently is to run the analysis once per day and print the results to logs if an ASMap file is used. ACKs for top commit: achow101: ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 mzumsande: ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 brunoerg: crACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 Tree-SHA512: 777acbfac43cc43ce4a0a3612434e4ddbc65f59ae8ffc9e24f21de09011bccb297f0599cbaa82bcf40ef68e5af582c4e98556379db7ceff7d9f97574a1cf8e09
2023-12-02net: Add continuous ASMap health check loggingFabian Jahr
2023-11-30Rename version.h to node/protocol_version.hMarcoFalke
2023-11-28Merge bitcoin/bitcoin#28892: refactor: P2P transport without serialize ↵fanquake
version and type fa79a881ce0537e1d74da296a7513730438d2a02 refactor: P2P transport without serialize version and type (MarcoFalke) fa9b5f4fe32c0cfe2e477bb11912756f84a52cfe refactor: NetMsg::Make() without nVersion (MarcoFalke) 66669da4a5ca9edf2a40d20879d9a8aaf2b9e2ee Remove unused Make() overload in netmessagemaker.h (MarcoFalke) fa0ed0794161d937d2d3385963c1aa5624b60d17 refactor: VectorWriter without nVersion (MarcoFalke) Pull request description: Now that the serialize framework ignores the serialize version and serialize type, everything related to it can be removed from the code. This is the first step, removing dead code from the P2P stack. A different pull will remove it from the wallet and other parts. ACKs for top commit: ajtowns: reACK fa79a881ce0537e1d74da296a7513730438d2a02 Tree-SHA512: 785b413580d980f51f0d4f70ea5e0a99ce14cd12cb065393de2f5254891be94a14f4266110c8b87bd2dbc37467676655bce13bdb295ab139749fcd8b61bd5110
2023-11-23refactor: P2P transport without serialize version and typeMarcoFalke
2023-11-22Merge bitcoin/bitcoin#28895: p2p: do not make automatic outbound connections ↵fanquake
to addnode peers 5e7cc4144bb09cfde48ba5bc055a2da3ca4252d1 test: add unit test for CConnman::AddedNodesContain() (Jon Atack) cc627169206fe902157806d88fcaf2b05701c38d p2p: do not make automatic outbound connections to addnode peers (Jon Atack) Pull request description: to allocate our limited outbound slots correctly, and to ensure addnode connections benefit from their intended protections. Our addnode logic usually connects the addnode peers before the automatic outbound logic does, but not always, as a connection race can occur. If an addnode peer disconnects us and if it was the only one from its network, there can be a race between reconnecting to it with the addnode thread, and it being picked as automatic network-specific outbound peer. Or our internet connection or router or the addnode peer could be temporarily offline, and then return online during the automatic outbound thread. Or we could add a new manual peer using the addnode RPC at that time. The race can be more apparent when our node doesn't know many peers, or with networks like cjdns that currently have few bitcoin peers. When an addnode peer is connected as an automatic outbound peer and is the only connection we have to a network, it can be protected by our new outbound eviction logic and persist in the "wrong role". Finally, there does not seem to be a reason to make block-relay or short-lived feeler connections to addnode peers, as the addnode logic will ensure we connect to them if they are up, within the addnode connection limit. Fix these issues by checking if the address is an addnode peer in our automatic outbound connection logic. ACKs for top commit: mzumsande: Tested ACK 5e7cc4144bb09cfde48ba5bc055a2da3ca4252d1 brunoerg: utACK 5e7cc4144bb09cfde48ba5bc055a2da3ca4252d1 vasild: ACK 5e7cc4144bb09cfde48ba5bc055a2da3ca4252d1 guggero: utACK 5e7cc4144bb09cfde48ba5bc055a2da3ca4252d1 Tree-SHA512: 2438c3ec92e98aebca2a0da960534e4655a9c6e1192a24a085fc01326d95cdb1b67d8c44e4ee706bc1d8af8564126d446a21b5579dcbec61bdea5fce2f0115ee
2023-11-16p2p: do not make automatic outbound connections to addnode peersJon Atack
to allocate our limited outbound slots correctly, and to ensure addnode connections benefit from their intended protections. Our addnode logic usually connects the addnode peers before the automatic outbound logic does, but not always, as a connection race can occur. If an addnode peer disconnects us and if it was the only one from its network, there can be a race between reconnecting to it with the addnode thread, and it being picked as automatic network-specific outbound peer. Or our internet connection or router, or the addnode peer, could be temporarily offline, and then return online during the automatic outbound thread. Or we could add a new manual peer using the addnode RPC at that time. The race can be more apparent when our node doesn't know many peers, or with networks like cjdns that currently have few bitcoin peers. When an addnode peer is connected as an automatic outbound peer and is the only connection we have to a network, it can be protected by our new outbound eviction logic and persist in the "wrong role". Examples on mainnet using logging added in the same pull request: 2023-08-12T14:51:05.681743Z [opencon] [net.cpp:1949] [ThreadOpenConnections] [net:debug] Not making automatic network-specific outbound-full-relay connection to i2p peer selected for manual (addnode) connection: [geh...odq.b32.i2p]:0 2023-08-13T03:59:28.050853Z [opencon] [net.cpp:1949] [ThreadOpenConnections] [net:debug] Not making automatic block-relay-only connection to onion peer selected for manual (addnode) connection: kpg...aid.onion:8333 2023-08-13T16:21:26.979052Z [opencon] [net.cpp:1949] [ThreadOpenConnections] [net:debug] Not making automatic network-specific outbound-full-relay connection to cjdns peer selected for manual (addnode) connection: [fcc...8ce]:8333 2023-08-14T20:43:53.401271Z [opencon] [net.cpp:1949] [ThreadOpenConnections] [net:debug] Not making automatic network-specific outbound-full-relay connection to cjdns peer selected for manual (addnode) connection: [fc7...59e]:8333 2023-08-15T00:10:01.894147Z [opencon] [net.cpp:1949] [ThreadOpenConnections] [net:debug] Not making automatic feeler connection to i2p peer selected for manual (addnode) connection: geh...odq.b32.i2p:8333 Finally, there does not seem to be a reason to make block-relay or short-lived feeler connections to addnode peers, as the addnode logic will ensure we connect to them if they are up, within the addnode connection limit. Fix these issues by checking if the address is an addnode peer in our automatic outbound connection logic.
2023-11-16Include version.h in fewer placesAnthony Towns
2023-11-13refactor: Initialize magic bytes in constructor initializerTheCharlatan
Also remove an assert that is already enforced by the compiler checking that the length of the std::array matches.
2023-11-08Merge bitcoin/bitcoin#28155: net: improves addnode / m_added_nodes logicglozow
0420f99f429ce2382057e101859067f40de47be0 Create net_peer_connection unit tests (Jon Atack) 4b834f649921aceb44d3e0b5a2ffd7847903f9f7 Allow unit tests to access additional CConnman members (Jon Atack) 34b9ef443bc2655a85c8802edc5d5d48d792a286 net/rpc: Makes CConnman::GetAddedNodeInfo able to return only non-connected address on request (Sergi Delgado Segura) 94e8882d820969ddc83f24f4cbe1515a886da4ea rpc: Prevents adding the same ip more than once when formatted differently (Sergi Delgado Segura) 2574b7e177ef045e64f1dd48cb000640ff5103d3 net/rpc: Check all resolved addresses in ConnectNode rather than just one (Sergi Delgado Segura) Pull request description: ## Rationale Currently, `addnode` has a couple of corner cases that allow it to either connect to the same peer more than once, hence wasting outbound connection slots, or add redundant information to `m_added_nodes`, hence making Bitcoin iterate through useless data on a regular basis. ### Connecting to the same node more than once In general, connecting to the same node more than once is something we should try to prevent. Currently, this is possible via `addnode` in two different ways: 1. Calling `addnode` more than once in a short time period, using two equivalent but distinct addresses 2. Calling `addnode add` using an IP, and `addnode onetry` after with an address that resolved to the same IP For the former, the issue boils down to `CConnman::ThreadOpenAddedConnections` calling `CConnman::GetAddedNodeInfo` once, and iterating over the result to open connections (`CConman::OpenNetworkConnection`) on the same loop for all addresses.`CConnman::ConnectNode` only checks a single address, at random, when resolving from a hostname, and uses it to check whether we are already connected to it. An example to test this would be calling: ``` bitcoin-cli addnode "127.0.0.1:port" add bitcoin-cli addnode "localhost:port" add ``` And check how it allows us to perform both connections some times, and some times it fails. The latter boils down to the same issue, but takes advantage of `onetry` bypassing the `CConnman::ThreadOpenAddedConnections` logic and calling `CConnman::OpenNetworkConnection` straightaway. A way to test this would be: ``` bitcoin-cli addnode "127.0.0.1:port" add bitcoin-cli addnode "localhost:port" onetry ``` ### Adding the same peer with two different, yet equivalent, addresses The current implementation of `addnode` is pretty naive when checking what data is added to `m_added_nodes`. Given the collection stores strings, the checks at `CConnman::AddNode()` basically check wether the exact provided string is already in the collection. If so, the data is rejected, otherwise, it is accepted. However, ips can be formatted in several ways that would bypass those checks. Two examples would be `127.0.0.1` being equal to `127.1` and `[::1]` being equal to `[0:0:0:0:0:0:0:1]`. Adding any pair of these will be allowed by the rpc command, and both will be reported as connected by `getaddednodeinfo`, given they map to the same `CService`. This is less severe than the previous issue, since even tough both nodes are reported as connected by `getaddednodeinfo`, there is only a single connection to them (as properly reported by `getpeerinfo`). However, this adds redundant data to `m_added_nodes`, which is undesirable. ### Parametrize `CConnman::GetAddedNodeInfo` Finally, this PR also parametrizes `CConnman::GetAddedNodeInfo` so it returns either all added nodes info, or only info about the nodes we are **not** connected to. This method is used both for `rpc`, in `getaddednodeinfo`, in which we are reporting all data to the user, so the former applies, and to check what nodes we are not connected to, in `CConnman::ThreadOpenAddedConnections`, in which we are currently returning more data than needed and then actively filtering using `CService.fConnected()` ACKs for top commit: jonatack: re-ACK 0420f99f429ce2382057e101859067f40de47be0 kashifs: > > tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0) sr-gi: > > > tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0) mzumsande: Tested ACK 0420f99f429ce2382057e101859067f40de47be0 Tree-SHA512: a3a10e748c12d98d439dfb193c75bc8d9486717cda5f41560f5c0ace1baef523d001d5e7eabac9fa466a9159a30bb925cc1327c2d6c4efb89dcaf54e176d1752
2023-11-07Merge bitcoin/bitcoin#28464: net: improve max-connection limits codeAndrew Chow
df69b22f2e3cc03764a582f29a16a36114f67e17 doc: improve documentation around connection limit maximums (Amiti Uttarwar) adc171edf45ec90857d990b8ec570f3c8c2242b7 scripted-diff: Rename connection limit variables (Amiti Uttarwar) e9fd9c0225527ec7727d2a7ccbdf028784aadc6c net: add m_max_inbound to connman (Amiti Uttarwar) c25e0e05550426f29d79571368d90f63fb472b02 net, refactor: move calculations for connection type limits into connman (Amiti Uttarwar) Pull request description: This is joint work with amitiuttarwar. This has the first few commits of #28463. It is not strictly a prerequisite for that, but has changes that in our opinion make sense on their own. It improves the handling of maximum numbers for different connection types (that are set during init and don’t change after) by: * moving all calculations into one place, `CConnMan::Init()`. Before, they were dispersed between `Init`, `CConnman::Init` and other parts of `CConnman`, resulting in some duplicated test code. * removing the possibility of having a negative maximum of inbound connections, which is hard to argue about * renaming of variables and doc improvements ACKs for top commit: amitiuttarwar: co-author review ACK df69b22f2e3cc03764a582f29a16a36114f67e17 naumenkogs: ACK df69b22f2e3cc03764a582f29a16a36114f67e17 achow101: ACK df69b22f2e3cc03764a582f29a16a36114f67e17 Tree-SHA512: 913d56136bc1df739978de50db67302f88bac2a9d34748ae96763288d97093e998fc0f94f9b6eff12867712d7e86225af6128f4170bf2b5b8ab76f024870a22c
2023-10-30net/rpc: Makes CConnman::GetAddedNodeInfo able to return only non-connected ↵Sergi Delgado Segura
address on request `CConnman::GetAddedNodeInfo` is used both to get a list of addresses to manually connect to in `CConnman::ThreadOpenAddedConnections`, and to report about manually added connections in `getaddednodeinfo`. In both cases, all addresses added to `m_added_nodes` are returned, however the nodes we are already connected to are only relevant to the latter, in the former they are actively discarded. Parametrizes `CConnman::GetAddedNodeInfo` so we can ask for only addresses we are not connected to, to avoid passing useless information around.
2023-10-16net: remove unused CConnman::FindNode(const CSubNet&)Vasil Dimov
2023-10-05net: move MaybeFlipIPv6toCJDNS() from net to netbaseVasil Dimov
It need not be in the `net` module and we need to call it from `LookupSubNet()`, thus move it to `netbase`.
2023-10-05net: move IsReachable() code to netbase and encapsulate itVasil Dimov
`vfLimited`, `IsReachable()`, `SetReachable()` need not be in the `net` module. Move them to `netbase` because they will be needed in `LookupSubNet()` to possibly flip the result to CJDNS (if that network is reachable). In the process, encapsulate them in a class. `NET_UNROUTABLE` and `NET_INTERNAL` are no longer ignored when adding or removing reachable networks. This was unnecessary.
2023-10-04net: Optionally include terrible addresses in GetAddr resultsFabian Jahr
2023-10-04net: raise V1_PREFIX_LEN from 12 to 16Pieter Wuille
A "version" message in the V1 protocol starts with a fixed 16 bytes: * The 4-byte network magic * The 12-byte zero-padded command "version" plus 5 0x00 bytes The current code detects incoming V1 connections by just looking at the first 12 bytes (matching an earlier version of BIP324), but 16 bytes is more precise. This isn't an observable difference right now, as a 12 byte prefix ought to be negligible already, but it may become observable with future extensions to the protocol, so make the code match the specification.
2023-10-03doc: improve documentation around connection limit maximumsAmiti Uttarwar
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-10-03scripted-diff: Rename connection limit variablesAmiti Uttarwar
-BEGIN VERIFY SCRIPT- sed -i 's/nMaxConnections/m_max_automatic_connections/g' src/net.h src/net.cpp sed -i 's/\.nMaxConnections/\.m_max_automatic_connections/g' src/init.cpp src/test/denialofservice_tests.cpp sed -i 's/nMaxFeeler/m_max_feeler/g' src/net.h sed -i 's/nMaxAddnode/m_max_addnode/g' src/net.h src/net.cpp sed -i 's/m_max_outbound\([^_]\)/m_max_automatic_outbound\1/g' src/net.h src/net.cpp -END VERIFY SCRIPT- Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-10-03net: add m_max_inbound to connmanAmiti Uttarwar
Extract the logic for calculating & maintaining inbound connection limits to be a member within connman for consistency with other maximum connection limits. Note that we now limit m_max_inbound to 0 and don't call AttemptToEvictConnection() when we don't have any inbounds. Previously, nMaxInbound could become negative if the user ran with a low -maxconnections, which didn't break any logic but didn't make sense. Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-10-03net, refactor: move calculations for connection type limits into connmanAmiti Uttarwar
Currently the logic is fragmented between init and connman. Encapsulating this logic within connman allows for less mental overhead and easier reuse in tests. Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-10-02net: expose transport types/session IDs of connections in RPC and logsPieter Wuille
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
2023-10-02net: reconnect with V1Transport under certain conditionsPieter Wuille
When an outbound v2 connection is disconnected without receiving anything, but at least 24 bytes of our pubkey were sent out (enough to constitute an invalid v1 header), add them to a queue of reconnections to be tried. The reconnections are in a queue rather than performed immediately, because we should not block the socket handler thread with connection creation (a blocking operation that can take multiple seconds).
2023-10-02sync: modernize CSemaphore / CSemaphoreGrantPieter Wuille
2023-10-02rpc: addnode arg to use BIP324 v2 p2pdhruv
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
2023-10-02net: use V2Transport when NODE_P2P_V2 service flag is presentPieter Wuille
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
2023-10-02net: advertise NODE_P2P_V2 if CLI arg -v2transport is onPieter Wuille
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
2023-09-27net: Simplify v2 recv logic by decoupling AAD from state machineTim Ruffing
2023-09-27net: Drop v2 garbage authentication packetTim Ruffing
See also https://github.com/bitcoin/bips/pull/1498 The benefit is a simpler implementation: - The protocol state machine does not need separate states for garbage authentication and version phases. - The special case of "ignoring the ignore bit" is removed. - The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing.
2023-09-21Merge bitcoin/bitcoin#28078: net, refactor: remove unneeded exports, use ↵Andrew Chow
helpers over low-level code, use optional 4ecfd3eaf434d868455466e001adae4b68515ab8 Inline short, often-called, rarely-changed basic CNetAddr getters (Jon Atack) 5316ae5dd8d90623f9bb883bb253fa6463ee4d34 Convert GetLocal() to std::optional and remove out-param (Jon Atack) f1304db136bbeac70b52522b5a4522165bfb3fc0 Use higher-level CNetAddr and CNode helpers in net.cpp (Jon Atack) 07f589158835151a7613e4b2a508c0dd61a18fd7 Add CNetAddr::IsPrivacyNet() and CNode::IsConnectedThroughPrivacyNet() (Jon Atack) df488563b280c63f5d74d5ac0fcb1a2cad489d55 GetLocal() type-safety, naming, const, and formatting cleanups (stickies-v) fb4265747c9eb022d80c6f2988e574c689130348 Add and use CNetAddr::HasCJDNSPrefix() helper (Jon Atack) 5ba73cd0ee1e661ec4d041ac8ae7a60cfd31f0c0 Move GetLocal() declaration from header to implementation (Jon Atack) 11426f6557ac09489d5e13bf3b9d94fbd5073c8e Move CaptureMessageToFile() declaration from header to implementation (Jon Atack) deccf1c4848620cfff2a9642c5a6b5acdfbc33bd Move IsPeerAddrLocalGood() declaration from header to implementation (Jon Atack) Pull request description: and other improvements noticed while reviewing #27411. Addresses https://github.com/bitcoin/bitcoin/pull/27411#discussion_r1263969104 and https://github.com/bitcoin/bitcoin/pull/27411#discussion_r1263967598. See commit messages for details. ACKs for top commit: achow101: ACK 4ecfd3eaf434d868455466e001adae4b68515ab8 vasild: ACK 4ecfd3eaf434d868455466e001adae4b68515ab8 stickies-v: ACK 4ecfd3eaf434d868455466e001adae4b68515ab8 Tree-SHA512: d792bb2cb24690aeae9bedf97e92b64fb6fd080c39385a4bdb8ed05f37f3134d85bda99da025490829c03017fd56382afe7951cdd039aede1c08ba98fb1aa7f9
2023-09-12[refactor] Remove netaddress.h from kernel headersTheCharlatan
Move functions requiring the netaddress.h include out of libbitcoinkernel source files. The netaddress.h file contains many non-consensus related definitions and should thus not be part of the libbitcoinkernel. This commit makes netaddress.h no longer a required include for users of the libbitcoinkernel. This commit is part of the libbitcoinkernel project, namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel.
2023-09-12[refactor] Add CChainParams member to CConnmanTheCharlatan
This is done in preparation to the next commit, but has the nice effect of removing one further data structure relying on the global `Params()`.
2023-09-12kernel: Move MessageStartChars to its own fileTheCharlatan
The protocol.h file contains many non-consensus related definitions and should thus not be part of the libbitcoinkernel. This commit makes protocol.h no longer a required include for users of the libbitcoinkernel. This commit is part of the libbitcoinkernel project, namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel. Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
2023-09-10doc: fix typos and mistakes in BIP324 code commentsPieter Wuille
2023-09-10net: do not use send buffer to store/cache garbagePieter Wuille
Before this commit the V2Transport::m_send_buffer is used to store the garbage: * During MAYBE_V1 state, it's there despite not being sent. * During AWAITING_KEY state, while it is being sent. * At the end of the AWAITING_KEY state it cannot be wiped as it's still needed to compute the garbage authentication packet. Change this by introducing a separate m_send_garbage field, taking over the first and last role listed above. This means the garbage is only in the send buffer when it's actually being sent, removing a few special cases related to this.
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-18 10:21:15 +0000