Skip to content

Commit 4307849

Browse files
glozowglozow
committed
[mempool] delete exists(uint256) function
Allowing callers to pass in a uint256 (which could be txid or wtxid) but then always assuming that it's a txid is a footgunny interface.
1 parent d50fbd4 commit 4307849

File tree

8 files changed

+29
-30
lines changed

8 files changed

+29
-30
lines changed

‎src/net_processing.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3247,7 +3247,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
32473247
// Always relay transactions received from peers with forcerelay
32483248
// permission, even if they were already in the mempool, allowing
32493249
// the node to function as a gateway for nodes hidden behind it.
3250-
if (!m_mempool.exists(tx.GetHash())) {
3250+
if (!m_mempool.exists(GenTxid::Txid(tx.GetHash()))) {
32513251
LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId());
32523252
} else {
32533253
LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId());

‎src/node/interfaces.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ class ChainImpl : public Chain
555555
{
556556
if (!m_node.mempool) return false;
557557
LOCK(m_node.mempool->cs);
558-
return m_node.mempool->exists(txid);
558+
return m_node.mempool->exists(GenTxid::Txid(txid));
559559
}
560560
bool hasDescendantsInMempool(const uint256& txid) override
561561
{

‎src/policy/rbf.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
2222

2323
// If this transaction is not in our mempool, then we can't be sure
2424
// we will know about all its inputs.
25-
if (!pool.exists(tx.GetHash())) {
25+
if (!pool.exists(GenTxid::Txid(tx.GetHash()))) {
2626
return RBFTransactionState::UNKNOWN;
2727
}
2828

@@ -98,7 +98,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
9898
if (!parents_of_conflicts.count(tx.vin[j].prevout.hash)) {
9999
// Rather than check the UTXO set - potentially expensive - it's cheaper to just check
100100
// if the new input refers to a tx that's in the mempool.
101-
if (pool.exists(tx.vin[j].prevout.hash)) {
101+
if (pool.exists(GenTxid::Txid(tx.vin[j].prevout.hash))) {
102102
return strprintf("replacement %s adds unconfirmed input, idx %d",
103103
tx.GetHash().ToString(), j);
104104
}

‎src/rpc/blockchain.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
516516
std::set<std::string> setDepends;
517517
for (const CTxIn& txin : tx.vin)
518518
{
519-
if (pool.exists(txin.prevout.hash))
519+
if (pool.exists(GenTxid::Txid(txin.prevout.hash)))
520520
setDepends.insert(txin.prevout.hash.ToString());
521521
}
522522

‎src/test/mempool_tests.cpp

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -444,12 +444,12 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
444444
pool.addUnchecked(entry.Fee(5000LL).FromTx(tx2));
445445

446446
pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing
447-
BOOST_CHECK(pool.exists(tx1.GetHash()));
448-
BOOST_CHECK(pool.exists(tx2.GetHash()));
447+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx1.GetHash())));
448+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx2.GetHash())));
449449

450450
pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // should remove the lower-feerate transaction
451-
BOOST_CHECK(pool.exists(tx1.GetHash()));
452-
BOOST_CHECK(!pool.exists(tx2.GetHash()));
451+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx1.GetHash())));
452+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx2.GetHash())));
453453

454454
pool.addUnchecked(entry.FromTx(tx2));
455455
CMutableTransaction tx3 = CMutableTransaction();
@@ -462,14 +462,14 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
462462
pool.addUnchecked(entry.Fee(20000LL).FromTx(tx3));
463463

464464
pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP)
465-
BOOST_CHECK(!pool.exists(tx1.GetHash()));
466-
BOOST_CHECK(pool.exists(tx2.GetHash()));
467-
BOOST_CHECK(pool.exists(tx3.GetHash()));
465+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx1.GetHash())));
466+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx2.GetHash())));
467+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx3.GetHash())));
468468

469469
pool.TrimToSize(GetVirtualTransactionSize(CTransaction(tx1))); // mempool is limited to tx1's size in memory usage, so nothing fits
470-
BOOST_CHECK(!pool.exists(tx1.GetHash()));
471-
BOOST_CHECK(!pool.exists(tx2.GetHash()));
472-
BOOST_CHECK(!pool.exists(tx3.GetHash()));
470+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx1.GetHash())));
471+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx2.GetHash())));
472+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx3.GetHash())));
473473

474474
CFeeRate maxFeeRateRemoved(25000, GetVirtualTransactionSize(CTransaction(tx3)) + GetVirtualTransactionSize(CTransaction(tx2)));
475475
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
@@ -529,19 +529,19 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
529529

530530
// we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that
531531
pool.TrimToSize(pool.DynamicMemoryUsage() - 1);
532-
BOOST_CHECK(pool.exists(tx4.GetHash()));
533-
BOOST_CHECK(pool.exists(tx6.GetHash()));
534-
BOOST_CHECK(!pool.exists(tx7.GetHash()));
532+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx4.GetHash())));
533+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx6.GetHash())));
534+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx7.GetHash())));
535535

536-
if (!pool.exists(tx5.GetHash()))
536+
if (!pool.exists(GenTxid::Txid(tx5.GetHash())))
537537
pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5));
538538
pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7));
539539

540540
pool.TrimToSize(pool.DynamicMemoryUsage() / 2); // should maximize mempool size by only removing 5/7
541-
BOOST_CHECK(pool.exists(tx4.GetHash()));
542-
BOOST_CHECK(!pool.exists(tx5.GetHash()));
543-
BOOST_CHECK(pool.exists(tx6.GetHash()));
544-
BOOST_CHECK(!pool.exists(tx7.GetHash()));
541+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx4.GetHash())));
542+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx5.GetHash())));
543+
BOOST_CHECK(pool.exists(GenTxid::Txid(tx6.GetHash())));
544+
BOOST_CHECK(!pool.exists(GenTxid::Txid(tx7.GetHash())));
545545

546546
pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5));
547547
pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7));

‎src/txmempool.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,7 @@ CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>& hashes) c
969969
bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const
970970
{
971971
for (unsigned int i = 0; i < tx.vin.size(); i++)
972-
if (exists(tx.vin[i].prevout.hash))
972+
if (exists(GenTxid::Txid(tx.vin[i].prevout.hash)))
973973
return false;
974974
return true;
975975
}
@@ -1140,7 +1140,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
11401140
if (pvNoSpendsRemaining) {
11411141
for (const CTransaction& tx : txn) {
11421142
for (const CTxIn& txin : tx.vin) {
1143-
if (exists(txin.prevout.hash)) continue;
1143+
if (exists(GenTxid::Txid(txin.prevout.hash))) continue;
11441144
pvNoSpendsRemaining->push_back(txin.prevout);
11451145
}
11461146
}

‎src/txmempool.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,6 @@ class CTxMemPool
782782
}
783783
return (mapTx.count(gtxid.GetHash()) != 0);
784784
}
785-
bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}); }
786785

787786
CTransactionRef get(const uint256& hash) const;
788787
txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
@@ -802,7 +801,7 @@ class CTxMemPool
802801
LOCK(cs);
803802
// Sanity check the transaction is in the mempool & insert into
804803
// unbroadcast set.
805-
if (exists(txid)) m_unbroadcast_txids.insert(txid);
804+
if (exists(GenTxid::Txid(txid))) m_unbroadcast_txids.insert(txid);
806805
};
807806

808807
/** Removes a transaction from the unbroadcast set */

‎src/validation.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ void CChainState::MaybeUpdateMempoolForReorg(
355355
// If the transaction doesn't make it in to the mempool, remove any
356356
// transactions that depend on it (which would now be orphans).
357357
m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG);
358-
} else if (m_mempool->exists((*it)->GetHash())) {
358+
} else if (m_mempool->exists(GenTxid::Txid((*it)->GetHash()))) {
359359
vHashUpdate.push_back((*it)->GetHash());
360360
}
361361
++it;
@@ -908,7 +908,7 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
908908
// trim mempool and check if tx was trimmed
909909
if (!bypass_limits) {
910910
LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip(), gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetIntArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)});
911-
if (!m_pool.exists(hash))
911+
if (!m_pool.exists(GenTxid::Txid(hash)))
912912
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full");
913913
}
914914
return true;
@@ -4500,7 +4500,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka
45004500
// wallet(s) having loaded it while we were processing
45014501
// mempool transactions; consider these as valid, instead of
45024502
// failed, but mark them as 'already there'
4503-
if (pool.exists(tx->GetHash())) {
4503+
if (pool.exists(GenTxid::Txid(tx->GetHash()))) {
45044504
++already_there;
45054505
} else {
45064506
++failed;

0 commit comments

Comments
 (0)