aboutsummaryrefslogtreecommitdiff
path: root/src/net_processing.cpp
AgeCommit message (Collapse)Author
2019-08-05Merge #16197: net: Use mockable time for tx downloadMarcoFalke
fab365835639a3da03f8ad9a58a0db6c6c4c2314 [qa] Test that getdata requests work as expected (Suhas Daftuar) fa883ab35ad2d4328e35b1e855d0833740a6b910 net: Use mockable time for tx download (MarcoFalke) Pull request description: Two commits: * First commit changes to mockable time for tx download (refactoring, should only have an effect on regtest) * Second commit adds a test that uses mocktime to test tx download ACKs for top commit: laanwj: code review ACK 16197/commits/fab365835639a3da03f8ad9a58a0db6c6c4c2314 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/16197/commits/fab365835639a3da03f8ad9a58a0db6c6c4c2314 Tree-SHA512: 3a64a3e283ec4bab1f6e506404b11f0a564a5b61d2a7508ae738a61f035e57220484c66e0ae47d847fe9f7e3ff5cc834909d7b34a9bbcea6abe01f8742806908
2019-07-24refactor : use RelayTransaction in BroadcastTransaction utilityAntoine Riard
To do so, we also refactor RelayTransaction to take a txid instead of passing a tx
2019-06-25Merge #16188: net: Document what happens to getdata of unknown typefanquake
dddd9270f85bd2e71fd281a0c6b4053e02fce93c net: Document what happens to getdata of unknonw type (MarcoFalke) Pull request description: Any getdata of unknown type will never be processed and blocks all future messages from a peer. This isn't obviously clear from reading the code, so document it. Top commit has no ACKs. Tree-SHA512: 4f8e43bbe6534242facfcfffae28b7a6aa2d228841fa2146a87d494e69f614b0da23cf7a5f3d4367358a7c1981fe2ec196a21c437ae1653f1c7e0351be22598a
2019-06-20net: Document what happens to getdata of unknonw typeMarcoFalke
2019-06-18Merge #16092: Don't use global (external) symbols for symbols that are used ↵MarcoFalke
in only one translation unit 0959d37e3e Don't use global (external) symbols for symbols that are used in only one translation unit (practicalswift) Pull request description: Don't use global (external) symbols for symbols that are used in only one translation unit. Before: ``` $ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h") N_REFERENCES=$(wc -l <<< "${REFERENCES}") if [[ ${N_REFERENCES} > 1 ]]; then continue fi echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}" done Global symbol g_chainstate is used in only one translation unit: src/validation.cpp Global symbol g_ui_signals is used in only one translation unit: src/ui_interface.cpp Global symbol instance_of_cmaincleanup is used in only one translation unit: src/validation.cpp Global symbol instance_of_cnetcleanup is used in only one translation unit: src/net.cpp Global symbol instance_of_cnetprocessingcleanup is used in only one translation unit: src/net_processing.cpp Global symbol pindexBestForkBase is used in only one translation unit: src/validation.cpp Global symbol pindexBestForkTip is used in only one translation unit: src/validation.cpp $ ``` After: ``` $ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h") N_REFERENCES=$(wc -l <<< "${REFERENCES}") if [[ ${N_REFERENCES} > 1 ]]; then continue fi echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}" done $ ``` ♻️ Think about future generations: save the global namespace from unnecessary pollution! ♻️ ACKs for commit 0959d3: Empact: ACK https://github.com/bitcoin/bitcoin/pull/16092/commits/0959d37e3e0f80010a78d175e3846dabf5d35919 MarcoFalke: ACK 0959d37e3e0f80010a78d175e3846dabf5d35919 hebasto: ACK 0959d37e3e0f80010a78d175e3846dabf5d35919 promag: ACK 0959d37. Tree-SHA512: 722f66bb50450f19b57e8a8fbe949f30cd651eb8564e5787cbb772a539bf3a288c048dc49e655fd73ece6a46f6dafade515ec4004729bf2b3ab83117b7c5d153
2019-06-17net: Use mockable time for tx downloadMarcoFalke
2019-06-12Merge #15834: Fix transaction relay bugs introduced in #14897 and expire ↵MarcoFalke
transactions from peer in-flight map 308b76732f Fix bug around transaction requests (Suhas Daftuar) f635a3ba11 Expire old entries from the in-flight tx map (Suhas Daftuar) e32e08407e Remove NOTFOUND transactions from in-flight data structures (Suhas Daftuar) 23163b7593 Add an explicit memory bound to m_tx_process_time (Suhas Daftuar) 218697b645 Improve NOTFOUND comment (Suhas Daftuar) Pull request description: #14897 introduced several bugs that could lead to a node no longer requesting transactions from one or more of its peers. Credit to ajtowns for originally reporting many of these bugs along with an originally proposed fix in #15776. This PR does a few things: - Fix a bug in NOTFOUND processing, where the in-flight map for a peer was keeping transactions it shouldn't - Eliminate the possibility of a memory attack on the CNodeState `m_tx_process_time` data structure by explicitly bounding its size - Remove entries from a peer's in-flight map after 10 minutes, so that we should always eventually resume transaction requests even if there are other bugs like the NOTFOUND one - Fix a bug relating to the coordination of request times when multiple peers announce the same transaction The expiry mechanism added here is something we'll likely want to remove in the future, but is belt-and-suspenders for now to try to ensure we don't have other bugs that could lead to transaction relay failing due to some unforeseen conditions. ACKs for commit 308b76: ajtowns: utACK 308b76732f97020c86977e29c854e8e27262cf7c morcos: light ACK 308b767 laanwj: Code review ACK 308b76732f97020c86977e29c854e8e27262cf7c jonatack: Light ACK 308b76732f97020c86977e29c854e8e27262cf7c. jamesob: ACK 308b76732f MarcoFalke: ACK 308b76732f97020c86977e29c854e8e27262cf7c (Tested two of the three bugs this pull fixes, see comment above) jamesob: Concept ACK https://github.com/bitcoin/bitcoin/pull/15834/commits/308b76732f97020c86977e29c854e8e27262cf7c MarcoFalke: ACK 308b76732f Tree-SHA512: 8865dca5294447859d95655e8699085643db60c22f0719e76e961651a1398251bc932494b68932e33f68d4f6084579ab3bed7d0e7dd4ac6c362590eaf9414eda
2019-06-06Merge #16129: refactor: Remove unused includesMarcoFalke
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift) eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift) Pull request description: Make reasoning about dependencies easier by not including unused dependencies. Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real. As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed: ``` $ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \ sed 's%^%src/%g' | xargs cat | wc -l 51393 ``` Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-) ACKs for commit 67f4e9: Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
2019-06-06Don't use global (external) symbols for symbols that are used in only one ↵practicalswift
translation unit
2019-06-05Merge #15976: refactor: move methods under CChainState (pt. 1)Wladimir J. van der Laan
403e677c9 refactoring: IsInitialBlockDownload -> CChainState (James O'Beirne) 3ccbc376d refactoring: FlushStateToDisk -> CChainState (James O'Beirne) 4d6688603 refactoring: introduce ChainstateActive() (James O'Beirne) d7c97edee move-only: make the CChainState interface public (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal --- This changeset starts moving functionality intimately related to CChainState into methods. Parameterizing these functions by a particular CChainState is necessary for the use of multiple chainstates simultaneously (e.g. for asynchronous background validation). In this change, we - make the CChainState interface public - since other units will start to invoke its methods directly, - introduce `::ChainstateActive()`, the CChainState equivalent for `::ChainActive()`, - and move `IsInitialBlockDownload()` and `FlushStateToDisk()` into methods on CChainState. Independent of assumeutxo, these changes better encapsulate chainstate behavior and allow easier use from a testing context. There are more methods that we'll move in the future, but they require other substantial changes (i.e. moving ownership of the `CCoinsView*` hierarchy into CChainState) so we'll save them for future PRs. --- The first move-only commit is most easily reviewed with `git diff ... --color-moved=dimmed_zebra`. ACKs for commit 403e67: Empact: utACK https://github.com/bitcoin/bitcoin/pull/15976/commits/403e677c9ebbf9744733010e6b0c2d1b182ee850 no need to address my nits herein Sjors: utACK 403e677 ryanofsky: utACK 403e677c9ebbf9744733010e6b0c2d1b182ee850. Only change since previous review is removing global state comment as suggested. MarcoFalke: utACK 403e677c9e, though the diff still seems a bit bloated with some unnecessary changes in the second commit. promag: utACK 403e677 and rebased with current [master](c7cfd20a7). Tree-SHA512: 6fcf260bb2dc201361170c0b4547405366f5f331fcc3a2bac29b24442814b7b244ca1b58aac5af716885f9a130c343b544590dff780da0bf835c7c5b3ccb2257
2019-06-02Make reasoning about dependencies easier by not including unused dependenciespracticalswift
2019-05-28Fix bug around transaction requestsSuhas Daftuar
If a transaction is already in-flight when a peer announces a new tx to us, we schedule a time in the future to reconsider whether to download. At that future time, there was a bug that would prevent transactions from being rescheduled for potential download again (ie if the transaction was still in-flight at the time of reconsideration, such as from some other peer). Fix this.
2019-05-28Expire old entries from the in-flight tx mapSuhas Daftuar
If a peer hasn't responded to a getdata request, eventually time out the request and remove it from the in-flight data structures. This is to prevent any bugs in our handling of those in-flight data structures from filling up the in-flight map and preventing us from requesting more transactions (such as the NOTFOUND bug, fixed in a previous commit). Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-05-20Merge #16021: p2p: Avoid logging transaction decode errors to stderrWladimir J. van der Laan
fa2b52af32f6a4b9c22c270f36e92960c29ef364 Disallow extended encoding for non-witness transactions (take 3) (MarcoFalke) Pull request description: (previous title "p2p: Disallow extended encoding for non-witness transactions (take 3)") Remote peers can send us illegally encoded txs and thus have us write to stderr. Fix that by not writing to stderr. This is a follow up to the previous (incomplete) attempts at this: * Disallow extended encoding for non-witness transactions #14039 * Add test for superfluous witness record in deserialization #15893 ACKs for commit fa2b52: laanwj: utACK fa2b52af32f6a4b9c22c270f36e92960c29ef364 ryanofsky: utACK fa2b52af32f6a4b9c22c270f36e92960c29ef364. Would change title to something like "Avoid logging transaction decode errors to stderr" instead of "Disallow extended encoding for non-witness transactions." The current title is confusing because this PR isn't really allowing or disallowing anything, just logging the condition differently. "Disallow" also seems to contradict the "Allow exceptions from..." comments in the actual code. Tree-SHA512: c66990e69b432d00dc1c5510bf976a1188664d0890a32d1e5c6459094e7e27da82a5d227627afcbc203676f5540eec74b7d9b1d71d2c62d3b2069e1781824b4d
2019-05-16refactoring: IsInitialBlockDownload -> CChainStateJames O'Beirne
We introduce CChainState.m_cached_finished_ibd because the static state it replaces would've been shared across all CChainState instances.
2019-05-14Disallow extended encoding for non-witness transactions (take 3)MarcoFalke
2019-05-09net: Rename ::fRelayTxes to ::g_relay_txesMarcoFalke
This helps to distinguish it from CNode::fRelayTxes and avoid bugs like 425278d17bd0edf8a3a7cc81e55016f7fd8e7726
2019-05-07Merge #15948: refactor: rename chainActiveMarcoFalke
486c1eea86 refactoring: remove unused chainActive (James O'Beirne) 631940aab2 scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne) a3a609079c refactoring: introduce unused ChainActive() (James O'Beirne) 1b6e6fcfd2 rename: CChainState.chainActive -> m_chain (James O'Beirne) Pull request description: This is part of the assumeutxo project: Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal --- This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data. The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot. This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review. ACKs for commit 486c1e: Sjors: utACK 486c1ee promag: utACK 486c1ee. practicalswift: utACK 486c1eea863a41e597ae4fddc392f446f2518b4b Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a
2019-05-03scripted-diff: replace chainActive -> ::ChainActive()James O'Beirne
Though at the moment ChainActive() simply references `g_chainstate.m_chain`, doing this change now clears the way for multiple chainstate usage and allows us to script the diff. -BEGIN VERIFY SCRIPT- git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g' -END VERIFY SCRIPT-
2019-05-02Separate reason for premature spends (coinbase/locktime)Suhas Daftuar
2019-05-02Assert validation reasons are contextually correctSuhas Daftuar
2019-05-02Fix handling of invalid headersSuhas Daftuar
We only disconnect outbound peers (excluding HB compact block peers and manual connections) when receiving a CACHED_INVALID header.
2019-05-02[refactor] Use Reasons directly instead of DoS codesMatt Corallo
2019-05-02CorruptionPossible -> TX_WITNESS_MUTATEDMatt Corallo
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-05-02LookupBlockIndex -> CACHED_INVALIDMatt Corallo
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-05-02[refactor] Drop redundant nDoS, corruptionPossible, SetCorruptionPossibleMatt Corallo
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2019-05-02[refactor] Add useful-for-dos "reason" field to CValidationStateMatt Corallo
This is a first step towards cleaning up our DoS interface - make validation return *why* something is invalid, and let net_processing figure out what that implies in terms of banning/disconnection/etc. Behavior change: peers will now be banned for providing blocks with premature coinbase spends. Co-authored-by: Anthony Towns <aj@erisian.com.au> Suhas Daftuar <sdaftuar@gmail.com>
2019-05-02[refactor] Refactor misbehavior ban decisions to MaybePunishNode()Matt Corallo
Isolate the decision of whether to ban a peer to one place in the code, rather than having it sprinkled throughout net_processing. Co-authored-by: Anthony Towns <aj@erisian.com.au> Suhas Daftuar <sdaftuar@gmail.com> John Newbery <john@johnnewbery.com>
2019-05-02[refactor] rename stateDummy -> orphan_stateMatt Corallo
Co-authored-by: Anthony Towns <aj@erisian.com.au> Suhas Daftuar <sdaftuar@gmail.com>
2019-04-26Remove NOTFOUND transactions from in-flight data structuresSuhas Daftuar
This prevents a bug where the in-flight queue for our peers will not be drained, resulting in not downloading any new transactions from our peers. Thanks to ajtowns for reporting this bug.
2019-04-26Add an explicit memory bound to m_tx_process_timeSuhas Daftuar
Previously there was an implicit bound based on the handling of m_tx_announced, but that approach is error-prone (particularly if we start automatically removing things from that set).
2019-04-26Improve NOTFOUND commentSuhas Daftuar
2019-04-09[build] Add several util unitsJohn Newbery
Adds the following util units and adds them to libbitcoin_util: - `util/url.cpp` takes `urlDecode` from `httpserver.cpp` - `util/error.cpp` takes `TransactionErrorString` from `node/transaction.cpp` and `AmountHighWarn` and `AmountErrMsg` from `ui_interface.cpp` - `util/fees.cpp` takes `StringForFeeReason` and `FeeModeFromString` from `policy/fees.cpp` - `util/rbf.cpp` takes `SignalsOptInRBF` from `policy/rbf.cpp` - 'util/validation.cpp` takes `FormatStateMessage` and `strMessageMagic` from 'validation.cpp`
2019-04-09[wallet] Schedule tx rebroadcasts in walletJohn Newbery
Removes the now-unused Broadcast/ResendWalletTransactions interface from validationinterface. The wallet_resendwallettransactions.py needs a sleep added at the start to make sure that the rebroadcast scheduler is warmed up before the next block is mined.
2019-04-09[wallet] Keep track of the best block time in the walletJohn Newbery
Move nTimeBestReceived (which is only used for wallet rebroadcasts) into the wallet.
2019-04-04Merge #15654: net: Remove unused unsanitized user agent string CNode::strSubVerMarcoFalke
fa8548c5d1 net: Remove unused unsanitized user agent string CNode::strSubVer (MarcoFalke) Pull request description: I fail to see a use case for this unsanitized byte array. In fact this can easily be confused with `cleanSubVer` and be displayed to the user (or logged) by a simple typo that is hard to find in review. Further reading: https://btcinformation.org/en/developer-reference#version ACKs for commit fa8548: promag: utACK fa8548c, good catch. practicalswift: utACK fa8548c5d13957f57f9b1e20e03002600962f7f0 sipa: utACK fa8548c5d13957f57f9b1e20e03002600962f7f0 Tree-SHA512: 3c3ff1504d1583ad099df9a6aa761458a82ec48a58ef7aaa9b5679a5281dd1b59036ba2932ed708488951a565b669a3083ef70be5a58472ff8677b971162ae2f
2019-03-23net: Remove unused unsanitized user agent string CNode::strSubVerMarcoFalke
2019-03-22Interrupt orphan processing after every transactionPieter Wuille
This makes orphan processing work like handling getdata messages: After every actual transaction validation attempt, interrupt processing to deal with messages arriving from other peers.
2019-03-22[MOVEONLY] Move processing of orphan queue to ProcessOrphanTxPieter Wuille
2019-03-22Simplify orphan processing in preparation for interruptibilityPieter Wuille
2019-03-20Merge #15597: net: Generate log entry when blocks messages are received ↵Wladimir J. van der Laan
unexpectedly ef0019e054734a14214dfbce56611ce4db1688a5 Generate log entry when blocks messages are received unexpectedly. (Patrick Strateman) Pull request description: Currently these are incorrectly logged as an unknown command. Tree-SHA512: dd272388a90b79897f8c1ea6d4c949323fcf75493f3a5b2ec9a26a2cf6a8ee743b497941702f21df8fae0f5b9481444363643379832dbd5053b0cc0b0363de04
2019-03-17Do not relay banned IP addressesPieter Wuille
2019-03-13Generate log entry when blocks messages are received unexpectedly.Patrick Strateman
2019-02-14Merge #14626: Select orphan transaction uniformly for evictionMarcoFalke
7257353b93 Select orphan transaction uniformly for eviction (Pieter Wuille) Pull request description: The previous code was biased towards evicting transactions whose txid has a larger gap (lexicographically) with the previous txid in the orphan pool. Tree-SHA512: e35f700aea5ed79d1bc57f64bffcb623424b40156fd0a12f05f74f981a8aa4175d5c18d042989243f7559242bdf1d6d720bcf588d28f43d74a798a4843f09c70
2019-02-06Change in transaction pull scheduling to prevent InvBlock-related attacksGleb Naumenko
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
2019-01-16net: move BanMan to its own filesCory Fields
2019-01-16banman: create and split out banmanCory Fields
Some say he has always been.
2019-01-16net: Break disconnecting out of Ban()Cory Fields
These are separate events which need to be carried out by separate subsystems. This also cleans up some whitespace and tabs in qt to avoid getting flagged by the linter. Current behavior is preserved.
2018-12-13Select orphan transaction uniformly for evictionPieter Wuille
The previous code was biased towards evicting transactions whose txid has a larger gap (lexicographically) with the previous txid in the orphan pool.
2018-12-12Use a FastRandomContext in LimitOrphanTxSizePieter Wuille
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-13 02:49:45 +0000