From b874747b51882a613895a100c4210c7f1dddde30 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 22 Mar 2019 00:42:17 -0400 Subject: 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 https://github.com/bitcoin/bitcoin/pull/15557#discussion_r267735431 --- src/interfaces/chain.cpp | 6 ++++++ src/interfaces/chain.h | 13 +++++++++++++ src/policy/policy.cpp | 18 +++++++++--------- src/policy/policy.h | 18 ++++++++++++++---- src/policy/settings.h | 18 ++++++++++++++++++ src/rpc/rawtransaction.cpp | 1 + src/wallet/feebumper.cpp | 2 +- src/wallet/rpcwallet.cpp | 6 +++--- src/wallet/wallet.cpp | 1 - 9 files changed, 65 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 13e7848cc6..b61a51b235 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -363,6 +363,12 @@ public: { return MakeUnique(command); } + bool rpcEnableDeprecated(const std::string& method) override { return IsDeprecatedRPCEnabled(method); } + void rpcRunLater(const std::string& name, std::function 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); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 3ace5a0ece..17d7b6d8f1 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -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 @@ public: //! needs to remain valid until Handler is disconnected. virtual std::unique_ptr 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 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 diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 44c23661fb..6f8542123d 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -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); } diff --git a/src/policy/policy.h b/src/policy/policy.h index 8660af26de..ebe040f0ea 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -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 diff --git a/src/policy/settings.h b/src/policy/settings.h index 1d2a1fb880..30a7189c93 100644 --- a/src/policy/settings.h +++ b/src/policy/settings.h @@ -6,7 +6,10 @@ #ifndef BITCOIN_POLICY_SETTINGS_H #define BITCOIN_POLICY_SETTINGS_H +#include + class CFeeRate; +class CTransaction; // Policy settings which are configurable at runtime. extern CFeeRate incrementalRelayFee; @@ -14,4 +17,19 @@ 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 diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 8d15ecc28e..91331198e2 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index e0f083b8ea..b547ea21d9 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -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 -- " diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 37e2930ee6..cb6f7637ad 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -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 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 @@ public: 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; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 68e2bb0252..212bce49fe 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1287,7 +1287,6 @@ void CWallet::UpdatedBlockTip() void CWallet::BlockUntilSyncedToCurrentChain() { - AssertLockNotHeld(cs_main); AssertLockNotHeld(cs_wallet); { -- cgit v1.2.3