diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -1102,8 +1102,8 @@ "-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted inbound peers" " with default permissions. This will relay transactions " - "even if the transactions were already in the mempool or " - "violate local relay policy (default: %d)", + "even if the transactions were already in the mempool " + "(default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); diff --git a/src/net_permissions.h b/src/net_permissions.h --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -22,8 +22,8 @@ // This peer is also not subject to limits on how many transaction INVs are // tracked PF_RELAY = (1U << 3), - // Always relay transactions from this peer, even if already in mempool or - // rejected from policy Keep parameter interaction: forcerelay implies relay + // Always relay transactions from this peer, even if already in mempool + // Keep parameter interaction: forcerelay implies relay PF_FORCERELAY = (1U << 2) | PF_RELAY, // Allow getheaders during IBD and block-download after maxuploadtarget // limit diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1381,15 +1381,6 @@ } } -/** - * Returns true if the given validation state result may result in a peer - * banning/disconnecting us. We use this to determine which unaccepted - * transactions from a whitelisted peer that we can safely relay. - */ -static bool TxRelayMayResultInDisconnect(const TxValidationState &state) { - return state.GetResult() == TxValidationResult::TX_CONSENSUS; -} - bool PeerManager::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState &state, bool via_compact_block, @@ -3677,17 +3668,13 @@ if (pfrom.HasPermission(PF_FORCERELAY)) { // Always relay transactions received from whitelisted peers, - // even if they were already in the mempool or rejected from it - // due to policy, allowing the node to function as a gateway for + // even if they were already in the mempool, allowing the node + // to function as a gateway for // nodes hidden behind it. - // - // Never relay transactions that might result in being - // disconnected (or banned). - if (state.IsInvalid() && TxRelayMayResultInDisconnect(state)) { - LogPrintf("Not relaying invalid transaction %s from " - "whitelisted peer=%d (%s)\n", - tx.GetId().ToString(), pfrom.GetId(), - state.ToString()); + if (!m_mempool.exists(tx.GetId())) { + LogPrintf("Not relaying non-mempool transaction %s from " + "whitelisted peer=%d\n", + tx.GetId().ToString(), pfrom.GetId()); } else { LogPrintf("Force relaying tx %s from whitelisted peer=%d\n", tx.GetId().ToString(), pfrom.GetId()); diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -6,13 +6,18 @@ Test that permissions are correctly calculated and applied """ - +from test_framework.messages import ( + CTransaction, + FromHex, +) +from test_framework.p2p import P2PDataStore from test_framework.test_node import ErrorMatch from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, connect_nodes, p2p_port, + wait_until ) @@ -117,6 +122,60 @@ "Cannot resolve -whitebind address", match=ErrorMatch.PARTIAL_REGEX) + def check_tx_relay(self): + address = self.nodes[0].get_deterministic_priv_key().address + block = self.nodes[0].getblock( + self.nodes[0].generatetoaddress( + 100, address)[0]) + self.sync_all() + + self.log.debug( + "Create a connection from a whitelisted wallet that rebroadcasts raw txs") + # A python mininode is needed to send the raw transaction directly. + # If a full node was used, it could only rebroadcast via the inv-getdata + # mechanism. However, even for whitelisted connections, a full node would + # currently not request a txid that is already in the mempool. + self.restart_node(1, extra_args=["-whitelist=forcerelay@127.0.0.1"]) + p2p_rebroadcast_wallet = self.nodes[1].add_p2p_connection( + P2PDataStore()) + + self.log.debug("Send a tx from the wallet initially") + tx = FromHex( + CTransaction(), + self.nodes[0].createrawtransaction( + inputs=[{ + 'txid': block['tx'][0], + 'vout': 0, + }], outputs=[{ + address: 5, + }]), + ) + txid = tx.rehash() + + self.log.debug("Wait until tx is in node[1]'s mempool") + p2p_rebroadcast_wallet.send_txs_and_test([tx], self.nodes[1]) + + self.log.debug( + "Check that node[1] will send the tx to node[0] even though it" + " is already in the mempool") + connect_nodes(self.nodes[1], 0) + with self.nodes[1].assert_debug_log( + ["Force relaying tx {} from whitelisted peer=0".format(txid)]): + p2p_rebroadcast_wallet.send_txs_and_test([tx], self.nodes[1]) + wait_until(lambda: txid in self.nodes[0].getrawmempool()) + + self.log.debug( + "Check that node[1] will not send an invalid tx to node[0]") + tx.vout[0].nValue += 1 + txid = tx.rehash() + p2p_rebroadcast_wallet.send_txs_and_test( + [tx], + self.nodes[1], + success=False, + reject_reason='Not relaying non-mempool transaction ' + '{} from whitelisted peer=0'.format(txid), + ) + def checkpermission(self, args, expectedPermissions, whitelisted): self.restart_node(1, args) connect_nodes(self.nodes[0], self.nodes[1])