Merge #12225: Mempool cleanups
669c943
Avoid leaking prioritization information when relaying transactions (Suhas Daftuar)e868b22
fee estimator: avoid sorting mempool on shutdown (Suhas Daftuar)0975406
Correct mempool mapTx comment (Suhas Daftuar) Pull request description: Following up on #12127 and #12118, this cleans up a comment that was left incorrect in txmempool.h, and addresses a couple of the observations @TheBlueMatt made about an unnecessary use of `queryHashes()` and a small information leak when prioritizing transactions. Left undone is nuking queryHashes altogether; that would require changing the behavior of the `getrawmempool` rpc call, which I think I might be in favor of doing, but wanted to save for its own PR. Tree-SHA512: c97d10b96dcd6520459287a4a2eda92774173757695100fcfe61e526aef86f394507c331d17f9e0c14b496c33ec46198a0f165a847762ca50f7c6780b993f162
This commit is contained in:
commit
67447ba060
4 changed files with 17 additions and 12 deletions
|
@ -213,7 +213,7 @@ void Shutdown()
|
||||||
|
|
||||||
if (fFeeEstimatesInitialized)
|
if (fFeeEstimatesInitialized)
|
||||||
{
|
{
|
||||||
::feeEstimator.FlushUnconfirmed(::mempool);
|
::feeEstimator.FlushUnconfirmed();
|
||||||
fs::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME;
|
fs::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME;
|
||||||
CAutoFile est_fileout(fsbridge::fopen(est_path, "wb"), SER_DISK, CLIENT_VERSION);
|
CAutoFile est_fileout(fsbridge::fopen(est_path, "wb"), SER_DISK, CLIENT_VERSION);
|
||||||
if (!est_fileout.IsNull())
|
if (!est_fileout.IsNull())
|
||||||
|
|
|
@ -981,16 +981,17 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein)
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
void CBlockPolicyEstimator::FlushUnconfirmed(CTxMemPool& pool) {
|
void CBlockPolicyEstimator::FlushUnconfirmed() {
|
||||||
int64_t startclear = GetTimeMicros();
|
int64_t startclear = GetTimeMicros();
|
||||||
std::vector<uint256> txids;
|
|
||||||
pool.queryHashes(txids);
|
|
||||||
LOCK(cs_feeEstimator);
|
LOCK(cs_feeEstimator);
|
||||||
for (auto& txid : txids) {
|
size_t num_entries = mapMemPoolTxs.size();
|
||||||
removeTx(txid, false);
|
// Remove every entry in mapMemPoolTxs
|
||||||
|
while (!mapMemPoolTxs.empty()) {
|
||||||
|
auto mi = mapMemPoolTxs.begin();
|
||||||
|
removeTx(mi->first, false); // this calls erase() on mapMemPoolTxs
|
||||||
}
|
}
|
||||||
int64_t endclear = GetTimeMicros();
|
int64_t endclear = GetTimeMicros();
|
||||||
LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n",txids.size(), (endclear - startclear)*0.000001);
|
LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, (endclear - startclear)*0.000001);
|
||||||
}
|
}
|
||||||
|
|
||||||
FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee)
|
FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee)
|
||||||
|
|
|
@ -223,7 +223,7 @@ public:
|
||||||
bool Read(CAutoFile& filein);
|
bool Read(CAutoFile& filein);
|
||||||
|
|
||||||
/** Empty mempool transactions on shutdown to record failure to confirm for txs still in mempool */
|
/** Empty mempool transactions on shutdown to record failure to confirm for txs still in mempool */
|
||||||
void FlushUnconfirmed(CTxMemPool& pool);
|
void FlushUnconfirmed();
|
||||||
|
|
||||||
/** Calculation of highest target that estimates are tracked for */
|
/** Calculation of highest target that estimates are tracked for */
|
||||||
unsigned int HighestTargetTracked(FeeEstimateHorizon horizon) const;
|
unsigned int HighestTargetTracked(FeeEstimateHorizon horizon) const;
|
||||||
|
|
|
@ -241,15 +241,18 @@ public:
|
||||||
|
|
||||||
/** \class CompareTxMemPoolEntryByScore
|
/** \class CompareTxMemPoolEntryByScore
|
||||||
*
|
*
|
||||||
* Sort by score of entry ((fee+delta)/size) in descending order
|
* Sort by feerate of entry (fee/size) in descending order
|
||||||
|
* This is only used for transaction relay, so we use GetFee()
|
||||||
|
* instead of GetModifiedFee() to avoid leaking prioritization
|
||||||
|
* information via the sort order.
|
||||||
*/
|
*/
|
||||||
class CompareTxMemPoolEntryByScore
|
class CompareTxMemPoolEntryByScore
|
||||||
{
|
{
|
||||||
public:
|
public:
|
||||||
bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
|
bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
|
||||||
{
|
{
|
||||||
double f1 = (double)a.GetModifiedFee() * b.GetTxSize();
|
double f1 = (double)a.GetFee() * b.GetTxSize();
|
||||||
double f2 = (double)b.GetModifiedFee() * a.GetTxSize();
|
double f2 = (double)b.GetFee() * a.GetTxSize();
|
||||||
if (f1 == f2) {
|
if (f1 == f2) {
|
||||||
return b.GetTx().GetHash() < a.GetTx().GetHash();
|
return b.GetTx().GetHash() < a.GetTx().GetHash();
|
||||||
}
|
}
|
||||||
|
@ -379,8 +382,9 @@ public:
|
||||||
*
|
*
|
||||||
* mapTx is a boost::multi_index that sorts the mempool on 4 criteria:
|
* mapTx is a boost::multi_index that sorts the mempool on 4 criteria:
|
||||||
* - transaction hash
|
* - transaction hash
|
||||||
* - feerate [we use max(feerate of tx, feerate of tx with all descendants)]
|
* - descendant feerate [we use max(feerate of tx, feerate of tx with all descendants)]
|
||||||
* - time in mempool
|
* - time in mempool
|
||||||
|
* - ancestor feerate [we use min(feerate of tx, feerate of tx with all unconfirmed ancestors)]
|
||||||
*
|
*
|
||||||
* Note: the term "descendant" refers to in-mempool transactions that depend on
|
* Note: the term "descendant" refers to in-mempool transactions that depend on
|
||||||
* this one, while "ancestor" refers to in-mempool transactions that a given
|
* this one, while "ancestor" refers to in-mempool transactions that a given
|
||||||
|
|
Loading…
Reference in a new issue