diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 94d4052fa..464e3de80 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2946,13 +2946,9 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat pfrom.AddKnownTx(txid); } - TxValidationState state; - m_txrequest.ReceivedResponse(pfrom.GetId(), txid); if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid); - std::list lRemovedTxn; - // We do the AlreadyHaveTx() check using wtxid, rather than txid - in the // absence of witness malleation, this is strictly better, because the // recent rejects filter may contain the wtxid but rarely contains @@ -2965,8 +2961,25 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (!AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool) && - AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */)) { + if (AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool)) { + if (pfrom.HasPermission(PF_FORCERELAY)) { + // Always relay transactions received from peers with forcerelay + // permission, even if they were already in the mempool, allowing + // the node to function as a gateway for nodes hidden behind it. + if (!m_mempool.exists(tx.GetHash())) { + LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); + } else { + LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman); + } + } + return; + } + + TxValidationState state; + std::list lRemovedTxn; + + if (AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */)) { m_mempool.check(&::ChainstateActive().CoinsTip()); // As this version of the transaction was acceptable, we can forget about any // requests for it. @@ -3088,19 +3101,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat AddToCompactExtraTransactions(ptx); } } - - if (pfrom.HasPermission(PF_FORCERELAY)) { - // Always relay transactions received from peers with forcerelay permission, even - // if they were already in the mempool, - // allowing the node to function as a gateway for - // nodes hidden behind it. - if (!m_mempool.exists(tx.GetHash())) { - LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); - } else { - LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); - RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman); - } - } } // If a tx has been detected by recentRejects, we will have reached diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 653e3894a..ed82e6a2e 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -153,11 +153,20 @@ class P2PPermissionsTests(BitcoinTestFramework): self.log.debug("Check that node[1] will not send an invalid tx to node[0]") tx.vout[0].nValue += 1 txid = tx.rehash() + # Send the transaction twice. The first time, it'll be rejected by ATMP because it conflicts + # with a mempool transaction. The second time, it'll be in the recentRejects filter. p2p_rebroadcast_wallet.send_txs_and_test( [tx], self.nodes[1], success=False, - reject_reason='Not relaying non-mempool transaction {} from forcerelay peer=0'.format(txid), + reject_reason='{} from peer=0 was not accepted: txn-mempool-conflict'.format(txid) + ) + + p2p_rebroadcast_wallet.send_txs_and_test( + [tx], + self.nodes[1], + success=False, + reject_reason='Not relaying non-mempool transaction {} from forcerelay peer=0'.format(txid) ) def checkpermission(self, args, expectedPermissions, whitelisted):