Skip to content

Commit

Permalink
merge bitcoin#15288: Remove wallet -> node global function calls
Browse files Browse the repository at this point in the history
  • Loading branch information
kwvg authored and pravblockc committed Nov 18, 2021
1 parent 491608f commit d6f9339
Show file tree
Hide file tree
Showing 22 changed files with 267 additions and 134 deletions.
9 changes: 9 additions & 0 deletions contrib/devtools/circular-dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,18 @@
'core_write.cpp': 'core_io.cpp',
}

# Directories with header-based modules, where the assumption that .cpp files
# define functions and variables declared in corresponding .h files is
# incorrect.
HEADER_MODULE_PATHS = [
'interfaces/'
]

def module_name(path):
if path in MAPPING:
path = MAPPING[path]
if any(path.startswith(dirpath) for dirpath in HEADER_MODULE_PATHS):
return path
if path.endswith(".h"):
return path[:-2]
if path.endswith(".c"):
Expand Down
2 changes: 1 addition & 1 deletion src/Makefile.bench.include
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ bench_bench_dash_SOURCES += bench/coin_selection.cpp
bench_bench_dash_SOURCES += bench/wallet_balance.cpp
endif

bench_bench_dash_LDADD += $(BACKTRACE_LIB) $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(BLS_LIBS) $(GMP_LIBS)
bench_bench_dash_LDADD += $(BACKTRACE_LIB) $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(BLS_LIBS) $(GMP_LIBS)
bench_bench_dash_LDFLAGS = $(LDFLAGS_WRAP_EXCEPTIONS) $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)

CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES)
Expand Down
4 changes: 2 additions & 2 deletions src/coinjoin/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, con
tallyItem(tallyItemIn)
{
// Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not
coinControl.m_discard_feerate = ::GetDiscardRate(*pwallet.get(), ::feeEstimator);
coinControl.m_discard_feerate = ::GetDiscardRate(*pwallet.get());
// Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
coinControl.m_feerate = std::max(::feeEstimator.estimateSmartFee((int)pwallet->m_confirm_target, nullptr, true), pwallet->m_pay_tx_fee);
// Change always goes back to origin
Expand Down Expand Up @@ -307,7 +307,7 @@ bool CTransactionBuilder::Commit(std::string& strResult)
}

CValidationState state;
if (!pwallet->CommitTransaction(tx, {}, {}, dummyReserveKey, g_connman.get(), state)) {
if (!pwallet->CommitTransaction(tx, {}, {}, dummyReserveKey, state)) {
strResult = state.GetRejectReason();
return false;
}
Expand Down
72 changes: 72 additions & 0 deletions src/interfaces/chain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,19 @@

#include <chain.h>
#include <chainparams.h>
#include <coinjoin/coinjoin.h>
#include <interfaces/wallet.h>
#include <net.h>
#include <policy/fees.h>
#include <policy/policy.h>
#include <primitives/block.h>
#include <primitives/transaction.h>
#include <protocol.h>
#include <sync.h>
#include <threadsafety.h>
#include <timedata.h>
#include <txmempool.h>
#include <ui_interface.h>
#include <uint256.h>
#include <util/system.h>
#include <validation.h>
Expand Down Expand Up @@ -146,6 +157,17 @@ class LockImpl : public Chain::Lock
}
return nullopt;
}
bool checkFinalTx(const CTransaction& tx) override
{
LockAnnotation lock(::cs_main);
return CheckFinalTx(tx);
}
bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) override
{
LockAnnotation lock(::cs_main);
return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */,
false /* bypass limits */, absurd_fee);
}
};

class LockingStateImpl : public LockImpl, public UniqueLock<CCriticalSection>
Expand Down Expand Up @@ -191,6 +213,56 @@ class ChainImpl : public Chain
LOCK(cs_main);
return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash));
}
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;
}
void relayTransaction(const uint256& txid) override
{
CInv inv(CCoinJoin::GetDSTX(txid) ? MSG_DSTX : MSG_TX, txid);
g_connman->ForEachNode([&inv](CNode* node) { node->PushInventory(inv); });
}
void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override
{
::mempool.GetTransactionAncestry(txid, ancestors, descendants);
}
bool checkChainLimits(CTransactionRef tx) override
{
LockPoints lp;
CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp);
CTxMemPool::setEntries ancestors;
auto limit_ancestor_count = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
auto limit_ancestor_size = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT) * 1000;
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,
limit_descendant_count, limit_descendant_size, unused_error_string);
}
CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override
{
return ::feeEstimator.estimateSmartFee(num_blocks, calc, conservative);
}
unsigned int estimateMaxBlocks() override
{
return ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
}
CFeeRate mempoolMinFee() override
{
return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
}
CAmount maxTxFee() override { return ::maxTxFee; }
bool getPruneMode() override { return ::fPruneMode; }
bool p2pEnabled() override { return g_connman != nullptr; }
bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); }
int64_t getAdjustedTime() override { return GetAdjustedTime(); }
void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); }
void initWarning(const std::string& message) override { InitWarning(message); }
void initError(const std::string& message) override { InitError(message); }
void loadWallet(std::unique_ptr<Wallet> wallet) override { ::uiInterface.LoadWallet(wallet); }
};

} // namespace
Expand Down
69 changes: 68 additions & 1 deletion src/interfaces/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,29 @@
#ifndef BITCOIN_INTERFACES_CHAIN_H
#define BITCOIN_INTERFACES_CHAIN_H

#include <optional.h>
#include <optional.h> // For Optional and nullopt
#include <primitives/transaction.h> // For CTransactionRef

#include <memory>
#include <stddef.h>
#include <stdint.h>
#include <string>
#include <vector>

class CBlock;
class CScheduler;
class CValidationState;
class CFeeRate;
class uint256;
struct CBlockLocator;
struct FeeCalculation;

typedef std::shared_ptr<const CTransaction> CTransactionRef;

namespace interfaces {

class Wallet;

//! Interface for giving wallet processes access to blockchain state.
class Chain
{
Expand Down Expand Up @@ -102,6 +111,13 @@ class Chain
//! is guaranteed to be an ancestor of the block used to create the
//! locator.
virtual Optional<int> findLocatorFork(const CBlockLocator& locator) = 0;

//! Check if transaction will be final given chain height current time.
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;
};

//! Return Lock interface. Chain is locked when this is called, and
Expand All @@ -127,6 +143,57 @@ class Chain
//! Estimate fraction of total transactions verified if blocks up to
//! the specified block hash are verified.
virtual double guessVerificationProgress(const uint256& block_hash) = 0;

//! Check if transaction has descendants in mempool.
virtual bool hasDescendantsInMempool(const uint256& txid) = 0;

//! Relay transaction.
virtual void relayTransaction(const uint256& txid) = 0;

//! 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;

//! Estimate smart fee.
virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0;

//! Fee estimator max target.
virtual unsigned int estimateMaxBlocks() = 0;

//! Pool min fee.
virtual CFeeRate mempoolMinFee() = 0;

//! Get 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.
virtual CAmount maxTxFee() = 0;

//! Check if pruning is enabled.
virtual bool getPruneMode() = 0;

//! Check if p2p enabled.
virtual bool p2pEnabled() = 0;

// Check if in IBD.
virtual bool isInitialBlockDownload() = 0;

//! Get adjusted time.
virtual int64_t getAdjustedTime() = 0;

//! Send init message.
virtual void initMessage(const std::string& message) = 0;

//! Send init warning.
virtual void initWarning(const std::string& message) = 0;

//! Send init error.
virtual void initError(const std::string& message) = 0;

//! Send wallet load notification to the GUI.
virtual void loadWallet(std::unique_ptr<Wallet> wallet) = 0;
};

//! Interface to let node manage chain clients (wallets, or maybe tools for
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ class NodeImpl : public Node
}
std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override
{
return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::shared_ptr<CWallet> wallet) { fn(MakeWallet(wallet)); }));
return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::unique_ptr<Wallet>& wallet) { fn(std::move(wallet)); }));
}
std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override
{
Expand Down
8 changes: 4 additions & 4 deletions src/interfaces/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class PendingWalletTxImpl : public PendingWalletTx
auto locked_chain = m_wallet.chain().lock();
LOCK2(mempool.cs, m_wallet.cs_wallet);
CValidationState state;
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, g_connman.get(), state)) {
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, state)) {
reject_reason = state.GetRejectReason();
return false;
}
Expand Down Expand Up @@ -110,7 +110,7 @@ WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, co
//! Construct wallet tx status struct.
WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx)
{
LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit.
LockAnnotation lock(::cs_main); // Temporary, for mapBlockIndex below. Removed in upcoming commit.

WalletTxStatus result;
auto mi = ::BlockIndex().find(wtx.hashBlock);
Expand All @@ -120,7 +120,7 @@ WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const C
result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain);
result.time_received = wtx.nTimeReceived;
result.lock_time = wtx.tx->nLockTime;
result.is_final = CheckFinalTx(*wtx.tx);
result.is_final = locked_chain.checkFinalTx(*wtx.tx);
result.is_trusted = wtx.IsTrusted(locked_chain);
result.is_abandoned = wtx.isAbandoned();
result.is_coinbase = wtx.IsCoinBase();
Expand Down Expand Up @@ -539,7 +539,7 @@ class WalletImpl : public Wallet
{
FeeCalculation fee_calc;
CAmount result;
result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, ::mempool, ::feeEstimator, &fee_calc);
result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, &fee_calc);
if (returned_target) *returned_target = fee_calc.returnedTarget;
if (reason) *reason = fee_calc.reason;
return result;
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ static UniValue gobject_prepare(const JSONRPCRequest& request)
CReserveKey reservekey(pwallet);
// -- send the tx to the network
CValidationState state;
if (!pwallet->CommitTransaction(tx, {}, {}, reservekey, g_connman.get(), state)) {
if (!pwallet->CommitTransaction(tx, {}, {}, reservekey, state)) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "CommitTransaction failed! Reason given: " + state.GetRejectReason());
}

Expand Down
6 changes: 4 additions & 2 deletions src/rpc/mining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,8 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request)

RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR});
RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
unsigned int conf_target = ParseConfirmTarget(request.params[0]);
unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
bool conservative = true;
if (!request.params[1].isNull()) {
FeeEstimateMode fee_mode;
Expand Down Expand Up @@ -939,7 +940,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request)

RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true);
RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
unsigned int conf_target = ParseConfirmTarget(request.params[0]);
unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
double threshold = 0.95;
if (!request.params[1].isNull()) {
threshold = request.params[1].get_real();
Expand Down
5 changes: 1 addition & 4 deletions src/rpc/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@

#include <key_io.h>
#include <keystore.h>
#include <policy/fees.h>
#include <pubkey.h>
#include <rpc/util.h>
#include <tinyformat.h>
#include <util/strencodings.h>
#include <validation.h>

InitInterfaces* g_rpc_interfaces = nullptr;

Expand Down Expand Up @@ -96,10 +94,9 @@ UniValue DescribeAddress(const CTxDestination& dest)
return boost::apply_visitor(DescribeAddressVisitor(), dest);
}

unsigned int ParseConfirmTarget(const UniValue& value)
unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target)
{
int target = value.get_int();
unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
if (target < 1 || (unsigned int)target > max_target) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u - %u", 1, max_target));
}
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey
UniValue DescribeAddress(const CTxDestination& dest);

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

/** Returns, given services flags, a list of humanly readable (known) network services */
UniValue GetServicesNames(ServiceFlags services);
Expand Down
2 changes: 1 addition & 1 deletion src/ui_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void CClientUIInterface::InitMessage(const std::string& message) { return g_ui_s
void CClientUIInterface::NotifyNumConnectionsChanged(int newNumConnections) { return g_ui_signals.NotifyNumConnectionsChanged(newNumConnections); }
void CClientUIInterface::NotifyNetworkActiveChanged(bool networkActive) { return g_ui_signals.NotifyNetworkActiveChanged(networkActive); }
void CClientUIInterface::NotifyAlertChanged() { return g_ui_signals.NotifyAlertChanged(); }
void CClientUIInterface::LoadWallet(std::shared_ptr<CWallet> wallet) { return g_ui_signals.LoadWallet(wallet); }
void CClientUIInterface::LoadWallet(std::unique_ptr<interfaces::Wallet>& wallet) { return g_ui_signals.LoadWallet(wallet); }
void CClientUIInterface::ShowProgress(const std::string& title, int nProgress, bool resume_possible) { return g_ui_signals.ShowProgress(title, nProgress, resume_possible); }
void CClientUIInterface::NotifyBlockTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(b, i); }
void CClientUIInterface::NotifyChainLock(const std::string& bestChainLockHash, int bestChainLockHeight) { return g_ui_signals.NotifyChainLock(bestChainLockHash, bestChainLockHeight); }
Expand Down
7 changes: 5 additions & 2 deletions src/ui_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <stdint.h>
#include <string>

class CWallet;
class CBlockIndex;
class CDeterministicMNList;
namespace boost {
Expand All @@ -20,6 +19,10 @@ class connection;
}
} // namespace boost

namespace interfaces {
class Wallet;
} // namespace interfaces

/** General change type (added, updated, removed). */
enum ChangeType
{
Expand Down Expand Up @@ -102,7 +105,7 @@ class CClientUIInterface
ADD_SIGNALS_DECL_WRAPPER(NotifyAlertChanged, void, );

/** A wallet has been loaded. */
ADD_SIGNALS_DECL_WRAPPER(LoadWallet, void, std::shared_ptr<CWallet> wallet);
ADD_SIGNALS_DECL_WRAPPER(LoadWallet, void, std::unique_ptr<interfaces::Wallet>& wallet);

/**
* Show progress e.g. for verifychain.
Expand Down
Loading

0 comments on commit d6f9339

Please sign in to comment.