HomePhabricator

miner: bug fix? update for ancestor inclusion using modified fees, not base

Description

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

Details

Provenance
glozow <gloriajzhao@gmail.com>Authored on Mar 11 2022, 16:02
FabienCommitted on Jan 6 2023, 18:37
FabienPushed on Jan 6 2023, 18:37
Reviewer
Restricted Project
Differential Revision
D12953: miner: bug fix? update for ancestor inclusion using modified fees, not base
Parents
rABC1c4d06f75dc9: policy: Remove unused locktime flags
Branches
Unknown
Tags
Unknown