net_proc: use a single set to deduplicate tx INVs #35917

pull darosior wants to merge 1 commits into bitcoin:master from darosior:2608_no_dup_dedup_sets changing 2 files +6 −8

darosior commented at 2:29 PM on August 6, 2026: member

If we get a hit across wtxid and txid relay, then it just means this is a legacy transaction and deduping is fine.

Therefore using two sets for the purpose of deduplication is overkill and we can simplify this code.
DrahtBot commented at 2:29 PM on August 6, 2026: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35917.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.
darosior force-pushed on Aug 6, 2026
DrahtBot added the label CI failed on Aug 6, 2026
DrahtBot removed the label CI failed on Aug 6, 2026
instagibbs commented at 4:14 PM on August 6, 2026: member

imo we should drop invalid bip144 inv types, aka MSG_WITNESS_TX along with this change if you want to do it. Then you only have wtxid for witness relay peers, and txid for non-witness peers
darosior force-pushed on Aug 6, 2026

net_proc: use a single set to deduplicate tx INVs

If we get a hit across wtxid and txid relay, then it just means this is
a legacy transaction and deduping is fine.

Therefore using two sets for the purpose of deduplication is overkill
and we can simplify this code.

b2f89599d0

darosior commented at 5:11 PM on August 6, 2026: member

Right MSG_WITNESS_TX makes the additional motivation incorrect. I think it would be fine to ignore MSG_WITNESS_TX announcements, but that seems like a bigger behaviour change that does not need to be bundled with this one?

EDIT: Dropped the additional motivation from commit message and OP, i think the initial motivation stands on its own.
ajtowns commented at 6:02 PM on August 6, 2026: contributor

The runtime cost of having a redundant empty set on the stack (for peers that don't send MSG_WITNESS_TX invs) is near enough to 0 to not matter IMO; and for peers that do send MSG_WITNESS_TX not deduping seems okay as "don't accidently change behaviour". If we are happy to change behaviour (and I think we should) then dropping the MSG_WITNESS_TX invs and reducing to a single set make sense to do at the same time, IMO.
darosior commented at 6:07 PM on August 6, 2026: member

Ok, fair enough. Seems like the taste consensus here would be to go with a clear behaviour change instead.

I may do that in the future, and cherry-pick that change on top, but i think it should be a separate PR.
darosior closed this on Aug 6, 2026

Contributors

net_proc: use a single set to deduplicate tx INVs #35917

Code Coverage & Benchmarks

Reviews