Skip to content
Permalink
Browse files

Remove access to node globals from wallet-linked code

Remove last few instances of accesses to node global variables from wallet
code. Also remove accesses to node globals from code in policy/policy.cpp that
isn't actually called by wallet code, but does get linked into wallet code.

This is the last change needed to allow bitcoin-wallet tool to be linked
without depending on libbitcoin_server.a, to ensure wallet code doesn't access
node global state and avoid bugs like
bitcoin#15557 (comment)
  • Loading branch information...
ryanofsky committed Mar 22, 2019
1 parent fbc6bb8 commit b874747b51882a613895a100c4210c7f1dddde30
@@ -363,6 +363,12 @@ class ChainImpl : public Chain
{
return MakeUnique<RpcHandlerImpl>(command);
}
bool rpcEnableDeprecated(const std::string& method) override { return IsDeprecatedRPCEnabled(method); }
void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) override
{
RPCRunLater(name, std::move(fn), seconds);
}
int rpcSerializationFlags() override { return RPCSerializationFlags(); }
void requestMempoolTransactions(Notifications& notifications) override
{
LOCK2(::cs_main, ::mempool.cs);
@@ -57,6 +57,10 @@ class Wallet;
//! 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).
//!
//! * The handleRpc, registerRpcs, rpcEnableDeprecated methods and other RPC
//! methods can go away if wallets listen for HTTP requests on their own
//! ports instead of registering to handle requests on the node HTTP port.
class Chain
{
public:
@@ -274,6 +278,15 @@ class Chain
//! needs to remain valid until Handler is disconnected.
virtual std::unique_ptr<Handler> handleRpc(const CRPCCommand& command) = 0;

//! Check if deprecated RPC is enabled.
virtual bool rpcEnableDeprecated(const std::string& method) = 0;

//! Run function after given number of seconds. Cancel any previous calls with same name.
virtual void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) = 0;

//! Current RPC serialization flags.
virtual int rpcSerializationFlags() = 0;

//! Synchronously send TransactionAddedToMempool notifications about all
//! current mempool transactions to the specified handler and return after
//! the last one is sent. These notifications aren't coordinated with async
@@ -77,7 +77,7 @@ bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType)
return true;
}

bool IsStandardTx(const CTransaction& tx, std::string& reason)
bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason)
{
if (tx.nVersion > CTransaction::MAX_STANDARD_VERSION || tx.nVersion < 1) {
reason = "version";
@@ -123,10 +123,10 @@ bool IsStandardTx(const CTransaction& tx, std::string& reason)

if (whichType == TX_NULL_DATA)
nDataOut++;
else if ((whichType == TX_MULTISIG) && (!fIsBareMultisigStd)) {
else if ((whichType == TX_MULTISIG) && (!permit_bare_multisig)) {
reason = "bare-multisig";
return false;
} else if (IsDust(txout, ::dustRelayFee)) {
} else if (IsDust(txout, dust_relay_fee)) {
reason = "dust";
return false;
}
@@ -239,17 +239,17 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
return true;
}

int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost)
int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost, unsigned int bytes_per_sigop)
{
return (std::max(nWeight, nSigOpCost * nBytesPerSigOp) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR;
return (std::max(nWeight, nSigOpCost * bytes_per_sigop) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR;
}

int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost)
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop)
{
return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost);
return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost, bytes_per_sigop);
}

int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost)
int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost, unsigned int bytes_per_sigop)
{
return GetVirtualTransactionSize(GetTransactionInputWeight(txin), nSigOpCost);
return GetVirtualTransactionSize(GetTransactionInputWeight(txin), nSigOpCost, bytes_per_sigop);
}
@@ -86,7 +86,7 @@ bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType);
* Check for standard transaction types
* @return True if all outputs (scriptPubKeys) use only standard transaction forms
*/
bool IsStandardTx(const CTransaction& tx, std::string& reason);
bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
/**
* Check for standard transaction types
* @param[in] mapInputs Map of previous transactions that have outputs we're spending
@@ -101,8 +101,18 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs);

/** Compute the virtual transaction size (weight reinterpreted as bytes). */
int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost);
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost = 0);
int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost = 0);
int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost, unsigned int bytes_per_sigop);
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop);
int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop);

static inline int64_t GetVirtualTransactionSize(const CTransaction& tx)
{
return GetVirtualTransactionSize(tx, 0, 0);
}

static inline int64_t GetVirtualTransactionInputSize(const CTxIn& tx)
{
return GetVirtualTransactionInputSize(tx, 0, 0);
}

#endif // BITCOIN_POLICY_POLICY_H
@@ -6,12 +6,30 @@
#ifndef BITCOIN_POLICY_SETTINGS_H
#define BITCOIN_POLICY_SETTINGS_H

#include <policy/policy.h>

class CFeeRate;
class CTransaction;

// Policy settings which are configurable at runtime.
extern CFeeRate incrementalRelayFee;
extern CFeeRate dustRelayFee;
extern unsigned int nBytesPerSigOp;
extern bool fIsBareMultisigStd;

static inline bool IsStandardTx(const CTransaction& tx, std::string& reason)
{
return IsStandardTx(tx, ::fIsBareMultisigStd, ::dustRelayFee, reason);
}

static inline int64_t GetVirtualTransactionSize(int64_t weight, int64_t sigop_cost)
{
return GetVirtualTransactionSize(weight, sigop_cost, ::nBytesPerSigOp);
}

static inline int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t sigop_cost)
{
return GetVirtualTransactionSize(tx, sigop_cost, ::nBytesPerSigOp);
}

#endif // BITCOIN_POLICY_SETTINGS_H
@@ -19,6 +19,7 @@
#include <node/transaction.h>
#include <policy/policy.h>
#include <policy/rbf.h>
#include <policy/settings.h>
#include <primitives/transaction.h>
#include <psbt.h>
#include <rpc/rawtransaction_util.h>
@@ -174,7 +174,7 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
// This may occur if the user set TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps,
// in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a
// moment earlier. In this case, we report an error to the user, who may use total_fee to make an adjustment.
CFeeRate minMempoolFeeRate = mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
CFeeRate minMempoolFeeRate = wallet->chain().mempoolMinFee();
if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
errors.push_back(strprintf(
"New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- "
@@ -1756,7 +1756,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */);
entry.pushKV("details", details);

std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags());
std::string strHex = EncodeHexTx(*wtx.tx, pwallet->chain().rpcSerializationFlags());
entry.pushKV("hex", strHex);

return entry;
@@ -1974,7 +1974,7 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
// wallet before the following callback is called. If a valid shared pointer
// is acquired in the callback then the wallet is still loaded.
std::weak_ptr<CWallet> weak_wallet = wallet;
RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet] {
pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet] {
if (auto shared_wallet = weak_wallet.lock()) {
LOCK(shared_wallet->cs_wallet);
shared_wallet->Lock();
@@ -3471,7 +3471,7 @@ class DescribeWalletAddressVisitor : public boost::static_visitor<UniValue>
UniValue obj(UniValue::VOBJ);
CScript subscript;
if (pwallet && pwallet->GetCScript(scriptID, subscript)) {
ProcessSubScript(subscript, obj, IsDeprecatedRPCEnabled("validateaddress"));
ProcessSubScript(subscript, obj, pwallet->chain().rpcEnableDeprecated("validateaddress"));
}
return obj;
}
@@ -1287,7 +1287,6 @@ void CWallet::UpdatedBlockTip()


void CWallet::BlockUntilSyncedToCurrentChain() {
AssertLockNotHeld(cs_main);
AssertLockNotHeld(cs_wallet);

{

0 comments on commit b874747

Please sign in to comment.
You can’t perform that action at this time.