Merge bitcoin#22955: p2p: Rename fBlocksOnly, Add test

fa66a7d p2p: Rename fBlocksOnly, Add test (MarcoFalke)
fac66d0 test: Simplify p2p_blocksonly test with new miniwallet rescan_utxos method (MarcoFalke)

Pull request description:

  `fBlocksOnly` has several issues:
  * The name is confusing
  * It is untested

  Fix both.

ACKs for top commit:
  laanwj:
    Code review ACK fa66a7d

Tree-SHA512: 4218f455eeb37297f74603d7d44895288605844ae828a40dfb7a70215f1a058ac5ad945a22732f5ebcad3ad375d54ba360bea69ea79639a30d4c88b042448f0f

src/net_processing.cpp

@@ -3684,7 +3684,7 @@ void PeerManagerImpl::ProcessMessage(
     // At this point, the outgoing message serialization version can't change.
     const CNetMsgMaker msgMaker(pfrom.GetCommonVersion());
 
-    bool fBlocksOnly = pfrom.IsBlockRelayOnly();
+    bool reject_tx_invs = pfrom.IsBlockRelayOnly();
 
     if (msg_type == NetMsgType::VERACK) {
         if (pfrom.fSuccessfullyConnected) {
@@ -3718,7 +3718,7 @@ void PeerManagerImpl::ProcessMessage(
             m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION));
         }
 
-        if (!fBlocksOnly) {
+        if (!reject_tx_invs) {
             // Tell our peer that he should send us CoinJoin queue messages
             m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true));
             // Tell our peer that he should send us intra-quorum messages
@@ -3797,7 +3797,7 @@ void PeerManagerImpl::ProcessMessage(
     }
 
     // Stop processing non-block data early in blocks only mode and for block-relay-only peers
-    if (fBlocksOnly && NetMessageViolatesBlocksOnly(msg_type)) {
+    if (reject_tx_invs && NetMessageViolatesBlocksOnly(msg_type)) {
         LogPrint(BCLog::NET, "%s sent in violation of protocol peer=%d\n", msg_type, pfrom.GetId());
         pfrom.fDisconnect = true;
         return;
@@ -3962,20 +3962,14 @@ void PeerManagerImpl::ProcessMessage(
                 };
 
                 AddKnownInv(*peer, inv.hash);
-                if (fBlocksOnly && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
+                if (reject_tx_invs && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
                     LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom.GetId());
                     pfrom.fDisconnect = true;
                     return;
                 } else if (!fAlreadyHave) {
-                    if (fBlocksOnly && inv.type == MSG_ISDLOCK) {
                         if (pfrom.GetCommonVersion() <= ADDRV2_PROTO_VERSION) {
                             // It's ok to receive these invs, we just ignore them
                             // and do not request corresponding objects.
-                            continue;
-                        }
-                        // Peers with newer versions should never send us these invs when we are in blocks-relay-only mode
-                        LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom.GetId());
-                        pfrom.fDisconnect = true;
                         return;
                     }
                     bool allowWhileInIBD = allowWhileInIBDObjs.count(inv.type);

test/functional/p2p_blocksonly.py

@@ -6,7 +6,7 @@
 
 import time
 
-from test_framework.messages import msg_tx
+from test_framework.messages import msg_tx, msg_inv, CInv
 from test_framework.p2p import P2PInterface, P2PTxInvStore
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
@@ -15,15 +15,13 @@
 
 class P2PBlocksOnly(BitcoinTestFramework):
     def set_test_params(self):
-        self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [["-blocksonly"]]
 
     def run_test(self):
         self.miniwallet = MiniWallet(self.nodes[0])
         # Add enough mature utxos to the wallet, so that all txs spend confirmed coins
-        self.miniwallet.generate(2)
-        self.nodes[0].generate(100)
+        self.miniwallet.rescan_utxos()
 
         self.blocksonly_mode_tests()
         self.blocks_relay_conn_tests()
@@ -35,6 +33,13 @@ def blocksonly_mode_tests(self):
         self.nodes[0].add_p2p_connection(P2PInterface())
         tx, txid, tx_hex = self.check_p2p_tx_violation()
 
+        self.log.info('Check that tx invs also violate the protocol')
+        self.nodes[0].add_p2p_connection(P2PInterface())
+        with self.nodes[0].assert_debug_log(['transaction (0000000000000000000000000000000000000000000000000000000000001234) inv sent in violation of protocol, disconnecting peer']):
+            self.nodes[0].p2ps[0].send_message(msg_inv([CInv(h=0x1234)]))
+            self.nodes[0].p2ps[0].wait_for_disconnect()
+            del self.nodes[0].p2ps[0]
+
         self.log.info('Check that txs from rpc are not rejected and relayed to other peers')
         tx_relay_peer = self.nodes[0].add_p2p_connection(P2PInterface())
         assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], True)
@@ -83,7 +88,7 @@ def blocks_relay_conn_tests(self):
         # Ensure we disconnect if a block-relay-only connection sends us a transaction
         self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="block-relay-only")
         assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], False)
-        _, txid, tx_hex = self.check_p2p_tx_violation(index=2)
+        _, txid, tx_hex = self.check_p2p_tx_violation()
 
         self.log.info("Check that txs from RPC are not sent to blockrelay connection")
         conn = self.nodes[0].add_outbound_p2p_connection(P2PTxInvStore(), p2p_idx=1, connection_type="block-relay-only")
@@ -96,11 +101,9 @@ def blocks_relay_conn_tests(self):
         conn.sync_send_with_ping()
         assert(int(txid, 16) not in conn.get_invs())
 
-    def check_p2p_tx_violation(self, index=1):
+    def check_p2p_tx_violation(self):
         self.log.info('Check that txs from P2P are rejected and result in disconnect')
-        input_txid = self.nodes[0].getblock(self.nodes[0].getblockhash(index), 2)['tx'][0]['txid']
-        utxo_to_spend = self.miniwallet.get_utxo(txid=input_txid)
-        spendtx = self.miniwallet.create_self_transfer(from_node=self.nodes[0], utxo_to_spend=utxo_to_spend)
+        spendtx = self.miniwallet.create_self_transfer(from_node=self.nodes[0])
 
         with self.nodes[0].assert_debug_log(['tx sent in violation of protocol peer=0']):
             self.nodes[0].p2ps[0].send_message(msg_tx(spendtx['tx']))

test/functional/test_framework/wallet.py

@@ -74,6 +74,14 @@ def __init__(self, test_node, *, mode=MiniWalletMode.ADDRESS_OP_TRUE):
             self._address = ADDRESS_BCRT1_P2SH_OP_TRUE
             self._scriptPubKey = hex_str_to_bytes(self._test_node.validateaddress(self._address)['scriptPubKey'])
 
+    def rescan_utxos(self):
+        """Drop all utxos and rescan the utxo set"""
+        self._utxos = []
+        res = self._test_node.scantxoutset(action="start", scanobjects=[f'raw({self._scriptPubKey.hex()})'])
+        assert_equal(True, res['success'])
+        for utxo in res['unspents']:
+            self._utxos.append({'txid': utxo['txid'], 'vout': utxo['vout'], 'value': utxo['amount']})
+
     def scan_blocks(self, *, start=1, num):
         """Scan the blocks for self._address outputs and add them to self._utxos"""
         for i in range(start, start + num):