Skip to content

Commit c536dfb

Browse files
meshcollidermeshcollider
committed
Merge #15639: bitcoin-wallet tool: Drop libbitcoin_server.a dependency
78a2fb5 bitcoin-wallet tool: Drop libbitcoin_server.a dependency (Russell Yanofsky) b874747 Remove access to node globals from wallet-linked code (Russell Yanofsky) fbc6bb8 bitcoin-wallet tool: Drop MakeChain calls (Russell Yanofsky) Pull request description: Dropping the `bitcoin-wallet` dependency on `libbitcoin_server.a` ensures wallet code can't access node global state, avoiding bugs like bitcoin/bitcoin#15557 (comment) ACKs for commit 78a2fb: jnewbery: utACK 78a2fb5. Nice work, Russ. MarcoFalke: utACK 78a2fb5 MeshCollider: utACK bitcoin/bitcoin@78a2fb5 Tree-SHA512: ee6ea774f683b936bea66638211dd53c42b8316e1ef03dd58d12fb7ee3891432a43c5c149944173c1e2436aa756b672e1679c39fc10043792ac55cd4d8af2823
2 parents f6120d4 + 78a2fb5 commit c536dfb

18 files changed

+89
-47
lines changed

‎src/Makefile.am

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,6 @@ endif
593593
bitcoin_wallet_LDADD = \
594594
$(LIBBITCOIN_WALLET_TOOL) \
595595
$(LIBBITCOIN_WALLET) \
596-
$(LIBBITCOIN_SERVER) \
597596
$(LIBBITCOIN_COMMON) \
598597
$(LIBBITCOIN_CONSENSUS) \
599598
$(LIBBITCOIN_UTIL) \

‎src/bench/coin_selection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<st
2929
static void CoinSelection(benchmark::State& state)
3030
{
3131
auto chain = interfaces::MakeChain();
32-
const CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy());
32+
const CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
3333
std::vector<std::unique_ptr<CWalletTx>> wtxs;
3434
LOCK(wallet.cs_wallet);
3535

@@ -61,7 +61,7 @@ static void CoinSelection(benchmark::State& state)
6161

6262
typedef std::set<CInputCoin> CoinSet;
6363
static auto testChain = interfaces::MakeChain();
64-
static const CWallet testWallet(*testChain, WalletLocation(), WalletDatabase::CreateDummy());
64+
static const CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy());
6565
std::vector<std::unique_ptr<CWalletTx>> wtxn;
6666

6767
// Copied from src/wallet/test/coinselector_tests.cpp

‎src/interfaces/chain.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,12 @@ class ChainImpl : public Chain
363363
{
364364
return MakeUnique<RpcHandlerImpl>(command);
365365
}
366+
bool rpcEnableDeprecated(const std::string& method) override { return IsDeprecatedRPCEnabled(method); }
367+
void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) override
368+
{
369+
RPCRunLater(name, std::move(fn), seconds);
370+
}
371+
int rpcSerializationFlags() override { return RPCSerializationFlags(); }
366372
void requestMempoolTransactions(Notifications& notifications) override
367373
{
368374
LOCK2(::cs_main, ::mempool.cs);

‎src/interfaces/chain.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ class Wallet;
5757
//! notifications to the GUI should go away when GUI and wallet can directly
5858
//! communicate with each other without going through the node
5959
//! (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096).
60+
//!
61+
//! * The handleRpc, registerRpcs, rpcEnableDeprecated methods and other RPC
62+
//! methods can go away if wallets listen for HTTP requests on their own
63+
//! ports instead of registering to handle requests on the node HTTP port.
6064
class Chain
6165
{
6266
public:
@@ -274,6 +278,15 @@ class Chain
274278
//! needs to remain valid until Handler is disconnected.
275279
virtual std::unique_ptr<Handler> handleRpc(const CRPCCommand& command) = 0;
276280

281+
//! Check if deprecated RPC is enabled.
282+
virtual bool rpcEnableDeprecated(const std::string& method) = 0;
283+
284+
//! Run function after given number of seconds. Cancel any previous calls with same name.
285+
virtual void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) = 0;
286+
287+
//! Current RPC serialization flags.
288+
virtual int rpcSerializationFlags() = 0;
289+
277290
//! Synchronously send TransactionAddedToMempool notifications about all
278291
//! current mempool transactions to the specified handler and return after
279292
//! the last one is sent. These notifications aren't coordinated with async

‎src/policy/policy.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType)
7777
return true;
7878
}
7979

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

124124
if (whichType == TX_NULL_DATA)
125125
nDataOut++;
126-
else if ((whichType == TX_MULTISIG) && (!fIsBareMultisigStd)) {
126+
else if ((whichType == TX_MULTISIG) && (!permit_bare_multisig)) {
127127
reason = "bare-multisig";
128128
return false;
129-
} else if (IsDust(txout, ::dustRelayFee)) {
129+
} else if (IsDust(txout, dust_relay_fee)) {
130130
reason = "dust";
131131
return false;
132132
}
@@ -239,17 +239,17 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
239239
return true;
240240
}
241241

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

247-
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost)
247+
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop)
248248
{
249-
return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost);
249+
return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost, bytes_per_sigop);
250250
}
251251

252-
int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost)
252+
int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost, unsigned int bytes_per_sigop)
253253
{
254-
return GetVirtualTransactionSize(GetTransactionInputWeight(txin), nSigOpCost);
254+
return GetVirtualTransactionSize(GetTransactionInputWeight(txin), nSigOpCost, bytes_per_sigop);
255255
}

‎src/policy/policy.h

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ bool IsStandard(const CScript& scriptPubKey, txnouttype& whichType);
8686
* Check for standard transaction types
8787
* @return True if all outputs (scriptPubKeys) use only standard transaction forms
8888
*/
89-
bool IsStandardTx(const CTransaction& tx, std::string& reason);
89+
bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
9090
/**
9191
* Check for standard transaction types
9292
* @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)
101101
bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs);
102102

103103
/** Compute the virtual transaction size (weight reinterpreted as bytes). */
104-
int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost);
105-
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost = 0);
106-
int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost = 0);
104+
int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost, unsigned int bytes_per_sigop);
105+
int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop);
106+
int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop);
107+
108+
static inline int64_t GetVirtualTransactionSize(const CTransaction& tx)
109+
{
110+
return GetVirtualTransactionSize(tx, 0, 0);
111+
}
112+
113+
static inline int64_t GetVirtualTransactionInputSize(const CTxIn& tx)
114+
{
115+
return GetVirtualTransactionInputSize(tx, 0, 0);
116+
}
107117

108118
#endif // BITCOIN_POLICY_POLICY_H

‎src/policy/settings.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,30 @@
66
#ifndef BITCOIN_POLICY_SETTINGS_H
77
#define BITCOIN_POLICY_SETTINGS_H
88

9+
#include <policy/policy.h>
10+
911
class CFeeRate;
12+
class CTransaction;
1013

1114
// Policy settings which are configurable at runtime.
1215
extern CFeeRate incrementalRelayFee;
1316
extern CFeeRate dustRelayFee;
1417
extern unsigned int nBytesPerSigOp;
1518
extern bool fIsBareMultisigStd;
1619

20+
static inline bool IsStandardTx(const CTransaction& tx, std::string& reason)
21+
{
22+
return IsStandardTx(tx, ::fIsBareMultisigStd, ::dustRelayFee, reason);
23+
}
24+
25+
static inline int64_t GetVirtualTransactionSize(int64_t weight, int64_t sigop_cost)
26+
{
27+
return GetVirtualTransactionSize(weight, sigop_cost, ::nBytesPerSigOp);
28+
}
29+
30+
static inline int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t sigop_cost)
31+
{
32+
return GetVirtualTransactionSize(tx, sigop_cost, ::nBytesPerSigOp);
33+
}
34+
1735
#endif // BITCOIN_POLICY_SETTINGS_H

‎src/qt/test/addressbooktests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void TestAddAddressesToSendBook()
5858
{
5959
TestChain100Setup test;
6060
auto chain = interfaces::MakeChain();
61-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(*chain, WalletLocation(), WalletDatabase::CreateMock());
61+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateMock());
6262
bool firstRun;
6363
wallet->LoadWallet(firstRun);
6464

‎src/qt/test/wallettests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ void TestGUI()
135135
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
136136
}
137137
auto chain = interfaces::MakeChain();
138-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(*chain, WalletLocation(), WalletDatabase::CreateMock());
138+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(chain.get(), WalletLocation(), WalletDatabase::CreateMock());
139139
bool firstRun;
140140
wallet->LoadWallet(firstRun);
141141
{

‎src/rpc/rawtransaction.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <node/transaction.h>
2020
#include <policy/policy.h>
2121
#include <policy/rbf.h>
22+
#include <policy/settings.h>
2223
#include <primitives/transaction.h>
2324
#include <psbt.h>
2425
#include <rpc/rawtransaction_util.h>

0 commit comments

Comments
 (0)