p2p: stop announcing txs with ancestors below fee filter #28533

pull glozow wants to merge 3 commits into bitcoin:master from glozow:2023-09-feefilter-parents changing 4 files +109 −7
  1. glozow commented at 9:56 am on September 26, 2023: member

    Our mempool may receive a CPFP of a transaction that is below our peer’s mempool minimum feerate. There are many possible reasons, but the most common is when our peer’s mempool has a smaller capacity and so their mempool minimum feerate is higher than ours. In the future, another likely reason is that we accept packages but they don’t (this change was originally part of package relay but seems beneficial today).

    Today, we broadcast this CPFP transaction to our peers (assuming its individual feerate is above the fee filter). Assuming they’re running Bitcoin Core, this happens:

    • Peer requests the child
    • We send the child
    • The peer sees this child as an orphan, stores it in orphanage, and requests the parent
    • We send the parent
    • The parent is rejected for being too low feerate

    This is a waste of bandwidth and computation. It also wastes the peer’s orphanage slots - instagibbs has been monitoring orphanage usage recently and noted that this is happening pretty frequently.

    When announcing a transaction, if it contains a mempool ancestor below the peer’s fee filter, drop it.

  2. [test] announcement of CPFPs to feefiltered peers 1b03c05497
  3. [txmempool] add GetMinimumAncestorFeerate and info_for_announcement
    Independent function and std::pair because we don't want to bloat the
    memory used for infoAll() and info() in other places that aren't
    interested in feerates of ancestor.
    f0eb8d8fbb
  4. [p2p] don't inv tx with ancestor below feefilter
    Nodes waste bandwidth, computation, and orphanage space on transactions
    that CPFP below-min-feerate parents. Without package relay, this can
    arise when nodes have different mempool capacities.
    add174260d
  5. glozow added the label P2P on Sep 26, 2023
  6. DrahtBot commented at 9:56 am on September 26, 2023: 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. A summary of reviews will appear here.

  7. ajtowns commented at 10:20 am on September 26, 2023: contributor

    There’s the case:

    • A was accepted into the mempool when fees were lower
    • B, spending A’s first output, has a higher fee, and CPFPs A sufficiently to keep it in the mempool
    • mempool minfee rises above A’s fee rate, but due to B, A is not evicted from the mempool
    • C, spending A’s second output, comes along CPFPing A at a fee rate high enough to get into the next block

    with this change, C won’t be relayed any longer, as I understand it. Orphan slots can already be wasted pretty trivially by an attacker, so is this really doing much good?

  8. glozow commented at 10:25 am on September 26, 2023: member
    Hm good point. Maybe we drop if there’s an ancestor below fee filter + not in their m_tx_inventory_known_filter?
  9. DrahtBot added the label CI failed on Sep 26, 2023
  10. ajtowns commented at 1:14 pm on September 26, 2023: contributor

    Hm good point. Maybe we drop if there’s an ancestor below fee filter + not in their m_tx_inventory_known_filter?

    We could reply NOTFOUND if they are requesting a tx below their feefilter via GETDATA (rather than GETPKGTXNS)?

    I don’t really understand the point of worrying about special casing this, vs “just” getting package relay deployed?

  11. glozow commented at 1:29 pm on September 26, 2023: member
    Just the same rationale for why we have fee filters in the first place - I thought it’d be nice to not waste our/their bandwidth if they’ve told us they’re going to reject this kind of stuff anyway :shrug:. I don’t think it’s something to worry about, just thought an easy drop condition could be worthwhile. Sounds like no, so closing this.
  12. glozow closed this on Sep 26, 2023

  13. bitcoin locked this on Sep 25, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-11 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me