aboutsummaryrefslogtreecommitdiff
path: root/src/net_processing.cpp
AgeCommit message (Collapse)Author
2023-05-11net_processing: Boost inv trickle rateAnthony Towns
If transactions are being added to the mempool at a rate faster than 7tx/s (INVENTORY_BROADCAST_PER_SECOND) then peers' inventory_to_send queue can become relatively large. If this happens, increase the number of txids we include in an INV message (normally capped at 35) by 5 for each 1000 txids in the queue. This will tend to clear a temporary excess out reasonably quickly; an excess of 4000 invs to send will be cleared down to 1000 in about 30 minutes, while an excess of 20000 invs would be cleared down to 1000 in about 60 minutes. Github-Pull: #27610 Rebased-From: 5b3406094f2679dfb3763de4414257268565b943
2023-05-11p2p: Avoid prematurely clearing download state for other peersSuhas Daftuar
Github-Pull: #27608 Rebased-From: 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4
2022-12-02[net processing] Assume that TxRelay::m_tx_inventory_to_send is empty pre-verackdergoegge
This commit documents our assumption about TxRelay::m_tx_inventory_to_send being empty prior to version handshake completion. The added Assume acts as testing oracle for our fuzzing tests to potentially detect if the assumption is violated. Github-Pull: #26569 Rebased-From: ce63fca13e9b500e9f687d80a457175ac967a371
2022-12-02[net processing] Ensure transaction announcements are only queued for fully ↵dergoegge
connected peers Github-Pull: #26569 Rebased-From: 845e3a34c49abcc634b5a10ccdd6b10fb4fcf449
2022-10-24[net processing] Handle IsContinuationOfLowWorkHeadersSync return value ↵dergoegge
correctly when new headers sync is started
2022-10-11p2p: ProcessHeadersMessage(): fix received_new_headerLarry Ruane
Follow-up to #25717. The commit "Utilize anti-DoS headers download strategy" changed how this bool variable is computed, so that its value is now the opposite of what it should be. GitHub-Pull: #26172 Rebased-From: bdcafb913398f0cdaff9c880618f9ebfc85c7693
2022-08-30Bypass headers anti-DoS checks for NoBan peersSuhas Daftuar
2022-08-30Merge bitcoin/bitcoin#25717: p2p: Implement anti-DoS headers syncfanquake
3add23454624c4c79c9eebc060b6fbed4e3131a7 ui: show header pre-synchronization progress (Pieter Wuille) 738421c50f2dbd7395b50a5dbdf6168b07435e62 Emit NotifyHeaderTip signals for pre-synchronization progress (Pieter Wuille) 376086fc5a187f5b2ab3a0d1202ed4e6c22bdb50 Make validation interface capable of signalling header presync (Pieter Wuille) 93eae27031a65b4156df49015ae45b2b541b4e5a Test large reorgs with headerssync logic (Suhas Daftuar) 355547334f7d08640ee1fa291227356d61145d1a Track headers presync progress and log it (Pieter Wuille) 03712dddfbb9fe0dc7a2ead53c65106189f5c803 Expose HeadersSyncState::m_current_height in getpeerinfo() (Suhas Daftuar) 150a5486db50ff77c91765392149000029c8a309 Test headers sync using minchainwork threshold (Suhas Daftuar) 0b6aa826b53470c9cc8ef4a153fa710dce80882f Add unit test for HeadersSyncState (Suhas Daftuar) 83c6a0c5249c4ecbd11f7828c84a50fb473faba3 Reduce spurious messages during headers sync (Suhas Daftuar) ed6cddd98e32263fc116a4380af6d66da20da990 Require callers of AcceptBlockHeader() to perform anti-dos checks (Suhas Daftuar) 551a8d957c4c44afbd0d608fcdf7c6a4352babce Utilize anti-DoS headers download strategy (Suhas Daftuar) ed470940cddbeb40425960d51cefeec4948febe4 Add functions to construct locators without CChain (Pieter Wuille) 84852bb6bb3579e475ce78fe729fd125ddbc715f Add bitdeque, an std::deque<bool> analogue that does bit packing. (Pieter Wuille) 1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51 Add function to validate difficulty changes (Suhas Daftuar) Pull request description: New nodes starting up for the first time lack protection against DoS from low-difficulty headers. While checkpoints serve as our protection against headers that fork from the main chain below the known checkpointed values, this protection only applies to nodes that have been able to download the honest chain to the checkpointed heights. We can protect all nodes from DoS from low-difficulty headers by adopting a different strategy: before we commit to storing a header in permanent storage, first verify that the header is part of a chain that has sufficiently high work (either `nMinimumChainWork`, or something comparable to our tip). This means that we will download headers from a given peer twice: once to verify the work on the chain, and a second time when permanently storing the headers. The p2p protocol doesn't provide an easy way for us to ensure that we receive the same headers during the second download of peer's headers chain. To ensure that a peer doesn't (say) give us the main chain in phase 1 to trick us into permanently storing an alternate, low-work chain in phase 2, we store commitments to the headers during our first download, which we validate in the second download. Some parameters must be chosen for commitment size/frequency in phase 1, and validation of commitments in phase 2. In this PR, those parameters are chosen to both (a) minimize the per-peer memory usage that an attacker could utilize, and (b) bound the expected amount of permanent memory that an attacker could get us to use to be well-below the memory growth that we'd get from the honest chain (where we expect 1 new block header every 10 minutes). After this PR, we should be able to remove checkpoints from our code, which is a nice philosophical change for us to make as well, as there has been confusion over the years about the role checkpoints play in Bitcoin's consensus algorithm. Thanks to Pieter Wuille for collaborating on this design. ACKs for top commit: Sjors: re-tACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 mzumsande: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 sipa: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 glozow: ACK 3add234546 Tree-SHA512: e7789d65f62f72141b8899eb4a2fb3d0621278394d2d7adaa004675250118f89a4e4cb42777fe56649d744ec445ad95141e10f6def65f0a58b7b35b2e654a875
2022-08-29net/net_processing: add missing thread safety annotationsAnthony Towns
2022-08-29Track headers presync progress and log itPieter Wuille
2022-08-29Expose HeadersSyncState::m_current_height in getpeerinfo()Suhas Daftuar
2022-08-29Reduce spurious messages during headers syncSuhas Daftuar
Delay sending SENDHEADERS (BIP 130) message until we know our peer's best header's chain has more than nMinimumChainWork. This reduces inadvertent headers messages received during initial headers sync due to block announcements, which throw off our sync algorithm.
2022-08-29Require callers of AcceptBlockHeader() to perform anti-dos checksSuhas Daftuar
In order to prevent memory DoS, we must ensure that we don't accept a new header into memory until we've performed anti-DoS checks, such as verifying that the header is part of a sufficiently high work chain. This commit adds a new argument to AcceptBlockHeader() so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation. This patch also fixes two places where low-difficulty-headers could have been processed without such validation (processing an unrequested block from the network, and processing a compact block). Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost for test code.
2022-08-29Utilize anti-DoS headers download strategySuhas Daftuar
Avoid permanently storing headers from a peer, unless the headers are part of a chain with sufficiently high work. This prevents memory attacks using low-work headers. Designed and co-authored with Pieter Wuille.
2022-08-23Add functions to construct locators without CChainPieter Wuille
This introduces an insignificant performance penalty, as it means locator construction needs to use the skiplist-based CBlockIndex::GetAncestor() function instead of the lookup-based CChain, but avoids the need for callers to have access to a relevant CChain object.
2022-08-22Merge bitcoin/bitcoin#25786: refactor: Make adjusted time type safefanquake
eeee5ada23f2a71d245671556b6ecfdaabfeddf4 Make adjusted time type safe (MacroFake) fa3be799fe951a7ea9b4de78d5a907c6db71eeb8 Add time helpers (MacroFake) Pull request description: This makes follow-ups easier to review. Also, it makes sense by itself. ACKs for top commit: ryanofsky: Code review ACK eeee5ada23f2a71d245671556b6ecfdaabfeddf4. Confirmed type changes and equivalent code changes only. Tree-SHA512: 51bf1ae5428552177286113babdd49e82459d6c710a07b6e80a0a045d373cf51045ee010461aba98e0151d8d71b9b3b5f8f73e302d46ba4558e0b55201f99e9f
2022-08-16refactor: Add lock annotations to Active* methodsMacroFake
This is a refactor, putting the burden to think about thread safety to the caller. Otherwise, there is a risk that the caller will assume thread safety where none exists, as is evident in the previous two commits.
2022-08-15Merge bitcoin/bitcoin#25720: p2p: Reduce bandwidth during initial headers ↵Andrew Chow
sync when a block is found f6a916683d75ed5489666dbfbd711f000ad0707f Add functional test for block announcements during initial headers sync (Suhas Daftuar) 05f7f31598b8bb06acb12e1e2a3ccf324b035ea8 Reduce bandwidth during initial headers sync when a block is found (Suhas Daftuar) Pull request description: On startup, if our headers chain is more than a day behind current time, we'll pick one peer to sync headers with until our best headers chain is caught up (at that point, we'll try to sync headers with all peers). However, if an INV for a block is received before our headers chain is caught up, we'll then start to sync headers from each peer announcing the block. This can result in doing a big headers sync with many (if not all) of our peers simultaneously, which wastes bandwidth. This PR would reduce that overhead by picking (at most) one new peer to try syncing headers with whenever a new block is announced, prior to our headers chain being caught up. ACKs for top commit: LarryRuane: ACK f6a916683d75ed5489666dbfbd711f000ad0707f ajtowns: ACK f6a916683d75ed5489666dbfbd711f000ad0707f mzumsande: ACK f6a916683d75ed5489666dbfbd711f000ad0707f dergoegge: Code review ACK f6a916683d75ed5489666dbfbd711f000ad0707f achow101: ACK f6a916683d75ed5489666dbfbd711f000ad0707f Tree-SHA512: 0662000bd68db146f55981de4adc2e2b07cbfda222b1176569d61c22055e5556752ffd648426f69687ed1cc203105515e7304c12b915d6270df8e41a4a0e1eaa
2022-08-12Reduce bandwidth during initial headers sync when a block is foundSuhas Daftuar
If our headers chain is behind on startup, then if a block is found we'll try to catch up from all peers announcing the block, in addition to our initial headers-sync peer. This commit changes behavior so that in this situation, we'll choose at most one peer announcing a block to additionally sync headers from.
2022-08-05Make adjusted time type safeMacroFake
2022-08-05Merge bitcoin/bitcoin#24662: addrman: Use system time instead of adjusted ↵fanquake
network time fadd8b2676f6d68ec87189871461c9a6a6aa3cac addrman: Use system time instead of adjusted network time (MarcoFalke) Pull request description: This changes addrman to use system time for address relay instead of the network adjusted time. This is an improvement, because network time has multiple issues: * It is non-monotonic, even if the system time is monotonic. * It may be wrong, even if the system time is correct. * It may be wrong, if the system time is wrong. For example, when the node has limited number of connections (`4`), or the system time is wrong by too much (more than +-70 minutes), or the system time only got wrong after timedata collected more than half of the entries while the time was correct, ...) This may slightly degrade addr relay for nodes where timedata successfully adjusted the time. Addr relay can already deal with minor offsets of up to 10 minutes. Offsets larger than this should still allow addr relay and not result in a DoS. ACKs for top commit: dergoegge: Code review ACK fadd8b2676f6d68ec87189871461c9a6a6aa3cac Tree-SHA512: b6c178fa01161544e5bc76c4cb23e11bcc30391f7b7a64accce864923766647bcfce2e8ae21d36fb1ffc1afa07bc46415aca612405bd8d4cc1f319c92a08498f
2022-08-02Remove ::incrementalRelayFee and ::minRelayTxFee globalsMacroFake
2022-07-30addrman: Use system time instead of adjusted network timeMarcoFalke
2022-07-29Merge bitcoin/bitcoin#25683: refactor: log `nEvicted` message in ↵MacroFake
`LimitOrphans` then return void b4b657ba57a2ce31b3c21ea9245aad26d5b06a57 refactor: log `nEvicted` message in `LimitOrphans` then return void (chinggg) Pull request description: Fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49347 LimitOrphans() can log expired tx and it should log evicted tx as well instead of returning the `nEvicted` number for caller to print the message. Since `LimitOrphans()` now returns void, the redundant assertion check in fuzz test is also removed. Top commit has no ACKs. Tree-SHA512: 18c41702321b0e59812590cd389f3163831d431f4ebdc3b3e1e0698496a6bdbac52288f28f779237a58813c6717da1a35e8933d509822978ff726c1b13cfc778
2022-07-28refactor: log `nEvicted` message in `LimitOrphans` then return voidchinggg
`LimitOrphans()` can log expired tx and it should log evicted tx as well instead of returning the number for caller to print the message. Since `LimitOrphans()` now return void, the redundant assertion check in fuzz test is also removed.
2022-07-26refactor: Use type-safe std::chrono for addrman timeMarcoFalke
2022-07-26Add type-safe AdjustedTime() getter to timedataMarcoFalke
Also, fix includes. The getter will be used in a future commit.
2022-07-19Merge bitcoin/bitcoin#25514: net processing: Move CNode::nServices and ↵MacroFake
CNode::nLocalServices to Peer 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67 [net processing] Remove CNode::nLocalServices (John Newbery) 5961f8eea1ad5be1a4bf8da63651e197a20359b2 [net] Return CService from GetLocalAddrForPeer and GetLocalAddress (dergoegge) d9079fe18dc5d81ce290876353555b51125127d1 [net processing] Remove CNode::nServices (John Newbery) 7d1c0369340cb752f0d78e24f4251595534bf5e9 [net processing] Replace fHaveWitness with CanServeWitnesses() (John Newbery) f65e83d51bfb6a34f1d5efccfb3d8786a51a4534 [net processing] Remove fClient and m_limited_node (John Newbery) fc5eb528f7d7b33e2f2e443c5610a1551c7f099b [tests] Connect peer in outbound_slow_chain_eviction by sending p2p messages (John Newbery) 1f52c47d5c09b59fd3153700751c74e63edc7d7e [net processing] Add m_our_services and m_their_services to Peer (John Newbery) Pull request description: Another step in #19398. Which services we offer to a peer and which services they offer to us is application layer data and should not be stored on `CNode`. This is also a prerequisite for adding `PeerManager` unit tests (See #25515). ACKs for top commit: MarcoFalke: ACK 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67 🔑 jnewbery: utACK 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67 mzumsande: Code Review ACK 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67 Tree-SHA512: e772eb2a0a85db346dd7b453a41011a12756fc7cbfda6a9ef6daa9633b9a47b9770ab3dc02377690f9d02127301c3905ff22905977f758bf90b17a9a35b37523
2022-07-14[net processing] Remove CNode::nLocalServicesJohn Newbery
2022-07-14[net] Return CService from GetLocalAddrForPeer and GetLocalAddressdergoegge
2022-07-14[net processing] Remove CNode::nServicesJohn Newbery
Use Peer::m_their_services instead
2022-07-14[net processing] Replace fHaveWitness with CanServeWitnesses()John Newbery
2022-07-14[net processing] Remove fClient and m_limited_nodeJohn Newbery
fClient is replaced by CanServeBlocks(), and m_limited_node is replaced by IsLimitedPeer().
2022-07-14[net processing] Add m_our_services and m_their_services to PeerJohn Newbery
Track services offered by us and the peer in the Peer object.
2022-07-12Remove atomic for m_last_getheaders_timestampSuhas Daftuar
This variable is only used in a single thread, so no atomic or mutex is necessary to guard it.
2022-07-04Merge bitcoin/bitcoin#25454: p2p: Avoid multiple getheaders messages in ↵fanquake
flight to the same peer 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 Replace GetTime() with NodeClock in MaybeSendGetHeaders() (Suhas Daftuar) abf5d16c24cb08b0451bdbd4d1de63a12930e8f5 Don't send getheaders message when another request is outstanding (Suhas Daftuar) ffe87db247b19ffb8bfba329c5dd0be39ef5a53f Cleanup received_new_header calculation to use WITH_LOCK (Suhas Daftuar) 6d95cd3e7444ebaaabb64a76783ea3551530f1d7 Move peer state updates from headers message into separate function (Suhas Daftuar) 2b341db731793844f12944363186edea23eabdeb Move headers direct fetch to end of ProcessHeadersMessage (Suhas Daftuar) 29c45185223441943ab610e62937a118c7c3a5b2 Move headers-direct-fetch logic into own function (Suhas Daftuar) bf8ea6df75749c27f753b562c4724b3f8d263ad4 Move additional headers fetching to own function (Suhas Daftuar) 9492e93bf9f4a841bf43ca4b593871c0863d5b63 Add helper function for checking header continuity (Suhas Daftuar) 7f2450871b3ea0b4d02d56bd2ca365fcc25cf90e Move handling of unconnecting headers into own function (Suhas Daftuar) Pull request description: Change `getheaders` messages so that we wait up to 2 minutes for a response to a prior `getheaders` message before issuing a new one. Also change the handling of the `getheaders` message sent in response to a block INV, so that we no longer use the hashstop variable (including the hash stop will just mean that if our peer's headers chain is longer, then we won't learn it, so there's no benefit to using hashstop). Also, now respond to a `getheaders` during IBD with an empty headers message (rather than nothing) -- this better conforms to the intent of the new logic that it's better to not ignore a peer's `getheaders` message, even if you have nothing to give. This also avoids a lot of functional tests breaking. This PR also reworks the headers processing logic to make it more readable. ACKs for top commit: ajtowns: ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 ; code review, check over new logic of when to send getheaders messages dergoegge: Code review ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 mzumsande: Code Review ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 sipa: utACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 w0xlt: tACK https://github.com/bitcoin/bitcoin/pull/25454/commits/99f4785cad94657dcf349d00fdd6f1d44cac9bb0 Good improvement in the code. Tree-SHA512: b8a63f6f71ac83e292edc0200def7835ad8b06b2955dd34e3ea6fac85980fa6962efd31d689ef5ea121ff5477ec14aafa4bbe2d0db134c05f4a31a57a8ced365
2022-07-01Replace GetTime() with NodeClock in MaybeSendGetHeaders()Suhas Daftuar
2022-06-29Merge bitcoin/bitcoin#25290: [kernel 3a/n] Decouple `CTxMemPool` from ↵MacroFake
`ArgsManager` d1684beabe5b738c2cc83de83e1aaef11a761b69 fees: Pass in a filepath instead of referencing gArgs (Carl Dong) 9a3d825c30e8e6118d74a4e568744cb9d03f7f5d init: Remove redundant -*mempool*, -limit* queries (Carl Dong) 6c5c60c4124293d948735756f84efc85262ea66f mempool: Use m_limit for UpdateTransactionsFromBlock (Carl Dong) 9e93b1030182eff92ef91181e17c7dd498c7e164 node/ifaces: Use existing MemPoolLimits (Carl Dong) 38af2bcf358a72b9457d370282e57f4be1c5c849 mempoolaccept: Use limits from mempool in constructor (Carl Dong) 9333427014695ac235c96d48791098168dfdc9db mempool: Introduce (still-unused) MemPoolLimits (Carl Dong) 716bb5fbd31077bbe99d11a54d6c2c250afc8085 scripted-diff: Rename anc/desc size limit vars to indicate SI unit (Carl Dong) 1ecc77321deb61b9f6888e4e10752b9d972fd26e scripted-diff: Rename DEFAULT_MEMPOOL_EXPIRY to indicate time unit (Carl Dong) aa9141cd8185cb7ad532bc16feb9d302b05d9697 mempool: Pass in -mempoolexpiry instead of referencing gArgs (Carl Dong) 51c7a41a5eb6fcb60333812c770d80227cf7b64d init: Only determine maxmempool once (Carl Dong) 386c9472c8764738282e6d163b42e15a8feda7ea mempool: Make GetMinFee() with custom size protected (Carl Dong) 82f00de7a6a60cbc9ad0c6e1d0ffb1bc70c49af5 mempool: Pass in -maxmempool instead of referencing gArgs (Carl Dong) f1941e8bfd2eecc478c7660434b1ebf6a64095a0 pool: Add and use MemPoolOptions, ApplyArgsManOptions (Carl Dong) 0199bd35bb44e32ee0db9b51c9d1bd7518c26f19 fuzz/rbf: Add missing TestingSetup (Carl Dong) ccbaf546a68d6cda8ed3efd0598c0e4121b366bb scripted-diff: Rename DEFAULT_MAX_MEMPOOL_SIZE to indicate SI unit (Carl Dong) fc02f77ca604f0221171bfde3059b34f5d0fb1cd ArgsMan: Add Get*Arg functions returning optional (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- As mentioned in the Stage 1 Step 2 description of [the `libbitcoinkernel` project](https://github.com/bitcoin/bitcoin/issues/24303), `ArgsManager` will not be part of `libbitcoinkernel`. Therefore, it is important that we remove any dependence on `ArgsManager` by code that will be part of `libbitcoinkernel`. This is the first in a series of PRs aiming to achieve this. This PR removes `CTxMemPool+MempoolAccept`'s dependency on `ArgsManager` by introducing a `CTxMemPool::Options` struct, which is used to specify `CTxMemPool`'s various options at construction time. These options are: - `-maxmempool` -> `CTxMemPool::Options::max_size` - `-mempoolexpiry` -> `CTxMemPool::Options::expiry` - `-limitancestorcount` -> `CTxMemPool::Options::limits::ancestor_count` - `-limitancestorsize` -> `CTxMemPool::Options::limits::ancestor_size` - `-limitdescendantcount` -> `CTxMemPool::Options::limits::descendant_count` - `-limitdescendantsize` -> `CTxMemPool::Options::limits::descendant_size` More context can be gleaned from the commit messages. The important commits are: - 56eb479ded8bfb2ef635bb6f3b484f9d5952c70d "pool: Add and use MemPoolOptions, ApplyArgsManOptions" - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs" - 6f4bf3ede5812b374828f08fc728ceded2f10024 "mempool: Pass in -mempoolexpiry instead of referencing gArgs" - 5958a7fe4806599fc620ee8c1a881ca10fa2dd16 "mempool: Introduce (still-unused) MemPoolLimits" Reviewers: Help needed in the following commits (see commit messages): - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs" - 0695081a797e9a5d7787b78b0f8289dafcc6bff7 "node/ifaces: Use existing MemPoolLimits" Note to Reviewers: There are perhaps an infinite number of ways to architect `CTxMemPool::Options`, the current one tries to keep it simple, usable, and flexible. I hope we don't spend too much time arguing over the design here since that's not the point. In the case that you're 100% certain that a different design is strictly better than this one in every regard, please show us a fully-implemented branch. ----- TODO: - [x] Use the more ergonomic `CTxMemPool::Options` where appropriate - [x] Doxygen comments for `ApplyArgsManOptions`, `MemPoolOptions` ----- Questions for Reviewers: 1. Should we use `std::chrono::seconds` for `CTxMemPool::Options::expiry` and `CTxMemPool::m_expiry` instead of an `int64_t`? Something else? (`std::chrono::hours`?) 2. Should I merge `CTxMemPool::Limits` inside `CTxMemPool::Options`? ACKs for top commit: MarcoFalke: ACK d1684beabe5b738c2cc83de83e1aaef11a761b69 🍜 ryanofsky: Code review ACK d1684beabe5b738c2cc83de83e1aaef11a761b69. Just minor cleanups since last review, mostly switching to brace initialization Tree-SHA512: 2c138e52d69f61c263f1c3648f01c801338a8f576762c815f478ef5148b8b2f51e91ded5c1be915e678c0b14f6cfba894b82afec58d999d39a7bb7c914736e0b
2022-06-28Don't send getheaders message when another request is outstandingSuhas Daftuar
Change getheaders messages so that we wait up to 2 minutes for a response to a prior getheaders message before issuing a new one. Also change the handling of the getheaders message sent in response to a block INV, so that we no longer use the hashstop variable (including the hash stop will just mean that if our peer's headers chain is longer, then we won't learn it, so there's no benefit to using hashstop). Also, now respond to a getheaders during IBD with an empty headers message (rather than nothing) -- this better conforms to the intent of the new logic that it's better to not ignore a peer's getheaders message, even if you have nothing to give. This also avoids a lot of functional tests breaking. p2p_segwit.py is modified to use this same strategy, as the test logic (of expecting a getheaders after a block inv) would otherwise be broken.
2022-06-28Cleanup received_new_header calculation to use WITH_LOCKSuhas Daftuar
2022-06-28Move peer state updates from headers message into separate functionSuhas Daftuar
2022-06-28Move headers direct fetch to end of ProcessHeadersMessageSuhas Daftuar
2022-06-28Move headers-direct-fetch logic into own functionSuhas Daftuar
2022-06-28Move additional headers fetching to own functionSuhas Daftuar
Also moves the call to happen directly after validation of a headers message (rather than mixed in with other state updates for the peer), and removes an incorrect comment in favor of one that explains why headers sync must continue from the last header a peer has sent.
2022-06-28Add helper function for checking header continuitySuhas Daftuar
2022-06-28Move handling of unconnecting headers into own functionSuhas Daftuar
2022-06-28mempool: Pass in -maxmempool instead of referencing gArgsCarl Dong
- Store the mempool size limit (-maxmempool) in CTxMemPool as a member. - Remove the requirement to explicitly specify a mempool size limit for CTxMemPool::GetMinFee(...) and LimitMempoolSize(...), just use the stored mempool size limit where possible. - Remove all now-unnecessary instances of: gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE_MB) * 1000000 The code change in CChainState::GetCoinsCacheSizeState() is correct since the coinscache should not repurpose "extra" mempool memory headroom for itself if the mempool doesn't even exist.
2022-06-28Merge bitcoin/bitcoin#25480: Replace CountSecondsDouble with ↵fanquake
Ticks<SecondsDouble> fa956e7508986991008e2f6126ab307924b3f353 Replace CountSecondsDouble with Ticks<SecondsDouble> (MacroFake) Pull request description: Seems odd to have two ways to say exactly the same thing when one is sufficient. ACKs for top commit: fanquake: ACK fa956e7508986991008e2f6126ab307924b3f353 shaavan: ACK fa956e7508986991008e2f6126ab307924b3f353 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25480/commits/fa956e7508986991008e2f6126ab307924b3f353 Tree-SHA512: b599470e19b693da1ed1102d1e86b08cb03adaddf2048752b6d050fdf86055be117ff0ae10b6953d03e00eaaf7b0cfa350137968b67d6c5b3ca68c5aa50ca6aa
2022-06-27Merge bitcoin/bitcoin#25404: p2p, doc: Use MAX_BLOCKS_TO_ANNOUNCE consistentlyfanquake
e357c8953880715a943b892deed04e7777187999 p2p, doc: Use MAX_BLOCKS_TO_ANNOUNCE consistently (Martin Zumsande) Pull request description: Block announcements via headers may have up to `MAX_BLOCKS_TO_ANNOUNCE = 8` entries according to the definition of this constant. However, there are a few spots saying they should have a size _less than_ `MAX_BLOCKS_TO_ANNOUNCE`. Fix these. I don't think that this is critical (this only changes behavior when we get a headers announcement with exactly `MAX_BLOCKS_TO_ANNOUNCE` blocks which we can't connect), but it would be nice to handle this limit consistently. ACKs for top commit: dergoegge: utACK e357c8953880715a943b892deed04e7777187999 - This PR makes the usage and docs of `MAX_BLOCKS_TO_ANNOUNCE` consistent with its description. Tree-SHA512: f3772026ab0f402e3a551127ef6e4a98fa9e7af250715fe317c05988b5b33f2f3e098a00e03960d4d28c8bd2b7a97231f7f99f22f1c152c000b2e27b658cf8f2
2022-06-27Merge bitcoin/bitcoin#25144: refactor: Pass Peer& to Misbehaving()MacroFake
fa8aa0aa8180c3a0369c7589b8747666778a0deb Pass Peer& to Misbehaving() (MacroFake) Pull request description: `Misbehaving` has several coding related issues (ignoring the conceptual issues here for now): * It is public, but it is not supposed to be called from outside of net_processing. Fix that by making it private and creating a public `UnitTestMisbehaving` method for unit testing only. * It doesn't do anything if a `nullptr` is passed. It would be less confusing to just skip the call instead. Fix that by passing `Peer&` to `Misbehaving()`. * It calls `GetPeerRef`, causing `!m_peer_mutex` lock annotations to be propagated. This is harmless, but verbose. Fix it by removing the no longer needed call to `GetPeerRef` and the no longer needed lock annotations. ACKs for top commit: vasild: ACK fa8aa0aa8180c3a0369c7589b8747666778a0deb w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25144/commits/fa8aa0aa8180c3a0369c7589b8747666778a0deb Tree-SHA512: e60a6b317f2b826f9e0724285d00b632d3e2a91ded9fa5ba01c80766c5d39270b719be234c01302d46eaba600910032693836aa116ff05ee1b590c7530881cd3