From 4d4e4c644826db03317d69a04fea03309c3ebabf Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 4 Mar 2019 15:57:58 -0500 Subject: [PATCH] Suggested interfaces::Chain cleanups from #15288 Mostly documentation improvements requested in the last review of #15288 before it was merged (https://github.com/bitcoin/bitcoin/pull/15288#pullrequestreview-210241864) --- src/interfaces/README.md | 4 ++-- src/interfaces/chain.cpp | 8 ++++---- src/interfaces/chain.h | 38 ++++++++++++++++++++++++++++++-------- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/interfaces/README.md b/src/interfaces/README.md index 57d41df74..f77d17215 100644 --- a/src/interfaces/README.md +++ b/src/interfaces/README.md @@ -2,9 +2,9 @@ The following interfaces are defined here: -* [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973). +* [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#14437](https://github.com/bitcoin/bitcoin/pull/14437), [#14711](https://github.com/bitcoin/bitcoin/pull/14711), [#15288](https://github.com/bitcoin/bitcoin/pull/15288), and [#10973](https://github.com/bitcoin/bitcoin/pull/10973). -* [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973). +* [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#14437](https://github.com/bitcoin/bitcoin/pull/14437). * [`Node`](node.h) — used by GUI to start & stop bitcoin node. Added in [#10244](https://github.com/bitcoin/bitcoin/pull/10244). diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index fb634e73d..2eecea28d 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -148,7 +148,7 @@ class LockImpl : public Chain::Lock LockAnnotation lock(::cs_main); return CheckFinalTx(tx); } - bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) override + bool submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state) override { LockAnnotation lock(::cs_main); return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */, nullptr /* txn replaced */, @@ -207,8 +207,8 @@ public: bool hasDescendantsInMempool(const uint256& txid) override { LOCK(::mempool.cs); - auto it_mp = ::mempool.mapTx.find(txid); - return it_mp != ::mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1; + auto it = ::mempool.GetIter(txid); + return it && (*it)->GetCountWithDescendants() > 1; } void relayTransaction(const uint256& txid) override { @@ -219,7 +219,7 @@ public: { ::mempool.GetTransactionAncestry(txid, ancestors, descendants); } - bool checkChainLimits(CTransactionRef tx) override + bool checkChainLimits(const CTransactionRef& tx) override { LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 60f8570e3..037e8e9ff 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -6,7 +6,6 @@ #define BITCOIN_INTERFACES_CHAIN_H #include // For Optional and nullopt -#include // For RBFTransactionState #include // For CTransactionRef #include @@ -16,9 +15,11 @@ #include class CBlock; +class CFeeRate; class CScheduler; class CValidationState; class uint256; +enum class RBFTransactionState; struct CBlockLocator; struct FeeCalculation; @@ -26,7 +27,27 @@ namespace interfaces { class Wallet; -//! Interface for giving wallet processes access to blockchain state. +//! Interface giving clients (wallet processes, maybe other analysis tools in +//! the future) ability to access to the chain state, receive notifications, +//! estimate fees, and submit transactions. +//! +//! TODO: Current chain methods are too low level, exposing too much of the +//! internal workings of the bitcoin node, and not being very convenient to use. +//! Chain methods should be cleaned up and simplified over time. Examples: +//! +//! * The Chain::lock() method, which lets clients delay chain tip updates +//! should be removed when clients are able to respond to updates +//! asynchronously +//! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269). +//! +//! * The relayTransactions() and submitToMemoryPool() methods could be replaced +//! with a higher-level broadcastTransaction method +//! (https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984). +//! +//! * The initMessages() and loadWallet() methods which the wallet uses to send +//! notifications to the GUI should go away when GUI and wallet can directly +//! communicate with each other without going through the node +//! (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096). class Chain { public: @@ -114,8 +135,9 @@ public: virtual bool checkFinalTx(const CTransaction& tx) = 0; //! Add transaction to memory pool if the transaction fee is below the - //! amount specified by absurd_fee (as a safeguard). */ - virtual bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) = 0; + //! amount specified by absurd_fee. Returns false if the transaction + //! could not be added due to the fee or for another reason. + virtual bool submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state) = 0; }; //! Return Lock interface. Chain is locked when this is called, and @@ -154,8 +176,8 @@ public: //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; - //! Check chain limits. - virtual bool checkChainLimits(CTransactionRef tx) = 0; + //! Check if transaction will pass the mempool's chain limits. + virtual bool checkChainLimits(const CTransactionRef& tx) = 0; //! Estimate smart fee. virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; @@ -163,10 +185,10 @@ public: //! Fee estimator max target. virtual unsigned int estimateMaxBlocks() = 0; - //! Pool min fee. + //! Mempool minimum fee. virtual CFeeRate mempoolMinFee() = 0; - //! Get node max tx fee setting (-maxtxfee). + //! Node max tx fee setting (-maxtxfee). //! This could be replaced by a per-wallet max fee, as proposed at //! https://github.com/bitcoin/bitcoin/issues/15355 //! But for the time being, wallets call this to access the node setting.