aboutsummaryrefslogtreecommitdiff
path: root/src/net_processing.cpp
Commit message (Collapse)AuthorAgeFilesLines
...
| * | [net processing] Add PeerJohn Newbery2020-08-121-0/+46
| | | | | | | | | | | | | | | Peer is a struct for holding per-peer data. This structure is not protected by cs_main since it does not contain validation-critical data.
| * | [net processing] Remove CNodeState.nameJohn Newbery2020-08-121-6/+3
| | | | | | | | | | | | | | | This has been unused since logging peer IPs was removed from Misbehaving() in a8865f8b.
* | | Merge #19704: Net processing: move ProcessMessage() to PeerLogicValidationfanquake2020-08-241-107/+97
|\ \ \ | |_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | daed542a12e0a6a4692aca12a61b84cd55accc33 [net_processing] Move ProcessMessage to PeerLogicValidation (John Newbery) c556770b5ef4306f10e980daff772bee87daaa30 [net_processing] Change PeerLogicValidation to hold a connman reference (John Newbery) Pull request description: Rather than ProcessMessage() being a static function in net_processing.cpp, make it a private member function of PeerLogicValidation. This is the start of moving static functions and global variables into PeerLogicValidation to make it better encapsulated. ACKs for top commit: jonatack: ACK daed542a12e0a6a4692aca12a61b84cd55accc33 code review and debug tested promag: Code review ACK daed542a12e0a6a4692aca12a61b84cd55accc33. MarcoFalke: re-ACK daed542a12, only change is removing second commit šŸŽ“ theStack: Code Review ACK daed542a12e0a6a4692aca12a61b84cd55accc33 Tree-SHA512: ddebf410d114d9ad5a9e536950018ff333a347c035d74fcc101fb4a3f20a281782c7eac2b7d1bd1c8f6bc7e59f5b5630fb52c2e1b4c32df454fa584673bd021e
| * | [net_processing] Move ProcessMessage to PeerLogicValidationJohn Newbery2020-08-211-65/+57
| | |
| * | [net_processing] Change PeerLogicValidation to hold a connman referenceJohn Newbery2020-08-121-43/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Hold a reference to connman rather than a pointer because: - PeerLogicValidation can't run without a connman - The pointer never gets reseated The alternative is to always assert that the pointer is non-null before dereferencing. Change the name from connman to m_connman at the same time to conform with current style guidelines.
* | | Merge #19070: p2p: Signal support for compact block filters with ā†µWladimir J. van der Laan2020-08-131-27/+26
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | NODE_COMPACT_FILTERS f5c003d3ead182335252558c5c6c9b9ca8968065 [test] Add test for NODE_COMPACT_FILTER. (Jim Posen) 132b30d9c84f2a8053714a438f227b583a89a9ea [net] Signal NODE_COMPACT_FILTERS if we're serving compact filters. (Jim Posen) b3fbc94d4f2937bb682f2766cc9a8d4fde328a3f Apply cfilters review fixups (John Newbery) Pull request description: If -peerblockfilters is configured, signal the `NODE_COMPACT_FILTERS` service bit to indicate that we are able to serve compact block filters, headers and checkpoints. ACKs for top commit: MarcoFalke: re-review and Concept ACK f5c003d3ead182335252558c5c6c9b9ca8968065 fjahr: Code review ACK f5c003d3ead182335252558c5c6c9b9ca8968065 clarkmoody: Concept ACK f5c003d3ead182335252558c5c6c9b9ca8968065 ariard: Concept and Code Review ACK f5c003d jonatack: ACK f5c003d3e Tree-SHA512: 34d1c153530a0e55d09046fe548c9dc37344b5d6d50e00af1b4e1de1e7b49de770fca8471346a17c151de9fe164776296bb3dd5af331977f0c3ef1e6fc906f85
| * | [net] Signal NODE_COMPACT_FILTERS if we're serving compact filters.Jim Posen2020-05-311-1/+1
| | | | | | | | | | | | | | | | | | If -peerblockfilters is configured, signal the NODE_COMPACT_FILTERS service bit to indicate that we are able to serve compact block filters, headers and checkpoints.
| * | Apply cfilters review fixupsJohn Newbery2020-05-311-26/+25
| | |
* | | Merge #19658: [rpc] Allow RPC to fetch all addrman records and add records ā†µWladimir J. van der Laan2020-08-121-2/+4
|\ \ \ | |_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | to addrman 37a480e0cd94895b6051abef12d984ff74bdc4a3 [net] Add addpeeraddress RPC method (John Newbery) ae8051bbd8377f2458ff1f167dc30c2d5f83e317 [test] Test that getnodeaddresses() can return all known addresses (John Newbery) f26502e9fc8a669b30717525597e3f468eaecf79 [addrman] Specify max addresses and pct when calling GetAddresses() (John Newbery) Pull request description: Currently addrman only allows a maximum of 1000 records or 23% of all records to be returned in a call to `GetAddr()`. Relax this limit and have the client specify the max records they want. For p2p, behaviour is unchanged (but the rate limiting is set inside net_processing, where it belongs). For RPC, `getnodeaddresses` can now return the complete addrman, which is helpful for testing and monitoring. Also add a test-only RPC `addpeeraddress`, which adds an IP address:port to addrman. This is helpful for testing (eg #18991). ACKs for top commit: naumenkogs: utACK 37a480e0cd94895b6051abef12d984ff74bdc4a3 laanwj: Code review and lightly manually tested ACK 37a480e0cd94895b6051abef12d984ff74bdc4a3 Tree-SHA512: f86dcd410aaebaf6e9ca18ce6f23556e5e4649c1325577213d873aa09967298e65ab2dc19a72670641ae92211a923afda1fe124a82e9d2c1cad73d478ef27fdc
| * | [addrman] Specify max addresses and pct when calling GetAddresses()John Newbery2020-08-121-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | CAddrMan.GetAddr() would previously limit the number and percentage of addresses returned (to ADDRMAN_GETADDR_MAX (1000) and ADDRMAN_GETADDR_MAX_PCT (23) respectively). Instead, make it the callers responsibility to specify the maximum addresses and percentage they want returned. For net_processing, the maximums are MAX_ADDR_TO_SEND (1000) and MAX_PCT_ADDR_TO_SEND (23). For rpc/net, the maximum is specified by the client.
* | | Merge #19316: [net] Cleanup logic around connection typesfanquake2020-08-121-28/+20
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 01e283068b9e6214f2d77a2f772a4244ebfe2274 [net] Remove unnecessary default args on CNode constructor (Amiti Uttarwar) bc5d65b3ca41eebb1738fdda4451d1466e77772e [refactor] Remove IsOutboundDisconnectionCandidate (Amiti Uttarwar) 2f2e13b6c2c8741ca9d825eaaef736ede484bc85 [net/refactor] Simplify multiple-connection checks (Amiti Uttarwar) 7f7b83deb2427599c129f4ff581d4d045461e459 [net/refactor] Rework ThreadOpenConnections logic (Amiti Uttarwar) 35839e963bf61d2da0d12f5b8cea74ac0e0fbd7b [net] Fix bug where AddrFetch connections would be counted as outbound full relay (Amiti Uttarwar) 4972c21b671ff73f13a1b5053338b6abbdb471b5 [net/refactor] Clarify logic for selecting connections in ThreadOpenConnections (Amiti Uttarwar) 60156f5fc40d56bb532278f16ce632c5a8b8035e [net/refactor] Remove fInbound flag from CNode (Amiti Uttarwar) 7b322df6296609570e368e5f326979279041c11f [net/refactor] Remove m_addr_fetch member var from CNode (Amiti Uttarwar) 14923422b08ac4b21b35c426bf0e1b9e7c97983b [net/refactor] Remove fFeeler flag from CNode (Amiti Uttarwar) 49efac5cae7333c6700d9b737d09fae0f3f4d7fa [net/refactor] Remove m_manual_connection flag from CNode (Amiti Uttarwar) d3698b5ee309cf0f0cdfb286d6b30a256d7deae5 [net/refactor] Add connection type as a member var to CNode (Amiti Uttarwar) 46578c03e92a55925308363ccdad04dcfc820d96 [doc] Describe different connection types (Amiti Uttarwar) 442abae2bac7bff85886143df01e14215532b974 [net/refactor] Add AddrFetch connections to ConnectionType enum (Amiti Uttarwar) af59feb05235ecb85ec9d75b09c66e71268c9889 [net/refactor] Extract m_addr_known logic from initializer list (Amiti Uttarwar) e1bc29812ddf1d946bc5acca406a7ed2dca064a6 [net/refactor] Add block relay only connections to ConnectionType enum (Amiti Uttarwar) 0e52a659a2de915fc3dce37fc8fac39be1c8b6fa [net/refactor] Add feeler connections to ConnectionType enum (Amiti Uttarwar) 1521c47438537e192230486dffcec0228a53878d [net/refactor] Add manual connections to ConnectionType enum (Amiti Uttarwar) 26304b4100201754fb32440bec3e3b78cd3f0e6d [net/refactor] Introduce an enum to distinguish type of connection (Amiti Uttarwar) 3f1b7140e95d0f8f958cb35f31c3d964c57e484d scripted-diff: Rename OneShot to AddrFetch (Amiti Uttarwar) Pull request description: **This is part 1 of #19315, which enables the ability to test `outbound` and `block-relay-only` connections from the functional tests.** Please see that PR for more information of overall functionality. **This PR simplifies how we manage different connection types.** It introduces an enum with the various types of connections so we can explicitly define the connection type. The existing system relies on a series of independent flags, then has asserts scattered around to ensure that conflicting flags are not enabled at the same time. I find this approach to be both brittle and confusing. While making these changes, I found a small bug due to the silent assumptions. This PR also proposes a rename from `OneShot` to `AddrFetch`. I find the name `OneShot` to be very confusing, especially when we also have `onetry` manual connections. Everyone I've talked to offline has agreed that the name is confusing, so I propose a potential alternative. I think this is a good opportunity for a rename since I'm creating an enum to explicitly define the connection types. (some context for the unfamiliar: `oneshot` or `addrfetch` connections are short-lived connections created on startup. They connect to the seed peers, send a `getaddr` to solicit addresses, then close the connection.) Overview of this PR: * rename `oneshot` to `addrfetch` * introduce `ConnectionType` enum * one by one, add different connection types to the enum * expose the `conn_type` on CNode, and use this to reduce reliance on flags (& asserts) * fix the bug in counting different type of connections * some additional cleanup to simplify logic and make expectations explicit/inclusive rather than implicit/exclusive. ACKs for top commit: jnewbery: utACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 laanwj: Code review ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274, the commits are pretty straightforward to follow, and I think this is a move in the right direction overall vasild: ACK 01e283068 sdaftuar: ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274. fanquake: ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 - I don't have as much experience with the networking code but these changes look fairly straight forward, the new code seems more robust/understandable and the additional documentation is great. I'm glad that a followup branch is already underway. There might be some more review comments here later today, so keep an eye on the discussion, however I'm going to merge this now. jb55: wow this code was messy before... ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 Tree-SHA512: 7bb644a6ed5849913d777ebc2ff89133ca0fbef680355a9a344e07496a979e6f9ff21a958e8eea93dcd7d5c343682b0c7174b1a3de380a4247eaae73da436e15
| * | | [refactor] Remove IsOutboundDisconnectionCandidateAmiti Uttarwar2020-08-071-9/+4
| | | |
| * | | [net/refactor] Simplify multiple-connection checksAmiti Uttarwar2020-08-071-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Extract logic that check multiple connection types into interface functions & structure as switch statements. This makes it very clear what touch points are for accessing `m_conn_type` & using the switch statements enables the compiler to warn if a new connection type is introduced but not handled for these cases.
| * | | [net/refactor] Remove fInbound flag from CNodeAmiti Uttarwar2020-08-071-13/+13
| | | |
| * | | [net/refactor] Remove m_addr_fetch member var from CNodeAmiti Uttarwar2020-08-071-4/+4
| | | |
| * | | [net/refactor] Remove fFeeler flag from CNodeAmiti Uttarwar2020-08-071-4/+3
| | | |
| * | | [net/refactor] Remove m_manual_connection flag from CNodeAmiti Uttarwar2020-08-071-6/+4
| | | |
| * | | scripted-diff: Rename OneShot to AddrFetchAmiti Uttarwar2020-08-071-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- sed -i 's/a oneshot/an addrfetch/g' src/chainparams.cpp #comment sed -i 's/oneshot/addrfetch/g' src/net.cpp #comment sed -i 's/AddOneShot/AddAddrFetch/g' src/net.h src/net.cpp sed -i 's/cs_vOneShots/m_addr_fetches_mutex/g' src/net.h src/net.cpp sed -i 's/vOneShots/m_addr_fetches/g' src/net.h src/net.cpp sed -i 's/fOneShot/m_addr_fetch/g' src/net.h src/net.cpp src/net_processing.cpp sed -i 's/ProcessOneShot/ProcessAddrFetch/g' src/net.h src/net.cpp -END VERIFY SCRIPT-
* | | | Merge #19596: Deduplicate parent txid loop of requested transactions and ā†µWladimir J. van der Laan2020-08-101-11/+33
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | missing parents of orphan transactions 4c0731f9c50b0556f8a57b912c8f295c7a9ea89c Deduplicate missing parents of orphan transactions (Suhas Daftuar) 81961762439fb72fc2ef168164689ddc29d7ef94 Rewrite parent txid loop of requested transactions (Suhas Daftuar) Pull request description: I noticed a couple of places recently where we loop over all inputs of a transaction in order to do some processing on the txids we find in those inputs. There may be thousands of inputs in a transaction, and the same txid may appear many times. In a couple of places in particular, we loop over those txids and add them to a rolling bloom filter; doing that multiple times for the same txid wastes entries in that filter. This PR fixes that in two places relating to transaction relay: one on the server side, where we look for parent transactions of a tx that we are delivering to a peer to ensure that getdata requests for those parents will succeed; and the other on the client side, where when we process an orphan tx we want to loop over the parent txids and ensure that all are eventually requested from the peer who provided the orphan. This addresses a couple of [related](https://github.com/bitcoin/bitcoin/pull/19109#discussion_r455197217) [comments](https://github.com/bitcoin/bitcoin/pull/19109#discussion_r456820373) left in #19109. ACKs for top commit: laanwj: Code review ACK 4c0731f9c50b0556f8a57b912c8f295c7a9ea89c jonatack: ACK 4c0731f9c50b0556f8a57b912c8f295c7a9ea89c ajtowns: ACK 4c0731f9c50b0556f8a57b912c8f295c7a9ea89c Tree-SHA512: 8af9df7f56c6e54b5915519d7d5465e081473ceb1bcc89bbebf83e78722cf51ff58145e588cf57126bce17071a8053273f4bcef0ad8166bec83ba14352e40f5d
| * | | | Deduplicate missing parents of orphan transactionsSuhas Daftuar2020-08-041-4/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the logic for requesting missing parents of orphan transactions, parent transactions with multiple outputs being spent by the given orphan were being processed multiple times. Fix this by deduplicating the set of missing parent txids first. Co-authored-by: Anthony Towns <[email protected]>
| * | | | Rewrite parent txid loop of requested transactionsSuhas Daftuar2020-08-041-7/+18
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we would potentially add the same txid many times to the rolling bloom filter of recently announced transactions to a peer, if many outputs of the same txid appeared as inputs in a transaction. Eliminate this problem and avoid redundant lookups by asking the mempool for the unique parents of a requested transaction.
* | | | Merge #19620: Add txids with non-standard inputs to reject filterfanquake2020-08-071-2/+33
|\ \ \ \ | |/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 test addition of unknown segwit spends to txid reject filter (Gregory Sanders) 7989901c7eb62ca28b3d1e5d5831041a7267e495 Add txids with non-standard inputs to reject filter (Suhas Daftuar) Pull request description: Our policy checks for non-standard inputs depend only on the non-witness portion of a transaction: we look up the scriptPubKey of the input being spent from our UTXO set (which is covered by the input txid), and the p2sh checks only rely on the scriptSig portion of the input. Consequently it's safe to add txids of transactions that fail these checks to the reject filter, as the witness is irrelevant to the failure. This is helpful for any situation where we might request the transaction again via txid (either from txid-relay peers, or if we might fetch the transaction via txid due to parent-fetching of orphans). Further, in preparation for future witness versions being deployed on the network, ensure that WITNESS_UNKNOWN transactions are rejected in AreInputsStandard(), so that transactions spending v1 (or greater) witness outputs will fall into this category of having their txid added to the reject filter. ACKs for top commit: ajtowns: ACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 - code review jnewbery: Code review ACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 ariard: Code Review/Tested ACK 9f88ded naumenkogs: utACK 9f88ded82b2898ca63d44c08072f1ba52f0e18d7 jonatack: ACK 9f88ded82b2 Tree-SHA512: 1e93c0a5b68cb432524780ffc0093db893911fdfed9e2ed17f888e59114cc75d2a07062aefad4e5ce2e87c9270886117a8abb3c78fb889c9b9f31967f1777148
| * | | Add txids with non-standard inputs to reject filterSuhas Daftuar2020-08-041-2/+33
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Our policy checks for non-standard inputs depend only on the non-witness portion of a transaction: we look up the scriptPubKey of the input being spent from our UTXO set (which is covered by the input txid), and the p2sh checks only rely on the scriptSig portion of the input. Consequently it's safe to add txids of transactions that fail these checks to the reject filter, as the witness is irrelevant to the failure. This is helpful for any situation where we might request the transaction again via txid (either from txid-relay peers, or if we might fetch the transaction via txid due to parent-fetching of orphans). Further, in preparation for future witness versions being deployed on the network, ensure that WITNESS_UNKNOWN transactions are rejected in AreInputsStandard(), so that transactions spending v1 (or greater) witness outputs will fall into this category of having their txid added to the reject filter.
* | | Merge #18991: Cache responses to GETADDR to prevent topology leaksWladimir J. van der Laan2020-08-031-8/+10
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6 Test addr response caching (Gleb Naumenko) cf1569e074505dbbb9d29422803dd31bb62072d4 Add addr permission flag enabling non-cached addr sharing (Gleb Naumenko) acd6135b43941fa51d52f5fcdb2ce944280ad01e Cache responses to addr requests (Gleb Naumenko) 7cc0e8101f01891aa8be093a00d993bb7579c385 Remove useless 2500 limit on AddrMan queries (Gleb Naumenko) ded742bc5b96e3215d69c11fb3628d224e7ae034 Move filtering banned addrs inside GetAddresses() (Gleb Naumenko) Pull request description: This is a very simple code change with a big p2p privacy benefit. Itā€™s currently trivial to scrape any reachable nodeā€™s AddrMan (a database of all nodes known to them along with the timestamps). We do have a limit of one GETADDR per connection, but a spy can disconnect and reconnect even from the same IP, and send GETADDR again and again. Since we respond with 1,000 random records at most, depending on the AddrMan size it takes probably up to 100 requests for an spy to make sure they scraped (almost) everything. I even have a script for that. It is totally doable within couple minutes. Then, with some extra protocol knowledge a spy can infer the direct peers of the victim, and other topological stuff. I suggest to cache responses to GETADDR on a daily basis, so that an attacker gets at most 1,000 records per day, and canā€™t track the changes in real time. I will be following up with more improvements to addr relay privacy, but this one alone is a very effective. And simple! I doubt any of the real software does *reconnect to get new addrs from a given peer*, so we shouldnā€™t be cutting anyone. I also believe it doesnā€™t have any negative implications on the overall topology quality. And the records being ā€œoutdatedā€ for at most a day doesnā€™t break any honest assumptions either. ACKs for top commit: jnewbery: reACK 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6 promag: Code review ACK 3bd67ba5a4ef3c20ef1f873b63c9f53a6c8608b6. ariard: Code Review ACK 3bd67ba Tree-SHA512: dfa5d03205c2424e40a3f8a41af9306227e1ca18beead3b3dda44aa2a082175bb1c6d929dbc7ea8e48e01aed0d50f0d54491caa1147471a2b72a46c3ca06b66f
| * | | Add addr permission flag enabling non-cached addr sharingGleb Naumenko2020-07-301-1/+6
| | | |
| * | | Cache responses to addr requestsGleb Naumenko2020-07-301-1/+1
| | | | | | | | | | | | | | | | | | | | Prevents a spy from scraping victim's AddrMan by reconnecting and re-requesting addrs.
| * | | Remove useless 2500 limit on AddrMan queriesGleb Naumenko2020-07-241-3/+3
| | | |
| * | | Move filtering banned addrs inside GetAddresses()Gleb Naumenko2020-07-241-4/+1
| | | |
* | | | refactor: make txmempool interface use GenTxidPieter Wuille2020-07-301-4/+5
| | | |
* | | | refactor: make FindTxForGetData use GenTxidPieter Wuille2020-07-301-6/+6
| | | |
* | | | refactor: use GenTxid in tx request functionsPieter Wuille2020-07-301-17/+17
| | | |
* | | | p2p: enable fetching of orphans from wtxid peersPieter Wuille2020-07-301-15/+17
| | | | | | | | | | | | | | | | Based on a commit by Anthony Towns.
* | | | refactor: add GenTxid (=txid or wtxid) type and use it for tx request logicPieter Wuille2020-07-301-17/+17
| | | |
* | | | Merge #19590: p2p, refactor: add `CInv` transaction message helpers; use in ā†µWladimir J. van der Laan2020-07-301-11/+10
|\ \ \ \ | |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | net processing c251d710a4c2981c6d52362a9a89db84da3d4a67 p2p, refactoring: use CInv helpers in net_processing.cpp (Jon Atack) 4254cd9f8f2437a916b06db4d925ce4eff8c94b9 p2p: add CInv transaction message helper methods (Jon Atack) Pull request description: Following the merge of wtxid relay in #18044, this is the first of three refactoring PRs (this one, #19610, and #19611) with no change in behavior, tightly scoped to ease review, to simplify the net processing code and improve encapsulation: - add `CInv` transaction message helper methods, defined in the class - use the new helpers in `net_processing.cpp` to simplify the code and improve encapsulation Test coverage is provided by the functional p2p tests, notably (from seeing which tests failed when breaking things to test coverage) `p2p_segwit`, `p2p_tx_download`, `p2p_feefilter`, and `p2p_permissions`. ACKs for top commit: fjahr: Code review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67 laanwj: Code review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67 vasild: ACK c251d71 theStack: Code-Review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67 hebasto: ACK c251d710a4c2981c6d52362a9a89db84da3d4a67, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: ead034b3c9e438909b4c5010c570d7930e69063c114290b051b7cebfd9bd5b19f573218bebe8a521256d32e830797f997adad3d85b4539c64ac5762b698e656d
| * | | p2p, refactoring: use CInv helpers in net_processing.cppJon Atack2020-07-271-11/+10
| | | | | | | | | | | | | | | | to simplify the code and reach less from it into the CInv class internals
* | | | [net processing] Tidy up Misbehaving()John Newbery2020-07-251-10/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | - Make const things const. - Replace conditional return with assert. - Don't log the peer's IP address. - Log the name Misbehaving directly instead of relying on __func__.
* | | | [net processing] Always supply debug message to Misbehaving()John Newbery2020-07-251-12/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Misbehaving() could optionally take a debug string for printing to the log file. Make this mandatory and always provide the string. A couple of additional minor changes: - remove the unnecessary forward declaration of Misbehaving() - don't include the nodeid or newline in the passed debug message. Misbehaving() adds these itself.
* | | | [net processing] Fixup MaybeDiscourageAndDisconnect() styleJohn Newbery2020-07-251-6/+6
|/ / / | | | | | | | | | Based on review comments from Marco Falke and Jon Atack.
* | | Merge #19472: [net processing] Reduce cs_main scope in ā†µWladimir J. van der Laan2020-07-241-62/+77
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | MaybeDiscourageAndDisconnect() 655b1957470c39bcab64917747c9f467444bd809 [net processing] Continue SendMessages processing if not disconnecting peer (John Newbery) a49781e56d2bd6a61ec027a09c1db9ee1a4abf2e [net processing] Only call MaybeDiscourageAndDisconnect from SendMessages (John Newbery) a1d5a428a24afe4f600be29e9d0d3bb4c720e816 [net processing] Fix bad indentation in SendMessages() (John Newbery) 1a1c23f8d40116741f0e26cdf22688fd91c923fc [net processing] Change cs_main TRY_LOCK to LOCK in SendMessages() (John Newbery) Pull request description: The motivation for this PR is to reduce the scope of cs_main locking in misbehavior logic. It is the first set of commits from a larger branch to move the misbehavior data out of CNodeState and into a new struct that doesn't take cs_main. There are some very minor behavior changes in this branch, such as: - Not checking for discouragement/disconnect in `ProcessMessages()` (and instead relying on the following check in `SendMessages()`) - Checking for discouragement/disconnect as the first action in `SendMessages()` (and not doing ping message sending first) - Continuing through `SendMessages()` if `MaybeDiscourageAndDisconnect()` doesn't disconnect the peer (rather than dropping out of `SendMessages()` ACKs for top commit: jonatack: re-ACK 655b195 per `git range-diff 505b4ed f54af5e 655b195`, code/commit messages review, a bit of code history, and debug build. MarcoFalke: ACK 655b195747 only some style-nits šŸš promag: Code review ACK 655b1957470c39bcab64917747c9f467444bd809. ariard: Code Review ACK 655b195 Tree-SHA512: fd6d7bc6bb789f5fb7771fb6a45f61a8faba32af93b766554f562144f9631d15c9cc849a383e71743ef73e610b4ee14853666f6fbf08a3ae35176d48c76c65d3
| * | [net processing] Continue SendMessages processing if not disconnecting peerJohn Newbery2020-07-111-20/+37
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we don't disconnect a peer in MaybeDiscourageAndDisconnect because it has NOBAN permissions or it's a manual connection, continue SendMessages processing rather than exiting early. The previous behaviour was that we'd miss the SendMessages processing on this iteration of the MessageHandler loop. That's not a problem since SendMessages() would just be called again on the next iteration, but it was slightly inefficient and confusing.
| * | [net processing] Only call MaybeDiscourageAndDisconnect from SendMessagesJohn Newbery2020-07-111-6/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `nMisbehavior` is a tally in `CNodeState` that can be incremented from anywhere. That almost always happens inside a `ProcessMessages()` call (because we increment the misbehavior score when receiving a bad messages from a peer), but not always. See, for example, the call to `MaybePunishNodeForBlock()` inside `BlockChecked()`, which is an asynchronous callback from the validation interface, executed on the scheduler thread. As long as `MaybeDiscourageAndDisconnect()` is called regularly for the node, then the misbehavior score exceeding the 100 threshold will eventually result in the peer being punished. It doesn't really matter where that `MaybeDiscourageAndDisconnect()` happens, but it makes most sense in `SendMessages()` which is where we do general peer housekeeping/maintenance. Therefore, remove the `MaybeDiscourageAndDisconnect()` call in `ProcessMessages()` and move the `MaybeDiscourageAndDisconnect()` call in `SendMessages()` to the top of the function. This moves it out of the cs_main lock scope, so take that lock directly inside `MaybeDiscourageAndDisconnect()`. Historic note: `MaybeDiscourageAndDisconnect()` was previously `SendRejectsAndCheckIfBanned()`, and before that was just sending rejects. All of those things required cs_main, which is why `MaybeDiscourageAndDisconnect()` was called after the ping logic.
| * | [net processing] Fix bad indentation in SendMessages()John Newbery2020-07-101-32/+32
| | | | | | | | | | | | Hint for reviewers: review ignoring whitespace changes.
| * | [net processing] Change cs_main TRY_LOCK to LOCK in SendMessages()John Newbery2020-07-101-5/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This was changed to TRY_LOCK in #1117 to fix a potential deadlock between cs_main and cs_vSend. cs_vSend was split into cs_vSend and cs_sendProcessing in #9535 (and cs_sendProcessing was changed from a TRY_LOCK to a LOCK in the same PR). Since cs_vSend can no longer be taken before cs_main, revert this to a LOCK(). This commit leaves part of the code with bad indentation. That is fixed by the next (whitespace change only) commit.
* | | Further improve comments around recentRejectsSuhas Daftuar2020-07-191-0/+15
| | |
* | | Disconnect peers sending wtxidrelay message after VERACKSuhas Daftuar2020-07-191-0/+6
| | |
* | | Rename AddInventoryKnown() to AddKnownTx()Suhas Daftuar2020-07-191-4/+4
| | |
* | | Make TX_WITNESS_STRIPPED its own rejection reasonSuhas Daftuar2020-07-191-8/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, TX_WITNESS_MUTATED could be returned during transaction validation for either transactions that had a witness that was non-standard, or for transactions that had no witness but were invalid due to segwit validation rules. However, for txid/wtxid-relay considerations, net_processing distinguishes the witness stripped case separately, because it affects whether a wtxid should be able to be added to the reject filter. It is safe to add the wtxid of a witness-mutated transaction to the filter (as that wtxid shouldn't collide with the txid, and hence it wouldn't interfere with transaction relay from txid-relay peers), but it is not safe to add the wtxid (== txid) of a witness-stripped transaction to the filter, because that would interfere with relay of another transaction with the same txid (but different wtxid) when relaying from txid-relay peers. Also updates the comment explaining this logic, and explaining that we can get rid of this complexity once there's a sufficient deployment of wtxid-relaying peers on the network.
* | | Delay getdata requests from peers using txid-based relaySuhas Daftuar2020-07-191-3/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | Using both txid and wtxid-based relay with peers means that we could sometimes download the same transaction twice, if announced via two different hashes from different peers. Use a heuristic of delaying txid-peer-getdata requests by 2 seconds, if we have at least one wtxid-based peer.
* | | Add p2p message "wtxidrelay"Suhas Daftuar2020-07-191-0/+16
| | | | | | | | | | | | | | | When sent to and received from a given peer, enables using wtxid's for announcing and fetching transactions with that peer.
* | | ignore non-wtxidrelay compliant invsAnthony Towns2020-07-191-0/+7
| | |
generated by cgit v1.2.3 (git 2.25.1) at 2026-03-12 01:41:54 +0000