aboutsummaryrefslogtreecommitdiff
path: root/src/net.cpp
AgeCommit message (Collapse)Author
2021-05-19Merge bitcoin/bitcoin#21506: p2p, refactor: make NetPermissionFlags an enum ↵W. J. van der Laan
class 7075f604e8d0b21b2255fa57e20cd365dc10a288 scripted-diff: update noban documentation in net_processing.cpp (Jon Atack) a95540cf435029f06e56749802d71315ca76b0dd scripted-diff: rename NetPermissionFlags enumerators (Jon Atack) 810d0929c1626bba141af3f779a3c9cd6ece7e75 p2p, refactor: make NetPermissionFlags a uint32 enum class (Jon Atack) 7b55a9449778c5ac89799ce4c607c8c8d797ddfb p2p: NetPermissions::HasFlag() pass flags param by value (Jon Atack) 91f6e6e6d1720e1154ad3f70a5098e9028efa84a scripted-diff: add NetPermissionFlags scopes where not already present (Jon Atack) Pull request description: While reviewing #20196, I noticed the `NetPermissionFlags` enums are frequently called as if they were scoped, yet are still global. This patch upgrades `NetPermissionFlags` to a scoped class enum and updates the enumerator naming, similarly to #19771. See https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum-enumerations for more info. This change would eliminate the class of bugs like https://github.com/bitcoin/bitcoin/pull/20196#discussion_r610770148 and #21644, as only defined operations on the flags would compile. ACKs for top commit: laanwj: Code review ACK 7075f604e8d0b21b2255fa57e20cd365dc10a288 vasild: ACK 7075f604e8d0b21b2255fa57e20cd365dc10a288 Tree-SHA512: 7fcea66ee499f059efc78c934b5f729b3c8573fe304dee2c27c837c2f662b89324790568246d75b2a574cf9f059b42d3551d928996862f4358055eb43521e6f4
2021-05-13Merge bitcoin/bitcoin#21914: net: use stronger AddLocal() for our I2P addressW. J. van der Laan
105941b726c078642e785ecb7b6834ba814381b0 net: use stronger AddLocal() for our I2P address (Vasil Dimov) Pull request description: There are two issues: ### 1. Our I2P address not added to local addresses. * `externalip=` is used with an IPv4 address (this sets automatically `discover=0`) * No `discover=1` is used * `i2psam=` is used * No `externalip=` is used for our I2P address * `listenonion=1 torcontrol=` are used In this case `AddLocal(LOCAL_MANUAL)` [is used](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/torcontrol.cpp#L354) for our `.onion` address and `AddLocal(LOCAL_BIND)` [for our](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L2247) `.b32.i2p` address, the latter being [ignored](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L232-L233) due to `discover=0`. ### 2. Our I2P address removed from local addresses even if specified with `externalip=` on I2P proxy restart. * `externalip=` is used with our I2P address (this sets automatically `discover=0`) * No `discover=1` is used * `i2psam=` is used In this case, initially `externalip=` causes our I2P address to be [added](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/init.cpp#L1266) with `AddLocal(LOCAL_MANUAL)` which overrides `discover=0` and works as expected. However, if later the I2P proxy is shut down [we do](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L2234) `RemoveLocal()` in order to stop advertising our I2P address (since we have lost I2P connectivity). When the I2P proxy is started and we reconnect to it, restoring the I2P connectivity, [we do](https://github.com/bitcoin/bitcoin/blob/94f83534e4b771944af7d9ed0f40746f392eb75e/src/net.cpp#L2247) `AddLocal(LOCAL_BIND)` which does nothing due to `discover=0`. To resolve those two issues, use `AddLocal(LOCAL_MANUAL)` for I2P which is also what we do with Tor. ACKs for top commit: laanwj: Code review ACK 105941b726c078642e785ecb7b6834ba814381b0 Tree-SHA512: 0c9daf6116b8d9c34ad7e6e9bbff6e8106e94e4394a815d7ae19287aea22a8c7c4e093c8dd8c58a4a1b1412b2575a9b42b8a93672c8d17f11c24508c534506c7
2021-05-12scripted-diff: rename NetPermissionFlags enumeratorsJon Atack
- drop redundant PF_ permission flags prefixes - drop ALL_CAPS naming per https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps - rename IsImplicit to Implicit -BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s 'PF_NONE' 'None' s 'PF_BLOOMFILTER' 'BloomFilter' s 'PF_RELAY' 'Relay' s 'PF_FORCERELAY' 'ForceRelay' s 'PF_DOWNLOAD' 'Download' s 'PF_NOBAN' 'NoBan' s 'PF_MEMPOOL' 'Mempool' s 'PF_ADDR' 'Addr' s 'PF_ISIMPLICIT' 'Implicit' s 'PF_ALL' 'All' -END VERIFY SCRIPT-
2021-05-12scripted-diff: add NetPermissionFlags scopes where not already presentJon Atack
-BEGIN VERIFY SCRIPT- s() { git grep -l "$1" -- 'src' ':!src/net_permissions.h' | xargs sed -i -E "s/([^:])$1/\1NetPermissionFlags::$1/"; } s 'PF_NONE' s 'PF_BLOOMFILTER' s 'PF_RELAY' s 'PF_FORCERELAY' s 'PF_DOWNLOAD' s 'PF_NOBAN' s 'PF_MEMPOOL' s 'PF_ADDR' s 'PF_ISIMPLICIT' s 'PF_ALL' -END VERIFY SCRIPT- Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-05-12Merge bitcoin/bitcoin#19064: refactor: Cleanup thread ctor callsMarcoFalke
792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov) a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov) 30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov) Pull request description: This PR does not change behavior. Its goal is to improve readability and maintainability of the code. ACKs for top commit: jnewbery: utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e jonatack: tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e MarcoFalke: cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
2021-05-11Merge bitcoin/bitcoin#21644: p2p, bugfix: use NetPermissions::HasFlag() in ↵W. J. van der Laan
CConnman::Bind() 36fb036d25e2a3016b36873456e5a9e6251ffef8 p2p: allow NetPermissions::ClearFlag() only with PF_ISIMPLICIT (Jon Atack) 4e0d5788ba5771c81bc0ff2e6523cf9accddae46 test: add net permissions noban/download unit test coverage (Jon Atack) dde69f20a01acca64ac21cb13993c6e4f8709f23 p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind() (Jon Atack) Pull request description: This is a bugfix follow-up to #16248 and #19191 that was noticed in #21506. Both v0.21 and master are affected. Since #19191, noban is a multi-flag that implies download, so the conditional in `CConnman::Bind()` using a bitwise AND on noban will return the same result for both the noban status and the download status. This means that download peers are incorrectly not being added to local addresses because they are mistakenly seen as noban peers. The second commit adds unit test coverage to illustrate and test the noban/download relationship and the `NetPermissions` operations involving them. The final commit adds documentation and disallows calling `NetPermissions::ClearFlag()` with any second param other than `NetPermissionFlags` "implicit" -- per current usage in the codebase -- because `ClearFlag()` should not be called with any second param that is a subflag of a multiflag, e.g. "relay" or "download," as that would leave the result in an invalid state corresponding to none of the existing NetPermissionFlags. Thanks to Vasil Dimov for noticing this. ACKs for top commit: theStack: re-ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 ☕ vasild: ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 hebasto: ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8, I have reviewed the code and it looks OK, I agree it can be merged. kallewoof: Code review ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 Tree-SHA512: 5fbc7ddbf31d06b35bf238f4d77ef311e6b6ef2e1bb9893f32f889c1a0f65774a3710dcb21d94317fe6166df9334a9f2d42630809e7fe8cbd797dd6f6fc49491
2021-05-11net: use stronger AddLocal() for our I2P addressVasil Dimov
There are two issues: 1. Our I2P address not added to local addresses. * `externalip=` is used with an IPv4 address (this sets automatically `discover=0`) * No `discover=1` is used * `i2psam=` is used * No `externalip=` is used for our I2P address * `listenonion=1 torcontrol=` are used In this case `AddLocal(LOCAL_MANUAL)` is used for our `.onion` address and `AddLocal(LOCAL_BIND)` for our `.b32.i2p` address, the latter being ignored due to `discover=0`. 2. Our I2P address removed from local addresses even if specified with `externalip=` on I2P proxy restart. * `externalip=` is used with our I2P address (this sets automatically `discover=0`) * No `discover=1` is used * `i2psam=` is used In this case, initially `externalip=` causes our I2P address to be added with `AddLocal(LOCAL_MANUAL)` which overrides `discover=0` and works as expected. However, if later the I2P proxy is shut down we do `RemoveLocal()` in order to stop advertising our I2P address (since we have lost I2P connectivity). When the I2P proxy is started and we reconnect to it, restoring the I2P connectivity, we do `AddLocal(LOCAL_BIND)` which does nothing due to `discover=0`. To resolve those two issues, use `AddLocal(LOCAL_MANUAL)` for I2P which is also what we do with Tor.
2021-05-10Merge bitcoin/bitcoin#21836: scripted-diff: Replace three dots with ellipsis ↵W. J. van der Laan
in the UI strings d66f283ac07edce432b964f7f814631f5a5bc33b scripted-diff: Replace three dots with ellipsis in the UI strings (Hennadii Stepanov) Pull request description: This PR is split from #21463. The change was suggested on [Transifex.com](https://www.transifex.com/bitcoin/bitcoin/), and it does not touch `LogPrint` and `LogPrintf` calls. The only comment on #21463 [was](https://github.com/bitcoin/bitcoin/pull/21463/commits/9030e4b5a6de54e041c59e98d91adecbebf3611a#r597220100): > Mind that these messages also end up in the log. In principle the log is already UTF-8 (as are all strings and text in bitcoind). But, just noting, that it might make browsing the log a less pleasant experience on systems with misconfigured locale like some BSDs by default. ACKs for top commit: laanwj: ACK d66f283ac07edce432b964f7f814631f5a5bc33b Tree-SHA512: 5ab1cb3160f3f996f1ad7d7486662da3eb7f06a857f4a1874963ce10caed5b86b0ad6151b1b9ebeb2b8aa5f0c85efad3b768ea9cafe5db86f78f88912b756d1e
2021-05-07net: Clarify message header validation errorsW. J. van der Laan
Make the errors less shouty and more descriptive.
2021-05-06net: Sanitize message type for loggingW. J. van der Laan
- Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. - For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes should be enough. - Update `p2p_invalid_messages.py` test to check this. Issue reported by gmaxwell.
2021-05-03Merge bitcoin/bitcoin#21750: net: remove unnecessary check of CNode::cs_vSendMarcoFalke
9096b13a4764873511b65f32a005ce4738b0d81c net: remove unnecessary check of CNode::cs_vSend (Vasil Dimov) Pull request description: It is not possible to have a node in `CConnman::vNodesDisconnected` and its reference count to be incremented - all `CNode::AddRef()` are done either before the node is added to `CConnman::vNodes` or while holding `CConnman::cs_vNodes` and the object being in `CConnman::vNodes`. So, the object being in `CConnman::vNodesDisconnected` and its reference count being zero means that it is not and will not start to be used by other threads. So, the lock of `CNode::cs_vSend` in `CConnman::DisconnectNodes()` will always succeed and is not necessary. Indeed all locks of `CNode::cs_vSend` are done either when the reference count is >0 or under the protection of `CConnman::cs_vNodes` and the node being in `CConnman::vNodes`. ACKs for top commit: MarcoFalke: review ACK 9096b13a4764873511b65f32a005ce4738b0d81c 🏧 jnewbery: utACK 9096b13a4764873511b65f32a005ce4738b0d81c Tree-SHA512: 910899cdcdc8934642eb0c40fcece8c3b01b7e20a0b023966b9d6972db6a885cb3a9a04e9562bae14d5833967e45e2ecb3687b94d495060c3da4b1f2afb0ac8f
2021-05-02scripted-diff: Replace three dots with ellipsis in the UI stringsHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i -E -e 's/\.\.\."\)(\.|,|\)| )/…"\)\1/' -- $(git ls-files -- 'src' ':(exclude)src/qt/bitcoinstrings.cpp') sed -i -e 's/\.\.\.\\"/…\\"/' src/qt/sendcoinsdialog.cpp sed -i -e 's|\.\.\.</string>|…</string>|' src/qt/forms/*.ui sed -i -e 's|\.\.\.)</string>|…)</string>|' src/qt/forms/sendcoinsdialog.ui -END VERIFY SCRIPT-
2021-04-29refactor: Replace std::bind with lambdasHennadii Stepanov
Lambdas are shorter and more readable. Changes are limited to std::thread ctor calls only.
2021-04-25refactor: Make TraceThread a non-template free functionHennadii Stepanov
Also it is moved into its own module.
2021-04-25Merge bitcoin/bitcoin#21563: net: Restrict period when cs_vNodes mutex is lockedMarcoFalke
8c8237a4a10feb2ac9ce46f67b5d14bf879b670f net, refactor: Fix style in CConnman::StopNodes (Hennadii Stepanov) 229ac1892d807a1eea5a7c24ae0fe27dc913b1bd net: Combine two loops into one, and update comments (Hennadii Stepanov) a3d090d1103cd6c25daf07afdf4e65febca6d3f7 net: Restrict period when cs_vNodes mutex is locked (Hennadii Stepanov) Pull request description: This PR restricts the period when the `cs_vNodes` mutex is locked, prevents the only case when `cs_vNodes` could be locked before the `::cs_main`. This change makes the explicit locking of recursive mutexes in the explicit order redundant. ACKs for top commit: jnewbery: utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f vasild: ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f ajtowns: utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f - logic seems sound MarcoFalke: review ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f 👢 Tree-SHA512: a8277924339622b188b12d260a100adf5d82781634cf974320cf6007341f946a7ff40351137c2f5369aed0d318f38aac2d32965c9b619432440d722a4e78bb73
2021-04-22net, refactor: Fix style in CConnman::StopNodesHennadii Stepanov
2021-04-22net: Combine two loops into one, and update commentsHennadii Stepanov
2021-04-22net: Restrict period when cs_vNodes mutex is lockedHennadii Stepanov
2021-04-22net: remove unnecessary check of CNode::cs_vSendVasil Dimov
It is not possible to have a node in `CConnman::vNodesDisconnected` and its reference count to be incremented - all `CNode::AddRef()` are done either before the node is added to `CConnman::vNodes` or while holding `CConnman::cs_vNodes` and the object being in `CConnman::vNodes`. So, the object being in `CConnman::vNodesDisconnected` and its reference count being zero means that it is not and will not start to be used by other threads. So, the lock of `CNode::cs_vSend` in `CConnman::DisconnectNodes()` will always succeed and is not necessary. Indeed all locks of `CNode::cs_vSend` are done either when the reference count is >0 or under the protection of `CConnman::cs_vNodes` and the node being in `CConnman::vNodes`.
2021-04-17refactor: Mark member functions constMarcoFalke
2021-04-14p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind()Jon Atack
PF_NOBAN is a multi-flag that includes PF_DOWNLOAD, so the conditional in CConnman::Bind() using a bitwise AND will return the same result for both the "noban" status and the "download" status. Example: `PF_DOWNLOAD` is `0b1000000` `PF_NOBAN` is `0b1010000` This makes a check like `flags & PF_NOBAN` return `true` even if `flags` is equal to `PF_DOWNLOAD`. If `-whitebind=download@1.1.1.1:8765` is specified, then `1.1.1.1:8765` should be added to the list of local addresses. We only want to avoid adding to local addresses (that are advertised) a whitebind that has a `noban@` flag. As a result of a mis-check in `CConnman::Bind()` we would not have added `1.1.1.1:8765` to the local addresses in the example above. Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2021-04-06Merge #21560: net: Add Tor v3 hardcoded seedsW. J. van der Laan
b2ee8b207de78f03356905bd60b7b00b6f49c252 net: Deserialize hardcoded seeds from BIP155 blob (W. J. van der Laan) 9b29d5df7fc555eaea42029f334f2995c6ccde3d contrib: Add explicit port numbers for testnet seeds (W. J. van der Laan) 2a257de113fd31539b68c28c47ef94f257b6e427 contrib: Add a few TorV3 seed nodes (W. J. van der Laan) 06030f7a42dea33c5120504dcd99d1714883f271 contrib: generate-seeds.py generates output in BIP155 format (W. J. van der Laan) Pull request description: Closes #20239 and mitigates my node's problem in #21351. - Add a few hardcoded seeds for TorV3 - As the [bitcoin-seeder](https://github.com/sipa/bitcoin-seeder) doesn't collect TorV3 addresses yet, I have extracted these from my own node using [a script](https://gist.github.com/laanwj/b3d7b01ef61ce07c2eff0a72a6b90183) and added them manually. This is intended to be a temporary stop gap until 22.0's seeds update. - Change hardcoded seeds to variable length BIP155 binary format. - It is stored as a single serialized blob in a byte array, instead of pseudo-IPv6 address slots. This is more flexible and, assuming most of the list is IPv4, more compact. - Only the (networkID, addr, port) subset (CService). Services and time are construed on the fly as before. - Change input format for `nodes_*.txt`. - Drop legacy `0xAABBCCDD` format for IPv4. It is never generated by `makeseeds.py`. - Stop interpreting lack of port as default port, interpret it as 'no port', to accomodate I2P and other port-less protocols (not handled in this PR). An explicit port is always generated by `makeseeds.py` so in practice this makes no difference right now. A follow-up to this PR could do the same for I2P. ACKs for top commit: jonatack: ACK b2ee8b207de78f03356905bd60b7b00b6f49c252 Tree-SHA512: 11a6b54f9fb0192560f2bd7b218f798f86c1abe01d1bf37f734cb88b91848124beb2de801ca4e6f856e9946aea5dc3ee16b0dbb9863799e42eec1b239d40d59d
2021-04-05net: Deserialize hardcoded seeds from BIP155 blobW. J. van der Laan
Switch from IPv6 slot-based format to more compact and flexible BIP155 format.
2021-04-01[net] Changes to RunInactivityChecksJohn Newbery
- rename to ShouldRunInactivityChecks (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r576394790) - take optional time now (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r575895661) - call from within InactivityChecks (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r575894665) - update comment (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r575894343) - change ordering of inequality (https://github.com/bitcoin/bitcoin/pull/20721#discussion_r574925129)
2021-03-31Merge #21486: build: link against -lsocket if required for *ifaddrsfanquake
4783115fd4cccb46a7f8c592b34fa7c094c29410 net: add ifaddrs.h include (fanquake) 879215e665a9f348c8d3fa92701c34065bc86a69 build: check if -lsocket is required with *ifaddrs (fanquake) 87deac66aa747481e6f34fc80599e1e490de3ea0 rand: only try and use freeifaddrs if available (fanquake) Pull request description: Fixes #21485 by linking against `-lsocket` when it's required for using `*ifaddrs` functions. ACKs for top commit: laanwj: Code review ACK 4783115fd4cccb46a7f8c592b34fa7c094c29410 hebasto: ACK 4783115fd4cccb46a7f8c592b34fa7c094c29410, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 4542e036e9b029de970eff8a9230fe45d9204bb22313d075f474295d49bdaf1f1cbb36c0c6e2fa8dbbcdba518d8d3a68a6116ce304b82414315f333baf9af0e4
2021-03-30Merge #21387: p2p: Refactor sock to add I2P fuzz and unit testsWladimir J. van der Laan
40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 test: add I2P test for a runaway SAM proxy (Vasil Dimov) 2d8ac779708322e1235e823edfc9c8f6e2dd65e4 fuzz: add tests for the I2P Session public interface (Vasil Dimov) 9947e44de0cbd79e99d883443a9ac441d8c69713 i2p: use pointers to Sock to accommodate mocking (Vasil Dimov) 82d360b5a88d9057b6c09b61cd69e426c7a2412d net: change ConnectSocketDirectly() to take a Sock argument (Vasil Dimov) b5861100f85fef77b00f55dcdf01ffb4a2a112d8 net: add connect() and getsockopt() wrappers to Sock (Vasil Dimov) 5a887d49b2b39e59d7cce8e9d5b89c21ad694f8b fuzz: avoid FuzzedSock::Recv() repeated errors with EAGAIN (Vasil Dimov) 3088f83d016e7ebb6e6aa559e6326fa0ef0d6282 fuzz: extend FuzzedSock::Recv() to support MSG_PEEK (Vasil Dimov) 9b05c49ade729311a0f4388a109530ff8d0ed1f9 fuzz: implement unimplemented FuzzedSock methods (Vasil Dimov) Pull request description: Change the networking code and the I2P code to be fully mockable and use `FuzzedSocket` to fuzz the I2P methods `Listen()`, `Accept()` and `Connect()`. Add a mocked `Sock` implementation that returns a predefined data on reads and use it for a regression unit test for the bug fixed in https://github.com/bitcoin/bitcoin/pull/21407. ACKs for top commit: practicalswift: Tested ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 MarcoFalke: Concept ACK 40316a37cb jonatack: re-ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 reviewed `git range-diff 01bb3afb 23c861d 40316a3` and the new unit test commit, debug built, ran unit tests, ran bitcoind with an I2P service and network operation with seven I2P peers (2 in, 5 out) is looking nominal laanwj: Code review ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 Tree-SHA512: 7fc4f129849e16e0c7e16662d9f4d35dfcc369bb31450ee369a2b97bdca95285533bee7787983e881e5a3d248f912afb42b4a2299d5860ace7129b0b19623cc8
2021-03-30Merge #20197: p2p: protect onions in AttemptToEvictConnection(), add ↵Wladimir J. van der Laan
eviction protection test coverage 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Add unit test coverage for our onion peer eviction protection (Jon Atack) caa21f586f951d626a67f391050c3644f1057f57 Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() (Jon Atack) 8f1a53eb027727a4c0eaac6d82f0a8279549f638 Use EraseLastKElements() throughout SelectNodeToEvict() (Jon Atack) 8b1e156143740a5548dc7b601d40fb141e6aae1c Add m_inbound_onion to AttemptToEvictConnection() (Jon Atack) 72e30e8e03f880eba4bd1c3fc18b5558d8cef680 Add unit tests for ProtectEvictionCandidatesByRatio() (Jon Atack) ca63b53ecdf377ce777fd959d400748912266748 Use std::unordered_set instead of std::vector in IsEvicted() (Jon Atack) 41f84d5eccd4c2620bf6fee616f2f8f717dbd6f6 Move peer eviction tests to a separate test file (Jon Atack) f126cbd6de6e1a8fee0e900ecfbc14a88e362541 Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvict (Jon Atack) Pull request description: Now that #19991 and #20210 have been merged, we can determine inbound onion peers using `CNode::m_inbound_onion` and add it to the localhost peers protection in `AttemptToEvictConnection`, which was added in #19670 to address issue #19500. Update 28 February 2021: I've updated this to follow gmaxwell's suggestion in https://github.com/bitcoin/bitcoin/pull/20197#issuecomment-713865992. This branch now protects up to 1/4 onion peers (connected via our tor control service), if any, sorted by longest uptime. If any (or all) onion slots remain after that operation, they are then allocated to protect localhost peers, or a minimum of 2 localhost peers in the case that no onion slots remain and 2 or more onion peers were protected, sorted as before by longest uptime. This patch also adds test coverage for the longest uptime, localhost, and onion peer eviction protection logic to build on the welcome initial unit testing of #20477. Suggest reviewing the commits that move code with `colorMoved = dimmed-zebra` and `colorMovedWs = allow-indentation-change`. Closes #11537. ACKs for top commit: laanwj: Code review ACK 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea vasild: ACK 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Tree-SHA512: 2f5a63f942acaae7882920fc61f0185dcd51da85e5b736df9d1fc72343726dd17da740e02f30fa5dc5eb3b2d8345707aed96031bec143d48a2497a610aa19abd
2021-03-29net: add ifaddrs.h includefanquake
2021-03-22[net] remove fUpdateConnectionTime from FinalizeNodeJohn Newbery
PeerManager can just call directly into CAddrMan::Connected() now.
2021-03-20[net] remove CConnman::AddNewAddressesJohn Newbery
It just forwards calls to CAddrMan::Add.
2021-03-20[net] remove CConnman::MarkAddressGoodJohn Newbery
It just forwards calls to CAddrMan::Good.
2021-03-20[net] remove CConnman::SetServicesJohn Newbery
It just forwards calls to CAddrMan::SetServices.
2021-03-20[net] Construct addrman outside connmanJohn Newbery
node.context owns the CAddrMan. CConnman holds a reference to the CAddrMan.
2021-03-19Merge #21328: net, refactor: pass uint16 CService::port as uint16MarcoFalke
52dd40a9febec1f4e70d777821b6764830bdec61 test: add missing netaddress include headers (Jon Atack) 6f09c0f6b57ac01a473c587a3e51e9d477866bb0 util: add missing braces and apply clang format to SplitHostPort() (Jon Atack) 2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack) 6423c8175fad3163c10ffdb49e0df48e4e4931f1 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack) Pull request description: As noticed during review today in https://github.com/bitcoin/bitcoin/pull/20685#discussion_r584873708 of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch. ACKs for top commit: practicalswift: cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61: patch looks correct MarcoFalke: cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61 vasild: ACK 52dd40a9febec1f4e70d777821b6764830bdec61 Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432
2021-03-19Protect onion+localhost peers in ProtectEvictionCandidatesByRatio()Jon Atack
Now that we have a reliable way to detect inbound onion peers, this commit updates our existing eviction protection of 1/4 localhost peers to instead protect up to 1/4 onion peers (connected via our tor control service), sorted by longest uptime. Any remaining slots of the 1/4 are then allocated to protect localhost peers, or 2 localhost peers if no slots remain and 2 or more onion peers are protected, sorted by longest uptime. The goal is to avoid penalizing onion peers, due to their higher min ping times relative to IPv4 and IPv6 peers, and improve our diversity of peer connections. Thank you to Gregory Maxwell, Suhas Daftuar, Vasil Dimov and Pieter Wuille for valuable review feedback that shaped the direction.
2021-03-19Use EraseLastKElements() throughout SelectNodeToEvict()Jon Atack
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2021-03-19Add m_inbound_onion to AttemptToEvictConnection()Jon Atack
and an `m_is_onion` struct member to NodeEvictionCandidate and tests. We'll use these in the peer eviction logic to protect inbound onion peers in addition to the existing protection of localhost peers.
2021-03-19Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvictJon Atack
to allow deterministic unit testing of the ratio-based peer eviction protection logic, which protects peers having longer connection times and those connected via higher-latency networks. Add documentation.
2021-03-17refactor: post Optional<> removal cleanupsfanquake
2021-03-16p2p, refactor: pass and use uint16_t CService::port as uint16_tJon Atack
2021-03-16i2p: use pointers to Sock to accommodate mockingVasil Dimov
Change the types of `i2p::Connection::sock` and `i2p::sam::Session::m_control_sock` from `Sock` to `std::unique_ptr<Sock>`. Using pointers would allow us to sneak `FuzzedSock` instead of `Sock` and have the methods of the former called. After this change a test only needs to replace `CreateSock()` with a function that returns `FuzzedSock`.
2021-03-16net: change ConnectSocketDirectly() to take a Sock argumentVasil Dimov
Change `ConnectSocketDirectly()` to take a `Sock` argument instead of a bare `SOCKET`. With this, use the `Sock`'s (possibly mocked) methods `Connect()`, `Wait()` and `GetSockOpt()` instead of calling the OS functions directly.
2021-03-15scripted-diff: remove Optional & nulloptfanquake
-BEGIN VERIFY SCRIPT- git rm src/optional.h sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src) sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src) sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src) sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src) sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src) sed -i -e '/optional.h \\/d' src/Makefile.am sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src) -END VERIFY SCRIPT-
2021-03-11scripted-diff: remove MakeUnique<T>()fanquake
-BEGIN VERIFY SCRIPT- git rm src/util/memory.h sed -i -e 's/MakeUnique/std::make_unique/g' $(git grep -l MakeUnique src) sed -i -e '/#include <util\/memory.h>/d' $(git grep -l '#include <util/memory.h>' src) sed -i -e '/util\/memory.h \\/d' src/Makefile.am -END VERIFY SCRIPT-
2021-03-04net: Replace enum CConnMan::NumConnections with enum class ConnectionDirectionLuke Dashjr
2021-03-03Make all Poisson delays use std::chrono typesPieter Wuille
2021-03-03Change all ping times to std::chrono typesPieter Wuille
2021-03-01net: accept incoming I2P connections from CConnmanVasil Dimov
2021-03-01net: make outgoing I2P connections from CConnmanVasil Dimov
2021-03-01init: introduce I2P connectivity optionsVasil Dimov
Introduce two new options to reach the I2P network: * `-i2psam=<ip:port>` point to the I2P SAM proxy. If this is set then the I2P network is considered reachable and we can make outgoing connections to I2P peers via that proxy. We listen for and accept incoming connections from I2P peers if the below is set in addition to `-i2psam=<ip:port>` * `-i2pacceptincoming` if this is set together with `-i2psam=<ip:port>` then we accept incoming I2P connections via the I2P SAM proxy.
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-03 05:45:25 +0000