Merge bitcoin/bitcoin#35972: fuzz: Fix assertion in `txorphan`
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#35972: fuzz: Fix assertion in `txorphan`
01dde6b20578616af7620a2180ee6b5db0ca4793 fuzz: Fix assertion in txorphan (marcofleon)
Pull request description:
`EraseTx()` calls `LimitOrphans()`, which may evict announcements from a peer that didn't announce the erased transaction, causing that peer's usage to decrease. Relax the assertion in the `EraseTx()` branch that claimed usage of a non-announcer peer should be unchanged. Also, add assertions for the other cases.
ACKs for top commit:
dergoegge:
utACK 01dde6b20578616af7620a2180ee6b5db0ca4793
instagibbs:
ACK 01dde6b20578616af7620a2180ee6b5db0ca4793
Tree-SHA512: 2e597b85fd41058c2fa79fa55f0d37e12505065b5e27aba7b9680e0c249a5450e6fa97b45394d6ffe1318f42538134ffa9c423b126c455f6f8e6d8ca59eed4b6
What changed, and why it matters
This change fixes a test-only assertion in a fuzzing harness for Bitcoin Core's orphan transaction handling. It does not alter production network code, consensus rules, or wallet behavior. The only effect is that an internal correctness check in a randomized test program now matches the actual behavior of the orphanage when transactions are removed. There is no security impact on real Bitcoin nodes or users.
No security action required. Treat as a normal test-code fix.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit modifies src/test/fuzz/txorphan.cpp, a fuzz target that exercises TxOrphanage. The previous assertion incorrectly assumed that after EraseTx(), a peer that did not announce the erased transaction would have unchanged byte usage. In reality, EraseTx() calls LimitOrphans(), which may evict other orphan announcements from any peer, so any peer’s usage can decrease. The patch relaxes that assertion and adds corresponding bounds for the cases where the transaction was not found, was found and announced by the peer, and was found but announced by a different peer. This is purely a test-code bug fix.
Changed components
src/test/fuzz/txorphan.cppInspect captured patch +7 / −5
### src/test/fuzz/txorphan.cpp
@@ -173,11 +173,13 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
{
auto bytes_from_peer_before{orphanage->UsageByPeer(peer_id)};
Assert(have_tx == orphanage->EraseTx(tx->GetWitnessHash()));
- // After EraseTx, the orphanage may trim itself, so all peers' usage may have gone up or down.
- if (have_tx) {
- if (!have_tx_and_peer) {
- Assert(orphanage->UsageByPeer(peer_id) == bytes_from_peer_before);
- }
+ // After EraseTx, the orphanage may trim itself, so any peer's usage may decrease.
+ if (!have_tx) {
+ Assert(orphanage->UsageByPeer(peer_id) == bytes_from_peer_before);
+ } else if (have_tx_and_peer) {
+ Assert(orphanage->UsageByPeer(peer_id) <= bytes_from_peer_before - tx_weight);
+ } else {
+ Assert(orphanage->UsageByPeer(peer_id) <= bytes_from_peer_before);
}
}
have_tx = orphanage->HaveTx(tx->GetWitnessHash());Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.