[p2p] allow entries with the same txid in TxOrphanage

Index by wtxid instead of txid to allow entries with the same txid but
different witnesses in orphanage. This prevents an attacker from
blocking a transaction from entering the orphanage by sending a mutated
version of it.
This commit is contained in:
glozow
2024-04-29 16:40:28 +01:00
parent c31f148166
commit 8923edfc1f
4 changed files with 23 additions and 30 deletions

View File

@@ -23,7 +23,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
const Txid& hash = tx->GetHash();
const Wtxid& wtxid = tx->GetWitnessHash();
if (m_orphans.count(hash))
if (m_orphans.count(wtxid))
return false;
// Ignore big transactions, to avoid a
@@ -40,11 +40,9 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
return false;
}
auto ret = m_orphans.emplace(hash, OrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()});
auto ret = m_orphans.emplace(wtxid, OrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()});
assert(ret.second);
m_orphan_list.push_back(ret.first);
// Allow for lookups in the orphan pool by wtxid, as well as txid
m_wtxid_to_orphan_it.emplace(tx->GetWitnessHash(), ret.first);
for (const CTxIn& txin : tx->vin) {
m_outpoint_to_orphan_it[txin.prevout].insert(ret.first);
}
@@ -63,10 +61,7 @@ int TxOrphanage::EraseTx(const Wtxid& wtxid)
int TxOrphanage::EraseTxNoLock(const Wtxid& wtxid)
{
AssertLockHeld(m_mutex);
auto it_by_wtxid = m_wtxid_to_orphan_it.find(wtxid);
if (it_by_wtxid == m_wtxid_to_orphan_it.end()) return 0;
std::map<Txid, OrphanTx>::iterator it = it_by_wtxid->second;
std::map<Wtxid, OrphanTx>::iterator it = m_orphans.find(wtxid);
if (it == m_orphans.end())
return 0;
for (const CTxIn& txin : it->second.tx->vin)
@@ -91,7 +86,6 @@ int TxOrphanage::EraseTxNoLock(const Wtxid& wtxid)
const auto& txid = it->second.tx->GetHash();
LogPrint(BCLog::TXPACKAGES, " removed orphan tx %s (wtxid=%s)\n", txid.ToString(), wtxid.ToString());
m_orphan_list.pop_back();
m_wtxid_to_orphan_it.erase(wtxid);
m_orphans.erase(it);
return 1;
@@ -104,13 +98,13 @@ void TxOrphanage::EraseForPeer(NodeId peer)
m_peer_work_set.erase(peer);
int nErased = 0;
std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin();
std::map<Wtxid, OrphanTx>::iterator iter = m_orphans.begin();
while (iter != m_orphans.end())
{
std::map<Txid, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
if (maybeErase->second.fromPeer == peer)
{
nErased += EraseTxNoLock(maybeErase->second.tx->GetWitnessHash());
// increment to avoid iterator becoming invalid after erasure
const auto& [wtxid, orphan] = *iter++;
if (orphan.fromPeer == peer) {
nErased += EraseTxNoLock(wtxid);
}
}
if (nErased > 0) LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx from peer=%d\n", nErased, peer);
@@ -127,10 +121,10 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
// Sweep out expired orphan pool entries:
int nErased = 0;
int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL;
std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin();
std::map<Wtxid, OrphanTx>::iterator iter = m_orphans.begin();
while (iter != m_orphans.end())
{
std::map<Txid, OrphanTx>::iterator maybeErase = iter++;
std::map<Wtxid, OrphanTx>::iterator maybeErase = iter++;
if (maybeErase->second.nTimeExpire <= nNow) {
nErased += EraseTxNoLock(maybeErase->second.tx->GetWitnessHash());
} else {
@@ -162,7 +156,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
for (const auto& elem : it_by_prev->second) {
// Get this source peer's work set, emplacing an empty set if it didn't exist
// (note: if this peer wasn't still connected, we would have removed the orphan tx already)
std::set<Txid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
std::set<Wtxid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
// Add this tx to the work set
orphan_work_set.insert(elem->first);
LogPrint(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
@@ -175,7 +169,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
{
LOCK(m_mutex);
return m_wtxid_to_orphan_it.count(wtxid);
return m_orphans.count(wtxid);
}
CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
@@ -186,10 +180,10 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
if (work_set_it != m_peer_work_set.end()) {
auto& work_set = work_set_it->second;
while (!work_set.empty()) {
Txid txid = *work_set.begin();
Wtxid wtxid = *work_set.begin();
work_set.erase(work_set.begin());
const auto orphan_it = m_orphans.find(txid);
const auto orphan_it = m_orphans.find(wtxid);
if (orphan_it != m_orphans.end()) {
return orphan_it->second.tx;
}