Skip to content

Commit

Permalink
partial bitcoin#15288: Remove wallet -> node global function calls
Browse files Browse the repository at this point in the history
  • Loading branch information
kwvg committed Oct 31, 2021
1 parent e5c21c9 commit d9f19ac
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 67 deletions.
4 changes: 3 additions & 1 deletion src/Makefile.bench.include
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ nodist_bench_bench_dash_SOURCES = $(GENERATED_BENCH_FILES)
bench_bench_dash_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/
bench_bench_dash_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
bench_bench_dash_LDADD = \
$(LIBBITCOIN_SERVER) \
$(LIBBITCOIN_WALLET) \
$(LIBBITCOIN_SERVER) \
$(LIBBITCOIN_WALLET) \
$(LIBBITCOIN_SERVER) \
Expand All @@ -76,7 +78,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
2 changes: 1 addition & 1 deletion 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
48 changes: 48 additions & 0 deletions src/interfaces/chain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@

#include <chain.h>
#include <chainparams.h>
#include <net.h>
#include <policy/fees.h>
#include <policy/policy.h>
#include <primitives/block.h>
#include <sync.h>
#include <threadsafety.h>
#include <txmempool.h>
#include <uint256.h>
#include <util/system.h>
#include <validation.h>
Expand Down Expand Up @@ -132,6 +137,11 @@ class LockImpl : public Chain::Lock
}
return nullopt;
}
bool checkFinalTx(const CTransaction& tx) override
{
LockAnnotation lock(::cs_main);
return CheckFinalTx(tx);
}
};

class LockingStateImpl : public LockImpl, public UniqueLock<CCriticalSection>
Expand Down Expand Up @@ -177,6 +187,44 @@ class ChainImpl : public Chain
LOCK(cs_main);
return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash));
}
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);
}
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;
}
bool getPruneMode() override { return ::fPruneMode; }
bool p2pEnabled() override { return g_connman != nullptr; }
};

} // namespace
Expand Down
33 changes: 33 additions & 0 deletions src/interfaces/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@
#include <optional.h>

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

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

typedef std::shared_ptr<const CTransaction> CTransactionRef;

namespace interfaces {

Expand Down Expand Up @@ -102,6 +108,9 @@ 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;
};

//! Return Lock interface. Chain is locked when this is called, and
Expand All @@ -127,6 +136,30 @@ 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;

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

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

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

//! Interface to let node manage chain clients (wallets, or maybe tools for
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/wallet.cpp
Original file line number Diff line number Diff line change
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
6 changes: 4 additions & 2 deletions src/rpc/mining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,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 @@ -930,7 +931,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 @@ -33,7 +33,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
17 changes: 8 additions & 9 deletions src/wallet/fees.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <wallet/fees.h>

#include <policy/policy.h>
#include <txmempool.h>
#include <util/system.h>
#include <validation.h>
#include <wallet/coincontrol.h>
Expand All @@ -18,9 +17,9 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes)
return GetRequiredFeeRate(wallet).GetFee(nTxBytes);
}

CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc)
CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc)
{
CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, pool, estimator, feeCalc).GetFee(nTxBytes);
CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes);
// Always obey the maximum
if (fee_needed > maxTxFee) {
fee_needed = maxTxFee;
Expand All @@ -34,7 +33,7 @@ CFeeRate GetRequiredFeeRate(const CWallet& wallet)
return std::max(wallet.m_min_fee, ::minRelayTxFee);
}

CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc)
CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, FeeCalculation* feeCalc)
{
/* User control of how to calculate fee uses the following parameter precedence:
1. coin_control.m_feerate
Expand Down Expand Up @@ -63,14 +62,14 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr
if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true;
else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false;

feerate_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate);
feerate_needed = wallet.chain().estimateSmartFee(target, conservative_estimate, feeCalc);
if (feerate_needed == CFeeRate(0)) {
// if we don't have enough data for estimateSmartFee, then use fallback fee
feerate_needed = wallet.m_fallback_fee;
if (feeCalc) feeCalc->reason = FeeReason::FALLBACK;
}
// Obey mempool min fee when using smart fee estimation
CFeeRate min_mempool_feerate = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
CFeeRate min_mempool_feerate = wallet.chain().mempoolMinFee();
if (feerate_needed < min_mempool_feerate) {
feerate_needed = min_mempool_feerate;
if (feeCalc) feeCalc->reason = FeeReason::MEMPOOL_MIN;
Expand All @@ -86,10 +85,10 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr
return feerate_needed;
}

CFeeRate GetDiscardRate(const CWallet& wallet, const CBlockPolicyEstimator& estimator)
CFeeRate GetDiscardRate(const CWallet& wallet)
{
unsigned int highest_target = estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
CFeeRate discard_rate = estimator.estimateSmartFee(highest_target, nullptr /* FeeCalculation */, false /* conservative */);
unsigned int highest_target = wallet.chain().estimateMaxBlocks();
CFeeRate discard_rate = wallet.chain().estimateSmartFee(highest_target, false /* conservative */);
// Don't let discard_rate be greater than longest possible fee estimate if we get a valid fee estimate
discard_rate = (discard_rate == CFeeRate(0)) ? wallet.m_discard_rate : std::min(discard_rate, wallet.m_discard_rate);
// Discard rate must be at least dustRelayFee
Expand Down
8 changes: 3 additions & 5 deletions src/wallet/fees.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@

#include <amount.h>

class CBlockPolicyEstimator;
class CCoinControl;
class CFeeRate;
class CTxMemPool;
class CWallet;
struct FeeCalculation;

Expand All @@ -25,7 +23,7 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes);
* Estimate the minimum fee considering user set parameters
* and the required fee
*/
CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc);
CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc);

/**
* Return the minimum required feerate taking into account the
Expand All @@ -37,11 +35,11 @@ CFeeRate GetRequiredFeeRate(const CWallet& wallet);
* Estimate the minimum fee rate considering user set parameters
* and the required fee
*/
CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc);
CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, FeeCalculation* feeCalc);

/**
* Return the maximum feerate for discarding change.
*/
CFeeRate GetDiscardRate(const CWallet& wallet, const CBlockPolicyEstimator& estimator);
CFeeRate GetDiscardRate(const CWallet& wallet);

#endif // BITCOIN_WALLET_FEES_H
8 changes: 4 additions & 4 deletions src/wallet/rpcdump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
if (!request.params[2].isNull())
fRescan = request.params[2].get_bool();

if (fRescan && fPruneMode)
if (fRescan && pwallet->chain().getPruneMode())
throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode");

if (fRescan && !reserver.reserve()) {
Expand Down Expand Up @@ -277,7 +277,7 @@ UniValue importaddress(const JSONRPCRequest& request)
if (!request.params[2].isNull())
fRescan = request.params[2].get_bool();

if (fRescan && fPruneMode)
if (fRescan && pwallet->chain().getPruneMode())
throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode");

WalletRescanReserver reserver(pwallet);
Expand Down Expand Up @@ -464,7 +464,7 @@ UniValue importpubkey(const JSONRPCRequest& request)
if (!request.params[2].isNull())
fRescan = request.params[2].get_bool();

if (fRescan && fPruneMode)
if (fRescan && pwallet->chain().getPruneMode())
throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode");

WalletRescanReserver reserver(pwallet);
Expand Down Expand Up @@ -521,7 +521,7 @@ UniValue importwallet(const JSONRPCRequest& request)
+ HelpExampleRpc("importwallet", "\"test\"")
);

if (fPruneMode)
if (pwallet->chain().getPruneMode())
throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode");

WalletRescanReserver reserver(pwallet);
Expand Down
Loading

0 comments on commit d9f19ac

Please sign in to comment.