Merge bitcoin/bitcoin#27501: mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0

67b7fecacd [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow)
c1061acb9d [functional test] prioritisation is not removed during replacement and expiry (glozow)
0e5874f0b0 [functional test] getprioritisedtransactions RPC (glozow)
99f8046829 [rpc] add getprioritisedtransactions (glozow)
9e9ca36c80 [mempool] add GetPrioritisedTransactions (glozow)

Pull request description:

  Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`.

  Motivation / Background
  - `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted.
  - Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen.
  - Since #8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart.
  - Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them.
  - Related: #4818 and #6464.
  - There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat.
  - Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while.

  Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are.

ACKs for top commit:
  achow101:
    ACK 67b7fecacd
  theStack:
    Code-review ACK 67b7fecacd
  instagibbs:
    code review ACK 67b7fecacd
  ajtowns:
    ACK 67b7fecacd code review only, some nits

Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
This commit is contained in:
Andrew Chow
2023-06-07 03:20:39 -04:00
7 changed files with 154 additions and 2 deletions

View File

@@ -876,8 +876,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
}
++nTransactionsUpdated;
}
if (delta == 0) {
mapDeltas.erase(hash);
LogPrintf("PrioritiseTransaction: %s (%sin mempool) delta cleared\n", hash.ToString(), it == mapTx.end() ? "not " : "");
} else {
LogPrintf("PrioritiseTransaction: %s (%sin mempool) fee += %s, new delta=%s\n",
hash.ToString(),
it == mapTx.end() ? "not " : "",
FormatMoney(nFeeDelta),
FormatMoney(delta));
}
}
LogPrintf("PrioritiseTransaction: %s fee += %s\n", hash.ToString(), FormatMoney(nFeeDelta));
}
void CTxMemPool::ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const
@@ -896,6 +905,22 @@ void CTxMemPool::ClearPrioritisation(const uint256& hash)
mapDeltas.erase(hash);
}
std::vector<CTxMemPool::delta_info> CTxMemPool::GetPrioritisedTransactions() const
{
AssertLockNotHeld(cs);
LOCK(cs);
std::vector<delta_info> result;
result.reserve(mapDeltas.size());
for (const auto& [txid, delta] : mapDeltas) {
const auto iter{mapTx.find(txid)};
const bool in_mempool{iter != mapTx.end()};
std::optional<CAmount> modified_fee;
if (in_mempool) modified_fee = iter->GetModifiedFee();
result.emplace_back(delta_info{in_mempool, delta, modified_fee, txid});
}
return result;
}
const CTransaction* CTxMemPool::GetConflictTx(const COutPoint& prevout) const
{
const auto it = mapNextTx.find(prevout);