Age | Commit message (Collapse) | Author |
|
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
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
|
|
-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.
|
|
|
|
Also rename to m_continuation_block to better communicate meaning.
|
|
-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-
|
|
|
|
Not done as a scripted diff to avoid misnaming the local variable in
ProcessMessage().
|
|
|
|
Suggested here:
- https://github.com/bitcoin/bitcoin/pull/19607#discussion_r467071878
- https://github.com/bitcoin/bitcoin/pull/19829#discussion_r546116786
|
|
|
|
|
|
|
|
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.
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
|
|
|
|
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.
|
|
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.
|
|
|
|
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.
|
|
|
|
|
|
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.
|
|
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
|
|
|
|
|
|
fa11110bff6288f63e0c487e2e4b4079fb0f4569 util: Allow use of C++14 chrono literals (MarcoFalke)
Pull request description:
I think we should allow the use of chrono literals for new code to make it less verbose. Obviously old code can stay as-is.
This patch pulls in the needed namespace and replaces some lines for illustrative purposes.
ACKs for top commit:
vasild:
ACK fa11110bff6288f63e0c487e2e4b4079fb0f4569
jonatack:
ACK fa11110bff6288f63e0c487e2e4b4079fb0f4569
Tree-SHA512: ee2b72c8f28dee07b33b9a8ee8f7c87c0bc43b05c56a17b786cf9803ef204c7628e01b02de1af1a4eb01f5cdf6fc336f69c2833e17acd606ebda20ac6917e6bb
|
|
3025ca9e7743d9b96c22e9c6ed7ef051dcea7e54 [net processing] Add RemovePeer() (John Newbery)
a20ab22786466fe5164b53e62de9d23a4062fbca [net processing] Make GetPeerRef const (John Newbery)
ed7e469ceec6f7101a3fb7b15c21a6fb69697866 [net_processing] Move peer_map to PeerManager (John Newbery)
a529fd3e3f2391e592ac937e291fec51e067ea2e [net processing] Move GetNodeStateStats into PeerManager (John Newbery)
Pull request description:
This moves `g_peer_map` from a global in net_processing.cpp's unnamed namespace to being a member `m_peer_map` of `PeerManager`.
ACKs for top commit:
theuni:
Re-ACK 3025ca9e7743d9b96c22e9c6ed7ef051dcea7e54.
dongcarl:
Re-ACK 3025ca9
hebasto:
re-ACK 3025ca9e7743d9b96c22e9c6ed7ef051dcea7e54, since my [previous](https://github.com/bitcoin/bitcoin/pull/19910#pullrequestreview-545574237) review only reverted the change that introduced NRVO in `PeerManager::GetPeerRef`, and comments are fixed in the proper commits.
Tree-SHA512: 6369eb3c688ac5b84f89f7674115f78ff02edbed76063ac2ebb1759894c9e973883e10821a35dab92bd3d738280acc095bd5368f552a060b83cd309330387d47
|
|
'verack'
1583498fb6781c01ca2f33c09319ed793964c574 Send and require SENDADDRV2 before VERACK (Pieter Wuille)
c5a89196602e43ebb1cdc9cd4f08d153419c13e1 Don't send 'sendaddrv2' to pre-70016 software (Pieter Wuille)
Pull request description:
BIP155 defines addrv2 and sendaddrv2 for all protocol versions, but some implementations reject messages they don't know. As a courtesy, don't send it to nodes with a version before 70016, as no software is known to support BIP155 that doesn't announce at least that protocol version number.
Also move the sending of sendaddrv2 earlier (before sending verack), as proposed in https://github.com/bitcoin/bips/pull/1043. This has the side effect that local address broadcast of torv3 will work (as it'll only trigger after we know whether or not the peer supports addrv2).
ACKs for top commit:
MarcoFalke:
ACK 1583498fb6781c01ca2f33c09319ed793964c574
jnewbery:
ACK 1583498fb6781c01ca2f33c09319ed793964c574
jonatack:
ACK 1583498fb6781c01ca2f33c09319ed793964c574
vasild:
ACK 1583498
Tree-SHA512: 3bd5833fa8c8567b6dedd99e4a9b6bb71c127aa66d5284b217503c86d597dc59aa7382c41f3a4bf561bb658b89db81d1a7703a700eef4ffc17cb916660e23a82
|
|
See the corresponding BIP change: https://github.com/bitcoin/bips/pull/1043
|
|
|
|
65273fa0e74f0c11dfbf0645dd962bdc779ea558 Clear m_addr_known before our periodic self-advertisement (Suhas Daftuar)
Pull request description:
We use a rolling bloom filter to track which addresses we've previously sent a peer, but after #7125 we no longer clear it every day before our own announcement. This looks to me like an oversight which has the effect of reducing the frequency with which we actually self-announce our own address, so this reintroduces resetting that filter.
ACKs for top commit:
naumenkogs:
ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
laanwj:
Code review ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
sipa:
utACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
Tree-SHA512: 602c155fb6d2249b054fcb6f1c0dd17143605ceb87132286bbd90babf26d258ff6c41f9925482c17e2be41805d33f9b83926cb447f394969ffecd4bccfa0a64f
|
|
as 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."
|
|
|
|
This allows us to avoid repeated locking in FinalizeNode()
|
|
|
|
|
|
1816327e533d359c237c53eb6440b2f3a7cbf4fa p2p: Put disconnecting logs into BCLog::NET category (Hennadii Stepanov)
Pull request description:
It's too noisy:
```
$ cat debug.log | wc -l
28529
$ cat debug.log | grep "Disconnecting and discouraging peer" | wc -l
10177
```
ACKs for top commit:
MarcoFalke:
noban, addnode and local peers are still unconditionally logged (as they should), but this one can go into a category, so cr-ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa
practicalswift:
ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa for the reasons MarcoFalke gave above.
ajtowns:
ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa
Tree-SHA512: c312c1009090840659b2cb1364d8ad9b6ab8e742fc462aef169996d93c76c248507639a00257ed9d73a6916c01176b1793491b2305e92fdded5f9de0935b6ba6
|
|
|