foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge 6db2961f60 into 85f96b01b7

Browse source
This commit is contained in:
Oghenovo Usiwoma 2025-01-31 21:49:29 +01:00 • committed by GitHub
commit eefd771d70
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 248 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/interfaces/chain.h
View file

@ -7,6 +7,7 @@
#include <blockfilter.h>
#include <common/settings.h>
#include <kernel/mempool_removal_reason.h>
#include <primitives/transaction.h> // For CTransactionRef
#include <util/result.h>
@ -26,7 +27,6 @@ class CRPCCommand;
class CScheduler;
class Coin;
class uint256;
enum class MemPoolRemovalReason;
enum class RBFTransactionState;
enum class ChainstateRole;
struct bilingual_str;
@ -319,7 +319,7 @@ public:
public:
virtual ~Notifications() = default;
virtual void transactionAddedToMempool(const CTransactionRef& tx) {}
virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}
virtual void transactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason) {}
virtual void blockConnected(ChainstateRole role, const BlockInfo& block) {}
virtual void blockDisconnected(const BlockInfo& block) {}
virtual void updatedBlockTip() {}

13
src/kernel/mempool_removal_reason.cpp
View file

@ -9,13 +9,8 @@
std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept
{
switch (r) {
case MemPoolRemovalReason::EXPIRY: return "expiry";
case MemPoolRemovalReason::SIZELIMIT: return "sizelimit";
case MemPoolRemovalReason::REORG: return "reorg";
case MemPoolRemovalReason::BLOCK: return "block";
case MemPoolRemovalReason::CONFLICT: return "conflict";
case MemPoolRemovalReason::REPLACED: return "replaced";
}
assert(false);
return std::visit([](auto&& arg) {
return arg.toString();
},
r);
}

71
src/kernel/mempool_removal_reason.h
View file

@ -5,20 +5,77 @@
#ifndef BITCOIN_KERNEL_MEMPOOL_REMOVAL_REASON_H
#define BITCOIN_KERNEL_MEMPOOL_REMOVAL_REASON_H
#include <primitives/transaction.h>
#include <uint256.h>
#include <string>
#include <variant>
/** Reason why a transaction was removed from the mempool,
* this is passed to the notification signal.
*/
enum class MemPoolRemovalReason {
EXPIRY, //!< Expired from mempool
SIZELIMIT, //!< Removed in size limiting
REORG, //!< Removed for reorganization
BLOCK, //!< Removed for block
CONFLICT, //!< Removed for conflict with in-block transaction
REPLACED, //!< Removed for replacement
struct ExpiryReason {
std::string toString() const noexcept
{
return "expiry";
}
};
struct SizeLimitReason {
std::string toString() const noexcept
{
return "sizelimit";
}
};
struct ReorgReason {
std::string toString() const noexcept
{
return "reorg";
}
};
struct BlockReason {
std::string toString() const noexcept
{
return "block";
}
};
struct ConflictReason {
uint256 conflicting_block_hash;
unsigned int conflicting_block_height;
explicit ConflictReason(const uint256& conflicting_block_hash, int conflicting_block_height) : conflicting_block_hash(conflicting_block_hash), conflicting_block_height(conflicting_block_height) {}
ConflictReason(std::nullptr_t, int conflicting_block_height) = delete;
std::string toString() const noexcept
{
return "conflict";
}
};
struct ReplacedReason {
std::optional<CTransactionRef> replacement_tx;
explicit ReplacedReason(const std::optional<CTransactionRef> replacement_tx) : replacement_tx(replacement_tx) {}
ReplacedReason(std::nullptr_t) = delete;
std::string toString() const noexcept
{
return "replaced";
}
};
using MemPoolRemovalReason = std::variant<ExpiryReason, SizeLimitReason, ReorgReason, BlockReason, ConflictReason, ReplacedReason>;
std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept;
template <typename T>
bool IsReason(const MemPoolRemovalReason& reason)
{
return std::get_if<T>(&reason) != nullptr;
}
#endif // BITCOIN_KERNEL_MEMPOOL_REMOVAL_REASON_H

2
src/node/interfaces.cpp
View file

@ -459,7 +459,7 @@ public:
{
m_notifications->transactionAddedToMempool(tx.info.m_tx);
}
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override
void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t mempool_sequence) override
{
m_notifications->transactionRemovedFromMempool(tx, reason);
}

2
src/policy/fees.cpp
View file

@ -582,7 +582,7 @@ void CBlockPolicyEstimator::TransactionAddedToMempool(const NewMempoolTransactio
processTransaction(tx);
}
void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/)
void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& /*unused*/, uint64_t /*unused*/)
{
removeTx(tx->GetHash());
}

2
src/policy/fees.h
View file

@ -266,7 +266,7 @@ protected:
/** Overridden from CValidationInterface. */
void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) override
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) override
void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& /*unused*/, uint64_t /*unused*/) override
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) override
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);

3
src/test/blockencodings_tests.cpp
View file

@ -6,6 +6,7 @@
#include <chainparams.h>
#include <consensus/merkle.h>
#include <pow.h>
#include <primitives/transaction.h>
#include <streams.h>
#include <test/util/random.h>
#include <test/util/txmempool.h>
@ -89,7 +90,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1);
size_t poolSize = pool.size();
pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::REPLACED);
pool.removeRecursive(*block.vtx[2], ReplacedReason(MakeTransactionRef(CTransaction(CMutableTransaction()))));
BOOST_CHECK_EQUAL(pool.size(), poolSize - 1);
CBlock block2;

4
src/test/fuzz/package_eval.cpp
View file

@ -72,7 +72,7 @@ struct OutpointsUpdater final : public CValidationInterface {
}
}
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override
void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t /* mempool_sequence */) override
{
// outpoints spent by this tx are now available
for (const auto& input : tx->vin) {
@ -98,7 +98,7 @@ struct TransactionsDelta final : public CValidationInterface {
m_added.insert(tx.info.m_tx);
}
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override
void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t /* mempool_sequence */) override
{
// Transactions may be entered and booted any number of times
m_added.erase(tx);

4
src/test/fuzz/tx_pool.cpp
View file

@ -71,7 +71,7 @@ struct TransactionsDelta final : public CValidationInterface {
Assert(m_added.insert(tx.info.m_tx).second);
}
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override
void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t /* mempool_sequence */) override
{
Assert(m_removed.insert(tx).second);
}
@ -107,7 +107,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, Cha
const auto info_all = tx_pool.infoAll();
if (!info_all.empty()) {
const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx;
WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, MemPoolRemovalReason::BLOCK /* dummy */));
WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, BlockReason{} /* dummy */));
assert(tx_pool.size() < info_all.size());
WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1));
}

8
src/test/mempool_tests.cpp
View file

@ -4,8 +4,10 @@
#include <common/system.h>
#include <policy/policy.h>
#include <primitives/transaction.h>
#include <test/util/txmempool.h>
#include <txmempool.h>
#include <uint256.h>
#include <util/time.h>
#include <test/util/setup_common.h>
@ -15,7 +17,7 @@
BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup)
static constexpr auto REMOVAL_REASON_DUMMY = MemPoolRemovalReason::REPLACED;
static const auto REMOVAL_REASON_DUMMY = ReplacedReason(MakeTransactionRef(CTransaction(CMutableTransaction())));
class MemPoolTest final : public CTxMemPool
{
@ -401,7 +403,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
/* after tx6 is mined, tx7 should move up in the sort */
std::vector<CTransactionRef> vtx;
vtx.push_back(MakeTransactionRef(tx6));
pool.removeForBlock(vtx, 1);
pool.removeForBlock(vtx, uint256::ZERO, 1);
sortedOrder.erase(sortedOrder.begin()+1);
// Ties are broken by hash
@ -561,7 +563,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
// ... we should keep the same min fee until we get a block
pool.removeForBlock(vtx, 1);
pool.removeForBlock(vtx, uint256::ZERO, 1);
SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/2.0));
// ... then feerate should drop 1/2 each halflife

2
src/test/miner_tests.cpp
View file

@ -192,7 +192,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
// Test that packages above the min relay fee do get included, even if one
// of the transactions is below the min relay fee
// Remove the low fee transaction and replace with a higher fee transaction
tx_mempool.removeRecursive(CTransaction(tx), MemPoolRemovalReason::REPLACED);
tx_mempool.removeRecursive(CTransaction(tx), ReplacedReason(MakeTransactionRef(CTransaction(CMutableTransaction()))));
tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee
hashLowFeeTx = tx.GetHash();
AddToMempool(tx_mempool, entry.Fee(feeToUse + 2).FromTx(tx));

10
src/test/policyestimator_tests.cpp
View file

@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
{
LOCK(mpool.cs);
mpool.removeForBlock(block, ++blocknum);
mpool.removeForBlock(block, uint256::ZERO, ++blocknum);
}
block.clear();
@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
// We haven't decayed the moving average enough so we still have enough data points in every bucket
while (blocknum < 250) {
LOCK(mpool.cs);
mpool.removeForBlock(block, ++blocknum);
mpool.removeForBlock(block, uint256::ZERO, ++blocknum);
}
// Wait for fee estimator to catch up
@ -184,7 +184,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
}
{
LOCK(mpool.cs);
mpool.removeForBlock(block, ++blocknum);
mpool.removeForBlock(block, uint256::ZERO, ++blocknum);
}
}
@ -208,7 +208,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
{
LOCK(mpool.cs);
mpool.removeForBlock(block, 266);
mpool.removeForBlock(block, uint256::ZERO, 266);
}
block.clear();
@ -252,7 +252,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
{
LOCK(mpool.cs);
mpool.removeForBlock(block, ++blocknum);
mpool.removeForBlock(block, uint256::ZERO, ++blocknum);
}
block.clear();

4
src/test/txvalidationcache_tests.cpp
View file

@ -81,7 +81,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
BOOST_CHECK(m_node.chainman->ActiveChain().Tip()->GetBlockHash() != block.GetHash());
}
BOOST_CHECK_EQUAL(m_node.mempool->size(), 1U);
WITH_LOCK(m_node.mempool->cs, m_node.mempool->removeRecursive(CTransaction{spends[0]}, MemPoolRemovalReason::CONFLICT));
WITH_LOCK(m_node.mempool->cs, m_node.mempool->removeRecursive(CTransaction{spends[0]}, ConflictReason(uint256::ZERO, 1)));
BOOST_CHECK_EQUAL(m_node.mempool->size(), 0U);
// Test 3: ... and should be rejected if spend2 is in the memory pool
@ -92,7 +92,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
BOOST_CHECK(m_node.chainman->ActiveChain().Tip()->GetBlockHash() != block.GetHash());
}
BOOST_CHECK_EQUAL(m_node.mempool->size(), 1U);
WITH_LOCK(m_node.mempool->cs, m_node.mempool->removeRecursive(CTransaction{spends[1]}, MemPoolRemovalReason::CONFLICT));
WITH_LOCK(m_node.mempool->cs, m_node.mempool->removeRecursive(CTransaction{spends[1]}, ConflictReason(uint256::ZERO, 1)));
BOOST_CHECK_EQUAL(m_node.mempool->size(), 0U);
// Final sanity test: first spend in *m_node.mempool, second in block, that's OK:

37
src/txmempool.cpp
View file

@ -155,7 +155,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes
// This txid may have been removed already in a prior call to removeRecursive.
// Therefore we ensure it is not yet removed already.
if (const std::optional<txiter> txiter = GetIter(txid)) {
removeRecursive((*txiter)->GetTx(), MemPoolRemovalReason::SIZELIMIT);
removeRecursive((*txiter)->GetTx(), SizeLimitReason{});
}
}
}
@ -435,7 +435,15 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n)
void CTxMemPool::Apply(ChangeSet* changeset)
{
AssertLockHeld(cs);
RemoveStaged(changeset->m_to_remove, false, MemPoolRemovalReason::REPLACED);
if (!changeset->m_entry_vec.empty()) {
// The replacement_tx is the transaction added in this changeset or,
// in the case of packages, the last child in the package added in this changeset
const auto replacement_tx = changeset->m_entry_vec[changeset->m_entry_vec.size() - 1]->GetSharedTx();
RemoveStaged(changeset->m_to_remove, false, ReplacedReason(replacement_tx));
} else {
Assume(false);
RemoveStaged(changeset->m_to_remove, false, ReplacedReason(std::nullopt));
}
for (size_t i=0; i<changeset->m_entry_vec.size(); ++i) {
auto tx_entry = changeset->m_entry_vec[i];
@ -517,13 +525,13 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit, CTxMemPool::setEntr
);
}
void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
void CTxMemPool::removeUnchecked(txiter it, const MemPoolRemovalReason& reason)
{
// We increment mempool sequence value no matter removal reason
// even if not directly reported below.
uint64_t mempool_sequence = GetAndIncrementSequence();
if (reason != MemPoolRemovalReason::BLOCK && m_opts.signals) {
if (!IsReason<BlockReason>(reason) && m_opts.signals) {
// Notify clients that a transaction has been removed from the mempool
// for any reason except being included in a block. Clients interested
// in transactions included in blocks can subscribe to the BlockConnected
@ -592,7 +600,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants
}
}
void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason)
void CTxMemPool::removeRecursive(const CTransaction& origTx, const MemPoolRemovalReason& reason)
{
// Remove transaction from memory pool
AssertLockHeld(cs);
@ -638,13 +646,13 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check
for (txiter it : txToRemove) {
CalculateDescendants(it, setAllRemoves);
}
RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);
RemoveStaged(setAllRemoves, false, ReorgReason{});
for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
assert(TestLockPointValidity(chain, it->GetLockPoints()));
}
}
void CTxMemPool::removeConflicts(const CTransaction &tx)
void CTxMemPool::removeConflicts(const CTransaction& tx, const uint256& blockHash, unsigned int nBlockHeight)
{
// Remove transactions which depend on inputs of tx, recursively
AssertLockHeld(cs);
@ -655,7 +663,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
if (txConflict != tx)
{
ClearPrioritisation(txConflict.GetHash());
removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT);
removeRecursive(txConflict, ConflictReason(blockHash, nBlockHeight));
}
}
}
@ -664,7 +672,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
/**
* Called when a block is connected. Removes from mempool.
*/
void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, const uint256& blockHash, unsigned int nBlockHeight)
{
AssertLockHeld(cs);
Assume(!m_have_changeset);
@ -677,9 +685,9 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
setEntries stage;
stage.insert(it);
txs_removed_for_block.emplace_back(*it);
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
RemoveStaged(stage, true, BlockReason{});
}
removeConflicts(*tx);
removeConflicts(*tx, blockHash, nBlockHeight);
ClearPrioritisation(tx->GetHash());
}
if (m_opts.signals) {
@ -1075,7 +1083,8 @@ void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked)
}
}
void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) {
void CTxMemPool::RemoveStaged(setEntries& stage, bool updateDescendants, const MemPoolRemovalReason& reason)
{
AssertLockHeld(cs);
UpdateForRemoveFromMempool(stage, updateDescendants);
for (txiter it : stage) {
@ -1097,7 +1106,7 @@ int CTxMemPool::Expire(std::chrono::seconds time)
for (txiter removeit : toremove) {
CalculateDescendants(removeit, stage);
}
RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY);
RemoveStaged(stage, false, ExpiryReason{});
return stage.size();
}
@ -1183,7 +1192,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
for (txiter iter : stage)
txn.push_back(iter->GetTx());
}
RemoveStaged(stage, false, MemPoolRemovalReason::SIZELIMIT);
RemoveStaged(stage, false, SizeLimitReason{});
if (pvNoSpendsRemaining) {
for (const CTransaction& tx : txn) {
for (const CTxIn& txin : tx.vin) {

14
src/txmempool.h
View file

@ -18,10 +18,11 @@
#include <policy/packages.h>
#include <primitives/transaction.h>
#include <sync.h>
#include <uint256.h>
#include <util/epochguard.h>
#include <util/feefrac.h>
#include <util/hasher.h>
#include <util/result.h>
#include <util/feefrac.h>
#include <boost/multi_index/hashed_index.hpp>
#include <boost/multi_index/identity.hpp>
@ -454,7 +455,7 @@ public:
void check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeRecursive(const CTransaction& tx, const MemPoolRemovalReason& reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
/** After reorg, filter the entries that would no longer be valid in the next block, and update
* the entries' cached LockPoints if needed. The mempool does not have any knowledge of
* consensus rules. It just applies the callable function and removes the ones for which it
@ -463,8 +464,8 @@ public:
* and updates an entry's LockPoints.
* */
void removeForReorg(CChain& chain, std::function<bool(txiter)> filter_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeConflicts(const CTransaction& tx, const uint256& blockHash, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeForBlock(const std::vector<CTransactionRef>& vtx, const uint256& blockHash, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid=false);
bool isSpent(const COutPoint& outpoint) const;
@ -719,7 +720,7 @@ private:
* Set updateDescendants to true when removing a tx that was in a block, so
* that any in-mempool descendants have their ancestor state updated.
*/
void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
void RemoveStaged(setEntries& stage, bool updateDescendants, const MemPoolRemovalReason& reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
/** UpdateForDescendants is used by UpdateTransactionsFromBlock to update
* the descendants for a single transaction that has been added to the
@ -770,7 +771,8 @@ private:
* transactions in a chain before we've updated all the state for the
* removal.
*/
void removeUnchecked(txiter entry, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeUnchecked(txiter entry, const MemPoolRemovalReason& reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
public:
/** visited marks a CTxMemPoolEntry as having been traversed
* during the lifetime of the most recently created Epoch::Guard

6
src/validation.cpp
View file

@ -318,7 +318,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
MempoolAcceptResult::ResultType::VALID) {
// If the transaction doesn't make it in to the mempool, remove any
// transactions that depend on it (which would now be orphans).
m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG);
m_mempool->removeRecursive(**it, ReorgReason{});
} else if (m_mempool->exists(GenTxid::Txid((*it)->GetHash()))) {
vHashUpdate.push_back((*it)->GetHash());
}
@ -3107,7 +3107,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra
// Save transactions to re-add to mempool at end of reorg. If any entries are evicted for
// exceeding memory limits, remove them and their descendants from the mempool.
for (auto&& evicted_tx : disconnectpool->AddTransactionsFromBlock(block.vtx)) {
m_mempool->removeRecursive(*evicted_tx, MemPoolRemovalReason::REORG);
m_mempool->removeRecursive(*evicted_tx, ReorgReason{});
}
}
@ -3235,7 +3235,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
Ticks<MillisecondsDouble>(m_chainman.time_chainstate) / m_chainman.num_blocks_total);
// Remove conflicting transactions from the mempool.;
if (m_mempool) {
m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->GetBlockHash(), pindexNew->nHeight);
disconnectpool.removeForBlock(blockConnecting.vtx);
}
// Update m_chain & related variables.

3
src/validationinterface.cpp
View file

@ -198,7 +198,8 @@ void ValidationSignals::TransactionAddedToMempool(const NewMempoolTransactionInf
tx.info.m_tx->GetWitnessHash().ToString());
}
void ValidationSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {
void ValidationSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t mempool_sequence)
{
auto event = [tx, reason, mempool_sequence, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(tx, reason, mempool_sequence); });
};

6
src/validationinterface.h
View file

@ -8,6 +8,7 @@
#include <kernel/chain.h>
#include <kernel/cs_main.h>
#include <kernel/mempool_removal_reason.h>
#include <primitives/transaction.h> // CTransaction(Ref)
#include <sync.h>
@ -25,7 +26,6 @@ class BlockValidationState;
class CBlock;
class CBlockIndex;
struct CBlockLocator;
enum class MemPoolRemovalReason;
struct RemovedMempoolTransactionInfo;
struct NewMempoolTransactionInfo;
@ -104,7 +104,7 @@ protected:
*
* Called on a background thread.
*/
virtual void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {}
virtual void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t mempool_sequence) {}
/*
* Notifies listeners of transactions removed from the mempool as
* as a result of new block being connected.
@ -220,7 +220,7 @@ public:
void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
void ActiveTipChange(const CBlockIndex&, bool);
void TransactionAddedToMempool(const NewMempoolTransactionInfo&, uint64_t mempool_sequence);
void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence);
void TransactionRemovedFromMempool(const CTransactionRef&, const MemPoolRemovalReason&, uint64_t mempool_sequence);
void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>&, unsigned int nBlockHeight);
void BlockConnected(ChainstateRole, const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);

2
src/wallet/transaction.h
View file

@ -78,7 +78,7 @@ struct TxStateUnrecognized {
using TxState = std::variant<TxStateConfirmed, TxStateInMempool, TxStateBlockConflicted, TxStateInactive, TxStateUnrecognized>;
//! Subset of states transaction sync logic is implemented to handle.
using SyncTxState = std::variant<TxStateConfirmed, TxStateInMempool, TxStateInactive>;
using SyncTxState = std::variant<TxStateConfirmed, TxStateInMempool, TxStateInactive, TxStateBlockConflicted>;
//! Try to interpret deserialized TxStateUnrecognized data as a recognized state.
static inline TxState TxStateInterpretSerialized(TxStateUnrecognized data)

68
src/wallet/wallet.cpp
View file

@ -1248,6 +1248,18 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
}
}
if (auto* conf = std::get_if<TxStateBlockConflicted>(&state)) {
WalletLogPrintf("A transaction in block %s conflicts with wallet transaction %s\n", conf->conflicting_block_hash.ToString(), ptx->GetHash().ToString());
auto try_updating_state = [&](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
if (std::get_if<TxStateBlockConflicted>(&wtx.m_state)) return TxUpdate::UNCHANGED;
wtx.m_state = TxStateBlockConflicted(conf->conflicting_block_hash, conf->conflicting_block_height);
return TxUpdate::CHANGED;
};
// Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too.
RecursiveUpdateTxState(ptx->GetHash(), try_updating_state);
}
bool fExisted = mapWallet.count(tx.GetHash()) != 0;
if (fExisted && !fUpdate) return false;
if (fExisted || IsMine(tx) || IsFromMe(tx))
@ -1457,40 +1469,39 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx) {
}
}
void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {
void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason)
{
LOCK(cs_wallet);
auto it = mapWallet.find(tx->GetHash());
if (it != mapWallet.end()) {
RefreshMempoolStatus(it->second, chain());
}
auto* replaced_reason = std::get_if<ReplacedReason>(&reason);
// Check if wallet transaction is being replaced by a parent transaction which is not from this wallet
if (IsFromMe(*tx) && replaced_reason != nullptr && replaced_reason->replacement_tx.has_value() && !IsFromMe(*replaced_reason->replacement_tx.value())) {
m_unrelated_conflict_tx_watchlist.insert(std::make_pair(replaced_reason->replacement_tx.value()->GetHash(), tx));
}
auto iter = m_unrelated_conflict_tx_watchlist.find(tx->GetHash());
if (iter != m_unrelated_conflict_tx_watchlist.end()) {
// The replacement tx was removed from the mempool, remove it from map
// This new replacement tx may not conflict with the original tx
// so leave wallet tx to remain as TxStateInactive
m_unrelated_conflict_tx_watchlist.erase(iter);
}
auto* conflict_reason = std::get_if<ConflictReason>(&reason);
// Handle transactions that were removed from the mempool because they
// conflict with transactions in a newly connected block.
if (reason == MemPoolRemovalReason::CONFLICT) {
// Trigger external -walletnotify notifications for these transactions.
// Set Status::UNCONFIRMED instead of Status::CONFLICTED for a few reasons:
//
// 1. The transactionRemovedFromMempool callback does not currently
// provide the conflicting block's hash and height, and for backwards
// compatibility reasons it may not be not safe to store conflicted
// wallet transactions with a null block hash. See
// https://github.com/bitcoin/bitcoin/pull/18600#discussion_r420195993.
// 2. For most of these transactions, the wallet's internal conflict
// detection in the blockConnected handler will subsequently call
// MarkConflicted and update them with CONFLICTED status anyway. This
// applies to any wallet transaction that has inputs spent in the
// block, or that has ancestors in the wallet with inputs spent by
// the block.
// 3. Longstanding behavior since the sync implementation in
// https://github.com/bitcoin/bitcoin/pull/9371 and the prior sync
// implementation before that was to mark these transactions
// unconfirmed rather than conflicted.
//
// Nothing described above should be seen as an unchangeable requirement
// when improving this code in the future. The wallet's heuristics for
if (conflict_reason != nullptr && IsFromMe(*tx)) {
// The wallet's heuristics for
// distinguishing between conflicted and unconfirmed transactions are
// imperfect, and could be improved in general, see
// https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking
SyncTransaction(tx, TxStateInactive{});
SyncTransaction(tx, TxStateBlockConflicted(conflict_reason->conflicting_block_hash, conflict_reason->conflicting_block_height));
}
const Txid& txid = tx->GetHash();
@ -1526,8 +1537,15 @@ void CWallet::blockConnected(ChainstateRole role, const interfaces::BlockInfo& b
// Scan block
for (size_t index = 0; index < block.data->vtx.size(); index++) {
auto it = m_unrelated_conflict_tx_watchlist.find(block.data->vtx[index]->GetHash());
if (it != m_unrelated_conflict_tx_watchlist.end()) {
// A conflicting unrelated parent transaction was confirmed in a block
// Mark child wallet tx as conflicted
SyncTransaction(it->second, TxStateBlockConflicted(block.hash, block.height));
}
SyncTransaction(block.data->vtx[index], TxStateConfirmed{block.hash, block.height, static_cast<int>(index)});
transactionRemovedFromMempool(block.data->vtx[index], MemPoolRemovalReason::BLOCK);
transactionRemovedFromMempool(block.data->vtx[index], BlockReason{});
}
}

13
src/wallet/wallet.h
View file

@ -11,6 +11,7 @@
#include <interfaces/chain.h>
#include <interfaces/handler.h>
#include <kernel/cs_main.h>
#include <kernel/mempool_removal_reason.h>
#include <logging.h>
#include <outputtype.h>
#include <policy/feerate.h>
@ -56,7 +57,6 @@ class CKeyID;
class CPubKey;
class Coin;
class SigningProvider;
enum class MemPoolRemovalReason;
enum class SigningResult;
namespace common {
enum class PSBTError;
@ -428,6 +428,15 @@ private:
//! Cache of descriptor ScriptPubKeys used for IsMine. Maps ScriptPubKey to set of spkms
std::unordered_map<CScript, std::vector<ScriptPubKeyMan*>, SaltedSipHasher> m_cached_spks;
/**
* Tracks txids of txs that replace parents of wallet transactions.
* The key is the txid of the replacing transaction which may be unrelated to this wallet and
* the value is the replaced transaction which is related to this wallet.
* On blockConnected, the wallet checks the block for replacement txids stored in this map,
* if found, the wallet marks the child wallet tx as 'CONFLICTED'
*/
std::unordered_map<Txid, CTransactionRef, SaltedTxidHasher> m_unrelated_conflict_tx_watchlist GUARDED_BY(cs_wallet);
/**
* Catch wallet up to current chain, scanning new blocks, updating the best
* block locator and m_last_block_processed, and registering for
@ -631,7 +640,7 @@ public:
uint256 last_failed_block;
};
ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress);
void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override;
void transactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason) override;
/** Set the next time this wallet should resend transactions to 12-36 hours from now, ~1 day on average. */
void SetNextResend() { m_next_resend = GetDefaultNextResend(); }
/** Return true if all conditions for periodically resending transactions are met. */

2
src/zmq/zmqnotificationinterface.cpp
View file

@ -166,7 +166,7 @@ void CZMQNotificationInterface::TransactionAddedToMempool(const NewMempoolTransa
});
}
void CZMQNotificationInterface::TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason, uint64_t mempool_sequence)
void CZMQNotificationInterface::TransactionRemovedFromMempool(const CTransactionRef& ptx, const MemPoolRemovalReason& reason, uint64_t mempool_sequence)
{
// Called for all non-block inclusion reasons
const CTransaction& tx = *ptx;

2
src/zmq/zmqnotificationinterface.h
View file

@ -34,7 +34,7 @@ protected:
// CValidationInterface
void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence) override;
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override;
void TransactionRemovedFromMempool(const CTransactionRef& tx, const MemPoolRemovalReason& reason, uint64_t mempool_sequence) override;
void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override;
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected) override;
void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override;

61
test/functional/wallet_conflicts.py
View file

@ -9,9 +9,11 @@ Test that wallet correctly tracks transactions that have been conflicted by bloc
from decimal import Decimal
from test_framework.messages import COIN
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_equal,
find_vout_for_address,
)
class TxConflicts(BitcoinTestFramework):
@ -36,6 +38,7 @@ class TxConflicts(BitcoinTestFramework):
"""
self.test_block_conflicts()
self.test_unknown_parent_conflict()
self.test_mempool_conflict()
self.test_mempool_and_block_conflicts()
self.test_descendants_with_mempool_conflicts()
@ -422,5 +425,61 @@ class TxConflicts(BitcoinTestFramework):
bob.unloadwallet()
carol.unloadwallet()
def test_unknown_parent_conflict(self):
self.log.info("Test that a conflict with parent not belonging to the wallet causes in-wallet children to also conflict")
def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
self.nodes[0].createwallet("parent_conflict")
wallet = self.nodes[0].get_wallet_rpc("parent_conflict")
# two utxos to target wallet
addr = wallet.getnewaddress()
addr_desc = wallet.getaddressinfo(addr)["desc"]
def_wallet.sendtoaddress(addr, 10)
def_wallet.sendtoaddress(addr, 10)
self.generate(self.nodes[0], 1)
txids = []
for node in [self.nodes[0], self.nodes[1]]:
utxo = wallet.listunspent()[0]
# Make utxo in grandparent that will be double spent
gp_addr = def_wallet.getnewaddress()
gp_txid = def_wallet.sendtoaddress(gp_addr, 7)
gp_utxo = {"txid": gp_txid, "vout": find_vout_for_address(self.nodes[0], gp_txid, gp_addr)}
self.generate(self.nodes[0], 1)
# make unconfirmed parent tx
parent_addr = def_wallet.getnewaddress()
parent_txid = def_wallet.send(outputs=[{parent_addr: 5}], inputs=[gp_utxo])["txid"]
parent_utxo = {"txid": parent_txid, "vout": find_vout_for_address(self.nodes[0], parent_txid, parent_addr)}
parent_me = self.nodes[0].getmempoolentry(parent_txid)
parent_feerate = parent_me["fees"]["base"] * COIN / parent_me["vsize"]
self.nodes[1].sendrawtransaction(def_wallet.gettransaction(parent_txid)["hex"])
# make child tx that has one input belonging to target wallet, and one input not
psbt = def_wallet.walletcreatefundedpsbt(inputs=[utxo, parent_utxo], outputs=[{def_wallet.getnewaddress(): 13}], solving_data={"descriptors":[addr_desc]})["psbt"]
psbt = def_wallet.walletprocesspsbt(psbt)["psbt"]
psbt_proc = wallet.walletprocesspsbt(psbt)
psbt = psbt_proc["psbt"]
assert_equal(psbt_proc["complete"], True)
child_txid = self.nodes[0].sendrawtransaction(psbt_proc["hex"])
txids.append(child_txid)
self.nodes[1].sendrawtransaction(psbt_proc["hex"])
# Make a conflict spending parent
conflict_psbt = def_wallet.walletcreatefundedpsbt(inputs=[gp_utxo], outputs=[{def_wallet.getnewaddress(): 2}], fee_rate="{:.8f}".format(Decimal(parent_feerate*3)))["psbt"]
conflict_proc = def_wallet.walletprocesspsbt(conflict_psbt)
node.sendrawtransaction(conflict_proc["hex"])
self.generate(node, 1, sync_fun=self.no_op)
assert_equal(wallet.gettransaction(txids[0])["confirmations"], -3)
self.log.info("Test that receiving a block with a conflict with parent not belonging to the wallet causes in-wallet children to also conflict")
# Sync block with conflict tx
self.sync_blocks([self.nodes[0], self.nodes[1]])
assert_equal(wallet.gettransaction(txids[1])["confirmations"], -1)
if __name__ == '__main__':
TxConflicts(__file__).main()