highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-11 21:20:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

blockencodings: fix extra transaction count

Browse Source 
A short ID collision can invalidate a mempool-sourced transaction after an unrelated transaction was found in extra_txn.

Track each slot's source so extra_count is decremented only when the invalidated slot came from extra_txn. Mark collided slots explicitly so later candidates do not refill them.
This commit is contained in:
Lőrinc
2026-07-15 12:37:48 -07:00
committed by Gregory Sanders
parent be4e64d9e4
commit 6aa5d8d948
2 changed files with 38 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
src/blockencodings.cpp
View File

@@ -113,25 +113,25 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
if (shorttxids.size() != cmpctblock.shorttxids.size())
return READ_STATUS_FAILED; // Short ID collision
std::vector<bool> have_txn(txn_available.size());
enum class TxSource : uint8_t { NONE, MEMPOOL, EXTRA, COLLIDED };
std::vector<TxSource> tx_source(txn_available.size(), TxSource::NONE);
{
LOCK(pool->cs);
for (const auto& [wtxid, txit] : pool->txns_randomized) {
uint64_t shortid = cmpctblock.GetShortID(wtxid);
std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid);
if (idit != shorttxids.end()) {
if (!have_txn[idit->second]) {
if (tx_source[idit->second] == TxSource::NONE) {
txn_available[idit->second] = txit->GetSharedTx();
have_txn[idit->second] = true;
tx_source[idit->second] = TxSource::MEMPOOL;
mempool_count++;
} else {
} else if (tx_source[idit->second] != TxSource::COLLIDED) {
// If we find two mempool txn that match the short id, just request it.
// This should be rare enough that the extra bandwidth doesn't matter,
// but eating a round-trip due to FillBlock failure would be annoying
if (txn_available[idit->second]) {
txn_available[idit->second].reset();
mempool_count--;
}
txn_available[idit->second].reset();
mempool_count--;
tx_source[idit->second] = TxSource::COLLIDED;
}
}
// Though ideally we'd continue scanning for the two-txn-match-shortid case,
@@ -146,24 +146,23 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
uint64_t shortid = cmpctblock.GetShortID(extra_txn[i].first);
std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid);
if (idit != shorttxids.end()) {
if (!have_txn[idit->second]) {
if (tx_source[idit->second] == TxSource::NONE) {
txn_available[idit->second] = extra_txn[i].second;
have_txn[idit->second] = true;
tx_source[idit->second] = TxSource::EXTRA;
mempool_count++;
extra_count++;
} else {
} else if (tx_source[idit->second] != TxSource::COLLIDED &&
txn_available[idit->second]->GetWitnessHash() != extra_txn[i].second->GetWitnessHash()) {
// If we find two mempool/extra txn that match the short id, just
// request it.
// This should be rare enough that the extra bandwidth doesn't matter,
// but eating a round-trip due to FillBlock failure would be annoying
// Note that we don't want duplication between extra_txn and mempool to
// trigger this case, so we compare witness hashes first
if (txn_available[idit->second] &&
txn_available[idit->second]->GetWitnessHash() != extra_txn[i].second->GetWitnessHash()) {
txn_available[idit->second].reset();
mempool_count--;
extra_count--;
}
txn_available[idit->second].reset();
mempool_count--;
extra_count -= (tx_source[idit->second] == TxSource::EXTRA);
tx_source[idit->second] = TxSource::COLLIDED;
}
}
// Though ideally we'd continue scanning for the two-txn-match-shortid case,

23
src/test/blockencodings_tests.cpp
View File

@@ -147,6 +147,7 @@ public:
struct TestPartiallyDownloadedBlock : PartiallyDownloadedBlock {
using PartiallyDownloadedBlock::PartiallyDownloadedBlock;
size_t GetMempoolCount() const { return mempool_count; }
size_t GetExtraCount() const { return extra_count; }
};
@@ -372,7 +373,27 @@ BOOST_AUTO_TEST_CASE(ReceiveWithExtraTransactions) {
BOOST_CHECK_EQUAL(partial_block_with_extra_collision.InitData(cmpctblock, extra_txn), READ_STATUS_OK);
BOOST_CHECK(partial_block_with_extra_collision.IsTxAvailable(1));
BOOST_CHECK(!partial_block_with_extra_collision.IsTxAvailable(2));
BOOST_CHECK_EQUAL(partial_block_with_extra_collision.GetExtraCount(), 0U); // TODO: This should be 1
BOOST_CHECK_EQUAL(partial_block_with_extra_collision.GetMempoolCount(), 1U);
BOOST_CHECK_EQUAL(partial_block_with_extra_collision.GetExtraCount(), 1U);
// Now also collide the extra-sourced slot: both counters decrement exactly once.
extra_txn[3] = {block.vtx[1]->GetWitnessHash(), non_block_tx};
TestPartiallyDownloadedBlock partial_block_with_extra_source_collision{&pool};
BOOST_CHECK_EQUAL(partial_block_with_extra_source_collision.InitData(cmpctblock, extra_txn), READ_STATUS_OK);
BOOST_CHECK(!partial_block_with_extra_source_collision.IsTxAvailable(1));
BOOST_CHECK(!partial_block_with_extra_source_collision.IsTxAvailable(2));
BOOST_CHECK_EQUAL(partial_block_with_extra_source_collision.GetMempoolCount(), 0U);
BOOST_CHECK_EQUAL(partial_block_with_extra_source_collision.GetExtraCount(), 0U);
// Collided slots are terminal: not even the genuine transactions refill them.
extra_txn[4] = {block.vtx[2]->GetWitnessHash(), block.vtx[2]};
extra_txn[5] = {block.vtx[1]->GetWitnessHash(), block.vtx[1]};
TestPartiallyDownloadedBlock partial_block_no_refill{&pool};
BOOST_CHECK_EQUAL(partial_block_no_refill.InitData(cmpctblock, extra_txn), READ_STATUS_OK);
BOOST_CHECK(!partial_block_no_refill.IsTxAvailable(1));
BOOST_CHECK(!partial_block_no_refill.IsTxAvailable(2));
BOOST_CHECK_EQUAL(partial_block_no_refill.GetMempoolCount(), 0U);
BOOST_CHECK_EQUAL(partial_block_no_refill.GetExtraCount(), 0U);
}
}