Page MenuHomePhabricator

Backport PR14897, PR15834 and PR16196

Authored by deadalnix on Nov 28 2019, 14:05.



They all are backported at once to avoid leaving master in a buggy state.

This is Core PR14897:

  • Change in transaction pull scheduling to prevent InvBlock-related attacks

Co-authored-by: Suhas Daftuar <>

This is Core PR15834:

  • Remove NOTFOUND transactions from in-flight data structures

This prevents a bug where the in-flight queue for our peers will not be
drained, resulting in not downloading any new transactions from our peers.

Thanks to ajtowns for reporting this bug.

  • Add an explicit memory bound to m_tx_process_time

Previously there was an implicit bound based on the handling of m_tx_announced,
but that approach is error-prone (particularly if we start automatically
removing things from that set).

  • Improve NOTFOUND comment
  • Expire old entries from the in-flight tx map

If a peer hasn't responded to a getdata request, eventually time out the request
and remove it from the in-flight data structures. This is to prevent any bugs in
our handling of those in-flight data structures from filling up the in-flight
map and preventing us from requesting more transactions (such as the NOTFOUND
bug, fixed in a previous commit).

Co-authored-by: Anthony Towns <>

  • Fix bug around transaction requests

If a transaction is already in-flight when a peer announces a new tx to us, we
schedule a time in the future to reconsider whether to download. At that future
time, there was a bug that would prevent transactions from being rescheduled
for potential download again (ie if the transaction was still in-flight at the
time of reconsideration, such as from some other peer). Fix this.

This is Core PR16196:

  • doc: Add release notes for 14897 & 15834
Test Plan
make check
./test/functional/ --extended

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Nov 28 2019, 14:05
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 28 2019, 14:05
Fabien requested changes to this revision.Nov 28 2019, 15:28
Fabien added a subscriber: Fabien.
Fabien added inline comments.
382 ↗(On Diff #14485)


385 ↗(On Diff #14485)


2436 ↗(On Diff #14485)

Nit: add a newline to avoid later merge conflict

2753 ↗(On Diff #14485)


3594 ↗(On Diff #14485)

Nit: in_flight_it can be const.
Also maybe make inv.hash a const TxId ?

const TxId txid(inv.hash);
auto in_flight_it = state->m_tx_download.m_tx_in_flight.find(txid);
4645 ↗(On Diff #14485)

inv.hash => txid

4657 ↗(On Diff #14485)


4658 ↗(On Diff #14485)


This revision now requires changes to proceed.Nov 28 2019, 15:28
deadalnix updated this revision to Diff 14489.Nov 28 2019, 17:40

Address various comments

Fabien accepted this revision.Nov 28 2019, 17:41
This revision is now accepted and ready to land.Nov 28 2019, 17:41
This revision was landed with ongoing or failed builds.Nov 28 2019, 17:53
This revision was automatically updated to reflect the committed changes.