aboutsummaryrefslogtreecommitdiff
path: root/src/net_processing.cpp
AgeCommit message (Collapse)Author
2021-11-30Merge bitcoin/bitcoin#21327: net_processing: ignore transactions while in IBDW. J. van der Laan
6aed8b7e9b206e728367fee9edfeb85b536bba69 [test] tx processing before and after ibd (glozow) b9e105b6643bada4c80f8d521394be4d55e8e4fc [net_processing] ignore all transactions during ibd (glozow) Pull request description: This is basically a mini, IBD-only version of #21224 Incoming transactions aren't really relevant until we're caught up. That's why we send a giant feefilter and don't send tx getdatas, but we also shouldn't process them if peers send them anyway. Simply ignore them. ACKs for top commit: jnewbery: reACK 6aed8b7e9b laanwj: Code review ACK 6aed8b7e9b206e728367fee9edfeb85b536bba69 Tree-SHA512: 8e1616bf355f9d0b180bdbc5461f24c757dc5d7bc7bf651470f3b0bffcca5d5e68287106255b5cede2d96b42bce448a0f8c0649de35a530c5e079f7c89c70a35
2021-11-17doc: Fix incorrect C++ named argsMarcoFalke
2021-11-03[validation] Always call mempool.check() after processing a new transactionJohn Newbery
CTxMemPool::check() will carry out internal consistency checks 1/n times, where n is set by the `-checkmempool` configuration option. By default, mempool consistency checks are disabled entirely on mainnet. Therefore, this change has no effect on mainnet nodes running with default configuration. It simply removes the responsibility to trigger mempool consistency checks from net_processing.
2021-11-03[refactor] Don't call AcceptToMemoryPool() from outside validation.cppJohn Newbery
2021-11-03[validation] Add CChainState::ProcessTransaction()John Newbery
This just calls through to AcceptToMemoryPool() internally, and is currently unused. Also add a new transaction validation failure reason TX_NO_MEMPOOL to indicate that there is no mempool.
2021-10-28[net_processing] ignore all transactions during ibdglozow
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-10-25Merge bitcoin/bitcoin#23157: txmempool -/-> validation 1/2: improve ↵MarcoFalke
performance of check() and remove dependency on validation 082c5bf099c64e3d27abe9b68a71ce500b693e7e [refactor] pass coinsview and height to check() (glozow) ed6115f1eae0eb4669601106a9aaff078a2f3a74 [mempool] simplify some check() logic (glozow) 9e8d7ad5d9cc4b013826daead9cee09aad539401 [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow) 09d18916afb0ecae90700d4befd9d5dc52767970 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow) e8639ec26aaf4de3fae280963434bf1cf2017b6f [mempool] remove now-unnecessary code (glozow) 54c6f3c1da01090aee9691a2c2bee0984a054ce8 [mempool] speed up check() by using coins cache and iterating in topo order (glozow) 30e240f65e69c6dffcd033afc63895345bd51f53 [bench] Benchmark CTxMemPool::check() (glozow) cb1407196fba648aa75504e3ab3d46aa0181563a [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow) Pull request description: Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`. This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children. ACKs for top commit: jnewbery: reACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e GeneFerneau: tACK [082c5bf](https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e) rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e theStack: Code-review ACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
2021-10-22Merge bitcoin/bitcoin#23336: refactor: Make GenTxid boolean constructor privateMarcoFalke
fa4ec1c0bdaef9f082a6661d7faf16149774e145 Make GenTxid boolean constructor private (MarcoFalke) faeb9a575367119dbff60c35fa2c13547718e179 remove unused CTxMemPool::info(const uint256& txid) (MarcoFalke) Pull request description: This boolean argument is either verbose (when used with a named arg) or unintuitive and dangerous (when used as a plain bool). Fix that by making the constructor private. ACKs for top commit: laanwj: Code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145 jnewbery: Code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145 glozow: code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145 Tree-SHA512: bf08ee09168885cfda71e5a01ec412b93964662a90dd9d91e75f7fdf2eaff7c21a95204d0e90b00438bfeab564d0aea66bdb9c0394ee7a05743e65a817159446
2021-10-22Merge bitcoin/bitcoin#23042: net: Avoid logging AlreadyHaveTx when ↵fanquake
disconnecting misbehaving peer fa2662c293ec0aaa93092b59b6632f74729c4283 net: Avoid logging AlreadyHaveTx when disconnecting misbehaving peer (MarcoFalke) Pull request description: There is no need to log `AlreadyHaveTx` for an inv when a peer is marked for disconnection due to sending that inv. In fact, I find it confusing that a `block-relay-only` connection calls `AlreadyHaveTx` at all. Also there is no need to call `AddKnownTx` when the peer is marked for disconnection. ACKs for top commit: naumenkogs: ACK fa2662c293ec0aaa93092b59b6632f74729c4283 jnewbery: Code review ACK fa2662c293ec0aaa93092b59b6632f74729c4283 dunxen: Concept and code review ACK fa2662c293ec0aaa93092b59b6632f74729c4283 Tree-SHA512: 9996b807a824021f992b5281d82ff0cbbe6a442c2fedf7dfd6adda64ccc5e0ef4fb0ff91ab75086f975837bbbb7a5934ac7e671a80dcababa7203c92fc0c7f84
2021-10-22Make GenTxid boolean constructor privateMarcoFalke
2021-10-22Merge bitcoin/bitcoin#23325: mempool: delete exists(uint256) functionMarcoFalke
4307849256761fe2440d82bbec892d0e8e6b4dd4 [mempool] delete exists(uint256) function (glozow) d50fbd4c5b4bc72415854d582cedf94541a46983 create explicit GenTxid::{Txid, Wtxid} ctors (glozow) Pull request description: We use the same type for txids and wtxids, `uint256`. In places where we want the ability to pass either one, we distinguish them using `GenTxid`. The (overloaded) `CTxMemPool::exists()` function is defined as follows: ```c bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}); } ``` It always assumes that a uint256 is a txid, which is a footgunny interface. Querying by wtxid returns a false negative if the transaction has a witness. :bug: Another approach would be to try both: ```c bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}) || exists(GenTxid{false, txid}); } ``` But that's slower and wrongfully placing the burden on the callee; the caller always knows whether the hash is a txid or a wtxid. ACKs for top commit: laanwj: Code review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 jnewbery: Tested and code review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 MarcoFalke: review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 👘 Tree-SHA512: 8ed167a96f3124b6c14e41073c8358658114ce121a15a4cca2db7a5ac565903a6236e34e88ac03382b8bb8b68e3999abbfc5718bc8c22476554d6b49a5298eec
2021-10-21[mempool] delete exists(uint256) functionglozow
Allowing callers to pass in a uint256 (which could be txid or wtxid) but then always assuming that it's a txid is a footgunny interface.
2021-10-07p2p: Use mocktime for ping timeoutMarcoFalke
2021-10-05Merge bitcoin/bitcoin#22950: [p2p] Pimpl AddrMan to abstract implementation ↵MarcoFalke
details 021f86953e8a1dff8ecc768186368d345c865cc2 [style] Run changed files through clang formatter. (Amiti Uttarwar) 375750387e35ed751d1f5ab48860bdec93977f64 scripted-diff: Rename CAddrInfo to AddrInfo (Amiti Uttarwar) dd8f7f250095e58bbf4cd4effb481b52143bd1ed scripted-diff: Rename CAddrMan to AddrMan (Amiti Uttarwar) 3c263d3f63c3598954ee2b65a0e721e3c22e52f8 [includes] Fix up included files (Amiti Uttarwar) 29727c2aa1233f7c5b91a17884c405e0aef10c6e [doc] Update comments (Amiti Uttarwar) 14f9e000d05f82b364d5a142cafc70b10406b660 [refactor] Update GetAddr_() function signature (Amiti Uttarwar) 40acd6fc9a8098fed85abf4fb727a5f0dff8a2ff [move-only] Move constants to test-only header (Amiti Uttarwar) 7cf41bbb38db5008f9b69037b88138076d6a6cc5 [addrman] Change CAddrInfo access (Amiti Uttarwar) e3f1ea659c9eb1e8be4579923d6acaaab148c2ef [move-only] Move CAddrInfo to test-only header file (Amiti Uttarwar) 7cba9d56185b9325ce41d79364e448462fff0f6a [net, addrman] Remove external dependencies on CAddrInfo objects (Amiti Uttarwar) 8af5b54f973e11c847345418d8631bc301b96130 [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. (Amiti Uttarwar) f2e5f38f09ee40933f752680fe7d75ee8e529fae [move-only] Match ordering of CAddrMan declarations and definitions (Amiti Uttarwar) 5faa7dd6d871eac1a0ec5c4a93f2ad7577781a56 [move-only] Move CAddrMan function definitions to cpp (Amiti Uttarwar) Pull request description: Introduce the pimpl pattern for AddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics. Since the unit & fuzz tests currently rely on accessing AddrMan internals, this PR introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files. ACKs for top commit: jnewbery: ACK 021f86953e8a1dff8ecc768186368d345c865cc2 GeneFerneau: utACK [021f869](https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2) mzumsande: ACK 021f86953e8a1dff8ecc768186368d345c865cc2 rajarshimaitra: Concept + Code Review ACK https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2 theuni: ACK 021f86953e8a1dff8ecc768186368d345c865cc2 Tree-SHA512: aa70cb77927a35c85230163c0cf6d3872382d79048b0fb79341493caa46f8e91498cb787d8b06aba4da17b2f921f2230e73f3d66385519794fff86a831b3a71d
2021-10-05Merge bitcoin/bitcoin#22951: consensus: move amount.h into consensusMarcoFalke
9d0379cea6c164610d05287ae6dd4e66f35b92b3 consensus: use <cstdint> over <stdint.h> in amount.h (fanquake) 863e52fe63a67fa020fb1ef527b9095a35ab77a5 consensus: make COIN & MAX_MONEY constexpr (fanquake) d09071da5bc997f2de1f55ca7a9babc3d7619329 [MOVEONLY] consensus: move amount.h into consensus (fanquake) Pull request description: A first step (of a few) towards some source code reorganization, as well as making libbitcoinconsensus slightly more self contained. Related to #15732. ACKs for top commit: MarcoFalke: concept ACK 9d0379cea6c164610d05287ae6dd4e66f35b92b 🏝 Tree-SHA512: 97fc79262dcb8c00996852a288fee69ddf8398ae2c95700bba5b326f1f38ffcfaf8fa66e29d0cb446d9b3f4e608a96525fae0c2ad9cd531ad98ad2a4a687cd6a
2021-10-04[refactor] pass coinsview and height to check()glozow
Removes check's dependency on validation.h
2021-09-30[MOVEONLY] consensus: move amount.h into consensusfanquake
Move amount.h to consensus/amount.h. Renames, adds missing and removes uneeded includes.
2021-09-28scripted-diff: Rename CAddrMan to AddrManAmiti Uttarwar
-BEGIN VERIFY SCRIPT- git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g' -END VERIFY SCRIPT-
2021-09-28[net processing] Dont request compact blocks in blocks-only modeNiklas Gögge
2021-09-20net: Avoid logging AlreadyHaveTx when disconnecting misbehaving peerMarcoFalke
Can be reviewed with --color-moved=dimmed-zebra
2021-09-12p2p: Rename fBlocksOnly, Add testMarcoFalke
The new name describes better what the bool does and also limits the confusion of the three different concepts: * fBlocksOnly (This bool to skip tx invs) * -blocksonly (A setting to ignore incoming txs) * block-relay-only (A connection type in the block-relay-only P2P graph)
2021-09-27scripted-diff: Rename overloaded int GetArg to GetIntArgRussell Yanofsky
Improve readability of code, simplify future scripted diff cleanup PRs, and be more consistent with naming for GetBoolArg. This will also be useful for replacing runtime settings type checking with compile time checking. -BEGIN VERIFY SCRIPT- git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g' -END VERIFY SCRIPT- Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-08-26Remove GetAddrNameMarcoFalke
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c131-avoid-trivial-getters-and-setters
2021-08-21Drop us=... message in net debug for sending version messagePieter Wuille
The us=... message in the debug log when sending a version message is always [::]:0, because we no longer send our own address there. Therefore, this information is entirely redundant. Remove it.
2021-08-21refactor: move CAddress-without-nTime logic to net_processingPieter Wuille
Historically, the VERSION message contains an "addrMe" and an "addrYou". As these are sent before version negotiation is complete, the protocol version is INIT_PROTO_VERSION (209), and in that protocol, CAddress is serialized without nTime. This is in fact the only situation left where a CAddress is (de)serialized without nTime. As it's such a simple structure (CService for ip/port + uint64_t for nServices), just inline that structure in the few places where it occurs, and remove the logic for dealing with missing nTime from CAddress.
2021-08-14Merge bitcoin/bitcoin#22604: p2p, rpc, test: address rate-limiting follow-upsfanquake
d930c7f5b091687eb4208a5ffe8a2abe311d8054 p2p, rpc, test: address rate-limiting follow-ups (Jon Atack) Pull request description: Incorporates review feedback in #22387. Edit, could be considered separately: should a release note (or two) be added for 22.0? e.g. the new getpeerinfo fields in `Updated RPCs` and the rate-limiting itself in `P2P and network changes`. ACKs for top commit: MarcoFalke: review ACK d930c7f5b091687eb4208a5ffe8a2abe311d8054 theStack: re-ACK d930c7f5b091687eb4208a5ffe8a2abe311d8054 🌮 Zero-1729: crACK d930c7f Tree-SHA512: b2101cad87f59c238603f38bd8e8df7a4d48929794e4de9e0e0ff2afa935a68475c2d369aa669d124a0bec2f50280fb47e8b980bde6ad812db08cf67b71c066a
2021-08-05Merge bitcoin/bitcoin#22618: [p2p] Small follow-ups to 21528MarcoFalke
9778b0fec13c047a4c7f3ae425d044da26eadc81 [net_processing] Provide debug error if code assumptions change. (Amiti Uttarwar) aa79c912608fdc77c69dd43653aa87f1f34e01dd [docs] Add release notes for #21528 (Amiti Uttarwar) Pull request description: Adds a release note & addresses [this](https://github.com/bitcoin/bitcoin/pull/21528#discussion_r680963101) review comment to make expectations more explicit. ACKs for top commit: Zero-1729: re-ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81 jonatack: ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81 Tree-SHA512: 9507df5f2746d05c6df8c86b7a19364610ebfafc81af7650be7e68d7536a0685cce9fd2e5f287ef92b6245c584f8875b24a958109ba5bd8acf3c8fc9fd19eef2
2021-08-04[net_processing] Provide debug error if code assumptions change.Amiti Uttarwar
Currently, this call to SetupAddressRelay will never return false because of the previous guard that returns early if the peer is not an inbound connection. Rather than implicitly relying on this guarantee, throw an error in the debug build if it ever changes.
2021-08-04p2p, rpc, test: address rate-limiting follow-upsJon Atack
2021-08-04Merge bitcoin/bitcoin#22616: p2p, rpc: address relay fixupsMarcoFalke
5e33f762d44557a1e3f0ff3c280d8a3ab98e3867 p2p, rpc: address relay fixups (Jon Atack) Pull request description: Following review of new changes merged today, move a use of `statestats` in getpeerinfo to within the section guarded by `if (fStateStats)`, e.g. `PeerManagerImpl::GetNodeStateStats` true, and pass an in-param by reference to const. ACKs for top commit: amitiuttarwar: ACK 5e33f762d4 jnewbery: ACK 5e33f762d44557a1e3f0ff3c280d8a3ab98e3867 Tree-SHA512: b42f33c615b14079e2c4e6060209de8707d71b351dd1e11e04a2a6fc12d15747d0c5d9b24850217080fd1ef92e63f96d6925c4badf280b781edd696c349be7d6
2021-08-04Merge bitcoin/bitcoin#22577: Close minor startup race between main and ↵MarcoFalke
scheduler threads 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 Close minor startup race between main and scheduler threads (Larry Ruane) Pull request description: This is a low-priority bug fix. The scheduler thread runs `CheckForStaleTipAndEvictPeers()` every 45 seconds (EXTRA_PEER_CHECK_INTERVAL). If its first run happens before the active chain is set up (`CChain::SetTip()`), `bitcoind` will assert: ``` (...) 2021-07-28T22:16:49Z init message: Loading block index… bitcoind: validation.cpp:4968: CChainState& ChainstateManager::ActiveChainstate() const: Assertion `m_active_chainstate' failed. Aborted (core dumped) ``` I ran into this while using the debugger to investigate an unrelated problem. Single-stepping through threads with a debugger can cause the relative thread execution timing to be very different than usual. I don't think any automated tests are needed for this PR. I'll give reproduction steps in the next PR comment. ACKs for top commit: MarcoFalke: cr ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 tryphe: tested ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 0xB10C: ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 glozow: code review ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 - it makes sense to me to start peerman's background tasks here, after `chainstate->LoadChainTip()` and `node.connman->Start()` have been called. Tree-SHA512: 9316ad768cba3b171f62e2eb400e3790af66c47d1886d7965edb38d9710fc8c8f8e4fb38232811c9346732ce311d39f740c5c2aaf5f6ca390ddc48c51a8d633b
2021-08-03p2p, rpc: address relay fixupsJon Atack
2021-08-03Merge bitcoin/bitcoin#21528: [p2p] Reduce addr blackholesfanquake
3f7250b328b8b2f5d63f323702445ac5c989b73d [test] Use the new endpoint to improve tests (Amiti Uttarwar) 3893da06db1eb622f540605700f8663f8d87b2df [RPC] Add field to getpeerinfo to indicate if addr relay is enabled (Amiti Uttarwar) 0980ca78cd930a00c9985d7f00083a3b8e8be89e [test] Test that we intentionally select addr relay peers. (Amiti Uttarwar) c061599e40dc3d379c10b914765061a7a8449dd7 [net_processing] Remove RelayAddrsWithPeer function (Amiti Uttarwar) 201e4964816f8896cfe7b4f6d8ddbfffe7102f87 [net_processing] Introduce new field to indicate if addr relay is enabled (Amiti Uttarwar) 1d1ef2db7ea0d93c7dab4a9800ec74afa7a019eb [net_processing] Defer initializing m_addr_known (Amiti Uttarwar) 6653fa3328b5608fcceda1c6ea8e68c5d58739ec [test] Update p2p_addr_relay test to prepare (Amiti Uttarwar) 2fcaec7bbb96d6fe72a7e3a5744b0c35c79733e8 [net_processing] Introduce SetupAddressRelay (Amiti Uttarwar) Pull request description: This PR builds on the test refactors extracted into #22306 (first 5 commits). This PR aims to reduce addr blackholes. When we receive an `addr` message that contains 10 or less addresses, we forward them to 1-2 peers. This is the main technique we use for self advertisements, so sending to peers that wouldn't relay would effectively "blackhole" the trickle. Although we cannot prevent this in a malicious case, we can improve it for the normal, honest cases, and reduce the overall likelihood of occurrence. Two known cases where peers would not participate in addr relay are if they have connected to you as a block-relay-only connection, or if they are a light client. This implementation defers initialization of `m_addr_known` until it is needed, then uses its presence to decide if the peer is participating in addr relay. For outbound (not block-relay-only) peers, we initialize the filter before sending the initial self announcement when processing their `version` message. For inbound peers, we initialize the filter if/when we get an addr related message (`ADDR`, `ADDRV2`, `GETADDR`). We do NOT initialize the filter based on a `SENDADDRV2` message. To communicate about these changes beyond bitcoin core & to (try to) ensure that no other software would be disrupted, I have: - Posted to the [mailing list](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-April/018784.html) - Researched other open source clients to confirm compatibility, opened issues in all the projects & documented in https://github.com/bitcoin/bitcoin/pull/21528#issuecomment-809906430. Many have confirmed that this change would not be problematic. - Raised as topic during [bitcoin-core-dev meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-03-25.html#l-954) - Raised as topic during [bitcoin p2p meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-04-20.html#l-439) ACKs for top commit: jnewbery: reACK 3f7250b328b8b2f5d63f323702445ac5c989b73d glozow: ACK 3f7250b328b8b2f5d63f323702445ac5c989b73d ajtowns: utACK 3f7250b328b8b2f5d63f323702445ac5c989b73d Tree-SHA512: 29069282af684c1cd37d107c395fdd432dcccb11626f3c2dabfe92fdc4c85e74c7c4056fbdfa88017fec240506639b72ac6c311f8ce7c583112eb15f47e421af
2021-07-30Close minor startup race between main and scheduler threadsLarry Ruane
Don't schedule class PeerManagerImpl's background tasks from its constructor, but instead do that from a separate method, StartScheduledTasks(), that can be called later at the end of startup, after other things, such as the active chain, are initialzed.
2021-07-29[RPC] Add field to getpeerinfo to indicate if addr relay is enabledAmiti Uttarwar
2021-07-29[net_processing] Remove RelayAddrsWithPeer functionAmiti Uttarwar
Now that we have a simple boolean stored on the field, the wrapper function is no longer necessary.
2021-07-29[net_processing] Introduce new field to indicate if addr relay is enabledAmiti Uttarwar
2021-07-29[net_processing] Defer initializing m_addr_knownAmiti Uttarwar
Use SetupAddressRelay to only initialize `m_addr_known` as needed. For outbound peers, we initialize the filter before sending our self announcement (not applicable for block-relay-only connections). For inbound peers, we initialize the filter when we get an addr related message (ADDR, ADDRV2, GETADDR). These changes intend to mitigate address blackholes. Since an inbound peer has to send us an addr related message to become eligible as a candidate for addr relay, this should reduce our likelihood of sending them self-announcements.
2021-07-29[net_processing] Introduce SetupAddressRelayAmiti Uttarwar
Idempotent function that initializes m_addr_known for connections that support address relay (anything other than block-relay-only). Unused until the next commit.
2021-07-28Merge bitcoin/bitcoin#21562: [net processing] Various tidying up of ↵MarcoFalke
PeerManagerImpl ctor fde1bf4f6136638e84cdf9806eedaae08e841bbf [net processing] Default initialize m_recent_confirmed_transactions (John Newbery) 37dcd12d539e4a875581fa049aa0f7fafeb932a4 scripted-diff: Rename recentRejects (John Newbery) cd9902ac5054c01228d52616bf85f7196364d4ff [net processing] Default initialize recentRejects (John Newbery) a28bfd1d4cfa523a6abf3832dbfd6183cd546944 [net processing] Default initialize m_stale_tip_check_time (John Newbery) 9190b01d8dcf03b74e9b9e1653688a97ac171b37 [net processing] Add Orphanage empty consistency check (John Newbery) Pull request description: - Use default initialization of PeerManagerImpl members where possible - Remove unique_ptr indirection where it's not needed ACKs for top commit: MarcoFalke: ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf 👞 theStack: re-ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf Tree-SHA512: 7ddedcc972df8e933e1fbe5c88b8ea17df89e1e58fc769518512c5540e49dc8eddb3f47e78d1329a6fc5644d2c1d11c981f681fd633f5218bfa4b3e6a86f3d7b
2021-07-27tracing: Tracepoints for in- and outbound P2P msgs0xb10c
Can be used to monitor in- and outbound node traffic. Based on ealier work by jb55. Co-authored-by: William Casarin <jb55@jb55.com>
2021-07-27Merge bitcoin/bitcoin#22495: p2p: refactor: tidy up ↵MarcoFalke
`PeerManagerImpl::Misbehaving(...)` 8858e88c840197cdcabea07dd1380ef2aa4ece02 p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)` (Sebastian Falbesoner) Pull request description: This simple refactoring PR has the goal to improve the readability of the `Misbehaving` method by - introducing constant variables `score_before` and `score_now` (to avoid repeatedly calculating the former) - deduplicating calls to LogPrint(), eliminates else-branch ACKs for top commit: jnewbery: utACK 8858e88c840197cdcabea07dd1380ef2aa4ece02 rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/22495/commits/8858e88c840197cdcabea07dd1380ef2aa4ece02 Tree-SHA512: 1d4dd5ac1d16ee9595edf4fa46e4960915a203641d74e6c33cffaba62ea71328834309a4451256fb45daf759f0cf6f4f199c46815afff6c89c0746e2ad4d4092
2021-07-26p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)`Sebastian Falbesoner
- introduce constant variables `score_before` and `score_after` in order to improve readability - deduplicate calls to LogPrint(), eliminates else-branch
2021-07-22Merge bitcoin/bitcoin#21090: Default to NODE_WITNESS in nLocalServicesW. J. van der Laan
a806647d260132a00cd633160040625c7dd17803 [validation] Always include merkle root in coinbase commitment (Dhruv Mehta) 189128c220190a588500b8e74ee7ae47671b9558 [validation] Set witness script flag with p2sh for blocks (Dhruv Mehta) ac82b99db77ec843af82dcdf040dfdbc98c8ff26 [p2p] remove redundant NODE_WITNESS checks (Dhruv Mehta) 6f8b198b8256a6703a6f5e592dfa77fa024a7035 [p2p] remove unused segwitheight=-1 option (Dhruv Mehta) eba5b1cd6460c98e75d0422bd394e12af7f11e4c [test] remove or move tests using `-segwitheight=-1` (Dhruv Mehta) Pull request description: Builds on #21009 and makes progress on remaining items in #17862 Removing `RewindBlockIndex()` in #21009 allows the following: - removal of tests using `segwitheight=-1` in `p2p_segwit.py`. - move `test_upgrade_after_activation()` out of `p2p_segwit.py` reducing runtime - in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test. - that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`. - that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS` ACKs for top commit: mzumsande: Code-Review ACK a806647d260132a00cd633160040625c7dd17803 laanwj: Code review ACK a806647d260132a00cd633160040625c7dd17803, nice cleanup jnewbery: utACK a806647d260132a00cd633160040625c7dd17803 theStack: ACK a806647d260132a00cd633160040625c7dd17803 Tree-SHA512: 73e1a69d1d7eca1f5c38558ec6672decd0b60b16c2ef6134df6f6af71bb159e6eea160f9bb5ab0eb6723c6632d29509811e29469d0d87abbe9b69a2890fbc73e
2021-07-20Merge bitcoin/bitcoin#22096: p2p: AddrFetch - don't disconnect on ↵fanquake
self-announcements 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande) c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande) 533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande) b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande) Pull request description: AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them. This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement. So we'll disconnect before the peer gets a chance to answer our `getaddr`. I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us. The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.  Fix this by only disconnecting after receiving an `addr` message of size > 1. [Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test. ACKs for top commit: amitiuttarwar: reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 naumenkogs: ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 jnewbery: ACK 5730a43703 Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
2021-07-20[net processing] Default initialize m_recent_confirmed_transactionsJohn Newbery
Now that m_recent_confirmed_transactions is owned by PeerManagerImpl, and PeerManagerImpl's lifetime is managed by the node context, we can just default initialize m_recent_confirmed_transactions during object initialization. We can also remove the unique_ptr indirection.
2021-07-20scripted-diff: Rename recentRejectsJohn Newbery
-BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren recentRejects m_recent_rejects -END VERIFY SCRIPT-
2021-07-20[net processing] Default initialize recentRejectsJohn Newbery
Now that recentRejects is owned by PeerManagerImpl, and PeerManagerImpl's lifetime is managed by the node context, we can just default initialize recentRejects during object initialization. We can also remove the unique_ptr indirection.
2021-07-20[net processing] Default initialize m_stale_tip_check_timeJohn Newbery
2021-07-20[net processing] Add Orphanage empty consistency checkJohn Newbery
When removing the final peer, assert that m_tx_orphanage is empty.