aboutsummaryrefslogtreecommitdiff
path: root/src/net_processing.cpp
AgeCommit message (Collapse)Author
2020-05-12Merge #18808: [net processing] Drop unknown types in getdatafanquake
9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 [docs] Improve commenting in ProcessGetData() (John Newbery) 2f032556e08a04807c71eb02104ca9589eaadf1b [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar) e257cf71c851e25e1a533bf1d4296f6b55c81332 [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar) 047ceac142246b5d51056a51dbf4645b31802be4 [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar) Pull request description: Currently we'll stall peers that send us an unknown INV type in a GETDATA message. Be a bit more friendly and just drop the invalid request. Ditto for blocks-relay-only peers that send us a GETDATA for a transaction. There's a test for the first part. The second is difficult to test in the functional test framework since we aren't able to make blocks-relay-only connections. ACKs for top commit: sipa: utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 brakmic: ACK https://github.com/bitcoin/bitcoin/commit/9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 luke-jr: utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 naumenkogs: utACK 9847e20 ajtowns: utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 Tree-SHA512: 6007f2fd839ffe737727f6fb8e8f083b2d9e05a510748f1d40b8f9be8fdf7b5419a36d8f1039923eec1ba2983e8f6f0436ec5fc196d9f6dcb0657f2ff8ff8e4c
2020-05-06Merge #18806: net: remove is{Empty,Full} flags from CBloomFilter, clarify ↵fanquake
CVE fix 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix (Sebastian Falbesoner) Pull request description: The BIP37 bloom filter class `CBloomFilter` contains two flags `isEmpty`/`isFull` together with an update method with the purpose to, according to the comments, "avoid wasting cpu", i.e. the mechanism should serve as an optimization for the trivial cases of empty (all bits zero) or full (all bits one) filters. However, the real reason of adding those flags (introduced with commit https://github.com/bitcoin/bitcoin/commit/37c6389c5a0ca63ae3573440ecdfe95d28ad8f07 by gmaxwell) was a _covert fix_ of [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700), a vulnerability that allowed a divide-by-zero remote node crash. According to gmaxwell himself (https://github.com/bitcoin/bitcoin/pull/9060#issuecomment-257749165): > the IsEmpty/IsFull optimizations were largely a pretextual optimization intended to make unexploitable a remote crash vulnerability (integer division by zero) that existed in the original bloom filtering code without disclosing it. I'm doubtful that they are all that useful. :) For more information on how to trigger this crash, see PR https://github.com/bitcoin/bitcoin/pull/18515 which contains a detailled description and a regression test. It has also been discussed on a [recent PR club meeting on fuzzing](https://bitcoincore.reviews/18521.html). The covert fix code already led to issues and PR based on the wrong assumption that the flags are there for optimization reasons (see #16886 and #16922). This PR gets rid of the flags and the update method and just focuses on the CVE fix itself, i.e. it can be seen as a revert of the covert fix commit modulo the actual fix. ACKs for top commit: meshcollider: utACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 laanwj: Concept and code review ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 jkczyz: ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 MarcoFalke: ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 fjahr: Code review ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 Tree-SHA512: 29f7ff9faece0285e11e16c024851f5bcb772dec64118ccc3f9067ec256267ec8e1b1e3105c7de2a72fd122c3b085e8fc840ab8f4e49813f1cc7a444df1867f7
2020-04-29[docs] Improve commenting in ProcessGetData()John Newbery
2020-04-29[net processing] ignore unknown INV types in GETDATA messagesAmiti Uttarwar
Co-Authored-By: John Newbery <john@johnnewbery.com>
2020-04-29[net processing] ignore tx GETDATA from blocks-only peersAmiti Uttarwar
Co-Authored-By: John Newbery <john@johnnewbery.com>
2020-04-29Merge #18038: P2P: Mempool tracks locally submitted transactions to improve ↵fanquake
wallet privacy 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 [mempool] Persist unbroadcast set to mempool.dat (Amiti Uttarwar) 297a1785360c4db662a7f3d3ade7b6b503258d39 [test] Integration tests for unbroadcast functionality (Amiti Uttarwar) 6851502472d3625416f0e7796e9f2a0379d14d49 [refactor/test] Extract P2PTxInvStore into test framework (Amiti Uttarwar) dc1da48dc5e5526215561311c184a8cbc345ecdc [wallet] Update the rebroadcast frequency to be ~1/day. (Amiti Uttarwar) e25e42f20a3aa39651fbc1f9fa3df1a49f1f5868 [p2p] Reattempt initial send of unbroadcast transactions (Amiti Uttarwar) 7e93eecce3bc5a1b7bb0284e06f9e2e69454f5ba [util] Add method that returns random time in milliseconds (Amiti Uttarwar) 89eeb4a3335f8e871cc3f5286af4546dff66172a [mempool] Track "unbroadcast" transactions (Amiti Uttarwar) Pull request description: This PR introduces mempool tracking of unbroadcast transactions and periodic reattempts at initial broadcast. This is a part of the rebroadcast project, and a standalone privacy win. The current rebroadcast logic is terrible for privacy because 1. only the source wallet rebroadcasts transactions and 2. it does so quite frequently. In the current system, if a user submits a transaction that does not immediately get broadcast to the network (eg. they are offline), this "rebroadcast" behavior is the safety net that can actually serve as the initial broadcast. So, keeping the attempts frequent is important for initial delivery within a reasonable timespan. This PR aims to improve # 2 by reducing the wallet rebroadcast frequency to ~1/day from ~1/15 min. It achieves this by separating the notion of initial broadcast from rebroadcasts. With these changes, the mempool tracks locally submitted transactions & periodically reattempts initial broadcast. Transactions submitted via the wallet or RPC are added to an "unbroadcast" set & are removed when a peer sends a `getdata` request, or the transaction is removed from the mempool. Every 10-15 minutes, the node reattempts an initial broadcast. This enables reducing the wallet rebroadcast frequency while ensuring the transactions will be propagated to the network. For privacy improvements around # 1, please see #16698. Thank you to gmaxwell for the idea of how to break out this subset of functionality (https://github.com/bitcoin/bitcoin/pull/16698#issuecomment-571399346) ACKs for top commit: fjahr: Code review ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 MarcoFalke: ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00, I think this is ready for merge now 👻 amitiuttarwar: The current tip `50fc4df` currently has 6 ACKs on it, so I've opened #18807 to address the last bits. jnewbery: utACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00. ariard: Code Review ACK 50fc4df (minor points no need to invalid other ACKs) robot-visions: ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 sipa: utACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00 naumenkogs: utACK 50fc4df Tree-SHA512: 2dd935d645d5e209f8abf87bfaa3ef0e4492705ce7e89ea64279cb27ffd37f4727fa94ad62d41be331177332f8edbebf3c7f4972f8cda10dd951b80a28ab3c0f
2020-04-28net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fixSebastian Falbesoner
2020-04-23[p2p] Reattempt initial send of unbroadcast transactionsAmiti Uttarwar
Every 10-15 minutes, the scheduler kicks off a job that queues unbroadcast transactions onto each node.
2020-04-23[mempool] Track "unbroadcast" transactionsAmiti Uttarwar
- Mempool tracks locally submitted transactions (wallet or rpc) - Transactions are removed from set when the node receives a GETDATA request from a peer, or if the transaction is removed from the mempool.
2020-04-23[net processing] Move all const declarations to top of net_processing.cppJohn Newbery
2020-04-23[net processing] Move net processing consts to net_processing.cppJohn Newbery
2020-04-20Merge #18544: net: limit BIP37 filter lifespan (active between ↵MarcoFalke
'filterload'..'filterclear') a9ecbdfcaa15499644d16e9c8ad2c63dfc45b37b test: add more inactive filter tests to p2p_filter.py (Sebastian Falbesoner) 5eae034996b340c19cebab9efb6c89d20fe051ef net: limit BIP37 filter lifespan (active between 'filterload' and 'filterclear') (Sebastian Falbesoner) Pull request description: This PR fixes https://github.com/bitcoin/bitcoin/issues/18483. On the master branch, there is currently _always_ a BIP37 filter set for every peer: if not a specific filter is set through a `filterload` message, a default match-everything filter is instanciated and pointed to via the `CBloomFilter` default constructor; that happens both initially, when the containing structure `TxRelay` is constructed: https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/src/net.h#L812 and after a loaded filter is removed again through a `filterclear` message: https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/src/net_processing.cpp#L3201 The behaviour was introduced by commit https://github.com/bitcoin/bitcoin/commit/37c6389c5a0ca63ae3573440ecdfe95d28ad8f07 (an intentional covert fix for [CVE-2013-5700](https://github.com/bitcoin/bitcoin/pull/18515), according to gmaxwell). This default match-everything filter leads to some unintended side-effects: 1. `getdata` request for filtered blocks (i.e. type `MSG_FILTERED_BLOCK`) are always responded to with `merkleblock`s, even if no filter was set by the peer, see issue #18483 (strictly speaking, this is a violation of BIP37) https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/src/net_processing.cpp#L1504-L1507 2. if a peer sends a `filteradd` message without having loaded a filter via `filterload` before, the intended increasing of the banscore never happens (triggered if `bad` is set to true, a few lines below) https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/src/net_processing.cpp#L3182-L3186 This PR basically activates the `else`-branch code paths for all checks of `pfilter` again (on the master branch, they are dead code) by limiting the pointer's lifespan: instead of always having a filter set, the `pfilter` is only pointing to a `CBloomFilter`-instance after receiving a `filterload` message and the instance is destroyed again (and the pointer nullified) after receiving a `filterclear` message. Here is a before/after comparison in behaviour: | code part / scenario | master branch | PR branch | | --------------------------------------------- | ---------------------------------- | ---------------------------------------------------- | | `getdata` processing for `MSG_FILTERED_BLOCK` | always responds with `merkleblock` | only responds if filter was set via `filterload` | | `filteradd` processing, no filter was loaded | nothing | peer's banscore increases by 100 (i.e. disconnect) | On the other code parts where `pfilter` is checked there is no change in the logic behaviour (except that `CBloomFilter::IsRelevantAndUpdate()` is unnecessarily called and immediately returned in the master branch). Note that the default constructor of `CBloomFilter` is only used for deserializing the received `filterload` message and nowhere else. The PR also contains a functional test checking that sending `getdata` for filtered blocks is ignored by the node if no bloom filter is set. ACKs for top commit: MarcoFalke: re-ACK a9ecbdfcaa, only change is in test code 🕙 Tree-SHA512: 1a656a6d74ccaf628e7fdca063ba63fbab2089e0b6d0a11be9bbd387c2ee6d3230706ff8ffc1a55711481df3d4547137dd7c9d9184d89eaa43ade4927792d0b6
2020-04-10Merge #18454: net: Make addr relay mockable, add testMarcoFalke
fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde test: Add basic addr relay test (MarcoFalke) fa1793c1c44a3f75a09f9c636467b8274c541bdd net: Pass connman const when relaying address (MarcoFalke) fa47a0b003f53708b6d5df1ed4e7f8a7c68aa3ac net: Make addr relay mockable (MarcoFalke) Pull request description: As usual: * Switch to std::chrono time to be type-safe and mockable * Add basic test that relies on mocktime to add code coverage ACKs for top commit: naumenkogs: utACK fa1da3d promag: ACK fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde (fabe56e44b6f683e24e37246a7a8851190947cb3 before https://github.com/bitcoin/bitcoin/pull/18454#issuecomment-607866453), fa5bf23d527a450e72c2bf13d013e5393b664ca3 was dropped since last review. Tree-SHA512: 0552bf8fcbe375baa3cab62acd8c23b2994efa47daff818ad1116d0ffaa0b9e520dc1bca2bbc68369b25584e85e54861fe6fd0968de4f503b95439c099df9bd7
2020-04-09net: limit BIP37 filter lifespan (active between 'filterload' and 'filterclear')Sebastian Falbesoner
Previously, a default match-everything bloom filter was set for every peer, i.e. even before receiving a 'filterload' message and after receiving a 'filterclear' message code branches checking for the existence of the filter by testing the pointer "pfilter" were _always_ executed.
2020-04-06scripted-diff: Replace strCommand with msg_typeMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i 's/\<strCommand\>/msg_type/g' ./src/net_processing.cpp ./src/test/fuzz/process_message.cpp -END VERIFY SCRIPT-
2020-04-02net: Pass connman const when relaying addressMarcoFalke
2020-03-27net: Make addr relay mockableMarcoFalke
2020-03-19Merge #17477: Remove the mempool's NotifyEntryAdded and NotifyEntryRemoved ↵Wladimir J. van der Laan
signals e57980b4738c10344baf136de3e050a3cb958ca5 [mempool] Remove NotifyEntryAdded and NotifyEntryRemoved callbacks (John Newbery) 2dd561f36124972d2364f941de9c3417c65f05b6 [validation] Remove pool member from ConnectTrace (John Newbery) 969b65f3f527631ede1a31c7855151e5c5d91f8f [validation] Remove NotifyEntryRemoved callback from ConnectTrace (John Newbery) 5613f9842b4000fed088b8cf7b99674c328d15e1 [validation] Remove conflictedTxs from PerBlockConnectTrace (John Newbery) cdb893443cc16edf974f099b8485e04b3db1b1d7 [validation interface] Remove vtxConflicted from BlockConnected (John Newbery) 1168394d759b13af68acec6d5bfa04aaa24561f8 [wallet] Notify conflicted transactions in TransactionRemovedFromMempool (John Newbery) Pull request description: These boost signals were added in #9371, before we had a `TransactionRemovedFromMempool` method in the validation interface. The `NotifyEntryAdded` callback was used by validation to build a vector of conflicted transactions when connecting a block, which the wallet was notified of in the `BlockConnected` CValidationInterface callback. Now that we have a `TransactionRemovedFromMempool` callback, we can fire that signal directly from the mempool for conflicted transactions. Note that #9371 was implemented to ensure `-walletnotify` events were fired for these conflicted transaction. We inadvertently stopped sending these notifications in #16624 (Sep 2019 commit 7e89994). We should probably fix that, but in a different PR. ACKs for top commit: jonatack: Re-ACK e57980b ryanofsky: Code review ACK e57980b4738c10344baf136de3e050a3cb958ca5, no code changes since previous review, but helpful new code comments have been added and the PR description is now more clear about where the old code came from Tree-SHA512: 3bdbaf1ef2731e788462d4756e69c42a1efdcf168691ce1bbfdaa4b7b55ac3c5b1fd4ab7b90bcdec653703600501b4224d252cfc086aef28f9ce0da3b0563a69
2020-03-18net: fix use-after-free in testsVasil Dimov
In PeerLogicValidation::PeerLogicValidation() we would schedule a lambda function to execute later, capturing the local variable `consensusParams` by reference. Presumably this was considered safe because `consensusParams` is a reference itself to a global variable which is not supposed to change, but it can in tests. Fixes https://github.com/bitcoin/bitcoin/issues/18372
2020-03-17Merge #18289: refactor: Make scheduler methods type safeMarcoFalke
fa36f3a29538012a6eb5c3402b3b3c18fd32b230 refactor: move DUMP_BANS_INTERVAL to banman.h (MarcoFalke) fadafb83cff9a9a340eac1b5a853e2467d5e0ef7 scheduler: Make schedule* methods type safe (MarcoFalke) fa70ccc6c4e304646b4610228f3975b3a9762643 scheduler: Use C++11 member initialization, add shutdown assert (MarcoFalke) Pull request description: Main benefit is that stuff like `15 * 60 * 1000` is replaced by `minutes{15}` ACKs for top commit: vasild: ACK fa36f3a (code review, not tested) ajtowns: ACK fa36f3a29538012a6eb5c3402b3b3c18fd32b230 jonatack: ACK fa36f3a Tree-SHA512: f35f1a1d643dfa676bd47474659f6492ed05cca04cdb556064b126f654a6a44a4b93fcaddcdcd41faf81b8f11439c11e5c7ab88685ba2eef12f7188843d17ad8
2020-03-12refactor: Remove mempool global from netMarcoFalke
This refactor does two things: * Pass mempool in to PeerLogicValidation * Pass m_mempool around where needed
2020-03-11[validation interface] Remove vtxConflicted from BlockConnectedJohn Newbery
The wallet now uses TransactionRemovedFromMempool to be notified about conflicted wallet, and no other clients use vtxConflicted.
2020-03-11tests: Add fuzzing harness for ProcessMessage(...)practicalswift
2020-03-10scheduler: Make schedule* methods type safeMarcoFalke
2020-02-27Refactor FormatStateMessage into ValidationStateJeffrey Czyz
2020-02-11net: Remove forcerelay of rejected txsMarcoFalke
2020-02-02net: reference instead of copy in BlockConnected range loopJon Atack
to fix -Wrange-loop-analysis warning introduced in a029e18
2020-01-31Merge #17951: Use rolling bloom filter of recent block txs for AlreadyHave() ↵Jonas Schnelli
check a029e18c2bf67dd00552b0f4bbc85fa2fa5b973b Use rolling bloom filter of recent block tx's for AlreadyHave() check (Suhas Daftuar) Pull request description: In order to determine whether to download or process a relayed transaction, we first try to check whether we already have the transaction -- either in the mempool, in our filter of recently rejected transactions, in our orphan pool, or already confirmed in a block. Prior to this commit, the heuristic for checking whether a transaction was confirmed in a block is based on whether there's a coin cache entry corresponding to the 0- or 1-index vout of the tx. While that is a quick check, it is very imprecise (eg if those outputs were already spent in another block, we wouldn't detect that the transaction has already been confirmed) -- we can do better by just keeping a rolling bloom filter of the transactions in recent blocks, which will better capture the case of a transaction which has been confirmed and then fully spent. This should reduce the bandwidth that we waste by requesting transactions which will not be accepted to the mempool. To avoid relay problems for transactions which have been included in a recent block but then reorged out of the chain, we clear the bloom filter whenever a block is disconnected. ACKs for top commit: MarcoFalke: re-ACK a029e18c2b only stylistic and comment fixups 🍴 sipa: utACK a029e18c2bf67dd00552b0f4bbc85fa2fa5b973b jonatack: Code review ACK a029e18c2bf67dd00552b0f4bbc85fa2fa5b973b also built/ran tests and am running bitcoind with mempool debug logging and custom logging. Looked a bit into CRollingBloomFilter and also the mempool median time past checks mentioned above; I don't have a deep understanding of those areas yet but the concept here and changes LGTM. Tests and other optimisations could be added as a follow-up. In favor of seeing this move forward if no major immediate concerns. Tree-SHA512: 784c9a35bcd3af5db469063ac7d26b4bac430e451e5637a34d8a538c3ffd1433abdd3f06e5584e7a84bfa9e791449e61819397b5a6c7890fa59d78ec3ba507b2
2020-01-29Use rolling bloom filter of recent block tx's for AlreadyHave() checkSuhas Daftuar
In order to determine whether to download or process a relayed transaction, we try to determine if we already have the transaction, either in the mempool, in our recently rejected filter, in our orphan pool, or already confirmed in the chain itself. Prior to this commit, the heuristic for checking the chain is based on whether there's an output corresponding to the 0- or 1-index vout in our coin cache. While that is a quick check, it is very imprecise (say if those outputs were already spent in a block) -- we can do better by just keeping a rolling bloom filter of the transactions in recent blocks, which will capture the case of a transaction which has been confirmed and then fully spent already. To avoid relay problems for transactions which have been included in a recent block but then reorged out of the chain, we clear the bloom filter whenever a block is disconnected.
2020-01-16Fix doxygen errorsBen Woosley
Identified via -Wdocumentation, e.g.: ./rpc/rawtransaction_util.h:31:13: error: parameter 'prevTxs' not found in the function declaration [-Werror,-Wdocumentation] * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain ^~~~~~~ ./rpc/rawtransaction_util.h:31:13: note: did you mean 'prevTxsUnival'? * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain ^~~~~~~ prevTxsUnival netbase.cpp:766:11: error: parameter 'outProxyConnectionFailed[out]' not found in the function declaration [-Werror,-Wdocumentation] * @param outProxyConnectionFailed[out] Whether or not the connection to the ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ netbase.cpp:766:11: note: did you mean 'outProxyConnectionFailed'? * @param outProxyConnectionFailed[out] Whether or not the connection to the ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ outProxyConnectionFailed
2020-01-15scripted-diff: Bump copyright of files changed in 2020MarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2020-01-15scripted-diff: Replace CCriticalSection with RecursiveMutexMarcoFalke
-BEGIN VERIFY SCRIPT- # Delete outdated alias for RecursiveMutex sed -i -e '/CCriticalSection/d' ./src/sync.h # Replace use of outdated alias with RecursiveMutex sed -i -e 's/CCriticalSection/RecursiveMutex/g' $(git grep -l CCriticalSection) -END VERIFY SCRIPT-
2020-01-02Merge #17762: net: Log to net category for exceptions in ProcessMessagesWladimir J. van der Laan
4bdd68f301a9cee3360deafc7531c638e923226b Add missing typeinfo includes (Wladimir J. van der Laan) 4d88c3dcb61e7c075ed3dd442044e0eff4e3c8de net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan) Pull request description: Remove the forest of special exceptions based on string matching, and simply log a short message to the NET logging category when an exception happens during packet processing. It is not good to panick end users with verbose errors (let alone writing to stderr) when any peer can generate them. ACKs for top commit: MarcoFalke: re-ACK 4bdd68f301a9cee3360deafc7531c638e923226b (only change is adding includes) 🕕 promag: ACK 4bdd68f301a9cee3360deafc7531c638e923226b, could squash. Tree-SHA512: a005591a3202b005c75e01dfa54249db3992e2f9eefa8b3d9d435acf66130417716ed926ce4e045179cf43788f1abc7362d999750681a9c80b318373d611c366
2020-01-02Add missing typeinfo includesWladimir J. van der Laan
The use of `typeid()` for logging exception types requires this include according to https://en.cppreference.com/w/cpp/language/typeid.
2019-12-30scripted-diff: Bump copyright of files changed in 2019MarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2019-12-17net: Log to net category for exceptions in ProcessMessagesWladimir J. van der Laan
Remove the forest of special exceptions, and simply log a short message to the NET logging category when an exception happens during packet processing. It is not good to panick end users with errors that any peer can generate (let alone writing to stderr).
2019-11-05Merge #17243: p2p: add PoissonNextSend method that returns mockable timeMarcoFalke
1a8f0d5a74d5cc0000456932babf35301f5c1686 [tools] update nNextInvSend to use mockable time (Amiti Uttarwar) 4de630354fc6808b9b13b9e82da1a82f2f50f26a [tools] add PoissonNextSend method that returns mockable time (Amiti Uttarwar) Pull request description: Introduce a Poisson helper method that wraps the existing method to return `std::chrono::duration` type, which is mockable. Needed for https://github.com/bitcoin/bitcoin/pull/16698. ACKs for top commit: ajtowns: ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686 MarcoFalke: re-ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686 naumenkogs: ACK 1a8f0d5, and let's merge it and come back to it later. Tree-SHA512: 7e2325d7c55fc0b4357cb86b83e0c218ba269f678c1786342d8bc380bfd9696373bc24ff124b9ff17a6e761c62b2b44ff5247c3911e2afdc7cc5c20417e8290b
2019-11-05[tools] update nNextInvSend to use mockable timeAmiti Uttarwar
2019-11-04Merge #17164: p2p: Avoid allocating memory for addrKnown where we don't need itMarcoFalke
b6d2183858975abc961207c125c15791e531edcc Minor refactoring to remove implied m_addr_relay_peer. (User) a552e8477c5bcd22a5457f4f73a2fd6db8acd2c2 added asserts to check m_addr_known when it's used (User) 090b75c14be6b9ba2efe38a17d141c6e6af575cb p2p: Avoid allocating memory for addrKnown where we don't need it (User) Pull request description: We should allocate memory for addrKnown filter only for those peers which are expected to participate in address relay. Currently, we do it for all peers (including SPV and block-relay-only), which results in extra RAM where it's not needed. Upd: In future, we would still allow SPVs to ask for addrs, so allocation still will be done by default. However, they will be able to opt-out via [this proposal](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-October/017428.html) and then we could save some more memory. This PR still saves memory for block-relay-only peers immediately after merging. Top commit has no ACKs. Tree-SHA512: e84d93b2615556d466f5ca0e543580fde763911a3bfea3127c493ddfaba8f05c8605cb94ff795d165af542b594400995a2c51338185c298581408687e7812463
2019-10-30Merge #15921: validation: Tidy up ValidationState interfaceWladimir J. van der Laan
3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf [validation] Remove fMissingInputs from AcceptToMemoryPool() (John Newbery) c428622a5bb1e37b2e6ab2c52791ac05d9271238 [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders() (John Newbery) 7204c6434b944f6ad51b3c895837729d3aa56eea [validation] Remove useless ret parameter from Invalid() (John Newbery) 1a37de4b3174d19a6d8691ae07e92b32fdfaef11 [validation] Remove error() calls from Invalid() calls (John Newbery) 067981e49246822421a7bcc720491427e1dba8a3 [validation] Tidy Up ValidationResult class (John Newbery) a27a2957ed9afbe5a96caa5f0f4cbec730d27460 [validation] Add CValidationState subclasses (John Newbery) Pull request description: Carries out some remaining tidy-ups remaining after PR 15141: - split ValidationState into TxValidationState and BlockValidationState (commit from ajtowns) - various minor code style tidy-ups to the ValidationState class - remove the useless `ret` parameter from `ValidationState::Invalid()` - remove the now unused `first_invalid` parameter from `ProcessNewBlockHeaders()` - remove the `fMissingInputs` parameter from `AcceptToMemoryPool()`, and deal with missing inputs the same way as other errors by using the `TxValidationState` object. Tip for reviewers (thanks ryanofsky!): The first commit ("[validation] Add CValidationState subclasses" ) is huge and can be easier to start reviewing if you revert the rote, mechanical changes: Substitute the commit hash of commit "[validation] Add CValidationState subclasses" for <CommitHash> in the commands below. ```sh git checkout <CommitHash> git grep -l ValidationState | xargs sed -i 's/BlockValidationState\|TxValidationState/CValidationState/g' git grep -l ValidationResult | xargs sed -i 's/BlockValidationResult\|TxValidationResult/ValidationInvalidReason/g' git grep -l MaybePunish | xargs sed -i 's/MaybePunishNode\(ForBlock\|ForTx\)/MaybePunishNode/g' git diff HEAD^ ``` After that it's possible to easily see the mechanical changes with: ```sh git log -p -n1 -U0 --word-diff-regex=. <CommitHash> ``` ACKs for top commit: laanwj: ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf amitiuttarwar: code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Also built & ran tests locally. fjahr: Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf . Only nit style change and pure virtual destructor added since my last review. ryanofsky: Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Just whitespace change and pure virtual destructor added since last review. Tree-SHA512: 511de1fb380a18bec1944ea82b513b6192df632ee08bb16344a2df3c40811a88f3872f04df24bc93a41643c96c48f376a04551840fd804a961490d6c702c3d36
2019-10-29[validation] Remove fMissingInputs from AcceptToMemoryPool()John Newbery
Handle this failure in the same way as all other failures: call Invalid() with the reasons for the failure.
2019-10-29[validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders()John Newbery
No callers use the returned value in first_invalid. Remove it from the function signature and don't set it in the function.
2019-10-29[validation] Add CValidationState subclassesJohn Newbery
Split CValidationState into TxValidationState and BlockValidationState to store validation results for transactions and blocks respectively.
2019-10-28Pass NodeContext, ConnMan, BanMan references more placesRussell Yanofsky
So g_connman and g_banman globals can be removed next commit.
2019-10-28Merge #16202: p2p: Refactor network message deserializationfanquake
ed2dc5e48abed1cde6ab98025dc8212917d47d21 Add override/final modifiers to V1TransportDeserializer (Pieter Wuille) f342a5e61a73e1edf389b662d265d20cf26a1d51 Make resetting implicit in TransportDeserializer::Read() (Pieter Wuille) 6a91499496d76c2b3e84489e9723b60514fb08db Remove oversized message detection from log and interface (Pieter Wuille) b0e10ff4df3d4c70fb172ea8c3128c82e6e368bb Force CNetMessage::m_recv to use std::move (Jonas Schnelli) efecb74677222f6c70adf7f860c315f430d39ec4 Use adapter pattern for the network deserializer (Jonas Schnelli) 1a5c656c3169ba525f84145d19ce8c64f2cf1efb Remove transport protocol knowhow from CNetMessage / net processing (Jonas Schnelli) 6294ecdb8bb4eb7049a18c721ee8cb4a53d80a06 Refactor: split network transport deserializing from message container (Jonas Schnelli) Pull request description: **This refactors the network message deserialization.** * It transforms the `CNetMessage` into a transport protocol agnostic message container. * A new class `TransportDeserializer` (unique pointer of `CNode`) is introduced, handling the network buffer reading and the decomposing to a `CNetMessage` * **No behavioral changes** (in terms of disconnecting, punishing) * Moves the checksum finalizing into the `SocketHandler` thread (finalizing was in `ProcessMessages` before) The **optional last commit** makes the `TransportDeserializer` following an adapter pattern (polymorphic interface) to make it easier to later add a V2 transport protocol deserializer. Intentionally not touching the sending part. Pre-Requirement for BIP324 (v2 message transport protocol). Replacement for #14046 and inspired by a [comment](https://github.com/bitcoin/bitcoin/pull/14046#issuecomment-431528330) from sipa ACKs for top commit: promag: Code review ACK ed2dc5e48abed1cde6ab98025dc8212917d47d21. marcinja: Code review ACK ed2dc5e48abed1cde6ab98025dc8212917d47d21 ryanofsky: Code review ACK ed2dc5e48abed1cde6ab98025dc8212917d47d21. 4 cleanup commits added since last review. Unaddressed comments: ariard: Code review and tested ACK ed2dc5e. Tree-SHA512: bab8d87464e2e8742529e488ddcdc8650f0c2025c9130913df00a0b17ecdb9a525061cbbbd0de0251b76bf75a8edb72e3ad0dbf5b79e26f2ad05d61b4e4ded6d
2019-10-25added asserts to check m_addr_known when it's usedUser
2019-10-24Merge #16851: Continue relaying transactions after they expire from mapRelayMarcoFalke
168b781fe7f3f13b24c52a151f36de4cdd0a340a Continue relaying transactions after they expire from mapRelay (Anthony Towns) Pull request description: This change allows peers to request transactions even after they've expired from mapRelay and even if they're not doing mempool requests. This is intended to allow for CPFP of old transactions -- if parent tx P wasn't relayed due to low fees, then a higher fee rate child C is relayed, peers will currently request the parent P, but we prior to this patch, we will not relay it due to it not being in mapRelay. ACKs for top commit: MarcoFalke: re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a (only change is comment fixup) sdaftuar: re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a sipa: ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a Tree-SHA512: b206666dd1450cd0a161ae55fd1a7eda2c3d226842ba27d91fe463b551fd924b65b92551b14d6786692e15cf9a9a989666550dfc980b48ab0f8d4ca305bc7762
2019-10-24Merge #17004: validation: Remove REJECT code from CValidationStateWladimir J. van der Laan
9075d13153ce06cd59a45644831ecc43126e1e82 [docs] Add release notes for removal of REJECT reasons (John Newbery) 04a2f326ec0f06fb4fce1c4f93500752f05dede8 [validation] Fix REJECT message comments (John Newbery) e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc [validation] Remove REJECT code from CValidationState (John Newbery) 0053e16714323c1694c834fdca74f064a1a33529 [logging] Don't log REJECT code when transaction is rejected (John Newbery) a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab [validation] Fix peer punishment for bad blocks (John Newbery) Pull request description: We no longer send BIP 61 REJECT messages, so there's no need to set a REJECT code in the CValidationState object. Note that there is a minor bug fix in p2p behaviour here. Because the call to `MaybePunishNode()` in `PeerLogicValidation::BlockChecked()` only previously happened if the REJECT code was > 0 and < `REJECT_INTERNAL`, then there are cases were `MaybePunishNode()` can get called where it wasn't previously: - when `AcceptBlockHeader()` fails with `CACHED_INVALID`. - when `AcceptBlockHeader()` fails with `BLOCK_MISSING_PREV`. Note that `BlockChecked()` cannot fail with an 'internal' reject code. The only internal reject code was `REJECT_HIGHFEE`, which was only set in ATMP. This reverts a minor bug introduced in 5d08c9c579ba8cc7b684105c6a08263992b08d52. ACKs for top commit: ariard: ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits fjahr: ACK 9075d13153ce06cd59a45644831ecc43126e1e82, confirmed diff to last review was fixing nits in docs/comments. ryanofsky: Code review ACK 9075d13153ce06cd59a45644831ecc43126e1e82. Only changes since last review are splitting the main commit and updating comments Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
2019-10-23Continue relaying transactions after they expire from mapRelayAnthony Towns
2019-10-18Remove transport protocol knowhow from CNetMessage / net processingJonas Schnelli
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-10 11:43:49 +0000