diff --git a/doc/release-notes-27279.md b/doc/release-notes-27279.md new file mode 100644 index 000000000000..82bc604e8485 --- /dev/null +++ b/doc/release-notes-27279.md @@ -0,0 +1,10 @@ +Wallet +------ + +- In the createwallet, loadwallet, unloadwallet, and restorewallet RPCs, the + "warning" string field is deprecated in favor of a "warnings" field that + returns a JSON array of strings to better handle multiple warning messages and + for consistency with other wallet RPCs. The "warning" field will be fully + removed from these RPCs in v25. It can be temporarily re-enabled during the + deprecation period by launching dashd with the configuration option + `-deprecatedrpc=walletwarningfield`. (dash#7711) diff --git a/src/init.cpp b/src/init.cpp index 570e7923fffd..320af728f8b3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -87,6 +87,7 @@ #include #include #include +#include #include #include #include @@ -832,6 +833,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !testnetChainParams->RequireStandard()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); argsman.AddArg("-dustrelayfee=", strprintf("Fee rate (in %s/kB) used to define dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); argsman.AddArg("-incrementalrelayfee=", strprintf("Fee rate (in %s/kB) used to define cost of relay, used for mempool limiting. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_INCREMENTAL_RELAY_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-acceptstalefeeestimates", strprintf("Read fee estimates even if they are stale (%sdefault: %u) fee estimates are considered stale if they are %s hours old", "regtest only; ", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES, Ticks(MAX_FILE_AGE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarriersize", @@ -1677,7 +1679,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, // as they would never get updated. - if (!ignores_incoming_txs) node.fee_estimator = std::make_unique(FeeestPath(args)); + if (!ignores_incoming_txs) { + bool read_stale_estimates = args.GetBoolArg("-acceptstalefeeestimates", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES); + if (read_stale_estimates && (chainparams.NetworkIDString() != CBaseChainParams::REGTEST)) { + return InitError(strprintf(_("acceptstalefeeestimates is not supported on %s chain."), chainparams.NetworkIDString())); + } + node.fee_estimator = std::make_unique(FeeestPath(args), read_stale_estimates); + + // Flush estimates to disk periodically + CBlockPolicyEstimator* fee_estimator = node.fee_estimator.get(); + node.scheduler->scheduleEvery([fee_estimator] { fee_estimator->FlushFeeEstimates(); }, FEE_FLUSH_INTERVAL); + } assert(!node.mn_metaman); node.mn_metaman = std::make_unique(); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 93fd34f1f3e5..fa035a627ddf 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -8,6 +8,7 @@ #include #include // For CTransactionRef #include // For util::SettingsValue +#include #include #include @@ -242,7 +243,7 @@ class Chain virtual void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) = 0; //! Check if transaction will pass the mempool's chain limits. - virtual bool checkChainLimits(const CTransactionRef& tx) = 0; + virtual util::Result checkChainLimits(const CTransactionRef& tx) = 0; //! Estimate smart fee. virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 0fe15348b1ef..c8ffaf42feeb 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -64,6 +64,7 @@ #include #include #include +#include #include #include #include @@ -1523,14 +1524,16 @@ class ChainImpl : public Chain limit_ancestor_count = limits.ancestor_count; limit_descendant_count = limits.descendant_count; } - bool checkChainLimits(const CTransactionRef& tx) override + util::Result checkChainLimits(const CTransactionRef& tx) override { - if (!m_node.mempool) return true; + if (!m_node.mempool) return {}; LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); const CTxMemPool::Limits& limits{m_node.mempool->m_limits}; LOCK(m_node.mempool->cs); - return m_node.mempool->CalculateMemPoolAncestors(entry, limits).has_value(); + auto ancestors{m_node.mempool->CalculateMemPoolAncestors(entry, limits)}; + if (!ancestors) return util::Error{util::ErrorString(ancestors)}; + return {}; } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index e7ade5887407..4e69a18f25df 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -540,7 +541,7 @@ bool CBlockPolicyEstimator::_removeTx(const uint256& hash, bool inBlock) } } -CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath) +CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath, const bool read_stale_estimates) : m_estimation_filepath{estimation_filepath} { static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero"); @@ -558,9 +559,22 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath shortStats = std::make_unique(buckets, bucketMap, SHORT_BLOCK_PERIODS, SHORT_DECAY, SHORT_SCALE); longStats = std::make_unique(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE); - // If the fee estimation file is present, read recorded estimations AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "rb")}; - if (est_file.IsNull() || !Read(est_file)) { + + // Whenever the fee estimation file is not present return early + if (est_file.IsNull()) { + LogPrintf("%s is not found. Continue anyway.\n", fs::PathToString(m_estimation_filepath)); + return; + } + + std::chrono::hours file_age = GetFeeEstimatorFileAge(); + // fee estimate file must not be too old to avoid wrong fee estimates. + if (file_age > MAX_FILE_AGE && !read_stale_estimates) { + LogPrintf("Fee estimation file %s too old (age=%lld > %lld hours) and will not be used to avoid serving stale estimates.\n", fs::PathToString(m_estimation_filepath), Ticks(file_age), Ticks(MAX_FILE_AGE)); + return; + } + + if (!Read(est_file)) { LogPrintf("Failed to read fee estimates from %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath)); } } @@ -915,10 +929,16 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation void CBlockPolicyEstimator::Flush() { FlushUnconfirmed(); + FlushFeeEstimates(); +} +void CBlockPolicyEstimator::FlushFeeEstimates() +{ AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "wb")}; if (est_file.IsNull() || !Write(est_file)) { LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath)); + } else { + LogPrintf("Flushed fee estimates to %s.\n", fs::PathToString(m_estimation_filepath.filename())); } } @@ -1022,3 +1042,10 @@ void CBlockPolicyEstimator::FlushUnconfirmed() const auto endclear{SteadyClock::now()}; LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks(endclear - startclear)); } + +std::chrono::hours CBlockPolicyEstimator::GetFeeEstimatorFileAge() +{ + auto file_time = std::filesystem::last_write_time(m_estimation_filepath); + auto now = std::filesystem::file_time_type::clock::now(); + return std::chrono::duration_cast(now - file_time); +} diff --git a/src/policy/fees.h b/src/policy/fees.h index 6653bfafc928..3b6f6d9698f3 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -14,12 +14,25 @@ #include #include +#include #include #include #include #include #include + +// How often to flush fee estimates to fee_estimates.dat. +static constexpr std::chrono::hours FEE_FLUSH_INTERVAL{1}; + +/** fee_estimates.dat that are more than 60 hours (2.5 days) will not be read, + * as the estimates in the file are stale. + */ +static constexpr std::chrono::hours MAX_FILE_AGE{60}; + +// Whether we allow importing a fee_estimates file older than MAX_FILE_AGE. +static constexpr bool DEFAULT_ACCEPT_STALE_FEE_ESTIMATES{false}; + class AutoFile; class CTxMemPoolEntry; class TxConfirmStats; @@ -183,7 +196,7 @@ class CBlockPolicyEstimator const fs::path m_estimation_filepath; public: /** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */ - CBlockPolicyEstimator(const fs::path& estimation_filepath); + CBlockPolicyEstimator(const fs::path& estimation_filepath, const bool read_stale_estimates); ~CBlockPolicyEstimator(); /** Process all the transactions that have been included in a block */ @@ -239,6 +252,13 @@ class CBlockPolicyEstimator void Flush() EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); + /** Record current fee estimations. */ + void FlushFeeEstimates() + EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); + + /** Calculates the age of the file, since last modified */ + std::chrono::hours GetFeeEstimatorFileAge(); + private: mutable Mutex m_cs_fee_estimator; diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 4a7d5287c0da..5302c5a53d5d 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -342,17 +342,15 @@ UniValue MempoolToJSON(const CTxMemPool& pool, const llmq::CInstantSendManager* } return o; } else { + UniValue a(UniValue::VARR); uint64_t mempool_sequence; - std::vector vtxid; { LOCK(pool.cs); - pool.queryHashes(vtxid); + for (const CTxMemPoolEntry& e : pool.mapTx) { + a.push_back(e.GetTx().GetHash().ToString()); + } mempool_sequence = pool.GetSequence(); } - UniValue a(UniValue::VARR); - for (const uint256& hash : vtxid) - a.push_back(hash.ToString()); - if (!include_mempool_sequence) { return a; } else { diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index c0666e6042e2..0f42873da46e 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -1167,3 +1167,25 @@ std::vector EvalDescriptorStringOrObject(const UniValue& scanobject, Fl } return ret; } +/** Convert a vector of bilingual strings to a UniValue::VARR containing their original untranslated values. */ +[[nodiscard]] static UniValue BilingualStringsToUniValue(const std::vector& bilingual_strings) +{ + CHECK_NONFATAL(!bilingual_strings.empty()); + UniValue result{UniValue::VARR}; + for (const auto& s : bilingual_strings) { + result.push_back(s.original); + } + return result; +} + +void PushWarnings(const UniValue& warnings, UniValue& obj) +{ + if (warnings.empty()) return; + obj.pushKV("warnings", warnings); +} + +void PushWarnings(const std::vector& warnings, UniValue& obj) +{ + if (warnings.empty()) return; + obj.pushKV("warnings", BilingualStringsToUniValue(warnings)); +} diff --git a/src/rpc/util.h b/src/rpc/util.h index 9285035a1896..50819a87bedb 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -453,4 +453,13 @@ class RPCHelpMan RPCErrorCode RPCErrorFromTransactionError(TransactionError terr); UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_string = ""); +/** + * Push warning messages to an RPC "warnings" field as a JSON array of strings. + * + * @param[in] warnings Warning messages to push. + * @param[out] obj UniValue object to push the warnings array object to. + */ +void PushWarnings(const UniValue& warnings, UniValue& obj); +void PushWarnings(const std::vector& warnings, UniValue& obj); + #endif // BITCOIN_RPC_UTIL_H diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 4454f256a0e3..292b009278e0 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1128,8 +1128,16 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider) +std::unique_ptr InferPubkey(const CPubKey& pubkey, ParseScriptContext ctx, const SigningProvider& provider) { + // Key cannot be hybrid + if (!pubkey.IsValidNonHybrid()) { + return nullptr; + } + // Uncompressed is only allowed in TOP and P2SH contexts + if (ctx != ParseScriptContext::TOP && ctx != ParseScriptContext::P2SH && !pubkey.IsCompressed()) { + return nullptr; + } std::unique_ptr key_provider = std::make_unique(0, pubkey); KeyOriginInfo info; if (provider.GetKeyOrigin(pubkey.GetID(), info)) { @@ -1145,8 +1153,8 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo if (txntype == TxoutType::PUBKEY) { CPubKey pubkey(data[0]); - if (pubkey.IsValidNonHybrid()) { - return std::make_unique(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ctx, provider)) { + return std::make_unique(std::move(pubkey_provider)); } } if (txntype == TxoutType::PUBKEYHASH) { @@ -1154,16 +1162,24 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo CKeyID keyid(hash); CPubKey pubkey; if (provider.GetPubKey(keyid, pubkey)) { - return std::make_unique(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ctx, provider)) { + return std::make_unique(std::move(pubkey_provider)); + } } } if (txntype == TxoutType::MULTISIG) { + bool ok = true; std::vector> providers; for (size_t i = 1; i + 1 < data.size(); ++i) { CPubKey pubkey(data[i]); - providers.push_back(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ctx, provider)) { + providers.push_back(std::move(pubkey_provider)); + } else { + ok = false; + break; + } } - return std::make_unique((int)data[0][0], std::move(providers)); + if (ok) return std::make_unique((int)data[0][0], std::move(providers)); } if (txntype == TxoutType::SCRIPTHASH && ctx == ParseScriptContext::TOP) { uint160 hash(data[0]); @@ -1175,6 +1191,10 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo } } + // The following descriptors are all top-level only descriptors. + // So if we are not at the top level, return early. + if (ctx != ParseScriptContext::TOP) return nullptr; + CTxDestination dest; if (ExtractDestination(script, dest)) { if (GetScriptForDestination(dest) == script) { diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 0cf8fd7457fe..b34ae3692cef 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -278,6 +278,54 @@ void Check(const std::string& prv, const std::string& pub, const std::string& no } } +void CheckInferDescriptor(const std::string& script_hex, const std::string& expected_desc, const std::vector& hex_scripts, const std::vector>& origin_pubkeys) +{ + std::vector script_bytes{ParseHex(script_hex)}; + const CScript& script{script_bytes.begin(), script_bytes.end()}; + + FlatSigningProvider provider; + for (const std::string& prov_script_hex : hex_scripts) { + std::vector prov_script_bytes{ParseHex(prov_script_hex)}; + const CScript& prov_script{prov_script_bytes.begin(), prov_script_bytes.end()}; + provider.scripts.emplace(CScriptID(prov_script), prov_script); + } + for (const auto& [pubkey_hex, origin_str] : origin_pubkeys) { + CPubKey origin_pubkey{ParseHex(pubkey_hex)}; + provider.pubkeys.emplace(origin_pubkey.GetID(), origin_pubkey); + + if (!origin_str.empty()) { + using namespace spanparsing; + KeyOriginInfo info; + Span origin_sp{origin_str}; + std::vector> origin_split = Split(origin_sp, "/"); + std::string fpr_str(origin_split[0].begin(), origin_split[0].end()); + auto fpr_bytes = ParseHex(fpr_str); + std::copy(fpr_bytes.begin(), fpr_bytes.end(), info.fingerprint); + for (size_t i = 1; i < origin_split.size(); ++i) { + Span elem = origin_split[i]; + bool hardened = false; + if (elem.size() > 0) { + const char last = elem[elem.size() - 1]; + if (last == '\'' || last == 'h') { + elem = elem.first(elem.size() - 1); + hardened = true; + } + } + uint32_t p; + assert(ParseUInt32(std::string(elem.begin(), elem.end()), &p)); + info.path.push_back(p | (((uint32_t)hardened) << 31)); + } + + provider.origins.emplace(origin_pubkey.GetID(), std::make_pair(origin_pubkey, info)); + } + } + + std::string checksum{GetDescriptorChecksum(expected_desc)}; + + std::unique_ptr desc = InferDescriptor(script, provider); + BOOST_CHECK_EQUAL(desc->ToString(), expected_desc + "#" + checksum); +} + } BOOST_FIXTURE_TEST_SUITE(descriptor_tests, BasicTestingSetup) @@ -391,6 +439,17 @@ BOOST_AUTO_TEST_CASE(descriptor_test) } nonminimalmultisig << std::vector{4} << OP_CHECKMULTISIG; CheckInferRaw(nonminimalmultisig); + + // Basic sh(pkh()) with key origin + CheckInferDescriptor("a9141a31ad23bf49c247dd531a623c2ef57da3c400c587", "sh(pkh([deadbeef/0h/0h/0]03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", {"76a9149a1c78a507689f6f54b847ad1cef1e614ee23f1e88ac"}, {{"03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd", "deadbeef/0h/0h/0"}}); + // p2pk script with hybrid key must infer as raw() + CheckInferDescriptor("41069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4ac", "raw(41069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4ac)", {}, {{"069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4", ""}}); + // p2pkh script with hybrid key must infer as addr() + CheckInferDescriptor("76a91445ff7c2327866472639d507334a9a00119dfd32688ac", "addr(Xh4xWtizCjdCnEZYH4tFEWgZV4qNfYYL82)", {}, {{"069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4", ""}}); + // Infer pkh() from p2pkh with uncompressed key + CheckInferDescriptor("76a914a31725c74421fadc50d35520ab8751ed120af80588ac", "pkh(04c56fe4a92d401bcbf1b3dfbe4ac3dac5602ca155a3681497f02c1b9a733b92d704e2da6ec4162e4846af9236ef4171069ac8b7f8234a8405b6cadd96f34f5a31)", {}, {{"04c56fe4a92d401bcbf1b3dfbe4ac3dac5602ca155a3681497f02c1b9a733b92d704e2da6ec4162e4846af9236ef4171069ac8b7f8234a8405b6cadd96f34f5a31", ""}}); + // Infer pk() from p2pk with uncompressed key + CheckInferDescriptor("4104032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220ac", "pk(04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220)", {}, {{"04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220", ""}}); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 6785677d0734..dd0e4a45b6ec 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -31,7 +31,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); bool good_data{true}; - CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args)}; + CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000) { CallOneOf( diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp index ee0548f245e1..c04ef8f5b0f5 100644 --- a/src/test/fuzz/policy_estimator_io.cpp +++ b/src/test/fuzz/policy_estimator_io.cpp @@ -28,7 +28,7 @@ FUZZ_TARGET(policy_estimator_io, .init = initialize_policy_estimator_io) FuzzedAutoFileProvider fuzzed_auto_file_provider = ConsumeAutoFile(fuzzed_data_provider); AutoFile fuzzed_auto_file{fuzzed_auto_file_provider.open()}; // Re-using block_policy_estimator across runs to avoid costly creation of CBlockPolicyEstimator object. - static CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args)}; + static CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; if (block_policy_estimator.Read(fuzzed_auto_file)) { block_policy_estimator.Write(fuzzed_auto_file); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 6febcccabf8b..176318a4a212 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -101,9 +101,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, con if (!info_all.empty()) { const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, MemPoolRemovalReason::BLOCK /* dummy */)); - std::vector all_txids; - tx_pool.queryHashes(all_txids); - assert(all_txids.size() < info_all.size()); + assert(tx_pool.size() < info_all.size()); WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1)); } SyncWithValidationInterfaceQueue(); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 29f5eaa7fa29..2ab2aca39e08 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -278,7 +278,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); }); GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler); - m_node.fee_estimator = std::make_unique(FeeestPath(*m_node.args)); + m_node.fee_estimator = std::make_unique(FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES); m_node.mempool = std::make_unique(MemPoolOptionsForTest(m_node)); m_cache_sizes = CalculateCacheSizes(m_args); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 98ed3a900a48..21d40e90244f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -202,9 +202,8 @@ util::Result CTxMemPool::CalculateAncestorsAndCheckLimit return ancestors; } -bool CTxMemPool::CheckPackageLimits(const Package& package, - const Limits& limits, - std::string &errString) const +util::Result CTxMemPool::CheckPackageLimits(const Package& package, + const Limits& limits) const { CTxMemPoolEntry::Parents staged_ancestors; size_t total_size = 0; @@ -215,8 +214,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, if (piter) { staged_ancestors.insert(**piter); if (staged_ancestors.size() + package.size() > static_cast(limits.ancestor_count)) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count); - return false; + return util::Error{Untranslated(strprintf("too many unconfirmed parents [limit: %u]", limits.ancestor_count))}; } } } @@ -227,8 +225,8 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, const auto ancestors{CalculateAncestorsAndCheckLimits(total_size, package.size(), staged_ancestors, limits)}; // It's possible to overestimate the ancestor/descendant totals. - if (!ancestors.has_value()) errString = "possibly " + util::ErrorString(ancestors).original; - return ancestors.has_value(); + if (!ancestors.has_value()) return util::Error{Untranslated("possibly " + util::ErrorString(ancestors).original)}; + return {}; } util::Result CTxMemPool::CalculateMemPoolAncestors( @@ -1461,19 +1459,6 @@ std::vector CTxMemPool::Get return iters; } -void CTxMemPool::queryHashes(std::vector& vtxid) const -{ - LOCK(cs); - auto iters = GetSortedDepthAndScore(); - - vtxid.clear(); - vtxid.reserve(mapTx.size()); - - for (auto it : iters) { - vtxid.push_back(it->GetTx().GetHash()); - } -} - static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) { return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; } diff --git a/src/txmempool.h b/src/txmempool.h index 7bb82331c688..83b73a9297b5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -544,7 +544,6 @@ class CTxMemPool void removeExpiredAssetUnlock(int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); - void queryHashes(std::vector& vtxid) const; bool isSpent(const COutPoint& outpoint) const; unsigned int GetTransactionsUpdated() const; void AddTransactionsUpdated(unsigned int n); @@ -638,11 +637,10 @@ class CTxMemPool * to mempool. The transactions need not be direct * ancestors/descendants of each other. * @param[in] limits Maximum number and size of ancestors and descendants - * @param[out] errString Populated with error reason if a limit is hit. + * @returns {} or the error reason if a limit is hit. */ - bool CheckPackageLimits(const Package& package, - const Limits& limits, - std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result CheckPackageLimits(const Package& package, + const Limits& limits) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything diff --git a/src/validation.cpp b/src/validation.cpp index 76063bea9693..cc7bb0566039 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -1105,10 +1106,10 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx) { return !m_pool.exists(tx->GetHash());})); - std::string err_string; - if (!m_pool.CheckPackageLimits(txns, m_limits, err_string)) { + auto result = m_pool.CheckPackageLimits(txns, m_limits); + if (!result) { // This is a package-wide error, separate from an individual transaction error. - return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", util::ErrorString(result).original); } return true; } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 5bfef2bb4ca0..2846643a5f99 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1451,7 +1451,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64 result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, "Missing required fields")); } - if (warnings.size()) result.pushKV("warnings", warnings); + PushWarnings(warnings, result); return result; } @@ -1806,7 +1806,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c result.pushKV("success", UniValue(false)); result.pushKV("error", e); } - if (warnings.size()) result.pushKV("warnings", warnings); + PushWarnings(warnings, result); return result; } @@ -1880,10 +1880,14 @@ RPCHelpMan importdescriptors() { RPCTypeCheck(main_request.params, {UniValue::VARR, UniValue::VOBJ}); WalletRescanReserver reserver(*pwallet); - if (!reserver.reserve()) { + if (!reserver.reserve(/*with_passphrase=*/true)) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } + // Ensure that the wallet is not locked for the remainder of this RPC, as + // the passphrase is used to top up the keypool. + LOCK(pwallet->m_relock_mutex); + const UniValue& requests = main_request.params[0]; const int64_t minimum_timestamp = 1; int64_t now = 0; @@ -2157,7 +2161,11 @@ RPCHelpMan restorewallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if restored successfully."}, - {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to restoring the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, } }, RPCExamples{ @@ -2187,7 +2195,10 @@ RPCHelpMan restorewallet() UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } + PushWarnings(warnings, obj); return obj; diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp index 703b00e4d871..5643bcc763b8 100644 --- a/src/wallet/rpc/encrypt.cpp +++ b/src/wallet/rpc/encrypt.cpp @@ -110,7 +110,7 @@ RPCHelpMan walletpassphrase() std::weak_ptr weak_wallet = wallet; pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet, relock_time] { if (auto shared_wallet = weak_wallet.lock()) { - LOCK(shared_wallet->cs_wallet); + LOCK2(shared_wallet->m_relock_mutex, shared_wallet->cs_wallet); // Skip if this is not the most recent rpcRunLater callback. if (shared_wallet->nRelockTime != relock_time) return; shared_wallet->Lock(); @@ -141,11 +141,16 @@ RPCHelpMan walletpassphrasechange() std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - LOCK(pwallet->cs_wallet); - if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called."); } + + if (pwallet->IsScanningWithPassphrase()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase."); + } + + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); + SecureString strOldWalletPass; strOldWalletPass.reserve(100); strOldWalletPass = std::string_view{request.params[0].get_str()}; @@ -199,12 +204,16 @@ RPCHelpMan walletlock() std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - LOCK(pwallet->cs_wallet); - if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletlock was called."); } + if (pwallet->IsScanningWithPassphrase()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet."); + } + + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); + pwallet->Lock(); pwallet->nRelockTime = 0; @@ -241,8 +250,6 @@ RPCHelpMan encryptwallet() std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - LOCK(pwallet->cs_wallet); - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: wallet does not contain private keys, nothing to encrypt."); } @@ -250,6 +257,13 @@ RPCHelpMan encryptwallet() if (pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called."); } + + if (pwallet->IsScanningWithPassphrase()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before encrypting the wallet."); + } + + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); + SecureString strWalletPass; strWalletPass.reserve(100); strWalletPass = std::string_view{request.params[0].get_str()}; diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 5e3d13bc5ffa..c80ebfd4e305 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -893,15 +893,18 @@ RPCHelpMan rescanblockchain() wallet.BlockUntilSyncedToCurrentChain(); WalletRescanReserver reserver(*pwallet); - if (!reserver.reserve()) { + if (!reserver.reserve(/*with_passphrase=*/true)) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } int start_height = 0; std::optional stop_height; uint256 start_block; + + LOCK(pwallet->m_relock_mutex); { LOCK(pwallet->cs_wallet); + EnsureWalletIsUnlocked(*pwallet); int tip_height = pwallet->GetLastBlockHeight(); if (!request.params[0].isNull()) { diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 9ca2e889919c..84e03b29462f 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -32,12 +32,21 @@ #include #include +#include #include #include namespace wallet { + +static const std::map WALLET_FLAG_CAVEATS{ + {WALLET_FLAG_AVOID_REUSE, + "You need to rescan the blockchain in order to correctly mark used " + "destinations in the past. Until this is done, some destinations may " + "be considered unused, even if the opposite is the case."}, +}; + /** Checks if a CKey is in the given CWallet compressed or otherwise*/ bool HaveKey(const SigningProvider& wallet, const CKey& key) { @@ -335,7 +344,7 @@ static RPCHelpMan upgradetohd() bool generate_mnemonic = request.params[0].isNull() || request.params[0].get_str().empty(); bool mnemonic_passphrase_has_null{false}; { - LOCK(pwallet->cs_wallet); + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); SecureString wallet_passphrase; wallet_passphrase.reserve(100); @@ -475,7 +484,11 @@ static RPCHelpMan loadwallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."}, - {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to loading the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, } }, RPCExamples{ @@ -508,7 +521,10 @@ static RPCHelpMan loadwallet() UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } + PushWarnings(warnings, obj); return obj; }, @@ -602,7 +618,11 @@ static RPCHelpMan createwallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path."}, - {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to creating the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, } }, RPCExamples{ @@ -675,7 +695,10 @@ static RPCHelpMan createwallet() UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } + PushWarnings(warnings, obj); return obj; }, @@ -692,7 +715,11 @@ static RPCHelpMan unloadwallet() {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, + {RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to unloading the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, }}, RPCExamples{ HelpExampleCli("unloadwallet", "wallet_name") @@ -730,11 +757,13 @@ static RPCHelpMan unloadwallet() throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } } + UniValue result(UniValue::VOBJ); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } + PushWarnings(warnings, result); UnloadWallet(std::move(wallet)); - - UniValue result(UniValue::VOBJ); - result.pushKV("warning", Join(warnings, Untranslated("\n")).original); return result; }, }; @@ -860,7 +889,14 @@ static RPCHelpMan sethdseed() } if (request.params[1].isNull()) { - spk_man.GenerateNewHDChain("", ""); + if (pwallet->IsCrypted()) { + pwallet->WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + spk_man.GenerateNewHDChain("", "", encryption_key); + return true; + }); + } else { + spk_man.GenerateNewHDChain("", ""); + } } else { CKey key = DecodeSecret(request.params[1].get_str()); if (!key.IsValid()) { @@ -873,6 +909,11 @@ static RPCHelpMan sethdseed() if (!newHdChain.SetSeed(SecureVector(key.begin(), key.end()), true)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key: SetSeed failed"); } + if (pwallet->IsCrypted() && !pwallet->WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return spk_man.EncryptHDChain(encryption_key, newHdChain); + })) { + throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Failed to encrypt the HD seed"); + } if (!spk_man.AddHDChainSingle(newHdChain)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key: AddHDChainSingle failed"); } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 2071d3de5c77..1bf6aebdbab3 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -315,7 +315,6 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv /** Add a KeyOriginInfo to the wallet */ bool AddKeyOriginWithDB(WalletBatch& batch, const CPubKey& pubkey, const KeyOriginInfo& info); - bool EncryptHDChain(const CKeyingMaterial& vMasterKeyIn, CHDChain& chain); bool DecryptHDChain(const CKeyingMaterial& vMasterKeyIn, CHDChain& hdChainRet) const; /* the HD chain data model (external chain counters) */ @@ -438,6 +437,8 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv * which causes db flush every time these methods are used */ bool AddHDChainSingle(const CHDChain& chain); + //! Encrypt the seed and mnemonic of an unencrypted HD chain with the wallet's master key + bool EncryptHDChain(const CKeyingMaterial& vMasterKeyIn, CHDChain& chain); //! Adds a watch-only address to the store, without saving it to disk (used by LoadWallet) bool LoadWatchOnly(const CScript &dest); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index fd07485dfd14..734c8d07270b 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1125,8 +1125,9 @@ static util::Result CreateTransactionInternal( if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits - if (!wallet.chain().checkChainLimits(tx)) { - return util::Error{_("Transaction has too long of a mempool chain")}; + auto result = wallet.chain().checkChainLimits(tx); + if (!result) { + return util::Error{util::ErrorString(result)}; } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 03f12e970f10..0ae127473cd2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -70,14 +70,6 @@ static isminetype InputIsMine(const CWallet& wallet, const CTxIn& txin) EXCLUSIV return ISMINE_NO; } -const std::map WALLET_FLAG_CAVEATS{ - {WALLET_FLAG_AVOID_REUSE, - "You need to rescan the blockchain in order to correctly mark used " - "destinations in the past. Until this is done, some destinations may " - "be considered unused, even if the opposite is the case." - }, -}; - bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) { util::SettingsValue setting_value = chain.getRwSetting("wallet"); @@ -586,7 +578,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, bool fWasLocked = IsLocked(true); { - LOCK(cs_wallet); + LOCK2(m_relock_mutex, cs_wallet); Lock(); CCrypter crypter; @@ -833,7 +825,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) return false; { - LOCK(cs_wallet); + LOCK2(m_relock_mutex, cs_wallet); if (IsCrypted()) { // verify again now that cs_wallet lock is held @@ -4144,7 +4136,7 @@ bool CWallet::Lock(bool fAllowMixing) return false; { - LOCK(cs_wallet); + LOCK2(m_relock_mutex, cs_wallet); if (!fAllowMixing) { if (!vMasterKey.empty()) { memory_cleanse(vMasterKey.data(), vMasterKey.size() * sizeof(decltype(vMasterKey)::value_type)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a80ac1993237..59b14780eba7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -165,8 +165,6 @@ static const std::map WALLET_FLAG_MAP{ {"external_signer", WALLET_FLAG_EXTERNAL_SIGNER} }; -extern const std::map WALLET_FLAG_CAVEATS; - /** A wrapper to reserve an address from a wallet * * ReserveDestination is used to reserve an address. It is passed around @@ -307,6 +305,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati std::atomic fAbortRescan{false}; // reset by WalletRescanReserver::reserve() std::atomic fScanningWallet{false}; // controlled by WalletRescanReserver std::atomic m_attaching_chain{false}; + std::atomic m_scanning_with_passphrase{false}; std::atomic m_scanning_start{SteadyClock::time_point{}}; std::atomic m_scanning_progress{0}; friend class WalletRescanReserver; @@ -690,6 +689,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void AbortRescan() { fAbortRescan = true; } bool IsAbortingRescan() const { return fAbortRescan; } bool IsScanning() const { return fScanningWallet; } + bool IsScanningWithPassphrase() const { return m_scanning_with_passphrase; } SteadyClock::duration ScanningDuration() const { return fScanningWallet ? SteadyClock::now() - m_scanning_start.load() : SteadyClock::duration{}; } double ScanningProgress() const { return fScanningWallet ? (double) m_scanning_progress : 0; } @@ -711,6 +711,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // Used to prevent concurrent calls to walletpassphrase RPC. Mutex m_unlock_mutex; + // Used to prevent deleting the passphrase from memory when it is still in use. + RecursiveMutex m_relock_mutex; + bool Unlock(const SecureString& strWalletPassphrase, bool fForMixingOnly = false); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); @@ -1240,12 +1243,13 @@ class WalletRescanReserver public: explicit WalletRescanReserver(CWallet& w) : m_wallet(w) {} - bool reserve() + bool reserve(bool with_passphrase = false) { assert(!m_could_reserve); if (m_wallet.fScanningWallet.exchange(true)) { return false; } + m_wallet.m_scanning_with_passphrase.exchange(with_passphrase); m_wallet.m_scanning_start = SteadyClock::now(); m_wallet.m_scanning_progress = 0; m_wallet.fAbortRescan = false; @@ -1266,6 +1270,7 @@ class WalletRescanReserver { if (m_could_reserve) { m_wallet.fScanningWallet = false; + m_wallet.m_scanning_with_passphrase = false; } } diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index e9776a33c22f..a37668e89136 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -5,7 +5,9 @@ """Test fee estimation code.""" from copy import deepcopy from decimal import Decimal +import os import random +import time from test_framework.messages import ( COIN, @@ -20,6 +22,8 @@ ) from test_framework.wallet import MiniWallet +MAX_FILE_AGE = 60 +SECONDS_PER_HOUR = 60 * 60 def small_txpuzzle_randfee( wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment, batch_reqs @@ -223,6 +227,95 @@ def test_feerate_mempoolminfee(self): self.stop_node(1) + def test_old_fee_estimate_file(self): + # Get the initial fee rate while node is running + fee_rate = self.nodes[0].estimatesmartfee(1)["feerate"] + + # Restart node to ensure fee_estimate.dat file is read + self.restart_node(0) + assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate) + + fee_dat = self.nodes[0].chain_path / "fee_estimates.dat" + + # Stop the node and backdate the fee_estimates.dat file more than MAX_FILE_AGE + self.stop_node(0) + last_modified_time = time.time() - (MAX_FILE_AGE + 1) * SECONDS_PER_HOUR + os.utime(fee_dat, (last_modified_time, last_modified_time)) + + # Start node and ensure the fee_estimates.dat file was not read + self.start_node(0) + assert_equal(self.nodes[0].estimatesmartfee(1)["errors"], ["Insufficient data or no feerate found"]) + + + def test_estimate_dat_is_flushed_periodically(self): + fee_dat = self.nodes[0].chain_path / "fee_estimates.dat" + os.remove(fee_dat) if os.path.exists(fee_dat) else None + + # Verify that fee_estimates.dat does not exist + assert_equal(os.path.isfile(fee_dat), False) + + # Verify if the string "Flushed fee estimates to fee_estimates.dat." is present in the debug log file. + # If present, it indicates that fee estimates have been successfully flushed to disk. + with self.nodes[0].assert_debug_log(expected_msgs=["Flushed fee estimates to fee_estimates.dat."], timeout=1): + # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat + self.nodes[0].mockscheduler(SECONDS_PER_HOUR) + + # Verify that fee estimates were flushed and fee_estimates.dat file is created + assert_equal(os.path.isfile(fee_dat), True) + + # Verify that the estimates remain the same if there are no blocks in the flush interval + block_hash_before = self.nodes[0].getbestblockhash() + fee_dat_initial_content = open(fee_dat, "rb").read() + with self.nodes[0].assert_debug_log(expected_msgs=["Flushed fee estimates to fee_estimates.dat."], timeout=1): + # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat + self.nodes[0].mockscheduler(SECONDS_PER_HOUR) + + # Verify that there were no blocks in between the flush interval + assert_equal(block_hash_before, self.nodes[0].getbestblockhash()) + + fee_dat_current_content = open(fee_dat, "rb").read() + assert_equal(fee_dat_current_content, fee_dat_initial_content) + + # Verify that the estimates remain the same after shutdown with no blocks before shutdown + self.restart_node(0) + fee_dat_current_content = open(fee_dat, "rb").read() + assert_equal(fee_dat_current_content, fee_dat_initial_content) + + # Verify that the estimates are not the same if new blocks were produced in the flush interval + with self.nodes[0].assert_debug_log(expected_msgs=["Flushed fee estimates to fee_estimates.dat."], timeout=1): + # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat + self.generate(self.nodes[0], 5, sync_fun=self.no_op) + self.nodes[0].mockscheduler(SECONDS_PER_HOUR) + + fee_dat_current_content = open(fee_dat, "rb").read() + assert fee_dat_current_content != fee_dat_initial_content + + fee_dat_initial_content = fee_dat_current_content + + # Generate blocks before shutdown and verify that the fee estimates are not the same + self.generate(self.nodes[0], 5, sync_fun=self.no_op) + self.restart_node(0) + fee_dat_current_content = open(fee_dat, "rb").read() + assert fee_dat_current_content != fee_dat_initial_content + + + def test_acceptstalefeeestimates_option(self): + # Get the initial fee rate while node is running + fee_rate = self.nodes[0].estimatesmartfee(1)["feerate"] + + self.stop_node(0) + + fee_dat = self.nodes[0].chain_path / "fee_estimates.dat" + + # Stop the node and backdate the fee_estimates.dat file more than MAX_FILE_AGE + last_modified_time = time.time() - (MAX_FILE_AGE + 1) * SECONDS_PER_HOUR + os.utime(fee_dat, (last_modified_time, last_modified_time)) + + # Restart node with -acceptstalefeeestimates option to ensure fee_estimate.dat file is read + self.start_node(0,extra_args=["-acceptstalefeeestimates"]) + assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate) + + def run_test(self): self.log.info("This test is time consuming, please be patient") self.log.info("Splitting inputs so we can generate tx's") @@ -245,12 +338,21 @@ def run_test(self): self.log.info("Testing estimates with single transactions.") self.sanity_check_estimates_range() + self.log.info("Test fee_estimates.dat is flushed periodically") + self.test_estimate_dat_is_flushed_periodically() + # check that the effective feerate is greater than or equal to the mempoolminfee even for high mempoolminfee self.log.info( "Test fee rate estimation after restarting node with high MempoolMinFee" ) self.test_feerate_mempoolminfee() + self.log.info("Test acceptstalefeeestimates option") + self.test_acceptstalefeeestimates_option() + + self.log.info("Test reading old fee_estimates.dat") + self.test_old_fee_estimate_file() + self.log.info("Testing that fee estimation is disabled in blocksonly.") self.restart_node(0, ["-blocksonly"]) assert_raises_rpc_error( diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index e32e4c770d47..62e9c26f6de0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -683,7 +683,7 @@ def run_test(self): node0_balance = self.nodes[0].getbalance() # With walletrejectlongchains we will not create the tx and store it in our wallet. - assert_raises_rpc_error(-6, "Transaction has too long of a mempool chain", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01')) + assert_raises_rpc_error(-6, f"too many unconfirmed ancestors [limit: {chainlimit * 2}]", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01')) # Verify nothing new in wallet assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999))) diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index f28a72d434ab..415eb7380bda 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -15,12 +15,16 @@ ) from test_framework.wallet_util import bytes_to_wif, generate_wif_key +EMPTY_PASSPHRASE_MSG = "Empty string given as passphrase, wallet will not be encrypted." + + class CreateWalletTest(BitcoinTestFramework): def add_options(self, parser): self.add_wallet_options(parser) def set_test_params(self): self.num_nodes = 1 + self.extra_args = [["-deprecatedrpc=walletwarningfield"]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -55,7 +59,7 @@ def run_test(self): else: result = w1.importmulti([{'scriptPubKey': {'address': key_to_p2pkh(eckey.get_pubkey().get_bytes())}, 'timestamp': 'now', 'keys': [privkey]}]) assert not result[0]['success'] - assert 'warning' not in result[0] + assert 'warnings' not in result[0] assert_equal(result[0]['error']['code'], -4) assert_equal(result[0]['error']['message'], 'Cannot import private keys to a wallet with private keys disabled') @@ -160,7 +164,9 @@ def run_test(self): assert_equal(walletinfo['keypoolsize_hd_internal'], keys) # Allow empty passphrase, but there should be a warning resp = self.nodes[0].createwallet(wallet_name='w7', disable_private_keys=False, blank=False, passphrase='') - assert 'Empty string given as passphrase, wallet will not be encrypted.' in resp['warning'] + assert_equal(resp["warning"], EMPTY_PASSPHRASE_MSG) + assert_equal(resp["warnings"], [EMPTY_PASSPHRASE_MSG]) + w7 = node.get_wallet_rpc('w7') assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 60) @@ -173,5 +179,11 @@ def run_test(self): self.log.info('Using a passphrase with private keys disabled returns error') assert_raises_rpc_error(-4, 'Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.', self.nodes[0].createwallet, wallet_name='w9', disable_private_keys=True, passphrase='thisisapassphrase') + self.log.info('Test "warning" field deprecation, i.e. not returned without -deprecatedrpc=walletwarningfield') + self.restart_node(0, extra_args=[]) + result = self.nodes[0].createwallet(wallet_name="w7_again", disable_private_keys=False, blank=False, passphrase="") + assert "warning" not in result + + if __name__ == '__main__': CreateWalletTest().main() diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index 5dca83303381..ccf303dbc229 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -15,6 +15,10 @@ - `test_address()` is called to call getaddressinfo for an address on node1 and test the values returned.""" +import threading + +from test_framework.authproxy import JSONRPCException +from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.descriptors import descsum_create from test_framework.util import ( @@ -611,5 +615,48 @@ def run_test(self): #warnings=["Unknown output type, cannot set descriptor to active."] ) + self.log.info("Test importing a descriptor to an encrypted wallet") + + descriptor = {"desc": descsum_create("pkh(" + xpriv + "/1h/*h)"), + "timestamp": "now", + "active": True, + "range": [0,4000], + "next_index": 4000} + + self.nodes[0].createwallet("temp_wallet", blank=True, descriptors=True) + temp_wallet = self.nodes[0].get_wallet_rpc("temp_wallet") + temp_wallet.importdescriptors([descriptor]) + self.generatetoaddress(self.nodes[0], COINBASE_MATURITY + 1, temp_wallet.getnewaddress()) + self.generatetoaddress(self.nodes[0], COINBASE_MATURITY + 1, temp_wallet.getnewaddress()) + + self.nodes[0].createwallet("encrypted_wallet", blank=True, descriptors=True, passphrase="passphrase") + encrypted_wallet = self.nodes[0].get_wallet_rpc("encrypted_wallet") + + descriptor["timestamp"] = 0 + descriptor["next_index"] = 0 + + encrypted_wallet.walletpassphrase("passphrase", 99999) + t = threading.Thread(target=encrypted_wallet.importdescriptors, args=([descriptor],)) + + with self.nodes[0].assert_debug_log(expected_msgs=[f'Rescan started from block 000008ca1832a4baf228eb1553c03d3a2c8e02399550dd6ea8d65cec3ef23d2e... (slow variant inspecting all blocks)'], timeout=5): + t.start() + + # Set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan + self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1) + + try: + self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletlock() + except JSONRPCException as e: + assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error['message'] + + try: + self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase") + except JSONRPCException as e: + assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error['message'] + + t.join() + + assert_equal(temp_wallet.getbalance(), encrypted_wallet.getbalance()) + if __name__ == '__main__': ImportDescriptorsTest().main() diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 0abc2f2e435e..98da6442a467 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -7,8 +7,12 @@ import os import random import shutil +from test_framework.address import key_to_p2pkh from test_framework.descriptors import descsum_create +from test_framework.key import ECPubKey from test_framework.test_framework import BitcoinTestFramework +from test_framework.messages import COIN, CTransaction, CTxOut +from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_script from test_framework.util import ( assert_equal, assert_raises_rpc_error, @@ -566,6 +570,86 @@ def test_direct_file(self): assert os.path.isdir(wallet_path) assert os.path.isfile(wallet_dat_path) + def test_migrate_raw_p2sh(self): + self.log.info("Test migration of watch-only raw p2sh script") + df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("raw_p2sh") + + def send_to_script(script, amount): + tx = CTransaction() + tx.vout.append(CTxOut(nValue=amount*COIN, scriptPubKey=script)) + + hex_tx = df_wallet.fundrawtransaction(tx.serialize().hex())['hex'] + signed_tx = df_wallet.signrawtransactionwithwallet(hex_tx) + df_wallet.sendrawtransaction(signed_tx['hex']) + self.generate(self.nodes[0], 1) + + # Craft sh(pkh(key)) script and send coins to it + pubkey = df_wallet.getaddressinfo(df_wallet.getnewaddress())["pubkey"] + script_pkh = key_to_p2pkh_script(pubkey) + script_sh_pkh = script_to_p2sh_script(script_pkh) + send_to_script(script=script_sh_pkh, amount=2) + + # Import script and check balance + wallet.rpc.importaddress(address=script_pkh.hex(), label="raw_spk", rescan=True, p2sh=True) + assert_equal(wallet.getbalances()['watchonly']['trusted'], 2) + + # Migrate wallet and re-check balance + info_migration = wallet.migratewallet() + wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"]) + + # Watch-only balance is under "mine". + assert_equal(wallet_wo.getbalances()['mine']['trusted'], 2) + # The watch-only scripts are no longer part of the main wallet + assert_equal(wallet.getbalances()['mine']['trusted'], 0) + + # Just in case, also verify wallet restart + self.nodes[0].unloadwallet(info_migration["watchonly_name"]) + self.nodes[0].loadwallet(info_migration["watchonly_name"]) + assert_equal(wallet_wo.getbalances()['mine']['trusted'], 2) + + def test_hybrid_pubkey(self): + self.log.info("Test migration when wallet contains a hybrid pubkey") + + wallet = self.create_legacy_wallet("hybrid_keys") + + # Get the hybrid pubkey for one of the keys in the wallet + normal_pubkey = wallet.getaddressinfo(wallet.getnewaddress())["pubkey"] + first_byte = bytes.fromhex(normal_pubkey)[0] + 4 # Get the hybrid pubkey first byte + parsed_pubkey = ECPubKey() + parsed_pubkey.set(bytes.fromhex(normal_pubkey)) + parsed_pubkey.compressed = False + hybrid_pubkey_bytes = bytearray(parsed_pubkey.get_bytes()) + hybrid_pubkey_bytes[0] = first_byte # Make it hybrid + hybrid_pubkey = hybrid_pubkey_bytes.hex() + + # Import the hybrid pubkey + wallet.importpubkey(hybrid_pubkey) + p2pkh_addr = key_to_p2pkh(hybrid_pubkey) + p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr) + assert_equal(p2pkh_addr_info["iswatchonly"], True) + assert_equal(p2pkh_addr_info["ismine"], False) # Things involving hybrid pubkeys are not spendable + + migrate_info = wallet.migratewallet() + + # The address should only appear in the watchonly wallet + p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr) + assert_equal(p2pkh_addr_info["iswatchonly"], False) + assert_equal(p2pkh_addr_info["ismine"], False) + + watchonly_wallet = self.nodes[0].get_wallet_rpc(migrate_info["watchonly_name"]) + watchonly_p2pkh_addr_info = watchonly_wallet.getaddressinfo(p2pkh_addr) + assert_equal(watchonly_p2pkh_addr_info["iswatchonly"], False) + assert_equal(watchonly_p2pkh_addr_info["ismine"], True) + + # There should only be raw or addr descriptors + for desc in watchonly_wallet.listdescriptors()["descriptors"]: + if desc["desc"].startswith("raw(") or desc["desc"].startswith("addr("): + continue + assert False, "Hybrid pubkey watchonly wallet has more than just raw() and addr()" + + wallet.unloadwallet() + def run_test(self): self.generate(self.nodes[0], 101) @@ -581,6 +665,8 @@ def run_test(self): self.test_wallet_name_with_slashes() self.test_default_wallet() self.test_direct_file() + self.test_migrate_raw_p2sh() + self.test_hybrid_pubkey() if __name__ == '__main__': WalletMigrationTest().main() diff --git a/test/functional/wallet_transactiontime_rescan.py b/test/functional/wallet_transactiontime_rescan.py index a66f720ff27a..306c22736d5e 100755 --- a/test/functional/wallet_transactiontime_rescan.py +++ b/test/functional/wallet_transactiontime_rescan.py @@ -5,8 +5,10 @@ """Test transaction time during old block rescanning """ +import threading import time +from test_framework.authproxy import JSONRPCException from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -14,6 +16,9 @@ assert_raises_rpc_error, set_node_times, ) +from test_framework.wallet_util import ( + get_generate_key, +) class TransactionTimeRescanTest(BitcoinTestFramework): @@ -24,6 +29,10 @@ def set_test_params(self): self.disable_mocktime = True self.setup_clean_chain = False self.num_nodes = 3 + self.extra_args = [["-keypool=400"], + ["-keypool=400"], + [] + ] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -172,6 +181,52 @@ def run_test(self): assert_raises_rpc_error(-8, "Invalid stop_height", restorewo_wallet.rescanblockchain, 1, -1) assert_raises_rpc_error(-8, "stop_height must be greater than start_height", restorewo_wallet.rescanblockchain, 20, 10) + self.log.info("Test `rescanblockchain` fails when wallet is encrypted and locked") + usernode.createwallet(wallet_name="enc_wallet", passphrase="passphrase") + enc_wallet = usernode.get_wallet_rpc("enc_wallet") + assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", enc_wallet.rescanblockchain) + + if not self.options.descriptors: + self.log.info("Test rescanning an encrypted wallet") + hd_seed = get_generate_key().privkey + + usernode.createwallet(wallet_name="temp_wallet", blank=True, descriptors=False) + temp_wallet = usernode.get_wallet_rpc("temp_wallet") + temp_wallet.sethdseed(seed=hd_seed) + + for i in range(399): + temp_wallet.getnewaddress() + + self.generatetoaddress(usernode, COINBASE_MATURITY + 1, temp_wallet.getnewaddress()) + self.generatetoaddress(usernode, COINBASE_MATURITY + 1, temp_wallet.getnewaddress()) + + minernode.createwallet("encrypted_wallet", blank=True, passphrase="passphrase", descriptors=False) + encrypted_wallet = minernode.get_wallet_rpc("encrypted_wallet") + + encrypted_wallet.walletpassphrase("passphrase", 99999) + encrypted_wallet.sethdseed(seed=hd_seed) + + t = threading.Thread(target=encrypted_wallet.rescanblockchain) + + with minernode.assert_debug_log(expected_msgs=[f'Rescan started from block 000008ca1832a4baf228eb1553c03d3a2c8e02399550dd6ea8d65cec3ef23d2e... (slow variant inspecting all blocks)'], timeout=5): + t.start() + + # set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan + minernode.cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1) + + try: + minernode.cli("-rpcwallet=encrypted_wallet").walletlock() + except JSONRPCException as e: + assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error['message'] + + try: + minernode.cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase") + except JSONRPCException as e: + assert e.error['code'] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error['message'] + + t.join() + + assert_equal(encrypted_wallet.getbalance(), temp_wallet.getbalance()) if __name__ == '__main__': TransactionTimeRescanTest().main() diff --git a/test/functional/wallet_upgradetohd.py b/test/functional/wallet_upgradetohd.py index 2ccf9cfd826b..36de68c23b37 100755 --- a/test/functional/wallet_upgradetohd.py +++ b/test/functional/wallet_upgradetohd.py @@ -188,7 +188,7 @@ def run_test(self): self.log.info("Same mnemonic, same mnemonic passphrase, encrypt wallet on upgrade, should recover all coins after rescan") walletpass = "111pass222" assert node.upgradetohd(mnemonic[0], "", walletpass) - self.nodes[0].rescanblockchain() + assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.rescanblockchain) assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.dumphdinfo) node.walletpassphrase(walletpass, 100) assert_equal(mnemonic, get_mnemonic(node)) @@ -209,7 +209,7 @@ def run_test(self): # Null characters are allowed in wallet passphrases since v23 walletpass = "111\0pass222" node.encryptwallet(walletpass) - self.nodes[0].rescanblockchain() + assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.rescanblockchain) assert_raises_rpc_error(-13, "Error: Wallet encrypted but passphrase not supplied to RPC.", node.upgradetohd, mnemonic[0]) assert_raises_rpc_error(-14, "Error: The wallet passphrase entered was incorrect", node.upgradetohd, mnemonic[0], "", "111") assert node.upgradetohd(mnemonic[0], "", walletpass)