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

Merge bitcoin/bitcoin#35727: blockencodings: fix extra transaction count

Browse Source 
6aa5d8d948 blockencodings: fix extra transaction count (Lőrinc)
be4e64d9e4 test: characterize extra transaction miscount (Lőrinc)

Pull request description:

  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. Retain the source after a collision to preserve the rule that later candidates do not refill the slot.

ACKs for top commit:
  l0rinc:
    retested ACK 6aa5d8d948
  andrewtoth:
    ACK 6aa5d8d948
  sedited:
    ACK 6aa5d8d948

Tree-SHA512: d4407dca7ca2b46795e52a5d611b66072d75ba966416d4ede27fa776ac0a7cbde52c8abae2daec532ec5f24d88e608622168ca066c0d1dbc133e098c8e6fe85a
This commit is contained in:
merge-script
2026-07-21 23:11:04 +02:00
parent d673ca765a 6aa5d8d948
commit 883ef1d85d
2 changed files with 58 additions and 17 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,

42
src/test/blockencodings_tests.cpp
View File

@@ -144,6 +144,13 @@ public:
SERIALIZE_METHODS(TestHeaderAndShortIDs, obj) { READWRITE(obj.header, obj.nonce, Using<VectorFormatter<CustomUintFormatter<CBlockHeaderAndShortTxIDs::SHORTTXIDS_LENGTH>>>(obj.shorttxids), obj.prefilledtxn); }
};
struct TestPartiallyDownloadedBlock : PartiallyDownloadedBlock {
using PartiallyDownloadedBlock::PartiallyDownloadedBlock;
size_t GetMempoolCount() const { return mempool_count; }
size_t GetExtraCount() const { return extra_count; }
};
BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
{
CTxMemPool& pool = *Assert(m_node.mempool);
@@ -319,6 +326,13 @@ BOOST_AUTO_TEST_CASE(ReceiveWithExtraTransactions) {
const CTransactionRef non_block_tx = MakeTransactionRef(std::move(mtx));
CBlock block(BuildBlockTestCase(rand_ctx));
// Leave one transaction missing so scanning doesn't stop before the collision.
mtx = BuildTransactionTestCase();
mtx.vin[0].prevout.hash = Txid::FromUint256(rand_ctx.rand256());
block.vtx.push_back(MakeTransactionRef(std::move(mtx)));
block.hashMerkleRoot = BlockMerkleRoot(block);
while (!CheckProofOfWork(block.GetHash(), block.nBits, Params().GetConsensus())) ++block.nNonce;
std::vector<std::pair<Wtxid, CTransactionRef>> extra_txn;
extra_txn.resize(10);
@@ -352,6 +366,34 @@ BOOST_AUTO_TEST_CASE(ReceiveWithExtraTransactions) {
// This transaction is now available via extra_txn:
BOOST_CHECK(partial_block_with_extra.IsTxAvailable(1));
BOOST_CHECK(partial_block_with_extra.IsTxAvailable(2));
// Simulate a mempool collision after finding an unrelated extra transaction.
extra_txn[2] = {block.vtx[2]->GetWitnessHash(), non_block_tx};
TestPartiallyDownloadedBlock partial_block_with_extra_collision{&pool};
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.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);
}
}