aboutsummaryrefslogtreecommitdiff
path: root/src/net.cpp
AgeCommit message (Collapse)Author
2020-09-03Merge #19670: Protect localhost and block-relay-only peers from evictionWladimir J. van der Laan
752e6ad5336d5af0db9fe16d24c0c6aa25b74a3f Protect localhost and block-relay-only peers from eviction (Suhas Daftuar) Pull request description: Onion peers are disadvantaged under our eviction criteria, so prevent eventual eviction of them in the presence of contention for inbound slots by reserving some slots for localhost peers (sorted by longest uptime). Block-relay-only connections exist as a protection against eclipse attacks, by creating a path for block propagation that may be unknown to adversaries. Protect against inbound peer connection slot attacks from disconnecting such peers by attempting to protect up to 8 peers that are not relaying transactions but have provided us with blocks. Thanks to gmaxwell for suggesting these strategies. ACKs for top commit: laanwj: Code review ACK 752e6ad5336d5af0db9fe16d24c0c6aa25b74a3f Tree-SHA512: dbf089c77c1f747aa1dbbbc2e9c2799c628028b0918d0c336d8d0e5338acedd573b530eb3b689c7f603a17221e557268a9f5c3f585f204bfb12e5d2e76de39a3
2020-09-02[doc] Follow developer notes, add comment about missing default.Amiti Uttarwar
2020-09-02[refactor] Simplify connection type logic in ThreadOpenConnectionsAmiti Uttarwar
Consolidate the logic to determine connection type into one conditional to clarify how they are chosen.
2020-09-02[trivial] Small style updatesAmiti Uttarwar
2020-09-02[refactor] Restructure logic to check for addr relay.Amiti Uttarwar
We previously identified if we relay addresses to the connection by checking for the existence of the m_addr_known data structure. With this commit, we answer this question based on the connection type. IsAddrRelayPeer() checked for the existence of the m_addr_known
2020-09-02scripted-diff: Rename `OUTBOUND` ConnectionType to `OUTBOUND_FULL_RELAY`Amiti Uttarwar
-BEGIN VERIFY SCRIPT- sed -i 's/OUTBOUND, /OUTBOUND_FULL_RELAY, /g' src/net.h sed -i 's/ConnectionType::OUTBOUND/ConnectionType::OUTBOUND_FULL_RELAY/g' src/test/net_tests.cpp src/test/fuzz/process_message.cpp src/test/fuzz/process_messages.cpp src/net.cpp src/test/denialofservice_tests.cpp src/net.h src/test/fuzz/net.cpp -END VERIFY SCRIPT-
2020-09-02Protect localhost and block-relay-only peers from evictionSuhas Daftuar
Onion peers are disadvantaged under our eviction criteria, so prevent eventual eviction of them in the presence of contention for inbound slots by reserving some slots for localhost peers (sorted by longest uptime). Block-relay-only connections exist as a protection against eclipse attacks, by creating a path for block propagation that may be unknown to adversaries. Protect against inbound peer connection slot attacks from disconnecting such peers by attempting to protect up to 8 peers that are not relaying transactions but appear to be full-nodes, sorted by recency of last delivered block. Thanks to gmaxwell for suggesting these strategies.
2020-09-01net: improve nLastBlockTime and nLastTXTime documentationJon Atack
Co-authored-by: John Newbery <john@johnnewbery.com>
2020-08-15net: add nLastBlockTime/TXTime to CNodeStats, CNode::copyStatsJon Atack
2020-08-12Merge #19658: [rpc] Allow RPC to fetch all addrman records and add records ↵Wladimir J. van der Laan
to addrman 37a480e0cd94895b6051abef12d984ff74bdc4a3 [net] Add addpeeraddress RPC method (John Newbery) ae8051bbd8377f2458ff1f167dc30c2d5f83e317 [test] Test that getnodeaddresses() can return all known addresses (John Newbery) f26502e9fc8a669b30717525597e3f468eaecf79 [addrman] Specify max addresses and pct when calling GetAddresses() (John Newbery) Pull request description: Currently addrman only allows a maximum of 1000 records or 23% of all records to be returned in a call to `GetAddr()`. Relax this limit and have the client specify the max records they want. For p2p, behaviour is unchanged (but the rate limiting is set inside net_processing, where it belongs). For RPC, `getnodeaddresses` can now return the complete addrman, which is helpful for testing and monitoring. Also add a test-only RPC `addpeeraddress`, which adds an IP address:port to addrman. This is helpful for testing (eg #18991). ACKs for top commit: naumenkogs: utACK 37a480e0cd94895b6051abef12d984ff74bdc4a3 laanwj: Code review and lightly manually tested ACK 37a480e0cd94895b6051abef12d984ff74bdc4a3 Tree-SHA512: f86dcd410aaebaf6e9ca18ce6f23556e5e4649c1325577213d873aa09967298e65ab2dc19a72670641ae92211a923afda1fe124a82e9d2c1cad73d478ef27fdc
2020-08-12[net] Add addpeeraddress RPC methodJohn Newbery
Allows addresses to be added to Address Manager for testing.
2020-08-12[addrman] Specify max addresses and pct when calling GetAddresses()John Newbery
CAddrMan.GetAddr() would previously limit the number and percentage of addresses returned (to ADDRMAN_GETADDR_MAX (1000) and ADDRMAN_GETADDR_MAX_PCT (23) respectively). Instead, make it the callers responsibility to specify the maximum addresses and percentage they want returned. For net_processing, the maximums are MAX_ADDR_TO_SEND (1000) and MAX_PCT_ADDR_TO_SEND (23). For rpc/net, the maximum is specified by the client.
2020-08-12Merge #19316: [net] Cleanup logic around connection typesfanquake
01e283068b9e6214f2d77a2f772a4244ebfe2274 [net] Remove unnecessary default args on CNode constructor (Amiti Uttarwar) bc5d65b3ca41eebb1738fdda4451d1466e77772e [refactor] Remove IsOutboundDisconnectionCandidate (Amiti Uttarwar) 2f2e13b6c2c8741ca9d825eaaef736ede484bc85 [net/refactor] Simplify multiple-connection checks (Amiti Uttarwar) 7f7b83deb2427599c129f4ff581d4d045461e459 [net/refactor] Rework ThreadOpenConnections logic (Amiti Uttarwar) 35839e963bf61d2da0d12f5b8cea74ac0e0fbd7b [net] Fix bug where AddrFetch connections would be counted as outbound full relay (Amiti Uttarwar) 4972c21b671ff73f13a1b5053338b6abbdb471b5 [net/refactor] Clarify logic for selecting connections in ThreadOpenConnections (Amiti Uttarwar) 60156f5fc40d56bb532278f16ce632c5a8b8035e [net/refactor] Remove fInbound flag from CNode (Amiti Uttarwar) 7b322df6296609570e368e5f326979279041c11f [net/refactor] Remove m_addr_fetch member var from CNode (Amiti Uttarwar) 14923422b08ac4b21b35c426bf0e1b9e7c97983b [net/refactor] Remove fFeeler flag from CNode (Amiti Uttarwar) 49efac5cae7333c6700d9b737d09fae0f3f4d7fa [net/refactor] Remove m_manual_connection flag from CNode (Amiti Uttarwar) d3698b5ee309cf0f0cdfb286d6b30a256d7deae5 [net/refactor] Add connection type as a member var to CNode (Amiti Uttarwar) 46578c03e92a55925308363ccdad04dcfc820d96 [doc] Describe different connection types (Amiti Uttarwar) 442abae2bac7bff85886143df01e14215532b974 [net/refactor] Add AddrFetch connections to ConnectionType enum (Amiti Uttarwar) af59feb05235ecb85ec9d75b09c66e71268c9889 [net/refactor] Extract m_addr_known logic from initializer list (Amiti Uttarwar) e1bc29812ddf1d946bc5acca406a7ed2dca064a6 [net/refactor] Add block relay only connections to ConnectionType enum (Amiti Uttarwar) 0e52a659a2de915fc3dce37fc8fac39be1c8b6fa [net/refactor] Add feeler connections to ConnectionType enum (Amiti Uttarwar) 1521c47438537e192230486dffcec0228a53878d [net/refactor] Add manual connections to ConnectionType enum (Amiti Uttarwar) 26304b4100201754fb32440bec3e3b78cd3f0e6d [net/refactor] Introduce an enum to distinguish type of connection (Amiti Uttarwar) 3f1b7140e95d0f8f958cb35f31c3d964c57e484d scripted-diff: Rename OneShot to AddrFetch (Amiti Uttarwar) Pull request description: **This is part 1 of #19315, which enables the ability to test `outbound` and `block-relay-only` connections from the functional tests.** Please see that PR for more information of overall functionality. **This PR simplifies how we manage different connection types.** It introduces an enum with the various types of connections so we can explicitly define the connection type. The existing system relies on a series of independent flags, then has asserts scattered around to ensure that conflicting flags are not enabled at the same time. I find this approach to be both brittle and confusing. While making these changes, I found a small bug due to the silent assumptions. This PR also proposes a rename from `OneShot` to `AddrFetch`. I find the name `OneShot` to be very confusing, especially when we also have `onetry` manual connections. Everyone I've talked to offline has agreed that the name is confusing, so I propose a potential alternative. I think this is a good opportunity for a rename since I'm creating an enum to explicitly define the connection types. (some context for the unfamiliar: `oneshot` or `addrfetch` connections are short-lived connections created on startup. They connect to the seed peers, send a `getaddr` to solicit addresses, then close the connection.) Overview of this PR: * rename `oneshot` to `addrfetch` * introduce `ConnectionType` enum * one by one, add different connection types to the enum * expose the `conn_type` on CNode, and use this to reduce reliance on flags (& asserts) * fix the bug in counting different type of connections * some additional cleanup to simplify logic and make expectations explicit/inclusive rather than implicit/exclusive. ACKs for top commit: jnewbery: utACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 laanwj: Code review ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274, the commits are pretty straightforward to follow, and I think this is a move in the right direction overall vasild: ACK 01e283068 sdaftuar: ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274. fanquake: ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 - I don't have as much experience with the networking code but these changes look fairly straight forward, the new code seems more robust/understandable and the additional documentation is great. I'm glad that a followup branch is already underway. There might be some more review comments here later today, so keep an eye on the discussion, however I'm going to merge this now. jb55: wow this code was messy before... ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 Tree-SHA512: 7bb644a6ed5849913d777ebc2ff89133ca0fbef680355a9a344e07496a979e6f9ff21a958e8eea93dcd7d5c343682b0c7174b1a3de380a4247eaae73da436e15
2020-08-07[net/refactor] Simplify multiple-connection checksAmiti Uttarwar
Extract logic that check multiple connection types into interface functions & structure as switch statements. This makes it very clear what touch points are for accessing `m_conn_type` & using the switch statements enables the compiler to warn if a new connection type is introduced but not handled for these cases.
2020-08-07[net/refactor] Rework ThreadOpenConnections logicAmiti Uttarwar
Make the connection counts explicit and extract into interface functions around m_conn_type. Using explicit counting and switch statements where possible should help prevent counting bugs in the future.
2020-08-07[net] Fix bug where AddrFetch connections would be counted as outbound full ↵Amiti Uttarwar
relay The desired logic is for us to only open feeler connections after we have hit the max count for outbound full relay connections. A short lived AddrFetch connection (previously called oneshot) could cause ThreadOpenConnections to miscount and mistakenly open a feeler instead of full relay.
2020-08-07[net/refactor] Clarify logic for selecting connections in ThreadOpenConnectionsAmiti Uttarwar
2020-08-07[net/refactor] Remove fInbound flag from CNodeAmiti Uttarwar
2020-08-07[net/refactor] Remove m_addr_fetch member var from CNodeAmiti Uttarwar
2020-08-07[net/refactor] Remove fFeeler flag from CNodeAmiti Uttarwar
2020-08-07[net/refactor] Remove m_manual_connection flag from CNodeAmiti Uttarwar
2020-08-07[net/refactor] Add connection type as a member var to CNodeAmiti Uttarwar
- Directly maintaining the connection type prevents having to deduce it from several flags.
2020-08-07[net/refactor] Add AddrFetch connections to ConnectionType enumAmiti Uttarwar
- AddrFetch connections are short lived connections used to getaddr from a peer - previously called "one shot" connections
2020-08-07[net/refactor] Extract m_addr_known logic from initializer listAmiti Uttarwar
2020-08-07[net/refactor] Add block relay only connections to ConnectionType enumAmiti Uttarwar
2020-08-07[net/refactor] Add feeler connections to ConnectionType enumAmiti Uttarwar
2020-08-07[net/refactor] Add manual connections to ConnectionType enumAmiti Uttarwar
2020-08-07[net/refactor] Introduce an enum to distinguish type of connectionAmiti Uttarwar
- extract inbound & outbound types
2020-08-07scripted-diff: Rename OneShot to AddrFetchAmiti Uttarwar
-BEGIN VERIFY SCRIPT- sed -i 's/a oneshot/an addrfetch/g' src/chainparams.cpp #comment sed -i 's/oneshot/addrfetch/g' src/net.cpp #comment sed -i 's/AddOneShot/AddAddrFetch/g' src/net.h src/net.cpp sed -i 's/cs_vOneShots/m_addr_fetches_mutex/g' src/net.h src/net.cpp sed -i 's/vOneShots/m_addr_fetches/g' src/net.h src/net.cpp sed -i 's/fOneShot/m_addr_fetch/g' src/net.h src/net.cpp src/net_processing.cpp sed -i 's/ProcessOneShot/ProcessAddrFetch/g' src/net.h src/net.cpp -END VERIFY SCRIPT-
2020-08-06refactor: Make HexStr take a spanWladimir J. van der Laan
Make HexStr take a span of bytes, instead of an awkward pair of templated iterators.
2020-08-03Merge #19326: Simplify hash.h interface using SpansWladimir J. van der Laan
77c507358bda9bd6c496f33e0f4418c0603bb08d Make Hash[160] consume range-like objects (Pieter Wuille) 02c4cc5c5ddf61f98ee366a4bea8abc26de492bd Make CHash256/CHash160 output to Span (Pieter Wuille) 0ef97b1b103231db54e04a64bbdb5dcc3f34f482 Make MurmurHash3 consume Spans (Pieter Wuille) e549bf8a9afae42fcda805e216a1cde62df195a6 Make CHash256 and CHash160 consume Spans (Pieter Wuille) 2a2182c387f607cd8284f33890bd285a81077b7f Make script/standard's BaseHash Span-convertible (Pieter Wuille) e63dcc3a6752e7d406e7a650c2d6c2e95cd39aab Add MakeUCharSpan, to help constructing Span<[const] unsigned char> (Pieter Wuille) 567825049fb0e47e698dcaad9caa65693a6b42d3 Make uint256 Span-convertible by adding ::data() (Pieter Wuille) 131a2f0337f5c396739a47b60bb856ed84ec8937 scripted-diff: rename base_blob::data to m_data (Pieter Wuille) Pull request description: This makes use of the implicit constructions and conversions to Span introduced in #18468 to simplify the hash.h interface: * All functions that take a pointer and a length are changed to take a Span instead. * The Hash() and Hash160() functions are changed to take in "range" objects instead of begin/end iterators. ACKs for top commit: laanwj: re-ACK 77c507358bda9bd6c496f33e0f4418c0603bb08d jonatack: Code review re-ACK 77c5073 per `git range-diff 14ceddd 49fc016 77c5073` Tree-SHA512: 9ec929891b1ddcf30eb14b946ee1bf142eca1442b9de0067ad6a3c181e0c7ea0c99c0e291e7f6e7a18bd7bdf78fe94ee3d5de66e167401674caf91e026269771
2020-07-30Make Hash[160] consume range-like objectsPieter Wuille
2020-07-30Make CHash256/CHash160 output to SpanPieter Wuille
2020-07-30Make CHash256 and CHash160 consume SpansPieter Wuille
2020-07-30Cache responses to addr requestsGleb Naumenko
Prevents a spy from scraping victim's AddrMan by reconnecting and re-requesting addrs.
2020-07-24Move filtering banned addrs inside GetAddresses()Gleb Naumenko
2020-07-23Merge #19473: net: Add -networkactive optionMarcoFalke
2aac093a3d60e446b85eebdf170ea6bed77bec92 test: Add test coverage for -networkactive option (Hennadii Stepanov) 3c58129b1293742a49aa196cb210ff345a7339e6 net: Log network activity status change unconditionally (Hennadii Stepanov) 62fe6aa87e4cdd8b06207abc1387c68d7bfc04c1 net: Add -networkactive option (Hennadii Stepanov) Pull request description: Some Bitcoin Core activity is completely local (offline), e.g., reindexing. The `setnetworkactive` RPC command is already present. This PR adds the corresponding command-line argument / config option, and allows to start the client with disabled p2p network by providing `-networkactive=0` or `-nonetworkactive`. This was done while reviewing #16981. ACKs for top commit: MarcoFalke: re-ACK 2aac093a3d60e446b85eebdf170ea6bed77bec92 🏠 LarryRuane: ACK 2aac093a3d60e446b85eebdf170ea6bed77bec92 Tree-SHA512: 446d791b46d7b556d7694df7b1f88cd4fbc09301fe4eaf036b45cb8166ed806156353cc03788a07b633d5887d5eee30a7c02a2d4307141c8ccc75e0a88145636
2020-07-22net: Log network activity status change unconditionallyHennadii Stepanov
2020-07-22net: Add -networkactive optionHennadii Stepanov
The `setnetworkactive' RPC command is already present. This new option allows to start the client with disabled p2p network activity for testing or reindexing.
2020-07-14[net/net processing] check banman pointer before dereferencingJohn Newbery
Although we currently don't do this, it should be possible to create a CConnman or PeerLogicValidation without a Banman instance. Therefore always check that banman exists before dereferencing the pointer. Also add comments to the m_banman members of CConnman and PeerLogicValidation to document that these may be nullptr.
2020-07-10Merge #14033: p2p: Drop CADDR_TIME_VERSION checks now that ↵MarcoFalke
MIN_PEER_PROTO_VERSION is greater 57b0c0a93a243769beb306c89560d1eda61f54bd Drop CADDR_TIME_VERSION checks now that MIN_PEER_PROTO_VERSION is greater (Ben Woosley) Pull request description: We do not connect to peers older than 31800 ACKs for top commit: sipa: Code reivew ACK 57b0c0a93a243769beb306c89560d1eda61f54bd jnewbery: Code review ACK 57b0c0a93a243769beb306c89560d1eda61f54bd vasild: ACK 57b0c0a9 Tree-SHA512: e1ca7c9203cbad83ab7c7a2312777ad07ed6a16119169b256648b8a8738c260a5168acdd4fb33f6e4b17f51ec7e033e110b76bde55b4e3b2d444dc02c01bc2b1
2020-07-10Merge #18638: net: Use mockable time for ping/pong, add testsMarcoFalke
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke) faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke) Pull request description: Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added. Mockable time is also type-safe, since it uses `std::chrono` ACKs for top commit: jonatack: Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid troygiorshev: ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
2020-07-09Merge #19191: net: Extract download permission from nobanMarcoFalke
fa0540cd46eaf44d9e1a9f91c3a937986826c4fa net: Extract download permission from noban (MarcoFalke) Pull request description: It should be possible to grant nodes in a local network (e.g. home, university, enterprise, ...) permission to download blocks even after the maxuploadtarget is hit. Currently this is only possible by setting the `noban` permission, which has some adverse effects, especially if the peers can't be fully trusted. Fix this by extracting a `download` permission from `noban`. ACKs for top commit: jonatack: ACK fa0540c Sjors: re-utACK fa0540cd46eaf44d9e1a9f91c3a937986826c4fa Tree-SHA512: 255566baa43ae925d93f5d0a3aa66b475a556d1590f662a88278a4872f16a1a05739a6119ae48a293011868042e05cb264cffe5822a50fb80db7333bf44376d9
2020-07-09Merge #19314: refactor: Use uint16_t instead of unsigned shortWladimir J. van der Laan
1cabbddbca615b26aa4510c75f459c28d6fe0afd refactor: Use uint16_t instead of unsigned short (Aaron Hook) Pull request description: I wanted to see if the `up for grabs` label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing. So I checked out the remote branch rebased it resolved three conflicts and continued the rebase. Hope everything is as expected (: ACKs for top commit: sipsorcery: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd. practicalswift: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd -- patch looks correct :) laanwj: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd hebasto: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6
2020-07-09net: Extract download permission from nobanMarcoFalke
2020-07-08Merge #19347: [net] Make cs_inventory nonrecursiveMarcoFalke
e8a2822119233ade0de84f791a9e92918a3d6896 [net] Don't try to take cs_inventory before deleting CNode (John Newbery) 3556227ddd3365cfac43b307204d73058b2943f0 [net] Make cs_inventory a non-recursive mutex (John Newbery) 344e831de54f7b864f03a90f6cb19692eafcd463 [net processing] Remove PushBlockInventory and PushBlockHash (John Newbery) Pull request description: - Remove PushBlockInventory() and PushBlockHash(). These are one-line functions that can easy be inlined into the calling code. Doing so also allows us to eliminate the one place that cs_inventory is recursively locked. - Make cs_inventory a nonrecursive mutex - Remove a redundant TRY_LOCK of cs_inventory when deleting CNode. ACKs for top commit: sipa: utACK e8a2822119233ade0de84f791a9e92918a3d6896 MarcoFalke: ACK e8a2822119233ade0de84f791a9e92918a3d6896 🍬 hebasto: re-ACK e8a2822119233ade0de84f791a9e92918a3d6896 Tree-SHA512: dbc721d102cdef7b5827a8f2549daf8b54f543050266999a7ea56c9f36618565b71e31ce0beb1209ba2db43d15388be173355a03fb6db8ad24e2475b145050bd
2020-07-03Clean up separated ban/discourage interfacePieter Wuille
2020-07-03Replace automatic bans with discouragement filterPieter Wuille
This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to better reflect reality; they're not banned, just discouraged. Contains release notes and several interface improvements by John Newbery.
2020-06-27scripted-diff: Move ui_interface to the node libMarcoFalke
-BEGIN VERIFY SCRIPT- # Move files git mv src/ui_interface.h src/node/ui_interface.h git mv src/ui_interface.cpp src/node/ui_interface.cpp sed -i -e 's/BITCOIN_UI_INTERFACE_H/BITCOIN_NODE_UI_INTERFACE_H/g' src/node/ui_interface.h # Adjust includes and makefile sed -i -e 's|ui_interface|node/ui_interface|g' $(git grep -l ui_interface) # Sort includes git diff -U0 | clang-format-diff -p1 -i -v -END VERIFY SCRIPT-
2020-06-23[net] Don't try to take cs_inventory before deleting CNodeJohn Newbery
The TRY_LOCK(cs_inventory) in DisconnectNodes() is taken after the CNode object has been removed from vNodes and when the CNode's nRefCount is zero. The only other places that cs_inventory can be taken are: - In ProcessMessages() or SendMessages(), when the CNode's nRefCount must be >0 (see ThreadMessageHandler(), where the refcount is incremented before calling ProcessMessages() and SendMessages()). - In a ForEachNode() lambda in PeerLogicValidation::UpdatedBlockTip(). ForEachNode() locks cs_vNodes and calls the function on the CNode objects in vNodes. Therefore, cs_inventory is never locked by another thread when the TRY_LOCK(cs_inventory) is reached in DisconnectNodes(). Since the only purpose of this TRY_LOCK is to ensure that the lock is not taken by another thread, this always succeeds. Remove the check.