miner: bug fix? update for ancestor inclusion using modified fees, not base
Summary:
Came up while reviewing #24364, where some of us incorrectly assumed that we use the same fee deduction in CTxMemPoolModifiedEntry::nModFeesWithAncestors when first constructing an entry and in update_for_parent_inclusion. Actually, the behavior is this: when a mempool entry's ancestor is included in the block template, we create a CTxMemPoolModifiedEntry for it, subtracting the ancestor's modified fees from nModFeesWithAncestors. If another ancestor is included, we update it again, but use the ancestor's base fees instead. I can't explain why we use GetFee in one place and GetModifiedFee in the other, but I'm quite certain we should be using the same one for both. And should it be base or modified fees? Modified, otherwise the child inherits the prioritisation of the parent, but only until the parent gets mined. If we want prioritisation to cascade down to current in-mempool descendants, we should probably document that in the prioritsetransaction helpstring and implement it in CTxMemPool::mapDeltas, not as a quirk in the mining code? Wrote a test in which a mempool entry has 2 ancestors, both prioritised, and both included in a block template individually. This test should fail without the s/GetFee/GetModifiedFee commit.
Backport of core#24538.
Depends on D12951.
Test Plan:
ninja check-all
Reviewers: #bitcoin_abc, sdulfari
Reviewed By: #bitcoin_abc, sdulfari
Subscribers: sdulfari
Differential Revision: https://reviews.bitcoinabc.org/D12953