Skip to content

Commit

Permalink
Use rolling bloom filter of recent block tx's for AlreadyHave() check
Browse files Browse the repository at this point in the history 
Summary:
> In order to determine whether to download or process a relayed transaction, we
> try to determine if we already have the transaction, either in the mempool, in
> our recently rejected filter, in our orphan pool, or already confirmed in the
> chain itself.
>
> Prior to this commit, the heuristic for checking the chain is based on whether
> there's an output corresponding to the 0- or 1-index vout in our coin cache.
> While that is a quick check, it is very imprecise (say if those outputs were
> already spent in a block) -- we can do better by just keeping a rolling bloom
> filter of the transactions in recent blocks, which will capture the case of a
> transaction which has been confirmed and then fully spent already.
>
> To avoid relay problems for transactions which have been included in a recent
> block but then reorged out of the chain, we clear the bloom filter whenever a
> block is disconnected.

This is a backport of [[bitcoin/bitcoin#17951 | PR17951]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta, deadalnix

Reviewed By: #bitcoin_abc, majcosta, deadalnix

Subscribers: deadalnix, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8695
  • Loading branch information
@sdaftuar @PiRK
sdaftuar authored and PiRK committed Dec 23, 2020
1 parent bf384a3 commit 1d95857
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 38 deletions.
115 changes: 77 additions & 38 deletions src/net_processing.cpp
View file
Expand Up @@ -206,6 +206,15 @@ std::map<BlockHash, std::pair<NodeId, bool>> mapBlockSource GUARDED_BY(cs_main);
std::unique_ptr<CRollingBloomFilter> recentRejects GUARDED_BY(cs_main);
uint256 hashRecentRejectsChainTip GUARDED_BY(cs_main);

/**
* Filter for transactions that have been recently confirmed.
* We use this to avoid requesting transactions that have already been
* confirmed.
*/
RecursiveMutex g_cs_recent_confirmed_transactions;
std::unique_ptr<CRollingBloomFilter> g_recent_confirmed_transactions
GUARDED_BY(g_cs_recent_confirmed_transactions);

/**
* Blocks that are in flight, and that are in the queue to be downloaded.
*/
Expand Down Expand Up @@ -1344,6 +1353,17 @@ PeerLogicValidation::PeerLogicValidation(CConnman *connmanIn, BanMan *banman,
// Initialize global variables that cannot be constructed at startup.
recentRejects.reset(new CRollingBloomFilter(120000, 0.000001));

// Blocks don't typically have more than 4000 transactions, so this should
// be at least six blocks (~1 hr) worth of transactions that we can store.
// If the number of transactions appearing in a block goes up, or if we are
// seeing getdata requests more than an hour after initial announcement, we
// can increase this number.
// The false positive rate of 1/1M should come out to less than 1
// transaction per day that would be inadvertently ignored (which is the
// same probability that we have in the reject filter).
g_recent_confirmed_transactions.reset(
new CRollingBloomFilter(24000, 0.000001));

const Consensus::Params &consensusParams = Params().GetConsensus();
// Stale tip checking and peer eviction are on two different timers, but we
// don't want them to get out of sync due to drift in the scheduler, so we
Expand All @@ -1367,41 +1387,63 @@ PeerLogicValidation::PeerLogicValidation(CConnman *connmanIn, BanMan *banman,
void PeerLogicValidation::BlockConnected(
const std::shared_ptr<const CBlock> &pblock, const CBlockIndex *pindex,
const std::vector<CTransactionRef> &vtxConflicted) {
LOCK(g_cs_orphans);
{
LOCK(g_cs_orphans);

std::vector<TxId> vOrphanErase;
std::vector<TxId> vOrphanErase;

for (const CTransactionRef &ptx : pblock->vtx) {
const CTransaction &tx = *ptx;
for (const CTransactionRef &ptx : pblock->vtx) {
const CTransaction &tx = *ptx;

// Which orphan pool entries must we evict?
for (const auto &txin : tx.vin) {
auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
if (itByPrev == mapOrphanTransactionsByPrev.end()) {
continue;
// Which orphan pool entries must we evict?
for (const auto &txin : tx.vin) {
auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
if (itByPrev == mapOrphanTransactionsByPrev.end()) {
continue;
}

for (auto mi = itByPrev->second.begin();
mi != itByPrev->second.end(); ++mi) {
const CTransaction &orphanTx = *(*mi)->second.tx;
const TxId &orphanId = orphanTx.GetId();
vOrphanErase.push_back(orphanId);
}
}
}

for (auto mi = itByPrev->second.begin();
mi != itByPrev->second.end(); ++mi) {
const CTransaction &orphanTx = *(*mi)->second.tx;
const TxId &orphanId = orphanTx.GetId();
vOrphanErase.push_back(orphanId);
// Erase orphan transactions included or precluded by this block
if (vOrphanErase.size()) {
int nErased = 0;
for (const auto &orphanId : vOrphanErase) {
nErased += EraseOrphanTx(orphanId);
}
LogPrint(BCLog::MEMPOOL,
"Erased %d orphan tx included or conflicted by block\n",
nErased);
}
}

// Erase orphan transactions included or precluded by this block
if (vOrphanErase.size()) {
int nErased = 0;
for (const auto &orphanId : vOrphanErase) {
nErased += EraseOrphanTx(orphanId);
g_last_tip_update = GetTime();
}
{
LOCK(g_cs_recent_confirmed_transactions);
for (const CTransactionRef &ptx : pblock->vtx) {
g_recent_confirmed_transactions->insert(ptx->GetId());
}
LogPrint(BCLog::MEMPOOL,
"Erased %d orphan tx included or conflicted by block\n",
nErased);
}
}

g_last_tip_update = GetTime();
void PeerLogicValidation::BlockDisconnected(
const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex) {
// To avoid relay problems with transactions that were previously
// confirmed, clear our filter of recently confirmed transactions whenever
// there's a reorg.
// This means that in a 1-block reorg (where 1 block is disconnected and
// then another block reconnected), our filter will drop to having only one
// block's worth of transactions in it, but that should be fine, since
// presumably the most common case of relaying a confirmed transaction
// should be just after a new block containing it is found.
LOCK(g_cs_recent_confirmed_transactions);
g_recent_confirmed_transactions->reset();
}

// All of the following cache a recent block, and are protected by
Expand Down Expand Up @@ -1562,25 +1604,22 @@ static bool AlreadyHave(const CInv &inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
recentRejects->reset();
}

const TxId txid(inv.hash);
{
LOCK(g_cs_orphans);
if (mapOrphanTransactions.count(TxId{inv.hash})) {
if (mapOrphanTransactions.count(txid)) {
return true;
}
}
const CCoinsViewCache &coins_cache =
::ChainstateActive().CoinsTip();

// Use pcoinsTip->HaveCoinInCache as a quick approximation to
// exclude requesting or processing some txs which have already been
// included in a block. As this is best effort, we only check for
// output 0 and 1. This works well enough in practice and we get
// diminishing returns with 2 onward.
const TxId txid(inv.hash);
return recentRejects->contains(inv.hash) ||
g_mempool.exists(txid) ||
coins_cache.HaveCoinInCache(COutPoint(txid, 0)) ||
coins_cache.HaveCoinInCache(COutPoint(txid, 1));

{
LOCK(g_cs_recent_confirmed_transactions);
if (g_recent_confirmed_transactions->contains(txid)) {
return true;
}
}

return recentRejects->contains(txid) || g_mempool.exists(txid);
}
case MSG_BLOCK:
return LookupBlockIndex(BlockHash(inv.hash)) != nullptr;
Expand Down
2 changes: 2 additions & 0 deletions src/net_processing.h
View file
Expand Up @@ -49,6 +49,8 @@ class PeerLogicValidation final : public CValidationInterface,
BlockConnected(const std::shared_ptr<const CBlock> &pblock,
const CBlockIndex *pindexConnected,
const std::vector<CTransactionRef> &vtxConflicted) override;
void BlockDisconnected(const std::shared_ptr<const CBlock> &block,
const CBlockIndex *pindex) override;
/**
* Overridden from CValidationInterface.
*/
Expand Down

0 comments on commit 1d95857

Please sign in to comment.