Skip to content

Commit

Permalink
[refactor] remove const CChainParams& object from CWallet
Browse files Browse the repository at this point in the history 
Summary:
this replaces the const chainParams& member in CWallet with a public method that retrieves it from its Chain interface

the exception is the case of the Chain-less bitcoin-wallet/wallet-tool, where it returns the global Params(), which at least is in single place now instead of a billion.

Test Plan:
  ninja check check-functional

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7041
  • Loading branch information
@majcosta
majcosta committed Dec 8, 2020
1 parent 854580d commit a1e3e13
Show file tree
Hide file tree
Showing 20 changed files with 89 additions and 84 deletions.
4 changes: 2 additions & 2 deletions src/bench/coin_selection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ static void CoinSelection(benchmark::State &state) {

NodeContext node;
auto chain = interfaces::MakeChain(node, Params());
CWallet wallet(Params(), chain.get(), WalletLocation(),
CWallet wallet(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
wallet.SetupLegacyScriptPubKeyMan();
std::vector<std::unique_ptr<CWalletTx>> wtxs;
Expand Down Expand Up @@ -110,7 +110,7 @@ static void BnBExhaustion(benchmark::State &state) {

NodeContext node;
auto chain = interfaces::MakeChain(node, Params());
CWallet wallet(Params(), chain.get(), WalletLocation(),
CWallet wallet(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());

LOCK(wallet.cs_wallet);
Expand Down
3 changes: 1 addition & 2 deletions src/bench/wallet_balance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ static void WalletBalance(benchmark::State &state, const bool set_dirty,
NodeContext node;
std::unique_ptr<interfaces::Chain> chain =
interfaces::MakeChain(node, config.GetChainParams());
CWallet wallet{config.GetChainParams(), chain.get(), WalletLocation(),
WalletDatabase::CreateMock()};
CWallet wallet{chain.get(), WalletLocation(), WalletDatabase::CreateMock()};
{
wallet.SetupLegacyScriptPubKeyMan();
bool first_run;
Expand Down
1 change: 1 addition & 0 deletions src/interfaces/chain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ namespace {
notifications.transactionAddedToMempool(entry.GetSharedTx());
}
}
const CChainParams &params() const override { return m_params; }
NodeContext &m_node;
const CChainParams &m_params;
};
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/chain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@ class Chain {
//! to be prepared to handle this by ignoring notifications about unknown
//! removed transactions and already added new transactions.
virtual void requestMempoolTransactions(Notifications &notifications) = 0;

//! This Chain's parameters
virtual const CChainParams &params() const = 0;
};

//! Interface to let node manage chain clients (wallets, or maybe tools for
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ namespace {
return m_wallet->GetNewDestination(type, label, dest, error);
}
const CChainParams &getChainParams() override {
return m_wallet->chainParams;
return m_wallet->GetChainParams();
}
bool getPubKey(const CScript &script, const CKeyID &address,
CPubKey &pub_key) override {
Expand Down
6 changes: 3 additions & 3 deletions src/qt/test/addressbooktests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ void TestAddAddressesToSendBook(interfaces::Node &node) {
TestChain100Setup test;
node.setContext(&test.m_node);

std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(
Params(), node.context()->chain.get(), WalletLocation(),
WalletDatabase::CreateMock());
std::shared_ptr<CWallet> wallet =
std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(),
WalletDatabase::CreateMock());
wallet->SetupLegacyScriptPubKeyMan();

bool firstRun;
Expand Down
6 changes: 3 additions & 3 deletions src/qt/test/wallettests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ void TestGUI(interfaces::Node &node) {
{}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
}
node.setContext(&test.m_node);
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(
Params(), node.context()->chain.get(), WalletLocation(),
WalletDatabase::CreateMock());
std::shared_ptr<CWallet> wallet =
std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(),
WalletDatabase::CreateMock());

bool firstRun;
wallet->LoadWallet(firstRun);
Expand Down
2 changes: 1 addition & 1 deletion src/test/util/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ std::string getnewaddress(const Config &config, CWallet &w) {
void importaddress(CWallet &wallet, const std::string &address) {
auto spk_man = wallet.GetLegacyScriptPubKeyMan();
LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore);
const auto dest = DecodeDestination(address, wallet.chainParams);
const auto dest = DecodeDestination(address, wallet.GetChainParams());
assert(IsValidDestination(dest));
const auto script = GetScriptForDestination(dest);
wallet.MarkDirty();
Expand Down
10 changes: 6 additions & 4 deletions src/wallet/rpcdump.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,8 @@ UniValue importaddress(const Config &config, const JSONRPCRequest &request) {
{
LOCK(pwallet->cs_wallet);

CTxDestination dest =
DecodeDestination(request.params[0].get_str(), wallet->chainParams);
CTxDestination dest = DecodeDestination(request.params[0].get_str(),
wallet->GetChainParams());
if (IsValidDestination(dest)) {
if (fP2SH) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
Expand Down Expand Up @@ -840,7 +840,8 @@ UniValue dumpprivkey(const Config &config, const JSONRPCRequest &request) {
EnsureWalletIsUnlocked(pwallet);

std::string strAddress = request.params[0].get_str();
CTxDestination dest = DecodeDestination(strAddress, wallet->chainParams);
CTxDestination dest =
DecodeDestination(strAddress, wallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
"Invalid Bitcoin address");
Expand Down Expand Up @@ -1138,7 +1139,8 @@ static UniValue ProcessImportLegacy(
// Generate the script and destination for the scriptPubKey provided
CScript script;
if (!isScript) {
CTxDestination dest = DecodeDestination(output, pwallet->chainParams);
CTxDestination dest =
DecodeDestination(output, pwallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
"Invalid address \"" + output + "\"");
Expand Down
35 changes: 19 additions & 16 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,8 @@ static UniValue setlabel(const Config &config, const JSONRPCRequest &request) {

LOCK(pwallet->cs_wallet);

CTxDestination dest =
DecodeDestination(request.params[0].get_str(), wallet->chainParams);
CTxDestination dest = DecodeDestination(request.params[0].get_str(),
wallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
"Invalid Bitcoin address");
Expand Down Expand Up @@ -473,8 +473,8 @@ static UniValue sendtoaddress(const Config &config,

LOCK(pwallet->cs_wallet);

CTxDestination dest =
DecodeDestination(request.params[0].get_str(), wallet->chainParams);
CTxDestination dest = DecodeDestination(request.params[0].get_str(),
wallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
}
Expand Down Expand Up @@ -629,7 +629,8 @@ static UniValue signmessage(const Config &config,
std::string strAddress = request.params[0].get_str();
std::string strMessage = request.params[1].get_str();

CTxDestination dest = DecodeDestination(strAddress, wallet->chainParams);
CTxDestination dest =
DecodeDestination(strAddress, wallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_TYPE_ERROR, "Invalid address");
}
Expand Down Expand Up @@ -663,7 +664,7 @@ static Amount GetReceived(const CWallet &wallet, const UniValue &params,
} else {
// Get the address
CTxDestination dest =
DecodeDestination(params[0].get_str(), wallet.chainParams);
DecodeDestination(params[0].get_str(), wallet.GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
"Invalid Bitcoin address");
Expand Down Expand Up @@ -1020,7 +1021,8 @@ static UniValue sendmany(const Config &config, const JSONRPCRequest &request) {

std::vector<std::string> keys = sendTo.getKeys();
for (const std::string &name_ : keys) {
CTxDestination dest = DecodeDestination(name_, wallet->chainParams);
CTxDestination dest =
DecodeDestination(name_, wallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
std::string("Invalid Bitcoin address: ") +
Expand Down Expand Up @@ -1152,7 +1154,7 @@ static UniValue addmultisigaddress(const Config &config,
keys_or_addrs[i].get_str().length() == 130)) {
pubkeys.push_back(HexToPubKey(keys_or_addrs[i].get_str()));
} else {
pubkeys.push_back(AddrToPubKey(wallet->chainParams, spk_man,
pubkeys.push_back(AddrToPubKey(wallet->GetChainParams(), spk_man,
keys_or_addrs[i].get_str()));
}
}
Expand Down Expand Up @@ -1208,12 +1210,12 @@ static UniValue ListReceived(const Config &config, const CWallet *const pwallet,
CTxDestination filtered_address = CNoDestination();
if (!by_label && params.size() > 3) {
if (!IsValidDestinationString(params[3].get_str(),
pwallet->chainParams)) {
pwallet->GetChainParams())) {
throw JSONRPCError(RPC_WALLET_ERROR,
"address_filter parameter was invalid");
}
filtered_address =
DecodeDestination(params[3].get_str(), pwallet->chainParams);
DecodeDestination(params[3].get_str(), pwallet->GetChainParams());
has_filtered_address = true;
}

Expand Down Expand Up @@ -3565,7 +3567,7 @@ static UniValue listunspent(const Config &config,
for (size_t idx = 0; idx < inputs.size(); idx++) {
const UniValue &input = inputs[idx];
CTxDestination dest =
DecodeDestination(input.get_str(), wallet->chainParams);
DecodeDestination(input.get_str(), wallet->GetChainParams());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY,
std::string("Invalid Bitcoin address: ") +
Expand Down Expand Up @@ -3729,8 +3731,9 @@ void FundTransaction(CWallet *const pwallet, CMutableTransaction &tx,
true, true);

if (options.exists("changeAddress")) {
CTxDestination dest = DecodeDestination(
options["changeAddress"].get_str(), pwallet->chainParams);
CTxDestination dest =
DecodeDestination(options["changeAddress"].get_str(),
pwallet->GetChainParams());

if (!IsValidDestination(dest)) {
throw JSONRPCError(
Expand Down Expand Up @@ -4390,8 +4393,8 @@ UniValue getaddressinfo(const Config &config, const JSONRPCRequest &request) {
LOCK(pwallet->cs_wallet);

UniValue ret(UniValue::VOBJ);
CTxDestination dest =
DecodeDestination(request.params[0].get_str(), wallet->chainParams);
CTxDestination dest = DecodeDestination(request.params[0].get_str(),
wallet->GetChainParams());
// Make sure the destination is valid
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
Expand Down Expand Up @@ -4937,7 +4940,7 @@ static UniValue walletcreatefundedpsbt(const Config &config,
Amount fee;
int change_position;
CMutableTransaction rawTx =
ConstructTransaction(wallet->chainParams, request.params[0],
ConstructTransaction(wallet->GetChainParams(), request.params[0],
request.params[1], request.params[2]);
FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]);

Expand Down
2 changes: 1 addition & 1 deletion src/wallet/salvage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ bool RecoverDatabaseFile(const fs::path &file_path) {
}

DbTxn *ptxn = env->TxnBegin();
CWallet dummyWallet(GetConfig().GetChainParams(), nullptr, WalletLocation(),
CWallet dummyWallet(nullptr, WalletLocation(),
WalletDatabase::CreateDummy());
for (KeyValPair &row : salvagedData) {
/* Filter for only private key type KV pairs to be added to the salvaged
Expand Down
9 changes: 4 additions & 5 deletions src/wallet/test/coinselector_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) {
// Make sure that can use BnB when there are preset inputs
empty_wallet();
{
auto wallet =
std::make_unique<CWallet>(Params(), m_chain.get(), WalletLocation(),
WalletDatabase::CreateMock());
auto wallet = std::make_unique<CWallet>(m_chain.get(), WalletLocation(),
WalletDatabase::CreateMock());
bool firstRun;
wallet->LoadWallet(firstRun);
LOCK(wallet->cs_wallet);
Expand All @@ -355,7 +354,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) {

BOOST_AUTO_TEST_CASE(knapsack_solver_test) {
auto testChain = interfaces::MakeChain(testNode, Params());
CWallet testWallet(Params(), testChain.get(), WalletLocation(),
CWallet testWallet(testChain.get(), WalletLocation(),
WalletDatabase::CreateDummy());

CoinSet setCoinsRet, setCoinsRet2;
Expand Down Expand Up @@ -786,7 +785,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) {
// to find a solution that can pay the target value
BOOST_AUTO_TEST_CASE(SelectCoins_test) {
auto testChain = interfaces::MakeChain(testNode, Params());
CWallet testWallet(Params(), testChain.get(), WalletLocation(),
CWallet testWallet(testChain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
testWallet.SetupLegacyScriptPubKeyMan();

Expand Down
20 changes: 10 additions & 10 deletions src/wallet/test/ismine_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// P2PK compressed
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// P2PK uncompressed
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// P2PKH compressed
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -92,7 +92,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// P2PKH uncompressed
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -111,7 +111,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// P2SH
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// (P2PKH inside) P2SH inside P2SH (invalid)
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// scriptPubKey multisig
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand Down Expand Up @@ -196,7 +196,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// P2SH multisig
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -221,7 +221,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// OP_RETURN
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand All @@ -236,7 +236,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) {

// Nonstandard
{
CWallet keystore(Params(), chain.get(), WalletLocation(),
CWallet keystore(chain.get(), WalletLocation(),
WalletDatabase::CreateDummy());
keystore.SetupLegacyScriptPubKeyMan();
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
Expand Down
3 changes: 1 addition & 2 deletions src/wallet/test/wallet_test_fixture.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@

WalletTestingSetup::WalletTestingSetup(const std::string &chainName)
: TestingSetup(chainName), m_chain(interfaces::MakeChain(m_node, Params())),
m_wallet(Params(), m_chain.get(), WalletLocation(),
WalletDatabase::CreateMock()) {
m_wallet(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()) {
bool fFirstRun;
m_wallet.LoadWallet(fFirstRun);
m_chain_notifications_handler =
Expand Down
Loading

0 comments on commit a1e3e13

Please sign in to comment.