foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-09 10:43:19 -05:00
Code Issues Activity

[net processing] Don't add AlreadyHave txs to recentRejects

Browse source 
Now, we only add a transaction to our recentRejects filter if we didn't
already have it, meaning that it is added at most once, as intended.
This commit is contained in:
Troy Giorshev 2020-08-12 11:04:29 -04:00
parent 55b1ffcd25
commit d419fdedbe
2 changed files with 29 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
src/net_processing.cpp
View file

@ -2946,13 +2946,9 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
pfrom.AddKnownTx(txid); pfrom.AddKnownTx(txid);
} }
TxValidationState state;
m_txrequest.ReceivedResponse(pfrom.GetId(), txid); m_txrequest.ReceivedResponse(pfrom.GetId(), txid);
if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid); if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid);
std::list<CTransactionRef> lRemovedTxn;
// We do the AlreadyHaveTx() check using wtxid, rather than txid - in the // We do the AlreadyHaveTx() check using wtxid, rather than txid - in the
// absence of witness malleation, this is strictly better, because the // absence of witness malleation, this is strictly better, because the
// recent rejects filter may contain the wtxid but rarely contains // 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 // already; and an adversary can already relay us old transactions
// (older than our recency filter) if trying to DoS us, without any need // (older than our recency filter) if trying to DoS us, without any need
// for witness malleation. // for witness malleation.
if (!AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool) && if (AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool)) {
AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */)) { 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<CTransactionRef> lRemovedTxn;
if (AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */)) {
m_mempool.check(&::ChainstateActive().CoinsTip()); m_mempool.check(&::ChainstateActive().CoinsTip());
// As this version of the transaction was acceptable, we can forget about any // As this version of the transaction was acceptable, we can forget about any
// requests for it. // requests for it.
@ -3088,19 +3101,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
AddToCompactExtraTransactions(ptx); 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 // If a tx has been detected by recentRejects, we will have reached

11
test/functional/p2p_permissions.py
View file

@ -153,11 +153,20 @@ class P2PPermissionsTests(BitcoinTestFramework):
self.log.debug("Check that node[1] will not send an invalid tx to node[0]") self.log.debug("Check that node[1] will not send an invalid tx to node[0]")
tx.vout[0].nValue += 1 tx.vout[0].nValue += 1
txid = tx.rehash() 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( p2p_rebroadcast_wallet.send_txs_and_test(
[tx], [tx],
self.nodes[1], self.nodes[1],
success=False, 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): def checkpermission(self, args, expectedPermissions, whitelisted):