highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-10 22:18:54 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#29013: test: doc: follow-up #28368

Browse Source 
b1318dcc56 test: change `m_submitted_in_package` input to fuzz data provider boolean (ismaelsadeeq)
5615e16b70 tx fees: update `m_from_disconnected_block` to `m_mempool_limit_bypassed` (ismaelsadeeq)
fcd4296648 doc: fix typo and update incorrect comment (ismaelsadeeq)
562664d263 test: wait for fee estimator to catch up before estimating fees (ismaelsadeeq)

Pull request description:

  This is a simple PR that does two things
  1.   Fixes #29000 by waiting for the fee estimator to catch up after `removeForBlock` calls before calling `estimateFee` in the `BlockPolicyEstimates` unit test.

  2. Addressed some outstanding review comments from #28368
  - Updated `NewMempoolTransactionInfo::m_from_disconnected_block` to `NewMempoolTransactionInfo::m_mempool_limit_bypassed` which now correctly indicates what the boolean does.
  - Changed  input of `processTransaction`'s tx_info  `m_submitted_in_package` input from false to fuzz data provider boolean.
  - Fixed some typos, and update incorrect comment

ACKs for top commit:
  martinus:
    re-ACK b1318dcc56
  glozow:
    utACK b1318dcc56

Tree-SHA512: 45268729bc044da4748fe004524e0df696d2ec92c5bd053db9aad6e15675f3838429b2a7b9061a6b694be4dc319d1782a876b44df506ddd439d62ad07252d0e1
This commit is contained in:
Gloria Zhao
2024-01-03 11:16:48 +00:00
parent c3038bf95a b1318dcc56
commit 65c05db660
6 changed files with 27 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/init.cpp
View File

@@ -313,7 +313,7 @@ void Shutdown(NodeContext& node)
DumpMempool(*node.mempool, MempoolPath(*node.args)); DumpMempool(*node.mempool, MempoolPath(*node.args));
} }
// Drop transactions we were still watching, record fee estimations and Unregister // Drop transactions we were still watching, record fee estimations and unregister
// fee estimator from validation interface. // fee estimator from validation interface.
if (node.fee_estimator) { if (node.fee_estimator) {
node.fee_estimator->Flush(); node.fee_estimator->Flush();

6
src/kernel/mempool_entry.h
View File

@@ -226,7 +226,7 @@ struct NewMempoolTransactionInfo {
* This boolean indicates whether the transaction was added * This boolean indicates whether the transaction was added
* without enforcing mempool fee limits. * without enforcing mempool fee limits.
*/ */
const bool m_from_disconnected_block; const bool m_mempool_limit_bypassed;
/* This boolean indicates whether the transaction is part of a package. */ /* This boolean indicates whether the transaction is part of a package. */
const bool m_submitted_in_package; const bool m_submitted_in_package;
/* /*
@@ -239,11 +239,11 @@ struct NewMempoolTransactionInfo {
explicit NewMempoolTransactionInfo(const CTransactionRef& tx, const CAmount& fee, explicit NewMempoolTransactionInfo(const CTransactionRef& tx, const CAmount& fee,
const int64_t vsize, const unsigned int height, const int64_t vsize, const unsigned int height,
const bool from_disconnected_block, const bool submitted_in_package, const bool mempool_limit_bypassed, const bool submitted_in_package,
const bool chainstate_is_current, const bool chainstate_is_current,
const bool has_no_mempool_parents) const bool has_no_mempool_parents)
: info{tx, fee, vsize, height}, : info{tx, fee, vsize, height},
m_from_disconnected_block{from_disconnected_block}, m_mempool_limit_bypassed{mempool_limit_bypassed},
m_submitted_in_package{submitted_in_package}, m_submitted_in_package{submitted_in_package},
m_chainstate_is_current{chainstate_is_current}, m_chainstate_is_current{chainstate_is_current},
m_has_no_mempool_parents{has_no_mempool_parents} {} m_has_no_mempool_parents{has_no_mempool_parents} {}

2
src/policy/fees.cpp
View File

@@ -612,7 +612,7 @@ void CBlockPolicyEstimator::processTransaction(const NewMempoolTransactionInfo&
// - the node is not behind // - the node is not behind
// - the transaction is not dependent on any other transactions in the mempool // - the transaction is not dependent on any other transactions in the mempool
// - it's not part of a package. // - it's not part of a package.
const bool validForFeeEstimation = !tx.m_from_disconnected_block && !tx.m_submitted_in_package && tx.m_chainstate_is_current && tx.m_has_no_mempool_parents; const bool validForFeeEstimation = !tx.m_mempool_limit_bypassed && !tx.m_submitted_in_package && tx.m_chainstate_is_current && tx.m_has_no_mempool_parents;
// Only want to be updating estimates when our blockchain is synced, // Only want to be updating estimates when our blockchain is synced,
// otherwise we'll miscalculate how many blocks its taking to get included. // otherwise we'll miscalculate how many blocks its taking to get included.

10
src/test/fuzz/policy_estimator.cpp
View File

@@ -45,12 +45,14 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
} }
const CTransaction tx{*mtx}; const CTransaction tx{*mtx};
const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx); const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx);
const auto tx_submitted_in_package = fuzzed_data_provider.ConsumeBool();
const auto tx_has_mempool_parents = fuzzed_data_provider.ConsumeBool();
const auto tx_info = NewMempoolTransactionInfo(entry.GetSharedTx(), entry.GetFee(), const auto tx_info = NewMempoolTransactionInfo(entry.GetSharedTx(), entry.GetFee(),
entry.GetTxSize(), entry.GetHeight(), entry.GetTxSize(), entry.GetHeight(),
/* m_from_disconnected_block */ false, /*mempool_limit_bypassed=*/false,
/* m_submitted_in_package */ false, tx_submitted_in_package,
/* m_chainstate_is_current */ true, /*chainstate_is_current=*/true,
/* m_has_no_mempool_parents */ fuzzed_data_provider.ConsumeBool()); tx_has_mempool_parents);
block_policy_estimator.processTransaction(tx_info); block_policy_estimator.processTransaction(tx_info);
if (fuzzed_data_provider.ConsumeBool()) { if (fuzzed_data_provider.ConsumeBool()) {
(void)block_policy_estimator.removeTx(tx.GetHash()); (void)block_policy_estimator.removeTx(tx.GetHash());

27
src/test/policyestimator_tests.cpp
View File

@@ -70,10 +70,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
feeV[j], feeV[j],
virtual_size, virtual_size,
entry.nHeight, entry.nHeight,
/* m_from_disconnected_block */ false, /*mempool_limit_bypassed=*/false,
/* m_submitted_in_package */ false, /*submitted_in_package=*/false,
/* m_chainstate_is_current */ true, /*chainstate_is_current=*/true,
/* m_has_no_mempool_parents */ true)}; /*has_no_mempool_parents=*/true)};
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
} }
uint256 hash = tx.GetHash(); uint256 hash = tx.GetHash();
@@ -112,6 +112,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
} }
} }
// Wait for fee estimator to catch up
SyncWithValidationInterfaceQueue();
std::vector<CAmount> origFeeEst; std::vector<CAmount> origFeeEst;
// Highest feerate is 10*baseRate and gets in all blocks, // Highest feerate is 10*baseRate and gets in all blocks,
// second highest feerate is 9*baseRate and gets in 9/10 blocks = 90%, // second highest feerate is 9*baseRate and gets in 9/10 blocks = 90%,
@@ -168,10 +171,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
feeV[j], feeV[j],
virtual_size, virtual_size,
entry.nHeight, entry.nHeight,
/* m_from_disconnected_block */ false, /*mempool_limit_bypassed=*/false,
/* m_submitted_in_package */ false, /*submitted_in_package=*/false,
/* m_chainstate_is_current */ true, /*chainstate_is_current=*/true,
/* m_has_no_mempool_parents */ true)}; /*has_no_mempool_parents=*/true)};
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
} }
uint256 hash = tx.GetHash(); uint256 hash = tx.GetHash();
@@ -232,10 +235,10 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
feeV[j], feeV[j],
virtual_size, virtual_size,
entry.nHeight, entry.nHeight,
/* m_from_disconnected_block */ false, /*mempool_limit_bypassed=*/false,
/* m_submitted_in_package */ false, /*submitted_in_package=*/false,
/* m_chainstate_is_current */ true, /*chainstate_is_current=*/true,
/* m_has_no_mempool_parents */ true)}; /*has_no_mempool_parents=*/true)};
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
} }
uint256 hash = tx.GetHash(); uint256 hash = tx.GetHash();

2
src/validationinterface.h
View File

@@ -150,7 +150,7 @@ protected:
virtual void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex) {} virtual void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex) {}
/** /**
* Notifies listeners of a block being disconnected * Notifies listeners of a block being disconnected
* Provides the block that was connected. * Provides the block that was disconnected.
* *
* Called on a background thread. Only called for the active chainstate, since * Called on a background thread. Only called for the active chainstate, since
* background chainstates should never disconnect blocks. * background chainstates should never disconnect blocks.