Merge bitcoin/bitcoin#24538: miner: bug fix? update for ancestor inclusion using modified fees, not base

e4303c337c8423f21c2c72ee1bcca3aaf46fa1cb [unit test] prioritisation in mining (glozow)
7a8d60676bc0eec289687b2dfd5d2b00b83c0eaa [miner] bug fix: update for parent inclusion using modified fee (glozow)
0f9a44461c294cf21a335e8a8c13e498baac110f MOVEONLY: group miner tests into MinerTestingSetup functions (glozow)

Pull request description:

  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.

ACKs for top commit:
  ccdle12:
    tested ACK e4303c3
  MarcoFalke:
    ACK e4303c337c8423f21c2c72ee1bcca3aaf46fa1cb 🚗

Tree-SHA512: 4cd94106fbc9353e9f9b6d5af268ecda5aec7539245298c940ca220606dd0737264505bfaae1f83d94765cc2d9e1a6e913a765048fe6c19292482241761a6762

1 files changed, 1 insertions, 1 deletions

diff --git a/src/node/miner.h b/src/node/miner.h
index c8093ec883..678df815c0 100644
--- a/src/node/miner.h
+++ b/src/node/miner.h
@@ -116,7 +116,7 @@ struct update_for_parent_inclusion
 
     void operator() (CTxMemPoolModifiedEntry &e)
     {
-        e.nModFeesWithAncestors -= iter->GetFee();
+        e.nModFeesWithAncestors -= iter->GetModifiedFee();
         e.nSizeWithAncestors -= iter->GetTxSize();
         e.nSigOpCostWithAncestors -= iter->GetSigOpCost();
     }