Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge bitcoin#15759: p2p: Add 2 outbound block-relay-only connections #4862

Merged
merged 15 commits into from
Jun 19, 2022

Conversation

knst
Copy link
Collaborator

@knst knst commented Jun 3, 2022

All commits there are actually a backport bitcoin#15759 but provided not squashed because it's easier to merge them one-by-one: changes are too big.
Before merge should be all squash except may be 94c99e0

Please, help to decide if need to have spare bloom filter for vInventoryOtherToSend because filterInventoryKnown can't be used anymore for relay-only peers. Related merge request in past is #2292


commit 189c19e
Merge: b5a8d0c 0ba0802
Author: fanquake fanquake@gmail.com
Date: Sat Sep 7 17:24:05 2019 +0800

Merge #15759: p2p: Add 2 outbound block-relay-only connections

0ba08020c9791f7caf5986ad6490c16a2b66cd83 Disconnect peers violating blocks-only mode (Suhas Daftuar)
937eba91e1550bc3038dc541c236ac83e0a0e6d5 doc: improve comments relating to block-relay-only peers (Suhas Daftuar)
430f489027f15c1e4948ea4378954df24e3fee88 Don't relay addr messages to block-relay-only peers (Suhas Daftuar)
3a5e885306ea954d7eccdc11502e91a51dab8ec6 Add 2 outbound block-relay-only connections (Suhas Daftuar)
b83f51a4bbe29bf130a2b0c0e85e5bffea107f75 Add comment explaining intended use of m_tx_relay (Suhas Daftuar)
e75c39cd425f8c4e5b6bbb2beecb9c80034fefe1 Check that tx_relay is initialized before access (Suhas Daftuar)
c4aa2ba82211ea5988ed7fe21e1b08bc3367e6d4 [refactor] Change tx_relay structure to be unique_ptr (Suhas Daftuar)
4de0dbac9b286c42a9b10132b7c2d76712f1a319 [refactor] Move tx relay state to separate structure (Suhas Daftuar)
26a93bce29fd813e1402b013f402869c25b656d1 Remove unused variable (Suhas Daftuar)

Pull request description:

  Transaction relay is optimized for a combination of redundancy/robustness as well as bandwidth minimization -- as a result transaction relay leaks information that adversaries can use to infer the network topology.

  Network topology is better kept private for (at least) two reasons:

  (a) Knowledge of the network graph can make it easier to find the source IP of a given transaction.

  (b) Knowledge of the network graph could be used to split a target node or nodes from the honest network (eg by knowing which peers to attack in order to achieve a network split).

  We can eliminate the risks of (b) by separating block relay from transaction relay; inferring network connectivity from the relay of blocks/block headers is much more expensive for an adversary.

  After this commit, bitcoind will make 2 additional outbound connections that are only used for block relay. (In the future, we might consider rotating our transaction-relay peers to help limit the effects of (a).)

sdaftuar and others added 14 commits June 4, 2022 03:38
Transaction relay is primarily optimized for balancing redundancy/robustness
with bandwidth minimization -- as a result transaction relay leaks information
that adversaries can use to infer the network topology.

Network topology is better kept private for (at least) two reasons:

(a) Knowledge of the network graph can make it easier to find the source IP of
a given transaction.

(b) Knowledge of the network graph could be used to split a target node or
nodes from the honest network (eg by knowing which peers to attack in order to
achieve a network split).

We can eliminate the risks of (b) by separating block relay from transaction
relay; inferring network connectivity from the relay of blocks/block headers is
much more expensive for an adversary.

After this commit, bitcoind will make 2 additional outbound connections that
are only used for block relay. (In the future, we might consider rotating our
transaction-relay peers to help limit the effects of (a).)
We don't want relay of addr messages to leak information about
these network links.
If we set fRelay=false in our VERSION message, and a peer sends an INV or TX
message anyway, disconnect. Since we use fRelay=false to minimize bandwidth,
we should not tolerate remaining connected to a peer violating the protocol.
It's made to avoid possible typos with arguments, because some of them have default values and it's very high probability to make a mistake here.
…factor] Move tx relay state to separate structure`
@github-actions
Copy link

github-actions bot commented Jun 8, 2022

This pull request has conflicts, please rebase.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as a backport. -blocksonly was broken already and fixing it is out of scope for this PR imo (see #4888)

ACK

@UdjinM6 UdjinM6 requested a review from PastaPastaPasta June 18, 2022 18:29
@PastaPastaPasta PastaPastaPasta changed the title Bitcoin backport #15759 Merge bitcoin#15759: p2p: Add 2 outbound block-relay-only connections Jun 19, 2022
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit ef3f738 into dashpay:develop Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants