Merge #15990: Add tests and documentation for blocksonly

fa8ced3 doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)

Pull request description:

  This is de-facto no longer hidden

ACKs for commit fa8ced:
  jamesob:
    utACK bitcoin/bitcoin@fa8ced3

Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245

Author: laanwj

doc/reduce-traffic.md

@@ -35,3 +35,16 @@ blocks and transactions to fewer nodes.
 Reducing the maximum connected nodes to a minimum could be desirable if traffic
 limits are tiny. Keep in mind that bitcoin's trustless model works best if you are
 connected to a handful of nodes.
+
+## 4. Turn off transaction relay (`-blocksonly`)
+
+Forwarding transactions to peers increases the P2P traffic. To only sync blocks
+with other peers, you can disable transaction relay.
+
+Be reminded of the effects of this setting.
+
+- Fee estimation will no longer work.
+- Not relaying other's transactions could hurt your privacy if used while a
+  wallet is loaded or if you use the node to broadcast transactions.
+- It makes block propagation slower because compact block relay can only be
+  used when transaction relay is enabled.

src/init.cpp

@@ -383,7 +383,7 @@ void SetupServerArgs()
     gArgs.AddArg("-blocksdir=<dir>", "Specify directory to hold blocks subdirectory for *.dat files (default: <datadir>)", false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-blocknotify=<cmd>", "Execute command when the best block changes (%s in cmd is replaced by block hash)", false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-blockreconstructionextratxn=<n>", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), false, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-blocksonly", strprintf("Whether to operate in a blocks only mode (default: %u)", DEFAULT_BLOCKSONLY), true, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Transactions from the wallet or RPC are not affected. (default: %u)", DEFAULT_BLOCKSONLY), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-datadir=<dir>", "Specify data directory", false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), true, OptionsCategory::OPTIONS);
@@ -1426,7 +1426,7 @@ bool AppInitMain(InitInterfaces& interfaces)
     // see Step 2: parameter interactions for more information about these
     fListen = gArgs.GetBoolArg("-listen", DEFAULT_LISTEN);
     fDiscover = gArgs.GetBoolArg("-discover", true);
-    fRelayTxes = !gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY);
+    g_relay_txes = !gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY);
 
     for (const std::string& strAddr : gArgs.GetArgs("-externalip")) {
         CService addrLocal;

src/net.cpp

@@ -79,7 +79,7 @@ static const uint64_t RANDOMIZER_ID_LOCALHOSTNONCE = 0xd93e69e2bbfa5735ULL; // S
 //
 bool fDiscover = true;
 bool fListen = true;
-bool fRelayTxes = true;
+bool g_relay_txes = !DEFAULT_BLOCKSONLY;
 CCriticalSection cs_mapLocalHost;
 std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost);
 static bool vfLimited[NET_MAX] GUARDED_BY(cs_mapLocalHost) = {};

src/net.h

@@ -519,7 +519,7 @@ CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices)
 
 extern bool fDiscover;
 extern bool fListen;
-extern bool fRelayTxes;
+extern bool g_relay_txes;
 
 /** Subversion as sent to the P2P network in `version` messages */
 extern std::string strSubVersion;

src/net_processing.cpp

@@ -422,7 +422,7 @@ static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime)
     CAddress addrMe = CAddress(CService(), nLocalNodeServices);
 
     connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe,
-            nonce, strSubVersion, nNodeStartingHeight, ::fRelayTxes));
+            nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes));
 
     if (fLogIPs) {
         LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), addrYou.ToString(), nodeid);
@@ -2189,7 +2189,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
             return false;
         }
 
-        bool fBlocksOnly = !fRelayTxes;
+        bool fBlocksOnly = !g_relay_txes;
 
         // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true
         if (pfrom->fWhitelisted && gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY))
@@ -2445,7 +2445,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
     if (strCommand == NetMsgType::TX) {
         // Stop processing the transaction early if
         // We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off
-        if (!fRelayTxes && (!pfrom->fWhitelisted || !gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)))
+        if (!g_relay_txes && (!pfrom->fWhitelisted || !gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)))
         {
             LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom->GetId());
             return true;

src/rpc/net.cpp

@@ -496,7 +496,7 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
     obj.pushKV("protocolversion",PROTOCOL_VERSION);
     if(g_connman)
         obj.pushKV("localservices", strprintf("%016x", g_connman->GetLocalServices()));
-    obj.pushKV("localrelay",     fRelayTxes);
+    obj.pushKV("localrelay", g_relay_txes);
     obj.pushKV("timeoffset",    GetTimeOffset());
     if (g_connman) {
         obj.pushKV("networkactive", g_connman->GetNetworkActive());

test/functional/p2p_blocksonly.py

@@ -0,0 +1,58 @@
+#!/usr/bin/env python3
+# Copyright (c) 2019 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test p2p blocksonly"""
+
+from test_framework.messages import msg_tx, CTransaction, FromHex
+from test_framework.mininode import P2PInterface
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+
+
+class P2PBlocksOnly(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = False
+        self.num_nodes = 1
+        self.extra_args = [["-blocksonly"]]
+
+    def run_test(self):
+        self.nodes[0].add_p2p_connection(P2PInterface())
+
+        self.log.info('Check that txs from p2p are rejected')
+        prevtx = self.nodes[0].getblock(self.nodes[0].getblockhash(1), 2)['tx'][0]
+        rawtx = self.nodes[0].createrawtransaction(
+            inputs=[{
+                'txid': prevtx['txid'],
+                'vout': 0
+            }],
+            outputs=[{
+                self.nodes[0].get_deterministic_priv_key().address: 50 - 0.00125
+            }],
+        )
+        sigtx = self.nodes[0].signrawtransactionwithkey(
+            hexstring=rawtx,
+            privkeys=[self.nodes[0].get_deterministic_priv_key().key],
+            prevtxs=[{
+                'txid': prevtx['txid'],
+                'vout': 0,
+                'scriptPubKey': prevtx['vout'][0]['scriptPubKey']['hex'],
+            }],
+        )['hex']
+        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()
+            assert_equal(self.nodes[0].getmempoolinfo()['size'], 0)
+
+        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)]):
+            self.nodes[0].sendrawtransaction(sigtx)
+            self.nodes[0].p2p.wait_for_tx(txid)
+            assert_equal(self.nodes[0].getmempoolinfo()['size'], 1)
+
+
+if __name__ == '__main__':
+    P2PBlocksOnly().main()

test/functional/test_framework/mininode.py

@@ -361,6 +361,14 @@ def wait_for_disconnect(self, timeout=60):
 
     # Message receiving helper methods
 
+    def wait_for_tx(self, txid, timeout=60):
+        def test_function():
+            if not self.last_message.get('tx'):
+                return False
+            return self.last_message['tx'].tx.rehash() == txid
+
+        wait_until(test_function, timeout=timeout, lock=mininode_lock)
+
     def wait_for_block(self, blockhash, timeout=60):
         test_function = lambda: self.last_message.get("block") and self.last_message["block"].block.rehash() == blockhash
         wait_until(test_function, timeout=timeout, lock=mininode_lock)

test/functional/test_framework/util.py

@@ -216,7 +216,7 @@ def wait_until(predicate, *, attempts=float('inf'), timeout=float('inf'), lock=N
         time.sleep(0.05)
 
     # Print the cause of the timeout
-    predicate_source = inspect.getsourcelines(predicate)
+    predicate_source = "''''\n" + inspect.getsource(predicate) + "'''"
     logger.error("wait_until() failed. Predicate: {}".format(predicate_source))
     if attempt >= attempts:
         raise AssertionError("Predicate {} not true after {} attempts".format(predicate_source, attempts))

test/functional/test_runner.py

@@ -141,6 +141,7 @@
     'rpc_net.py',
     'wallet_keypool.py',
     'p2p_mempool.py',
+    'p2p_blocksonly.py',
     'mining_prioritisetransaction.py',
     'p2p_invalid_locator.py',
     'p2p_invalid_block.py',