Auto merge of #2814 - str4d:2074-node-bloom, r=str4d

Add NODE_BLOOM service bit

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6579
  - Zcash equivalent of BIP 111
- bitcoin/bitcoin#6652
  - Docs for BIP 111
- bitcoin/bitcoin#7087
- bitcoin/bitcoin#7174
- bitcoin/bitcoin#8709

Part of #2074. Closes #2738.

doc/bips.md

@@ -0,0 +1,4 @@
+BIPs that are implemented by Zcash (up-to-date up to **v1.1.0**):
+
+* Numerous historic BIPs were present in **v1.0.0** at launch; see [the protocol spec](https://github.com/zcash/zips/blob/master/protocol/protocol.pdf) for details.
+* [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, but only enforced for peer versions `>=170004` as of **v1.1.0** ([PR #2814](https://github.com/zcash/zcash/pull/2814)).

doc/release-notes.md

@@ -20,3 +20,20 @@ inputs each. Starting from this release, `-mempooltxinputlimit` will be enforced
 before the Overwinter activation height is reached, but will be ignored once
 Overwinter activates. The option will be removed entirely in a future release
 after Overwinter has activated.
+
+`NODE_BLOOM` service bit
+------------------------
+
+Support for the `NODE_BLOOM` service bit, as described in [BIP
+111](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki), has been
+added to the P2P protocol code.
+
+BIP 111 defines a service bit to allow peers to advertise that they support
+Bloom filters (such as used by SPV clients) explicitly. It also bumps the protocol
+version to allow peers to identify old nodes which allow Bloom filtering of the
+connection despite lacking the new service bit.
+
+In this version, it is only enforced for peers that send protocol versions
+`>=170004`. For the next major version it is planned that this restriction will be
+removed. It is recommended to update SPV clients to check for the `NODE_BLOOM`
+service bit for nodes that report version 170004 or newer.

qa/pull-tester/rpc-tests.sh

@@ -57,6 +57,7 @@ testScripts=(
     'overwinter_peer_management.py'
     'rewind_index.py'
     'p2p_txexpiry_dos.py'
+    'p2p_node_bloom.py'
 );
 testScriptsExt=(
     'getblocktemplate_longpoll.py'

qa/rpc-tests/p2p_node_bloom.py

@@ -0,0 +1,111 @@
+#!/usr/bin/env python2
+# Copyright (c) 2018 The Zcash developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+from test_framework.mininode import NodeConn, NodeConnCB, NetworkThread, \
+    msg_filteradd, msg_filterclear, mininode_lock, MY_VERSION
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import initialize_chain_clean, start_nodes, \
+    p2p_port, assert_equal
+
+import time
+
+
+class TestNode(NodeConnCB):
+    def __init__(self):
+        NodeConnCB.__init__(self)
+        self.create_callback_map()
+        self.connection = None
+
+    def add_connection(self, conn):
+        self.connection = conn
+
+    # Spin until verack message is received from the node.
+    # We use this to signal that our test can begin. This
+    # is called from the testing thread, so it needs to acquire
+    # the global lock.
+    def wait_for_verack(self):
+        while True:
+            with mininode_lock:
+                if self.verack_received:
+                    return
+            time.sleep(0.05)
+
+    # Wrapper for the NodeConn's send_message function
+    def send_message(self, message):
+        self.connection.send_message(message)
+
+    def on_close(self, conn):
+        pass
+
+    def on_reject(self, conn, message):
+        conn.rejectMessage = message
+
+
+class NodeBloomTest(BitcoinTestFramework):
+
+    def setup_chain(self):
+        print "Initializing test directory "+self.options.tmpdir
+        initialize_chain_clean(self.options.tmpdir, 2)
+
+    def setup_network(self):
+        self.nodes = start_nodes(2, self.options.tmpdir,
+                                 extra_args=[['-nopeerbloomfilters', '-enforcenodebloom'], []])
+
+    def run_test(self):
+        nobf_node = TestNode()
+        bf_node = TestNode()
+
+        connections = []
+        connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], nobf_node))
+        connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], bf_node))
+        nobf_node.add_connection(connections[0])
+        bf_node.add_connection(connections[1])
+
+        # Start up network handling in another thread
+        NetworkThread().start()
+
+        nobf_node.wait_for_verack()
+        bf_node.wait_for_verack()
+
+        # Verify mininodes are connected to zcashd nodes
+        peerinfo = self.nodes[0].getpeerinfo()
+        versions = [x["version"] for x in peerinfo]
+        assert_equal(1, versions.count(MY_VERSION))
+        peerinfo = self.nodes[1].getpeerinfo()
+        versions = [x["version"] for x in peerinfo]
+        assert_equal(1, versions.count(MY_VERSION))
+
+        # Mininodes send filterclear message to zcashd node.
+        nobf_node.send_message(msg_filterclear())
+        bf_node.send_message(msg_filterclear())
+
+        time.sleep(3)
+
+        # Verify mininodes are still connected to zcashd nodes
+        peerinfo = self.nodes[0].getpeerinfo()
+        versions = [x["version"] for x in peerinfo]
+        assert_equal(1, versions.count(MY_VERSION))
+        peerinfo = self.nodes[1].getpeerinfo()
+        versions = [x["version"] for x in peerinfo]
+        assert_equal(1, versions.count(MY_VERSION))
+
+        # Mininodes send filteradd message to zcashd node.
+        nobf_node.send_message(msg_filteradd())
+        bf_node.send_message(msg_filteradd())
+
+        time.sleep(3)
+
+        # Verify NoBF mininode has been dropped, and BF mininode is still connected.
+        peerinfo = self.nodes[0].getpeerinfo()
+        versions = [x["version"] for x in peerinfo]
+        assert_equal(0, versions.count(MY_VERSION))
+        peerinfo = self.nodes[1].getpeerinfo()
+        versions = [x["version"] for x in peerinfo]
+        assert_equal(1, versions.count(MY_VERSION))
+
+        [ c.disconnect_node() for c in connections ]
+
+if __name__ == '__main__':
+    NodeBloomTest().main()

qa/rpc-tests/test_framework/mininode.py

@@ -1263,6 +1263,38 @@ class msg_reject(object):
             % (self.message, self.code, self.reason, self.data)
 
 
+class msg_filteradd(object):
+    command = "filteradd"
+
+    def __init__(self):
+        self.data = ""
+
+    def deserialize(self, f):
+        self.data = deser_string(f)
+
+    def serialize(self):
+        return ser_string(self.data)
+
+    def __repr__(self):
+        return "msg_filteradd(data=%s)" % (repr(self.data))
+
+
+class msg_filterclear(object):
+    command = "filterclear"
+
+    def __init__(self):
+        pass
+
+    def deserialize(self, f):
+        pass
+
+    def serialize(self):
+        return ""
+
+    def __repr__(self):
+        return "msg_filterclear()"
+
+
 # This is what a callback should look like for NodeConn
 # Reimplement the on_* functions to provide handling for events
 class NodeConnCB(object):

src/init.cpp

@@ -389,6 +389,9 @@ std::string HelpMessage(HelpMessageMode mode)
     strUsage += HelpMessageOpt("-onion=<ip:port>", strprintf(_("Use separate SOCKS5 proxy to reach peers via Tor hidden services (default: %s)"), "-proxy"));
     strUsage += HelpMessageOpt("-onlynet=<net>", _("Only connect to nodes in network <net> (ipv4, ipv6 or onion)"));
     strUsage += HelpMessageOpt("-permitbaremultisig", strprintf(_("Relay non-P2SH multisig (default: %u)"), 1));
+    strUsage += HelpMessageOpt("-peerbloomfilters", strprintf(_("Support filtering of blocks and transaction with Bloom filters (default: %u)"), 1));
+    if (showDebug)
+        strUsage += HelpMessageOpt("-enforcenodebloom", strprintf("Enforce minimum protocol version to limit use of Bloom filters (default: %u)", 0));
     strUsage += HelpMessageOpt("-port=<port>", strprintf(_("Listen for connections on <port> (default: %u or testnet: %u)"), 8233, 18233));
     strUsage += HelpMessageOpt("-proxy=<ip:port>", _("Connect through SOCKS5 proxy"));
     strUsage += HelpMessageOpt("-proxyrandomize", strprintf(_("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)"), 1));
@@ -1025,6 +1028,9 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
     // Option to startup with mocktime set (used for regression testing):
     SetMockTime(GetArg("-mocktime", 0)); // SetMockTime(0) is a no-op
 
+    if (GetBoolArg("-peerbloomfilters", true))
+        nLocalServices |= NODE_BLOOM;
+
 #ifdef ENABLE_MINING
     if (mapArgs.count("-mineraddress")) {
         CBitcoinAddress addr;

src/main.cpp

@@ -4721,8 +4721,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
     }
 
 
-
-
     if (strCommand == "version")
     {
         // Each connection can only send one version message
@@ -5371,12 +5369,13 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
         vector<CInv> vInv;
         BOOST_FOREACH(uint256& hash, vtxid) {
             CInv inv(MSG_TX, hash);
-            CTransaction tx;
-            bool fInMemPool = mempool.lookup(hash, tx);
-            if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
-            if ((pfrom->pfilter && pfrom->pfilter->IsRelevantAndUpdate(tx)) ||
-               (!pfrom->pfilter))
-                vInv.push_back(inv);
+            if (pfrom->pfilter) {
+                CTransaction tx;
+                bool fInMemPool = mempool.lookup(hash, tx);
+                if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
+                if (!pfrom->pfilter->IsRelevantAndUpdate(tx)) continue;
+            }
+            vInv.push_back(inv);
             if (vInv.size() == MAX_INV_SZ) {
                 pfrom->PushMessage("inv", vInv);
                 vInv.clear();
@@ -5498,6 +5497,20 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
     }
 
 
+    else if (!(nLocalServices & NODE_BLOOM) &&
+              (strCommand == "filterload" ||
+               strCommand == "filteradd"))
+    {
+        if (pfrom->nVersion >= NO_BLOOM_VERSION) {
+            Misbehaving(pfrom->GetId(), 100);
+            return false;
+        } else if (GetBoolArg("-enforcenodebloom", false)) {
+            pfrom->fDisconnect = true;
+            return false;
+        }
+    }
+
+
     else if (strCommand == "filterload")
     {
         CBloomFilter filter;
@@ -5540,8 +5553,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
     else if (strCommand == "filterclear")
     {
         LOCK(pfrom->cs_filter);
-        delete pfrom->pfilter;
-        pfrom->pfilter = new CBloomFilter();
+        if (nLocalServices & NODE_BLOOM) {
+            delete pfrom->pfilter;
+            pfrom->pfilter = new CBloomFilter();
+        }
         pfrom->fRelayTxes = true;
     }
 

src/protocol.h

@@ -71,6 +71,10 @@ enum {
     // set by all Bitcoin Core nodes, and is unset by SPV clients or other peers that just want
     // network services but don't provide them.
     NODE_NETWORK = (1 << 0),
+    // NODE_BLOOM means the node is capable and willing to handle bloom-filtered connections.
+    // Zcash nodes used to support this by default, without advertising this bit,
+    // but no longer do as of protocol version 170004 (= NO_BLOOM_VERSION)
+    NODE_BLOOM = (1 << 2),
 
     // Bits 24-31 are reserved for temporary experiments. Just pick a bit that
     // isn't getting used, or one not being used much, and notify the

src/version.h

@@ -9,7 +9,7 @@
  * network protocol versioning
  */
 
-static const int PROTOCOL_VERSION = 170003;
+static const int PROTOCOL_VERSION = 170004;
 
 //! initial proto version, to be increased after version/verack negotiation
 static const int INIT_PROTO_VERSION = 209;
@@ -30,4 +30,7 @@ static const int BIP0031_VERSION = 60000;
 //! "mempool" command, enhanced "getdata" behavior starts with this version
 static const int MEMPOOL_GD_VERSION = 60002;
 
+//! "filter*" commands are disabled without NODE_BLOOM after and including this version
+static const int NO_BLOOM_VERSION = 170004;
+
 #endif // BITCOIN_VERSION_H