aboutsummaryrefslogtreecommitdiff
path: root/src/net_processing.cpp
AgeCommit message (Collapse)Author
2021-02-11Merge #21062: refactor: return MempoolAcceptResult from ATMPMarcoFalke
53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408) 174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408) f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408) 9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408) Pull request description: This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things: - It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error. - Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param. - We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`. - We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it. ACKs for top commit: MarcoFalke: ACK 53e716ea119658c28935fee24eb50090907c500e 💿 jnewbery: Code review ACK 53e716ea119658c28935fee24eb50090907c500e ariard: Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes. Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f
2021-02-11Merge #21043: net: Avoid UBSan warning in ProcessMessage(...)MarcoFalke
3ddbf22ed179a2db733af4b521bec5d2b13ebf4b util: Disallow negative mocktime (MarcoFalke) f5f2f9716885e7548809e77f46b493c896a019bf net: Avoid UBSan warning in ProcessMessage(...) (practicalswift) Pull request description: Avoid UBSan warning in `ProcessMessage(...)`. Context: https://github.com/bitcoin/bitcoin/pull/20380#issuecomment-770427182 (thanks Crypt-iQ!) ACKs for top commit: MarcoFalke: re-ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b only change is adding patch written by me ajtowns: ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b -- code review only Tree-SHA512: e8d7af0457ca86872b75a4e406c0a93aafd841c2962e244e147e748cc7ca118c56be0fdafe53765f4b291410030b2c3cc8f76f733b37a955d34fc885ab6037b9
2021-02-09[refactor] return MempoolAcceptResultgzhao408
This creates a cleaner interface with ATMP, allows us to make results const, and makes accessing values that don't make sense (e.g. fee when tx is invalid) an error.
2021-02-02doc: refer to BIPs 339/155 in feature negotiationJon Atack
and add fSuccessfullyConnected doxygen documentation to clarify that it is set to true on VERACK
2021-02-02Merge #19509: Per-Peer Message CaptureMarcoFalke
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev) 381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev) e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev) 4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev) f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev) dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev) Pull request description: This PR introduces per-peer message capture into Bitcoin Core. 📓 ## Purpose The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?". ## Functionality When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue. The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so: ``` message_capture/203.0.113.7:56072/msgs_recv.dat message_capture/203.0.113.7:56072/msgs_sent.dat ``` Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option. ## Future Maintenance I sympathize greatly with anyone who says "the best code is no code". The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool. Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small. ## FAQ "Why not just use Wireshark" Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark. Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done. Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable. ACKs for top commit: MarcoFalke: re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚 jnewbery: utACK bff7c66e67aa2f18ef70139338643656a54444fe theStack: re-ACK bff7c66e67aa2f18ef70139338643656a54444fe Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
2021-02-02net: Avoid UBSan warning in ProcessMessage(...)practicalswift
2021-02-01Merge #20749: [Bundle 1/n] Prune g_chainman usage related to ::LookupBlockIndexWladimir J. van der Laan
67c9a83df19c6e2a2edb32336879204e7770b4a7 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong) b8e95658d5909f93dfc7d1e6532661db8919e5b7 style-only: Make TestBlockValidity signature readable (Carl Dong) 0cdad753903640ff4240b715dec9d62f68e51407 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong) ea4fed90219be17160136313c68c06d84176af08 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong) e0dc3057277c9576ddbfb8541599db0149e08bb6 validation: Move LoadExternalBlockFile to CChainState (Carl Dong) 5f8cd7b3a527999512161956db4d718688cb956f validation: Remove global ::ActivateBestChain (Carl Dong) 2a696472a1423e877bfa83f016f66c7e45be7369 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong) 9c300cc8b3ce3d82874982fbf3087e48a6ac0ef2 validation: Pass in chainstate to TestBlockValidity (Carl Dong) 0e17c833cda67cdba5338bd7409061772b6d5edb validation: Make CChainState.m_blockman public (Carl Dong) d363d06bf7d6c3736140672ba8a7f82f4d6fb6ab validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong) f11d11600ddb0ddff6538250ae2a35df6112c3db validation: Move GetLastCheckpoint to BlockManager (Carl Dong) e4b95eefbc700ebc915bec312f77477ce3e87a7e validation: Move GetSpendHeight to BlockManager (Carl Dong) b026e318c39f59a06e29f1b25c7f577e01b25ccb validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong) 3664a150ac7547c9336b571557af223d9e31aac9 validation: Remove global LookupBlockIndex (Carl Dong) eae54e6e60d7ed05b29d8345c0bb055397149ce8 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong) 15d20f40e1321b24963b40c12958c7d30ad112ec validation: Move LookupBlockIndex to BlockManager (Carl Dong) f92dc6557a153b390a1ae1d0808ff7ed5d02c66e validation: Guard the active_chainstate with cs_main (Carl Dong) Pull request description: Overall PR: #20158 (tree-wide: De-globalize ChainstateManager) Note to reviewers: 1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so: 1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only** 2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase** 3. Remove `old_function` ACKs for top commit: jnewbery: utACK 67c9a83df19c6e2a2edb32336879204e7770b4a7 laanwj: re-ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7 ryanofsky: Code review ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7. Changes since last review: Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
2021-01-28validation: Remove global ::ActivateBestChainCarl Dong
Instead use CChainState::ActivateBestChain, which is what the global one calls anyway.
2021-01-28validation: Move FindForkInGlobalIndex to BlockManagerCarl Dong
[META] This commit should be followed up by removing the comments and assertions meant only to show that the change is correct. FindForkInGlobalIndex only acts on BlockManager. Note to reviewers: Since FindForkInGlobalIndex is always called with ::ChainActive() as its first parameter, it is possible to move FindForkInGlobalIndex to CChainState and remove this const CChain& parameter to instead use m_chain. However, it seems like the original intention was for FindForkInGlobalIndex to work with _any_ chain, not just the current active chain. Let me know if this should be changed.
2021-01-28scripted-diff: Use BlockManager::LookupBlockIndexCarl Dong
[META] In a previous commit, we moved ::LookupBlockIndex to become a member function of BlockManager. This commit is split out from that one since it can be expressed nicely as a scripted-diff. -BEGIN VERIFY SCRIPT- find_regex='LookupBlockIndex' \ && git grep -l -E "$find_regex" -- src \ | grep -v '^src/validation\.\(cpp\|h\)$' \ | xargs sed -i -E "s@${find_regex}@g_chainman.m_blockman.LookupBlockIndex@g" -END VERIFY SCRIPT-
2021-01-28net_processing: log txrelay flag from version messageAnthony Towns
2021-01-28net_processing: additional debug logging for ignored messagesAnthony Towns
2021-01-28net, net_processing: log disconnect reasons with -debug=netAnthony Towns
2021-01-23Call CaptureMessage at appropriate locationsTroy Giorshev
These calls are toggled by a debug-only "capturemessages" flag. Default disabled.
2021-01-17Clean PushMessage and ProcessMessagesTroy Giorshev
This brings PushMessage and ProcessMessages further in line with the style guide by fixing their if statements. LogMessage is later called, inside an if statement, inside both of these methods.
2021-01-13Merge #20811: refactor: move net_processing implementation details out of headerWladimir J. van der Laan
c97f70c861ac6959b8116a9bca3031edeb2b2aaa net_processing: move Peer definition to .cpp (Anthony Towns) e0f2e6d2df7117a8dbf17c63c5149fc53a6fe2b2 net_processing: move PeerManagerImpl into cpp file (Anthony Towns) a568b82febb3ecbd5ebb7c3f9da27e762b0c68f6 net_processing: split PeerManager into interface and implementation classes (Anthony Towns) 0df3d3fd6bbbd0e06116797177ba797580553250 net_processing: make more of PeerManager private (Anthony Towns) 0d246a59b606c51728d10cb70004a6eedb951bca net, net_processing: move NetEventsInterface method docs to net.h (Anthony Towns) Pull request description: Moves the implementation details of `PeerManager` and all of `struct Peer` into net_processing.cpp. ACKs for top commit: jnewbery: ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa Sjors: ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa laanwj: Code review ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa vasild: ACK c97f70c861ac6959b8116a9bca3031edeb2b2aaa Tree-SHA512: 2e081e491d981c61bd78436a6a6c2eb41d3c2d86a1a8ef9d4d6f801b82cb64a8bf707a96db3429418d1704cffb60a657d1037a0336cbc8173fb79aef9eb72001
2021-01-09net_processing: move Peer definition to .cppAnthony Towns
2021-01-09net_processing: move PeerManagerImpl into cpp fileAnthony Towns
2021-01-09net_processing: split PeerManager into interface and implementation classesAnthony Towns
2021-01-07net: Remove unused cs_feeFilterMarcoFalke
2021-01-02refactor: Remove nMyStartingHeight from CNode/ConnmanMarcoFalke
2020-12-26[rpc] Remove deprecated "banscore" field from getpeerinfoAmiti Uttarwar
2020-12-25Merge #15451: [doc] clarify getdata limit after #14897MarcoFalke
c119ba3c9b321a7f4418860741b3f69173e9c891 [doc] clarify getdata limit after #14897 (Michael Polzer) Pull request description: GETDATA is limited to blocks and transactions now and can't be used for other non-block data ACKs for top commit: laanwj: ACK c119ba3c9b321a7f4418860741b3f69173e9c891 theStack: ACK https://github.com/bitcoin/bitcoin/pull/15451/commits/c119ba3c9b321a7f4418860741b3f69173e9c891 benthecarman: ACK c119ba3c9b321a7f4418860741b3f69173e9c891 Tree-SHA512: d6e9c109bcce4ef004ec83a9ec591163279476524dec97ed5f5c34e322dca35af66a168f0878ff972bbcec79d81623903f3619fedf8f88cdced3f3f66a779173
2020-12-22[net processing] Remove dropmessagestestJohn Newbery
-dropmessagestest is a command line option that causes 1 in n received messages to be dropped. The Bitcoin P2P protocol is stateful and in general cannot handle messages being dropped. Dropped version/verack/ping/pong messages will cause the connection to time out and be torn down. Other dropped messages may also cause the peer to believe that the peer has stalled and tear down the connection. It seems difficult to uncover any actual issues with -dropmessagestest, and any coverage that could be generated would probably be easier to trigger with fuzz testing.
2020-12-20[net processing] Guard m_continuation_block with m_block_inv_mutexJohn Newbery
2020-12-20[net processing] Move hashContinue to net processingJohn Newbery
Also rename to m_continuation_block to better communicate meaning.
2020-12-20scripted-diff: rename vBlockHashesToAnnounce and vInventoryBlockToSendJohn Newbery
-BEGIN VERIFY SCRIPT- sed -i 's/vBlockHashesToAnnounce/m_blocks_for_headers_relay/g' src/net_processing.* sed -i 's/vInventoryBlockToSend/m_blocks_for_inv_relay/g' src/net_processing.* -END VERIFY SCRIPT-
2020-12-20[net processing] Move block inventory data to PeerJohn Newbery
2020-12-20[net processing] Rename nStartingHeight to m_starting_heightJohn Newbery
Not done as a scripted diff to avoid misnaming the local variable in ProcessMessage().
2020-12-20[net processing] Move nStartingHeight to PeerJohn Newbery
2020-12-19[net processing] Improve documentation for Peer destruction/lockingJohn Newbery
Suggested here: - https://github.com/bitcoin/bitcoin/pull/19607#discussion_r467071878 - https://github.com/bitcoin/bitcoin/pull/19829#discussion_r546116786
2020-12-17Clean up logging of outbound connection typeSuhas Daftuar
2020-12-16doc: Remove shouty enums in net_processing commentsSuhas Daftuar
2020-12-15Only consider addrv2 peers for relay of non-addrv1 addressesPieter Wuille
2020-12-14[net processing] Tolerate sendheaders and sendcmpct messages before verackJohn Newbery
BIP 130 (sendheaders) and BIP 152 (compact blocks) do not specify at which stage the `sendheaders` or `sendcmpct` messages should be sent. Therefore we should tolerate them being sent before the version-verack handshake is complete.
2020-12-14Merge #19763: net: don't try to relay to the address' originatorMarcoFalke
7fabe0f359ae16ed36ce4ca2c33631d038c21448 net: don't relay to the address' originator (Vasil Dimov) Pull request description: For each address to be relayed we "randomly" pick 2 nodes to send the address to (in `RelayAddress()`). However we do not take into consideration that it does not make sense to relay the address back to its originator (`CNode::PushAddress()` will do nothing in that case). This means that if the originator is among the "randomly" picked nodes, then we will relay to one node less than intended. Fix this by skipping the originating node when choosing candidates to relay to. ACKs for top commit: sdaftuar: ACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 (this time I looked at the test, and verified the test breaks in expected ways if I break the code). jnewbery: utACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 (only net_processing changes. I haven't reviewed the test changes) jonatack: re-ACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 per `git range-diff b76abae fd897f8 7fabe0f`, change since last review is rebase and more readable Doxygen documentation Tree-SHA512: c6a9d11c7afc97ab4e8960513f6416648d4a8c0c64b713c145a7482a7b9e54946f81386a3351e3ec0011e5594ba5ccff4d10c6f656bb80680d9f0d0a63366165
2020-12-14Merge #20624: net processing: Remove nStartingHeight check from block relayMarcoFalke
f6360088de8ca02f9d198da2f8cca4ea8d64c992 [net processing] Clarify UpdatedBlockTip() (John Newbery) 94d2cc35be98d3b20db88b2a3745322e5b0aa9d4 [net processing] Remove unnecesary nNewHeight variable in UpdatedBlockTip() (John Newbery) 8b57013473c44fc87c9f1c4611224a89187c289a [net processing] Remove nStartingHeight check from block relay (John Newbery) Pull request description: nStartingHeight was introduced in commit 7a47324c7 (Bitcoin version 0.2.9, P2P version 209) with the comment "better prevention of inventory relaying during initial download". At that time, there was no function to determine whether the node was still in Initial Block Download, so to prevent syncing nodes from relaying old blocks to their peers, a check was added to never relay a block to a peer where the height was lower than 2000 less than the peer's best block. That check was updated several times in later commits to ensure that we weren't relaying blocks before the latest checkpoint if the peer didn't provide a startingheight. The checkpoint comparison was changed to compare with an estimate of the highest block in commit eae82d8e. In commit 202e0194, all block relay was gated on being out of Initial Block Download. In commit 0278fb5f, the comparison to nBlockEstimate was removed since "we already checked IsIBD()". We can remove the check against nStartingHeight entirely. If the node is out of Initial Block Download, then its tip height must have been within 24 hours of current time, so should not be more than ~144 blocks behind the most work tip. This simplifies moving block inventory state into the `Peer` object (#19829). ACKs for top commit: Sjors: utACK f636008 jonatack: ACK f6360088de8ca02f9d198da2f8cca4ea8d64c992 MarcoFalke: ACK f6360088de8ca02f9d198da2f8cca4ea8d64c992 💽 ariard: Code Review ACK f636008 Tree-SHA512: 4959cf35f1dcde46f34bffec1375729a157e1b2a1fd8a8ca33da9771c3c89a6c43e7050cdeeab8d90bb507b0795703db8c8bc304a1a5065ef00aae7a6992ca4f
2020-12-14Merge #20592: doc: update wtxidrelay documentation per BIP339MarcoFalke
4b7b58b3fef5b9566a475871c441ed6ae73af89e Update net_processing WTXID documentation per BIP339 (Jon Atack) Pull request description: BIP339 currently states: *The wtxidrelay message MUST be sent in response to a version message from a peer whose protocol version is >= 70016 and prior to sending a verack. A wtxidrelay message received after a verack message MUST be ignored or treated as invalid.* ACKs for top commit: MarcoFalke: ACK 4b7b58b3fef5b9566a475871c441ed6ae73af89e practicalswift: ACK 4b7b58b3fef5b9566a475871c441ed6ae73af89e RiccardoMasutti: ACK 4b7b58b Tree-SHA512: 58ca6b197618cc73c70aa5de0a2d9d89a68b4cad9d5a708278ef17a9d6854d4362bcc384b6d29696642924977204a8fc120b31e91e2d97b6072b7b0d41c9f2dc
2020-12-14Merge #20617: p2p: Remove m_is_manual_connection from CNodeStatefanquake
a33442fdc73eabd1c5596ab92954344edc9517e6 Remove m_is_manual_connection from CNodeState (Antoine Riard) Pull request description: Currently, this member is only used to exclude MANUAL peers from discouragement in MaybePunishNodeForBlock(). Manual connections are already protected in MaybeDiscourageAndDisconnect(), independently from their network processing behaviors. ACKs for top commit: MarcoFalke: cr ACK a33442fdc73eabd1c5596ab92954344edc9517e6 promag: Code review ACK a33442fdc73eabd1c5596ab92954344edc9517e6. jnewbery: utACK a33442fdc73eabd1c5596ab92954344edc9517e6 amitiuttarwar: code review ACK a33442fdc73eabd1c5596ab92954344edc9517e6 Tree-SHA512: cfe3f3dfa131373e3299002d34ae9e22ca6e1a966831bab32fcf06ff1d08f06095b4ab020cc4d267f3ec05ae23fbdc22373382ab828b999c0db11b8c842a4f0c
2020-12-12Merge #20079: p2p: Treat handshake misbehavior like unknown messageMarcoFalke
faaad1bbac46cfeb22654b4c59f0aac7a680c03a p2p: Ignore version msgs after initial version msg (MarcoFalke) fad68afcff731153d1c83f7f56c91ecbb264b59a p2p: Ignore non-version msgs before version msg (MarcoFalke) Pull request description: Handshake misbehaviour doesn't cost us more than any other unknown message, so it seems odd to treat it differently ACKs for top commit: jnewbery: utACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a practicalswift: ACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a: patch looks correct Tree-SHA512: 9f30c3b5c1f6604fd02cff878f10999956152419a3dd9825f8267cbdeff7d06787418b41c7fde8a00a5e557fe89204546e05d5689042dbf7b07fbb7eb95cddff
2020-12-11[net processing] Clarify UpdatedBlockTip()John Newbery
2020-12-11[net processing] Remove unnecesary nNewHeight variable in UpdatedBlockTip()John Newbery
2020-12-11[net processing] Remove nStartingHeight check from block relayJohn Newbery
nStartingHeight was introduced in commit 7a47324c7 (Bitcoin version 0.2.9, P2P version 209) with the comment "better prevention of inventory relaying during initial download". At that time, there was no function to determine whether the node was still in Initial Block Download, so to prevent syncing nodes from relaying old blocks to their peers, a check was added to never relay a block to a peer where the height was lower than 2000 less than the peer's best block. That check was updated several times in later commits to ensure that we weren't relaying blocks before the latest checkpoint if the peer didn't provide a startingheight. The checkpoint comparison was changed to compare with an estimate of the highest block in commit eae82d8e. In commit 202e0194, all block relay was gated on being out of Initial Block Download. In commit 0278fb5f, the comparison to nBlockEstimate was removed since "we already checked IsIBD()". We can remove the check against nStartingHeight entirely. If the node is out of Initial Block Download, then its tip height must have been within 24 hours of current time, so should not be more than ~144 blocks behind the most work tip.
2020-12-10Remove m_is_manual_connection from CNodeStateAntoine Riard
Currently, this member is only used to exclude MANUAL peers from discouragement in MaybePunishNodeForBlock(). Manual connections are already protected in MaybeDiscourageAndDisconnect(), independently from their network processing behaviors.
2020-12-10Clarify comments around outbound peer evictionSuhas Daftuar
2020-12-10Periodically make block-relay connections and sync headersSuhas Daftuar
To make eclipse attacks more difficult, regularly initiate outbound connections and stay connected long enough to sync headers and potentially learn of new blocks. If we learn a new block, rotate out an existing block-relay peer in favor of the new peer. This augments the existing outbound peer rotation that exists -- currently we make new full-relay connections when our tip is stale, which we disconnect after waiting a small time to see if we learn a new block. As block-relay connections use minimal bandwidth, we can make these connections regularly and not just when our tip is stale. Like feeler connections, these connections are not aggressive; whenever our timer fires (once every 5 minutes on average), we'll try to initiate a new block-relay connection as described, but if we fail to connect we just wait for our timer to fire again before repeating with a new peer.
2020-12-10Use conn_type to identify block-relay peers, rather than m_tx_relay == nullptrSuhas Daftuar
2020-12-10Simplify and clarify extra outbound peer countingSuhas Daftuar
2020-12-10net: don't relay to the address' originatorVasil Dimov
For each address to be relayed we "randomly" pick 2 nodes to send the address to (in `RelayAddress()`). However we do not take into consideration that it does not make sense to relay the address back to its originator (`CNode::PushAddress()` will do nothing in that case). This means that if the originator is among the "randomly" picked nodes, then we will relay to one node less than intended. Fix this by skipping the originating node when choosing candidates to relay to.
2020-12-10Merge #19776: net, rpc: expose high bandwidth mode state via getpeerinfoMarcoFalke
343dc4760fd2407895fc8b3417a504b194429156 test: add test for high-bandwidth mode states in getpeerinfo (Sebastian Falbesoner) dab6583307ceb7dd94affcc3482ddcc1a5747147 doc: release note for new getpeerinfo fields "bip152_hb_{from,to}" (Sebastian Falbesoner) a7ed00f8bbc07dfc09f9e0a5bae10a1afe7612bb rpc: expose high-bandwidth mode states via getpeerinfo (Sebastian Falbesoner) 30bc8fab6833e0447ceadd3fff1566a680e33a98 net: save high-bandwidth mode states in CNodeStats (Sebastian Falbesoner) Pull request description: Fixes #19676, "_For every peer expose through getpeerinfo RPC whether or not we selected them as HB peers, and whether or not they selected us as HB peers._" See [BIP152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki), in particular the [protocol flow diagram](https://github.com/bitcoin/bips/raw/master/bip-0152/protocol-flow.png). The newly introduced states are changed on the following places in the code: * on reception of a `SENDCMPCT` message with valid version, the field `m_highbandwidth_from` is changed depending on the first integer parameter in the message (1=high bandwidth, 0=low bandwidth), i.e. it just mirrors the field `CNodeState.fPreferHeaderAndIDs`. * after adding a `SENDCMPCT` message to the send queue, the field `m_highbandwidth_to` is changed depending on how the first integer parameter is set (same as above) Note that after receiving `VERACK`, the node also sends `SENDCMPCT`, but that is only to announce the preferred version and never selects high-bandwidth mode, hence there is no need to change the state variables there, which are initialized to `false` anyways. ACKs for top commit: naumenkogs: reACK 343dc4760fd2407895fc8b3417a504b194429156 jonatack: re-ACK 343dc4760fd2407895fc8b3417a504b194429156 per `git range-diff 7ea6499 4df1d12 343dc47` Tree-SHA512: f4999e6a935266812c2259a9b5dc459710037d3c9e938006d282557cc225e56128f72965faffb207fc60c6531fab1206db976dd8729a69e8ca29d4835317b99f
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-20 06:31:12 +0000