diff options
80 files changed, 1475 insertions, 496 deletions
diff --git a/contrib/builder-keys/keys.txt b/contrib/builder-keys/keys.txt index db28cd07a0..890406c745 100644 --- a/contrib/builder-keys/keys.txt +++ b/contrib/builder-keys/keys.txt @@ -5,6 +5,7 @@ E944AE667CF960B1004BC32FCA662BE18B877A60 Andreas Schildbach (aschildbach) 590B7292695AFFA5B672CBB2E13FC145CD3F4304 Antoine Poinsot (darosior) 0AD83877C1F0CD1EE9BD660AD7CC770B81FD22A8 Ben Carman (benthecarman) 912FD3228387123DC97E0E57D5566241A0295FA9 BtcDrak (btcdrak) +04017A2A6D9A0CCDC81D8EC296AB007F1A7ED999 Carl Dong (dongcarl) C519EBCF3B926298946783EFF6430754120EC2F4 Christian Decker (cdecker) 18AE2F798E0D239755DA4FD24B79F986CBDF8736 Chun Kuan Le (ken2812221) 101598DC823C1B5F9A6624ABA5E0907A0380E6C3 CoinForensics (CoinForensics) @@ -19,6 +20,7 @@ D35176BE9264832E4ACA8986BF0792FBE95DC863 fivepiece (fivepiece) 01CDF4627A3B88AAE4A571C87588242FBE38D3A8 Gavin Andresen (gavinandresen) D1DBF2C4B96F2DEBF4C16654410108112E7EA81F Hennadii Stepanov (hebasto) A2FD494D0021AA9B4FA58F759102B7AE654A4A5A Ilyas Ridhuan (IlyasRidhuan) +2688F5A9A4BE0F295E921E8A25F27A38A47AD566 James O'Beirne (jamesob) D3F22A3A4C366C2DCB66D3722DA9C5A7FA81EA35 Jarol Rodriguez (jarolrod) 7480909378D544EA6B6DCEB7535B12980BB8A4D3 Jeffri H Frontz (jhfrontz) D3CC177286005BB8FF673294C5242A1AB3936517 jl2012 (jl2012) diff --git a/contrib/guix/guix-attest b/contrib/guix/guix-attest index dcf709b542..1503c330b2 100755 --- a/contrib/guix/guix-attest +++ b/contrib/guix/guix-attest @@ -159,20 +159,6 @@ Hint: You may wish to remove the existing attestations and their signatures by EOF } -# Given a document with unix line endings (just <LF>) in stdin, make all lines -# end in <CR><LF> and make sure there's no trailing <LF> at the end of the file. -# -# This is necessary as cleartext signatures are calculated on text after their -# line endings are canonicalized. -# -# For more information: -# 1. https://security.stackexchange.com/a/104261 -# 2. https://datatracker.ietf.org/doc/html/rfc4880#section-7.1 -# -rfc4880_normalize_document() { - sed 's/$/\r/' | head -c -2 -} - echo "Attesting to build outputs for version: '${VERSION}'" echo "" @@ -188,7 +174,6 @@ mkdir -p "$outsigdir" cat "${noncodesigned_fragments[@]}" \ | sort -u \ | sort -k2 \ - | rfc4880_normalize_document \ > "$temp_noncodesigned" if [ -e noncodesigned.SHA256SUMS ]; then # The SHA256SUMS already exists, make sure it's exactly what we @@ -216,7 +201,6 @@ mkdir -p "$outsigdir" cat "${sha256sum_fragments[@]}" \ | sort -u \ | sort -k2 \ - | rfc4880_normalize_document \ > "$temp_all" if [ -e all.SHA256SUMS ]; then # The SHA256SUMS already exists, make sure it's exactly what we diff --git a/contrib/guix/guix-verify b/contrib/guix/guix-verify index e4863f115b..02ae022741 100755 --- a/contrib/guix/guix-verify +++ b/contrib/guix/guix-verify @@ -77,11 +77,13 @@ verify() { echo "" echo "Hint: Either the signature is invalid or the public key is missing" echo "" + failure=1 elif ! diff --report-identical "$compare_manifest" "$current_manifest" 1>&2; then echo "ERR: The SHA256SUMS attestation in these two directories differ:" echo " '${compare_manifest}'" echo " '${current_manifest}'" echo "" + failure=1 else echo "Verified: '${current_manifest}'" echo "" @@ -166,3 +168,7 @@ if (( ${#all_noncodesigned[@]} + ${#all_all[@]} == 0 )); then echo "" exit 1 fi + +if [ -n "$failure" ]; then + exit 1 +fi diff --git a/doc/release-notes.md b/doc/release-notes.md index 61c65d5a3e..01ef3610c9 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -108,6 +108,9 @@ RPC Tests ----- +- For the `regtest` network the BIP 66 (DERSIG) activation height was changed + from 1251 to 102. (#22632) + Credits ======= diff --git a/doc/release-process.md b/doc/release-process.md index c57fa5b23a..1b6472e812 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -199,26 +199,13 @@ popd ### After 3 or more people have guix-built and their results match: -Combine `all.SHA256SUMS` and `all.SHA256SUMS.asc` into a clear-signed -`SHA256SUMS.asc` message: - -```sh -echo -e "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n$(cat all.SHA256SUMS)\n$(cat filename.txt.asc)" > SHA256SUMS.asc -``` - -Here's an equivalent, more readable command if you're confident that you won't -mess up whitespaces when copy-pasting: +Combine the `all.SHA256SUMS.asc` file from all signers into `SHA256SUMS.asc`: ```bash -cat << EOF > SHA256SUMS.asc ------BEGIN PGP SIGNED MESSAGE----- -Hash: SHA256 - -$(cat all.SHA256SUMS) -$(cat all.SHA256SUMS.asc) -EOF +cat "$VERSION"/*/all.SHA256SUMS.asc > SHA256SUMS.asc ``` + - Upload to the bitcoincore.org server (`/var/www/bin/bitcoin-core-${VERSION}`): 1. The contents of `./bitcoin/guix-build-${VERSION}/output`, except for `*-debug*` files. @@ -230,7 +217,9 @@ EOF as save storage space *do not upload these to the bitcoincore.org server, nor put them in the torrent*. - 2. The combined clear-signed message you just created `SHA256SUMS.asc` + 2. The `SHA256SUMS` file + + 3. The `SHA256SUMS.asc` combined signature file you just created - Create a torrent of the `/var/www/bin/bitcoin-core-${VERSION}` directory such that at the top level there is only one file: the `bitcoin-core-${VERSION}` diff --git a/src/addrman.cpp b/src/addrman.cpp index 96139182d3..e8f98f727b 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -431,11 +431,16 @@ CAddrInfo CAddrMan::Select_(bool newOnly) const } } -#ifdef DEBUG_ADDRMAN -int CAddrMan::Check_() +int CAddrMan::Check_() const { AssertLockHeld(cs); + // Run consistency checks 1 in m_consistency_check_ratio times if enabled + if (m_consistency_check_ratio == 0) return 0; + if (insecure_rand.randrange(m_consistency_check_ratio) >= 1) return 0; + + LogPrint(BCLog::ADDRMAN, "Addrman checks started: new %i, tried %i, total %u\n", nNew, nTried, vRandom.size()); + std::unordered_set<int> setTried; std::unordered_map<int, int> mapNew; @@ -458,8 +463,10 @@ int CAddrMan::Check_() return -4; mapNew[n] = info.nRefCount; } - if (mapAddr[info] != n) + const auto it{mapAddr.find(info)}; + if (it == mapAddr.end() || it->second != n) { return -5; + } if (info.nRandomPos < 0 || (size_t)info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n) return -14; if (info.nLastTry < 0) @@ -478,10 +485,13 @@ int CAddrMan::Check_() if (vvTried[n][i] != -1) { if (!setTried.count(vvTried[n][i])) return -11; - if (mapInfo[vvTried[n][i]].GetTriedBucket(nKey, m_asmap) != n) + const auto it{mapInfo.find(vvTried[n][i])}; + if (it == mapInfo.end() || it->second.GetTriedBucket(nKey, m_asmap) != n) { return -17; - if (mapInfo[vvTried[n][i]].GetBucketPosition(nKey, false, n) != i) + } + if (it->second.GetBucketPosition(nKey, false, n) != i) { return -18; + } setTried.erase(vvTried[n][i]); } } @@ -492,8 +502,10 @@ int CAddrMan::Check_() if (vvNew[n][i] != -1) { if (!mapNew.count(vvNew[n][i])) return -12; - if (mapInfo[vvNew[n][i]].GetBucketPosition(nKey, true, n) != i) + const auto it{mapInfo.find(vvNew[n][i])}; + if (it == mapInfo.end() || it->second.GetBucketPosition(nKey, true, n) != i) { return -19; + } if (--mapNew[vvNew[n][i]] == 0) mapNew.erase(vvNew[n][i]); } @@ -507,9 +519,9 @@ int CAddrMan::Check_() if (nKey.IsNull()) return -16; + LogPrint(BCLog::ADDRMAN, "Addrman checks completed successfully\n"); return 0; } -#endif void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) const { diff --git a/src/addrman.h b/src/addrman.h index 1dd1932421..3ee8c3ee09 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -26,6 +26,9 @@ #include <unordered_map> #include <vector> +/** Default for -checkaddrman */ +static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0}; + /** * Extended statistics about a CAddress */ @@ -124,8 +127,8 @@ public: * attempt was unsuccessful. * * Bucket selection is based on cryptographic hashing, using a randomly-generated 256-bit key, which should not * be observable by adversaries. - * * Several indexes are kept for high performance. Defining DEBUG_ADDRMAN will introduce frequent (and expensive) - * consistency checks for the entire data structure. + * * Several indexes are kept for high performance. Setting m_consistency_check_ratio with the -checkaddrman + * configuration option will introduce (expensive) consistency checks for the entire data structure. */ //! total number of buckets for tried addresses @@ -493,9 +496,12 @@ public: mapAddr.clear(); } - CAddrMan() + explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio) + : insecure_rand{deterministic}, + m_consistency_check_ratio{consistency_check_ratio} { Clear(); + if (deterministic) nKey = uint256{1}; } ~CAddrMan() @@ -637,13 +643,13 @@ protected: //! secret key to randomize bucket select with uint256 nKey; - //! Source of random numbers for randomization in inner loops - mutable FastRandomContext insecure_rand GUARDED_BY(cs); - //! A mutex to protect the inner data structures. mutable Mutex cs; private: + //! Source of random numbers for randomization in inner loops + mutable FastRandomContext insecure_rand GUARDED_BY(cs); + //! Serialization versions. enum Format : uint8_t { V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 @@ -698,6 +704,9 @@ private: //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions. std::set<int> m_tried_collisions; + /** Perform consistency checks every m_consistency_check_ratio operations (if non-zero). */ + const int32_t m_consistency_check_ratio; + //! Find an entry. CAddrInfo* Find(const CNetAddr& addr, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -735,22 +744,19 @@ private: CAddrInfo SelectTriedCollision_() EXCLUSIVE_LOCKS_REQUIRED(cs); //! Consistency check - void Check() const - EXCLUSIVE_LOCKS_REQUIRED(cs) + void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs) { -#ifdef DEBUG_ADDRMAN AssertLockHeld(cs); + const int err = Check_(); if (err) { LogPrintf("ADDRMAN CONSISTENCY CHECK FAILED!!! err=%i\n", err); + assert(false); } -#endif } -#ifdef DEBUG_ADDRMAN //! Perform consistency check. Returns an error code or zero. int Check_() const EXCLUSIVE_LOCKS_REQUIRED(cs); -#endif /** * Return all or many randomly selected addresses, optionally by network. diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index b7bd8a3261..5ae2dafd5a 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -72,7 +72,7 @@ static void AddrManAdd(benchmark::Bench& bench) { CreateAddresses(); - CAddrMan addrman; + CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0); bench.run([&] { AddAddressesToAddrMan(addrman); @@ -82,7 +82,7 @@ static void AddrManAdd(benchmark::Bench& bench) static void AddrManSelect(benchmark::Bench& bench) { - CAddrMan addrman; + CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0); FillAddrMan(addrman); @@ -94,7 +94,7 @@ static void AddrManSelect(benchmark::Bench& bench) static void AddrManGetAddr(benchmark::Bench& bench) { - CAddrMan addrman; + CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0); FillAddrMan(addrman); @@ -112,10 +112,12 @@ static void AddrManGood(benchmark::Bench& bench) * we want to do the same amount of work in every loop iteration. */ bench.epochs(5).epochIterations(1); + const size_t addrman_count{bench.epochs() * bench.epochIterations()}; - std::vector<CAddrMan> addrmans(bench.epochs() * bench.epochIterations()); - for (auto& addrman : addrmans) { - FillAddrMan(addrman); + std::vector<std::unique_ptr<CAddrMan>> addrmans(addrman_count); + for (size_t i{0}; i < addrman_count; ++i) { + addrmans[i] = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ 0); + FillAddrMan(*addrmans[i]); } auto markSomeAsGood = [](CAddrMan& addrman) { @@ -130,7 +132,7 @@ static void AddrManGood(benchmark::Bench& bench) uint64_t i = 0; bench.run([&] { - markSomeAsGood(addrmans.at(i)); + markSomeAsGood(*addrmans.at(i)); ++i; }); } diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 1ec6411e32..bc0af6398c 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -885,6 +885,29 @@ static void GetWalletBalances(UniValue& result) } /** + * GetProgressBar contructs a progress bar with 5% intervals. + * + * @param[in] progress The proportion of the progress bar to be filled between 0 and 1. + * @param[out] progress_bar String representation of the progress bar. + */ +static void GetProgressBar(double progress, std::string& progress_bar) +{ + if (progress < 0 || progress > 1) return; + + static constexpr double INCREMENT{0.05}; + static const std::string COMPLETE_BAR{"\u2592"}; + static const std::string INCOMPLETE_BAR{"\u2591"}; + + for (int i = 0; i < progress / INCREMENT; ++i) { + progress_bar += COMPLETE_BAR; + } + + for (int i = 0; i < (1 - progress) / INCREMENT; ++i) { + progress_bar += INCOMPLETE_BAR; + } +} + +/** * ParseGetInfoResult takes in -getinfo result in UniValue object and parses it * into a user friendly UniValue string to be printed on the console. * @param[out] result Reference to UniValue result containing the -getinfo output. @@ -926,7 +949,17 @@ static void ParseGetInfoResult(UniValue& result) std::string result_string = strprintf("%sChain: %s%s\n", BLUE, result["chain"].getValStr(), RESET); result_string += strprintf("Blocks: %s\n", result["blocks"].getValStr()); result_string += strprintf("Headers: %s\n", result["headers"].getValStr()); - result_string += strprintf("Verification progress: %.4f%%\n", result["verificationprogress"].get_real() * 100); + + const double ibd_progress{result["verificationprogress"].get_real()}; + std::string ibd_progress_bar; + // Display the progress bar only if IBD progress is less than 99% + if (ibd_progress < 0.99) { + GetProgressBar(ibd_progress, ibd_progress_bar); + // Add padding between progress bar and IBD progress + ibd_progress_bar += " "; + } + + result_string += strprintf("Verification progress: %s%.4f%%\n", ibd_progress_bar, ibd_progress * 100); result_string += strprintf("Difficulty: %s\n\n", result["difficulty"].getValStr()); result_string += strprintf( diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 0b3242b1aa..c3bbb147be 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -393,7 +393,7 @@ public: consensus.BIP34Height = 2; // BIP34 activated on regtest (Block at height 1 not enforced for testing purposes) consensus.BIP34Hash = uint256(); consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in functional tests) - consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in functional tests) + consensus.BIP66Height = 102; // BIP66 activated on regtest (Block at height 101 and earlier not enforced for testing purposes) consensus.CSVHeight = 432; // CSV activated on regtest (Used in rpc activation tests) consensus.SegwitHeight = 0; // SEGWIT is always activated on regtest unless overridden consensus.MinBIP9WarningHeight = 0; diff --git a/src/clientversion.cpp b/src/clientversion.cpp index 29c38e2d3b..195f58b3f3 100644 --- a/src/clientversion.cpp +++ b/src/clientversion.cpp @@ -30,8 +30,10 @@ const std::string CLIENT_NAME("Satoshi"); #define BUILD_DESC BUILD_GIT_TAG #define BUILD_SUFFIX "" #else - #define BUILD_DESC "v" STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD) - #ifdef BUILD_GIT_COMMIT + #define BUILD_DESC "v" PACKAGE_VERSION + #if CLIENT_VERSION_IS_RELEASE + #define BUILD_SUFFIX "" + #elif defined(BUILD_GIT_COMMIT) #define BUILD_SUFFIX "-" BUILD_GIT_COMMIT #elif defined(GIT_COMMIT_ID) #define BUILD_SUFFIX "-g" GIT_COMMIT_ID diff --git a/src/init.cpp b/src/init.cpp index 1b406bed28..9154fc0a6f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -501,7 +501,8 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-checkblocks=<n>", strprintf("How many blocks to check at startup (default: %u, 0 = all)", DEFAULT_CHECKBLOCKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checklevel=<n>", strprintf("How thorough the block verification of -checkblocks is: %s (0-4, default: %u)", Join(CHECKLEVEL_DOC, ", "), DEFAULT_CHECKLEVEL), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checkblockindex", strprintf("Do a consistency check for the block tree, chainstate, and other validation data structures occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-checkmempool=<n>", strprintf("Run checks every <n> transactions (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-checkaddrman=<n>", strprintf("Run addrman consistency checks every <n> operations. Use 0 to disable. (default: %u)", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-checkmempool=<n>", strprintf("Run mempool consistency checks every <n> transactions. Use 0 to disable. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-checkpoints", strprintf("Enable rejection of any forks from the known historical chain until block %s (default: %u)", defaultChainParams->Checkpoints().GetHeight(), DEFAULT_CHECKPOINTS_ENABLED), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-deprecatedrpc=<method>", "Allows deprecated RPC method(s) to be used", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-stopafterblockimport", strprintf("Stop running after importing blocks from disk (default: %u)", DEFAULT_STOPAFTERBLOCKIMPORT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); @@ -1164,7 +1165,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)}; assert(!node.addrman); - node.addrman = std::make_unique<CAddrMan>(); + auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); + node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ check_addrman); assert(!node.banman); node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8243ef0f55..0cb13f9253 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -161,7 +161,7 @@ static constexpr size_t MAX_ADDR_TO_SEND{1000}; static constexpr double MAX_ADDR_RATE_PER_SECOND{0.1}; /** The soft limit of the address processing token bucket (the regular MAX_ADDR_RATE_PER_SECOND * based increments won't go above this, but the MAX_ADDR_TO_SEND increment following GETADDR - * is exempt from this limit. */ + * is exempt from this limit). */ static constexpr size_t MAX_ADDR_PROCESSING_TOKEN_BUCKET{MAX_ADDR_TO_SEND}; // Internal stuff @@ -263,14 +263,14 @@ struct Peer { std::atomic_bool m_wants_addrv2{false}; /** Whether this peer has already sent us a getaddr message. */ bool m_getaddr_recvd{false}; - /** Number of addr messages that can be processed from this peer. Start at 1 to + /** Number of addresses that can be processed from this peer. Start at 1 to * permit self-announcement. */ double m_addr_token_bucket{1.0}; /** When m_addr_token_bucket was last updated */ std::chrono::microseconds m_addr_token_timestamp{GetTime<std::chrono::microseconds>()}; /** Total number of addresses that were dropped due to rate limiting. */ std::atomic<uint64_t> m_addr_rate_limited{0}; - /** Total number of addresses that were processed (excludes rate limited ones). */ + /** Total number of addresses that were processed (excludes rate-limited ones). */ std::atomic<uint64_t> m_addr_processed{0}; /** Set of txids to reconsider once their parent transactions have been accepted **/ @@ -2848,11 +2848,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; // Apply rate limiting. - if (rate_limited) { - if (peer->m_addr_token_bucket < 1.0) { + if (peer->m_addr_token_bucket < 1.0) { + if (rate_limited) { ++num_rate_limit; continue; } + } else { peer->m_addr_token_bucket -= 1.0; } // We only bother storing full nodes, though this may include @@ -2880,12 +2881,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } peer->m_addr_processed += num_proc; peer->m_addr_rate_limited += num_rate_limit; - LogPrint(BCLog::NET, "Received addr: %u addresses (%u processed, %u rate-limited) from peer=%d%s\n", - vAddr.size(), - num_proc, - num_rate_limit, - pfrom.GetId(), - fLogIPs ? ", peeraddr=" + pfrom.addr.ToString() : ""); + LogPrint(BCLog::NET, "Received addr: %u addresses (%u processed, %u rate-limited) from peer=%d\n", + vAddr.size(), num_proc, num_rate_limit, pfrom.GetId()); m_addrman.Add(vAddrOk, pfrom.addr, 2 * 60 * 60); if (vAddr.size() < 1000) peer->m_getaddr_sent = false; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 4ab4e388d9..f6ea147ddb 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -28,6 +28,7 @@ #include <qt/utilitydialog.h> #include <qt/winshutdownmonitor.h> #include <uint256.h> +#include <util/string.h> #include <util/system.h> #include <util/threadnames.h> #include <util/translation.h> @@ -144,6 +145,53 @@ static void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTrans QApplication::installTranslator(&translator); } +static bool InitSettings() +{ + if (!gArgs.GetSettingsPath()) { + return true; // Do nothing if settings file disabled. + } + + std::vector<std::string> errors; + if (!gArgs.ReadSettingsFile(&errors)) { + bilingual_str error = _("Settings file could not be read"); + InitError(Untranslated(strprintf("%s:\n%s\n", error.original, MakeUnorderedList(errors)))); + + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort); + /*: Explanatory text shown on startup when the settings file cannot be read. + Prompts user to make a choice between resetting or aborting. */ + messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); + messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(errors))); + messagebox.setTextFormat(Qt::PlainText); + messagebox.setDefaultButton(QMessageBox::Reset); + switch (messagebox.exec()) { + case QMessageBox::Reset: + break; + case QMessageBox::Abort: + return false; + default: + assert(false); + } + } + + errors.clear(); + if (!gArgs.WriteSettingsFile(&errors)) { + bilingual_str error = _("Settings file could not be written"); + InitError(Untranslated(strprintf("%s:\n%s\n", error.original, MakeUnorderedList(errors)))); + + QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Ok); + /*: Explanatory text shown on startup when the settings file could not be written. + Prompts user to check that we have the ability to write to the file. + Explains that the user has the option of running without a settings file.*/ + messagebox.setInformativeText(QObject::tr("A fatal error occurred. Check that settings file is writable, or try running with -nosettings.")); + messagebox.setDetailedText(QString::fromStdString(MakeUnorderedList(errors))); + messagebox.setTextFormat(Qt::PlainText); + messagebox.setDefaultButton(QMessageBox::Ok); + messagebox.exec(); + return false; + } + return true; +} + /* qDebug() message handler --> debug.log */ void DebugMessageHandler(QtMsgType type, const QMessageLogContext& context, const QString &msg) { @@ -295,6 +343,17 @@ void BitcoinApplication::requestShutdown() window->setClientModel(nullptr); pollShutdownTimer->stop(); +#ifdef ENABLE_WALLET + // Delete wallet controller here manually, instead of relying on Qt object + // tracking (https://doc.qt.io/qt-5/objecttrees.html). This makes sure + // walletmodel m_handle_* notification handlers are deleted before wallets + // are unloaded, which can simplify wallet implementations. It also avoids + // these notifications having to be handled while GUI objects are being + // destroyed, making GUI code less fragile as well. + delete m_wallet_controller; + m_wallet_controller = nullptr; +#endif // ENABLE_WALLET + delete clientModel; clientModel = nullptr; @@ -512,9 +571,8 @@ int GuiMain(int argc, char* argv[]) // Parse URIs on command line -- this can affect Params() PaymentServer::ipcParseCommandLine(argc, argv); #endif - if (!gArgs.InitSettings(error)) { - InitError(Untranslated(error)); - QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error initializing settings: %1").arg(QString::fromStdString(error))); + + if (!InitSettings()) { return EXIT_FAILURE; } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 863225099a..fe606519af 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -110,6 +110,9 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); activity->create(); }); + connect(walletFrame, &WalletFrame::message, [this](const QString& title, const QString& message, unsigned int style) { + this->message(title, message, style); + }); setCentralWidget(walletFrame); } else #endif // ENABLE_WALLET diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp index dc24bbc6a6..f9a61c3e60 100644 --- a/src/qt/createwalletdialog.cpp +++ b/src/qt/createwalletdialog.cpp @@ -32,7 +32,7 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) : // set to true, enable it when isEncryptWalletChecked is false. ui->disable_privkeys_checkbox->setEnabled(!checked); #ifdef ENABLE_EXTERNAL_SIGNER - ui->external_signer_checkbox->setEnabled(!checked); + ui->external_signer_checkbox->setEnabled(m_has_signers && !checked); #endif // When the disable_privkeys_checkbox is disabled, uncheck it. if (!ui->disable_privkeys_checkbox->isEnabled()) { @@ -115,7 +115,8 @@ CreateWalletDialog::~CreateWalletDialog() void CreateWalletDialog::setSigners(const std::vector<ExternalSigner>& signers) { - if (!signers.empty()) { + m_has_signers = !signers.empty(); + if (m_has_signers) { ui->external_signer_checkbox->setEnabled(true); ui->external_signer_checkbox->setChecked(true); ui->encrypt_wallet_checkbox->setEnabled(false); diff --git a/src/qt/createwalletdialog.h b/src/qt/createwalletdialog.h index 25ddf97585..fc13cc44eb 100644 --- a/src/qt/createwalletdialog.h +++ b/src/qt/createwalletdialog.h @@ -35,6 +35,7 @@ public: private: Ui::CreateWalletDialog *ui; + bool m_has_signers = false; }; #endif // BITCOIN_QT_CREATEWALLETDIALOG_H diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index bd72328c02..2ff1445709 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -51,20 +51,20 @@ </spacer> </item> <item> - <layout class="QHBoxLayout" name="horizontalLayout_Main_Prune"> - <item> - <widget class="QCheckBox" name="prune"> - <property name="toolTip"> - <string>Enabling pruning significantly reduces the disk space required to store transactions. All blocks are still fully validated. Reverting this setting requires re-downloading the entire blockchain.</string> - </property> - <property name="text"> - <string>Prune &block storage to</string> - </property> - </widget> - </item> - <item> - <widget class="QSpinBox" name="pruneSize"/> - </item> + <layout class="QHBoxLayout" name="horizontalLayout_Main_Prune"> + <item> + <widget class="QCheckBox" name="prune"> + <property name="toolTip"> + <string>Enabling pruning significantly reduces the disk space required to store transactions. All blocks are still fully validated. Reverting this setting requires re-downloading the entire blockchain.</string> + </property> + <property name="text"> + <string>Prune &block storage to</string> + </property> + </widget> + </item> + <item> + <widget class="QSpinBox" name="pruneSize"/> + </item> <item> <widget class="QLabel" name="pruneSizeUnitLabel"> <property name="text"> @@ -201,6 +201,16 @@ </attribute> <layout class="QVBoxLayout" name="verticalLayout_Wallet"> <item> + <widget class="QCheckBox" name="subFeeFromAmount"> + <property name="toolTip"> + <string extracomment="Tooltip text for Options window setting that sets subtracting the fee from a sending amount as default.">Whether to set subtract fee from amount as default or not.</string> + </property> + <property name="text"> + <string extracomment="An Options window setting to set subtracting the fee from a sending amount as default.">Subtract &fee from amount by default</string> + </property> + </widget> + </item> + <item> <widget class="QGroupBox" name="groupBox"> <property name="title"> <string>Expert</string> @@ -235,27 +245,27 @@ <string>External Signer (e.g. hardware wallet)</string> </property> <layout class="QVBoxLayout" name="verticalLayoutHww"> - <item> - <layout class="QHBoxLayout" name="horizontalLayoutHww"> - <item> - <widget class="QLabel" name="externalSignerPathLabel"> - <property name="text"> - <string>&External signer script path</string> - </property> - <property name="buddy"> - <cstring>externalSignerPath</cstring> - </property> - </widget> - </item> - <item> - <widget class="QLineEdit" name="externalSignerPath"> - <property name="toolTip"> - <string>Full path to a Bitcoin Core compatible script (e.g. C:\Downloads\hwi.exe or /Users/you/Downloads/hwi.py). Beware: malware can steal your coins!</string> - </property> - </widget> - </item> - </layout> - </item> + <item> + <layout class="QHBoxLayout" name="horizontalLayoutHww"> + <item> + <widget class="QLabel" name="externalSignerPathLabel"> + <property name="text"> + <string>&External signer script path</string> + </property> + <property name="buddy"> + <cstring>externalSignerPath</cstring> + </property> + </widget> + </item> + <item> + <widget class="QLineEdit" name="externalSignerPath"> + <property name="toolTip"> + <string>Full path to a Bitcoin Core compatible script (e.g. C:\Downloads\hwi.exe or /Users/you/Downloads/hwi.py). Beware: malware can steal your coins!</string> + </property> + </widget> + </item> + </layout> + </item> </layout> </widget> </item> diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index b12fe96567..5ad4fc9b33 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -239,6 +239,7 @@ void OptionsDialog::setMapper() /* Wallet */ mapper->addMapping(ui->spendZeroConfChange, OptionsModel::SpendZeroConfChange); mapper->addMapping(ui->coinControlFeatures, OptionsModel::CoinControlFeatures); + mapper->addMapping(ui->subFeeFromAmount, OptionsModel::SubFeeFromAmount); mapper->addMapping(ui->externalSignerPath, OptionsModel::ExternalSignerPath); /* Network */ diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 24a4e9ee96..d87fc1f84a 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -124,6 +124,11 @@ void OptionsModel::Init(bool resetSettings) if (!gArgs.SoftSetArg("-signer", settings.value("external_signer_path").toString().toStdString())) { addOverriddenOption("-signer"); } + + if (!settings.contains("SubFeeFromAmount")) { + settings.setValue("SubFeeFromAmount", false); + } + m_sub_fee_from_amount = settings.value("SubFeeFromAmount", false).toBool(); #endif // Network @@ -335,6 +340,8 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const return settings.value("bSpendZeroConfChange"); case ExternalSignerPath: return settings.value("external_signer_path"); + case SubFeeFromAmount: + return m_sub_fee_from_amount; #endif case DisplayUnit: return nDisplayUnit; @@ -460,6 +467,10 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in setRestartRequired(true); } break; + case SubFeeFromAmount: + m_sub_fee_from_amount = value.toBool(); + settings.setValue("SubFeeFromAmount", m_sub_fee_from_amount); + break; #endif case DisplayUnit: setDisplayUnit(value); diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 535843e8ba..203ee27ad8 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -61,6 +61,7 @@ public: Language, // QString UseEmbeddedMonospacedFont, // bool CoinControlFeatures, // bool + SubFeeFromAmount, // bool ThreadsScriptVerif, // int Prune, // bool PruneSize, // int @@ -88,6 +89,7 @@ public: QString getThirdPartyTxUrls() const { return strThirdPartyTxUrls; } bool getUseEmbeddedMonospacedFont() const { return m_use_embedded_monospaced_font; } bool getCoinControlFeatures() const { return fCoinControlFeatures; } + bool getSubFeeFromAmount() const { return m_sub_fee_from_amount; } const QString& getOverriddenByCommandLine() { return strOverriddenByCommandLine; } /* Explicit setters */ @@ -112,6 +114,7 @@ private: QString strThirdPartyTxUrls; bool m_use_embedded_monospaced_font; bool fCoinControlFeatures; + bool m_sub_fee_from_amount; /* settings that were overridden by command-line */ QString strOverriddenByCommandLine; diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 1b7fda6e77..98efaf29d7 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -72,8 +72,13 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const case NetNodeId: return (qint64)rec->nodeStats.nodeid; case Address: - // prepend to peer address down-arrow symbol for inbound connection and up-arrow for outbound connection - return QString::fromStdString((rec->nodeStats.fInbound ? "↓ " : "↑ ") + rec->nodeStats.addrName); + return QString::fromStdString(rec->nodeStats.addrName); + case Direction: + return QString(rec->nodeStats.fInbound ? + //: An Inbound Connection from a Peer. + tr("Inbound") : + //: An Outbound Connection to a Peer. + tr("Outbound")); case ConnectionType: return GUIUtil::ConnectionTypeToQString(rec->nodeStats.m_conn_type, /* prepend_direction */ false); case Network: @@ -94,6 +99,7 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const return QVariant(Qt::AlignRight | Qt::AlignVCenter); case Address: return {}; + case Direction: case ConnectionType: case Network: return QVariant(Qt::AlignCenter); diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 0d841ebf28..40265ee266 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -48,6 +48,7 @@ public: enum ColumnIndex { NetNodeId = 0, Address, + Direction, ConnectionType, Network, Ping, @@ -84,6 +85,9 @@ private: /*: Title of Peers Table column which contains the IP/Onion/I2P address of the connected peer. */ tr("Address"), + /*: Title of Peers Table column which indicates the direction + the peer connection was initiated from. */ + tr("Direction"), /*: Title of Peers Table column which describes the type of peer connection. The "type" describes why the connection exists. */ tr("Type"), diff --git a/src/qt/peertablesortproxy.cpp b/src/qt/peertablesortproxy.cpp index 78932da8d4..f92eef48f1 100644 --- a/src/qt/peertablesortproxy.cpp +++ b/src/qt/peertablesortproxy.cpp @@ -26,6 +26,8 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd return left_stats.nodeid < right_stats.nodeid; case PeerTableModel::Address: return left_stats.addrName.compare(right_stats.addrName) < 0; + case PeerTableModel::Direction: + return left_stats.fInbound > right_stats.fInbound; // default sort Inbound, then Outbound case PeerTableModel::ConnectionType: return left_stats.m_conn_type < right_stats.m_conn_type; case PeerTableModel::Network: diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index 2adfeeaaf0..289fb9f7c8 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -47,18 +47,22 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx) { m_transaction_data = psbtx; - bool complete; - size_t n_could_sign; - FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness. - TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, &n_could_sign, m_transaction_data, complete); - if (err != TransactionError::OK) { - showStatus(tr("Failed to load transaction: %1") - .arg(QString::fromStdString(TransactionErrorString(err).translated)), StatusLevel::ERR); - return; + bool complete = FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness. + if (m_wallet_model) { + size_t n_could_sign; + TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, &n_could_sign, m_transaction_data, complete); + if (err != TransactionError::OK) { + showStatus(tr("Failed to load transaction: %1") + .arg(QString::fromStdString(TransactionErrorString(err).translated)), + StatusLevel::ERR); + return; + } + m_ui->signTransactionButton->setEnabled(!complete && !m_wallet_model->wallet().privateKeysDisabled() && n_could_sign > 0); + } else { + m_ui->signTransactionButton->setEnabled(false); } m_ui->broadcastTransactionButton->setEnabled(complete); - m_ui->signTransactionButton->setEnabled(!complete && !m_wallet_model->wallet().privateKeysDisabled() && n_could_sign > 0); updateTransactionDisplay(); } @@ -133,7 +137,7 @@ void PSBTOperationsDialog::saveTransaction() { } CTxDestination address; ExtractDestination(out.scriptPubKey, address); - QString amount = BitcoinUnits::format(m_wallet_model->getOptionsModel()->getDisplayUnit(), out.nValue); + QString amount = BitcoinUnits::format(m_client_model->getOptionsModel()->getDisplayUnit(), out.nValue); QString address_str = QString::fromStdString(EncodeDestination(address)); filename_suggestion.append(address_str + "-" + amount); first = false; @@ -224,6 +228,10 @@ void PSBTOperationsDialog::showStatus(const QString &msg, StatusLevel level) { } size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &psbtx) { + if (!m_wallet_model) { + return 0; + } + size_t n_signed; bool complete; TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, false /* bip32derivs */, &n_signed, m_transaction_data, complete); @@ -246,7 +254,10 @@ void PSBTOperationsDialog::showTransactionStatus(const PartiallySignedTransactio case PSBTRole::SIGNER: { QString need_sig_text = tr("Transaction still needs signature(s)."); StatusLevel level = StatusLevel::INFO; - if (m_wallet_model->wallet().privateKeysDisabled()) { + if (!m_wallet_model) { + need_sig_text += " " + tr("(But no wallet is loaded.)"); + level = StatusLevel::WARN; + } else if (m_wallet_model->wallet().privateKeysDisabled()) { need_sig_text += " " + tr("(But this wallet cannot sign transactions.)"); level = StatusLevel::WARN; } else if (n_could_sign < 1) { diff --git a/src/qt/sendcoinsentry.cpp b/src/qt/sendcoinsentry.cpp index 683c0441fa..5fa5165615 100644 --- a/src/qt/sendcoinsentry.cpp +++ b/src/qt/sendcoinsentry.cpp @@ -97,7 +97,9 @@ void SendCoinsEntry::clear() ui->payTo->clear(); ui->addAsLabel->clear(); ui->payAmount->clear(); - ui->checkboxSubtractFeeFromAmount->setCheckState(Qt::Unchecked); + if (model && model->getOptionsModel()) { + ui->checkboxSubtractFeeFromAmount->setChecked(model->getOptionsModel()->getSubFeeFromAmount()); + } ui->messageTextLabel->clear(); ui->messageTextLabel->hide(); ui->messageLabel->hide(); diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index a631f497af..75cbd6b3be 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -9,15 +9,8 @@ #include <cstdlib> -// Earliest date that can be represented (far in the past) -const QDateTime TransactionFilterProxy::MIN_DATE = QDateTime::fromTime_t(0); -// Last date that can be represented (far in the future) -const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromTime_t(0xFFFFFFFF); - TransactionFilterProxy::TransactionFilterProxy(QObject *parent) : QSortFilterProxyModel(parent), - dateFrom(MIN_DATE), - dateTo(MAX_DATE), m_search_string(), typeFilter(ALL_TYPES), watchOnlyFilter(WatchOnlyFilter_All), @@ -46,8 +39,8 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & return false; QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime(); - if (datetime < dateFrom || datetime > dateTo) - return false; + if (dateFrom && datetime < *dateFrom) return false; + if (dateTo && datetime > *dateTo) return false; QString address = index.data(TransactionTableModel::AddressRole).toString(); QString label = index.data(TransactionTableModel::LabelRole).toString(); @@ -65,10 +58,10 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & return true; } -void TransactionFilterProxy::setDateRange(const QDateTime &from, const QDateTime &to) +void TransactionFilterProxy::setDateRange(const std::optional<QDateTime>& from, const std::optional<QDateTime>& to) { - this->dateFrom = from; - this->dateTo = to; + dateFrom = from; + dateTo = to; invalidateFilter(); } diff --git a/src/qt/transactionfilterproxy.h b/src/qt/transactionfilterproxy.h index 693b363692..09bc9e75db 100644 --- a/src/qt/transactionfilterproxy.h +++ b/src/qt/transactionfilterproxy.h @@ -10,6 +10,8 @@ #include <QDateTime> #include <QSortFilterProxyModel> +#include <optional> + /** Filter the transaction list according to pre-specified rules. */ class TransactionFilterProxy : public QSortFilterProxyModel { @@ -18,10 +20,6 @@ class TransactionFilterProxy : public QSortFilterProxyModel public: explicit TransactionFilterProxy(QObject *parent = nullptr); - /** Earliest date that can be represented (far in the past) */ - static const QDateTime MIN_DATE; - /** Last date that can be represented (far in the future) */ - static const QDateTime MAX_DATE; /** Type filter bit field (all types) */ static const quint32 ALL_TYPES = 0xFFFFFFFF; @@ -34,7 +32,8 @@ public: WatchOnlyFilter_No }; - void setDateRange(const QDateTime &from, const QDateTime &to); + /** Filter transactions between date range. Use std::nullopt for open range. */ + void setDateRange(const std::optional<QDateTime>& from, const std::optional<QDateTime>& to); void setSearchString(const QString &); /** @note Type filter takes a bit field created with TYPE() or ALL_TYPES @@ -55,8 +54,8 @@ protected: bool filterAcceptsRow(int source_row, const QModelIndex & source_parent) const override; private: - QDateTime dateFrom; - QDateTime dateTo; + std::optional<QDateTime> dateFrom; + std::optional<QDateTime> dateTo; QString m_search_string; quint32 typeFilter; WatchOnlyFilter watchOnlyFilter; diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 83d17a32c0..908cb917f1 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -19,6 +19,8 @@ #include <node/ui_interface.h> +#include <optional> + #include <QApplication> #include <QComboBox> #include <QDateTimeEdit> @@ -266,26 +268,26 @@ void TransactionView::chooseDate(int idx) { case All: transactionProxyModel->setDateRange( - TransactionFilterProxy::MIN_DATE, - TransactionFilterProxy::MAX_DATE); + std::nullopt, + std::nullopt); break; case Today: transactionProxyModel->setDateRange( GUIUtil::StartOfDay(current), - TransactionFilterProxy::MAX_DATE); + std::nullopt); break; case ThisWeek: { // Find last Monday QDate startOfWeek = current.addDays(-(current.dayOfWeek()-1)); transactionProxyModel->setDateRange( GUIUtil::StartOfDay(startOfWeek), - TransactionFilterProxy::MAX_DATE); + std::nullopt); } break; case ThisMonth: transactionProxyModel->setDateRange( GUIUtil::StartOfDay(QDate(current.year(), current.month(), 1)), - TransactionFilterProxy::MAX_DATE); + std::nullopt); break; case LastMonth: transactionProxyModel->setDateRange( @@ -295,7 +297,7 @@ void TransactionView::chooseDate(int idx) case ThisYear: transactionProxyModel->setDateRange( GUIUtil::StartOfDay(QDate(current.year(), 1, 1)), - TransactionFilterProxy::MAX_DATE); + std::nullopt); break; case Range: dateRangeWidget->setVisible(true); diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index a1f357e0db..3d8bc0c7c5 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -4,12 +4,18 @@ #include <qt/walletframe.h> +#include <node/ui_interface.h> +#include <psbt.h> +#include <qt/guiutil.h> #include <qt/overviewpage.h> +#include <qt/psbtoperationsdialog.h> #include <qt/walletmodel.h> #include <qt/walletview.h> #include <cassert> +#include <QApplication> +#include <QClipboard> #include <QGroupBox> #include <QHBoxLayout> #include <QLabel> @@ -184,10 +190,40 @@ void WalletFrame::gotoVerifyMessageTab(QString addr) void WalletFrame::gotoLoadPSBT(bool from_clipboard) { - WalletView *walletView = currentWalletView(); - if (walletView) { - walletView->gotoLoadPSBT(from_clipboard); + std::string data; + + if (from_clipboard) { + std::string raw = QApplication::clipboard()->text().toStdString(); + bool invalid; + data = DecodeBase64(raw, &invalid); + if (invalid) { + Q_EMIT message(tr("Error"), tr("Unable to decode PSBT from clipboard (invalid base64)"), CClientUIInterface::MSG_ERROR); + return; + } + } else { + QString filename = GUIUtil::getOpenFileName(this, + tr("Load Transaction Data"), QString(), + tr("Partially Signed Transaction (*.psbt)"), nullptr); + if (filename.isEmpty()) return; + if (GetFileSize(filename.toLocal8Bit().data(), MAX_FILE_SIZE_PSBT) == MAX_FILE_SIZE_PSBT) { + Q_EMIT message(tr("Error"), tr("PSBT file must be smaller than 100 MiB"), CClientUIInterface::MSG_ERROR); + return; + } + std::ifstream in(filename.toLocal8Bit().data(), std::ios::binary); + data = std::string(std::istreambuf_iterator<char>{in}, {}); } + + std::string error; + PartiallySignedTransaction psbtx; + if (!DecodeRawPSBT(psbtx, data, error)) { + Q_EMIT message(tr("Error"), tr("Unable to decode PSBT") + "\n" + QString::fromStdString(error), CClientUIInterface::MSG_ERROR); + return; + } + + PSBTOperationsDialog* dlg = new PSBTOperationsDialog(this, currentWalletModel(), clientModel); + dlg->openWithPSBT(psbtx); + dlg->setAttribute(Qt::WA_DeleteOnClose); + dlg->exec(); } void WalletFrame::encryptWallet() diff --git a/src/qt/walletframe.h b/src/qt/walletframe.h index 4f77bd716f..fe42293abc 100644 --- a/src/qt/walletframe.h +++ b/src/qt/walletframe.h @@ -48,6 +48,7 @@ public: Q_SIGNALS: void createWalletButtonClicked(); + void message(const QString& title, const QString& message, unsigned int style); private: QStackedWidget *walletStack; diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index 3b8cf4c7ed..2326af80b6 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -8,7 +8,6 @@ #include <qt/askpassphrasedialog.h> #include <qt/clientmodel.h> #include <qt/guiutil.h> -#include <qt/psbtoperationsdialog.h> #include <qt/optionsmodel.h> #include <qt/overviewpage.h> #include <qt/platformstyle.h> @@ -21,13 +20,10 @@ #include <interfaces/node.h> #include <node/ui_interface.h> -#include <psbt.h> #include <util/strencodings.h> #include <QAction> #include <QActionGroup> -#include <QApplication> -#include <QClipboard> #include <QFileDialog> #include <QHBoxLayout> #include <QProgressDialog> @@ -205,44 +201,6 @@ void WalletView::gotoVerifyMessageTab(QString addr) signVerifyMessageDialog->setAddress_VM(addr); } -void WalletView::gotoLoadPSBT(bool from_clipboard) -{ - std::string data; - - if (from_clipboard) { - std::string raw = QApplication::clipboard()->text().toStdString(); - bool invalid; - data = DecodeBase64(raw, &invalid); - if (invalid) { - Q_EMIT message(tr("Error"), tr("Unable to decode PSBT from clipboard (invalid base64)"), CClientUIInterface::MSG_ERROR); - return; - } - } else { - QString filename = GUIUtil::getOpenFileName(this, - tr("Load Transaction Data"), QString(), - tr("Partially Signed Transaction (*.psbt)"), nullptr); - if (filename.isEmpty()) return; - if (GetFileSize(filename.toLocal8Bit().data(), MAX_FILE_SIZE_PSBT) == MAX_FILE_SIZE_PSBT) { - Q_EMIT message(tr("Error"), tr("PSBT file must be smaller than 100 MiB"), CClientUIInterface::MSG_ERROR); - return; - } - std::ifstream in(filename.toLocal8Bit().data(), std::ios::binary); - data = std::string(std::istreambuf_iterator<char>{in}, {}); - } - - std::string error; - PartiallySignedTransaction psbtx; - if (!DecodeRawPSBT(psbtx, data, error)) { - Q_EMIT message(tr("Error"), tr("Unable to decode PSBT") + "\n" + QString::fromStdString(error), CClientUIInterface::MSG_ERROR); - return; - } - - PSBTOperationsDialog* dlg = new PSBTOperationsDialog(this, walletModel, clientModel); - dlg->openWithPSBT(psbtx); - dlg->setAttribute(Qt::WA_DeleteOnClose); - dlg->exec(); -} - bool WalletView::handlePaymentRequest(const SendCoinsRecipient& recipient) { return sendCoinsPage->handlePaymentRequest(recipient); diff --git a/src/qt/walletview.h b/src/qt/walletview.h index fedf06b710..5c42a9ffc0 100644 --- a/src/qt/walletview.h +++ b/src/qt/walletview.h @@ -83,8 +83,6 @@ public Q_SLOTS: void gotoSignMessageTab(QString addr = ""); /** Show Sign/Verify Message dialog and switch to verify message tab */ void gotoVerifyMessageTab(QString addr = ""); - /** Load Partially Signed Bitcoin Transaction */ - void gotoLoadPSBT(bool from_clipboard = false); /** Show incoming transaction notification for new transactions. diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 4956ee39e9..909019d796 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -36,6 +36,7 @@ #include <txmempool.h> #include <undo.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/system.h> #include <util/translation.h> #include <validation.h> @@ -1328,7 +1329,7 @@ static RPCHelpMan verifychain() "\nVerifies blockchain database.\n", { {"checklevel", RPCArg::Type::NUM, RPCArg::DefaultHint{strprintf("%d, range=0-4", DEFAULT_CHECKLEVEL)}, - strprintf("How thorough the block verification is:\n - %s", Join(CHECKLEVEL_DOC, "\n- "))}, + strprintf("How thorough the block verification is:\n%s", MakeUnorderedList(CHECKLEVEL_DOC))}, {"nblocks", RPCArg::Type::NUM, RPCArg::DefaultHint{strprintf("%d, 0=all", DEFAULT_CHECKBLOCKS)}, "The number of blocks to check."}, }, RPCResult{ diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 9c8582c7a3..4357ab2bb3 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -142,6 +142,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "importmulti", 0, "requests" }, { "importmulti", 1, "options" }, { "importdescriptors", 0, "requests" }, + { "listdescriptors", 0, "private" }, { "verifychain", 0, "checklevel" }, { "verifychain", 1, "nblocks" }, { "getblockstats", 0, "hash_or_height" }, @@ -186,6 +187,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createwallet", 5, "descriptors"}, { "createwallet", 6, "load_on_startup"}, { "createwallet", 7, "external_signer"}, + { "restorewallet", 2, "load_on_startup"}, { "loadwallet", 1, "load_on_startup"}, { "unloadwallet", 1, "load_on_startup"}, { "getnodeaddresses", 0, "count"}, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index abc9ec3ce3..4d066b8957 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -151,6 +151,8 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::NUM, "n", "The heights of blocks we're currently asking from this peer"}, }}, {RPCResult::Type::BOOL, "addr_relay_enabled", "Whether we participate in address relay with this peer"}, + {RPCResult::Type::NUM, "addr_processed", "The total number of addresses processed, excluding those dropped due to rate limiting"}, + {RPCResult::Type::NUM, "addr_rate_limited", "The total number of addresses dropped due to rate limiting"}, {RPCResult::Type::ARR, "permissions", "Any special permissions that have been granted to this peer", { {RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"}, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index c617b0389c..00e77d89e5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -903,7 +903,7 @@ static RPCHelpMan testmempoolaccept() RPCResult{ RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n" "Returns results for each transaction in the same order they were passed in.\n" - "It is possible for transactions to not be fully validated ('allowed' unset) if another transaction failed.\n", + "Transactions that cannot be fully validated due to failures in other transactions will not contain an 'allowed' result.\n", { {RPCResult::Type::OBJ, "", "", { diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 122a92f084..f21eddf56c 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -18,6 +18,7 @@ #include <univalue.h> #include <util/rbf.h> #include <util/strencodings.h> +#include <util/translation.h> CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime, bool rbf) { @@ -280,22 +281,22 @@ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, int nHashType = ParseSighashString(hashType); // Script verification errors - std::map<int, std::string> input_errors; + std::map<int, bilingual_str> input_errors; bool complete = SignTransaction(mtx, keystore, coins, nHashType, input_errors); SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); } -void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, const std::map<int, std::string>& input_errors, UniValue& result) +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, const std::map<int, bilingual_str>& input_errors, UniValue& result) { // Make errors UniValue UniValue vErrors(UniValue::VARR); for (const auto& err_pair : input_errors) { - if (err_pair.second == "Missing amount") { + if (err_pair.second.original == "Missing amount") { // This particular error needs to be an exception for some reason throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Missing amount for %s", coins.at(mtx.vin.at(err_pair.first).prevout).out.ToString())); } - TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second); + TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second.original); } result.pushKV("hex", EncodeHexTx(CTransaction(mtx))); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index ce7d5834fa..d2e116f7ee 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -8,6 +8,7 @@ #include <map> #include <string> +struct bilingual_str; class FillableSigningProvider; class UniValue; struct CMutableTransaction; @@ -25,7 +26,7 @@ class SigningProvider; * @param result JSON object where signed transaction results accumulate */ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result); -void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, const std::map<int, std::string>& input_errors, UniValue& result); +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, const std::map<int, bilingual_str>& input_errors, UniValue& result); /** * Parse a prevtxs UniValue array and get the map of coins from it diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 7864e690d8..2faf7e5048 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -11,6 +11,7 @@ #include <script/signingprovider.h> #include <script/standard.h> #include <uint256.h> +#include <util/translation.h> #include <util/vector.h> typedef std::vector<unsigned char> valtype; @@ -629,7 +630,7 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) return false; } -bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, int nHashType, std::map<int, std::string>& input_errors) +bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, int nHashType, std::map<int, bilingual_str>& input_errors) { bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE); @@ -661,7 +662,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, CTxIn& txin = mtx.vin[i]; auto coin = coins.find(txin.prevout); if (coin == coins.end() || coin->second.IsSpent()) { - input_errors[i] = "Input not found or already spent"; + input_errors[i] = _("Input not found or already spent"); continue; } const CScript& prevPubKey = coin->second.out.scriptPubKey; @@ -677,7 +678,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, // amount must be specified for valid segwit signature if (amount == MAX_MONEY && !txin.scriptWitness.IsNull()) { - input_errors[i] = "Missing amount"; + input_errors[i] = _("Missing amount"); continue; } @@ -685,12 +686,12 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) { if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) { // Unable to sign input and verification failed (possible attempt to partially sign). - input_errors[i] = "Unable to sign input, invalid stack size (possibly missing key)"; + input_errors[i] = Untranslated("Unable to sign input, invalid stack size (possibly missing key)"); } else if (serror == SCRIPT_ERR_SIG_NULLFAIL) { // Verification failed (possibly due to insufficient signatures). - input_errors[i] = "CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)"; + input_errors[i] = Untranslated("CHECK(MULTI)SIG failing with non-zero signature (possibly need more signatures)"); } else { - input_errors[i] = ScriptErrorString(serror); + input_errors[i] = Untranslated(ScriptErrorString(serror)); } } else { // If this input succeeds, make sure there is no error set for it diff --git a/src/script/sign.h b/src/script/sign.h index b4e7318892..b8fcac2e3c 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -21,6 +21,7 @@ class CScript; class CTransaction; class SigningProvider; +struct bilingual_str; struct CMutableTransaction; /** Interface for signature creators. */ @@ -178,6 +179,6 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script); bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); /** Sign the CMutableTransaction */ -bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* provider, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors); +bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* provider, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors); #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 79c7102c4f..3c64461605 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -21,24 +21,13 @@ private: bool deterministic; public: explicit CAddrManTest(bool makeDeterministic = true, - std::vector<bool> asmap = std::vector<bool>()) + std::vector<bool> asmap = std::vector<bool>()) + : CAddrMan(makeDeterministic, /* consistency_check_ratio */ 100) { - if (makeDeterministic) { - // Set addrman addr placement to be deterministic. - MakeDeterministic(); - } deterministic = makeDeterministic; m_asmap = asmap; } - //! Ensure that bucket placement is always the same for testing purposes. - void MakeDeterministic() - { - LOCK(cs); - nKey.SetNull(); - insecure_rand = FastRandomContext(true); - } - CAddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) { LOCK(cs); @@ -89,7 +78,7 @@ public: CAddrMan::Clear(); if (deterministic) { LOCK(cs); - nKey.SetNull(); + nKey = uint256{1}; insecure_rand = FastRandomContext(true); } } @@ -267,24 +256,27 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) CNetAddr source = ResolveIP("252.2.2.2"); - BOOST_CHECK_EQUAL(addrman.size(), 0U); + uint32_t num_addrs{0}; + + BOOST_CHECK_EQUAL(addrman.size(), num_addrs); - for (unsigned int i = 1; i < 18; i++) { - CService addr = ResolveService("250.1.1." + ToString(i)); + while (num_addrs < 22) { // Magic number! 250.1.1.1 - 250.1.1.22 do not collide with deterministic key = 1 + CService addr = ResolveService("250.1.1." + ToString(++num_addrs)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); //Test: No collision in new table yet. - BOOST_CHECK_EQUAL(addrman.size(), i); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs); } //Test: new table collision! - CService addr1 = ResolveService("250.1.1.18"); + CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs)); + uint32_t collisions{1}; BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 17U); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); - CService addr2 = ResolveService("250.1.1.19"); + CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 18U); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); } BOOST_AUTO_TEST_CASE(addrman_tried_collisions) @@ -293,25 +285,28 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) CNetAddr source = ResolveIP("252.2.2.2"); - BOOST_CHECK_EQUAL(addrman.size(), 0U); + uint32_t num_addrs{0}; + + BOOST_CHECK_EQUAL(addrman.size(), num_addrs); - for (unsigned int i = 1; i < 80; i++) { - CService addr = ResolveService("250.1.1." + ToString(i)); + while (num_addrs < 64) { // Magic number! 250.1.1.1 - 250.1.1.64 do not collide with deterministic key = 1 + CService addr = ResolveService("250.1.1." + ToString(++num_addrs)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(CAddress(addr, NODE_NONE)); //Test: No collision in tried table yet. - BOOST_CHECK_EQUAL(addrman.size(), i); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs); } //Test: tried table collision! - CService addr1 = ResolveService("250.1.1.80"); + CService addr1 = ResolveService("250.1.1." + ToString(++num_addrs)); + uint32_t collisions{1}; BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 79U); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); - CService addr2 = ResolveService("250.1.1.81"); + CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 80U); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); } BOOST_AUTO_TEST_CASE(addrman_find) @@ -861,9 +856,9 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) { CAddrManTest addrman; - // Add twenty two addresses. + // Add 35 addresses. CNetAddr source = ResolveIP("252.2.2.2"); - for (unsigned int i = 1; i < 23; i++) { + for (unsigned int i = 1; i < 36; i++) { CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); @@ -873,20 +868,20 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); } - // Collision between 23 and 19. - CService addr23 = ResolveService("250.1.1.23"); - BOOST_CHECK(addrman.Add(CAddress(addr23, NODE_NONE), source)); - addrman.Good(addr23); + // Collision between 36 and 19. + CService addr36 = ResolveService("250.1.1.36"); + BOOST_CHECK(addrman.Add(CAddress(addr36, NODE_NONE), source)); + addrman.Good(addr36); - BOOST_CHECK(addrman.size() == 23); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.19:0"); + BOOST_CHECK(addrman.size() == 36); + BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.19:0"); - // 23 should be discarded and 19 not evicted. + // 36 should be discarded and 19 not evicted. addrman.ResolveCollisions(); BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); // Lets create two collisions. - for (unsigned int i = 24; i < 33; i++) { + for (unsigned int i = 37; i < 59; i++) { CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); @@ -896,17 +891,17 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) } // Cause a collision. - CService addr33 = ResolveService("250.1.1.33"); - BOOST_CHECK(addrman.Add(CAddress(addr33, NODE_NONE), source)); - addrman.Good(addr33); - BOOST_CHECK(addrman.size() == 33); + CService addr59 = ResolveService("250.1.1.59"); + BOOST_CHECK(addrman.Add(CAddress(addr59, NODE_NONE), source)); + addrman.Good(addr59); + BOOST_CHECK(addrman.size() == 59); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.27:0"); + BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.10:0"); // Cause a second collision. - BOOST_CHECK(!addrman.Add(CAddress(addr23, NODE_NONE), source)); - addrman.Good(addr23); - BOOST_CHECK(addrman.size() == 33); + BOOST_CHECK(!addrman.Add(CAddress(addr36, NODE_NONE), source)); + addrman.Good(addr36); + BOOST_CHECK(addrman.size() == 59); BOOST_CHECK(addrman.SelectTriedCollision().ToString() != "[::]:0"); addrman.ResolveCollisions(); @@ -922,9 +917,9 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) // Empty addrman should return blank addrman info. BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); - // Add twenty two addresses. + // Add 35 addresses CNetAddr source = ResolveIP("252.2.2.2"); - for (unsigned int i = 1; i < 23; i++) { + for (unsigned int i = 1; i < 36; i++) { CService addr = ResolveService("250.1.1."+ToString(i)); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); @@ -934,34 +929,34 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); } - // Collision between 23 and 19. - CService addr = ResolveService("250.1.1.23"); + // Collision between 36 and 19. + CService addr = ResolveService("250.1.1.36"); BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); - BOOST_CHECK(addrman.size() == 23); + BOOST_CHECK_EQUAL(addrman.size(), 36); CAddrInfo info = addrman.SelectTriedCollision(); - BOOST_CHECK(info.ToString() == "250.1.1.19:0"); + BOOST_CHECK_EQUAL(info.ToString(), "250.1.1.19:0"); // Ensure test of address fails, so that it is evicted. addrman.SimConnFail(info); - // Should swap 23 for 19. + // Should swap 36 for 19. addrman.ResolveCollisions(); BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); - // If 23 was swapped for 19, then this should cause no collisions. + // If 36 was swapped for 19, then this should cause no collisions. BOOST_CHECK(!addrman.Add(CAddress(addr, NODE_NONE), source)); addrman.Good(addr); BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); - // If we insert 19 is should collide with 23. + // If we insert 19 it should collide with 36 CService addr19 = ResolveService("250.1.1.19"); BOOST_CHECK(!addrman.Add(CAddress(addr19, NODE_NONE), source)); addrman.Good(addr19); - BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.23:0"); + BOOST_CHECK_EQUAL(addrman.SelectTriedCollision().ToString(), "250.1.1.36:0"); addrman.ResolveCollisions(); BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 4c29a8ee53..60fba5730a 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -29,7 +29,8 @@ public: FuzzedDataProvider& m_fuzzed_data_provider; explicit CAddrManDeterministic(FuzzedDataProvider& fuzzed_data_provider) - : m_fuzzed_data_provider(fuzzed_data_provider) + : CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 0) + , m_fuzzed_data_provider(fuzzed_data_provider) { WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); if (fuzzed_data_provider.ConsumeBool()) { diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index de211f601f..46a9f623ac 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -108,9 +108,7 @@ FUZZ_TARGET_INIT(banman, initialize_banman) BanMan ban_man_read{banlist_file, /* client_interface */ nullptr, /* default_ban_time */ 0}; banmap_t banmap_read; ban_man_read.GetBanned(banmap_read); - // Assert temporarily disabled to allow the remainder of the fuzz test to run while a - // fix is being worked on. See https://github.com/bitcoin/bitcoin/pull/22517 - (void)(banmap == banmap_read); + assert(banmap == banmap_read); } } fs::remove(banlist_file.string() + ".json"); diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index bbec5943af..0e323ddc20 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -25,7 +25,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - CAddrMan addrman; + CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0); CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), addrman, fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp index 473caec6ff..53400082ab 100644 --- a/src/test/fuzz/data_stream.cpp +++ b/src/test/fuzz/data_stream.cpp @@ -21,6 +21,6 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider); - CAddrMan addr_man; + CAddrMan addr_man(/* deterministic */ false, /* consistency_check_ratio */ 0); CAddrDB::Read(addr_man, data_stream); } diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index d5b56cb7cd..49503e8dc6 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -188,7 +188,7 @@ FUZZ_TARGET_DESERIALIZE(blockmerkleroot, { BlockMerkleRoot(block, &mutated); }) FUZZ_TARGET_DESERIALIZE(addrman_deserialize, { - CAddrMan am; + CAddrMan am(/* deterministic */ false, /* consistency_check_ratio */ 0); DeserializeFromFuzzingInput(buffer, am); }) FUZZ_TARGET_DESERIALIZE(blockheader_deserialize, { diff --git a/src/test/fuzz/script_sign.cpp b/src/test/fuzz/script_sign.cpp index fe850a6959..684324c36e 100644 --- a/src/test/fuzz/script_sign.cpp +++ b/src/test/fuzz/script_sign.cpp @@ -13,6 +13,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <util/translation.h> #include <cassert> #include <cstdint> @@ -135,7 +136,7 @@ FUZZ_TARGET_INIT(script_sign, initialize_script_sign) } coins[*outpoint] = *coin; } - std::map<int, std::string> input_errors; + std::map<int, bilingual_str> input_errors; (void)SignTransaction(sign_transaction_tx_to, &provider, coins, fuzzed_data_provider.ConsumeIntegral<int>(), input_errors); } } diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index acbbf357d2..1915f9c7d5 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -34,13 +34,9 @@ class CAddrManSerializationMock : public CAddrMan public: virtual void Serialize(CDataStream& s) const = 0; - //! Ensure that bucket placement is always the same for testing purposes. - void MakeDeterministic() - { - LOCK(cs); - nKey.SetNull(); - insecure_rand = FastRandomContext(true); - } + CAddrManSerializationMock() + : CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 100) + {} }; class CAddrManUncorrupted : public CAddrManSerializationMock @@ -105,7 +101,6 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_AUTO_TEST_CASE(caddrdb_read) { CAddrManUncorrupted addrmanUncorrupted; - addrmanUncorrupted.MakeDeterministic(); CService addr1, addr2, addr3; BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); @@ -124,7 +119,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) // Test that the de-serialization does not throw an exception. CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); bool exceptionThrown = false; - CAddrMan addrman1; + CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman1.size() == 0); try { @@ -141,7 +136,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) // Test that CAddrDB::Read creates an addrman with the correct number of addrs. CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); - CAddrMan addrman2; + CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); BOOST_CHECK(CAddrDB::Read(addrman2, ssPeers2)); BOOST_CHECK(addrman2.size() == 3); @@ -151,12 +146,11 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) { CAddrManCorrupted addrmanCorrupted; - addrmanCorrupted.MakeDeterministic(); // Test that the de-serialization of corrupted addrman throws an exception. CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); bool exceptionThrown = false; - CAddrMan addrman1; + CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman1.size() == 0); try { unsigned char pchMsgTmp[4]; @@ -172,7 +166,7 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) // Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails. CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); - CAddrMan addrman2; + CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2)); BOOST_CHECK(addrman2.size() == 0); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2d044af184..c9bb863a7b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -193,7 +193,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString())); } - m_node.addrman = std::make_unique<CAddrMan>(); + m_node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ 0); m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, @@ -292,7 +292,7 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio input_coins.insert({outpoint_to_spend, utxo_to_spend}); // - Default signature hashing type int nHashType = SIGHASH_ALL; - std::map<int, std::string> input_errors; + std::map<int, bilingual_str> input_errors; assert(SignTransaction(mempool_txn, &keystore, input_coins, nHashType, input_errors)); // If submit=true, add transaction to the mempool. diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index fd6012e9fe..061659818f 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -8,6 +8,7 @@ #include <outputtype.h> #include <script/standard.h> #ifdef ENABLE_WALLET +#include <util/translation.h> #include <wallet/wallet.h> #endif @@ -18,7 +19,7 @@ std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; CTxDestination dest; - std::string error; + bilingual_str error; if (!w.GetNewDestination(output_type, "", dest, error)) assert(false); return EncodeDestination(dest); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index c5a4bbf1b0..d5a888ac67 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -151,33 +151,17 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes } } -bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) const +bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, + size_t entry_count, + setEntries& setAncestors, + CTxMemPoolEntry::Parents& staged_ancestors, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const { - CTxMemPoolEntry::Parents staged_ancestors; - const CTransaction &tx = entry.GetTx(); - - if (fSearchForParents) { - // Get parents of this transaction that are in the mempool - // GetMemPoolParents() is only valid for entries in the mempool, so we - // iterate mapTx to find parents. - for (unsigned int i = 0; i < tx.vin.size(); i++) { - std::optional<txiter> piter = GetIter(tx.vin[i].prevout.hash); - if (piter) { - staged_ancestors.insert(**piter); - if (staged_ancestors.size() + 1 > limitAncestorCount) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); - return false; - } - } - } - } else { - // If we're not searching for parents, we require this to be an - // entry in the mempool already. - txiter it = mapTx.iterator_to(entry); - staged_ancestors = it->GetMemPoolParentsConst(); - } - - size_t totalSizeWithAncestors = entry.GetTxSize(); + size_t totalSizeWithAncestors = entry_size; while (!staged_ancestors.empty()) { const CTxMemPoolEntry& stage = staged_ancestors.begin()->get(); @@ -187,10 +171,10 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr staged_ancestors.erase(stage); totalSizeWithAncestors += stageit->GetTxSize(); - if (stageit->GetSizeWithDescendants() + entry.GetTxSize() > limitDescendantSize) { + if (stageit->GetSizeWithDescendants() + entry_size > limitDescendantSize) { errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantSize); return false; - } else if (stageit->GetCountWithDescendants() + 1 > limitDescendantCount) { + } else if (stageit->GetCountWithDescendants() + entry_count > limitDescendantCount) { errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantCount); return false; } else if (totalSizeWithAncestors > limitAncestorSize) { @@ -206,7 +190,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr if (setAncestors.count(parent_it) == 0) { staged_ancestors.insert(parent); } - if (staged_ancestors.size() + setAncestors.size() + 1 > limitAncestorCount) { + if (staged_ancestors.size() + setAncestors.size() + entry_count > limitAncestorCount) { errString = strprintf("too many unconfirmed ancestors [limit: %u]", limitAncestorCount); return false; } @@ -216,6 +200,80 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr return true; } +bool CTxMemPool::CheckPackageLimits(const Package& package, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const +{ + CTxMemPoolEntry::Parents staged_ancestors; + size_t total_size = 0; + for (const auto& tx : package) { + total_size += GetVirtualTransactionSize(*tx); + for (const auto& input : tx->vin) { + std::optional<txiter> piter = GetIter(input.prevout.hash); + if (piter) { + staged_ancestors.insert(**piter); + if (staged_ancestors.size() + package.size() > limitAncestorCount) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + return false; + } + } + } + } + // When multiple transactions are passed in, the ancestors and descendants of all transactions + // considered together must be within limits even if they are not interdependent. This may be + // stricter than the limits for each individual transaction. + setEntries setAncestors; + const auto ret = CalculateAncestorsAndCheckLimits(total_size, package.size(), + setAncestors, staged_ancestors, + limitAncestorCount, limitAncestorSize, + limitDescendantCount, limitDescendantSize, errString); + // It's possible to overestimate the ancestor/descendant totals. + if (!ret) errString.insert(0, "possibly "); + return ret; +} + +bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, + setEntries &setAncestors, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString, + bool fSearchForParents /* = true */) const +{ + CTxMemPoolEntry::Parents staged_ancestors; + const CTransaction &tx = entry.GetTx(); + + if (fSearchForParents) { + // Get parents of this transaction that are in the mempool + // GetMemPoolParents() is only valid for entries in the mempool, so we + // iterate mapTx to find parents. + for (unsigned int i = 0; i < tx.vin.size(); i++) { + std::optional<txiter> piter = GetIter(tx.vin[i].prevout.hash); + if (piter) { + staged_ancestors.insert(**piter); + if (staged_ancestors.size() + 1 > limitAncestorCount) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + return false; + } + } + } + } else { + // If we're not searching for parents, we require this to already be an + // entry in the mempool and use the entry's cached parents. + txiter it = mapTx.iterator_to(entry); + staged_ancestors = it->GetMemPoolParentsConst(); + } + + return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /* entry_count */ 1, + setAncestors, staged_ancestors, + limitAncestorCount, limitAncestorSize, + limitDescendantCount, limitDescendantSize, errString); +} + void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) { CTxMemPoolEntry::Parents parents = it->GetMemPoolParents(); diff --git a/src/txmempool.h b/src/txmempool.h index ae4b16d377..0a84a6e6b1 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -18,6 +18,7 @@ #include <coins.h> #include <indirectmap.h> #include <policy/feerate.h> +#include <policy/packages.h> #include <primitives/transaction.h> #include <random.h> #include <sync.h> @@ -585,6 +586,25 @@ private: */ std::set<uint256> m_unbroadcast_txids GUARDED_BY(cs); + + /** + * Helper function to calculate all in-mempool ancestors of staged_ancestors and apply ancestor + * and descendant limits (including staged_ancestors thsemselves, entry_size and entry_count). + * param@[in] entry_size Virtual size to include in the limits. + * param@[in] entry_count How many entries to include in the limits. + * param@[in] staged_ancestors Should contain entries in the mempool. + * param@[out] setAncestors Will be populated with all mempool ancestors. + */ + bool CalculateAncestorsAndCheckLimits(size_t entry_size, + size_t entry_count, + setEntries& setAncestors, + CTxMemPoolEntry::Parents &staged_ancestors, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + public: indirectmap<COutPoint, const CTransaction*> mapNextTx GUARDED_BY(cs); std::map<uint256, CAmount> mapDeltas GUARDED_BY(cs); @@ -681,6 +701,28 @@ public: */ bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, setEntries& setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string& errString, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and + * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and + * descendant count of all entries if the package were to be added to the mempool. The limits + * are applied to the union of all package transactions. For example, if the package has 3 + * transactions and limitAncestorCount = 25, the union of all 3 sets of ancestors (including the + * transactions themselves) must be <= 22. + * @param[in] package Transaction package being evaluated for acceptance + * to mempool. The transactions need not be direct + * ancestors/descendants of each other. + * @param[in] limitAncestorCount Max number of txns including ancestors. + * @param[in] limitAncestorSize Max virtual size including ancestors. + * @param[in] limitDescendantCount Max number of txns including descendants. + * @param[in] limitDescendantSize Max virtual size including descendants. + * @param[out] errString Populated with error reason if a limit is hit. + */ + bool CheckPackageLimits(const Package& package, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything * already in it. */ diff --git a/src/util/string.h b/src/util/string.h index b26facc502..5617e4acc1 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -65,6 +65,14 @@ inline std::string Join(const std::vector<std::string>& list, const std::string& } /** + * Create an unordered multi-line list of items. + */ +inline std::string MakeUnorderedList(const std::vector<std::string>& items) +{ + return Join(items, "\n", [](const std::string& item) { return "- " + item; }); +} + +/** * Check if a string does not contain any embedded NUL (\0) characters */ [[nodiscard]] inline bool ValidAsCString(const std::string& str) noexcept diff --git a/src/util/system.cpp b/src/util/system.cpp index 258ba2f235..30d4103819 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -502,11 +502,11 @@ bool ArgsManager::InitSettings(std::string& error) std::vector<std::string> errors; if (!ReadSettingsFile(&errors)) { - error = strprintf("Failed loading settings file:\n- %s\n", Join(errors, "\n- ")); + error = strprintf("Failed loading settings file:\n%s\n", MakeUnorderedList(errors)); return false; } if (!WriteSettingsFile(&errors)) { - error = strprintf("Failed saving settings file:\n- %s\n", Join(errors, "\n- ")); + error = strprintf("Failed saving settings file:\n%s\n", MakeUnorderedList(errors)); return false; } return true; diff --git a/src/util/translation.h b/src/util/translation.h index 99899ef3c2..62388b568c 100644 --- a/src/util/translation.h +++ b/src/util/translation.h @@ -28,6 +28,12 @@ struct bilingual_str { { return original.empty(); } + + void clear() + { + original.clear(); + translated.clear(); + } }; inline bilingual_str operator+(bilingual_str lhs, const bilingual_str& rhs) diff --git a/src/validation.cpp b/src/validation.cpp index 1b3d00bc6d..ec457da5cc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1079,6 +1079,19 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: m_viewmempool.PackageAddTransaction(ws.m_ptx); } + // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction, + // because it's unnecessary. Also, CPFP carve out can increase the limit for individual + // transactions, but this exemption is not extended to packages in CheckPackageLimits(). + std::string err_string; + if (txns.size() > 1 && + !m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, + m_limit_descendant_size, err_string)) { + // All transactions must have individually passed mempool ancestor and descendant limits + // inside of PreChecks(), so this is separate from an individual transaction error. + package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + return PackageMempoolAcceptResult(package_state, std::move(results)); + } + for (Workspace& ws : workspaces) { PrecomputedTransactionData txdata; if (!PolicyScriptChecks(args, ws, txdata)) { diff --git a/src/validation.h b/src/validation.h index 9d8d7c06a9..b80fa9d328 100644 --- a/src/validation.h +++ b/src/validation.h @@ -199,7 +199,8 @@ struct PackageMempoolAcceptResult /** * Map from wtxid to finished MempoolAcceptResults. The client is responsible * for keeping track of the transaction objects themselves. If a result is not - * present, it means validation was unfinished for that transaction. + * present, it means validation was unfinished for that transaction. If there + * was a package-wide error (see result in m_state), m_tx_results will be empty. */ std::map<const uint256, const MempoolAcceptResult> m_tx_results; @@ -227,7 +228,8 @@ MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPoo * @param[in] txns Group of transactions which may be independent or contain * parent-child dependencies. The transactions must not conflict * with each other, i.e., must not spend the same inputs. If any -* dependencies exist, parents must appear before children. +* dependencies exist, parents must appear anywhere in the list +* before their children. * @returns a PackageMempoolAcceptResult which includes a MempoolAcceptResult for each transaction. * If a transaction fails, validation will exit early and some results may be missing. */ diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 6d502e1df1..25b1ee07e4 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -195,7 +195,7 @@ static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const //the selection random. if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) { - nTotal += groups[i].m_value; + nTotal += groups[i].GetSelectionAmount(); vfIncluded[i] = true; if (nTotal >= nTargetValue) { @@ -205,7 +205,7 @@ static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const nBest = nTotal; vfBest = vfIncluded; } - nTotal -= groups[i].m_value; + nTotal -= groups[i].GetSelectionAmount(); vfIncluded[i] = false; } } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index e33adf94c9..2c891c3c1e 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -16,6 +16,7 @@ #include <uint256.h> #include <util/check.h> #include <util/system.h> +#include <util/translation.h> #include <util/ui_change_type.h> #include <wallet/context.h> #include <wallet/feebumper.h> @@ -130,7 +131,7 @@ public: bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) override { LOCK(m_wallet->cs_wallet); - std::string error; + bilingual_str error; return m_wallet->GetNewDestination(type, label, dest, error); } bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ac60504419..cccaff9d65 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1760,8 +1760,10 @@ RPCHelpMan listdescriptors() { return RPCHelpMan{ "listdescriptors", - "\nList descriptors imported into a descriptor-enabled wallet.", - {}, + "\nList descriptors imported into a descriptor-enabled wallet.\n", + { + {"private", RPCArg::Type::BOOL, RPCArg::Default{false}, "Show private descriptors."} + }, RPCResult{RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"}, {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects", @@ -1781,6 +1783,7 @@ RPCHelpMan listdescriptors() }}, RPCExamples{ HelpExampleCli("listdescriptors", "") + HelpExampleRpc("listdescriptors", "") + + HelpExampleCli("listdescriptors", "true") + HelpExampleRpc("listdescriptors", "true") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -1791,6 +1794,11 @@ RPCHelpMan listdescriptors() throw JSONRPCError(RPC_WALLET_ERROR, "listdescriptors is not available for non-descriptor wallets"); } + const bool priv = !request.params[0].isNull() && request.params[0].get_bool(); + if (priv) { + EnsureWalletIsUnlocked(*wallet); + } + LOCK(wallet->cs_wallet); UniValue descriptors(UniValue::VARR); @@ -1804,8 +1812,9 @@ RPCHelpMan listdescriptors() LOCK(desc_spk_man->cs_desc_man); const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor(); std::string descriptor; - if (!desc_spk_man->GetDescriptorString(descriptor)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Can't get normalized descriptor string."); + + if (!desc_spk_man->GetDescriptorString(descriptor, priv)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string."); } spk.pushKV("desc", descriptor); spk.pushKV("timestamp", wallet_descriptor.creation_time); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 43b67076cd..2a5b547858 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -276,9 +276,9 @@ static RPCHelpMan getnewaddress() } CTxDestination dest; - std::string error; + bilingual_str error; if (!pwallet->GetNewDestination(output_type, label, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); } return EncodeDestination(dest); @@ -324,9 +324,9 @@ static RPCHelpMan getrawchangeaddress() } CTxDestination dest; - std::string error; + bilingual_str error; if (!pwallet->GetNewChangeDestination(output_type, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); } return EncodeDestination(dest); }, @@ -2572,6 +2572,37 @@ static RPCHelpMan listwallets() }; } +static std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>> LoadWalletHelper(WalletContext& context, UniValue load_on_start_param, const std::string wallet_name) +{ + DatabaseOptions options; + DatabaseStatus status; + options.require_existing = true; + bilingual_str error; + std::vector<bilingual_str> warnings; + std::optional<bool> load_on_start = load_on_start_param.isNull() ? std::nullopt : std::optional<bool>(load_on_start_param.get_bool()); + std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, wallet_name, load_on_start, options, status, error, warnings); + + if (!wallet) { + // Map bad format to not found, since bad format is returned when the + // wallet directory exists, but doesn't contain a data file. + RPCErrorCode code = RPC_WALLET_ERROR; + switch (status) { + case DatabaseStatus::FAILED_NOT_FOUND: + case DatabaseStatus::FAILED_BAD_FORMAT: + code = RPC_WALLET_NOT_FOUND; + break; + case DatabaseStatus::FAILED_ALREADY_LOADED: + code = RPC_WALLET_ALREADY_LOADED; + break; + default: // RPC_WALLET_ERROR is returned for all other cases. + break; + } + throw JSONRPCError(code, error.original); + } + + return { wallet, warnings }; +} + static RPCHelpMan loadwallet() { return RPCHelpMan{"loadwallet", @@ -2598,30 +2629,7 @@ static RPCHelpMan loadwallet() WalletContext& context = EnsureWalletContext(request.context); const std::string name(request.params[0].get_str()); - DatabaseOptions options; - DatabaseStatus status; - options.require_existing = true; - bilingual_str error; - std::vector<bilingual_str> warnings; - std::optional<bool> load_on_start = request.params[1].isNull() ? std::nullopt : std::optional<bool>(request.params[1].get_bool()); - std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, name, load_on_start, options, status, error, warnings); - if (!wallet) { - // Map bad format to not found, since bad format is returned when the - // wallet directory exists, but doesn't contain a data file. - RPCErrorCode code = RPC_WALLET_ERROR; - switch (status) { - case DatabaseStatus::FAILED_NOT_FOUND: - case DatabaseStatus::FAILED_BAD_FORMAT: - code = RPC_WALLET_NOT_FOUND; - break; - case DatabaseStatus::FAILED_ALREADY_LOADED: - code = RPC_WALLET_ALREADY_LOADED; - break; - default: // RPC_WALLET_ERROR is returned for all other cases. - break; - } - throw JSONRPCError(code, error.original); - } + auto [wallet, warnings] = LoadWalletHelper(context, request.params[1], name); UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); @@ -2795,6 +2803,68 @@ static RPCHelpMan createwallet() }; } +static RPCHelpMan restorewallet() +{ + return RPCHelpMan{ + "restorewallet", + "\nRestore and loads a wallet from backup.\n", + { + {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name that will be applied to the restored wallet"}, + {"backup_file", RPCArg::Type::STR, RPCArg::Optional::NO, "The backup file that will be used to restore the wallet."}, + {"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, "name", "The wallet name if restored successfully."}, + {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + } + }, + RPCExamples{ + HelpExampleCli("restorewallet", "\"testwallet\" \"home\\backups\\backup-file.bak\"") + + HelpExampleRpc("restorewallet", "\"testwallet\" \"home\\backups\\backup-file.bak\"") + + HelpExampleCliNamed("restorewallet", {{"wallet_name", "testwallet"}, {"backup_file", "home\\backups\\backup-file.bak\""}, {"load_on_startup", true}}) + + HelpExampleRpcNamed("restorewallet", {{"wallet_name", "testwallet"}, {"backup_file", "home\\backups\\backup-file.bak\""}, {"load_on_startup", true}}) + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + + WalletContext& context = EnsureWalletContext(request.context); + + std::string backup_file = request.params[1].get_str(); + + if (!fs::exists(backup_file)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Backup file does not exist"); + } + + std::string wallet_name = request.params[0].get_str(); + + const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), wallet_name); + + if (fs::exists(wallet_path)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Wallet name already exists."); + } + + if (!TryCreateDirectories(wallet_path)) { + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Failed to create database path '%s'. Database already exists.", wallet_path.string())); + } + + auto wallet_file = wallet_path / "wallet.dat"; + + fs::copy_file(backup_file, wallet_file, fs::copy_option::fail_if_exists); + + auto [wallet, warnings] = LoadWalletHelper(context, request.params[2], wallet_name); + + UniValue obj(UniValue::VOBJ); + obj.pushKV("name", wallet->GetName()); + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + + return obj; + +}, + }; +} + static RPCHelpMan unloadwallet() { return RPCHelpMan{"unloadwallet", @@ -3392,7 +3462,7 @@ RPCHelpMan signrawtransactionwithwallet() int nHashType = ParseSighashString(request.params[2]); // Script verification errors - std::map<int, std::string> input_errors; + std::map<int, bilingual_str> input_errors; bool complete = pwallet->SignTransaction(mtx, coins, nHashType, input_errors); UniValue result(UniValue::VOBJ); @@ -3875,7 +3945,7 @@ RPCHelpMan getaddressinfo() DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(pwallet->GetScriptPubKeyMan(scriptPubKey)); if (desc_spk_man) { std::string desc_str; - if (desc_spk_man->GetDescriptorString(desc_str)) { + if (desc_spk_man->GetDescriptorString(desc_str, /* priv */ false)) { ret.pushKV("parent_desc", desc_str); } } @@ -4639,6 +4709,7 @@ static const CRPCCommand commands[] = { "wallet", &bumpfee, }, { "wallet", &psbtbumpfee, }, { "wallet", &createwallet, }, + { "wallet", &restorewallet, }, { "wallet", &dumpprivkey, }, { "wallet", &dumpwallet, }, { "wallet", &encryptwallet, }, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 73433214f1..fe41f9b8cc 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -20,10 +20,10 @@ //! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details. const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; -bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) +bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated; + error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types"); return false; } assert(type != OutputType::BECH32M); @@ -34,7 +34,7 @@ bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestinat // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, type)) { - error = _("Error: Keypool ran out, please call keypoolrefill first").translated; + error = _("Error: Keypool ran out, please call keypoolrefill first"); return false; } LearnRelatedScripts(new_key, type); @@ -295,22 +295,22 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat return true; } -bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) +bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types").translated; + error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types"); return false; } assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); if (!CanGetAddresses(internal)) { - error = _("Error: Keypool ran out, please call keypoolrefill first").translated; + error = _("Error: Keypool ran out, please call keypoolrefill first"); return false; } if (!ReserveKeyFromKeyPool(index, keypool, internal)) { - error = _("Error: Keypool ran out, please call keypoolrefill first").translated; + error = _("Error: Keypool ran out, please call keypoolrefill first"); return false; } address = GetDestinationForKey(keypool.vchPubKey, type); @@ -592,7 +592,7 @@ bool LegacyScriptPubKeyMan::CanProvide(const CScript& script, SignatureData& sig } } -bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const +bool LegacyScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const { return ::SignTransaction(tx, this, coins, sighash, input_errors); } @@ -1613,11 +1613,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const return set_address; } -bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) +bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later if (!CanGetAddresses()) { - error = "No addresses available"; + error = _("No addresses available"); return false; } { @@ -1636,12 +1636,12 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest std::vector<CScript> scripts_temp; if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys - error = "Error: Keypool ran out, please call keypoolrefill first"; + error = _("Error: Keypool ran out, please call keypoolrefill first"); return false; } if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) { // We can't generate anymore keys - error = "Error: Keypool ran out, please call keypoolrefill first"; + error = _("Error: Keypool ran out, please call keypoolrefill first"); return false; } @@ -1721,7 +1721,7 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle return true; } -bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) +bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { LOCK(cs_desc_man); bool result = GetNewDestination(type, address, error); @@ -2046,7 +2046,7 @@ bool DescriptorScriptPubKeyMan::CanProvide(const CScript& script, SignatureData& return IsMine(script); } -bool DescriptorScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const +bool DescriptorScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const { std::unique_ptr<FlatSigningProvider> keys = std::make_unique<FlatSigningProvider>(); for (const auto& coin_pair : coins) { @@ -2258,13 +2258,20 @@ const std::vector<CScript> DescriptorScriptPubKeyMan::GetScriptPubKeys() const return script_pub_keys; } -bool DescriptorScriptPubKeyMan::GetDescriptorString(std::string& out) const +bool DescriptorScriptPubKeyMan::GetDescriptorString(std::string& out, const bool priv) const { LOCK(cs_desc_man); FlatSigningProvider provider; provider.keys = GetKeys(); + if (priv) { + // For the private version, always return the master key to avoid + // exposing child private keys. The risk implications of exposing child + // private keys together with the parent xpub may be non-obvious for users. + return m_wallet_descriptor.descriptor->ToPrivateString(provider, out); + } + return m_wallet_descriptor.descriptor->ToNormalizedString(provider, out, &m_wallet_descriptor.cache); } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index e329e0cf8f..93e1886102 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -174,14 +174,14 @@ protected: public: explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {} virtual ~ScriptPubKeyMan() {}; - virtual bool GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) { return false; } + virtual bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { return false; } virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } - virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) { return false; } + virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { return false; } virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} @@ -230,7 +230,7 @@ public: virtual bool CanProvide(const CScript& script, SignatureData& sigdata) { return false; } /** Creates new signatures and adds them to the transaction. Returns whether all inputs were signed */ - virtual bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const { return false; } + virtual bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const { return false; } /** Sign a message with the given script */ virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; }; /** Adds script and derivation path information to a PSBT, and optionally signs it. */ @@ -355,13 +355,13 @@ private: public: using ScriptPubKeyMan::ScriptPubKeyMan; - bool GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) override; + bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) override; + bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override; void KeepDestination(int64_t index, const OutputType& type) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override; @@ -396,7 +396,7 @@ public: bool CanProvide(const CScript& script, SignatureData& sigdata) override; - bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override; + bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; @@ -559,13 +559,13 @@ public: mutable RecursiveMutex cs_desc_man; - bool GetNewDestination(const OutputType type, CTxDestination& dest, std::string& error) override; + bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, std::string& error) override; + bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) override; // Tops up the descriptor cache and m_map_script_pub_keys. The cache is stored in the wallet file @@ -601,7 +601,7 @@ public: bool CanProvide(const CScript& script, SignatureData& sigdata) override; - bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const override; + bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; @@ -621,7 +621,7 @@ public: const WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); const std::vector<CScript> GetScriptPubKeys() const; - bool GetDescriptorString(std::string& out) const; + bool GetDescriptorString(std::string& out, const bool priv) const; void UpgradeDescriptorCache(); }; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 6a8df437ae..3bb7134b24 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -618,9 +618,9 @@ bool CWallet::CreateTransactionInternal( // Reserve a new key pair from key pool. If it fails, provide a dummy // destination in case we don't need change. CTxDestination dest; - std::string dest_err; + bilingual_str dest_err; if (!reservedest.GetReservedDestination(dest, true, dest_err)) { - error = strprintf(_("Transaction needs a change address, but we can't generate it. %s"), dest_err); + error = _("Transaction needs a change address, but we can't generate it.") + Untranslated(" ") + dest_err; } scriptChange = GetScriptForDestination(dest); // A valid destination implies a change script (and @@ -778,6 +778,10 @@ bool CWallet::CreateTransactionInternal( fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); } + // The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when + // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug. + assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= change_and_fee - change_amount); + // Update nFeeRet in case fee_needed changed due to dropping the change output if (fee_needed <= change_and_fee - change_amount) { nFeeRet = change_and_fee - change_amount; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index c65ebad52f..3488ae3526 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -7,6 +7,7 @@ #include <primitives/transaction.h> #include <random.h> #include <test/util/setup_common.h> +#include <util/translation.h> #include <wallet/coincontrol.h> #include <wallet/coinselection.h> #include <wallet/test/wallet_test_fixture.h> @@ -66,7 +67,7 @@ static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bo tx.vout[nInput].nValue = nValue; if (spendable) { CTxDestination dest; - std::string error; + bilingual_str error; const bool destination_ok = wallet.GetNewDestination(OutputType::BECH32, "", dest, error); assert(destination_ok); tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 75a08b6f74..c8c5215e1b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -69,7 +69,7 @@ static CMutableTransaction TestSimpleSpend(const CTransaction& from, uint32_t in keystore.AddKey(key); std::map<COutPoint, Coin> coins; coins[mtx.vin[0].prevout].out = from.vout[index]; - std::map<int, std::string> input_errors; + std::map<int, bilingual_str> input_errors; BOOST_CHECK(SignTransaction(mtx, &keystore, coins, SIGHASH_ALL, input_errors)); return mtx; } @@ -589,7 +589,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); BOOST_CHECK(!wallet->TopUpKeyPool(1000)); CTxDestination dest; - std::string error; + bilingual_str error; BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 741540f724..e6227048d2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -94,6 +94,16 @@ static void UpdateWalletSetting(interfaces::Chain& chain, } } +/** + * Refresh mempool status so the wallet is in an internally consistent state and + * immediately knows the transaction's status: Whether it can be considered + * trusted and is eligible to be abandoned ... + */ +static void RefreshMempoolStatus(CWalletTx& tx, interfaces::Chain& chain) +{ + tx.fInMempool = chain.isInMempool(tx.GetHash()); +} + bool AddWallet(const std::shared_ptr<CWallet>& wallet) { LOCK(cs_wallets); @@ -803,10 +813,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash) wtx.mapValue["replaced_by_txid"] = newHash.ToString(); // Refresh mempool status without waiting for transactionRemovedFromMempool - // notification so the wallet is in an internally consistent state and - // immediately knows the old transaction should not be considered trusted - // and is eligible to be abandoned - wtx.fInMempool = chain().isInMempool(originalHash); + RefreshMempoolStatus(wtx, chain()); WalletBatch batch(GetDatabase()); @@ -1206,7 +1213,7 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx, uint64_t memp auto it = mapWallet.find(tx->GetHash()); if (it != mapWallet.end()) { - it->second.fInMempool = true; + RefreshMempoolStatus(it->second, chain()); } } @@ -1214,7 +1221,7 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe LOCK(cs_wallet); auto it = mapWallet.find(tx->GetHash()); if (it != mapWallet.end()) { - it->second.fInMempool = false; + RefreshMempoolStatus(it->second, chain()); } // Handle transactions that were removed from the mempool because they // conflict with transactions in a newly connected block. @@ -1822,11 +1829,11 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const const CWalletTx& wtx = mi->second; coins[input.prevout] = Coin(wtx.tx->vout[input.prevout.n], wtx.m_confirm.block_height, wtx.IsCoinBase()); } - std::map<int, std::string> input_errors; + std::map<int, bilingual_str> input_errors; return SignTransaction(tx, coins, SIGHASH_DEFAULT, input_errors); } -bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const +bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const { // Try to sign with all ScriptPubKeyMans for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) { @@ -2128,7 +2135,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) return res; } -bool CWallet::GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error) +bool CWallet::GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, bilingual_str& error) { LOCK(cs_wallet); error.clear(); @@ -2138,7 +2145,7 @@ bool CWallet::GetNewDestination(const OutputType type, const std::string label, spk_man->TopUp(); result = spk_man->GetNewDestination(type, dest, error); } else { - error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)).translated; + error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); } if (result) { SetAddressBook(dest, label, "receive"); @@ -2147,7 +2154,7 @@ bool CWallet::GetNewDestination(const OutputType type, const std::string label, return result; } -bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error) +bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { LOCK(cs_wallet); error.clear(); @@ -2200,11 +2207,11 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co return result; } -bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal, std::string& error) +bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal, bilingual_str& error) { m_spk_man = pwallet->GetScriptPubKeyMan(type, internal); if (!m_spk_man) { - error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)).translated; + error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3997751f52..25f89e8ea4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -183,7 +183,7 @@ public: } //! Reserve an address - bool GetReservedDestination(CTxDestination& pubkey, bool internal, std::string& error); + bool GetReservedDestination(CTxDestination& pubkey, bool internal, bilingual_str& error); //! Return reserved address void ReturnDestination(); //! Keep the address. Do not return it's key to the keypool when this object goes out of scope @@ -563,7 +563,7 @@ public: /** Fetch the inputs and sign with SIGHASH_ALL. */ bool SignTransaction(CMutableTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Sign the tx given the input coins and sighash. */ - bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& input_errors) const; + bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const; /** @@ -665,8 +665,8 @@ public: */ void MarkDestinationsDirty(const std::set<CTxDestination>& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error); - bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error); + bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, bilingual_str& error); + bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, bilingual_str& error); isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index eb027c554a..5dd6cb6cb2 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -4,10 +4,11 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test BIP66 (DER SIG). -Test that the DERSIG soft-fork activates at (regtest) height 1251. +Test the DERSIG soft-fork activation on regtest. """ from test_framework.blocktools import ( + DERSIG_HEIGHT, create_block, create_coinbase, ) @@ -23,8 +24,6 @@ from test_framework.wallet import ( MiniWalletMode, ) -DERSIG_HEIGHT = 1251 - # A canonical signature consists of: # <30> <total len> <02> <len R> <R> <02> <len S> <S> <hashtype> @@ -90,8 +89,10 @@ class BIP66Test(BitcoinTestFramework): block.rehash() block.solve() + assert_equal(self.nodes[0].getblockcount(), DERSIG_HEIGHT - 2) self.test_dersig_info(is_active=False) # Not active as of current tip and next block does not need to obey rules peer.send_and_ping(msg_block(block)) + assert_equal(self.nodes[0].getblockcount(), DERSIG_HEIGHT - 1) self.test_dersig_info(is_active=True) # Not active as of current tip, but next block must obey rules assert_equal(self.nodes[0].getbestblockhash(), block.hash) diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py new file mode 100755 index 0000000000..749ec6aa77 --- /dev/null +++ b/test/functional/mempool_package_limits.py @@ -0,0 +1,475 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test logic for limiting mempool and package ancestors/descendants.""" + +from decimal import Decimal + +from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE +from test_framework.test_framework import BitcoinTestFramework +from test_framework.messages import ( + COIN, + CTransaction, + CTxInWitness, + tx_from_hex, + WITNESS_SCALE_FACTOR, +) +from test_framework.script import ( + CScript, + OP_TRUE, +) +from test_framework.util import ( + assert_equal, +) +from test_framework.wallet import ( + bulk_transaction, + create_child_with_parents, + make_chain, +) + +class MempoolPackageLimitsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def run_test(self): + self.log.info("Generate blocks to create UTXOs") + node = self.nodes[0] + self.privkeys = [node.get_deterministic_priv_key().key] + self.address = node.get_deterministic_priv_key().address + self.coins = [] + # The last 100 coinbase transactions are premature + for b in node.generatetoaddress(200, self.address)[:100]: + coinbase = node.getblock(blockhash=b, verbosity=2)["tx"][0] + self.coins.append({ + "txid": coinbase["txid"], + "amount": coinbase["vout"][0]["value"], + "scriptPubKey": coinbase["vout"][0]["scriptPubKey"], + }) + + self.test_chain_limits() + self.test_desc_count_limits() + self.test_anc_count_limits() + self.test_anc_count_limits_2() + self.test_anc_count_limits_bushy() + + # The node will accept our (nonstandard) extra large OP_RETURN outputs + self.restart_node(0, extra_args=["-acceptnonstdtxn=1"]) + self.test_anc_size_limits() + self.test_desc_size_limits() + + def test_chain_limits_helper(self, mempool_count, package_count): + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + first_coin = self.coins.pop() + spk = None + txid = first_coin["txid"] + chain_hex = [] + chain_txns = [] + value = first_coin["amount"] + + for i in range(mempool_count + package_count): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < mempool_count: + node.sendrawtransaction(txhex) + assert_equal(node.getrawmempool(verbose=True)[txid]["ancestorcount"], i + 1) + else: + chain_hex.append(txhex) + chain_txns.append(tx) + testres_too_long = node.testmempoolaccept(rawtxs=chain_hex) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=chain_hex)]) + + def test_chain_limits(self): + """Create chains from mempool and package transactions that are longer than 25, + but only if both in-mempool and in-package transactions are considered together. + This checks that both mempool and in-package transactions are taken into account when + calculating ancestors/descendant limits. + """ + self.log.info("Check that in-package ancestors count for mempool ancestor limits") + + # 24 transactions in the mempool and 2 in the package. The parent in the package has + # 24 in-mempool ancestors and 1 in-package descendant. The child has 0 direct parents + # in the mempool, but 25 in-mempool and in-package ancestors in total. + self.test_chain_limits_helper(24, 2) + # 2 transactions in the mempool and 24 in the package. + self.test_chain_limits_helper(2, 24) + # 13 transactions in the mempool and 13 in the package. + self.test_chain_limits_helper(13, 13) + + def test_desc_count_limits(self): + """Create an 'A' shaped package with 24 transactions in the mempool and 2 in the package: + M1 + ^ ^ + M2a M2b + . . + . . + . . + M12a ^ + ^ M13b + ^ ^ + Pa Pb + The top ancestor in the package exceeds descendant limits but only if the in-mempool and in-package + descendants are all considered together (24 including in-mempool descendants and 26 including both + package transactions). + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + self.log.info("Check that in-mempool and in-package descendants are calculated properly in packages") + # Top parent in mempool, M1 + first_coin = self.coins.pop() + parent_value = (first_coin["amount"] - Decimal("0.0002")) / 2 # Deduct reasonable fee and make 2 outputs + inputs = [{"txid": first_coin["txid"], "vout": 0}] + outputs = [{self.address : parent_value}, {ADDRESS_BCRT1_P2WSH_OP_TRUE : parent_value}] + rawtx = node.createrawtransaction(inputs, outputs) + + parent_signed = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys) + assert parent_signed["complete"] + parent_tx = tx_from_hex(parent_signed["hex"]) + parent_txid = parent_tx.rehash() + node.sendrawtransaction(parent_signed["hex"]) + + package_hex = [] + + # Chain A + spk = parent_tx.vout[0].scriptPubKey.hex() + value = parent_value + txid = parent_txid + for i in range(12): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < 11: # M2a... M12a + node.sendrawtransaction(txhex) + else: # Pa + package_hex.append(txhex) + + # Chain B + value = parent_value - Decimal("0.0001") + rawtx_b = node.createrawtransaction([{"txid": parent_txid, "vout": 1}], {self.address : value}) + tx_child_b = tx_from_hex(rawtx_b) # M2b + tx_child_b.wit.vtxinwit = [CTxInWitness()] + tx_child_b.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])] + tx_child_b_hex = tx_child_b.serialize().hex() + node.sendrawtransaction(tx_child_b_hex) + spk = tx_child_b.vout[0].scriptPubKey.hex() + txid = tx_child_b.rehash() + for i in range(12): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < 11: # M3b... M13b + node.sendrawtransaction(txhex) + else: # Pb + package_hex.append(txhex) + + assert_equal(24, node.getmempoolinfo()["size"]) + assert_equal(2, len(package_hex)) + testres_too_long = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + + def test_anc_count_limits(self): + """Create a 'V' shaped chain with 24 transactions in the mempool and 3 in the package: + M1a M1b + ^ ^ + M2a M2b + . . + . . + . . + M12a M12b + ^ ^ + Pa Pb + ^ ^ + Pc + The lowest descendant, Pc, exceeds ancestor limits, but only if the in-mempool + and in-package ancestors are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + package_hex = [] + parents_tx = [] + values = [] + scripts = [] + + self.log.info("Check that in-mempool and in-package ancestors are calculated properly in packages") + + # Two chains of 13 transactions each + for _ in range(2): + spk = None + top_coin = self.coins.pop() + txid = top_coin["txid"] + value = top_coin["amount"] + for i in range(13): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < 12: + node.sendrawtransaction(txhex) + else: # Save the 13th transaction for the package + package_hex.append(txhex) + parents_tx.append(tx) + scripts.append(spk) + values.append(value) + + # Child Pc + child_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, scripts) + package_hex.append(child_hex) + + assert_equal(24, node.getmempoolinfo()["size"]) + assert_equal(3, len(package_hex)) + testres_too_long = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + + def test_anc_count_limits_2(self): + """Create a 'Y' shaped chain with 24 transactions in the mempool and 2 in the package: + M1a M1b + ^ ^ + M2a M2b + . . + . . + . . + M12a M12b + ^ ^ + Pc + ^ + Pd + The lowest descendant, Pd, exceeds ancestor limits, but only if the in-mempool + and in-package ancestors are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + parents_tx = [] + values = [] + scripts = [] + + self.log.info("Check that in-mempool and in-package ancestors are calculated properly in packages") + # Two chains of 12 transactions each + for _ in range(2): + spk = None + top_coin = self.coins.pop() + txid = top_coin["txid"] + value = top_coin["amount"] + for i in range(12): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + value -= Decimal("0.0001") + node.sendrawtransaction(txhex) + if i == 11: + # last 2 transactions will be the parents of Pc + parents_tx.append(tx) + values.append(value) + scripts.append(spk) + + # Child Pc + pc_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, scripts) + pc_tx = tx_from_hex(pc_hex) + pc_value = sum(values) - Decimal("0.0002") + pc_spk = pc_tx.vout[0].scriptPubKey.hex() + + # Child Pd + (_, pd_hex, _, _) = make_chain(node, self.address, self.privkeys, pc_tx.rehash(), pc_value, 0, pc_spk) + + assert_equal(24, node.getmempoolinfo()["size"]) + testres_too_long = node.testmempoolaccept(rawtxs=[pc_hex, pd_hex]) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=[pc_hex, pd_hex])]) + + def test_anc_count_limits_bushy(self): + """Create a tree with 20 transactions in the mempool and 6 in the package: + M1...M4 M5...M8 M9...M12 M13...M16 M17...M20 + ^ ^ ^ ^ ^ (each with 4 parents) + P0 P1 P2 P3 P4 + ^ ^ ^ ^ ^ (5 parents) + PC + Where M(4i+1)...M+(4i+4) are the parents of Pi and P0, P1, P2, P3, and P4 are the parents of PC. + P0... P4 individually only have 4 parents each, and PC has no in-mempool parents. But + combined, PC has 25 in-mempool and in-package parents. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + package_hex = [] + parent_txns = [] + parent_values = [] + scripts = [] + for _ in range(5): # Make package transactions P0 ... P4 + gp_tx = [] + gp_values = [] + gp_scripts = [] + for _ in range(4): # Make mempool transactions M(4i+1)...M(4i+4) + parent_coin = self.coins.pop() + value = parent_coin["amount"] + txid = parent_coin["txid"] + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value) + gp_tx.append(tx) + gp_values.append(value) + gp_scripts.append(spk) + node.sendrawtransaction(txhex) + # Package transaction Pi + pi_hex = create_child_with_parents(node, self.address, self.privkeys, gp_tx, gp_values, gp_scripts) + package_hex.append(pi_hex) + pi_tx = tx_from_hex(pi_hex) + parent_txns.append(pi_tx) + parent_values.append(Decimal(pi_tx.vout[0].nValue) / COIN) + scripts.append(pi_tx.vout[0].scriptPubKey.hex()) + # Package transaction PC + package_hex.append(create_child_with_parents(node, self.address, self.privkeys, parent_txns, parent_values, scripts)) + + assert_equal(20, node.getmempoolinfo()["size"]) + assert_equal(6, len(package_hex)) + testres = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + + def test_anc_size_limits(self): + """Test Case with 2 independent transactions in the mempool and a parent + child in the + package, where the package parent is the child of both mempool transactions (30KvB each): + A B + ^ ^ + C + ^ + D + The lowest descendant, D, exceeds ancestor size limits, but only if the in-mempool + and in-package ancestors are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + parents_tx = [] + values = [] + scripts = [] + target_weight = WITNESS_SCALE_FACTOR * 1000 * 30 # 30KvB + high_fee = Decimal("0.003") # 10 sats/vB + self.log.info("Check that in-mempool and in-package ancestor size limits are calculated properly in packages") + # Mempool transactions A and B + for _ in range(2): + spk = None + top_coin = self.coins.pop() + txid = top_coin["txid"] + value = top_coin["amount"] + (tx, _, _, _) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk, high_fee) + bulked_tx = bulk_transaction(tx, node, target_weight, self.privkeys) + node.sendrawtransaction(bulked_tx.serialize().hex()) + parents_tx.append(bulked_tx) + values.append(Decimal(bulked_tx.vout[0].nValue) / COIN) + scripts.append(bulked_tx.vout[0].scriptPubKey.hex()) + + # Package transaction C + small_pc_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, scripts, high_fee) + pc_tx = bulk_transaction(tx_from_hex(small_pc_hex), node, target_weight, self.privkeys) + pc_value = Decimal(pc_tx.vout[0].nValue) / COIN + pc_spk = pc_tx.vout[0].scriptPubKey.hex() + pc_hex = pc_tx.serialize().hex() + + # Package transaction D + (small_pd, _, val, spk) = make_chain(node, self.address, self.privkeys, pc_tx.rehash(), pc_value, 0, pc_spk, high_fee) + prevtxs = [{ + "txid": pc_tx.rehash(), + "vout": 0, + "scriptPubKey": spk, + "amount": val, + }] + pd_tx = bulk_transaction(small_pd, node, target_weight, self.privkeys, prevtxs) + pd_hex = pd_tx.serialize().hex() + + assert_equal(2, node.getmempoolinfo()["size"]) + testres_too_heavy = node.testmempoolaccept(rawtxs=[pc_hex, pd_hex]) + for txres in testres_too_heavy: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=[pc_hex, pd_hex])]) + + def test_desc_size_limits(self): + """Create 3 mempool transactions and 2 package transactions (25KvB each): + Ma + ^ ^ + Mb Mc + ^ ^ + Pd Pe + The top ancestor in the package exceeds descendant size limits but only if the in-mempool + and in-package descendants are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + target_weight = 21 * 1000 * WITNESS_SCALE_FACTOR + high_fee = Decimal("0.0021") # 10 sats/vB + self.log.info("Check that in-mempool and in-package descendant sizes are calculated properly in packages") + # Top parent in mempool, Ma + first_coin = self.coins.pop() + parent_value = (first_coin["amount"] - high_fee) / 2 # Deduct fee and make 2 outputs + inputs = [{"txid": first_coin["txid"], "vout": 0}] + outputs = [{self.address : parent_value}, {ADDRESS_BCRT1_P2WSH_OP_TRUE: parent_value}] + rawtx = node.createrawtransaction(inputs, outputs) + parent_tx = bulk_transaction(tx_from_hex(rawtx), node, target_weight, self.privkeys) + node.sendrawtransaction(parent_tx.serialize().hex()) + + package_hex = [] + for j in range(2): # Two legs (left and right) + # Mempool transaction (Mb and Mc) + mempool_tx = CTransaction() + spk = parent_tx.vout[j].scriptPubKey.hex() + value = Decimal(parent_tx.vout[j].nValue) / COIN + txid = parent_tx.rehash() + prevtxs = [{ + "txid": txid, + "vout": j, + "scriptPubKey": spk, + "amount": value, + }] + if j == 0: # normal key + (tx_small, _, _, _) = make_chain(node, self.address, self.privkeys, txid, value, j, spk, high_fee) + mempool_tx = bulk_transaction(tx_small, node, target_weight, self.privkeys, prevtxs) + else: # OP_TRUE + inputs = [{"txid": txid, "vout": 1}] + outputs = {self.address: value - high_fee} + small_tx = tx_from_hex(node.createrawtransaction(inputs, outputs)) + mempool_tx = bulk_transaction(small_tx, node, target_weight, None, prevtxs) + node.sendrawtransaction(mempool_tx.serialize().hex()) + + # Package transaction (Pd and Pe) + spk = mempool_tx.vout[0].scriptPubKey.hex() + value = Decimal(mempool_tx.vout[0].nValue) / COIN + txid = mempool_tx.rehash() + (tx_small, _, _, _) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk, high_fee) + prevtxs = [{ + "txid": txid, + "vout": 0, + "scriptPubKey": spk, + "amount": value, + }] + package_tx = bulk_transaction(tx_small, node, target_weight, self.privkeys, prevtxs) + package_hex.append(package_tx.serialize().hex()) + + assert_equal(3, node.getmempoolinfo()["size"]) + assert_equal(2, len(package_hex)) + testres_too_heavy = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres_too_heavy: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + +if __name__ == "__main__": + MempoolPackageLimitsTest().main() diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 113e1f9492..e93698b08e 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -311,7 +311,7 @@ class AddrTest(BitcoinTestFramework): self.nodes[0].disconnect_p2ps() - def send_addrs_and_test_rate_limiting(self, peer, no_relay, new_addrs, total_addrs): + def send_addrs_and_test_rate_limiting(self, peer, no_relay, *, new_addrs, total_addrs): """Send an addr message and check that the number of addresses processed and rate-limited is as expected""" peer.send_and_ping(self.setup_rand_addr_msg(new_addrs)) @@ -329,27 +329,26 @@ class AddrTest(BitcoinTestFramework): assert_equal(addrs_rate_limited, max(0, total_addrs - peer.tokens)) def rate_limit_tests(self): - self.mocktime = int(time.time()) self.restart_node(0, []) self.nodes[0].setmocktime(self.mocktime) - for contype, no_relay in [("outbound-full-relay", False), ("block-relay-only", True), ("inbound", False)]: - self.log.info(f'Test rate limiting of addr processing for {contype} peers') - if contype == "inbound": + for conn_type, no_relay in [("outbound-full-relay", False), ("block-relay-only", True), ("inbound", False)]: + self.log.info(f'Test rate limiting of addr processing for {conn_type} peers') + if conn_type == "inbound": peer = self.nodes[0].add_p2p_connection(AddrReceiver()) else: - peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type=contype) + peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type=conn_type) # Send 600 addresses. For all but the block-relay-only peer this should result in addresses being processed. - self.send_addrs_and_test_rate_limiting(peer, no_relay, 600, 600) + self.send_addrs_and_test_rate_limiting(peer, no_relay, new_addrs=600, total_addrs=600) # Send 600 more addresses. For the outbound-full-relay peer (which we send a GETADDR, and thus will # process up to 1001 incoming addresses), this means more addresses will be processed. - self.send_addrs_and_test_rate_limiting(peer, no_relay, 600, 1200) + self.send_addrs_and_test_rate_limiting(peer, no_relay, new_addrs=600, total_addrs=1200) # Send 10 more. As we reached the processing limit for all nodes, no more addresses should be procesesd. - self.send_addrs_and_test_rate_limiting(peer, no_relay, 10, 1210) + self.send_addrs_and_test_rate_limiting(peer, no_relay, new_addrs=10, total_addrs=1210) # Advance the time by 100 seconds, permitting the processing of 10 more addresses. # Send 200 and verify that 10 are processed. @@ -357,7 +356,7 @@ class AddrTest(BitcoinTestFramework): self.nodes[0].setmocktime(self.mocktime) peer.increment_tokens(10) - self.send_addrs_and_test_rate_limiting(peer, no_relay, 200, 1410) + self.send_addrs_and_test_rate_limiting(peer, no_relay, new_addrs=200, total_addrs=1410) # Advance the time by 1000 seconds, permitting the processing of 100 more addresses. # Send 200 and verify that 100 are processed. @@ -365,9 +364,10 @@ class AddrTest(BitcoinTestFramework): self.nodes[0].setmocktime(self.mocktime) peer.increment_tokens(100) - self.send_addrs_and_test_rate_limiting(peer, no_relay, 200, 1610) + self.send_addrs_and_test_rate_limiting(peer, no_relay, new_addrs=200, total_addrs=1610) self.nodes[0].disconnect_p2ps() + if __name__ == '__main__': AddrTest().main() diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 721e3f93a3..1e73dcf5cd 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -27,6 +27,7 @@ import subprocess from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE from test_framework.blocktools import ( + DERSIG_HEIGHT, create_block, create_coinbase, TIME_GENESIS_BLOCK, @@ -141,7 +142,7 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res['softforks'], { 'bip34': {'type': 'buried', 'active': True, 'height': 2}, - 'bip66': {'type': 'buried', 'active': False, 'height': 1251}, + 'bip66': {'type': 'buried', 'active': True, 'height': DERSIG_HEIGHT}, 'bip65': {'type': 'buried', 'active': False, 'height': 1351}, 'csv': {'type': 'buried', 'active': False, 'height': 432}, 'segwit': {'type': 'buried', 'active': True, 'height': 0}, diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index fa98c44152..2524eaa7a5 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -99,6 +99,7 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_subtract_fee_with_presets() self.test_transaction_too_large() self.test_include_unsafe() + self.test_22670() def test_change_position(self): """Ensure setting changePosition in fundraw with an exact match is handled properly.""" @@ -969,6 +970,62 @@ class RawTransactionsTest(BitcoinTestFramework): signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex']) wallet.sendrawtransaction(signedtx['hex']) + def test_22670(self): + # In issue #22670, it was observed that ApproximateBestSubset may + # choose enough value to cover the target amount but not enough to cover the transaction fees. + # This leads to a transaction whose actual transaction feerate is lower than expected. + # However at normal feerates, the difference between the effective value and the real value + # that this bug is not detected because the transaction fee must be at least 0.01 BTC (the minimum change value). + # Otherwise the targeted minimum change value will be enough to cover the transaction fees that were not + # being accounted for. So the minimum relay fee is set to 0.1 BTC/kvB in this test. + self.log.info("Test issue 22670 ApproximateBestSubset bug") + # Make sure the default wallet will not be loaded when restarted with a high minrelaytxfee + self.nodes[0].unloadwallet(self.default_wallet_name, False) + feerate = Decimal("0.1") + self.restart_node(0, [f"-minrelaytxfee={feerate}", "-discardfee=0"]) # Set high minrelayfee, set discardfee to 0 for easier calculation + + self.nodes[0].loadwallet(self.default_wallet_name, True) + funds = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet(wallet_name="tester") + tester = self.nodes[0].get_wallet_rpc("tester") + + # Because this test is specifically for ApproximateBestSubset, the target value must be greater + # than any single input available, and require more than 1 input. So we make 3 outputs + for i in range(0, 3): + funds.sendtoaddress(tester.getnewaddress(address_type="bech32"), 1) + self.nodes[0].generate(1) + + # Create transactions in order to calculate fees for the target bounds that can trigger this bug + change_tx = tester.fundrawtransaction(tester.createrawtransaction([], [{funds.getnewaddress(): 1.5}])) + tx = tester.createrawtransaction([], [{funds.getnewaddress(): 2}]) + no_change_tx = tester.fundrawtransaction(tx, {"subtractFeeFromOutputs": [0]}) + + overhead_fees = feerate * len(tx) / 2 / 1000 + cost_of_change = change_tx["fee"] - no_change_tx["fee"] + fees = no_change_tx["fee"] + assert_greater_than(fees, 0.01) + + def do_fund_send(target): + create_tx = tester.createrawtransaction([], [{funds.getnewaddress(): lower_bound}]) + funded_tx = tester.fundrawtransaction(create_tx) + signed_tx = tester.signrawtransactionwithwallet(funded_tx["hex"]) + assert signed_tx["complete"] + decoded_tx = tester.decoderawtransaction(signed_tx["hex"]) + assert_equal(len(decoded_tx["vin"]), 3) + assert tester.testmempoolaccept([signed_tx["hex"]])[0]["allowed"] + + # We want to choose more value than is available in 2 inputs when considering the fee, + # but not enough to need 3 inputs when not considering the fee. + # So the target value must be at least 2.00000001 - fee. + lower_bound = Decimal("2.00000001") - fees + # The target value must be at most 2 - cost_of_change - not_input_fees - min_change (these are all + # included in the target before ApproximateBestSubset). + upper_bound = Decimal("2.0") - cost_of_change - overhead_fees - Decimal("0.01") + assert_greater_than_or_equal(upper_bound, lower_bound) + do_fund_send(lower_bound) + do_fund_send(upper_bound) + + self.restart_node(0) if __name__ == '__main__': RawTransactionsTest().main() diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 4b2ed20958..3cb4154601 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -22,6 +22,11 @@ from test_framework.script import ( from test_framework.util import ( assert_equal, ) +from test_framework.wallet import ( + create_child_with_parents, + create_raw_chain, + make_chain, +) class RPCPackagesTest(BitcoinTestFramework): def set_test_params(self): @@ -78,26 +83,6 @@ class RPCPackagesTest(BitcoinTestFramework): self.test_conflicting() self.test_rbf() - def chain_transaction(self, parent_txid, parent_value, n=0, parent_locking_script=None): - """Build a transaction that spends parent_txid.vout[n] and produces one output with - amount = parent_value with a fee deducted. - Return tuple (CTransaction object, raw hex, nValue, scriptPubKey of the output created). - """ - node = self.nodes[0] - inputs = [{"txid": parent_txid, "vout": n}] - my_value = parent_value - Decimal("0.0001") - outputs = {self.address : my_value} - rawtx = node.createrawtransaction(inputs, outputs) - prevtxs = [{ - "txid": parent_txid, - "vout": n, - "scriptPubKey": parent_locking_script, - "amount": parent_value, - }] if parent_locking_script else None - signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys, prevtxs=prevtxs) - assert signedtx["complete"] - tx = tx_from_hex(signedtx["hex"]) - return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex()) def test_independent(self): self.log.info("Test multiple independent transactions in a package") @@ -148,20 +133,7 @@ class RPCPackagesTest(BitcoinTestFramework): def test_chain(self): node = self.nodes[0] first_coin = self.coins.pop() - - # Chain of 25 transactions - parent_locking_script = None - txid = first_coin["txid"] - chain_hex = [] - chain_txns = [] - value = first_coin["amount"] - - for _ in range(25): - (tx, txhex, value, parent_locking_script) = self.chain_transaction(txid, value, 0, parent_locking_script) - txid = tx.rehash() - chain_hex.append(txhex) - chain_txns.append(tx) - + (chain_hex, chain_txns) = create_raw_chain(node, first_coin, self.address, self.privkeys) self.log.info("Check that testmempoolaccept requires packages to be sorted by dependency") assert_equal(node.testmempoolaccept(rawtxs=chain_hex[::-1]), [{"txid": tx.rehash(), "wtxid": tx.getwtxid(), "package-error": "package-not-sorted"} for tx in chain_txns[::-1]]) @@ -201,7 +173,7 @@ class RPCPackagesTest(BitcoinTestFramework): child_value = value - Decimal("0.0001") # Child A - (_, tx_child_a_hex, _, _) = self.chain_transaction(parent_txid, child_value, 0, parent_locking_script_a) + (_, tx_child_a_hex, _, _) = make_chain(node, self.address, self.privkeys, parent_txid, child_value, 0, parent_locking_script_a) assert not node.testmempoolaccept([tx_child_a_hex])[0]["allowed"] # Child B @@ -226,19 +198,6 @@ class RPCPackagesTest(BitcoinTestFramework): node.sendrawtransaction(rawtx) assert_equal(testres_single, testres_multiple_ab) - def create_child_with_parents(self, parents_tx, values, locking_scripts): - """Creates a transaction that spends the first output of each parent in parents_tx.""" - num_parents = len(parents_tx) - total_value = sum(values) - inputs = [{"txid": tx.rehash(), "vout": 0} for tx in parents_tx] - outputs = {self.address : total_value - num_parents * Decimal("0.0001")} - rawtx_child = self.nodes[0].createrawtransaction(inputs, outputs) - prevtxs = [] - for i in range(num_parents): - prevtxs.append({"txid": parents_tx[i].rehash(), "vout": 0, "scriptPubKey": locking_scripts[i], "amount": values[i]}) - signedtx_child = self.nodes[0].signrawtransactionwithkey(hexstring=rawtx_child, privkeys=self.privkeys, prevtxs=prevtxs) - assert signedtx_child["complete"] - return signedtx_child["hex"] def test_multiple_parents(self): node = self.nodes[0] @@ -253,12 +212,12 @@ class RPCPackagesTest(BitcoinTestFramework): for _ in range(num_parents): parent_coin = self.coins.pop() value = parent_coin["amount"] - (tx, txhex, value, parent_locking_script) = self.chain_transaction(parent_coin["txid"], value) + (tx, txhex, value, parent_locking_script) = make_chain(node, self.address, self.privkeys, parent_coin["txid"], value) package_hex.append(txhex) parents_tx.append(tx) values.append(value) parent_locking_scripts.append(parent_locking_script) - child_hex = self.create_child_with_parents(parents_tx, values, parent_locking_scripts) + child_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, parent_locking_scripts) # Package accept should work with the parents in any order (as long as parents come before child) for _ in range(10): random.shuffle(package_hex) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 52ca579b1b..11d0ab40d5 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -54,6 +54,7 @@ TIME_GENESIS_BLOCK = 1296688602 COINBASE_MATURITY = 100 # Soft-fork activation heights +DERSIG_HEIGHT = 102 # BIP 66 CLTV_HEIGHT = 1351 CSV_ACTIVATION_HEIGHT = 432 diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 609553c6d0..ba5b95f930 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -4,8 +4,10 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """A limited-functionality wallet, which may replace a real wallet in tests""" +from copy import deepcopy from decimal import Decimal from enum import Enum +from random import choice from typing import Optional from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE from test_framework.key import ECKey @@ -16,6 +18,7 @@ from test_framework.messages import ( CTxIn, CTxInWitness, CTxOut, + tx_from_hex, ) from test_framework.script import ( CScript, @@ -27,9 +30,11 @@ from test_framework.script import ( ) from test_framework.util import ( assert_equal, + assert_greater_than_or_equal, satoshi_round, ) +DEFAULT_FEE = Decimal("0.0001") class MiniWalletMode(Enum): """Determines the transaction type the MiniWallet is creating and spending. @@ -176,3 +181,75 @@ class MiniWallet: def sendrawtransaction(self, *, from_node, tx_hex): from_node.sendrawtransaction(tx_hex) self.scan_tx(from_node.decoderawtransaction(tx_hex)) + +def make_chain(node, address, privkeys, parent_txid, parent_value, n=0, parent_locking_script=None, fee=DEFAULT_FEE): + """Build a transaction that spends parent_txid.vout[n] and produces one output with + amount = parent_value with a fee deducted. + Return tuple (CTransaction object, raw hex, nValue, scriptPubKey of the output created). + """ + inputs = [{"txid": parent_txid, "vout": n}] + my_value = parent_value - fee + outputs = {address : my_value} + rawtx = node.createrawtransaction(inputs, outputs) + prevtxs = [{ + "txid": parent_txid, + "vout": n, + "scriptPubKey": parent_locking_script, + "amount": parent_value, + }] if parent_locking_script else None + signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=privkeys, prevtxs=prevtxs) + assert signedtx["complete"] + tx = tx_from_hex(signedtx["hex"]) + return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex()) + +def create_child_with_parents(node, address, privkeys, parents_tx, values, locking_scripts, fee=DEFAULT_FEE): + """Creates a transaction that spends the first output of each parent in parents_tx.""" + num_parents = len(parents_tx) + total_value = sum(values) + inputs = [{"txid": tx.rehash(), "vout": 0} for tx in parents_tx] + outputs = {address : total_value - fee} + rawtx_child = node.createrawtransaction(inputs, outputs) + prevtxs = [] + for i in range(num_parents): + prevtxs.append({"txid": parents_tx[i].rehash(), "vout": 0, "scriptPubKey": locking_scripts[i], "amount": values[i]}) + signedtx_child = node.signrawtransactionwithkey(hexstring=rawtx_child, privkeys=privkeys, prevtxs=prevtxs) + assert signedtx_child["complete"] + return signedtx_child["hex"] + +def create_raw_chain(node, first_coin, address, privkeys, chain_length=25): + """Helper function: create a "chain" of chain_length transactions. The nth transaction in the + chain is a child of the n-1th transaction and parent of the n+1th transaction. + """ + parent_locking_script = None + txid = first_coin["txid"] + chain_hex = [] + chain_txns = [] + value = first_coin["amount"] + + for _ in range(chain_length): + (tx, txhex, value, parent_locking_script) = make_chain(node, address, privkeys, txid, value, 0, parent_locking_script) + txid = tx.rehash() + chain_hex.append(txhex) + chain_txns.append(tx) + + return (chain_hex, chain_txns) + +def bulk_transaction(tx, node, target_weight, privkeys, prevtxs=None): + """Pad a transaction with extra outputs until it reaches a target weight (or higher). + returns CTransaction object + """ + tx_heavy = deepcopy(tx) + assert_greater_than_or_equal(target_weight, tx_heavy.get_weight()) + while tx_heavy.get_weight() < target_weight: + random_spk = "6a4d0200" # OP_RETURN OP_PUSH2 512 bytes + for _ in range(512*2): + random_spk += choice("0123456789ABCDEF") + tx_heavy.vout.append(CTxOut(0, bytes.fromhex(random_spk))) + # Re-sign the transaction + if privkeys: + signed = node.signrawtransactionwithkey(tx_heavy.serialize().hex(), privkeys, prevtxs) + return tx_from_hex(signed["hex"]) + # OP_TRUE + tx_heavy.wit.vtxinwit = [CTxInWitness()] + tx_heavy.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])] + return tx_heavy diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index fecf52d53a..1a1a6a263a 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -218,6 +218,7 @@ BASE_SCRIPTS = [ 'rpc_createmultisig.py --legacy-wallet', 'rpc_createmultisig.py --descriptors', 'rpc_packages.py', + 'mempool_package_limits.py', 'feature_versionbits_warning.py', 'rpc_preciousblock.py', 'wallet_importprunedfunds.py --legacy-wallet', diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 05a0ef0ea1..c7a983556d 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -111,6 +111,18 @@ class WalletBackupTest(BitcoinTestFramework): os.remove(os.path.join(self.nodes[1].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)) os.remove(os.path.join(self.nodes[2].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)) + def restore_nonexistent_wallet(self): + node = self.nodes[3] + nonexistent_wallet_file = os.path.join(self.nodes[0].datadir, 'nonexistent_wallet.bak') + wallet_name = "res0" + assert_raises_rpc_error(-8, "Backup file does not exist", node.restorewallet, wallet_name, nonexistent_wallet_file) + + def restore_wallet_existent_name(self): + node = self.nodes[3] + wallet_file = os.path.join(self.nodes[0].datadir, 'wallet.bak') + wallet_name = "res0" + assert_raises_rpc_error(-8, "Wallet name already exists.", node.restorewallet, wallet_name, wallet_file) + def init_three(self): self.init_wallet(0) self.init_wallet(1) @@ -169,26 +181,27 @@ class WalletBackupTest(BitcoinTestFramework): ## # Test restoring spender wallets from backups ## - self.log.info("Restoring using wallet.dat") - self.stop_three() - self.erase_three() + self.log.info("Restoring wallets on node 3 using backup files") - # Start node2 with no chain - shutil.rmtree(os.path.join(self.nodes[2].datadir, self.chain, 'blocks')) - shutil.rmtree(os.path.join(self.nodes[2].datadir, self.chain, 'chainstate')) + self.restore_nonexistent_wallet() - # Restore wallets from backup - shutil.copyfile(os.path.join(self.nodes[0].datadir, 'wallet.bak'), os.path.join(self.nodes[0].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)) - shutil.copyfile(os.path.join(self.nodes[1].datadir, 'wallet.bak'), os.path.join(self.nodes[1].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)) - shutil.copyfile(os.path.join(self.nodes[2].datadir, 'wallet.bak'), os.path.join(self.nodes[2].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename)) + backup_file_0 = os.path.join(self.nodes[0].datadir, 'wallet.bak') + backup_file_1 = os.path.join(self.nodes[1].datadir, 'wallet.bak') + backup_file_2 = os.path.join(self.nodes[2].datadir, 'wallet.bak') - self.log.info("Re-starting nodes") - self.start_three() - self.sync_blocks() + self.nodes[3].restorewallet("res0", backup_file_0) + self.nodes[3].restorewallet("res1", backup_file_1) + self.nodes[3].restorewallet("res2", backup_file_2) + + res0_rpc = self.nodes[3].get_wallet_rpc("res0") + res1_rpc = self.nodes[3].get_wallet_rpc("res1") + res2_rpc = self.nodes[3].get_wallet_rpc("res2") + + assert_equal(res0_rpc.getbalance(), balance0) + assert_equal(res1_rpc.getbalance(), balance1) + assert_equal(res2_rpc.getbalance(), balance2) - assert_equal(self.nodes[0].getbalance(), balance0) - assert_equal(self.nodes[1].getbalance(), balance1) - assert_equal(self.nodes[2].getbalance(), balance2) + self.restore_wallet_existent_name() if not self.options.descriptors: self.log.info("Restoring using dumped wallet") diff --git a/test/functional/wallet_listdescriptors.py b/test/functional/wallet_listdescriptors.py index c2565d84f6..221f5262d9 100755 --- a/test/functional/wallet_listdescriptors.py +++ b/test/functional/wallet_listdescriptors.py @@ -72,11 +72,39 @@ class ListDescriptorsTest(BitcoinTestFramework): ], } assert_equal(expected, wallet.listdescriptors()) + assert_equal(expected, wallet.listdescriptors(False)) + + self.log.info('Test list private descriptors') + expected_private = { + 'wallet_name': 'w2', + 'descriptors': [ + {'desc': descsum_create('wpkh(' + xprv + hardened_path + '/0/*)'), + 'timestamp': 1296688602, + 'active': False, + 'range': [0, 0], + 'next': 0}, + ], + } + assert_equal(expected_private, wallet.listdescriptors(True)) self.log.info("Test listdescriptors with encrypted wallet") wallet.encryptwallet("pass") assert_equal(expected, wallet.listdescriptors()) + self.log.info('Test list private descriptors with encrypted wallet') + assert_raises_rpc_error(-13, 'Please enter the wallet passphrase with walletpassphrase first.', wallet.listdescriptors, True) + wallet.walletpassphrase(passphrase="pass", timeout=1000000) + assert_equal(expected_private, wallet.listdescriptors(True)) + + self.log.info('Test list private descriptors with watch-only wallet') + node.createwallet(wallet_name='watch-only', descriptors=True, disable_private_keys=True) + watch_only_wallet = node.get_wallet_rpc('watch-only') + watch_only_wallet.importdescriptors([{ + 'desc': descsum_create('wpkh(' + xpub_acc + ')'), + 'timestamp': 1296688602, + }]) + assert_raises_rpc_error(-4, 'Can\'t get descriptor string', watch_only_wallet.listdescriptors, True) + self.log.info('Test non-active non-range combo descriptor') node.createwallet(wallet_name='w4', blank=True, descriptors=True) wallet = node.get_wallet_rpc('w4') |