From c26722fc0e8257390f7e07c869f7c459849aff54 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 10 Mar 2021 08:24:47 +0100 Subject: [PATCH 01/10] Merge #21331: rpc: replace wallet raw pointers with references (#18592 rebased) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7c90c67b7e6f598f9ffdc136ded2b533b78ed044 rpc: refactor rpc wallet functions to take references instead of pointers (fanquake) 48669340080feaff86b8fc0403ef22c820477697 rpc: remove calls to CWallet.get() (fanquake) Pull request description: This is a rebased #18592. > This PR replaces raw pointers in `rpcwallet.cpp` and `rpcdump.cpp` with **shared_ptr**. The motivation for this PR is described here https://github.com/bitcoin/bitcoin/issues/18590 > It seems that this PR is indirectly related to this issue: https://github.com/bitcoin/bitcoin/pull/13063#discussion_r186740049 > Notice: I have deliberately **not** changed the class `WalletRescanReserver ` whose constructor expects a raw pointer, because it's external and affects other areas, which I didn't touch to avoid making this PR "viral". > Fixes https://github.com/bitcoin/bitcoin/issues/18590 ACKs for top commit: MarcoFalke: ACK 7c90c67b7e6f598f9ffdc136ded2b533b78ed044 🐧 ryanofsky: Code review ACK 7c90c67b7e6f598f9ffdc136ded2b533b78ed044. Changes easy to review with `--word-diff-regex=. -U0` Tree-SHA512: 32d69c813026b02260e8a89de9d6a5ab9e87826ba230687246583ac7a80c8c3fd00318da4658f1450e04c23d2c77ae765862de0d2a110b1312b3b69a1161e7ba --- src/rpc/evo.cpp | 56 ++++--- src/rpc/governance.cpp | 8 +- src/wallet/rpcdump.cpp | 132 ++++++++-------- src/wallet/rpcwallet.cpp | 314 +++++++++++++++++---------------------- src/wallet/rpcwallet.h | 2 +- 5 files changed, 228 insertions(+), 284 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index aa5a3d55f7..6b82ac692b 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -214,15 +214,13 @@ static bool ValidatePlatformPort(const int32_t port) #ifdef ENABLE_WALLET template -static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const SpecialTxPayload& payload, const CTxDestination& fundDest) +static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const SpecialTxPayload& payload, const CTxDestination& fundDest) { - CHECK_NONFATAL(pwallet != nullptr); - // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now - pwallet->BlockUntilSyncedToCurrentChain(); + wallet.BlockUntilSyncedToCurrentChain(); - LOCK(pwallet->cs_wallet); + LOCK(wallet.cs_wallet); CTxDestination nodest = CNoDestination(); if (fundDest == nodest) { @@ -253,7 +251,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci coinControl.fRequireAllInputs = false; std::vector vecOutputs; - pwallet->AvailableCoins(vecOutputs); + wallet.AvailableCoins(vecOutputs); for (const auto& out : vecOutputs) { CTxDestination txDest; @@ -272,7 +270,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci bilingual_str strFailReason; FeeCalculation fee_calc_out; - if (!pwallet->CreateTransaction(vecSend, newTx, nFee, nChangePos, strFailReason, coinControl, fee_calc_out, false, tx.vExtraPayload.size())) { + if (!wallet.CreateTransaction(vecSend, newTx, nFee, nChangePos, strFailReason, coinControl, fee_calc_out, false, tx.vExtraPayload.size())) { throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason.original); } @@ -677,11 +675,11 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, const bool isEvoRequested = mnType == MnType::Evo; - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; if (action == ProTxRegisterAction::External || action == ProTxRegisterAction::Fund) { - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*pwallet); } const bool isV19active{DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; @@ -796,7 +794,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } if (action == ProTxRegisterAction::Fund) { - FundSpecialTx(wallet.get(), tx, ptx, fundDest); + FundSpecialTx(*pwallet, tx, ptx, fundDest); UpdateSpecialTxInputsHash(tx, ptx); CAmount fundCollateral = GetMnType(mnType).collat_amount; uint32_t collateralIndex = (uint32_t) -1; @@ -815,14 +813,14 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, // referencing external collateral const bool unlockOnError = [&]() { - if (LOCK(wallet->cs_wallet); !wallet->IsLockedCoin(ptx.collateralOutpoint.hash, ptx.collateralOutpoint.n)) { - wallet->LockCoin(ptx.collateralOutpoint); + if (LOCK(pwallet->cs_wallet); !pwallet->IsLockedCoin(ptx.collateralOutpoint.hash, ptx.collateralOutpoint.n)) { + pwallet->LockCoin(ptx.collateralOutpoint); return true; } return false; }(); try { - FundSpecialTx(wallet.get(), tx, ptx, fundDest); + FundSpecialTx(*pwallet, tx, ptx, fundDest); UpdateSpecialTxInputsHash(tx, ptx); Coin coin; if (!GetUTXOCoin(chainman.ActiveChainstate(), ptx.collateralOutpoint, coin)) { @@ -847,13 +845,13 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, return ret; } else { { - LOCK(wallet->cs_wallet); + LOCK(pwallet->cs_wallet); // lets prove we own the collateral CScript scriptPubKey = GetScriptForDestination(txDest); - std::unique_ptr provider = wallet->GetSolvingProvider(scriptPubKey); + std::unique_ptr provider = pwallet->GetSolvingProvider(scriptPubKey); std::string signed_payload; - SigningResult err = wallet->SignMessage(ptx.MakeSignString(), *pkhash, signed_payload); + SigningResult err = pwallet->SignMessage(ptx.MakeSignString(), *pkhash, signed_payload); if (err == SigningResult::SIGNING_FAILED) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, SigningResultString(err)); } else if (err != SigningResult::OK){ @@ -868,7 +866,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, } } catch (...) { if (unlockOnError) { - WITH_LOCK(wallet->cs_wallet, wallet->UnlockCoin(ptx.collateralOutpoint)); + WITH_LOCK(pwallet->cs_wallet, pwallet->UnlockCoin(ptx.collateralOutpoint)); } throw; } @@ -903,7 +901,7 @@ static RPCHelpMan protx_register_submit() std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); CMutableTransaction tx; if (!DecodeHexTx(tx, request.params[0].get_str())) { @@ -1020,7 +1018,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); const bool isV19active{DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; const bool is_bls_legacy = !isV19active; @@ -1109,7 +1107,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques } } - FundSpecialTx(wallet.get(), tx, ptx, feeSource); + FundSpecialTx(*wallet, tx, ptx, feeSource); SignSpecialTxPayloadByHash(tx, ptx, keyOperator); SetTxPayload(tx, ptx); @@ -1155,7 +1153,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); CProUpRegTx ptx; ptx.proTxHash = ParseHashV(request.params[0], "proTxHash"); @@ -1217,7 +1215,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + request.params[4].get_str()); } - FundSpecialTx(wallet.get(), tx, ptx, feeSourceDest); + FundSpecialTx(*wallet, tx, ptx, feeSourceDest); SignSpecialTxPayloadByHash(tx, ptx, dmn->pdmnState->keyIDOwner, *wallet); SetTxPayload(tx, ptx); @@ -1268,10 +1266,10 @@ static RPCHelpMan protx_revoke() CChainstateHelper& chain_helper = *node.chain_helper; - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*pwallet); const bool isV19active{DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; const bool is_bls_legacy = !isV19active; @@ -1306,17 +1304,17 @@ static RPCHelpMan protx_revoke() CTxDestination feeSourceDest = DecodeDestination(request.params[3].get_str()); if (!IsValidDestination(feeSourceDest)) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + request.params[3].get_str()); - FundSpecialTx(wallet.get(), tx, ptx, feeSourceDest); + FundSpecialTx(*pwallet, tx, ptx, feeSourceDest); } else if (dmn->pdmnState->scriptOperatorPayout != CScript()) { // Using funds from previousely specified operator payout address CTxDestination txDest; ExtractDestination(dmn->pdmnState->scriptOperatorPayout, txDest); - FundSpecialTx(wallet.get(), tx, ptx, txDest); + FundSpecialTx(*pwallet, tx, ptx, txDest); } else if (dmn->pdmnState->scriptPayout != CScript()) { // Using funds from previousely specified masternode payout address CTxDestination txDest; ExtractDestination(dmn->pdmnState->scriptPayout, txDest); - FundSpecialTx(wallet.get(), tx, ptx, txDest); + FundSpecialTx(*pwallet, tx, ptx, txDest); } else { throw JSONRPCError(RPC_INTERNAL_ERROR, "No payout or fee source addresses found, can't revoke"); } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 9028d5cb20..f92d18b3a1 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -144,7 +144,7 @@ static RPCHelpMan gobject_prepare() std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); // ASSEMBLE NEW GOVERNANCE OBJECT FROM USER PARAMETERS @@ -253,7 +253,7 @@ static RPCHelpMan gobject_list_prepared() { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); int64_t nCount = request.params.empty() ? 10 : ParseInt64V(request.params[0], "count"); if (nCount < 0) { @@ -546,7 +546,7 @@ static RPCHelpMan gobject_vote_many() throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid vote outcome. Please use one of the following: 'yes', 'no' or 'abstain'"); } - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); std::map votingKeys; @@ -600,7 +600,7 @@ static RPCHelpMan gobject_vote_alias() throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid vote outcome. Please use one of the following: 'yes', 'no' or 'abstain'"); } - EnsureWalletIsUnlocked(wallet.get()); + EnsureWalletIsUnlocked(*wallet); uint256 proTxHash(ParseHashV(request.params[3], "protx-hash")); auto dmn = node.dmnman->GetListAtChainTip().GetValidMN(proTxHash); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 4d12648240..075ac8e5e2 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -96,15 +96,14 @@ RPCHelpMan importprivkey() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled"); } - EnsureLegacyScriptPubKeyMan(*wallet, true); + EnsureLegacyScriptPubKeyMan(*pwallet, true); WalletBatch batch(pwallet->GetDatabase()); WalletRescanReserver reserver(*pwallet); @@ -112,7 +111,7 @@ RPCHelpMan importprivkey() { LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); std::string strSecret = request.params[0].get_str(); std::string strLabel = ""; @@ -177,9 +176,8 @@ RPCHelpMan abortrescan() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; if (!pwallet->IsScanning() || pwallet->IsAbortingRescan()) return false; pwallet->AbortRescan(); @@ -215,9 +213,9 @@ RPCHelpMan importaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + EnsureLegacyScriptPubKeyMan(*pwallet, true); std::string strLabel; @@ -300,9 +298,8 @@ RPCHelpMan importprunedfunds() RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; CMutableTransaction tx; if (!DecodeHexTx(tx, request.params[0].get_str())) { @@ -362,9 +359,8 @@ RPCHelpMan removeprunedfunds() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -409,10 +405,10 @@ RPCHelpMan importpubkey() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); - EnsureLegacyScriptPubKeyMan(*wallet, true); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + + EnsureLegacyScriptPubKeyMan(*pwallet, true); std::string strLabel; if (!request.params[1].isNull()) @@ -488,10 +484,10 @@ RPCHelpMan importwallet() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); - EnsureLegacyScriptPubKeyMan(*wallet, true); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + + EnsureLegacyScriptPubKeyMan(*pwallet, true); if (pwallet->chain().havePruned()) { // Exit early and print an error. @@ -511,7 +507,7 @@ RPCHelpMan importwallet() { LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); fsbridge::ifstream file; file.open(request.params[0].get_str(), std::ios::in | std::ios::ate); @@ -650,9 +646,8 @@ RPCHelpMan importelectrumwallet() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; if (pwallet->chain().havePruned()) throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode"); @@ -661,11 +656,11 @@ RPCHelpMan importelectrumwallet() throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); } - LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet); + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); fsbridge::ifstream file; std::string strFileName = request.params[0].get_str(); @@ -828,15 +823,14 @@ RPCHelpMan dumpprivkey() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; - LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet); + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); std::string strAddress = request.params[0].get_str(); CTxDestination dest = DecodeDestination(strAddress); @@ -875,15 +869,14 @@ RPCHelpMan dumphdinfo() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); - LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet); + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); CHDChain hdChainCurrent; if (!spk_man.GetHDChain(hdChainCurrent)) throw JSONRPCError(RPC_WALLET_ERROR, "This wallet is not a HD wallet."); @@ -941,7 +934,7 @@ RPCHelpMan dumpwallet() LOCK(wallet.cs_wallet); - EnsureWalletIsUnlocked(&wallet); + EnsureWalletIsUnlocked(wallet); fs::path filepath = request.params[0].get_str(); filepath = fs::absolute(filepath); @@ -1364,7 +1357,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::mapcs_wallet) +static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { UniValue warnings(UniValue::VARR); UniValue result(UniValue::VOBJ); @@ -1379,7 +1372,7 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con const bool add_keypool = data.exists("keypool") ? data["keypool"].get_bool() : false; // Add to keypool only works with privkeys disabled - if (add_keypool && !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + if (add_keypool && !wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Keys can only be imported to the keypool when private keys are disabled"); } @@ -1401,29 +1394,29 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con } // If private keys are disabled, abort if private keys are being imported - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !privkey_map.empty()) { + if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !privkey_map.empty()) { throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled"); } // Check whether we have any work to do for (const CScript& script : script_pub_keys) { - if (pwallet->IsMine(script) & ISMINE_SPENDABLE) { + if (wallet.IsMine(script) & ISMINE_SPENDABLE) { throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script (\"" + HexStr(script) + "\")"); } } // All good, time to import - pwallet->MarkDirty(); - if (!pwallet->ImportScripts(import_data.import_scripts, timestamp)) { + wallet.MarkDirty(); + if (!wallet.ImportScripts(import_data.import_scripts, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding script to wallet"); } - if (!pwallet->ImportPrivKeys(privkey_map, timestamp)) { + if (!wallet.ImportPrivKeys(privkey_map, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } - if (!pwallet->ImportPubKeys(ordered_pubkeys, pubkey_map, import_data.key_origins, add_keypool, internal, timestamp)) { + if (!wallet.ImportPubKeys(ordered_pubkeys, pubkey_map, import_data.key_origins, add_keypool, internal, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } - if (!pwallet->ImportScriptPubKeys(label, script_pub_keys, have_solving_data, !internal, timestamp)) { + if (!wallet.ImportScriptPubKeys(label, script_pub_keys, have_solving_data, !internal, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } @@ -1530,17 +1523,14 @@ RPCHelpMan importmulti() }, [&](const RPCHelpMan& self, const JSONRPCRequest& mainRequest) -> UniValue { - - RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ}); const UniValue& requests = mainRequest.params[0]; - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(mainRequest); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(mainRequest); + if (!pwallet) return NullUniValue; - EnsureLegacyScriptPubKeyMan(*wallet, true); + EnsureLegacyScriptPubKeyMan(*pwallet, true); //Default options bool fRescan = true; @@ -1564,7 +1554,7 @@ RPCHelpMan importmulti() UniValue response(UniValue::VARR); { LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); // Verify all timestamps are present before importing any keys. CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(nLowestTimestamp).mtpTime(now))); @@ -1576,7 +1566,7 @@ RPCHelpMan importmulti() for (const UniValue& data : requests.getValues()) { const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp); - const UniValue result = ProcessImport(pwallet, data, timestamp); + const UniValue result = ProcessImport(*pwallet, data, timestamp); response.push_back(result); if (!fRescan) { @@ -1643,7 +1633,7 @@ RPCHelpMan importmulti() }; } -static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { UniValue warnings(UniValue::VARR); UniValue result(UniValue::VOBJ); @@ -1712,7 +1702,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& } // If the wallet disabled private keys, abort if private keys exist - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !keys.keys.empty()) { + if (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !keys.keys.empty()) { throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled"); } @@ -1736,7 +1726,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& } // If private keys are enabled, check some things. - if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + if (!wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { if (keys.keys.empty()) { throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import descriptor without private keys to a wallet with private keys enabled"); } @@ -1748,7 +1738,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& WalletDescriptor w_desc(std::move(parsed_desc), timestamp, range_start, range_end, next_index); // Check if the wallet already contains the descriptor - auto existing_spk_manager = pwallet->GetDescriptorScriptPubKeyMan(w_desc); + auto existing_spk_manager = wallet.GetDescriptorScriptPubKeyMan(w_desc); if (existing_spk_manager) { if (!existing_spk_manager->CanUpdateToWalletDescriptor(w_desc, error)) { throw JSONRPCError(RPC_INVALID_PARAMETER, error); @@ -1756,7 +1746,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& } // Add descriptor to the wallet - auto spk_manager = pwallet->AddWalletDescriptor(w_desc, keys, label, internal); + auto spk_manager = wallet.AddWalletDescriptor(w_desc, keys, label, internal); if (spk_manager == nullptr) { throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Could not add descriptor '%s'", descriptor)); } @@ -1766,11 +1756,11 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& if (!w_desc.descriptor->GetOutputType()) { warnings.push_back("Unknown output type, cannot set descriptor to active."); } else { - pwallet->AddActiveScriptPubKeyMan(spk_manager->GetID(), internal); + wallet.AddActiveScriptPubKeyMan(spk_manager->GetID(), internal); } } else { if (w_desc.descriptor->GetOutputType()) { - pwallet->DeactivateScriptPubKeyMan(spk_manager->GetID(), internal); + wallet.DeactivateScriptPubKeyMan(spk_manager->GetID(), internal); } } @@ -1836,10 +1826,8 @@ RPCHelpMan importdescriptors() { [&](const RPCHelpMan& self, const JSONRPCRequest& main_request) -> UniValue { // Acquire the wallet - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(main_request); - if (!wallet) return NullUniValue; - - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(main_request); + if (!pwallet) return NullUniValue; // Make sure wallet is a descriptor wallet if (!pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { @@ -1861,7 +1849,7 @@ RPCHelpMan importdescriptors() { UniValue response(UniValue::VARR); { LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); CHECK_NONFATAL(pwallet->chain().findBlock(pwallet->GetLastBlockHash(), FoundBlock().time(lowest_timestamp).mtpTime(now))); @@ -1869,7 +1857,7 @@ RPCHelpMan importdescriptors() { for (const UniValue& request : requests.getValues()) { // This throws an error if "timestamp" doesn't exist const int64_t timestamp = std::max(GetImportTimestamp(request, now), minimum_timestamp); - const UniValue result = ProcessDescriptorImport(pwallet, request, timestamp); + const UniValue result = ProcessDescriptorImport(*pwallet, request, timestamp); response.push_back(result); if (lowest_timestamp > timestamp ) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a6e87aae83..02e3beacc7 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -54,8 +54,8 @@ static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; static const uint32_t WALLET_DASH_KB_TO_DUFF_B = COIN / 1000; // 1 duff / B = 0.00001 DASH / kB -static inline bool GetAvoidReuseFlag(const CWallet* const pwallet, const UniValue& param) { - bool can_avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); +static inline bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) { + bool can_avoid_reuse = wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool(); if (avoid_reuse && !can_avoid_reuse) { @@ -70,11 +70,11 @@ static inline bool GetAvoidReuseFlag(const CWallet* const pwallet, const UniValu * We default to true for watchonly wallets if include_watchonly isn't * explicitly set. */ -static bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& pwallet) +static bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet) { if (include_watchonly.isNull()) { // if include_watchonly isn't explicitly set, then check if we have a watchonly wallet - return pwallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + return wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); } // otherwise return whatever include_watchonly was set to @@ -124,9 +124,9 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques "Wallet file not specified (must request wallet RPC through /wallet/ uri-path)."); } -void EnsureWalletIsUnlocked(const CWallet* pwallet) +void EnsureWalletIsUnlocked(const CWallet& wallet) { - if (pwallet->IsLocked()) { + if (wallet.IsLocked()) { throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please enter the wallet passphrase with walletpassphrase first."); } } @@ -203,13 +203,13 @@ static std::string LabelFromValue(const UniValue& value) /** * Update coin control with fee estimation based on the given parameters * - * @param[in] pwallet Wallet pointer + * @param[in] wallet Wallet reference * @param[in,out] cc Coin control which is to be updated * @param[in] estimate_mode String value (e.g. "ECONOMICAL") * @param[in] estimate_param Parameter (blocks to confirm, explicit fee rate, etc) * @throws a JSONRPCError if estimate_mode is unknown, or if estimate_param is missing when required */ -static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const UniValue& estimate_mode, const UniValue& estimate_param) +static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const UniValue& estimate_mode, const UniValue& estimate_param) { if (!estimate_mode.isNull()) { if (!FeeModeFromString(estimate_mode.get_str(), cc.m_fee_mode)) { @@ -230,7 +230,7 @@ static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const U cc.m_feerate = CFeeRate(fee_rate); } else if (!estimate_param.isNull()) { - cc.m_confirm_target = ParseConfirmTarget(estimate_param, pwallet->chain().estimateMaxBlocks()); + cc.m_confirm_target = ParseConfirmTarget(estimate_param, wallet.chain().estimateMaxBlocks()); } } @@ -252,9 +252,8 @@ RPCHelpMan getnewaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -292,9 +291,8 @@ RPCHelpMan getrawchangeaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -328,9 +326,8 @@ static RPCHelpMan setlabel() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -382,9 +379,9 @@ void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_f } } -UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector &recipients, mapValue_t map_value, bool verbose) +UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vector &recipients, mapValue_t map_value, bool verbose) { - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(wallet); if (coin_control.IsUsingCoinJoin()) { map_value["DS"] = "1"; @@ -396,11 +393,11 @@ UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std bilingual_str error; CTransactionRef tx; FeeCalculation fee_calc_out; - bool fCreated = pwallet->CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + bool fCreated = wallet.CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, !wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); if (!fCreated) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } - pwallet->CommitTransaction(tx, std::move(map_value), {} /* orderForm */); + wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */); if (verbose) { UniValue entry(UniValue::VOBJ); entry.pushKV("txid", tx->GetHash().GetHex()); @@ -456,9 +453,8 @@ static RPCHelpMan sendtoaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -484,14 +480,13 @@ static RPCHelpMan sendtoaddress() coin_control.UseCoinJoin(request.params[6].get_bool()); } - - coin_control.m_avoid_address_reuse = GetAvoidReuseFlag(pwallet, request.params[9]); + coin_control.m_avoid_address_reuse = GetAvoidReuseFlag(*pwallet, request.params[9]); // We also enable partial spend avoidance if reuse avoidance is set. coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse; - SetFeeEstimateMode(pwallet, coin_control, request.params[8], request.params[7]); + SetFeeEstimateMode(*pwallet, coin_control, request.params[8], request.params[7]); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); UniValue address_amounts(UniValue::VOBJ); const std::string address = request.params[0].get_str(); @@ -505,7 +500,7 @@ static RPCHelpMan sendtoaddress() ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); bool verbose = request.params[10].isNull() ? false: request.params[10].get_bool(); - return SendMoney(pwallet, coin_control, recipients, mapValue, verbose); + return SendMoney(*pwallet, coin_control, recipients, mapValue, verbose); }, }; } @@ -554,9 +549,8 @@ static RPCHelpMan listaddressgroupings() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -609,9 +603,8 @@ static RPCHelpMan listaddressbalances() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -657,13 +650,12 @@ static RPCHelpMan signmessage() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); std::string strAddress = request.params[0].get_str(); std::string strMessage = request.params[1].get_str(); @@ -763,9 +755,8 @@ static RPCHelpMan getreceivedbyaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -803,9 +794,8 @@ static RPCHelpMan getreceivedbylabel() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -845,9 +835,8 @@ static RPCHelpMan getbalance() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -873,7 +862,7 @@ static RPCHelpMan getbalance() bool include_watchonly = ParseIncludeWatchonly(request.params[3], *pwallet); - bool avoid_reuse = GetAvoidReuseFlag(pwallet, request.params[4]); + bool avoid_reuse = GetAvoidReuseFlag(*pwallet, request.params[4]); const auto bal = pwallet->GetBalance(min_depth, avoid_reuse, fAddLocked); return ValueFromAmount(bal.m_mine_trusted + (include_watchonly ? bal.m_watchonly_trusted : 0)); @@ -890,10 +879,8 @@ static RPCHelpMan getunconfirmedbalance() RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -961,9 +948,8 @@ static RPCHelpMan sendmany() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -991,13 +977,13 @@ static RPCHelpMan sendmany() coin_control.UseCoinJoin(request.params[7].get_bool()); } - SetFeeEstimateMode(pwallet, coin_control, request.params[9], request.params[8]); + SetFeeEstimateMode(*pwallet, coin_control, request.params[9], request.params[8]); std::vector recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); bool verbose = request.params[10].isNull() ? false : request.params[10].get_bool(); - return SendMoney(pwallet, coin_control, recipients, std::move(mapValue), verbose); + return SendMoney(*pwallet, coin_control, recipients, std::move(mapValue), verbose); }, }; } @@ -1035,9 +1021,8 @@ RPCHelpMan addmultisigaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); @@ -1089,7 +1074,7 @@ struct tallyitem } }; -static UniValue ListReceived(const CWallet * const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +static UniValue ListReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { // Minimum confirmations int nMinDepth = 1; @@ -1105,7 +1090,8 @@ static UniValue ListReceived(const CWallet * const pwallet, const UniValue& para fIncludeEmpty = params[2].get_bool(); isminefilter filter = ISMINE_SPENDABLE; - if (ParseIncludeWatchonly(params[3], *pwallet)) { + + if (ParseIncludeWatchonly(params[3], wallet)) { filter |= ISMINE_WATCH_ONLY; } @@ -1121,10 +1107,10 @@ static UniValue ListReceived(const CWallet * const pwallet, const UniValue& para // Tally std::map mapTally; - for (const std::pair& pairWtx : pwallet->mapWallet) { + for (const std::pair& pairWtx : wallet.mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !pwallet->chain().checkFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !wallet.chain().checkFinalTx(*wtx.tx)) continue; int nDepth = wtx.GetDepthInMainChain(); @@ -1141,7 +1127,7 @@ static UniValue ListReceived(const CWallet * const pwallet, const UniValue& para continue; } - isminefilter mine = pwallet->IsMine(address); + isminefilter mine = wallet.IsMine(address); if(!(mine & filter)) continue; @@ -1160,11 +1146,11 @@ static UniValue ListReceived(const CWallet * const pwallet, const UniValue& para // Create m_address_book iterator // If we aren't filtering, go from begin() to end() - auto start = pwallet->m_address_book.begin(); - auto end = pwallet->m_address_book.end(); + auto start = wallet.m_address_book.begin(); + auto end = wallet.m_address_book.end(); // If we are filtering, find() the applicable entry if (has_filtered_address) { - start = pwallet->m_address_book.find(filtered_address); + start = wallet.m_address_book.find(filtered_address); if (start != end) { end = std::next(start); } @@ -1179,7 +1165,7 @@ static UniValue ListReceived(const CWallet * const pwallet, const UniValue& para if (it == mapTally.end() && !fIncludeEmpty) continue; - isminefilter mine = pwallet->IsMine(address); + isminefilter mine = wallet.IsMine(address); if(!(mine & filter)) continue; @@ -1279,9 +1265,8 @@ static RPCHelpMan listreceivedbyaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1289,7 +1274,7 @@ static RPCHelpMan listreceivedbyaddress() LOCK(pwallet->cs_wallet); - return ListReceived(pwallet, request.params, false); + return ListReceived(*pwallet, request.params, false); }, }; } @@ -1323,9 +1308,8 @@ static RPCHelpMan listreceivedbylabel() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1333,7 +1317,7 @@ static RPCHelpMan listreceivedbylabel() LOCK(pwallet->cs_wallet); - return ListReceived(pwallet, request.params, true); + return ListReceived(*pwallet, request.params, true); }, }; } @@ -1356,7 +1340,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) * @param filter_ismine The "is mine" filter flags. * @param filter_label Optional label string to filter incoming transactions. */ -static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { CAmount nFee; std::list listReceived; @@ -1372,21 +1356,21 @@ static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, for (const COutputEntry& s : listSent) { UniValue entry(UniValue::VOBJ); - if (involvesWatchonly || (pwallet->IsMine(s.destination) & ISMINE_WATCH_ONLY)) { + if (involvesWatchonly || (wallet.IsMine(s.destination) & ISMINE_WATCH_ONLY)) { entry.pushKV("involvesWatchonly", true); } MaybePushAddress(entry, s.destination); std::map::const_iterator it = wtx.mapValue.find("DS"); entry.pushKV("category", (it != wtx.mapValue.end() && it->second == "1") ? "coinjoin" : "send"); entry.pushKV("amount", ValueFromAmount(-s.amount)); - const auto* address_book_entry = pwallet->FindAddressBookEntry(s.destination); + const auto* address_book_entry = wallet.FindAddressBookEntry(s.destination); if (address_book_entry) { entry.pushKV("label", address_book_entry->GetLabel()); } entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); if (fLong) - WalletTxToJSON(pwallet->chain(), wtx, entry); + WalletTxToJSON(wallet.chain(), wtx, entry); entry.pushKV("abandoned", wtx.isAbandoned()); ret.push_back(entry); } @@ -1398,7 +1382,7 @@ static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, for (const COutputEntry& r : listReceived) { std::string label; - const auto* address_book_entry = pwallet->FindAddressBookEntry(r.destination); + const auto* address_book_entry = wallet.FindAddressBookEntry(r.destination); if (address_book_entry) { label = address_book_entry->GetLabel(); } @@ -1406,7 +1390,7 @@ static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, continue; } UniValue entry(UniValue::VOBJ); - if (involvesWatchonly || (pwallet->IsMine(r.destination) & ISMINE_WATCH_ONLY)) { + if (involvesWatchonly || (wallet.IsMine(r.destination) & ISMINE_WATCH_ONLY)) { entry.pushKV("involvesWatchonly", true); } MaybePushAddress(entry, r.destination); @@ -1429,7 +1413,7 @@ static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, } entry.pushKV("vout", r.vout); if (fLong) - WalletTxToJSON(pwallet->chain(), wtx, entry); + WalletTxToJSON(wallet.chain(), wtx, entry); ret.push_back(entry); } } @@ -1508,9 +1492,8 @@ static RPCHelpMan listtransactions() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1551,7 +1534,7 @@ static RPCHelpMan listtransactions() for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { CWalletTx *const pwtx = (*it).second; - ListTransactions(pwallet, *pwtx, 0, true, ret, filter, filter_label); + ListTransactions(*pwallet, *pwtx, 0, true, ret, filter, filter_label); if ((int)ret.size() >= (nCount+nFrom)) break; } } @@ -1672,7 +1655,7 @@ static RPCHelpMan listsinceblock() const CWalletTx& tx = pairWtx.second; if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) { - ListTransactions(&wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); + ListTransactions(wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); } } @@ -1689,7 +1672,7 @@ static RPCHelpMan listsinceblock() if (it != wallet.mapWallet.end()) { // We want all transactions regardless of confirmation count to appear here, // even negative confirmation ones, hence the big negative. - ListTransactions(&wallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */); + ListTransactions(wallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */); } } blockId = block.hashPrevBlock; @@ -1764,9 +1747,8 @@ static RPCHelpMan gettransaction() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1803,7 +1785,7 @@ static RPCHelpMan gettransaction() WalletTxToJSON(pwallet->chain(), wtx, entry); UniValue details(UniValue::VARR); - ListTransactions(pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */); + ListTransactions(*pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */); entry.pushKV("details", details); std::string strHex = EncodeHexTx(*wtx.tx); @@ -1838,9 +1820,8 @@ static RPCHelpMan abandontransaction() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1877,9 +1858,8 @@ static RPCHelpMan backupwallet() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -1913,9 +1893,8 @@ static RPCHelpMan keypoolrefill() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; if (pwallet->IsLegacy() && pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); @@ -1931,7 +1910,7 @@ static RPCHelpMan keypoolrefill() kpSize = (unsigned int)request.params[0].get_int(); } - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); pwallet->TopUpKeyPool(kpSize); if (pwallet->GetKeyPoolSize() < (pwallet->IsHDEnabled() ? kpSize * 2 : kpSize)) { @@ -2068,9 +2047,8 @@ static RPCHelpMan walletpassphrasechange() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -2122,9 +2100,8 @@ static RPCHelpMan walletlock() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -2167,9 +2144,8 @@ static RPCHelpMan encryptwallet() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -2241,9 +2217,8 @@ static RPCHelpMan lockunspent() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -2356,9 +2331,8 @@ static RPCHelpMan listlockunspent() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -2397,9 +2371,8 @@ static RPCHelpMan settxfee() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -2603,9 +2576,8 @@ static RPCHelpMan getwalletinfo() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now @@ -2938,9 +2910,8 @@ static RPCHelpMan setwalletflag() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; std::string flag_str = request.params[0].get_str(); @@ -3243,9 +3214,8 @@ static RPCHelpMan listunspent() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; int nMinDepth = 1; if (!request.params[0].isNull()) { @@ -3405,11 +3375,11 @@ static RPCHelpMan listunspent() }; } -void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl) +void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl) { // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now - pwallet->BlockUntilSyncedToCurrentChain(); + wallet.BlockUntilSyncedToCurrentChain(); change_position = -1; bool lockUnspents = false; @@ -3467,7 +3437,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f } const UniValue include_watching_option = options.exists("include_watching") ? options["include_watching"] : options["includeWatching"]; - coinControl.fAllowWatchOnly = ParseIncludeWatchonly(include_watching_option, *pwallet); + coinControl.fAllowWatchOnly = ParseIncludeWatchonly(include_watching_option, wallet); if (options.exists("lockUnspents") || options.exists("lock_unspents")) { lockUnspents = (options.exists("lock_unspents") ? options["lock_unspents"] : options["lockUnspents"]).get_bool(); @@ -3491,12 +3461,12 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f if (options.exists("subtractFeeFromOutputs") || options.exists("subtract_fee_from_outputs") ) subtractFeeFromOutputs = (options.exists("subtract_fee_from_outputs") ? options["subtract_fee_from_outputs"] : options["subtractFeeFromOutputs"]).get_array(); - SetFeeEstimateMode(pwallet, coinControl, options["estimate_mode"], options["conf_target"]); + SetFeeEstimateMode(wallet, coinControl, options["estimate_mode"], options["conf_target"]); } } else { // if options is null and not a bool - coinControl.fAllowWatchOnly = ParseIncludeWatchonly(NullUniValue, *pwallet); + coinControl.fAllowWatchOnly = ParseIncludeWatchonly(NullUniValue, wallet); } if (tx.vout.size() == 0) @@ -3518,7 +3488,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f bilingual_str error; - if (!pwallet->FundTransaction(tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) { + if (!wallet.FundTransaction(tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) { throw JSONRPCError(RPC_WALLET_ERROR, error.original); } } @@ -3586,12 +3556,10 @@ static RPCHelpMan fundrawtransaction() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType(), UniValue::VBOOL}); - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; // parse hex string from parameter CMutableTransaction tx; @@ -3604,7 +3572,7 @@ static RPCHelpMan fundrawtransaction() CCoinControl coin_control; // Automatically select (additional) coins. Can be overridden by options.add_inputs. coin_control.m_add_inputs = true; - FundTransaction(pwallet, tx, fee, change_position, request.params[1], coin_control); + FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(CTransaction(tx))); @@ -3673,9 +3641,8 @@ RPCHelpMan signrawtransactionwithwallet() RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VSTR}, true); - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; CMutableTransaction mtx; if (!DecodeHexTx(mtx, request.params[0].get_str())) { @@ -3684,7 +3651,7 @@ RPCHelpMan signrawtransactionwithwallet() // Sign the transaction LOCK(pwallet->cs_wallet); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); // Fetch previous transactions (inputs): std::map coins; @@ -3731,9 +3698,8 @@ static RPCHelpMan rescanblockchain() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; WalletRescanReserver reserver(*pwallet); if (!reserver.reserve()) { @@ -3908,15 +3874,13 @@ public: } }; -static UniValue DescribeWalletAddress(const CWallet* const pwallet, const CTxDestination& dest) +static UniValue DescribeWalletAddress(const CWallet& wallet, const CTxDestination& dest) { UniValue ret(UniValue::VOBJ); UniValue detail = DescribeAddress(dest); CScript script = GetScriptForDestination(dest); std::unique_ptr provider = nullptr; - if (pwallet) { - provider = pwallet->GetSolvingProvider(script); - } + provider = wallet.GetSolvingProvider(script); ret.pushKVs(detail); ret.pushKVs(std::visit(DescribeWalletAddressVisitor(provider.get()), dest)); return ret; @@ -3983,9 +3947,8 @@ RPCHelpMan getaddressinfo() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -4030,7 +3993,7 @@ RPCHelpMan getaddressinfo() ret.pushKV("iswatchonly", bool(mine & ISMINE_WATCH_ONLY)); - UniValue detail = DescribeWalletAddress(pwallet, dest); + UniValue detail = DescribeWalletAddress(*pwallet, dest); ret.pushKVs(detail); ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); @@ -4094,9 +4057,8 @@ static RPCHelpMan getaddressesbylabel() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -4156,9 +4118,8 @@ static RPCHelpMan listlabels() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LOCK(pwallet->cs_wallet); @@ -4272,9 +4233,8 @@ static RPCHelpMan send() }, true ); - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; UniValue options = request.params[3]; if (options.exists("feeRate") || options.exists("fee_rate") || options.exists("estimate_mode") || options.exists("conf_target")) { @@ -4313,7 +4273,7 @@ static RPCHelpMan send() // Automatically select coins, unless at least one is manually selected. Can // be overriden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; - FundTransaction(pwallet, rawTx, fee, change_position, options, coin_control); + FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control); bool add_to_wallet = true; if (options.exists("add_to_wallet")) { @@ -4384,9 +4344,8 @@ static RPCHelpMan sethdseed() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { // TODO: add mnemonic feature to sethdseed or remove it in favour of upgradetohd - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet, true); @@ -4404,7 +4363,7 @@ static RPCHelpMan sethdseed() throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed. The wallet already has a seed"); } - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); bool flush_key_pool = true; if (!request.params[0].isNull()) { @@ -4612,7 +4571,7 @@ static RPCHelpMan walletcreatefundedpsbt() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; - FundTransaction(&wallet, rawTx, fee, change_position, request.params[3], coin_control); + FundTransaction(*pwallet, rawTx, fee, change_position, request.params[3], coin_control); // Make a blank psbt PartiallySignedTransaction psbtx{rawTx}; @@ -4662,13 +4621,12 @@ static RPCHelpMan upgradewallet() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - CWallet* const pwallet = wallet.get(); + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; RPCTypeCheck(request.params, {UniValue::VNUM}, true); - EnsureWalletIsUnlocked(pwallet); + EnsureWalletIsUnlocked(*pwallet); int version = 0; if (!request.params[0].isNull()) { diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index d27872ba49..5c8c643a43 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -33,7 +33,7 @@ Span GetWalletRPCCommands(); */ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request); -void EnsureWalletIsUnlocked(const CWallet *); +void EnsureWalletIsUnlocked(const CWallet&); WalletContext& EnsureWalletContext(const CoreContext& context); LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false); From aab2a665c34d901ac60b75a33e5c2c3d0929a8e7 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 15 Mar 2021 15:20:19 +0800 Subject: [PATCH 02/10] Merge #20556: rpc: Properly document return values (submitblock, gettxout, getblocktemplate, scantxoutset) fa7ff0790ec21d187f1a32310918b0c8d66e5266 rpc: Properly document submitblock return value (MarcoFalke) fae542c28b269d4a8a39f48ef829734b1241dd4f rpc: Properly document getblocktemplate return value (MarcoFalke) fabaccf031cfac718bf05b140f1901d93ee8be67 rpc: Properly document scantxoutset return value (MarcoFalke) faa2059547389e342991ab0d9382f8694f74fce1 rpc: Properly document gettxout return value (MarcoFalke) Pull request description: Currently a few return values are undocumented. This is causing confusion at the least. See for example #18476 ACKs for top commit: fjahr: utACK fa7ff0790ec21d187f1a32310918b0c8d66e5266 amitiuttarwar: tACK fa7ff0790e Tree-SHA512: 933cb8f003163d93dbedb302d4c162514c2698ec6d58dbb9a053da8b8b9a4459b0701a3d9e830ecdabd7f278a46b7a07a3af49ec60703a80fcd75390877294ea --- src/rpc/blockchain.cpp | 17 ++++++++++++----- src/rpc/mining.cpp | 12 +++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8b4e7b40f5..7ec30ab5e3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1494,9 +1494,9 @@ static RPCHelpMan gettxout() {"n", RPCArg::Type::NUM, RPCArg::Optional::NO, "vout number"}, {"include_mempool", RPCArg::Type::BOOL, /* default */ "true", "Whether to include the mempool. Note that an unspent output that is spent in the mempool won't appear."}, }, - RPCResult{ - RPCResult::Type::OBJ, "", "", - { + { + RPCResult{"If the UTXO was not found", RPCResult::Type::NONE, "", ""}, + RPCResult{"Otherwise", RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at the tip of the chain"}, {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT}, @@ -1512,6 +1512,7 @@ static RPCHelpMan gettxout() }}, {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"}, }}, + }, RPCExamples{ "\nGet unspent transactions\n" + HelpExampleCli("listunspent", "") + @@ -2721,9 +2722,14 @@ static RPCHelpMan scantxoutset() }, "[scanobjects,...]"}, }, - RPCResult{ - RPCResult::Type::OBJ, "", "", + { + RPCResult{"When action=='abort'", RPCResult::Type::BOOL, "", ""}, + RPCResult{"When action=='status' and no scan is in progress", RPCResult::Type::NONE, "", ""}, + RPCResult{"When action=='status' and scan is in progress", RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::NUM, "progress", "The scan progress"}, + }}, + RPCResult{"When action=='start'", RPCResult::Type::OBJ, "", "", { {RPCResult::Type::BOOL, "success", "Whether the scan was completed"}, {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs scanned"}, {RPCResult::Type::NUM, "height", "The current block height (index)"}, @@ -2742,6 +2748,7 @@ static RPCHelpMan scantxoutset() }}, {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of all found unspent outputs in " + CURRENCY_UNIT}, }}, + }, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9e75dabfe3..cb733a55e1 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -579,8 +579,10 @@ static RPCHelpMan getblocktemplate() }, "\"template_request\""}, }, - RPCResult{ - RPCResult::Type::OBJ, "", "", + { + RPCResult{"If the proposal was accepted with mode=='proposal'", RPCResult::Type::NONE, "", ""}, + RPCResult{"If the proposal was not accepted with mode=='proposal'", RPCResult::Type::STR, "", "According to BIP22"}, + RPCResult{"Otherwise", RPCResult::Type::OBJ, "", "", { {RPCResult::Type::ARR, "capabilities", "specific client side supported features", { @@ -655,6 +657,7 @@ static RPCHelpMan getblocktemplate() {RPCResult::Type::BOOL, "superblocks_enabled", "true, if superblock payments are enabled"}, {RPCResult::Type::STR_HEX, "coinbase_payload", "coinbase transaction payload data encoded in hexadecimal"}, }}, + }, RPCExamples{ HelpExampleCli("getblocktemplate", "") + HelpExampleRpc("getblocktemplate", "") @@ -1019,7 +1022,10 @@ static RPCHelpMan submitblock() {"hexdata", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hex-encoded block data to submit"}, {"dummy", RPCArg::Type::STR, /* default */ "ignored", "dummy value, for compatibility with BIP22. This value is ignored."}, }, - RPCResult{RPCResult::Type::NONE, "", "Returns JSON Null when valid, a string according to BIP22 otherwise"}, + { + RPCResult{"If the block was accepted", RPCResult::Type::NONE, "", ""}, + RPCResult{"Otherwise", RPCResult::Type::STR, "", "According to BIP22"}, + }, RPCExamples{ HelpExampleCli("submitblock", "\"mydata\"") + HelpExampleRpc("submitblock", "\"mydata\"") From a3bee9c8eca8b8b3edb55ed6867b3792ccf39414 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 15 Mar 2021 17:36:13 +0100 Subject: [PATCH 03/10] Merge #21424: Net processing: Tidy up CNodeState ctor 6927933782acb9b158787e6f35debb916793f6b1 [net processing] Add ChainSyncTimeoutState default initializers (John Newbery) 55966e0cc03f0e380d21a9434b048d4d515b6729 [net processing] Remove CNodeState ctor body (John Newbery) Pull request description: This addresses the two outstanding review comments from #21370. ACKs for top commit: practicalswift: cr ACK 6927933782acb9b158787e6f35debb916793f6b1: patch looks correct hebasto: ACK 6927933782acb9b158787e6f35debb916793f6b1, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: b3ef5c8a096e447887df255406b3a760f01c73e2b942374595416b4b4031fc69b89cd93168c45040489d581f340b2a62d3fbabd207d4307f587c00a7a7daacd1 --- src/net_processing.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6511ed614b..5670e5f3eb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -826,7 +826,7 @@ struct CNodeState { bool m_protect{false}; }; - ChainSyncTimeoutState m_chain_sync{0, nullptr, false, false}; + ChainSyncTimeoutState m_chain_sync; //! Time of last new block announcement int64_t m_last_block_announcement{0}; @@ -900,11 +900,7 @@ struct CNodeState { //! A rolling bloom filter of all announced tx CInvs to this peer. CRollingBloomFilter m_recently_announced_invs = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; - CNodeState(bool is_inbound) : - m_is_inbound(is_inbound) - { - m_recently_announced_invs.reset(); - } + CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} }; // Keeps track of the time (in microseconds) when transactions were requested last time From 709652bff777cb0a02f1d0118dc5dd158cdf27be Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 15 Mar 2021 19:46:59 +0100 Subject: [PATCH 04/10] Merge #21141: wallet: Add new format string placeholders for walletnotify 06e1fb0b170a69996a7ce1ef5203785a7bc6b278 Add new format string placeholders for walletnotify to include relevant block information for transactions (Maayan Keshet) Pull request description: This patch includes two new format placeholders for walletnotify: %b - the hash of the block containting the transaction (zeroed if a mempool transaction) %h - the height of the block containing the transaction (zero if a mempool transaction) I've included test suite changes to check and validate the above functional requirements as well as doc/help description changes. **Motivation** The walletnotify option is used to be notified of new transactions relevant to the wallet of the node. A common usage pattern is to perform afterwards additional RPC calls to determine: 1. If this is a mempool transaction or not (i.e. are there any confirmations?) 2. What block was it included in? 3. Did this transaction was seen before and is now seen again because of a fork? All of these questions can be answered with the current features, but the resulting RPC calls may be expensive in a heavily used node. As this information is readily available when calling the walletnotify callback, it makes sense to save expensive round trips by optionally sending this information at that point in time. I can definitely say we would like to use it in Fireblocks, my employer. Please let me know of any questions and suggestions. ACKs for top commit: laanwj: ACK 06e1fb0b170a69996a7ce1ef5203785a7bc6b278 Tree-SHA512: d2744e2a7a883f9c3a9fd32237110e048c4b6b11fea8221c33d10b74157f65bbc4351211f441e8c1a4af5d5d38e2ba6b1943a7673dc18860c0553d7b41e00775 --- src/wallet/init.cpp | 2 +- src/wallet/wallet.cpp | 8 +++++ test/functional/feature_notifications.py | 38 +++++++++++++++++------- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index e81e06fdc9..15b3306f82 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -69,7 +69,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletbroadcast", strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-walletdir=", "Specify directory to hold wallets (default: /wallets if it exists, otherwise )", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET); #if HAVE_SYSTEM - argsman.AddArg("-walletnotify=", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID and %w is replaced by wallet name. %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-walletnotify=", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif argsman.AddArg("-discardfee=", strprintf("The fee rate (in %s/kB) that indicates your tolerance for discarding change by adding it to the fee (default: %s). " diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 36cdf3f171..31c050cf16 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -938,6 +938,14 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio if (!strCmd.empty()) { ReplaceAll(strCmd, "%s", hash.GetHex()); + if (confirm.status == CWalletTx::Status::CONFIRMED) + { + ReplaceAll(strCmd, "%b", confirm.hashBlock.GetHex()); + ReplaceAll(strCmd, "%h", ToString(confirm.block_height)); + } else { + ReplaceAll(strCmd, "%b", "unconfirmed"); + ReplaceAll(strCmd, "%h", "-1"); + } #ifndef WIN32 // Substituting the wallet name isn't currently supported on windows // because windows shell escaping has not been implemented yet: diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index e6d91e438b..f98fcab8d8 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -19,7 +19,7 @@ from test_framework.util import ( FILE_CHAR_START = 32 if os.name == 'nt' else 1 FILE_CHAR_END = 128 FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if os.name == 'nt' else '/' - +UNCONFIRMED_HASH_STRING = 'unconfirmed' def notify_outputname(walletname, txid): return txid if os.name == 'nt' else '{}_{}'.format(walletname, txid) @@ -46,7 +46,7 @@ class NotificationsTest(DashTestFramework): self.extra_args[0].append("-alertnotify=echo > {}".format(os.path.join(self.alertnotify_dir, '%s'))) self.extra_args[0].append("-blocknotify=echo > {}".format(os.path.join(self.blocknotify_dir, '%s'))) self.extra_args[1].append("-rescan") - self.extra_args[1].append("-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s')))) + self.extra_args[1].append("-walletnotify=echo %h_%b > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s')))) # -chainlocknotify on node0, -instantsendnotify on node1 self.extra_args[0].append("-chainlocknotify=echo > {}".format(os.path.join(self.chainlocknotify_dir, '%s'))) @@ -78,12 +78,9 @@ class NotificationsTest(DashTestFramework): self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) # directory content should equal the generated transaction hashes - txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count))) - assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) + tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count))) self.stop_node(1) - - for tx_file in os.listdir(self.walletnotify_dir): - os.remove(os.path.join(self.walletnotify_dir, tx_file)) + self.expect_wallet_notify(tx_details) self.log.info("test -walletnotify after rescan") # restart node to rescan to force wallet notifications @@ -94,10 +91,8 @@ class NotificationsTest(DashTestFramework): self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) # directory content should equal the generated transaction hashes - txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count))) - assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) - for tx_file in os.listdir(self.walletnotify_dir): - os.remove(os.path.join(self.walletnotify_dir, tx_file)) + tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count))) + self.expect_wallet_notify(tx_details) self.log.info("test -chainlocknotify") @@ -145,6 +140,27 @@ class NotificationsTest(DashTestFramework): # TODO: add test for `-alertnotify` large fork notifications + def expect_wallet_notify(self, tx_details): + self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_details), timeout=10) + # Should have no more and no less files than expected + assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id, _, _ in tx_details), sorted(os.listdir(self.walletnotify_dir))) + # Should now verify contents of each file + for tx_id, blockheight, blockhash in tx_details: + fname = os.path.join(self.walletnotify_dir, notify_outputname(self.wallet, tx_id)) + with open(fname, 'rt', encoding='utf-8') as f: + text = f.read() + # Universal newline ensures '\n' on 'nt' + assert_equal(text[-1], '\n') + text = text[:-1] + if os.name == 'nt': + # On Windows, echo as above will append a whitespace + assert_equal(text[-1], ' ') + text = text[:-1] + expected = str(blockheight) + '_' + blockhash + assert_equal(text, expected) + + for tx_file in os.listdir(self.walletnotify_dir): + os.remove(os.path.join(self.walletnotify_dir, tx_file)) if __name__ == '__main__': NotificationsTest().main() From ddc6fca7f3ab93d43f800f095fce1d88ecb77b35 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 18 Mar 2021 11:32:33 +0800 Subject: [PATCH 05/10] Merge #21343: doc: revamp macOS build doc c180c911b88b2bd2baf2c9c2b24e276787ffb69b doc: revamp macOS build doc (Jarol Rodriguez) Pull request description: This PR makes the macOS build-docs more informative and adds in the following information: - Proper descriptions and delineation of required/optional dependencies - walk-through of optional dependencies - configuration walk-through - various other tidbits of information This is a part of the efforts done in https://github.com/bitcoin/bitcoin/pull/20601 and https://github.com/bitcoin/bitcoin/pull/20610 to update the docs and introduce some consistency between them. This update does not add instructions for arm-based M1 Macbooks as I do not have one to test with. It would be nice to have someone follow up with an update containing instructions for arm-based Macs. **Before/Master:** [render](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md) **After/PR:** [render](https://github.com/bitcoin/bitcoin/blob/c180c911b88b2bd2baf2c9c2b24e276787ffb69b/doc/build-osx.md) ACKs for top commit: fanquake: ACK c180c911b88b2bd2baf2c9c2b24e276787ffb69b - I still think these are getting too verbose and we're duplicating information all over the place; dependencies, configure options, combinations of options etc. However if people are happy to maintain them, I guess it's fine for now, and this revamping has already happened for some of the other build READMEs. Tree-SHA512: 1440046c723fe80d4158e4a429e3aa8bd93570acb84ad202d5d24c749ab9a89a3aca8b61b49e75e042a4bf4317acd632d3906e1b5808a9052e74209256528b45 --- doc/build-osx.md | 336 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 263 insertions(+), 73 deletions(-) diff --git a/doc/build-osx.md b/doc/build-osx.md index 8a78ed7072..eaae458475 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -1,51 +1,238 @@ -# macOS Build Instructions and Notes +# macOS Build Guide + +**Updated for MacOS [11.2](https://www.apple.com/macos/big-sur/)** + +This guide describes how to build dashd, command-line utilities, and GUI on macOS + +**Note:** The following is for Intel Macs only! + +## Dependencies + +The following dependencies are **required**: + +Library | Purpose | Description +-----------------------------------------------------------|------------|---------------------- +[automake](https://formulae.brew.sh/formula/automake) | Build | Generate makefile +[libtool](https://formulae.brew.sh/formula/libtool) | Build | Shared library support +[pkg-config](https://formulae.brew.sh/formula/pkg-config) | Build | Configure compiler and linker flags +[boost](https://formulae.brew.sh/formula/boost) | Utility | Library for threading, data structures, etc +[libevent](https://formulae.brew.sh/formula/libevent) | Networking | OS independent asynchronous networking + +The following dependencies are **optional**: + +Library | Purpose | Description +--------------------------------------------------------------- |------------------|---------------------- +[berkeley-db@4](https://formulae.brew.sh/formula/berkeley-db@4) | Berkeley DB | Wallet storage (only needed when wallet enabled) +[qt@5](https://formulae.brew.sh/formula/qt@5) | GUI | GUI toolkit (only needed when GUI enabled) +[qrencode](https://formulae.brew.sh/formula/qrencode) | QR codes in GUI | Generating QR codes (only needed when GUI enabled) +[zeromq](https://formulae.brew.sh/formula/zeromq) | ZMQ notification | Allows generating ZMQ notifications (requires ZMQ version >= 4.0.0) +[sqlite](https://formulae.brew.sh/formula/sqlite) | SQLite DB | Wallet storage (only needed when wallet enabled) +[miniupnpc](https://formulae.brew.sh/formula/miniupnpc) | UPnP Support | Firewall-jumping support (needed for port mapping support) +[libnatpmp](https://formulae.brew.sh/formula/libnatpmp) | NAT-PMP Support | Firewall-jumping support (needed for port mapping support) +[python3](https://formulae.brew.sh/formula/python@3.9) | Testing | Python Interpreter (only needed when running the test suite) + +The following dependencies are **optional** packages required for deploying: + +Library | Purpose | Description +----------------------------------------------------|------------------|---------------------- +[librsvg](https://formulae.brew.sh/formula/librsvg) | Deploy Dependency| Library to render SVG files +[ds_store](https://pypi.org/project/ds-store/) | Deploy Dependency| Examine and modify .DS_Store files +[mac_alias](https://pypi.org/project/mac-alias/) | Deploy Dependency| Generate/Read binary alias and bookmark records + +See [dependencies.md](dependencies.md) for a complete overview. + +## Preparation The commands in this guide should be executed in a Terminal application. -The built-in one is located in +macOS comes with a built-in Terminal located in: + ``` /Applications/Utilities/Terminal.app ``` -## Preparation -Install the macOS command line tools: +### 1. Xcode Command Line Tools -```shell +The Xcode Command Line Tools are a collection of build tools for macOS. +These tools must be installed in order to build Dash Core from source. + +To install, run the following command from your terminal: + +``` bash xcode-select --install ``` -When the popup appears, click `Install`. +Upon running the command, you should see a popup appear. +Click on `Install` to continue the installation process. -Then install [Homebrew](https://brew.sh). +### 2. Homebrew Package Manager -## Dependencies -```shell -brew install automake libtool boost gmp miniupnpc pkg-config python qt@5 libevent libnatpmp qrencode +Homebrew is a package manager for macOS that allows one to install packages from the command line easily. +While several package managers are available for macOS, this guide will focus on Homebrew as it is the most popular. +Since the examples in this guide which walk through the installation of a package will use Homebrew, it is recommended that you install it to follow along. +Otherwise, you can adapt the commands to your package manager of choice. + +To install the Homebrew package manager, see: https://brew.sh + +Note: If you run into issues while installing Homebrew or pulling packages, refer to [Homebrew's troubleshooting page](https://docs.brew.sh/Troubleshooting). + +### 3. Install Required Dependencies + +The first step is to download the required dependencies. +These dependencies represent the packages required to get a barebones installation up and running. +To install, run the following from your terminal: + +``` bash +brew install automake libtool boost gmp pkg-config libevent ``` -If you run into issues, check [Homebrew's troubleshooting page](https://docs.brew.sh/Troubleshooting). -See [dependencies.md](dependencies.md) for a complete overview. +### 4. Clone Dash repository -The wallet support requires one or both of the dependencies ([*SQLite*](#sqlite) and [*Berkeley DB*](#berkeley-db)) in the sections below. -To build Dash Core without wallet, see [*Disable-wallet mode*](#disable-wallet-mode). +`git` should already be installed by default on your system. +Now that all the required dependencies are installed, let's clone the Dash Core repository to a directory. +All build scripts and commands will run from this directory. -#### SQLite +``` bash +git clone https://github.com/dashpay/dash.git +``` -Usually, macOS installation already has a suitable SQLite installation. -Also, the Homebrew package could be installed: +### 5. Install Optional Dependencies -```shell +#### Wallet Dependencies + +It is not necessary to build wallet functionality to run `dashd` or `dash-qt`. +To enable legacy wallets, you must install `berkeley-db@4`. +To enable [descriptor wallets](https://github.com/dashpay/dash/blob/master/doc/descriptors.md), `sqlite` is required. +Skip `berkeley-db@4` if you intend to *exclusively* use descriptor wallets. + +###### Legacy Wallet Support + +`berkeley-db@4` is required to enable support for legacy wallets. +Skip if you don't intend to use legacy wallets. + +``` bash +brew install berkeley-db@4 +``` + +###### Descriptor Wallet Support + +Note: Apple has included a useable `sqlite` package since macOS 10.14. +You may not need to install this package. + +`sqlite` is required to enable support for descriptor wallets. +Skip if you don't intend to use descriptor wallets. + +``` bash brew install sqlite ``` +--- -In that case the Homebrew package will prevail. +#### GUI Dependencies -If you want to build the disk image with `make deploy` (.dmg / optional), you need: -[`macdeployqtplus`](../contrib/macdeploy/README.md) dependencies: -```shell +###### Qt + +Dash Core includes a GUI built with the cross-platform Qt Framework. +To compile the GUI, we need to install `qt@5`. +Skip if you don't intend to use the GUI. + +``` bash +brew install qt@5 +``` + +Note: Building with Qt binaries downloaded from the Qt website is not officially supported. +See the notes in [#7714](https://github.com/dashpay/dash/issues/7714). + +###### qrencode + +The GUI can encode addresses in a QR Code. To build in QR support for the GUI, install `qrencode`. +Skip if not using the GUI or don't want QR code functionality. + +``` bash +brew install qrencode +``` +--- + +#### Port Mapping Dependencies + +###### miniupnpc + +miniupnpc may be used for UPnP port mapping. +Skip if you do not need this functionality. + +``` bash +brew install miniupnpc +``` + +###### libnatpmp + +libnatpmp may be used for NAT-PMP port mapping. +Skip if you do not need this functionality. + +``` bash +brew install libnatpmp +``` + +Note: UPnP and NAT-PMP support will be compiled in and disabled by default. +Check out the [further configuration](#further-configuration) section for more information. + +--- + +#### ZMQ Dependencies + +Support for ZMQ notifications requires the following dependency. +Skip if you do not need ZMQ functionality. + +``` bash +brew install zeromq +``` + +ZMQ is automatically compiled in and enabled if the dependency is detected. +Check out the [further configuration](#further-configuration) section for more information. + +For more information on ZMQ, see: [zmq.md](zmq.md) + +--- + +#### Test Suite Dependencies + +There is an included test suite that is useful for testing code changes when developing. +To run the test suite (recommended), you will need to have Python 3 installed: + +``` bash +brew install python +``` + +--- + +#### Deploy Dependencies + +You can deploy a `.dmg` containing the Dash Core application using `make deploy`. +This command depends on a couple of python packages, so it is required that you have `python` installed. + +Ensuring that `python` is installed, you can install the deploy dependencies by running the following commands in your terminal: + +``` bash pip3 install ds_store mac_alias ``` -#### Berkeley DB +## Building Dash Core + +### 1. Configuration + +There are many ways to configure Dash Core, here are a few common examples: + +##### Wallet (BDB + SQlite) Support, No GUI: + +If `berkeley-db@4` is installed, then legacy wallet support will be built. +If `berkeley-db@4` is not installed, then this will throw an error. +If `sqlite` is installed, then descriptor wallet support will also be built. +Additionally, this explicitly disables the GUI. + +``` bash +./autogen.sh +./configure --with-gui=no +``` + +###### Berkeley DB It is recommended to use Berkeley DB 4.8. If you have to build it yourself, you can use [the installation script included in contrib/](contrib/install_db4.sh) @@ -55,59 +242,68 @@ like so: ./contrib/install_db4.sh . ``` -from the root of the repository. +##### Wallet (only SQlite) and GUI Support: -Also, the Homebrew package could be installed: +This explicitly enables the GUI and disables legacy wallet support. +If `qt` is not installed, this will throw an error. +If `sqlite` is installed then descriptor wallet functionality will be built. +If `sqlite` is not installed, then wallet functionality will be disabled. -```shell -brew install berkeley-db4 +``` bash +./autogen.sh +./configure --without-bdb --with-gui=yes ``` -## Build Dash Core +##### No Wallet or GUI -1. Clone the Dash Core source code: - ```shell - git clone https://github.com/dashpay/dash - cd dash - ``` - -2. Build Dash Core: - - Configure and build the headless Dash Core binaries as well as the GUI (if Qt is found). - - You can disable the GUI build by passing `--without-gui` to configure. - ```shell - ./autogen.sh - ./configure - make - ``` - -3. It is recommended to build and run the unit tests: - ```shell - make check - ``` - -4. You can also create a `.dmg` that contains the `.app` bundle (optional): - ```shell - make deploy - ``` - -## Disable-wallet mode -When the intention is to run only a P2P node without a wallet, Dash Core may be -compiled in disable-wallet mode with: -```shell -./configure --disable-wallet +``` bash +./autogen.sh +./configure --without-wallet --with-gui=no ``` -In this case there is no dependency on [*Berkeley DB*](#berkeley-db) and [*SQLite*](#sqlite). +##### Further Configuration -Mining is also possible in disable-wallet mode using the `getblocktemplate` RPC call. +You may want to dig deeper into the configuration options to achieve your desired behavior. +Examine the output of the following command for a full list of configuration options: -## Running +``` bash +./configure -help +``` -Dash Core is now available at `./src/dashd` +### 2. Compile + +After configuration, you are ready to compile. +Run the following in your terminal to compile Dash Core: + +``` bash +make -jx # use -jX here for parallelism +make check # Run tests if Python 3 is available +``` + +### 3. Deploy (optional) + +You can also create a `.dmg` containing the `.app` bundle by running the following command: + +``` bash +make deploy +``` + +## Running Dash Core + +Dash Core should now be available at `./src/dashd`. +If you compiled support for the GUI, it should be available at `./src/qt/dash-qt`. + +The first time you run `dashd` or `dash-qt`, it will start downloading the blockchain. +This process could take many hours, or even days on slower than average systems. + +By default, blockchain and wallet data files will be stored in: + +``` bash +/Users/${USER}/Library/Application Support/Dash/ +``` Before running, you may create an empty configuration file: + ```shell mkdir -p "/Users/${USER}/Library/Application Support/DashCore" @@ -116,9 +312,8 @@ touch "/Users/${USER}/Library/Application Support/DashCore/dash.conf" chmod 600 "/Users/${USER}/Library/Application Support/DashCore/dash.conf" ``` -The first time you run dashd, it will start downloading the blockchain. This process could take many hours, or even days on slower than average systems. - You can monitor the download process by looking at the debug.log file: + ```shell tail -f $HOME/Library/Application\ Support/DashCore/debug.log ``` @@ -126,13 +321,8 @@ tail -f $HOME/Library/Application\ Support/DashCore/debug.log ## Other commands: ```shell -./src/dashd -daemon # Starts the dash daemon. +./src/dashd -daemon # Starts the dashd daemon. ./src/dash-cli --help # Outputs a list of command-line options. ./src/dash-cli help # Outputs a list of RPC commands when the daemon is running. +./src/qt/dash-qt -server # Starts the dash-qt server mode, allows dash-cli control ``` - -## Notes - -* Tested on OS X 10.12 Sierra through macOS 10.15 Catalina on 64-bit Intel -processors only. -* Building with downloaded Qt binaries is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714). From b55fdf8e68a36d60b2468a6d01d16ebb41511e35 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 19 Mar 2021 18:56:12 +0100 Subject: [PATCH 06/10] Merge #21235: p2p: Clarify disconnect log message in ProcessGetBlockData, remove send bool fa8177324392c923c6ce39056cfd870af55ab673 style-only: Remove whitespace (MarcoFalke) fae77b9e6dc9e59b355d56df49c4d9685b6f40a4 net: Simplify ProcessGetBlockData execution by removing send flag. (Patrick Strateman) fae7c0429f96e08bcac944f6fa30264636dfda8c log: Clarify that block request below NODE_NETWORK_LIMITED_MIN_BLOCKS disconnects (MarcoFalke) Pull request description: * Clarify that "ignoring" really means "disconnect" in the log * Revive a refactor I took from #13670 ACKs for top commit: jnewbery: utACK fa8177324392c923c6ce39056cfd870af55ab673 sipa: utACK fa8177324392c923c6ce39056cfd870af55ab673 Tree-SHA512: 0a4fcb979cb82c4e26012881eeaf903c38dfbb85d461476c01e35294760744746a79c48ffad827fe31c1b830f40c6e4240529c71e375146e4d0313c3b7d784ca --- src/net_processing.cpp | 168 ++++++++++++++++++++--------------------- 1 file changed, 82 insertions(+), 86 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5670e5f3eb..d259a1c1d0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2381,7 +2381,6 @@ void PeerManagerImpl::RelayAddress(NodeId originator, void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& inv, llmq::CInstantSendManager& isman) { - bool send = false; std::shared_ptr a_recent_block; std::shared_ptr a_recent_compact_block; { @@ -2415,115 +2414,112 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& LOCK(cs_main); const CBlockIndex* pindex = m_chainman.m_blockman.LookupBlockIndex(inv.hash); - if (pindex) { - send = BlockRequestAllowed(pindex); - if (!send) { - LogPrint(BCLog::NET,"%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId()); - } + if (!pindex) { + return; + } + if (!BlockRequestAllowed(pindex)) { + LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId()); + return; } const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); // disconnect node in case we have reached the outbound limit for serving historical blocks - if (send && - m_connman.OutboundTargetReached(true) && + if (m_connman.OutboundTargetReached(true) && (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && !pfrom.HasPermission(NetPermissionFlags::Download) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); - - //disconnect node pfrom.fDisconnect = true; - send = false; + return; } // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold - if (send && !pfrom.HasPermission(NetPermissionFlags::NoBan) && ( + if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && ( (((pfrom.GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom.GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) )) { - LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom.GetId()); - + LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, disconnect peer=%d\n", pfrom.GetId()); //disconnect node and prevent it from stalling (would otherwise wait for the missing block) pfrom.fDisconnect = true; - send = false; + return; } // Pruned nodes may have deleted the block, so check whether // it's available before trying to send. - if (send && (pindex->nStatus & BLOCK_HAVE_DATA)) - { - std::shared_ptr pblock; - if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { - pblock = a_recent_block; - } else { - // Send block from disk - std::shared_ptr pblockRead = std::make_shared(); - if (!ReadBlockFromDisk(*pblockRead, pindex, m_chainparams.GetConsensus())) - assert(!"cannot load block from disk"); - pblock = pblockRead; - } - if (pblock) { - if (inv.IsMsgBlk()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); - } else if (inv.IsMsgFilteredBlk()) { - bool sendMerkleBlock = false; - CMerkleBlock merkleBlock; - if (!pfrom.IsBlockOnlyConn()) { - LOCK(peer.m_tx_relay->m_bloom_filter_mutex); - if (peer.m_tx_relay->m_bloom_filter) { - sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *peer.m_tx_relay->m_bloom_filter); + if (!(pindex->nStatus & BLOCK_HAVE_DATA)) { + return; + } + std::shared_ptr pblock; + if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { + pblock = a_recent_block; + } else { + // Send block from disk + std::shared_ptr pblockRead = std::make_shared(); + if (!ReadBlockFromDisk(*pblockRead, pindex, m_chainparams.GetConsensus())) + assert(!"cannot load block from disk"); + pblock = pblockRead; + } + if (pblock) { + if (inv.IsMsgBlk()) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); + } else if (inv.IsMsgFilteredBlk()) { + bool sendMerkleBlock = false; + CMerkleBlock merkleBlock; + if (!pfrom.IsBlockOnlyConn()) { + LOCK(peer.m_tx_relay->m_bloom_filter_mutex); + if (peer.m_tx_relay->m_bloom_filter) { + sendMerkleBlock = true; + merkleBlock = CMerkleBlock(*pblock, *peer.m_tx_relay->m_bloom_filter); + } + } + if (sendMerkleBlock) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); + // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see + // This avoids hurting performance by pointlessly requiring a round-trip + // Note that there is currently no way for a node to request any single transactions we didn't send here - + // they must either disconnect and retry or request the full block. + // Thus, the protocol spec specified allows for us to provide duplicate txn here, + // however we MUST always provide at least what the remote peer needs + typedef std::pair PairType; + for (PairType &pair : merkleBlock.vMatchedTxn) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::TX, *pblock->vtx[pair.first])); + } + for (PairType &pair : merkleBlock.vMatchedTxn) { + auto islock = isman.GetInstantSendLockByTxid(pair.second); + if (islock != nullptr) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::ISDLOCK, *islock)); } } - if (sendMerkleBlock) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); - // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see - // This avoids hurting performance by pointlessly requiring a round-trip - // Note that there is currently no way for a node to request any single transactions we didn't send here - - // they must either disconnect and retry or request the full block. - // Thus, the protocol spec specified allows for us to provide duplicate txn here, - // however we MUST always provide at least what the remote peer needs - typedef std::pair PairType; - for (PairType &pair : merkleBlock.vMatchedTxn) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::TX, *pblock->vtx[pair.first])); - } - for (PairType &pair : merkleBlock.vMatchedTxn) { - auto islock = isman.GetInstantSendLockByTxid(pair.second); - if (islock != nullptr) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::ISDLOCK, *islock)); - } - } - } - // else - // no response - } else if (inv.IsMsgCmpctBlk()) { - // If a peer is asking for old blocks, we're almost guaranteed - // they won't have a useful mempool to match against a compact block, - // and we don't feel like constructing the object for them, so - // instead we respond with the full, non-compact block. - if (CanDirectFetch() && - pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { - if (a_recent_compact_block && - a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); - } else { - CBlockHeaderAndShortTxIDs cmpctblock{*pblock}; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); - } + } + // else + // no response + } else if (inv.IsMsgCmpctBlk()) { + // If a peer is asking for old blocks, we're almost guaranteed + // they won't have a useful mempool to match against a compact block, + // and we don't feel like constructing the object for them, so + // instead we respond with the full, non-compact block. + if (CanDirectFetch() && + pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { + if (a_recent_compact_block && + a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); } else { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); + CBlockHeaderAndShortTxIDs cmpctblock{*pblock}; + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } + } else { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); } } + } - { - LOCK(peer.m_block_inv_mutex); - // Trigger the peer node to send a getblocks request for the next batch of inventory - if (inv.hash == peer.m_continuation_block) { - // Send immediately. This must send even if redundant, - // and we want it right after the last block so they don't - // wait for other stuff first. - std::vector vInv; - vInv.push_back(CInv(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash())); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); - peer.m_continuation_block.SetNull(); - } + { + LOCK(peer.m_block_inv_mutex); + // Trigger the peer node to send a getblocks request for the next batch of inventory + if (inv.hash == peer.m_continuation_block) { + // Send immediately. This must send even if redundant, + // and we want it right after the last block so they don't + // wait for other stuff first. + std::vector vInv; + vInv.push_back(CInv(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash())); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + peer.m_continuation_block.SetNull(); } } } From 1cc6aa6c836c09bd648c328b9d0bd1de0fa24fb8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 20 Apr 2021 07:14:57 +0200 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#21691: test: Check that no versionbits are re-used fa8eaee6a8531db970cc84436bf2ae8150a58642 test: Run versionbits_sanity for all chains (MarcoFalke) faec1e9ee1f12612831ad5b0f0a767d87bd2d024 test: Address outstanding versionbits_test feedback (MarcoFalke) fad4167871c3c9fde462e64e3ef3be937e585084 test: Check that no versionbits are re-used (MarcoFalke) Pull request description: ACKs for top commit: jnewbery: Code review ACK fa8eaee6a8531db970cc84436bf2ae8150a58642 ajtowns: ACK fa8eaee6a8531db970cc84436bf2ae8150a58642 code review only Tree-SHA512: e99ffcca8970921fd07fa9e04cf1ea2515a317409865d34ddfd70be0f0b0616b29d1fad58262d96a3c3418c0cf7018a6a955802a178b8f78f6ecfaa30a37d91c --- src/test/versionbits_tests.cpp | 65 +++++++++++++--------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 104b55b138..af31496fc5 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -83,11 +83,9 @@ class VersionBitsTester TestNeverActiveConditionChecker checker_never[CHECKERS]; // Test counter (to identify failures) - int num; + int num{1000}; public: - VersionBitsTester() : num(1000) {} - VersionBitsTester& Reset() { // Have each group of tests be counted by the 1000s part, starting at 1000 num = num - (num % 1000) + 1000; @@ -259,39 +257,6 @@ BOOST_AUTO_TEST_CASE(versionbits_test) } } -BOOST_AUTO_TEST_CASE(versionbits_sanity) -{ - // Sanity checks of version bit deployments - const auto chainParams = CreateChainParams(*m_node.args, CBaseChainParams::MAIN); - const Consensus::Params &mainnetParams = chainParams->GetConsensus(); - for (int i=0; i<(int) Consensus::MAX_VERSION_BITS_DEPLOYMENTS; i++) { - uint32_t bitmask = g_versionbitscache.Mask(mainnetParams, static_cast(i)); - // Make sure that no deployment tries to set an invalid bit. - BOOST_CHECK_EQUAL(bitmask & ~(uint32_t)VERSIONBITS_TOP_MASK, bitmask); - - // Check min_activation_height is on a retarget boundary - BOOST_CHECK_EQUAL(mainnetParams.vDeployments[i].min_activation_height % mainnetParams.nMinerConfirmationWindow, 0); - // Check min_activation_height is 0 for ALWAYS_ACTIVE and never active deployments - if (mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) { - BOOST_CHECK_EQUAL(mainnetParams.vDeployments[i].min_activation_height, 0); - } - - // Verify that the deployment windows of different deployment using the - // same bit are disjoint. - // This test may need modification at such time as a new deployment - // is proposed that reuses the bit of an activated soft fork, before the - // end time of that soft fork. (Alternatively, the end time of that - // activated soft fork could be later changed to be earlier to avoid - // overlap.) - for (int j=i+1; j<(int) Consensus::MAX_VERSION_BITS_DEPLOYMENTS; j++) { - if (g_versionbitscache.Mask(mainnetParams, static_cast(j)) == bitmask) { - BOOST_CHECK(mainnetParams.vDeployments[j].nStartTime > mainnetParams.vDeployments[i].nTimeout || - mainnetParams.vDeployments[i].nStartTime > mainnetParams.vDeployments[j].nTimeout); - } - } - } -} - /** Check that ComputeBlockVersion will set the appropriate bit correctly */ static void check_computeblockversion(const Consensus::Params& params, Consensus::DeploymentPos dep) { @@ -313,16 +278,25 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus BOOST_CHECK_EQUAL(g_versionbitscache.ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS); // always/never active deployments shouldn't need to be tested further - if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE) return; - if (nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return; + if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || + nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) + { + BOOST_CHECK_EQUAL(min_activation_height, 0); + return; + } BOOST_REQUIRE(nStartTime < nTimeout); BOOST_REQUIRE(nStartTime >= 0); BOOST_REQUIRE(nTimeout <= std::numeric_limits::max() || nTimeout == Consensus::BIP9Deployment::NO_TIMEOUT); BOOST_REQUIRE(0 <= bit && bit < 32); + // Make sure that no deployment tries to set an invalid bit. BOOST_REQUIRE(((1 << bit) & VERSIONBITS_TOP_MASK) == 0); BOOST_REQUIRE(min_activation_height >= 0); - BOOST_REQUIRE_EQUAL(min_activation_height % window_size, 0); + // Check min_activation_height is on a retarget boundary + BOOST_REQUIRE_EQUAL(min_activation_height % window_size, 0U); + + const uint32_t bitmask{g_versionbitscache.Mask(params, dep)}; + BOOST_CHECK_EQUAL(bitmask, uint32_t{1} << bit); // In the first chain, test that the bit is set by CBV until it has failed. // In the second chain, test the bit is set by CBV while STARTED and @@ -453,8 +427,19 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) if (chain_name == CBaseChainParams::DEVNET) gArgs.SoftSetBoolArg("-devnet", true); const auto chainParams = CreateChainParams(*m_node.args, chain_name); if (chain_name == CBaseChainParams::DEVNET) gArgs.ForceRemoveArg("devnet"); + + uint32_t chain_all_vbits{0}; for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++i) { - check_computeblockversion(chainParams->GetConsensus(), static_cast(i)); + const auto dep = static_cast(i); + // Check that no bits are re-used (within the same chain). This is + // disallowed because the transition to FAILED (on timeout) does + // not take precedence over STARTED/LOCKED_IN. So all softforks on + // the same bit might overlap, even when non-overlapping start-end + // times are picked. + const uint32_t dep_mask{g_versionbitscache.Mask(chainParams->GetConsensus(), dep)}; + BOOST_CHECK(!(chain_all_vbits & dep_mask)); + chain_all_vbits |= dep_mask; + check_computeblockversion(chainParams->GetConsensus(), dep); } } From 5336f42ea8254f1206fc1163a7286077f9a57583 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 22 Apr 2021 12:58:18 +0200 Subject: [PATCH 08/10] Merge bitcoin/bitcoin#19801: test: check for all possible OP_CLTV fail reasons in feature_cltv.py (BIP 65) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit b01cd9471f435bb36b8ed5211a56baad51111ad2 test: check that _all_ invalid-CLTV txs are rejected after BIP65 activation (Sebastian Falbesoner) dbc19814743cb12960a99793197c811e2750a06b test: check that _all_ invalid-CLTV txs are allowed in a block pre-BIP65 (Sebastian Falbesoner) 8d0ce50c4826529a2d30ffc850bce4d44da6019b test: prepare cltv_invalidate to test all failure reasons in feature_cltv.py (Sebastian Falbesoner) ce994e1202c4820b1ad5c375d3d671fd0a18e092 test: add tx modfication helper function in feature_cltv.py (Sebastian Falbesoner) Pull request description: The functional test for [BIP65](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki) / `OP_CHECKLOCKTIMEVERIFY` (`feature_cltv.py`) currently only tests one out of five conditions that lead to failure of the op-code -- by prepending the script `OP_1NEGATE OP_CHECKLOCKTIMEVERIFY OP_DROP` to a tx's first input's scriptSig, the case of "_the top item on the stack is less than 0_" is checked: https://github.com/bitcoin/bitcoin/blob/f8462a6d2794be728cf8550f45d19a354aae59cf/test/functional/feature_cltv.py#L26-L35 This PR adds the other cases (5 in total) by taking an integer argument to the function `cltv_invalidate` that is called in a loop instead of only once per testing scenario. Here is the full list of failure conditions and how they are tested (note that the scriptSig should still be valid before activation of BIP65, when `OP_CLTV` is simply a no-op): * _the stack is empty_ ➡️ prepending `OP_CHECKLOCKTIMEVERIFY` to scriptSig * _the top item on the stack is less than 0_ ➡️ prepending `OP_1NEGATE OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig * _the lock-time type (height vs. timestamp) of the top stack item and the nLockTime field are not the same_ ➡️ prepending `OPNum(1000) OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig ➡️ setting tx.vin[0].nSequence=0 and tx.nCheckTimeLock=1296688602 (genesis block timestamp) * _the top stack item is greater than the transaction's nLockTime field_ ➡️ prepending `OPNum(1000) OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig ➡️ setting tx.vin[0].nSequence=0 and tx.nCheckTimeLock=500 * _the nSequence field of the txin is 0xffffffff_ ➡️ prepending `OPNum(500) OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig ➡️ setting tx.vin[0].nSequence=0xffffffff and tx.nCheckTimeLock=500 The first commit creates a helper function for the tx modification and also includes some tidying up like turning single-line to multi-line Python imports where necessary and cleaning up some PEP8 warnings. The second commit prepares the invalidation function `cltv_invalidate` and the third and the fourth use it and check for the expected reject reason strings ("Operation not valid with the current stack size", "Negative locktime" and "Locktime requirement not satisfied"). ACKs for top commit: MarcoFalke: review ACK b01cd9471f435bb36b8ed5211a56baad51111ad2 🐣 Tree-SHA512: dd82ae86e2bc4f3ab9bb1cfc9f04e4431b2b59c8aaf2a9f4b28654a1577e003fb43c500f99d76ff57e96262168e1cad7c1a0d71158e4b01063737e8f4be1e07d --- test/functional/feature_cltv.py | 145 +++++++++++++++++++++----------- 1 file changed, 97 insertions(+), 48 deletions(-) diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 934ff4b6b8..1a0aa77d51 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -8,10 +8,23 @@ Test that the CHECKLOCKTIMEVERIFY soft-fork activates at (regtest) block height 1351. """ -from test_framework.blocktools import create_coinbase, create_block, create_transaction -from test_framework.messages import CTransaction, msg_block +from test_framework.blocktools import ( + create_block, + create_coinbase, + create_transaction, +) +from test_framework.messages import ( + CTransaction, + msg_block, +) from test_framework.p2p import P2PInterface -from test_framework.script import CScript, OP_1NEGATE, OP_CHECKLOCKTIMEVERIFY, OP_DROP, CScriptNum +from test_framework.script import ( + CScript, + CScriptNum, + OP_1NEGATE, + OP_CHECKLOCKTIMEVERIFY, + OP_DROP, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -24,32 +37,54 @@ from io import BytesIO CLTV_HEIGHT = 1351 -def cltv_invalidate(tx): - '''Modify the signature in vin 0 of the tx to fail CLTV +# Helper function to modify a transaction by +# 1) prepending a given script to the scriptSig of vin 0 and +# 2) (optionally) modify the nSequence of vin 0 and the tx's nLockTime +def cltv_modify_tx(node, tx, prepend_scriptsig, nsequence=None, nlocktime=None): + if nsequence is not None: + tx.vin[0].nSequence = nsequence + tx.nLockTime = nlocktime - Prepends -1 CLTV DROP in the scriptSig itself. + # Need to re-sign, since nSequence and nLockTime changed + signed_result = node.signrawtransactionwithwallet(tx.serialize().hex()) + new_tx = CTransaction() + new_tx.deserialize(BytesIO(hex_str_to_bytes(signed_result['hex']))) + else: + new_tx = tx + + new_tx.vin[0].scriptSig = CScript(prepend_scriptsig + list(CScript(new_tx.vin[0].scriptSig))) + return new_tx + + +def cltv_invalidate(node, tx, failure_reason): + # Modify the signature in vin 0 and nSequence/nLockTime of the tx to fail CLTV + # + # According to BIP65, OP_CHECKLOCKTIMEVERIFY can fail due the following reasons: + # 1) the stack is empty + # 2) the top item on the stack is less than 0 + # 3) the lock-time type (height vs. timestamp) of the top stack item and the + # nLockTime field are not the same + # 4) the top stack item is greater than the transaction's nLockTime field + # 5) the nSequence field of the txin is 0xffffffff + assert failure_reason in range(5) + scheme = [ + # | Script to prepend to scriptSig | nSequence | nLockTime | + # +-------------------------------------------------+------------+--------------+ + [[OP_CHECKLOCKTIMEVERIFY], None, None], + [[OP_1NEGATE, OP_CHECKLOCKTIMEVERIFY, OP_DROP], None, None], + [[CScriptNum(1000), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 1296688602], # timestamp of genesis block + [[CScriptNum(1000), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, 500], + [[CScriptNum(500), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0xffffffff, 500], + ][failure_reason] + + return cltv_modify_tx(node, tx, prepend_scriptsig=scheme[0], nsequence=scheme[1], nlocktime=scheme[2]) - TODO: test more ways that transactions using CLTV could be invalid (eg - locktime requirements fail, sequence time requirements fail, etc). - ''' - tx.vin[0].scriptSig = CScript([OP_1NEGATE, OP_CHECKLOCKTIMEVERIFY, OP_DROP] + - list(CScript(tx.vin[0].scriptSig))) def cltv_validate(node, tx, height): - '''Modify the signature in vin 0 of the tx to pass CLTV - Prepends CLTV DROP in the scriptSig, and sets - the locktime to height''' - tx.vin[0].nSequence = 0 - tx.nLockTime = height + # Modify the signature in vin 0 and nSequence/nLockTime of the tx to pass CLTV + scheme = [[CScriptNum(height), OP_CHECKLOCKTIMEVERIFY, OP_DROP], 0, height] - # Need to re-sign, since nSequence and nLockTime changed - signed_result = node.signrawtransactionwithwallet(tx.serialize().hex()) - new_tx = CTransaction() - new_tx.deserialize(BytesIO(hex_str_to_bytes(signed_result['hex']))) - - new_tx.vin[0].scriptSig = CScript([CScriptNum(height), OP_CHECKLOCKTIMEVERIFY, OP_DROP] + - list(CScript(new_tx.vin[0].scriptSig))) - return new_tx + return cltv_modify_tx(node, tx, prepend_scriptsig=scheme[0], nsequence=scheme[1], nlocktime=scheme[2]) class BIP65Test(BitcoinTestFramework): @@ -66,8 +101,7 @@ class BIP65Test(BitcoinTestFramework): self.rpc_timeout = 480 def test_cltv_info(self, *, is_active): - assert_equal(self.nodes[0].getblockchaininfo()['softforks']['bip65'], - { + assert_equal(self.nodes[0].getblockchaininfo()['softforks']['bip65'], { "active": is_active, "height": CLTV_HEIGHT, "type": "buried", @@ -86,18 +120,22 @@ class BIP65Test(BitcoinTestFramework): self.coinbase_txids = [self.nodes[0].getblock(b)['tx'][0] for b in self.nodes[0].generate(CLTV_HEIGHT - 2)] self.nodeaddress = self.nodes[0].getnewaddress() - self.log.info("Test that an invalid-according-to-CLTV transaction can still appear in a block") + self.log.info("Test that invalid-according-to-CLTV transactions can still appear in a block") - spendtx = create_transaction(self.nodes[0], self.coinbase_txids[0], - self.nodeaddress, amount=1.0) - cltv_invalidate(spendtx) - spendtx.rehash() + # create one invalid tx per CLTV failure reason (5 in total) and collect them + invalid_ctlv_txs = [] + for i in range(5): + spendtx = create_transaction(self.nodes[0], self.coinbase_txids[i], + self.nodeaddress, amount=1.0) + spendtx = cltv_invalidate(self.nodes[0], spendtx, i) + spendtx.rehash() + invalid_ctlv_txs.append(spendtx) tip = self.nodes[0].getbestblockhash() block_time = self.nodes[0].getblockheader(tip)['mediantime'] + 1 block = create_block(int(tip, 16), create_coinbase(CLTV_HEIGHT - 1), block_time) block.nVersion = 3 - block.vtx.append(spendtx) + block.vtx.extend(invalid_ctlv_txs) block.hashMerkleRoot = block.calc_merkle_root() block.solve() @@ -118,27 +156,38 @@ class BIP65Test(BitcoinTestFramework): assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) peer.sync_with_ping() - self.log.info("Test that invalid-according-to-cltv transactions cannot appear in a block") + self.log.info("Test that invalid-according-to-CLTV transactions cannot appear in a block") block.nVersion = 4 + block.vtx.append(CTransaction()) # dummy tx after coinbase that will be replaced later - spendtx = create_transaction(self.nodes[0], self.coinbase_txids[1], - self.nodeaddress, amount=1.0) - cltv_invalidate(spendtx) - spendtx.rehash() + # create and test one invalid tx per CLTV failure reason (5 in total) + for i in range(5): + spendtx = create_transaction(self.nodes[0], self.coinbase_txids[10+i], + self.nodeaddress, amount=1.0) + spendtx = cltv_invalidate(self.nodes[0], spendtx, i) + spendtx.rehash() - # First we show that this tx is valid except for CLTV by getting it - # rejected from the mempool for exactly that reason. - assert_raises_rpc_error(-26, 'non-mandatory-script-verify-flag (Negative locktime)', self.nodes[0].sendrawtransaction, spendtx.serialize().hex(), 0) + expected_cltv_reject_reason = [ + "non-mandatory-script-verify-flag (Operation not valid with the current stack size)", + "non-mandatory-script-verify-flag (Negative locktime)", + "non-mandatory-script-verify-flag (Locktime requirement not satisfied)", + "non-mandatory-script-verify-flag (Locktime requirement not satisfied)", + "non-mandatory-script-verify-flag (Locktime requirement not satisfied)", + ][i] + # First we show that this tx is valid except for CLTV by getting it + # rejected from the mempool for exactly that reason. + assert_raises_rpc_error(-26, expected_cltv_reject_reason, self.nodes[0].sendrawtransaction, spendtx.serialize().hex(), 0) - # Now we verify that a block with this transaction is also invalid. - block.vtx.append(spendtx) - block.hashMerkleRoot = block.calc_merkle_root() - block.solve() + # Now we verify that a block with this transaction is also invalid. + block.vtx[1] = spendtx + block.hashMerkleRoot = block.calc_merkle_root() + block.solve() - with self.nodes[0].assert_debug_log(expected_msgs=['CheckInputScripts on {} failed with non-mandatory-script-verify-flag (Negative locktime)'.format(block.vtx[-1].hash)]): - peer.send_and_ping(msg_block(block)) - assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) - peer.sync_with_ping() + with self.nodes[0].assert_debug_log(expected_msgs=['CheckInputScripts on {} failed with {}'.format( + block.vtx[-1].hash, expected_cltv_reject_reason)]): + peer.send_and_ping(msg_block(block)) + assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) + peer.sync_with_ping() self.log.info("Test that a version 4 block with a valid-according-to-CLTV transaction is accepted") spendtx = cltv_validate(self.nodes[0], spendtx, CLTV_HEIGHT - 1) From 044ddb4c8009f3642e8ad67d24d9773893678ed2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 26 Apr 2021 18:04:19 +0200 Subject: [PATCH 09/10] Merge bitcoin/bitcoin#21777: test: Fix feature_notifications.py intermittent issue fa4aec2b26696cc16dc44c6425f7dca3ef91c8ee test: Fix feature_notifications.py intermittent issue (MarcoFalke) Pull request description: Fixes #21683 Top commit has no ACKs. Tree-SHA512: 256806d82877477f4b3d795658f61127c0de4eff07216f6071f40a8ec1f5d43f3c587f35dd436d480dc261ef6646ac5547db104d22f3fcfeeb61bbdbe04bcc31 --- test/functional/feature_notifications.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index f98fcab8d8..bdffbf3c9d 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -147,6 +147,8 @@ class NotificationsTest(DashTestFramework): # Should now verify contents of each file for tx_id, blockheight, blockhash in tx_details: fname = os.path.join(self.walletnotify_dir, notify_outputname(self.wallet, tx_id)) + # Wait for the cached writes to hit storage + self.wait_until(lambda: os.path.getsize(fname) > 0, timeout=10) with open(fname, 'rt', encoding='utf-8') as f: text = f.read() # Universal newline ensures '\n' on 'nt' From 4731f7045fb17458d01c1775f7eb8414d73da44c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 15 Jul 2024 13:51:40 +0700 Subject: [PATCH 10/10] docs: release notes for bitcoin#21141 - walletnotify %h %b --- doc/release-notes-21141.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 doc/release-notes-21141.md diff --git a/doc/release-notes-21141.md b/doc/release-notes-21141.md new file mode 100644 index 0000000000..c69da4f1c1 --- /dev/null +++ b/doc/release-notes-21141.md @@ -0,0 +1,6 @@ +## Command-line options +### Changes in existing cmd-line options: + +- `-walletnotify=` has a new format options "%h" and "%b". +%b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included). +%h is replaced by the block height (-1 if not included).