From fa831684e54783f6b40533ca218eb7636bdae667 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 19 Jul 2020 11:09:59 +0200 Subject: [PATCH 1/3] refactor: Add IsRBFOptInEmptyMempool Co-authored-by: John Newbery --- src/interfaces/chain.cpp | 5 +++-- src/policy/rbf.cpp | 6 ++++++ src/policy/rbf.h | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 313c1265dec..19de8ab202f 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -276,8 +276,9 @@ public: } RBFTransactionState isRBFOptIn(const CTransaction& tx) override { - LOCK(::mempool.cs); - return IsRBFOptIn(tx, ::mempool); + if (!m_node.mempool) return IsRBFOptInEmptyMempool(tx); + LOCK(m_node.mempool->cs); + return IsRBFOptIn(tx, *m_node.mempool); } bool hasDescendantsInMempool(const uint256& txid) override { diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index f8b17d18d52..4b559348919 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -36,3 +36,9 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) } return RBFTransactionState::FINAL; } + +RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx) +{ + // If we don't have a local mempool we can only check the transaction itself. + return SignalsOptInRBF(tx) ? RBFTransactionState::REPLACEABLE_BIP125 : RBFTransactionState::UNKNOWN; +} diff --git a/src/policy/rbf.h b/src/policy/rbf.h index d335fbbb367..463c5b61e68 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -18,5 +18,6 @@ enum class RBFTransactionState { // This involves checking sequence numbers of the transaction, as well // as the sequence numbers of all in-mempool ancestors. RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs); +RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx); #endif // BITCOIN_POLICY_RBF_H From faef4fc9b4990e563022b6ab595cb02c4060c216 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 19 Jul 2020 15:04:29 +0200 Subject: [PATCH 2/3] Remove mempool global from interfaces --- src/interfaces/chain.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 19de8ab202f..13d885a20d7 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -282,8 +282,9 @@ public: } bool hasDescendantsInMempool(const uint256& txid) override { - LOCK(::mempool.cs); - auto it = ::mempool.GetIter(txid); + if (!m_node.mempool) return false; + LOCK(m_node.mempool->cs); + auto it = m_node.mempool->GetIter(txid); return it && (*it)->GetCountWithDescendants() > 1; } bool broadcastTransaction(const CTransactionRef& tx, @@ -299,7 +300,9 @@ public: } void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override { - ::mempool.GetTransactionAncestry(txid, ancestors, descendants); + ancestors = descendants = 0; + if (!m_node.mempool) return; + m_node.mempool->GetTransactionAncestry(txid, ancestors, descendants); } void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) override { @@ -308,6 +311,7 @@ public: } bool checkChainLimits(const CTransactionRef& tx) override { + if (!m_node.mempool) return true; LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries ancestors; @@ -316,8 +320,9 @@ public: auto limit_descendant_count = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); auto limit_descendant_size = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT) * 1000; std::string unused_error_string; - LOCK(::mempool.cs); - return ::mempool.CalculateMemPoolAncestors(entry, ancestors, limit_ancestor_count, limit_ancestor_size, + LOCK(m_node.mempool->cs); + return m_node.mempool->CalculateMemPoolAncestors( + entry, ancestors, limit_ancestor_count, limit_ancestor_size, limit_descendant_count, limit_descendant_size, unused_error_string); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override @@ -330,7 +335,8 @@ public: } CFeeRate mempoolMinFee() override { - return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + if (!m_node.mempool) return {}; + return m_node.mempool->GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } CFeeRate relayMinFee() override { return ::minRelayTxFee; } CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; } @@ -396,8 +402,9 @@ public: } void requestMempoolTransactions(Notifications& notifications) override { - LOCK2(::cs_main, ::mempool.cs); - for (const CTxMemPoolEntry& entry : ::mempool.mapTx) { + if (!m_node.mempool) return; + LOCK2(::cs_main, m_node.mempool->cs); + for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) { notifications.transactionAddedToMempool(entry.GetSharedTx()); } } From fa9ee52556f493e4a896e2570ca1a3102d777d9a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sun, 19 Jul 2020 11:01:05 +0200 Subject: [PATCH 3/3] doc: Add doxygen comment to IsRBFOptIn --- src/policy/rbf.h | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/policy/rbf.h b/src/policy/rbf.h index 463c5b61e68..f84e6e5286f 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -7,16 +7,27 @@ #include +/** The rbf state of unconfirmed transactions */ enum class RBFTransactionState { + /** Unconfirmed tx that does not signal rbf and is not in the mempool */ UNKNOWN, + /** Either this tx or a mempool ancestor signals rbf */ REPLACEABLE_BIP125, - FINAL + /** Neither this tx nor a mempool ancestor signals rbf */ + FINAL, }; -// Determine whether an in-mempool transaction is signaling opt-in to RBF -// according to BIP 125 -// This involves checking sequence numbers of the transaction, as well -// as the sequence numbers of all in-mempool ancestors. +/** + * Determine whether an unconfirmed transaction is signaling opt-in to RBF + * according to BIP 125 + * This involves checking sequence numbers of the transaction, as well + * as the sequence numbers of all in-mempool ancestors. + * + * @param tx The unconfirmed transaction + * @param pool The mempool, which may contain the tx + * + * @return The rbf state + */ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs); RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx);