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).)

ACKs for top commit:
  sipa:
    ACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83
  ajtowns:
    ACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 -- code review, ran tests. ran it on mainnet for a couple of days with MAX_BLOCKS_ONLY_CONNECTIONS upped from 2 to 16 and didn't observe any unexpected behaviour: it disconnected a couple of peers that tried sending inv's, and it successfully did compact block relay with some block relay peers.
  TheBlueMatt:
    re-utACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83. Pointed out that stats.fRelayTxes was sometimes uninitialized for blocksonly peers (though its not a big deal and only effects RPC), which has since been fixed here. Otherwise changes are pretty trivial so looks good.
  jnewbery:
    utACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83
  jamesob:
    ACK https://github.com/bitcoin/bitcoin/commit/0ba08020c9791f7caf5986ad6490c16a2b66cd83

Tree-SHA512: 4c3629434472c7dd4125253417b1be41967a508c3cfec8af5a34cad685464fbebbb6558f0f8f5c0d4463e3ffa4fa3aabd58247692cb9ab8395f4993078b9bcdf

1 files changed, 7 insertions, 3 deletions

diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py
index 12cb06a407..3258a38e3c 100755
--- a/test/functional/p2p_blocksonly.py
+++ b/test/functional/p2p_blocksonly.py
@@ -19,7 +19,7 @@ class P2PBlocksOnly(BitcoinTestFramework):
     def run_test(self):
         self.nodes[0].add_p2p_connection(P2PInterface())
 
-        self.log.info('Check that txs from p2p are rejected')
+        self.log.info('Check that txs from p2p are rejected and result in disconnect')
         prevtx = self.nodes[0].getblock(self.nodes[0].getblockhash(1), 2)['tx'][0]
         rawtx = self.nodes[0].createrawtransaction(
             inputs=[{
@@ -42,13 +42,17 @@ class P2PBlocksOnly(BitcoinTestFramework):
         assert_equal(self.nodes[0].getnetworkinfo()['localrelay'], False)
         with self.nodes[0].assert_debug_log(['transaction sent in violation of protocol peer=0']):
             self.nodes[0].p2p.send_message(msg_tx(FromHex(CTransaction(), sigtx)))
-            self.nodes[0].p2p.sync_with_ping()
+            self.nodes[0].p2p.wait_for_disconnect()
             assert_equal(self.nodes[0].getmempoolinfo()['size'], 0)
 
+        # Remove the disconnected peer and add a new one.
+        del self.nodes[0].p2ps[0]
+        self.nodes[0].add_p2p_connection(P2PInterface())
+
         self.log.info('Check that txs from rpc are not rejected and relayed to other peers')
         assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], True)
         txid = self.nodes[0].testmempoolaccept([sigtx])[0]['txid']
-        with self.nodes[0].assert_debug_log(['received getdata for: tx {} peer=0'.format(txid)]):
+        with self.nodes[0].assert_debug_log(['received getdata for: tx {} peer=1'.format(txid)]):
             self.nodes[0].sendrawtransaction(sigtx)
             self.nodes[0].p2p.wait_for_tx(txid)
             assert_equal(self.nodes[0].getmempoolinfo()['size'], 1)