Skip to content

Commit 45f434f

Browse files
author
MarcoFalke
committed
Merge #15288: Remove wallet -> node global function calls
f7efd87 Change brace formatting (Russell Yanofsky) a1df1b4 Remove use of IsInitialBlockDownload in wallet code (Russell Yanofsky) 1106a6f Remove use of uiInterface.LoadWallet in wallet code (Russell Yanofsky) 318f41f circular-dependencies: Avoid treating some .h/.cpp files as a unit (Russell Yanofsky) d02b34c Remove use of AcceptToMemoryPool in wallet code (Russell Yanofsky) e2c8ba9 Remove uses of InitMessage/Warning/Error in wallet code (Russell Yanofsky) c5e59a9 Remove uses of GetAdjustedTime in wallet code (Russell Yanofsky) 6d6bcc7 Remove use of g_connman / PushInventory in wallet code (Russell Yanofsky) 00dfb2a Remove uses of g_connman in wallet code (Russell Yanofsky) cc3836e Remove uses of fPruneMode in wallet code (Russell Yanofsky) cc02c79 Remove uses of fee globals in wallet code (Russell Yanofsky) 1fb0a4a Remove use of CalculateMemPoolAncestors in wallet code (Russell Yanofsky) cd32160 Remove use of GetTransactionAncestry in wallet code (Russell Yanofsky) 291276f Remove use of GetCountWithDescendants in wallet code (Russell Yanofsky) bdc6628 Remove use of IsRBFOptIn in wallet code (Russell Yanofsky) 80f52a2 Remove uses of CheckFinalTx in wallet code (Russell Yanofsky) Pull request description: This change removes wallet calls to node functions that access global chain and mempool state. This is the next step in the larger #10973 refactoring change, which removes all other accesses to node global variables from wallet code. Doing this is useful to provide a better defined interface between the wallet and node, and necessary to allow wallet and node code to run in separate processes in #10102. Tree-SHA512: 40dbaf1f59fb22b32e70b054b30ba5638d638aa3240fa30e0f721d53c721cd6138a7ab4d423a24d7d2fda0b956e68d44c733abc2c9259c3d6c9fd6d4be89aa23
2 parents 57acfcb + f7efd87 commit 45f434f

21 files changed

+298
-156
lines changed

contrib/devtools/circular-dependencies.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,18 @@
88
'core_write.cpp': 'core_io.cpp',
99
}
1010

11+
# Directories with header-based modules, where the assumption that .cpp files
12+
# define functions and variables declared in corresponding .h files is
13+
# incorrect.
14+
HEADER_MODULE_PATHS = [
15+
'interfaces/'
16+
]
17+
1118
def module_name(path):
1219
if path in MAPPING:
1320
path = MAPPING[path]
21+
if any(path.startswith(dirpath) for dirpath in HEADER_MODULE_PATHS):
22+
return path
1423
if path.endswith(".h"):
1524
return path[:-2]
1625
if path.endswith(".c"):

src/Makefile.am

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,12 +568,14 @@ bitcoin_wallet_LDADD = \
568568
$(LIBBITCOIN_CONSENSUS) \
569569
$(LIBBITCOIN_UTIL) \
570570
$(LIBBITCOIN_CRYPTO) \
571+
$(LIBBITCOIN_ZMQ) \
571572
$(LIBLEVELDB) \
572573
$(LIBLEVELDB_SSE42) \
573574
$(LIBMEMENV) \
574-
$(LIBSECP256K1)
575+
$(LIBSECP256K1) \
576+
$(LIBUNIVALUE)
575577

576-
bitcoin_wallet_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS)
578+
bitcoin_wallet_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) $(ZMQ_LIBS)
577579
#
578580

579581
# bitcoinconsensus library #

src/Makefile.bench.include

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ if ENABLE_WALLET
5757
bench_bench_bitcoin_SOURCES += bench/coin_selection.cpp
5858
endif
5959

60-
bench_bench_bitcoin_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS)
60+
bench_bench_bitcoin_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS)
6161
bench_bench_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
6262

6363
CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES)

src/interfaces/chain.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,19 @@
66

77
#include <chain.h>
88
#include <chainparams.h>
9+
#include <interfaces/wallet.h>
10+
#include <net.h>
11+
#include <policy/fees.h>
12+
#include <policy/policy.h>
13+
#include <policy/rbf.h>
914
#include <primitives/block.h>
15+
#include <primitives/transaction.h>
16+
#include <protocol.h>
1017
#include <sync.h>
18+
#include <threadsafety.h>
19+
#include <timedata.h>
20+
#include <txmempool.h>
21+
#include <ui_interface.h>
1122
#include <uint256.h>
1223
#include <util/system.h>
1324
#include <validation.h>
@@ -132,6 +143,17 @@ class LockImpl : public Chain::Lock
132143
}
133144
return nullopt;
134145
}
146+
bool checkFinalTx(const CTransaction& tx) override
147+
{
148+
LockAnnotation lock(::cs_main);
149+
return CheckFinalTx(tx);
150+
}
151+
bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) override
152+
{
153+
LockAnnotation lock(::cs_main);
154+
return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */, nullptr /* txn replaced */,
155+
false /* bypass limits */, absurd_fee);
156+
}
135157
};
136158

137159
class LockingStateImpl : public LockImpl, public UniqueLock<CCriticalSection>
@@ -177,6 +199,61 @@ class ChainImpl : public Chain
177199
LOCK(cs_main);
178200
return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash));
179201
}
202+
RBFTransactionState isRBFOptIn(const CTransaction& tx) override
203+
{
204+
LOCK(::mempool.cs);
205+
return IsRBFOptIn(tx, ::mempool);
206+
}
207+
bool hasDescendantsInMempool(const uint256& txid) override
208+
{
209+
LOCK(::mempool.cs);
210+
auto it_mp = ::mempool.mapTx.find(txid);
211+
return it_mp != ::mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1;
212+
}
213+
void relayTransaction(const uint256& txid) override
214+
{
215+
CInv inv(MSG_TX, txid);
216+
g_connman->ForEachNode([&inv](CNode* node) { node->PushInventory(inv); });
217+
}
218+
void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override
219+
{
220+
::mempool.GetTransactionAncestry(txid, ancestors, descendants);
221+
}
222+
bool checkChainLimits(CTransactionRef tx) override
223+
{
224+
LockPoints lp;
225+
CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp);
226+
CTxMemPool::setEntries ancestors;
227+
auto limit_ancestor_count = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
228+
auto limit_ancestor_size = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT) * 1000;
229+
auto limit_descendant_count = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT);
230+
auto limit_descendant_size = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT) * 1000;
231+
std::string unused_error_string;
232+
LOCK(::mempool.cs);
233+
return ::mempool.CalculateMemPoolAncestors(entry, ancestors, limit_ancestor_count, limit_ancestor_size,
234+
limit_descendant_count, limit_descendant_size, unused_error_string);
235+
}
236+
CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override
237+
{
238+
return ::feeEstimator.estimateSmartFee(num_blocks, calc, conservative);
239+
}
240+
unsigned int estimateMaxBlocks() override
241+
{
242+
return ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
243+
}
244+
CFeeRate mempoolMinFee() override
245+
{
246+
return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
247+
}
248+
CAmount maxTxFee() override { return ::maxTxFee; }
249+
bool getPruneMode() override { return ::fPruneMode; }
250+
bool p2pEnabled() override { return g_connman != nullptr; }
251+
bool isInitialBlockDownload() override { return IsInitialBlockDownload(); }
252+
int64_t getAdjustedTime() override { return GetAdjustedTime(); }
253+
void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); }
254+
void initWarning(const std::string& message) override { InitWarning(message); }
255+
void initError(const std::string& message) override { InitError(message); }
256+
void loadWallet(std::unique_ptr<Wallet> wallet) override { ::uiInterface.LoadWallet(wallet); }
180257
};
181258

182259
} // namespace

src/interfaces/chain.h

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,27 @@
55
#ifndef BITCOIN_INTERFACES_CHAIN_H
66
#define BITCOIN_INTERFACES_CHAIN_H
77

8-
#include <optional.h>
8+
#include <optional.h> // For Optional and nullopt
9+
#include <policy/rbf.h> // For RBFTransactionState
10+
#include <primitives/transaction.h> // For CTransactionRef
911

1012
#include <memory>
13+
#include <stddef.h>
1114
#include <stdint.h>
1215
#include <string>
1316
#include <vector>
1417

1518
class CBlock;
1619
class CScheduler;
20+
class CValidationState;
1721
class uint256;
1822
struct CBlockLocator;
23+
struct FeeCalculation;
1924

2025
namespace interfaces {
2126

27+
class Wallet;
28+
2229
//! Interface for giving wallet processes access to blockchain state.
2330
class Chain
2431
{
@@ -102,6 +109,13 @@ class Chain
102109
//! is guaranteed to be an ancestor of the block used to create the
103110
//! locator.
104111
virtual Optional<int> findLocatorFork(const CBlockLocator& locator) = 0;
112+
113+
//! Check if transaction will be final given chain height current time.
114+
virtual bool checkFinalTx(const CTransaction& tx) = 0;
115+
116+
//! Add transaction to memory pool if the transaction fee is below the
117+
//! amount specified by absurd_fee (as a safeguard). */
118+
virtual bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) = 0;
105119
};
106120

107121
//! Return Lock interface. Chain is locked when this is called, and
@@ -127,6 +141,60 @@ class Chain
127141
//! Estimate fraction of total transactions verified if blocks up to
128142
//! the specified block hash are verified.
129143
virtual double guessVerificationProgress(const uint256& block_hash) = 0;
144+
145+
//! Check if transaction is RBF opt in.
146+
virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0;
147+
148+
//! Check if transaction has descendants in mempool.
149+
virtual bool hasDescendantsInMempool(const uint256& txid) = 0;
150+
151+
//! Relay transaction.
152+
virtual void relayTransaction(const uint256& txid) = 0;
153+
154+
//! Calculate mempool ancestor and descendant counts for the given transaction.
155+
virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0;
156+
157+
//! Check chain limits.
158+
virtual bool checkChainLimits(CTransactionRef tx) = 0;
159+
160+
//! Estimate smart fee.
161+
virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0;
162+
163+
//! Fee estimator max target.
164+
virtual unsigned int estimateMaxBlocks() = 0;
165+
166+
//! Pool min fee.
167+
virtual CFeeRate mempoolMinFee() = 0;
168+
169+
//! Get node max tx fee setting (-maxtxfee).
170+
//! This could be replaced by a per-wallet max fee, as proposed at
171+
//! https://github.com/bitcoin/bitcoin/issues/15355
172+
//! But for the time being, wallets call this to access the node setting.
173+
virtual CAmount maxTxFee() = 0;
174+
175+
//! Check if pruning is enabled.
176+
virtual bool getPruneMode() = 0;
177+
178+
//! Check if p2p enabled.
179+
virtual bool p2pEnabled() = 0;
180+
181+
// Check if in IBD.
182+
virtual bool isInitialBlockDownload() = 0;
183+
184+
//! Get adjusted time.
185+
virtual int64_t getAdjustedTime() = 0;
186+
187+
//! Send init message.
188+
virtual void initMessage(const std::string& message) = 0;
189+
190+
//! Send init warning.
191+
virtual void initWarning(const std::string& message) = 0;
192+
193+
//! Send init error.
194+
virtual void initError(const std::string& message) = 0;
195+
196+
//! Send wallet load notification to the GUI.
197+
virtual void loadWallet(std::unique_ptr<Wallet> wallet) = 0;
130198
};
131199

132200
//! Interface to let node manage chain clients (wallets, or maybe tools for

src/interfaces/node.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class NodeImpl : public Node
275275
}
276276
std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override
277277
{
278-
return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::shared_ptr<CWallet> wallet) { fn(MakeWallet(wallet)); }));
278+
return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::unique_ptr<Wallet>& wallet) { fn(std::move(wallet)); }));
279279
}
280280
std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override
281281
{

src/interfaces/wallet.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class PendingWalletTxImpl : public PendingWalletTx
5656
auto locked_chain = m_wallet.chain().lock();
5757
LOCK(m_wallet.cs_wallet);
5858
CValidationState state;
59-
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, g_connman.get(), state)) {
59+
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, state)) {
6060
reject_reason = state.GetRejectReason();
6161
return false;
6262
}
@@ -99,7 +99,7 @@ WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, co
9999
//! Construct wallet tx status struct.
100100
WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx)
101101
{
102-
LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit.
102+
LockAnnotation lock(::cs_main); // Temporary, for mapBlockIndex below. Removed in upcoming commit.
103103

104104
WalletTxStatus result;
105105
auto mi = ::mapBlockIndex.find(wtx.hashBlock);
@@ -109,7 +109,7 @@ WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const C
109109
result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain);
110110
result.time_received = wtx.nTimeReceived;
111111
result.lock_time = wtx.tx->nLockTime;
112-
result.is_final = CheckFinalTx(*wtx.tx);
112+
result.is_final = locked_chain.checkFinalTx(*wtx.tx);
113113
result.is_trusted = wtx.IsTrusted(locked_chain);
114114
result.is_abandoned = wtx.isAbandoned();
115115
result.is_coinbase = wtx.IsCoinBase();
@@ -457,7 +457,7 @@ class WalletImpl : public Wallet
457457
{
458458
FeeCalculation fee_calc;
459459
CAmount result;
460-
result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, ::mempool, ::feeEstimator, &fee_calc);
460+
result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, &fee_calc);
461461
if (returned_target) *returned_target = fee_calc.returnedTarget;
462462
if (reason) *reason = fee_calc.reason;
463463
return result;

src/rpc/mining.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,8 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request)
843843

844844
RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR});
845845
RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
846-
unsigned int conf_target = ParseConfirmTarget(request.params[0]);
846+
unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
847+
unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
847848
bool conservative = true;
848849
if (!request.params[1].isNull()) {
849850
FeeEstimateMode fee_mode;
@@ -915,7 +916,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request)
915916

916917
RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true);
917918
RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
918-
unsigned int conf_target = ParseConfirmTarget(request.params[0]);
919+
unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
920+
unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
919921
double threshold = 0.95;
920922
if (!request.params[1].isNull()) {
921923
threshold = request.params[1].get_real();

src/rpc/util.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44

55
#include <key_io.h>
66
#include <keystore.h>
7-
#include <policy/fees.h>
87
#include <rpc/util.h>
98
#include <tinyformat.h>
109
#include <util/strencodings.h>
11-
#include <validation.h>
1210

1311
InitInterfaces* g_rpc_interfaces = nullptr;
1412

@@ -130,10 +128,9 @@ UniValue DescribeAddress(const CTxDestination& dest)
130128
return boost::apply_visitor(DescribeAddressVisitor(), dest);
131129
}
132130

133-
unsigned int ParseConfirmTarget(const UniValue& value)
131+
unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target)
134132
{
135133
int target = value.get_int();
136-
unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
137134
if (target < 1 || (unsigned int)target > max_target) {
138135
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u - %u", 1, max_target));
139136
}

src/rpc/util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey
3333
UniValue DescribeAddress(const CTxDestination& dest);
3434

3535
//! Parse a confirm target option and raise an RPC error if it is invalid.
36-
unsigned int ParseConfirmTarget(const UniValue& value);
36+
unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target);
3737

3838
RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
3939
UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = "");

0 commit comments

Comments
 (0)