doc: remove stale shortid collision TODO #34989

pull w0xlt wants to merge 1 commits into bitcoin:master from w0xlt:remove_stale_blockencodings_comment changing 1 files +0 −2

w0xlt commented at 6:58 PM on April 1, 2026: contributor

PartiallyDownloadedBlock::InitData() already treats duplicate short IDs in a compact block as READ_STATUS_FAILED.

For an honest peer, block-level short-ID collisions are rare enough that failing the compact block early is preferable to scanning the mempool or attempting selective recovery (see #34932 (comment))

This removes the old TODO suggesting that both collided transactions should be requested, since that comment no longer matches the intended handling.

No behavior change. Comment-only cleanup.

doc: remove stale shortid collision TODO

PartiallyDownloadedBlock::InitData() intentionally treats duplicate short IDs in a compact block as READ_STATUS_FAILED.

For an honest peer, block-level short-ID collisions are rare enough that failing the compact block early is preferable to spending CPU scanning the mempool or attempting selective recovery.

Remove the old TODO that suggested requesting both collided transactions, since it since it points at unwanted behavior.

fc9987dfc6

DrahtBot added the label Docs on Apr 1, 2026
DrahtBot commented at 6:59 PM on April 1, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK l0rinc, instagibbs

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
gmaxwell commented at 12:58 AM on April 2, 2026: contributor

Point of clarification, AJ's 1 in 8.8m number is right for single reconstructions, but a network of 80,000 nodes does 80,000 reconstructions per block, and 144 blocks a day.

A node somewhere on the network will fail reconstruction (under the extreme example of 8000 txn blocks) due to a duplicate shortid in the block once every 3.7 days.

Perhaps that's a cost not worth worrying too much about, but if you go reasoning from the without-multiple-comparisons number you might think it's evidence of broken peers and go disconnecting them and such. :P
fanquake commented at 1:22 AM on April 3, 2026: member

cc @instagibbs
l0rinc commented at 9:25 AM on April 6, 2026: contributor

Regardless of whether collisions would be addressed in the future, I think it's fine to remove the TODO exactly because it's not obvious that it should be applied.

ACK fc9987dfc64d4da8d2c29d999caee3c3b352c561
instagibbs commented at 12:39 PM on April 6, 2026: member

ACK fc9987dfc64d4da8d2c29d999caee3c3b352c561

A single node having to ask for a full block once every ~3.7 days isn't worth much complexity
fanquake merged this on Apr 6, 2026
fanquake closed this on Apr 6, 2026

Type	Reviewers
ACK	l0rinc, instagibbs

Contributors

Labels

Linked (view graph)

#34932 blockencodings: recover from cmpctblock short-id collisions

doc: remove stale shortid collision TODO #34989

Reviews