diff options
Diffstat (limited to 'src')
106 files changed, 1628 insertions, 1013 deletions
diff --git a/src/addrdb.cpp b/src/addrdb.cpp index c3e224ee83..a5383be7cf 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -244,12 +244,7 @@ bool CAddrDB::Read(CAddrMan& addr) bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers) { - bool ret = DeserializeDB(ssPeers, addr, false); - if (!ret) { - // Ensure addrman is left in a clean state - addr.Clear(); - } - return ret; + return DeserializeDB(ssPeers, addr, false); } void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors) diff --git a/src/addrman.cpp b/src/addrman.cpp index 96139182d3..edcf97f846 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -77,6 +77,23 @@ double CAddrInfo::GetChance(int64_t nNow) const return fChance; } +CAddrMan::CAddrMan(bool deterministic, int32_t consistency_check_ratio) + : insecure_rand{deterministic} + , nKey{deterministic ? uint256{1} : insecure_rand.rand256()} + , m_consistency_check_ratio{consistency_check_ratio} +{ + for (auto& bucket : vvNew) { + for (auto& entry : bucket) { + entry = -1; + } + } + for (auto& bucket : vvTried) { + for (auto& entry : bucket) { + entry = -1; + } + } +} + CAddrInfo* CAddrMan::Find(const CNetAddr& addr, int* pnId) { AssertLockHeld(cs); @@ -431,11 +448,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 +480,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 +502,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 +519,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 +536,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 4d8d05a99a..e2cb60b061 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 */ @@ -58,6 +61,7 @@ private: mutable int nRandomPos{-1}; friend class CAddrMan; + friend class CAddrManDeterministic; public: @@ -104,23 +108,27 @@ public: * * Make sure no (localized) attacker can fill the entire table with his nodes/addresses. * * To that end: - * * Addresses are organized into buckets. - * * Addresses that have not yet been tried go into 1024 "new" buckets. - * * Based on the address range (/16 for IPv4) of the source of information, 64 buckets are selected at random. + * * Addresses are organized into buckets that can each store up to 64 entries. + * * Addresses to which our node has not successfully connected go into 1024 "new" buckets. + * * Based on the address range (/16 for IPv4) of the source of information, or if an asmap is provided, + * the AS it belongs to (for IPv4/IPv6), 64 buckets are selected at random. * * The actual bucket is chosen from one of these, based on the range in which the address itself is located. + * * The position in the bucket is chosen based on the full address. * * One single address can occur in up to 8 different buckets to increase selection chances for addresses that * are seen frequently. The chance for increasing this multiplicity decreases exponentially. - * * When adding a new address to a full bucket, a randomly chosen entry (with a bias favoring less recently seen - * ones) is removed from it first. + * * When adding a new address to an occupied position of a bucket, it will not replace the existing entry + * unless that address is also stored in another bucket or it doesn't meet one of several quality criteria + * (see IsTerrible for exact criteria). * * Addresses of nodes that are known to be accessible go into 256 "tried" buckets. * * Each address range selects at random 8 of these buckets. * * The actual bucket is chosen from one of these, based on the full address. - * * When adding a new good address to a full bucket, a randomly chosen entry (with a bias favoring less recently - * tried ones) is evicted from it, back to the "new" buckets. + * * When adding a new good address to an occupied position of a bucket, a FEELER connection to the + * old address is attempted. The old entry is only replaced and moved back to the "new" buckets if this + * 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 @@ -463,35 +471,7 @@ public: Check(); } - void Clear() - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - std::vector<int>().swap(vRandom); - nKey = insecure_rand.rand256(); - for (size_t bucket = 0; bucket < ADDRMAN_NEW_BUCKET_COUNT; bucket++) { - for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { - vvNew[bucket][entry] = -1; - } - } - for (size_t bucket = 0; bucket < ADDRMAN_TRIED_BUCKET_COUNT; bucket++) { - for (size_t entry = 0; entry < ADDRMAN_BUCKET_SIZE; entry++) { - vvTried[bucket][entry] = -1; - } - } - - nIdCount = 0; - nTried = 0; - nNew = 0; - nLastGood = 1; //Initially at 1 so that "never" is strictly worse. - mapInfo.clear(); - mapAddr.clear(); - } - - CAddrMan() - { - Clear(); - } + explicit CAddrMan(bool deterministic, int32_t consistency_check_ratio); ~CAddrMan() { @@ -506,22 +486,7 @@ public: return vRandom.size(); } - //! Add a single address. - bool Add(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty = 0) - EXCLUSIVE_LOCKS_REQUIRED(!cs) - { - LOCK(cs); - bool fRet = false; - Check(); - fRet |= Add_(addr, source, nTimePenalty); - Check(); - if (fRet) { - LogPrint(BCLog::ADDRMAN, "Added %s from %s: %i tried, %i new\n", addr.ToStringIPPort(), source.ToString(), nTried, nNew); - } - return fRet; - } - - //! Add multiple addresses. + //! Add addresses to addrman's new table. bool Add(const std::vector<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0) EXCLUSIVE_LOCKS_REQUIRED(!cs) { @@ -628,17 +593,16 @@ public: Check(); } -protected: - //! secret key to randomize bucket select with - uint256 nKey; +private: + //! A mutex to protect the inner data structures. + mutable Mutex cs; //! 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; + //! secret key to randomize bucket select with + uint256 nKey; -private: //! Serialization versions. enum Format : uint8_t { V0_HISTORICAL = 0, //!< historic format, before commit e6b343d88 @@ -662,7 +626,7 @@ private: static constexpr uint8_t INCOMPATIBILITY_BASE = 32; //! last used nId - int nIdCount GUARDED_BY(cs); + int nIdCount GUARDED_BY(cs){0}; //! table with information about all nIds std::unordered_map<int, CAddrInfo> mapInfo GUARDED_BY(cs); @@ -676,28 +640,30 @@ private: mutable std::vector<int> vRandom GUARDED_BY(cs); // number of "tried" entries - int nTried GUARDED_BY(cs); + int nTried GUARDED_BY(cs){0}; //! list of "tried" buckets int vvTried[ADDRMAN_TRIED_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); //! number of (unique) "new" entries - int nNew GUARDED_BY(cs); + int nNew GUARDED_BY(cs){0}; //! list of "new" buckets int vvNew[ADDRMAN_NEW_BUCKET_COUNT][ADDRMAN_BUCKET_SIZE] GUARDED_BY(cs); - //! last time Good was called (memory only) - int64_t nLastGood GUARDED_BY(cs); + //! last time Good was called (memory only). Initially set to 1 so that "never" is strictly worse. + int64_t nLastGood GUARDED_BY(cs){1}; //! 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); - //! find an entry, creating it if necessary. - //! nTime and nServices of the found node are updated, if necessary. + //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom. CAddrInfo* Create(const CAddress &addr, const CNetAddr &addrSource, int *pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Swap two elements in vRandom. @@ -731,22 +697,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. @@ -775,6 +738,7 @@ private: void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs); friend class CAddrManTest; + friend class CAddrManDeterministic; }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index b7bd8a3261..d69a651811 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -72,17 +72,15 @@ static void AddrManAdd(benchmark::Bench& bench) { CreateAddresses(); - CAddrMan addrman; - bench.run([&] { + CAddrMan addrman{/* deterministic */ false, /* consistency_check_ratio */ 0}; AddAddressesToAddrMan(addrman); - addrman.Clear(); }); } static void AddrManSelect(benchmark::Bench& bench) { - CAddrMan addrman; + CAddrMan addrman(/* deterministic */ false, /* consistency_check_ratio */ 0); FillAddrMan(addrman); @@ -94,7 +92,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 +110,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 +130,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-res.rc b/src/bitcoin-cli-res.rc index 405a302261..d9e5dcf7fd 100644 --- a/src/bitcoin-cli-res.rc +++ b/src/bitcoin-cli-res.rc @@ -2,9 +2,7 @@ #include "clientversion.h" // holds the needed client version information #define VER_PRODUCTVERSION CLIENT_VERSION_MAJOR,CLIENT_VERSION_MINOR,CLIENT_VERSION_BUILD -#define VER_PRODUCTVERSION_STR STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD) #define VER_FILEVERSION VER_PRODUCTVERSION -#define VER_FILEVERSION_STR VER_PRODUCTVERSION_STR VS_VERSION_INFO VERSIONINFO FILEVERSION VER_FILEVERSION @@ -18,13 +16,13 @@ BEGIN BEGIN VALUE "CompanyName", "Bitcoin" VALUE "FileDescription", "bitcoin-cli (JSON-RPC client for " PACKAGE_NAME ")" - VALUE "FileVersion", VER_FILEVERSION_STR + VALUE "FileVersion", PACKAGE_VERSION VALUE "InternalName", "bitcoin-cli" VALUE "LegalCopyright", COPYRIGHT_STR VALUE "LegalTrademarks1", "Distributed under the MIT software license, see the accompanying file COPYING or http://www.opensource.org/licenses/mit-license.php." VALUE "OriginalFilename", "bitcoin-cli.exe" VALUE "ProductName", "bitcoin-cli" - VALUE "ProductVersion", VER_PRODUCTVERSION_STR + VALUE "ProductVersion", PACKAGE_VERSION END END 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/bitcoin-tx-res.rc b/src/bitcoin-tx-res.rc index b545ce9dbe..46e4fc9274 100644 --- a/src/bitcoin-tx-res.rc +++ b/src/bitcoin-tx-res.rc @@ -2,9 +2,7 @@ #include "clientversion.h" // holds the needed client version information #define VER_PRODUCTVERSION CLIENT_VERSION_MAJOR,CLIENT_VERSION_MINOR,CLIENT_VERSION_BUILD -#define VER_PRODUCTVERSION_STR STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD) #define VER_FILEVERSION VER_PRODUCTVERSION -#define VER_FILEVERSION_STR VER_PRODUCTVERSION_STR VS_VERSION_INFO VERSIONINFO FILEVERSION VER_FILEVERSION @@ -18,13 +16,13 @@ BEGIN BEGIN VALUE "CompanyName", "Bitcoin" VALUE "FileDescription", "bitcoin-tx (CLI Bitcoin transaction editor utility)" - VALUE "FileVersion", VER_FILEVERSION_STR + VALUE "FileVersion", PACKAGE_VERSION VALUE "InternalName", "bitcoin-tx" VALUE "LegalCopyright", COPYRIGHT_STR VALUE "LegalTrademarks1", "Distributed under the MIT software license, see the accompanying file COPYING or http://www.opensource.org/licenses/mit-license.php." VALUE "OriginalFilename", "bitcoin-tx.exe" VALUE "ProductName", "bitcoin-tx" - VALUE "ProductVersion", VER_PRODUCTVERSION_STR + VALUE "ProductVersion", PACKAGE_VERSION END END diff --git a/src/bitcoin-util-res.rc b/src/bitcoin-util-res.rc index 3f0fa8ab6d..0de8c5befa 100644 --- a/src/bitcoin-util-res.rc +++ b/src/bitcoin-util-res.rc @@ -2,9 +2,7 @@ #include "clientversion.h" // holds the needed client version information #define VER_PRODUCTVERSION CLIENT_VERSION_MAJOR,CLIENT_VERSION_MINOR,CLIENT_VERSION_BUILD -#define VER_PRODUCTVERSION_STR STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD) #define VER_FILEVERSION VER_PRODUCTVERSION -#define VER_FILEVERSION_STR VER_PRODUCTVERSION_STR VS_VERSION_INFO VERSIONINFO FILEVERSION VER_FILEVERSION @@ -18,13 +16,13 @@ BEGIN BEGIN VALUE "CompanyName", "Bitcoin" VALUE "FileDescription", "bitcoin-util (CLI Bitcoin utility)" - VALUE "FileVersion", VER_FILEVERSION_STR + VALUE "FileVersion", PACKAGE_VERSION VALUE "InternalName", "bitcoin-util" VALUE "LegalCopyright", COPYRIGHT_STR VALUE "LegalTrademarks1", "Distributed under the MIT software license, see the accompanying file COPYING or http://www.opensource.org/licenses/mit-license.php." VALUE "OriginalFilename", "bitcoin-util.exe" VALUE "ProductName", "bitcoin-util" - VALUE "ProductVersion", VER_PRODUCTVERSION_STR + VALUE "ProductVersion", PACKAGE_VERSION END END diff --git a/src/bitcoin-wallet-res.rc b/src/bitcoin-wallet-res.rc index 59346ab8f6..d86ffbd9f1 100644 --- a/src/bitcoin-wallet-res.rc +++ b/src/bitcoin-wallet-res.rc @@ -2,9 +2,7 @@ #include "clientversion.h" // holds the needed client version information #define VER_PRODUCTVERSION CLIENT_VERSION_MAJOR,CLIENT_VERSION_MINOR,CLIENT_VERSION_BUILD -#define VER_PRODUCTVERSION_STR STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD) #define VER_FILEVERSION VER_PRODUCTVERSION -#define VER_FILEVERSION_STR VER_PRODUCTVERSION_STR VS_VERSION_INFO VERSIONINFO FILEVERSION VER_FILEVERSION @@ -18,13 +16,13 @@ BEGIN BEGIN VALUE "CompanyName", "Bitcoin" VALUE "FileDescription", "bitcoin-wallet (CLI tool for " PACKAGE_NAME " wallets)" - VALUE "FileVersion", VER_FILEVERSION_STR + VALUE "FileVersion", PACKAGE_VERSION VALUE "InternalName", "bitcoin-wallet" VALUE "LegalCopyright", COPYRIGHT_STR VALUE "LegalTrademarks1", "Distributed under the MIT software license, see the accompanying file COPYING or http://www.opensource.org/licenses/mit-license.php." VALUE "OriginalFilename", "bitcoin-wallet.exe" VALUE "ProductName", "bitcoin-wallet" - VALUE "ProductVersion", VER_PRODUCTVERSION_STR + VALUE "ProductVersion", PACKAGE_VERSION END END diff --git a/src/bitcoind-res.rc b/src/bitcoind-res.rc index a98b50c899..353761dfa7 100644 --- a/src/bitcoind-res.rc +++ b/src/bitcoind-res.rc @@ -2,9 +2,7 @@ #include "clientversion.h" // holds the needed client version information #define VER_PRODUCTVERSION CLIENT_VERSION_MAJOR,CLIENT_VERSION_MINOR,CLIENT_VERSION_BUILD -#define VER_PRODUCTVERSION_STR STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD) #define VER_FILEVERSION VER_PRODUCTVERSION -#define VER_FILEVERSION_STR VER_PRODUCTVERSION_STR VS_VERSION_INFO VERSIONINFO FILEVERSION VER_FILEVERSION @@ -18,13 +16,13 @@ BEGIN BEGIN VALUE "CompanyName", "Bitcoin" VALUE "FileDescription", "bitcoind (Bitcoin node with a JSON-RPC server)" - VALUE "FileVersion", VER_FILEVERSION_STR + VALUE "FileVersion", PACKAGE_VERSION VALUE "InternalName", "bitcoind" VALUE "LegalCopyright", COPYRIGHT_STR VALUE "LegalTrademarks1", "Distributed under the MIT software license, see the accompanying file COPYING or http://www.opensource.org/licenses/mit-license.php." VALUE "OriginalFilename", "bitcoind.exe" VALUE "ProductName", "bitcoind" - VALUE "ProductVersion", VER_PRODUCTVERSION_STR + VALUE "ProductVersion", PACKAGE_VERSION END END 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..f97e4097e8 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 @@ -40,8 +42,6 @@ const std::string CLIENT_NAME("Satoshi"); #endif #endif -const std::string CLIENT_BUILD(BUILD_DESC BUILD_SUFFIX); - static std::string FormatVersion(int nVersion) { return strprintf("%d.%d.%d", nVersion / 10000, (nVersion / 100) % 100, nVersion % 100); @@ -49,6 +49,7 @@ static std::string FormatVersion(int nVersion) std::string FormatFullVersion() { + static const std::string CLIENT_BUILD(BUILD_DESC BUILD_SUFFIX); return CLIENT_BUILD; } diff --git a/src/clientversion.h b/src/clientversion.h index 0ed3f68094..a3e6233437 100644 --- a/src/clientversion.h +++ b/src/clientversion.h @@ -36,7 +36,6 @@ static const int CLIENT_VERSION = + 1 * CLIENT_VERSION_BUILD; extern const std::string CLIENT_NAME; -extern const std::string CLIENT_BUILD; std::string FormatFullVersion(); diff --git a/src/crypto/chacha_poly_aead.cpp b/src/crypto/chacha_poly_aead.cpp index 0582a60c4f..b73b22a2b8 100644 --- a/src/crypto/chacha_poly_aead.cpp +++ b/src/crypto/chacha_poly_aead.cpp @@ -31,8 +31,9 @@ ChaCha20Poly1305AEAD::ChaCha20Poly1305AEAD(const unsigned char* K_1, size_t K_1_ { assert(K_1_len == CHACHA20_POLY1305_AEAD_KEY_LEN); assert(K_2_len == CHACHA20_POLY1305_AEAD_KEY_LEN); - m_chacha_main.SetKey(K_1, CHACHA20_POLY1305_AEAD_KEY_LEN); - m_chacha_header.SetKey(K_2, CHACHA20_POLY1305_AEAD_KEY_LEN); + + m_chacha_header.SetKey(K_1, CHACHA20_POLY1305_AEAD_KEY_LEN); + m_chacha_main.SetKey(K_2, CHACHA20_POLY1305_AEAD_KEY_LEN); // set the cached sequence number to uint64 max which hints for an unset cache. // we can't hit uint64 max since the rekey rule (which resets the sequence number) is 1GB diff --git a/src/init.cpp b/src/init.cpp index aee8b78999..bf027b2879 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); @@ -859,6 +860,11 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(Untranslated("Cannot set -bind or -whitebind together with -listen=0")); } + // if listen=0, then disallow listenonion=1 + if (!args.GetBoolArg("-listen", DEFAULT_LISTEN) && args.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) { + return InitError(Untranslated("Cannot set -listen=0 together with -listenonion=1")); + } + // Make sure enough file descriptors are available int nBind = std::max(nUserBind, size_t(1)); nUserMaxConnections = args.GetArg("-maxconnections", DEFAULT_MAX_PEER_CONNECTIONS); @@ -1164,7 +1170,22 @@ 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); + { + // Load addresses from peers.dat + uiInterface.InitMessage(_("Loading P2P addresses…").translated); + int64_t nStart = GetTimeMillis(); + CAddrDB adb; + if (adb.Read(*node.addrman)) { + LogPrintf("Loaded %i addresses from peers.dat %dms\n", node.addrman->size(), GetTimeMillis() - nStart); + } else { + // Addrman can be in an inconsistent state after failure, reset it + node.addrman = std::make_unique<CAddrMan>(/* deterministic */ false, /* consistency_check_ratio */ check_addrman); + LogPrintf("Recreating peers.dat\n"); + adb.Write(*node.addrman); + } + } assert(!node.banman); node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); @@ -1185,7 +1206,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.peerman); node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(), - *node.scheduler, chainman, *node.mempool, ignores_incoming_txs); + chainman, *node.mempool, ignores_incoming_txs); RegisterValidationInterface(node.peerman.get()); // sanitize comments per BIP-0014, format user agent and check total size @@ -1794,6 +1815,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) banman->DumpBanlist(); }, DUMP_BANS_INTERVAL); + if (node.peerman) node.peerman->StartScheduledTasks(*node.scheduler); + #if HAVE_SYSTEM StartupNotify(args); #endif diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 7cac435e96..eceede3c8f 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -35,7 +35,9 @@ namespace interfaces { class Handler; class Wallet; -//! Helper for findBlock to selectively return pieces of block data. +//! Helper for findBlock to selectively return pieces of block data. If block is +//! found, data will be returned by setting specified output variables. If block +//! is not found, output variables will keep their previous values. class FoundBlock { public: @@ -60,6 +62,7 @@ public: bool* m_in_active_chain = nullptr; const FoundBlock* m_next_block = nullptr; CBlock* m_data = nullptr; + mutable bool found = false; }; //! Interface giving clients (wallet processes, maybe other analysis tools in @@ -262,11 +265,18 @@ public: //! Current RPC serialization flags. virtual int rpcSerializationFlags() = 0; + //! Get settings value. + virtual util::SettingsValue getSetting(const std::string& arg) = 0; + + //! Get list of settings values. + virtual std::vector<util::SettingsValue> getSettingsList(const std::string& arg) = 0; + //! Return <datadir>/settings.json setting value. virtual util::SettingsValue getRwSetting(const std::string& name) = 0; - //! Write a setting to <datadir>/settings.json. - virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0; + //! Write a setting to <datadir>/settings.json. Optionally just update the + //! setting in memory and do not write the file. + virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write=true) = 0; //! Synchronously send transactionAddedToMempool notifications about all //! current mempool transactions to the specified handler and return after diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index fb1febc11b..a85db04b8b 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -332,6 +332,9 @@ public: //! loaded at startup or by RPC. using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>; virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0; + + //! Return pointer to internal context, useful for testing. + virtual WalletContext* context() { return nullptr; } }; //! Information about one wallet address. @@ -410,7 +413,7 @@ struct WalletTxOut //! Return implementation of Wallet interface. This function is defined in //! dummywallet.cpp and throws if the wallet component is not compiled. -std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet); +std::unique_ptr<Wallet> MakeWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet); //! Return implementation of ChainClient interface for a wallet client. This //! function will be undefined in builds where ENABLE_WALLET is false. @@ -133,10 +133,15 @@ public: * optionally tweaked by *merkle_root. Additional nonce entropy can be provided through * aux. * - * When merkle_root is not nullptr, this results in a signature with a modified key as - * specified in BIP341: - * - If merkle_root->IsNull(): key + H_TapTweak(pubkey)*G - * - Otherwise: key + H_TapTweak(pubkey || *merkle_root) + * merkle_root is used to optionally perform tweaking of the private key, as specified + * in BIP341: + * - If merkle_root == nullptr: no tweaking is done, sign with key directly (this is + * used for signatures in BIP342 script). + * - If merkle_root->IsNull(): sign with key + H_TapTweak(pubkey) (this is used for + * key path spending when no scripts are present). + * - Otherwise: sign with key + H_TapTweak(pubkey || *merkle_root) + * (this is used for key path spending, with specific + * Merkle root of the script tree). */ bool SignSchnorr(const uint256& hash, Span<unsigned char> sig, const uint256* merkle_root = nullptr, const uint256* aux = nullptr) const; diff --git a/src/net.cpp b/src/net.cpp index 8ef770ede2..57b8844d6b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1304,8 +1304,9 @@ void CConnman::NotifyNumConnectionsChanged() } if(vNodesSize != nPrevNodeCount) { nPrevNodeCount = vNodesSize; - if(clientInterface) - clientInterface->NotifyNumConnectionsChanged(vNodesSize); + if (m_client_interface) { + m_client_interface->NotifyNumConnectionsChanged(vNodesSize); + } } } @@ -2448,7 +2449,9 @@ void CConnman::SetNetworkActive(bool active) fNetworkActive = active; - uiInterface.NotifyNetworkActiveChanged(fNetworkActive); + if (m_client_interface) { + m_client_interface->NotifyNetworkActiveChanged(fNetworkActive); + } } CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, CAddrMan& addrman_in, bool network_active) @@ -2473,8 +2476,8 @@ bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags } bilingual_str strError; if (!BindListenPort(addr, strError, permissions)) { - if ((flags & BF_REPORT_ERROR) && clientInterface) { - clientInterface->ThreadSafeMessageBox(strError, "", CClientUIInterface::MSG_ERROR); + if ((flags & BF_REPORT_ERROR) && m_client_interface) { + m_client_interface->ThreadSafeMessageBox(strError, "", CClientUIInterface::MSG_ERROR); } return false; } @@ -2513,8 +2516,8 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) Init(connOptions); if (fListen && !InitBinds(connOptions)) { - if (clientInterface) { - clientInterface->ThreadSafeMessageBox( + if (m_client_interface) { + m_client_interface->ThreadSafeMessageBox( _("Failed to listen on any port. Use -listen=0 if you want this."), "", CClientUIInterface::MSG_ERROR); } @@ -2531,22 +2534,6 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) AddAddrFetch(strDest); } - if (clientInterface) { - clientInterface->InitMessage(_("Loading P2P addresses…").translated); - } - // Load addresses from peers.dat - int64_t nStart = GetTimeMillis(); - { - CAddrDB adb; - if (adb.Read(addrman)) - LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman.size(), GetTimeMillis() - nStart); - else { - addrman.Clear(); // Addrman can be in an inconsistent state after failure, reset it - LogPrintf("Recreating peers.dat\n"); - DumpAddresses(); - } - } - if (m_use_addrman_outgoing) { // Load addresses from anchors.dat m_anchors = ReadAnchors(gArgs.GetDataDirNet() / ANCHORS_DATABASE_FILENAME); @@ -2556,7 +2543,9 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) LogPrintf("%i block-relay-only anchors will be tried for connections.\n", m_anchors.size()); } - uiInterface.InitMessage(_("Starting network threads…").translated); + if (m_client_interface) { + m_client_interface->InitMessage(_("Starting network threads…").translated); + } fAddressesInitialized = true; @@ -2594,8 +2583,8 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) threadOpenAddedConnections = std::thread(&util::TraceThread, "addcon", [this] { ThreadOpenAddedConnections(); }); if (connOptions.m_use_addrman_outgoing && !connOptions.m_specified_outgoing.empty()) { - if (clientInterface) { - clientInterface->ThreadSafeMessageBox( + if (m_client_interface) { + m_client_interface->ThreadSafeMessageBox( _("Cannot provide specific connections and have addrman find outgoing connections at the same."), "", CClientUIInterface::MSG_ERROR); } @@ -787,7 +787,7 @@ public: nMaxAddnode = connOptions.nMaxAddnode; nMaxFeeler = connOptions.nMaxFeeler; m_max_outbound = m_max_outbound_full_relay + m_max_outbound_block_relay + nMaxFeeler; - clientInterface = connOptions.uiInterface; + m_client_interface = connOptions.uiInterface; m_banman = connOptions.m_banman; m_msgproc = connOptions.m_msgproc; nSendBufferMaxSize = connOptions.nSendBufferMaxSize; @@ -1126,7 +1126,7 @@ private: int nMaxFeeler; int m_max_outbound; bool m_use_addrman_outgoing; - CClientUIInterface* clientInterface; + CClientUIInterface* m_client_interface; NetEventsInterface* m_msgproc; /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */ BanMan* m_banman; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 005fe1bf0c..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 **/ @@ -292,7 +292,7 @@ class PeerManagerImpl final : public PeerManager { public: PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, - BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs); /** Overridden from CValidationInterface. */ @@ -309,6 +309,7 @@ public: bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing); /** Implement PeerManager */ + void StartScheduledTasks(CScheduler& scheduler) override; void CheckForStaleTipAndEvictPeers() override; bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override; bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; } @@ -652,7 +653,7 @@ private: * @return True if address relay is enabled with peer * False if address relay is disallowed */ - bool SetupAddressRelay(CNode& node, Peer& peer); + bool SetupAddressRelay(const CNode& node, Peer& peer); }; } // namespace @@ -1419,14 +1420,14 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) } std::unique_ptr<PeerManager> PeerManager::make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, - BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs) { - return std::make_unique<PeerManagerImpl>(chainparams, connman, addrman, banman, scheduler, chainman, pool, ignore_incoming_txs); + return std::make_unique<PeerManagerImpl>(chainparams, connman, addrman, banman, chainman, pool, ignore_incoming_txs); } PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, - BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs) : m_chainparams(chainparams), m_connman(connman), @@ -1436,6 +1437,10 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn m_mempool(pool), m_ignore_incoming_txs(ignore_incoming_txs) { +} + +void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler) +{ // Stale tip checking and peer eviction are on two different timers, but we // don't want them to get out of sync due to drift in the scheduler, so we // combine them in one function and schedule at the quicker (peer-eviction) @@ -2843,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 @@ -2875,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; @@ -3744,7 +3746,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - SetupAddressRelay(pfrom, *peer); + // Since this must be an inbound connection, SetupAddressRelay will + // never fail. + Assume(SetupAddressRelay(pfrom, *peer)); // Only send one GetAddr response per connection to reduce resource waste // and discourage addr stamping of INV announcements. @@ -4461,7 +4465,7 @@ public: }; } -bool PeerManagerImpl::SetupAddressRelay(CNode& node, Peer& peer) +bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer) { // We don't participate in addr relay with outbound block-relay-only // connections to prevent providing adversaries with the additional diff --git a/src/net_processing.h b/src/net_processing.h index 4532a0505e..9d8d788583 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -38,10 +38,13 @@ class PeerManager : public CValidationInterface, public NetEventsInterface { public: static std::unique_ptr<PeerManager> make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, - BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, + BanMan* banman, ChainstateManager& chainman, CTxMemPool& pool, bool ignore_incoming_txs); virtual ~PeerManager() { } + /** Begin running background tasks, should only be called once */ + virtual void StartScheduledTasks(CScheduler& scheduler) = 0; + /** Get statistics from node state */ virtual bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 183b5a5d91..b46ad0333e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -334,6 +334,7 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<Rec REVERSE_LOCK(lock); if (!ReadBlockFromDisk(*block.m_data, index, Params().GetConsensus())) block.m_data->SetNull(); } + block.found = true; return true; } @@ -660,6 +661,14 @@ public: RPCRunLater(name, std::move(fn), seconds); } int rpcSerializationFlags() override { return RPCSerializationFlags(); } + util::SettingsValue getSetting(const std::string& name) override + { + return gArgs.GetSetting(name); + } + std::vector<util::SettingsValue> getSettingsList(const std::string& name) override + { + return gArgs.GetSettingsList(name); + } util::SettingsValue getRwSetting(const std::string& name) override { util::SettingsValue result; @@ -670,7 +679,7 @@ public: }); return result; } - bool updateRwSetting(const std::string& name, const util::SettingsValue& value) override + bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write) override { gArgs.LockSettings([&](util::Settings& settings) { if (value.isNull()) { @@ -679,7 +688,7 @@ public: settings.rw_settings[name] = value; } }); - return gArgs.WriteSettingsFile(); + return !write || gArgs.WriteSettingsFile(); } void requestMempoolTransactions(Notifications& notifications) override { diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 8ede7b9974..b5f1df9792 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -13,6 +13,7 @@ #include <util/vector.h> #include <assert.h> +#include <optional> #include <string> static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy"; @@ -20,22 +21,18 @@ static const std::string OUTPUT_TYPE_STRING_P2SH_SEGWIT = "p2sh-segwit"; static const std::string OUTPUT_TYPE_STRING_BECH32 = "bech32"; static const std::string OUTPUT_TYPE_STRING_BECH32M = "bech32m"; -bool ParseOutputType(const std::string& type, OutputType& output_type) +std::optional<OutputType> ParseOutputType(const std::string& type) { if (type == OUTPUT_TYPE_STRING_LEGACY) { - output_type = OutputType::LEGACY; - return true; + return OutputType::LEGACY; } else if (type == OUTPUT_TYPE_STRING_P2SH_SEGWIT) { - output_type = OutputType::P2SH_SEGWIT; - return true; + return OutputType::P2SH_SEGWIT; } else if (type == OUTPUT_TYPE_STRING_BECH32) { - output_type = OutputType::BECH32; - return true; + return OutputType::BECH32; } else if (type == OUTPUT_TYPE_STRING_BECH32M) { - output_type = OutputType::BECH32M; - return true; + return OutputType::BECH32M; } - return false; + return std::nullopt; } const std::string& FormatOutputType(OutputType type) diff --git a/src/outputtype.h b/src/outputtype.h index 2b83235cd0..0de7689125 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -11,6 +11,7 @@ #include <script/standard.h> #include <array> +#include <optional> #include <string> #include <vector> @@ -28,7 +29,7 @@ static constexpr auto OUTPUT_TYPES = std::array{ OutputType::BECH32M, }; -[[nodiscard]] bool ParseOutputType(const std::string& str, OutputType& output_type); +std::optional<OutputType> ParseOutputType(const std::string& str); const std::string& FormatOutputType(OutputType type); /** 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 f8aeb01659..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 @@ -682,8 +685,6 @@ void BitcoinGUI::addWallet(WalletModel* walletModel) m_wallet_selector_label_action->setVisible(true); m_wallet_selector_action->setVisible(true); } - const QString display_name = walletModel->getDisplayName(); - m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel)); connect(wallet_view, &WalletView::outOfSyncWarningClicked, this, &BitcoinGUI::showModalOverlay); connect(wallet_view, &WalletView::transactionClicked, this, &BitcoinGUI::gotoHistoryPage); @@ -696,6 +697,8 @@ void BitcoinGUI::addWallet(WalletModel* walletModel) connect(wallet_view, &WalletView::hdEnabledStatusChanged, this, &BitcoinGUI::updateWalletStatus); connect(this, &BitcoinGUI::setPrivacy, wallet_view, &WalletView::setPrivacy); wallet_view->setPrivacy(isPrivacyModeActivated()); + const QString display_name = walletModel->getDisplayName(); + m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel)); } void BitcoinGUI::removeWallet(WalletModel* walletModel) 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/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 39c69fe184..022f367422 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -109,9 +109,10 @@ void TestAddAddressesToSendBook(interfaces::Node& node) std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other")); OptionsModel optionsModel; ClientModel clientModel(node, &optionsModel); - AddWallet(wallet); - WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel, platformStyle.get()); - RemoveWallet(wallet, std::nullopt); + WalletContext& context = *node.walletClient().context(); + AddWallet(context, wallet); + WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel, platformStyle.get()); + RemoveWallet(context, wallet, /* load_on_startup= */ std::nullopt); EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress); editAddressDialog.setModel(walletModel.getAddressTableModel()); diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 9c31cd50df..8489b33144 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -12,7 +12,6 @@ #include <qt/rpcconsole.h> #include <shutdown.h> #include <test/util/setup_common.h> -#include <univalue.h> #include <validation.h> #if defined(HAVE_CONFIG_H) @@ -21,8 +20,10 @@ #include <QAction> #include <QLineEdit> +#include <QRegularExpression> #include <QScopedPointer> #include <QSignalSpy> +#include <QString> #include <QTest> #include <QTextEdit> #include <QtGlobal> @@ -30,6 +31,13 @@ #include <QtTest/QtTestGui> namespace { +//! Regex find a string group inside of the console output +QString FindInConsole(const QString& output, const QString& pattern) +{ + const QRegularExpression re(pattern); + return re.match(output).captured(1); +} + //! Call getblockchaininfo RPC and check first field of JSON output. void TestRpcCommand(RPCConsole* console) { @@ -41,10 +49,9 @@ void TestRpcCommand(RPCConsole* console) QTest::keyClick(lineEdit, Qt::Key_Return); QVERIFY(mw_spy.wait(1000)); QCOMPARE(mw_spy.count(), 4); - QString output = messagesWidget->toPlainText(); - UniValue value; - value.read(output.right(output.size() - output.lastIndexOf(QChar::ObjectReplacementCharacter) - 1).toStdString()); - QCOMPARE(value["chain"].get_str(), std::string("regtest")); + const QString output = messagesWidget->toPlainText(); + const QString pattern = QStringLiteral("\"chain\": \"(\\w+)\""); + QCOMPARE(FindInConsole(output, pattern), QString("regtest")); } } // namespace diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index e883337fb5..1976bee74b 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -164,9 +164,10 @@ void TestGUI(interfaces::Node& node) TransactionView transactionView(platformStyle.get()); OptionsModel optionsModel; ClientModel clientModel(node, &optionsModel); - AddWallet(wallet); - WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel, platformStyle.get()); - RemoveWallet(wallet, std::nullopt); + WalletContext& context = *node.walletClient().context(); + AddWallet(context, wallet); + WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel, platformStyle.get()); + RemoveWallet(context, wallet, /* load_on_startup= */ std::nullopt); sendCoinsDialog.setModel(&walletModel); transactionView.setModel(&walletModel); 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/misc.cpp b/src/rpc/misc.cpp index 5178ce60e8..1a94abf6d3 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -24,6 +24,7 @@ #include <util/strencodings.h> #include <util/system.h> +#include <optional> #include <stdint.h> #include <tuple> #ifdef HAVE_MALLOC_INFO @@ -128,12 +129,13 @@ static RPCHelpMan createmultisig() // Get the output type OutputType output_type = OutputType::LEGACY; if (!request.params[2].isNull()) { - if (!ParseOutputType(request.params[2].get_str(), output_type)) { + std::optional<OutputType> parsed = ParseOutputType(request.params[2].get_str()); + if (!parsed) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[2].get_str())); - } - if (output_type == OutputType::BECH32M) { + } else if (parsed.value() == OutputType::BECH32M) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "createmultisig cannot create bech32m multisig addresses"); } + output_type = parsed.value(); } // Construct using pay-to-script-hash: diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 3962a13924..861b889118 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -118,7 +118,6 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::STR, "addr", "(host:port) The IP address and port of the peer"}, {RPCResult::Type::STR, "addrbind", "(ip:port) Bind address of the connection to the peer"}, {RPCResult::Type::STR, "addrlocal", "(ip:port) Local address as reported by the peer"}, - {RPCResult::Type::BOOL, "addr_relay_enabled", "Whether we participate in address relay with this peer"}, {RPCResult::Type::STR, "network", "Network (" + Join(GetNetworkNames(/* append_unroutable */ true), ", ") + ")"}, {RPCResult::Type::NUM, "mapped_as", "The AS in the BGP route to the peer used for diversifying\n" "peer selection (only available if the asmap config flag is set)"}, @@ -151,6 +150,9 @@ 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"}, @@ -202,7 +204,6 @@ static RPCHelpMan getpeerinfo() if (!(stats.addrLocal.empty())) { obj.pushKV("addrlocal", stats.addrLocal); } - obj.pushKV("addr_relay_enabled", statestats.m_addr_relay_enabled); obj.pushKV("network", GetNetworkName(stats.m_network)); if (stats.m_mapped_as != 0) { obj.pushKV("mapped_as", uint64_t(stats.m_mapped_as)); @@ -244,6 +245,7 @@ static RPCHelpMan getpeerinfo() heights.push_back(height); } obj.pushKV("inflight", heights); + obj.pushKV("addr_relay_enabled", statestats.m_addr_relay_enabled); obj.pushKV("addr_processed", statestats.m_addr_processed); obj.pushKV("addr_rate_limited", statestats.m_addr_rate_limited); } @@ -949,7 +951,7 @@ static RPCHelpMan addpeeraddress() address.nTime = GetAdjustedTime(); // The source address is set equal to the address. This is equivalent to the peer // announcing itself. - if (node.addrman->Add(address, address)) success = true; + if (node.addrman->Add({address}, address)) success = true; } obj.pushKV("success", success); 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/interpreter.cpp b/src/script/interpreter.cpp index dd7c0a4a05..eafa9840d7 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1874,9 +1874,9 @@ static bool VerifyTaprootCommitment(const std::vector<unsigned char>& control, c assert(control.size() >= TAPROOT_CONTROL_BASE_SIZE); assert(program.size() >= uint256::size()); //! The internal pubkey (x-only, so no Y coordinate parity). - const XOnlyPubKey p{uint256(std::vector<unsigned char>(control.begin() + 1, control.begin() + TAPROOT_CONTROL_BASE_SIZE))}; + const XOnlyPubKey p{Span<const unsigned char>{control.data() + 1, control.data() + TAPROOT_CONTROL_BASE_SIZE}}; //! The output pubkey (taken from the scriptPubKey). - const XOnlyPubKey q{uint256(program)}; + const XOnlyPubKey q{program}; // Compute the Merkle root from the leaf and the provided path. const uint256 merkle_root = ComputeTaprootMerkleRoot(control, tapleaf_hash); // Verify that the output pubkey matches the tweaked internal pubkey, after correcting for parity. diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 93136a0b79..ab49e84577 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -170,6 +170,13 @@ struct PrecomputedTransactionData PrecomputedTransactionData() = default; + /** Initialize this PrecomputedTransactionData with transaction data. + * + * @param[in] tx The transaction for which data is being precomputed. + * @param[in] spent_outputs The CTxOuts being spent, one for each tx.vin, in order. + * @param[in] force Whether to precompute data for all optional features, + * regardless of what is in the inputs (used at signing + * time, when the inputs aren't filled in yet). */ template <class T> void Init(const T& tx, std::vector<CTxOut>&& spent_outputs, bool force = false); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 7864e690d8..4714d0ef11 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; @@ -60,7 +61,7 @@ bool MutableTransactionSignatureCreator::CreateSchnorrSig(const SigningProvider& CKey key; { - // For now, use the old full pubkey-based key derivation logic. As it indexed by + // For now, use the old full pubkey-based key derivation logic. As it is indexed by // Hash160(full pubkey), we need to try both a version prefixed with 0x02, and one // with 0x03. unsigned char b[33] = {0x02}; @@ -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); @@ -639,29 +640,26 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, PrecomputedTransactionData txdata; std::vector<CTxOut> spent_outputs; - spent_outputs.resize(mtx.vin.size()); - bool have_all_spent_outputs = true; - for (unsigned int i = 0; i < mtx.vin.size(); i++) { + for (unsigned int i = 0; i < mtx.vin.size(); ++i) { CTxIn& txin = mtx.vin[i]; auto coin = coins.find(txin.prevout); if (coin == coins.end() || coin->second.IsSpent()) { - have_all_spent_outputs = false; + txdata.Init(txConst, /* spent_outputs */ {}, /* force */ true); + break; } else { - spent_outputs[i] = CTxOut(coin->second.out.nValue, coin->second.out.scriptPubKey); + spent_outputs.emplace_back(coin->second.out.nValue, coin->second.out.scriptPubKey); } } - if (have_all_spent_outputs) { + if (spent_outputs.size() == mtx.vin.size()) { txdata.Init(txConst, std::move(spent_outputs), true); - } else { - txdata.Init(txConst, {}, true); } // Sign what we can: - for (unsigned int i = 0; i < mtx.vin.size(); i++) { + for (unsigned int i = 0; i < mtx.vin.size(); ++i) { 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 +675,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 +683,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..6d3479c143 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. */ @@ -44,8 +45,8 @@ class MutableTransactionSignatureCreator : public BaseSignatureCreator { const PrecomputedTransactionData* m_txdata; public: - MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn = SIGHASH_ALL); - MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData* txdata, int nHashTypeIn = SIGHASH_ALL); + MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn); + MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData* txdata, int nHashTypeIn); const BaseSignatureChecker& Checker() const override { return checker; } bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override; bool CreateSchnorrSig(const SigningProvider& provider, std::vector<unsigned char>& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const override; @@ -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/script/standard.cpp b/src/script/standard.cpp index b8349bb9ab..67a79a157c 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -504,6 +504,7 @@ WitnessV1Taproot TaprootBuilder::GetOutput() { return WitnessV1Taproot{m_output_ TaprootSpendData TaprootBuilder::GetSpendData() const { + assert(IsComplete()); TaprootSpendData spd; spd.merkle_root = m_branch.size() == 0 ? uint256() : m_branch[0]->hash; spd.internal_key = m_internal_key; diff --git a/src/script/standard.h b/src/script/standard.h index ac4e2f3276..78492733db 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -227,8 +227,11 @@ struct TaprootSpendData /** The Merkle root of the script tree (0 if no scripts). */ uint256 merkle_root; /** Map from (script, leaf_version) to (sets of) control blocks. - * The control blocks are sorted by size, so that the signing logic can - * easily prefer the cheapest one. */ + * More than one control block for a given script is only possible if it + * appears in multiple branches of the tree. We keep them all so that + * inference can reconstruct the full tree. Within each set, the control + * blocks are sorted by size, so that the signing logic can easily + * prefer the cheapest one. */ std::map<std::pair<CScript, int>, std::set<std::vector<unsigned char>, ShortestVectorFirstComparator>> scripts; /** Merge other TaprootSpendData (for the same scriptPubKey) into this. */ void Merge(TaprootSpendData other); @@ -252,7 +255,7 @@ private: /** Merkle hash of this node. */ uint256 hash; /** Tracked leaves underneath this node (either from the node itself, or its children). - * The merkle_branch field for each is the partners to get to *this* node. */ + * The merkle_branch field of each is the partners to get to *this* node. */ std::vector<LeafInfo> leaves; }; /** Whether the builder is in a valid state so far. */ diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 79c7102c4f..cd5dc2370f 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -1,7 +1,10 @@ // Copyright (c) 2012-2020 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <addrdb.h> #include <addrman.h> +#include <chainparams.h> #include <test/data/asmap.raw.h> #include <test/util/setup_common.h> #include <util/asmap.h> @@ -15,30 +18,76 @@ #include <optional> #include <string> +using namespace std::literals; + +class CAddrManSerializationMock : public CAddrMan +{ +public: + virtual void Serialize(CDataStream& s) const = 0; + + CAddrManSerializationMock() + : CAddrMan(/* deterministic */ true, /* consistency_check_ratio */ 100) + {} +}; + +class CAddrManUncorrupted : public CAddrManSerializationMock +{ +public: + void Serialize(CDataStream& s) const override + { + CAddrMan::Serialize(s); + } +}; + +class CAddrManCorrupted : public CAddrManSerializationMock +{ +public: + void Serialize(CDataStream& s) const override + { + // Produces corrupt output that claims addrman has 20 addrs when it only has one addr. + unsigned char nVersion = 1; + s << nVersion; + s << ((unsigned char)32); + s << uint256::ONE; + s << 10; // nNew + s << 10; // nTried + + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); + s << nUBuckets; + + CService serv; + BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); + CAddress addr = CAddress(serv, NODE_NONE); + CNetAddr resolved; + BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); + CAddrInfo info = CAddrInfo(addr, resolved); + s << info; + } +}; + +static CDataStream AddrmanToStream(const CAddrManSerializationMock& _addrman) +{ + CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); + ssPeersIn << Params().MessageStart(); + ssPeersIn << _addrman; + std::string str = ssPeersIn.str(); + std::vector<unsigned char> vchData(str.begin(), str.end()); + return CDataStream(vchData, SER_DISK, CLIENT_VERSION); +} + class CAddrManTest : public CAddrMan { 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); @@ -83,16 +132,6 @@ public: int64_t nLastTry = GetAdjustedTime()-61; Attempt(addr, count_failure, nLastTry); } - - void Clear() - { - CAddrMan::Clear(); - if (deterministic) { - LOCK(cs); - nKey.SetNull(); - insecure_rand = FastRandomContext(true); - } - } }; static CNetAddr ResolveIP(const std::string& ip) @@ -126,27 +165,27 @@ BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(addrman_simple) { - CAddrManTest addrman; + auto addrman = std::make_unique<CAddrManTest>(); CNetAddr source = ResolveIP("252.2.2.2"); // Test: Does Addrman respond correctly when empty. - BOOST_CHECK_EQUAL(addrman.size(), 0U); - CAddrInfo addr_null = addrman.Select(); + BOOST_CHECK_EQUAL(addrman->size(), 0U); + CAddrInfo addr_null = addrman->Select(); BOOST_CHECK_EQUAL(addr_null.ToString(), "[::]:0"); // Test: Does Addrman::Add work as expected. CService addr1 = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 1U); - CAddrInfo addr_ret1 = addrman.Select(); + BOOST_CHECK(addrman->Add({CAddress(addr1, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), 1U); + CAddrInfo addr_ret1 = addrman->Select(); BOOST_CHECK_EQUAL(addr_ret1.ToString(), "250.1.1.1:8333"); // Test: Does IP address deduplication work correctly. // Expected dup IP should not be added. CService addr1_dup = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(!addrman.Add(CAddress(addr1_dup, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 1U); + BOOST_CHECK(!addrman->Add({CAddress(addr1_dup, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman->size(), 1U); // Test: New table has one addr and we add a diff addr we should @@ -156,21 +195,16 @@ BOOST_AUTO_TEST_CASE(addrman_simple) // success. CService addr2 = ResolveService("250.1.1.2", 8333); - BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source)); - BOOST_CHECK(addrman.size() >= 1); - - // Test: AddrMan::Clear() should empty the new table. - addrman.Clear(); - BOOST_CHECK_EQUAL(addrman.size(), 0U); - CAddrInfo addr_null2 = addrman.Select(); - BOOST_CHECK_EQUAL(addr_null2.ToString(), "[::]:0"); + BOOST_CHECK(addrman->Add({CAddress(addr2, NODE_NONE)}, source)); + BOOST_CHECK(addrman->size() >= 1); - // Test: AddrMan::Add multiple addresses works as expected + // Test: reset addrman and test AddrMan::Add multiple addresses works as expected + addrman = std::make_unique<CAddrManTest>(); std::vector<CAddress> vAddr; vAddr.push_back(CAddress(ResolveService("250.1.1.3", 8333), NODE_NONE)); vAddr.push_back(CAddress(ResolveService("250.1.1.4", 8333), NODE_NONE)); - BOOST_CHECK(addrman.Add(vAddr, source)); - BOOST_CHECK(addrman.size() >= 1); + BOOST_CHECK(addrman->Add(vAddr, source)); + BOOST_CHECK(addrman->size() >= 1); } BOOST_AUTO_TEST_CASE(addrman_ports) @@ -183,11 +217,11 @@ BOOST_AUTO_TEST_CASE(addrman_ports) // Test 7; Addr with same IP but diff port does not replace existing addr. CService addr1 = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); + BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); BOOST_CHECK_EQUAL(addrman.size(), 1U); CService addr1_port = ResolveService("250.1.1.1", 8334); - BOOST_CHECK(!addrman.Add(CAddress(addr1_port, NODE_NONE), source)); + BOOST_CHECK(!addrman.Add({CAddress(addr1_port, NODE_NONE)}, source)); BOOST_CHECK_EQUAL(addrman.size(), 1U); CAddrInfo addr_ret2 = addrman.Select(); BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333"); @@ -210,7 +244,7 @@ BOOST_AUTO_TEST_CASE(addrman_select) // Test: Select from new with 1 addr in new. CService addr1 = ResolveService("250.1.1.1", 8333); - BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); + BOOST_CHECK(addrman.Add({CAddress(addr1, NODE_NONE)}, source)); BOOST_CHECK_EQUAL(addrman.size(), 1U); bool newOnly = true; @@ -234,20 +268,20 @@ BOOST_AUTO_TEST_CASE(addrman_select) CService addr3 = ResolveService("250.3.2.2", 9999); CService addr4 = ResolveService("250.3.3.3", 9999); - BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), ResolveService("250.3.1.1", 8333))); - BOOST_CHECK(addrman.Add(CAddress(addr3, NODE_NONE), ResolveService("250.3.1.1", 8333))); - BOOST_CHECK(addrman.Add(CAddress(addr4, NODE_NONE), ResolveService("250.4.1.1", 8333))); + BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman.Add({CAddress(addr3, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman.Add({CAddress(addr4, NODE_NONE)}, ResolveService("250.4.1.1", 8333))); // Add three addresses to tried table. CService addr5 = ResolveService("250.4.4.4", 8333); CService addr6 = ResolveService("250.4.5.5", 7777); CService addr7 = ResolveService("250.4.6.6", 8333); - BOOST_CHECK(addrman.Add(CAddress(addr5, NODE_NONE), ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman.Add({CAddress(addr5, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); addrman.Good(CAddress(addr5, NODE_NONE)); - BOOST_CHECK(addrman.Add(CAddress(addr6, NODE_NONE), ResolveService("250.3.1.1", 8333))); + BOOST_CHECK(addrman.Add({CAddress(addr6, NODE_NONE)}, ResolveService("250.3.1.1", 8333))); addrman.Good(CAddress(addr6, NODE_NONE)); - BOOST_CHECK(addrman.Add(CAddress(addr7, NODE_NONE), ResolveService("250.1.1.3", 8333))); + BOOST_CHECK(addrman.Add({CAddress(addr7, NODE_NONE)}, ResolveService("250.1.1.3", 8333))); addrman.Good(CAddress(addr7, NODE_NONE)); // Test: 6 addrs + 1 addr from last test = 7. @@ -267,24 +301,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)); - BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); + 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"); - BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 17U); - - CService addr2 = ResolveService("250.1.1.19"); - BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 18U); + 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(), num_addrs - collisions); + + CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); + BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); } BOOST_AUTO_TEST_CASE(addrman_tried_collisions) @@ -293,25 +330,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}; - for (unsigned int i = 1; i < 80; i++) { - CService addr = ResolveService("250.1.1." + ToString(i)); - BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs); + + 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"); - BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 79U); - - CService addr2 = ResolveService("250.1.1.81"); - BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source)); - BOOST_CHECK_EQUAL(addrman.size(), 80U); + 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(), num_addrs - collisions); + + CService addr2 = ResolveService("250.1.1." + ToString(++num_addrs)); + BOOST_CHECK(addrman.Add({CAddress(addr2, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman.size(), num_addrs - collisions); } BOOST_AUTO_TEST_CASE(addrman_find) @@ -327,9 +367,9 @@ BOOST_AUTO_TEST_CASE(addrman_find) CNetAddr source1 = ResolveIP("250.1.2.1"); CNetAddr source2 = ResolveIP("250.1.2.2"); - BOOST_CHECK(addrman.Add(addr1, source1)); - BOOST_CHECK(!addrman.Add(addr2, source2)); - BOOST_CHECK(addrman.Add(addr3, source1)); + BOOST_CHECK(addrman.Add({addr1}, source1)); + BOOST_CHECK(!addrman.Add({addr2}, source2)); + BOOST_CHECK(addrman.Add({addr3}, source1)); // Test: ensure Find returns an IP matching what we searched on. CAddrInfo* info1 = addrman.Find(addr1); @@ -411,11 +451,8 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) CNetAddr source2 = ResolveIP("250.2.3.3"); // Test: Ensure GetAddr works with new addresses. - BOOST_CHECK(addrman.Add(addr1, source1)); - BOOST_CHECK(addrman.Add(addr2, source2)); - BOOST_CHECK(addrman.Add(addr3, source1)); - BOOST_CHECK(addrman.Add(addr4, source2)); - BOOST_CHECK(addrman.Add(addr5, source1)); + BOOST_CHECK(addrman.Add({addr1, addr3, addr5}, source1)); + BOOST_CHECK(addrman.Add({addr2, addr4}, source2)); BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U); // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down. @@ -436,7 +473,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) // Ensure that for all addrs in addrman, isTerrible == false. addr.nTime = GetAdjustedTime(); - addrman.Add(addr, ResolveIP(strAddr)); + addrman.Add({addr}, ResolveIP(strAddr)); if (i % 8 == 0) addrman.Good(addr); } @@ -722,23 +759,23 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) { std::vector<bool> asmap1 = FromBytes(asmap_raw, sizeof(asmap_raw) * 8); - CAddrManTest addrman_asmap1(true, asmap1); - CAddrManTest addrman_asmap1_dup(true, asmap1); - CAddrManTest addrman_noasmap; + auto addrman_asmap1 = std::make_unique<CAddrManTest>(true, asmap1); + auto addrman_asmap1_dup = std::make_unique<CAddrManTest>(true, asmap1); + auto addrman_noasmap = std::make_unique<CAddrManTest>(); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); CAddress addr = CAddress(ResolveService("250.1.1.1"), NODE_NONE); CNetAddr default_source; - addrman_asmap1.Add(addr, default_source); + addrman_asmap1->Add({addr}, default_source); - stream << addrman_asmap1; + stream << *addrman_asmap1; // serizalizing/deserializing addrman with the same asmap - stream >> addrman_asmap1_dup; + stream >> *addrman_asmap1_dup; - std::pair<int, int> bucketAndEntry_asmap1 = addrman_asmap1.GetBucketAndEntry(addr); - std::pair<int, int> bucketAndEntry_asmap1_dup = addrman_asmap1_dup.GetBucketAndEntry(addr); + std::pair<int, int> bucketAndEntry_asmap1 = addrman_asmap1->GetBucketAndEntry(addr); + std::pair<int, int> bucketAndEntry_asmap1_dup = addrman_asmap1_dup->GetBucketAndEntry(addr); BOOST_CHECK(bucketAndEntry_asmap1.second != -1); BOOST_CHECK(bucketAndEntry_asmap1_dup.second != -1); @@ -746,40 +783,39 @@ BOOST_AUTO_TEST_CASE(addrman_serialization) BOOST_CHECK(bucketAndEntry_asmap1.second == bucketAndEntry_asmap1_dup.second); // deserializing asmaped peers.dat to non-asmaped addrman - stream << addrman_asmap1; - stream >> addrman_noasmap; - std::pair<int, int> bucketAndEntry_noasmap = addrman_noasmap.GetBucketAndEntry(addr); + stream << *addrman_asmap1; + stream >> *addrman_noasmap; + std::pair<int, int> bucketAndEntry_noasmap = addrman_noasmap->GetBucketAndEntry(addr); BOOST_CHECK(bucketAndEntry_noasmap.second != -1); BOOST_CHECK(bucketAndEntry_asmap1.first != bucketAndEntry_noasmap.first); BOOST_CHECK(bucketAndEntry_asmap1.second != bucketAndEntry_noasmap.second); // deserializing non-asmaped peers.dat to asmaped addrman - addrman_asmap1.Clear(); - addrman_noasmap.Clear(); - addrman_noasmap.Add(addr, default_source); - stream << addrman_noasmap; - stream >> addrman_asmap1; - std::pair<int, int> bucketAndEntry_asmap1_deser = addrman_asmap1.GetBucketAndEntry(addr); + addrman_asmap1 = std::make_unique<CAddrManTest>(true, asmap1); + addrman_noasmap = std::make_unique<CAddrManTest>(); + addrman_noasmap->Add({addr}, default_source); + stream << *addrman_noasmap; + stream >> *addrman_asmap1; + std::pair<int, int> bucketAndEntry_asmap1_deser = addrman_asmap1->GetBucketAndEntry(addr); BOOST_CHECK(bucketAndEntry_asmap1_deser.second != -1); BOOST_CHECK(bucketAndEntry_asmap1_deser.first != bucketAndEntry_noasmap.first); BOOST_CHECK(bucketAndEntry_asmap1_deser.first == bucketAndEntry_asmap1_dup.first); BOOST_CHECK(bucketAndEntry_asmap1_deser.second == bucketAndEntry_asmap1_dup.second); // used to map to different buckets, now maps to the same bucket. - addrman_asmap1.Clear(); - addrman_noasmap.Clear(); + addrman_asmap1 = std::make_unique<CAddrManTest>(true, asmap1); + addrman_noasmap = std::make_unique<CAddrManTest>(); CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE); - addrman_noasmap.Add(addr, default_source); - addrman_noasmap.Add(addr2, default_source); - std::pair<int, int> bucketAndEntry_noasmap_addr1 = addrman_noasmap.GetBucketAndEntry(addr1); - std::pair<int, int> bucketAndEntry_noasmap_addr2 = addrman_noasmap.GetBucketAndEntry(addr2); + addrman_noasmap->Add({addr, addr2}, default_source); + std::pair<int, int> bucketAndEntry_noasmap_addr1 = addrman_noasmap->GetBucketAndEntry(addr1); + std::pair<int, int> bucketAndEntry_noasmap_addr2 = addrman_noasmap->GetBucketAndEntry(addr2); BOOST_CHECK(bucketAndEntry_noasmap_addr1.first != bucketAndEntry_noasmap_addr2.first); BOOST_CHECK(bucketAndEntry_noasmap_addr1.second != bucketAndEntry_noasmap_addr2.second); - stream << addrman_noasmap; - stream >> addrman_asmap1; - std::pair<int, int> bucketAndEntry_asmap1_deser_addr1 = addrman_asmap1.GetBucketAndEntry(addr1); - std::pair<int, int> bucketAndEntry_asmap1_deser_addr2 = addrman_asmap1.GetBucketAndEntry(addr2); + stream << *addrman_noasmap; + stream >> *addrman_asmap1; + std::pair<int, int> bucketAndEntry_asmap1_deser_addr1 = addrman_asmap1->GetBucketAndEntry(addr1); + std::pair<int, int> bucketAndEntry_asmap1_deser_addr2 = addrman_asmap1->GetBucketAndEntry(addr2); BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.first == bucketAndEntry_asmap1_deser_addr2.first); BOOST_CHECK(bucketAndEntry_asmap1_deser_addr1.second != bucketAndEntry_asmap1_deser_addr2.second); } @@ -788,7 +824,7 @@ BOOST_AUTO_TEST_CASE(remove_invalid) { // Confirm that invalid addresses are ignored in unserialization. - CAddrManTest addrman; + auto addrman = std::make_unique<CAddrManTest>(); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); const CAddress new1{ResolveService("5.5.5.5"), NODE_NONE}; @@ -796,12 +832,12 @@ BOOST_AUTO_TEST_CASE(remove_invalid) const CAddress tried1{ResolveService("7.7.7.7"), NODE_NONE}; const CAddress tried2{ResolveService("8.8.8.8"), NODE_NONE}; - addrman.Add({new1, tried1, new2, tried2}, CNetAddr{}); - addrman.Good(tried1); - addrman.Good(tried2); - BOOST_REQUIRE_EQUAL(addrman.size(), 4); + addrman->Add({new1, tried1, new2, tried2}, CNetAddr{}); + addrman->Good(tried1); + addrman->Good(tried2); + BOOST_REQUIRE_EQUAL(addrman->size(), 4); - stream << addrman; + stream << *addrman; const std::string str{stream.str()}; size_t pos; @@ -820,9 +856,9 @@ BOOST_AUTO_TEST_CASE(remove_invalid) BOOST_REQUIRE(pos + sizeof(tried2_raw_replacement) <= stream.size()); memcpy(stream.data() + pos, tried2_raw_replacement, sizeof(tried2_raw_replacement)); - addrman.Clear(); - stream >> addrman; - BOOST_CHECK_EQUAL(addrman.size(), 2); + addrman = std::make_unique<CAddrManTest>(); + stream >> *addrman; + BOOST_CHECK_EQUAL(addrman->size(), 2); } BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) @@ -838,7 +874,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) CNetAddr source = ResolveIP("252.2.2.2"); for (unsigned int i = 1; i < 23; i++) { CService addr = ResolveService("250.1.1."+ToString(i)); - BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); + BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); addrman.Good(addr); // No collisions yet. @@ -861,11 +897,11 @@ 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)); + BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); addrman.Good(addr); // No collision yet. @@ -873,22 +909,22 @@ 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)); + BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); addrman.Good(addr); BOOST_CHECK(addrman.size() == i); @@ -896,17 +932,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,11 +958,11 @@ 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)); + BOOST_CHECK(addrman.Add({CAddress(addr, NODE_NONE)}, source)); addrman.Good(addr); // No collision yet. @@ -934,38 +970,111 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0"); } - // Collision between 23 and 19. - CService addr = ResolveService("250.1.1.23"); - BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source)); + // 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. - BOOST_CHECK(!addrman.Add(CAddress(addr, NODE_NONE), source)); + // 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)); + 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"); } +BOOST_AUTO_TEST_CASE(caddrdb_read) +{ + CAddrManUncorrupted addrmanUncorrupted; + + CService addr1, addr2, addr3; + BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); + BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false)); + BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false)); + BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false)); + BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false)); + + // Add three addresses to new table. + CService source; + BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false)); + std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)}; + BOOST_CHECK(addrmanUncorrupted.Add(addresses, source)); + BOOST_CHECK(addrmanUncorrupted.size() == 3); + + // Test that the de-serialization does not throw an exception. + CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); + bool exceptionThrown = false; + CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100); + + BOOST_CHECK(addrman1.size() == 0); + try { + unsigned char pchMsgTmp[4]; + ssPeers1 >> pchMsgTmp; + ssPeers1 >> addrman1; + } catch (const std::exception&) { + exceptionThrown = true; + } + + BOOST_CHECK(addrman1.size() == 3); + BOOST_CHECK(exceptionThrown == false); + + // Test that CAddrDB::Read creates an addrman with the correct number of addrs. + CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); + + CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100); + BOOST_CHECK(addrman2.size() == 0); + BOOST_CHECK(CAddrDB::Read(addrman2, ssPeers2)); + BOOST_CHECK(addrman2.size() == 3); +} + + +BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) +{ + CAddrManCorrupted addrmanCorrupted; + + // Test that the de-serialization of corrupted addrman throws an exception. + CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); + bool exceptionThrown = false; + CAddrMan addrman1(/* deterministic */ false, /* consistency_check_ratio */ 100); + BOOST_CHECK(addrman1.size() == 0); + try { + unsigned char pchMsgTmp[4]; + ssPeers1 >> pchMsgTmp; + ssPeers1 >> addrman1; + } catch (const std::exception&) { + exceptionThrown = true; + } + // Even through de-serialization failed addrman is not left in a clean state. + BOOST_CHECK(addrman1.size() == 1); + BOOST_CHECK(exceptionThrown); + + // Test that CAddrDB::Read fails if peers.dat is corrupt + CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); + + CAddrMan addrman2(/* deterministic */ false, /* consistency_check_ratio */ 100); + BOOST_CHECK(addrman2.size() == 0); + BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index edec5f0a31..5b3b39fdb8 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -617,7 +617,7 @@ static void TestChaCha20Poly1305AEAD(bool must_succeed, unsigned int expected_aa ChaCha20Poly1305AEAD aead(aead_K_1.data(), aead_K_1.size(), aead_K_2.data(), aead_K_2.size()); // create a chacha20 instance to compare against - ChaCha20 cmp_ctx(aead_K_2.data(), 32); + ChaCha20 cmp_ctx(aead_K_1.data(), 32); // encipher bool res = aead.Crypt(seqnr_payload, seqnr_aad, aad_pos, ciphertext_buf.data(), ciphertext_buf.size(), plaintext_buf.data(), plaintext_buf.size(), true); @@ -708,8 +708,8 @@ BOOST_AUTO_TEST_CASE(chacha20_poly1305_aead_testvector) "b1a03d5bd2855d60699e7d3a3133fa47be740fe4e4c1f967555e2d9271f31c3a8bd94d54b5ecabbc41ffbb0c90924080"); TestChaCha20Poly1305AEAD(true, 255, "ff0000f195e66982105ffb640bb7757f579da31602fc93ec01ac56f85ac3c134a4547b733b46413042c9440049176905d3be59ea1c53f15916155c2be8241a38008b9a26bc35941e2444177c8ade6689de95264986d95889fb60e84629c9bd9a5acb1cc118be563eb9b3a4a472f82e09a7e778492b562ef7130e88dfe031c79db9d4f7c7a899151b9a475032b63fc385245fe054e3dd5a97a5f576fe064025d3ce042c566ab2c507b138db853e3d6959660996546cc9c4a6eafdc777c040d70eaf46f76dad3979e5c5360c3317166a1c894c94a371876a94df7628fe4eaaf2ccb27d5aaae0ad7ad0f9d4b6ad3b54098746d4524d38407a6deb3ab78fab78c9", - "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", "ff0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", + "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", "c640c1711e3ee904ac35c57ab9791c8a1c408603a90b77a83b54f6c844cb4b06d94e7fc6c800e165acd66147e80ec45a567f6ce66d05ec0cae679dceeb890017", "3940c1e92da4582ff6f92a776aeb14d014d384eeb30f660dacf70a14a23fd31e91212701334e2ce1acf5199dc84f4d61ddbe6571bca5af874b4c9226c26e650995d157644e1848b96ed6c2102d5489a050e71d29a5a66ece11de5fb5c9558d54da28fe45b0bc4db4e5b88030bfc4a352b4b7068eccf656bae7ad6a35615315fc7c49d4200388d5eca67c2e822e069336c69b40db67e0f3c81209c50f3216a4b89fb3ae1b984b7851a2ec6f68ab12b101ab120e1ea7313bb93b5a0f71185c7fea017ddb92769861c29dba4fbc432280d5dff21b36d1c4c790128b22699950bb18bf74c448cdfe547d8ed4f657d8005fdc0cd7a050c2d46050a44c4376355858981fbe8b184288276e7a93eabc899c4a", "f039c6689eaeef0456685200feaab9d54bbd9acde4410a3b6f4321296f4a8ca2604b49727d8892c57e005d799b2a38e85e809f20146e08eec75169691c8d4f54a0d51a1e1c7b381e0474eb02f994be9415ef3ffcbd2343f0601e1f3b172a1d494f838824e4df570f8e3b0c04e27966e36c82abd352d07054ef7bd36b84c63f9369afe7ed79b94f953873006b920c3fa251a771de1b63da927058ade119aa898b8c97e42a606b2f6df1e2d957c22f7593c1e2002f4252f4c9ae4bf773499e5cfcfe14dfc1ede26508953f88553bf4a76a802f6a0068d59295b01503fd9a600067624203e880fdf53933b96e1f4d9eb3f4e363dd8165a278ff667a41ee42b9892b077cefff92b93441f7be74cf10e6cd"); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 5668ead1fb..0bfe6eecd9 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) const CChainParams& chainparams = Params(); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, - *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); + *m_node.chainman, *m_node.mempool, false); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -121,7 +121,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) const CChainParams& chainparams = Params(); auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, - *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); + *m_node.chainman, *m_node.mempool, false); constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS; CConnman::Options options; @@ -194,7 +194,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), - *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); + *m_node.chainman, *m_node.mempool, false); CNetAddr tor_netaddr; BOOST_REQUIRE( @@ -288,7 +288,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), - *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); + *m_node.chainman, *m_node.mempool, false); banman->ClearBanned(); int64_t nStartTime = GetTime(); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 344d1dde8e..95aa53bff4 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -12,6 +12,7 @@ #include <time.h> #include <util/asmap.h> +#include <cassert> #include <cstdint> #include <optional> #include <string> @@ -25,10 +26,201 @@ void initialize_addrman() class CAddrManDeterministic : public CAddrMan { public: - void MakeDeterministic(const uint256& random_seed) + FuzzedDataProvider& m_fuzzed_data_provider; + + explicit CAddrManDeterministic(FuzzedDataProvider& 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()) { + m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); + if (!SanityCheckASMap(m_asmap)) { + m_asmap.clear(); + } + } + } + + /** + * Generate a random address. Always returns a valid address. + */ + CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(cs) + { + CNetAddr addr; + if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(m_fuzzed_data_provider); + } else { + // The networks [1..6] correspond to CNetAddr::BIP155Network (private). + static const std::map<uint8_t, uint8_t> net_len_map = {{1, ADDR_IPV4_SIZE}, + {2, ADDR_IPV6_SIZE}, + {4, ADDR_TORV3_SIZE}, + {5, ADDR_I2P_SIZE}, + {6, ADDR_CJDNS_SIZE}}; + uint8_t net = insecure_rand.randrange(5) + 1; // [1..5] + if (net == 3) { + net = 6; + } + + CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); + + s << net; + s << insecure_rand.randbytes(net_len_map.at(net)); + + s >> addr; + } + + // Return a dummy IPv4 5.5.5.5 if we generated an invalid address. + if (!addr.IsValid()) { + in_addr v4_addr = {}; + v4_addr.s_addr = 0x05050505; + addr = CNetAddr{v4_addr}; + } + + return addr; + } + + /** + * Fill this addrman with lots of addresses from lots of sources. + */ + void Fill() + { + LOCK(cs); + + // Add some of the addresses directly to the "tried" table. + + // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33% + const size_t n = m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 3); + + const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange<size_t>(10, 50); + CNetAddr prev_source; + // Use insecure_rand inside the loops instead of m_fuzzed_data_provider because when + // the latter is exhausted it just returns 0. + for (size_t i = 0; i < num_sources; ++i) { + const auto source = RandAddr(); + const size_t num_addresses = insecure_rand.randrange(500) + 1; // [1..500] + + for (size_t j = 0; j < num_addresses; ++j) { + const auto addr = CAddress{CService{RandAddr(), 8333}, NODE_NETWORK}; + const auto time_penalty = insecure_rand.randrange(100000001); +#if 1 + // 2.83 sec to fill. + if (n > 0 && mapInfo.size() % n == 0 && mapAddr.find(addr) == mapAddr.end()) { + // Add to the "tried" table (if the bucket slot is free). + const CAddrInfo dummy{addr, source}; + const int bucket = dummy.GetTriedBucket(nKey, m_asmap); + const int bucket_pos = dummy.GetBucketPosition(nKey, false, bucket); + if (vvTried[bucket][bucket_pos] == -1) { + int id; + CAddrInfo* addr_info = Create(addr, source, &id); + vvTried[bucket][bucket_pos] = id; + addr_info->fInTried = true; + ++nTried; + } + } else { + // Add to the "new" table. + Add_(addr, source, time_penalty); + } +#else + // 261.91 sec to fill. + Add_(addr, source, time_penalty); + if (n > 0 && mapInfo.size() % n == 0) { + Good_(addr, false, GetTime()); + } +#endif + // Add 10% of the addresses from more than one source. + if (insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { + Add_(addr, prev_source, time_penalty); + } + } + prev_source = source; + } + } + + /** + * Compare with another AddrMan. + * This compares: + * - the values in `mapInfo` (the keys aka ids are ignored) + * - vvNew entries refer to the same addresses + * - vvTried entries refer to the same addresses + */ + bool operator==(const CAddrManDeterministic& other) { - WITH_LOCK(cs, insecure_rand = FastRandomContext{random_seed}); - Clear(); + LOCK2(cs, other.cs); + + if (mapInfo.size() != other.mapInfo.size() || nNew != other.nNew || + nTried != other.nTried) { + return false; + } + + // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. + // Keys may be different. + + using CAddrInfoHasher = std::function<size_t(const CAddrInfo&)>; + using CAddrInfoEq = std::function<bool(const CAddrInfo&, const CAddrInfo&)>; + + CNetAddrHash netaddr_hasher; + + CAddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const CAddrInfo& a) { + return netaddr_hasher(static_cast<CNetAddr>(a)) ^ netaddr_hasher(a.source) ^ + a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried; + }; + + CAddrInfoEq addrinfo_eq = [](const CAddrInfo& lhs, const CAddrInfo& rhs) { + return static_cast<CNetAddr>(lhs) == static_cast<CNetAddr>(rhs) && + lhs.source == rhs.source && lhs.nLastSuccess == rhs.nLastSuccess && + lhs.nAttempts == rhs.nAttempts && lhs.nRefCount == rhs.nRefCount && + lhs.fInTried == rhs.fInTried; + }; + + using Addresses = std::unordered_set<CAddrInfo, CAddrInfoHasher, CAddrInfoEq>; + + const size_t num_addresses{mapInfo.size()}; + + Addresses addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : mapInfo) { + addresses.insert(addr); + } + + Addresses other_addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : other.mapInfo) { + other_addresses.insert(addr); + } + + if (addresses != other_addresses) { + return false; + } + + auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(cs, other.cs) { + if (id == -1 && other_id == -1) { + return true; + } + if ((id == -1 && other_id != -1) || (id != -1 && other_id == -1)) { + return false; + } + return mapInfo.at(id) == other.mapInfo.at(other_id); + }; + + // Check that `vvNew` contains the same addresses as `other.vvNew`. Notice - `vvNew[i][j]` + // contains just an id and the address is to be found in `mapInfo.at(id)`. The ids + // themselves may differ between `vvNew` and `other.vvNew`. + for (size_t i = 0; i < ADDRMAN_NEW_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(vvNew[i][j], other.vvNew[i][j])) { + return false; + } + } + } + + // Same for `vvTried`. + for (size_t i = 0; i < ADDRMAN_TRIED_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(vvTried[i][j], other.vvTried[i][j])) { + return false; + } + } + } + + return true; } }; @@ -36,45 +228,29 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - CAddrManDeterministic addr_man; - addr_man.MakeDeterministic(ConsumeUInt256(fuzzed_data_provider)); - if (fuzzed_data_provider.ConsumeBool()) { - addr_man.m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(addr_man.m_asmap)) { - addr_man.m_asmap.clear(); - } - } + auto addr_man_ptr = std::make_unique<CAddrManDeterministic>(fuzzed_data_provider); if (fuzzed_data_provider.ConsumeBool()) { const std::vector<uint8_t> serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION); const auto ser_version{fuzzed_data_provider.ConsumeIntegral<int32_t>()}; ds.SetVersion(ser_version); try { - ds >> addr_man; + ds >> *addr_man_ptr; } catch (const std::ios_base::failure&) { - addr_man.Clear(); + addr_man_ptr = std::make_unique<CAddrManDeterministic>(fuzzed_data_provider); } } + CAddrManDeterministic& addr_man = *addr_man_ptr; while (fuzzed_data_provider.ConsumeBool()) { CallOneOf( fuzzed_data_provider, [&] { - addr_man.Clear(); - }, - [&] { addr_man.ResolveCollisions(); }, [&] { (void)addr_man.SelectTriedCollision(); }, [&] { - const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); - const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider); - if (opt_address && opt_net_addr) { - addr_man.Add(*opt_address, *opt_net_addr, fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, 100000000)); - } - }, - [&] { std::vector<CAddress> addresses; while (fuzzed_data_provider.ConsumeBool()) { const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); @@ -123,3 +299,21 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); data_stream << const_addr_man; } + +// Check that serialize followed by unserialize produces the same addrman. +FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + SetMockTime(ConsumeTime(fuzzed_data_provider)); + + CAddrManDeterministic addr_man1{fuzzed_data_provider}; + CAddrManDeterministic addr_man2{fuzzed_data_provider}; + addr_man2.m_asmap = addr_man1.m_asmap; + + CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); + + addr_man1.Fill(); + data_stream << addr_man1; + data_stream >> addr_man2; + assert(addr_man1 == addr_man2); +} 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/fuzz.h b/src/test/fuzz/fuzz.h index 2bad77bdc1..ce8fd660aa 100644 --- a/src/test/fuzz/fuzz.h +++ b/src/test/fuzz/fuzz.h @@ -11,6 +11,9 @@ #include <functional> #include <string_view> +#define LIMITED_WHILE(condition, limit) \ + for (unsigned _count{limit}; (condition) && _count; --_count) + using FuzzBufferType = Span<const uint8_t>; using TypeTestOneInput = std::function<void(FuzzBufferType)>; diff --git a/src/test/fuzz/kitchen_sink.cpp b/src/test/fuzz/kitchen_sink.cpp index 908e9a1c83..82f3a306c5 100644 --- a/src/test/fuzz/kitchen_sink.cpp +++ b/src/test/fuzz/kitchen_sink.cpp @@ -13,6 +13,7 @@ #include <array> #include <cstdint> +#include <optional> #include <vector> namespace { @@ -46,11 +47,10 @@ FUZZ_TARGET(kitchen_sink) const OutputType output_type = fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES); const std::string& output_type_string = FormatOutputType(output_type); - OutputType output_type_parsed; - const bool parsed = ParseOutputType(output_type_string, output_type_parsed); + const std::optional<OutputType> parsed = ParseOutputType(output_type_string); assert(parsed); - assert(output_type == output_type_parsed); - (void)ParseOutputType(fuzzed_data_provider.ConsumeRandomLengthString(64), output_type_parsed); + assert(output_type == parsed.value()); + (void)ParseOutputType(fuzzed_data_provider.ConsumeRandomLengthString(64)); const std::vector<uint8_t> bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); const std::vector<bool> bits = BytesToBits(bytes); 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/fuzz/string.cpp b/src/test/fuzz/string.cpp index 286375f7ae..0c1b45b86c 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -66,8 +66,7 @@ FUZZ_TARGET(string) (void)ParseNonRFCJSONValue(random_string_1); } catch (const std::runtime_error&) { } - OutputType output_type; - (void)ParseOutputType(random_string_1, output_type); + (void)ParseOutputType(random_string_1); (void)RemovePrefix(random_string_1, random_string_2); (void)ResolveErrMsg(random_string_1, random_string_2); try { diff --git a/src/test/fuzz/system.cpp b/src/test/fuzz/system.cpp index b25dcfcd3b..0f53939eac 100644 --- a/src/test/fuzz/system.cpp +++ b/src/test/fuzz/system.cpp @@ -31,7 +31,8 @@ FUZZ_TARGET(system) SetupHelpOptions(args_manager); } - while (fuzzed_data_provider.ConsumeBool()) { + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 3000) + { CallOneOf( fuzzed_data_provider, [&] { diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index e20c5e4e8f..7f44dcf20e 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2011-2020 The Bitcoin Core developers +// Copyright (c) 2011-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. @@ -16,6 +16,7 @@ #include <util/system.h> #include <util/time.h> #include <validation.h> +#include <versionbits.h> #include <test/util/setup_common.h> @@ -51,36 +52,25 @@ BlockAssembler MinerTestingSetup::AssemblerForTest(const CChainParams& params) constexpr static struct { unsigned char extranonce; unsigned int nonce; -} blockinfo[] = { - {4, 0xa4a3e223}, {2, 0x15c32f9e}, {1, 0x0375b547}, {1, 0x7004a8a5}, - {2, 0xce440296}, {2, 0x52cfe198}, {1, 0x77a72cd0}, {2, 0xbb5d6f84}, - {2, 0x83f30c2c}, {1, 0x48a73d5b}, {1, 0xef7dcd01}, {2, 0x6809c6c4}, - {2, 0x0883ab3c}, {1, 0x087bbbe2}, {2, 0x2104a814}, {2, 0xdffb6daa}, - {1, 0xee8a0a08}, {2, 0xba4237c1}, {1, 0xa70349dc}, {1, 0x344722bb}, - {3, 0xd6294733}, {2, 0xec9f5c94}, {2, 0xca2fbc28}, {1, 0x6ba4f406}, - {2, 0x015d4532}, {1, 0x6e119b7c}, {2, 0x43e8f314}, {2, 0x27962f38}, - {2, 0xb571b51b}, {2, 0xb36bee23}, {2, 0xd17924a8}, {2, 0x6bc212d9}, - {1, 0x630d4948}, {2, 0x9a4c4ebb}, {2, 0x554be537}, {1, 0xd63ddfc7}, - {2, 0xa10acc11}, {1, 0x759a8363}, {2, 0xfb73090d}, {1, 0xe82c6a34}, - {1, 0xe33e92d7}, {3, 0x658ef5cb}, {2, 0xba32ff22}, {5, 0x0227a10c}, - {1, 0xa9a70155}, {5, 0xd096d809}, {1, 0x37176174}, {1, 0x830b8d0f}, - {1, 0xc6e3910e}, {2, 0x823f3ca8}, {1, 0x99850849}, {1, 0x7521fb81}, - {1, 0xaacaabab}, {1, 0xd645a2eb}, {5, 0x7aea1781}, {5, 0x9d6e4b78}, - {1, 0x4ce90fd8}, {1, 0xabdc832d}, {6, 0x4a34f32a}, {2, 0xf2524c1c}, - {2, 0x1bbeb08a}, {1, 0xad47f480}, {1, 0x9f026aeb}, {1, 0x15a95049}, - {2, 0xd1cb95b2}, {2, 0xf84bbda5}, {1, 0x0fa62cd1}, {1, 0xe05f9169}, - {1, 0x78d194a9}, {5, 0x3e38147b}, {5, 0x737ba0d4}, {1, 0x63378e10}, - {1, 0x6d5f91cf}, {2, 0x88612eb8}, {2, 0xe9639484}, {1, 0xb7fabc9d}, - {2, 0x19b01592}, {1, 0x5a90dd31}, {2, 0x5bd7e028}, {2, 0x94d00323}, - {1, 0xa9b9c01a}, {1, 0x3a40de61}, {1, 0x56e7eec7}, {5, 0x859f7ef6}, - {1, 0xfd8e5630}, {1, 0x2b0c9f7f}, {1, 0xba700e26}, {1, 0x7170a408}, - {1, 0x70de86a8}, {1, 0x74d64cd5}, {1, 0x49e738a1}, {2, 0x6910b602}, - {0, 0x643c565f}, {1, 0x54264b3f}, {2, 0x97ea6396}, {2, 0x55174459}, - {2, 0x03e8779a}, {1, 0x98f34d8f}, {1, 0xc07b2b07}, {1, 0xdfe29668}, - {1, 0x3141c7c1}, {1, 0xb3b595f4}, {1, 0x735abf08}, {5, 0x623bfbce}, - {2, 0xd351e722}, {1, 0xf4ca48c9}, {1, 0x5b19c670}, {1, 0xa164bf0e}, - {2, 0xbbbeb305}, {2, 0xfe1c810a}, -}; +} BLOCKINFO[]{{8, 582909131}, {0, 971462344}, {2, 1169481553}, {6, 66147495}, {7, 427785981}, {8, 80538907}, + {8, 207348013}, {2, 1951240923}, {4, 215054351}, {1, 491520534}, {8, 1282281282}, {4, 639565734}, + {3, 248274685}, {8, 1160085976}, {6, 396349768}, {5, 393780549}, {5, 1096899528}, {4, 965381630}, + {0, 728758712}, {5, 318638310}, {3, 164591898}, {2, 274234550}, {2, 254411237}, {7, 561761812}, + {2, 268342573}, {0, 402816691}, {1, 221006382}, {6, 538872455}, {7, 393315655}, {4, 814555937}, + {7, 504879194}, {6, 467769648}, {3, 925972193}, {2, 200581872}, {3, 168915404}, {8, 430446262}, + {5, 773507406}, {3, 1195366164}, {0, 433361157}, {3, 297051771}, {0, 558856551}, {2, 501614039}, + {3, 528488272}, {2, 473587734}, {8, 230125274}, {2, 494084400}, {4, 357314010}, {8, 60361686}, + {7, 640624687}, {3, 480441695}, {8, 1424447925}, {4, 752745419}, {1, 288532283}, {6, 669170574}, + {5, 1900907591}, {3, 555326037}, {3, 1121014051}, {0, 545835650}, {8, 189196651}, {5, 252371575}, + {0, 199163095}, {6, 558895874}, {6, 1656839784}, {6, 815175452}, {6, 718677851}, {5, 544000334}, + {0, 340113484}, {6, 850744437}, {4, 496721063}, {8, 524715182}, {6, 574361898}, {6, 1642305743}, + {6, 355110149}, {5, 1647379658}, {8, 1103005356}, {7, 556460625}, {3, 1139533992}, {5, 304736030}, + {2, 361539446}, {2, 143720360}, {6, 201939025}, {7, 423141476}, {4, 574633709}, {3, 1412254823}, + {4, 873254135}, {0, 341817335}, {6, 53501687}, {3, 179755410}, {5, 172209688}, {8, 516810279}, + {4, 1228391489}, {8, 325372589}, {6, 550367589}, {0, 876291812}, {7, 412454120}, {7, 717202854}, + {2, 222677843}, {6, 251778867}, {7, 842004420}, {7, 194762829}, {4, 96668841}, {1, 925485796}, + {0, 792342903}, {6, 678455063}, {6, 773251385}, {5, 186617471}, {6, 883189502}, {7, 396077336}, + {8, 254702874}, {0, 455592851}}; static CBlockIndex CreateBlockIndex(int nHeight, CBlockIndex* active_chain_tip) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -220,20 +210,18 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // We can't make transactions until we have inputs // Therefore, load 110 blocks :) - static_assert(std::size(blockinfo) == 110, "Should have 110 blocks to import"); + static_assert(std::size(BLOCKINFO) == 110, "Should have 110 blocks to import"); int baseheight = 0; std::vector<CTransactionRef> txFirst; - for (const auto& bi : blockinfo) { + for (const auto& bi : BLOCKINFO) { CBlock *pblock = &pblocktemplate->block; // pointer for convenience { LOCK(cs_main); - pblock->nVersion = 1; + pblock->nVersion = VERSIONBITS_TOP_BITS; pblock->nTime = m_node.chainman->ActiveChain().Tip()->GetMedianTimePast()+1; CMutableTransaction txCoinbase(*pblock->vtx[0]); txCoinbase.nVersion = 1; - txCoinbase.vin[0].scriptSig = CScript(); - txCoinbase.vin[0].scriptSig.push_back(bi.extranonce); - txCoinbase.vin[0].scriptSig.push_back(m_node.chainman->ActiveChain().Height()); + txCoinbase.vin[0].scriptSig = CScript{} << (m_node.chainman->ActiveChain().Height() + 1) << bi.extranonce; txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this) txCoinbase.vout[0].scriptPubKey = CScript(); pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index acbbf357d2..803a7b8b15 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -2,8 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <addrdb.h> -#include <addrman.h> #include <chainparams.h> #include <clientversion.h> #include <cstdint> @@ -29,65 +27,6 @@ using namespace std::literals; -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); - } -}; - -class CAddrManUncorrupted : public CAddrManSerializationMock -{ -public: - void Serialize(CDataStream& s) const override - { - CAddrMan::Serialize(s); - } -}; - -class CAddrManCorrupted : public CAddrManSerializationMock -{ -public: - void Serialize(CDataStream& s) const override - { - // Produces corrupt output that claims addrman has 20 addrs when it only has one addr. - unsigned char nVersion = 1; - s << nVersion; - s << ((unsigned char)32); - s << nKey; - s << 10; // nNew - s << 10; // nTried - - int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); - s << nUBuckets; - - CService serv; - BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false)); - CAddress addr = CAddress(serv, NODE_NONE); - CNetAddr resolved; - BOOST_CHECK(LookupHost("252.2.2.2", resolved, false)); - CAddrInfo info = CAddrInfo(addr, resolved); - s << info; - } -}; - -static CDataStream AddrmanToStream(const CAddrManSerializationMock& _addrman) -{ - CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); - ssPeersIn << Params().MessageStart(); - ssPeersIn << _addrman; - std::string str = ssPeersIn.str(); - std::vector<unsigned char> vchData(str.begin(), str.end()); - return CDataStream(vchData, SER_DISK, CLIENT_VERSION); -} - BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(cnode_listen_port) @@ -102,82 +41,6 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_CHECK(port == altPort); } -BOOST_AUTO_TEST_CASE(caddrdb_read) -{ - CAddrManUncorrupted addrmanUncorrupted; - addrmanUncorrupted.MakeDeterministic(); - - CService addr1, addr2, addr3; - BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false)); - BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false)); - BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false)); - BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false)); - BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false)); - - // Add three addresses to new table. - CService source; - BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false)); - BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr1, NODE_NONE), source)); - BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr2, NODE_NONE), source)); - BOOST_CHECK(addrmanUncorrupted.Add(CAddress(addr3, NODE_NONE), source)); - - // Test that the de-serialization does not throw an exception. - CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); - bool exceptionThrown = false; - CAddrMan addrman1; - - BOOST_CHECK(addrman1.size() == 0); - try { - unsigned char pchMsgTmp[4]; - ssPeers1 >> pchMsgTmp; - ssPeers1 >> addrman1; - } catch (const std::exception&) { - exceptionThrown = true; - } - - BOOST_CHECK(addrman1.size() == 3); - BOOST_CHECK(exceptionThrown == false); - - // Test that CAddrDB::Read creates an addrman with the correct number of addrs. - CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); - - CAddrMan addrman2; - BOOST_CHECK(addrman2.size() == 0); - BOOST_CHECK(CAddrDB::Read(addrman2, ssPeers2)); - BOOST_CHECK(addrman2.size() == 3); -} - - -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; - BOOST_CHECK(addrman1.size() == 0); - try { - unsigned char pchMsgTmp[4]; - ssPeers1 >> pchMsgTmp; - ssPeers1 >> addrman1; - } catch (const std::exception&) { - exceptionThrown = true; - } - // Even through de-serialization failed addrman is not left in a clean state. - BOOST_CHECK(addrman1.size() == 1); - BOOST_CHECK(exceptionThrown); - - // Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails. - CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); - - CAddrMan addrman2; - BOOST_CHECK(addrman2.size() == 0); - BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2)); - BOOST_CHECK(addrman2.size() == 0); -} - BOOST_AUTO_TEST_CASE(cnode_simple_test) { SOCKET hSocket = INVALID_SOCKET; diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 56e2aa63b9..2c39cbffb9 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1160,7 +1160,7 @@ SignatureData CombineSignatures(const CTxOut& txout, const CMutableTransaction& SignatureData data; data.MergeSignatureData(scriptSig1); data.MergeSignatureData(scriptSig2); - ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&tx, 0, txout.nValue), txout.scriptPubKey, data); + ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&tx, 0, txout.nValue, SIGHASH_DEFAULT), txout.scriptPubKey, data); return data; } diff --git a/src/test/serfloat_tests.cpp b/src/test/serfloat_tests.cpp index 7876c0bcda..15612e2950 100644 --- a/src/test/serfloat_tests.cpp +++ b/src/test/serfloat_tests.cpp @@ -102,11 +102,12 @@ BOOST_AUTO_TEST_CASE(double_serfloat_tests) { Python code to generate the below hashes: def reversed_hex(x): - return binascii.hexlify(''.join(reversed(x))) + return bytes(reversed(x)).hex() + def dsha256(x): return hashlib.sha256(hashlib.sha256(x).digest()).digest() - reversed_hex(dsha256(''.join(struct.pack('<d', x) for x in range(0,1000)))) == '43d0c82591953c4eafe114590d392676a01585d25b25d433557f0d7878b23f96' + reversed_hex(dsha256(b''.join(struct.pack('<d', x) for x in range(0,1000)))) == '43d0c82591953c4eafe114590d392676a01585d25b25d433557f0d7878b23f96' */ BOOST_AUTO_TEST_CASE(doubles) { diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 571f792a53..97fd0600fa 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -561,7 +561,7 @@ SignatureData CombineSignatures(const CMutableTransaction& input1, const CMutabl SignatureData sigdata; sigdata = DataFromTransaction(input1, 0, tx->vout[0]); sigdata.MergeSignatureData(DataFromTransaction(input2, 0, tx->vout[0])); - ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&input1, 0, tx->vout[0].nValue), tx->vout[0].scriptPubKey, sigdata); + ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&input1, 0, tx->vout[0].nValue, SIGHASH_ALL), tx->vout[0].scriptPubKey, sigdata); return sigdata; } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 5334c4623e..c9bb863a7b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -193,11 +193,11 @@ 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, - m_node.banman.get(), *m_node.scheduler, *m_node.chainman, + m_node.banman.get(), *m_node.chainman, *m_node.mempool, false); { CConnman::Options options; @@ -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 85a23731a2..4e16a83c87 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/system.h b/src/util/system.h index 3547bad585..3c1399629c 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -205,6 +205,7 @@ protected: */ bool UseDefaultSection(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); + public: /** * Get setting value. * @@ -219,7 +220,6 @@ protected: */ std::vector<util::SettingsValue> GetSettingsList(const std::string& arg) const; -public: ArgsManager(); ~ArgsManager(); 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/context.h b/src/wallet/context.h index a83591154f..a382fb9021 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -5,11 +5,22 @@ #ifndef BITCOIN_WALLET_CONTEXT_H #define BITCOIN_WALLET_CONTEXT_H +#include <sync.h> + +#include <functional> +#include <list> +#include <memory> +#include <vector> + class ArgsManager; +class CWallet; namespace interfaces { class Chain; +class Wallet; } // namespace interfaces +using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>; + //! WalletContext struct containing references to state shared between CWallet //! instances, like the reference to the chain interface, and the list of opened //! wallets. @@ -22,7 +33,10 @@ class Chain; //! behavior. struct WalletContext { interfaces::Chain* chain{nullptr}; - ArgsManager* args{nullptr}; + ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct + Mutex wallets_mutex; + std::vector<std::shared_ptr<CWallet>> wallets GUARDED_BY(wallets_mutex); + std::list<LoadWalletFn> wallet_load_fns GUARDED_BY(wallets_mutex); //! Declare default constructor and destructor that are not inline, so code //! instantiating the WalletContext struct doesn't need to #include class diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index e33adf94c9..0d4b98ecaf 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> @@ -109,7 +110,7 @@ WalletTxOut MakeWalletTxOut(const CWallet& wallet, class WalletImpl : public Wallet { public: - explicit WalletImpl(const std::shared_ptr<CWallet>& wallet) : m_wallet(wallet) {} + explicit WalletImpl(WalletContext& context, const std::shared_ptr<CWallet>& wallet) : m_context(context), m_wallet(wallet) {} bool encryptWallet(const SecureString& wallet_passphrase) override { @@ -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 @@ -457,7 +458,7 @@ public: CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; } void remove() override { - RemoveWallet(m_wallet, false /* load_on_start */); + RemoveWallet(m_context, m_wallet, false /* load_on_start */); } bool isLegacy() override { return m_wallet->IsLegacy(); } std::unique_ptr<Handler> handleUnload(UnloadFn fn) override @@ -493,6 +494,7 @@ public: } CWallet* wallet() override { return m_wallet.get(); } + WalletContext& m_context; std::shared_ptr<CWallet> m_wallet; }; @@ -504,7 +506,7 @@ public: m_context.chain = &chain; m_context.args = &args; } - ~WalletClientImpl() override { UnloadWallets(); } + ~WalletClientImpl() override { UnloadWallets(m_context); } //! ChainClient methods void registerRpcs() override @@ -518,11 +520,11 @@ public: m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back())); } } - bool verify() override { return VerifyWallets(*m_context.chain); } - bool load() override { return LoadWallets(*m_context.chain); } - void start(CScheduler& scheduler) override { return StartWallets(scheduler, *Assert(m_context.args)); } - void flush() override { return FlushWallets(); } - void stop() override { return StopWallets(); } + bool verify() override { return VerifyWallets(m_context); } + bool load() override { return LoadWallets(m_context); } + void start(CScheduler& scheduler) override { return StartWallets(m_context, scheduler); } + void flush() override { return FlushWallets(m_context); } + void stop() override { return StopWallets(m_context); } void setMockTime(int64_t time) override { return SetMockTime(time); } //! WalletClient methods @@ -534,14 +536,14 @@ public: options.require_create = true; options.create_flags = wallet_creation_flags; options.create_passphrase = passphrase; - return MakeWallet(CreateWallet(*m_context.chain, name, true /* load_on_start */, options, status, error, warnings)); + return MakeWallet(m_context, CreateWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); } std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override { DatabaseOptions options; DatabaseStatus status; options.require_existing = true; - return MakeWallet(LoadWallet(*m_context.chain, name, true /* load_on_start */, options, status, error, warnings)); + return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); } std::string getWalletDir() override { @@ -558,15 +560,16 @@ public: std::vector<std::unique_ptr<Wallet>> getWallets() override { std::vector<std::unique_ptr<Wallet>> wallets; - for (const auto& wallet : GetWallets()) { - wallets.emplace_back(MakeWallet(wallet)); + for (const auto& wallet : GetWallets(m_context)) { + wallets.emplace_back(MakeWallet(m_context, wallet)); } return wallets; } std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override { - return HandleLoadWallet(std::move(fn)); + return HandleLoadWallet(m_context, std::move(fn)); } + WalletContext* context() override { return &m_context; } WalletContext m_context; const std::vector<std::string> m_wallet_filenames; @@ -577,7 +580,7 @@ public: } // namespace wallet namespace interfaces { -std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? std::make_unique<wallet::WalletImpl>(wallet) : nullptr; } +std::unique_ptr<Wallet> MakeWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet) { return wallet ? std::make_unique<wallet::WalletImpl>(context, wallet) : nullptr; } std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args) { diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index dbf9fd46b6..88e9b9c78f 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -11,13 +11,15 @@ #include <util/string.h> #include <util/system.h> #include <util/translation.h> +#include <wallet/context.h> #include <wallet/wallet.h> #include <wallet/walletdb.h> #include <univalue.h> -bool VerifyWallets(interfaces::Chain& chain) +bool VerifyWallets(WalletContext& context) { + interfaces::Chain& chain = *context.chain; if (gArgs.IsArgSet("-walletdir")) { fs::path wallet_dir = gArgs.GetArg("-walletdir", ""); boost::system::error_code error; @@ -50,18 +52,20 @@ bool VerifyWallets(interfaces::Chain& chain) options.require_existing = true; options.verify = false; if (MakeWalletDatabase("", options, status, error_string)) { - gArgs.LockSettings([&](util::Settings& settings) { - util::SettingsValue wallets(util::SettingsValue::VARR); - wallets.push_back(""); // Default wallet name is "" - settings.rw_settings["wallet"] = wallets; - }); + util::SettingsValue wallets(util::SettingsValue::VARR); + wallets.push_back(""); // Default wallet name is "" + // Pass write=false because no need to write file and probably + // better not to. If unnamed wallet needs to be added next startup + // and the setting is empty, this code will just run again. + chain.updateRwSetting("wallet", wallets, /* write= */ false); } } // Keep track of each wallet absolute path to detect duplicates. std::set<fs::path> wallet_paths; - for (const auto& wallet_file : gArgs.GetArgs("-wallet")) { + for (const auto& wallet : chain.getSettingsList("wallet")) { + const auto& wallet_file = wallet.get_str(); const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), wallet_file); if (!wallet_paths.insert(path).second) { @@ -87,11 +91,13 @@ bool VerifyWallets(interfaces::Chain& chain) return true; } -bool LoadWallets(interfaces::Chain& chain) +bool LoadWallets(WalletContext& context) { + interfaces::Chain& chain = *context.chain; try { std::set<fs::path> wallet_paths; - for (const std::string& name : gArgs.GetArgs("-wallet")) { + for (const auto& wallet : chain.getSettingsList("wallet")) { + const auto& name = wallet.get_str(); if (!wallet_paths.insert(name).second) { continue; } @@ -106,13 +112,13 @@ bool LoadWallets(interfaces::Chain& chain) continue; } chain.initMessage(_("Loading wallet…").translated); - std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(&chain, name, std::move(database), options.create_flags, error, warnings) : nullptr; + std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error); return false; } - AddWallet(pwallet); + AddWallet(context, pwallet); } return true; } catch (const std::runtime_error& e) { @@ -121,41 +127,41 @@ bool LoadWallets(interfaces::Chain& chain) } } -void StartWallets(CScheduler& scheduler, const ArgsManager& args) +void StartWallets(WalletContext& context, CScheduler& scheduler) { - for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { + for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { pwallet->postInitProcess(); } // Schedule periodic wallet flushes and tx rebroadcasts - if (args.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { - scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); + if (context.args->GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { + scheduler.scheduleEvery([&context] { MaybeCompactWalletDB(context); }, std::chrono::milliseconds{500}); } - scheduler.scheduleEvery(MaybeResendWalletTxs, std::chrono::milliseconds{1000}); + scheduler.scheduleEvery([&context] { MaybeResendWalletTxs(context); }, std::chrono::milliseconds{1000}); } -void FlushWallets() +void FlushWallets(WalletContext& context) { - for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { + for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { pwallet->Flush(); } } -void StopWallets() +void StopWallets(WalletContext& context) { - for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { + for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { pwallet->Close(); } } -void UnloadWallets() +void UnloadWallets(WalletContext& context) { - auto wallets = GetWallets(); + auto wallets = GetWallets(context); while (!wallets.empty()) { auto wallet = wallets.back(); wallets.pop_back(); std::vector<bilingual_str> warnings; - RemoveWallet(wallet, std::nullopt, warnings); + RemoveWallet(context, wallet, /* load_on_startup= */ std::nullopt, warnings); UnloadWallet(std::move(wallet)); } } diff --git a/src/wallet/load.h b/src/wallet/load.h index 7910f0d6e1..e207bc2e09 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -11,27 +11,28 @@ class ArgsManager; class CScheduler; +struct WalletContext; namespace interfaces { class Chain; } // namespace interfaces //! Responsible for reading and validating the -wallet arguments and verifying the wallet database. -bool VerifyWallets(interfaces::Chain& chain); +bool VerifyWallets(WalletContext& context); //! Load wallet databases. -bool LoadWallets(interfaces::Chain& chain); +bool LoadWallets(WalletContext& context); //! Complete startup of wallets. -void StartWallets(CScheduler& scheduler, const ArgsManager& args); +void StartWallets(WalletContext& context, CScheduler& scheduler); //! Flush all wallets in preparation for shutdown. -void FlushWallets(); +void FlushWallets(WalletContext& context); //! Stop all wallets. Wallets will be flushed first. -void StopWallets(); +void StopWallets(WalletContext& context); //! Close all wallets. -void UnloadWallets(); +void UnloadWallets(WalletContext& context); #endif // BITCOIN_WALLET_LOAD_H diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ac60504419..72c60c8fe2 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <chain.h> +#include <clientversion.h> #include <core_io.h> #include <interfaces/chain.h> #include <key_io.h> @@ -783,7 +784,7 @@ RPCHelpMan dumpwallet() std::sort(vKeyBirth.begin(), vKeyBirth.end()); // produce output - file << strprintf("# Wallet dump created by Bitcoin %s\n", CLIENT_BUILD); + file << strprintf("# Wallet dump created by %s %s\n", PACKAGE_NAME, FormatFullVersion()); file << strprintf("# * Created on %s\n", FormatISO8601DateTime(GetTime())); file << strprintf("# * Best block at time of backup was %i (%s),\n", wallet.GetLastBlockHeight(), wallet.GetLastBlockHash().ToString()); file << strprintf("# mined on %s\n", FormatISO8601DateTime(block_time)); @@ -1760,8 +1761,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 +1784,7 @@ RPCHelpMan listdescriptors() }}, RPCExamples{ HelpExampleCli("listdescriptors", "") + HelpExampleRpc("listdescriptors", "") + + HelpExampleCli("listdescriptors", "true") + HelpExampleRpc("listdescriptors", "true") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -1791,6 +1795,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 +1813,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 f1d5117415..916f811f9b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -96,14 +96,16 @@ bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request) { CHECK_NONFATAL(request.mode == JSONRPCRequest::EXECUTE); + WalletContext& context = EnsureWalletContext(request.context); + std::string wallet_name; if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { - std::shared_ptr<CWallet> pwallet = GetWallet(wallet_name); + std::shared_ptr<CWallet> pwallet = GetWallet(context, wallet_name); if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); return pwallet; } - std::vector<std::shared_ptr<CWallet>> wallets = GetWallets(); + std::vector<std::shared_ptr<CWallet>> wallets = GetWallets(context); if (wallets.size() == 1) { return wallets[0]; } @@ -266,18 +268,19 @@ static RPCHelpMan getnewaddress() OutputType output_type = pwallet->m_default_address_type; if (!request.params[1].isNull()) { - if (!ParseOutputType(request.params[1].get_str(), output_type)) { + std::optional<OutputType> parsed = ParseOutputType(request.params[1].get_str()); + if (!parsed) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str())); - } - if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) { + } else if (parsed.value() == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses"); } + output_type = parsed.value(); } 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); @@ -313,18 +316,19 @@ static RPCHelpMan getrawchangeaddress() OutputType output_type = pwallet->m_default_change_type.value_or(pwallet->m_default_address_type); if (!request.params[0].isNull()) { - if (!ParseOutputType(request.params[0].get_str(), output_type)) { + std::optional<OutputType> parsed = ParseOutputType(request.params[0].get_str()); + if (!parsed) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); - } - if (output_type == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) { + } else if (parsed.value() == OutputType::BECH32M && pwallet->GetLegacyScriptPubKeyMan()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Legacy wallets cannot provide bech32m addresses"); } + output_type = parsed.value(); } 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); }, @@ -1007,12 +1011,13 @@ static RPCHelpMan addmultisigaddress() OutputType output_type = pwallet->m_default_address_type; if (!request.params[3].isNull()) { - if (!ParseOutputType(request.params[3].get_str(), output_type)) { + std::optional<OutputType> parsed = ParseOutputType(request.params[3].get_str()); + if (!parsed) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[3].get_str())); - } - if (output_type == OutputType::BECH32M) { + } else if (parsed.value() == OutputType::BECH32M) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Bech32m multisig addresses cannot be created with legacy wallets"); } + output_type = parsed.value(); } // Construct using pay-to-script-hash: @@ -2559,7 +2564,8 @@ static RPCHelpMan listwallets() { UniValue obj(UniValue::VARR); - for (const std::shared_ptr<CWallet>& wallet : GetWallets()) { + WalletContext& context = EnsureWalletContext(request.context); + for (const std::shared_ptr<CWallet>& wallet : GetWallets(context)) { LOCK(wallet->cs_wallet); obj.push_back(wallet->GetName()); } @@ -2569,6 +2575,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, 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", @@ -2595,30 +2632,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()); @@ -2777,7 +2791,7 @@ static RPCHelpMan createwallet() options.create_passphrase = passphrase; bilingual_str error; std::optional<bool> load_on_start = request.params[6].isNull() ? std::nullopt : std::optional<bool>(request.params[6].get_bool()); - std::shared_ptr<CWallet> wallet = CreateWallet(*context.chain, request.params[0].get_str(), load_on_start, options, status, error, warnings); + std::shared_ptr<CWallet> wallet = CreateWallet(context, request.params[0].get_str(), load_on_start, options, status, error, warnings); if (!wallet) { RPCErrorCode code = status == DatabaseStatus::FAILED_ENCRYPT ? RPC_WALLET_ENCRYPTION_FAILED : RPC_WALLET_ERROR; throw JSONRPCError(code, error.original); @@ -2792,6 +2806,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", @@ -2819,7 +2895,8 @@ static RPCHelpMan unloadwallet() wallet_name = request.params[0].get_str(); } - std::shared_ptr<CWallet> wallet = GetWallet(wallet_name); + WalletContext& context = EnsureWalletContext(request.context); + std::shared_ptr<CWallet> wallet = GetWallet(context, wallet_name); if (!wallet) { throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); } @@ -2829,7 +2906,7 @@ static RPCHelpMan unloadwallet() // is destroyed (see CheckUniqueFileid). 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()); - if (!RemoveWallet(wallet, load_on_start, warnings)) { + if (!RemoveWallet(context, wallet, load_on_start, warnings)) { throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } @@ -3133,11 +3210,11 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, if (options.exists("changeAddress") || options.exists("change_address")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both change address and address type options"); } - OutputType out_type; - if (!ParseOutputType(options["change_type"].get_str(), out_type)) { + if (std::optional<OutputType> parsed = ParseOutputType(options["change_type"].get_str())) { + coinControl.m_change_type.emplace(parsed.value()); + } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str())); } - coinControl.m_change_type.emplace(out_type); } const UniValue include_watching_option = options.exists("include_watching") ? options["include_watching"] : options["includeWatching"]; @@ -3389,7 +3466,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); @@ -3872,7 +3949,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); } } @@ -4636,6 +4713,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/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 66e7de4273..8821f680b3 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -54,7 +54,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) // Send full input minus more than the fee amount to recipient, check // leftover input amount is paid to recipient not the miner (to_reduce == // -123). This overpays the recipient instead of overpaying the miner more - // than double the neccesary fee. + // than double the necessary fee. BOOST_CHECK_EQUAL(fee, check_tx(fee + 123)); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 75a08b6f74..9fcea5826b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -20,6 +20,7 @@ #include <util/translation.h> #include <validation.h> #include <wallet/coincontrol.h> +#include <wallet/context.h> #include <wallet/test/util.h> #include <wallet/test/wallet_test_fixture.h> @@ -30,8 +31,6 @@ RPCHelpMan importmulti(); RPCHelpMan dumpwallet(); RPCHelpMan importwallet(); -extern RecursiveMutex cs_wallets; - // Ensure that fee levels defined in the wallet are at least as high // as the default levels for node policy. static_assert(DEFAULT_TRANSACTION_MINFEE >= DEFAULT_MIN_RELAY_TX_FEE, "wallet minimum fee is smaller than default relay fee"); @@ -39,15 +38,15 @@ static_assert(WALLET_INCREMENTAL_RELAY_FEE >= DEFAULT_INCREMENTAL_RELAY_FEE, "wa BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static std::shared_ptr<CWallet> TestLoadWallet(interfaces::Chain* chain) +static std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) { DatabaseOptions options; DatabaseStatus status; bilingual_str error; std::vector<bilingual_str> warnings; auto database = MakeWalletDatabase("", options, status, error); - auto wallet = CWallet::Create(chain, "", std::move(database), options.create_flags, error, warnings); - if (chain) { + auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings); + if (context.chain) { wallet->postInitProcess(); } return wallet; @@ -69,7 +68,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; } @@ -200,7 +199,8 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash())); - AddWallet(wallet); + WalletContext context; + AddWallet(context, wallet); UniValue keys; keys.setArray(); UniValue key; @@ -218,6 +218,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) key.pushKV("internal", UniValue(true)); keys.push_back(key); JSONRPCRequest request; + request.context = &context; request.params.setArray(); request.params.push_back(keys); @@ -231,7 +232,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) "downloading and rescanning the relevant blocks (see -reindex and -rescan " "options).\"}},{\"success\":true}]", 0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW)); - RemoveWallet(wallet, std::nullopt); + RemoveWallet(context, wallet, /* load_on_startup= */ std::nullopt); } } @@ -258,6 +259,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { + WalletContext context; std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); @@ -265,15 +267,16 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) spk_man->mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); - AddWallet(wallet); + AddWallet(context, wallet); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } JSONRPCRequest request; + request.context = &context; request.params.setArray(); request.params.push_back(backup_file); ::dumpwallet().HandleRequest(request); - RemoveWallet(wallet, std::nullopt); + RemoveWallet(context, wallet, /* load_on_startup= */ std::nullopt); } // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME @@ -283,13 +286,15 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) LOCK(wallet->cs_wallet); wallet->SetupLegacyScriptPubKeyMan(); + WalletContext context; JSONRPCRequest request; + request.context = &context; request.params.setArray(); request.params.push_back(backup_file); - AddWallet(wallet); + AddWallet(context, wallet); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); ::importwallet().HandleRequest(request); - RemoveWallet(wallet, std::nullopt); + RemoveWallet(context, wallet, /* load_on_startup= */ std::nullopt); BOOST_CHECK_EQUAL(wallet->mapWallet.size(), 3U); BOOST_CHECK_EQUAL(m_coinbase_txns.size(), 103U); @@ -589,7 +594,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)); } @@ -679,7 +684,9 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); // Create new wallet with known key and unload it. - auto wallet = TestLoadWallet(m_node.chain.get()); + WalletContext context; + context.chain = m_node.chain.get(); + auto wallet = TestLoadWallet(context); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); @@ -719,7 +726,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // Reload wallet and make sure new transactions are detected despite events // being blocked - wallet = TestLoadWallet(m_node.chain.get()); + wallet = TestLoadWallet(context); BOOST_CHECK(rescan_completed); BOOST_CHECK_EQUAL(addtx_count, 2); { @@ -746,20 +753,20 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // deadlock during the sync and simulates a new block notification happening // as soon as possible. addtx_count = 0; - auto handler = HandleLoadWallet([&](std::unique_ptr<interfaces::Wallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->wallet()->cs_wallet, cs_wallets) { + auto handler = HandleLoadWallet(context, [&](std::unique_ptr<interfaces::Wallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->wallet()->cs_wallet, context.wallets_mutex) { BOOST_CHECK(rescan_completed); m_coinbase_txns.push_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error)); - LEAVE_CRITICAL_SECTION(cs_wallets); + LEAVE_CRITICAL_SECTION(context.wallets_mutex); LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet); SyncWithValidationInterfaceQueue(); ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); - ENTER_CRITICAL_SECTION(cs_wallets); + ENTER_CRITICAL_SECTION(context.wallets_mutex); }); - wallet = TestLoadWallet(m_node.chain.get()); + wallet = TestLoadWallet(context); BOOST_CHECK_EQUAL(addtx_count, 4); { LOCK(wallet->cs_wallet); @@ -773,7 +780,8 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) { - auto wallet = TestLoadWallet(nullptr); + WalletContext context; + auto wallet = TestLoadWallet(context); BOOST_CHECK(wallet); UnloadWallet(std::move(wallet)); } @@ -781,7 +789,9 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); - auto wallet = TestLoadWallet(m_node.chain.get()); + WalletContext context; + context.chain = m_node.chain.get(); + auto wallet = TestLoadWallet(context); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9a61ca698d..cf869fac0c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -33,6 +33,7 @@ #include <util/string.h> #include <util/translation.h> #include <wallet/coincontrol.h> +#include <wallet/context.h> #include <wallet/fees.h> #include <wallet/external_signer_scriptpubkeyman.h> @@ -54,10 +55,6 @@ const std::map<uint64_t,std::string> WALLET_FLAG_CAVEATS{ }, }; -RecursiveMutex cs_wallets; -static std::vector<std::shared_ptr<CWallet>> vpwallets GUARDED_BY(cs_wallets); -static std::list<LoadWalletFn> g_load_wallet_fns GUARDED_BY(cs_wallets); - bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) { util::SettingsValue setting_value = chain.getRwSetting("wallet"); @@ -94,19 +91,29 @@ static void UpdateWalletSetting(interfaces::Chain& chain, } } -bool AddWallet(const std::shared_ptr<CWallet>& wallet) +/** + * 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) { - LOCK(cs_wallets); + tx.fInMempool = chain.isInMempool(tx.GetHash()); +} + +bool AddWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet) +{ + LOCK(context.wallets_mutex); assert(wallet); - std::vector<std::shared_ptr<CWallet>>::const_iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); - if (i != vpwallets.end()) return false; - vpwallets.push_back(wallet); + std::vector<std::shared_ptr<CWallet>>::const_iterator i = std::find(context.wallets.begin(), context.wallets.end(), wallet); + if (i != context.wallets.end()) return false; + context.wallets.push_back(wallet); wallet->ConnectScriptPubKeyManNotifiers(); wallet->NotifyCanGetAddressesChanged(); return true; } -bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start, std::vector<bilingual_str>& warnings) +bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start, std::vector<bilingual_str>& warnings) { assert(wallet); @@ -115,10 +122,10 @@ bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, std::optional<bool> lo // Unregister with the validation interface which also drops shared ponters. wallet->m_chain_notifications_handler.reset(); - LOCK(cs_wallets); - std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); - if (i == vpwallets.end()) return false; - vpwallets.erase(i); + LOCK(context.wallets_mutex); + std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(context.wallets.begin(), context.wallets.end(), wallet); + if (i == context.wallets.end()) return false; + context.wallets.erase(i); // Write the wallet setting UpdateWalletSetting(chain, name, load_on_start, warnings); @@ -126,32 +133,32 @@ bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, std::optional<bool> lo return true; } -bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start) +bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start) { std::vector<bilingual_str> warnings; - return RemoveWallet(wallet, load_on_start, warnings); + return RemoveWallet(context, wallet, load_on_start, warnings); } -std::vector<std::shared_ptr<CWallet>> GetWallets() +std::vector<std::shared_ptr<CWallet>> GetWallets(WalletContext& context) { - LOCK(cs_wallets); - return vpwallets; + LOCK(context.wallets_mutex); + return context.wallets; } -std::shared_ptr<CWallet> GetWallet(const std::string& name) +std::shared_ptr<CWallet> GetWallet(WalletContext& context, const std::string& name) { - LOCK(cs_wallets); - for (const std::shared_ptr<CWallet>& wallet : vpwallets) { + LOCK(context.wallets_mutex); + for (const std::shared_ptr<CWallet>& wallet : context.wallets) { if (wallet->GetName() == name) return wallet; } return nullptr; } -std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet) +std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext& context, LoadWalletFn load_wallet) { - LOCK(cs_wallets); - auto it = g_load_wallet_fns.emplace(g_load_wallet_fns.end(), std::move(load_wallet)); - return interfaces::MakeHandler([it] { LOCK(cs_wallets); g_load_wallet_fns.erase(it); }); + LOCK(context.wallets_mutex); + auto it = context.wallet_load_fns.emplace(context.wallet_load_fns.end(), std::move(load_wallet)); + return interfaces::MakeHandler([&context, it] { LOCK(context.wallets_mutex); context.wallet_load_fns.erase(it); }); } static Mutex g_loading_wallet_mutex; @@ -203,7 +210,7 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet) } namespace { -std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) +std::shared_ptr<CWallet> LoadWalletInternal(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) { try { std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(name, options, status, error); @@ -212,18 +219,18 @@ std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const std: return nullptr; } - chain.initMessage(_("Loading wallet…").translated); - std::shared_ptr<CWallet> wallet = CWallet::Create(&chain, name, std::move(database), options.create_flags, error, warnings); + context.chain->initMessage(_("Loading wallet…").translated); + std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; return nullptr; } - AddWallet(wallet); + AddWallet(context, wallet); wallet->postInitProcess(); // Write the wallet setting - UpdateWalletSetting(chain, name, load_on_start, warnings); + UpdateWalletSetting(*context.chain, name, load_on_start, warnings); return wallet; } catch (const std::runtime_error& e) { @@ -234,7 +241,7 @@ std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const std: } } // namespace -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) +std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) { auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(name)); if (!result.second) { @@ -242,12 +249,12 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& status = DatabaseStatus::FAILED_LOAD; return nullptr; } - auto wallet = LoadWalletInternal(chain, name, load_on_start, options, status, error, warnings); + auto wallet = LoadWalletInternal(context, name, load_on_start, options, status, error, warnings); WITH_LOCK(g_loading_wallet_mutex, g_loading_wallet_set.erase(result.first)); return wallet; } -std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) +std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) { uint64_t wallet_creation_flags = options.create_flags; const SecureString& passphrase = options.create_passphrase; @@ -292,8 +299,8 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin } // Make the wallet - chain.initMessage(_("Loading wallet…").translated); - std::shared_ptr<CWallet> wallet = CWallet::Create(&chain, name, std::move(database), wallet_creation_flags, error, warnings); + context.chain->initMessage(_("Loading wallet…").translated); + std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -335,11 +342,11 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin wallet->Lock(); } } - AddWallet(wallet); + AddWallet(context, wallet); wallet->postInitProcess(); // Write the wallet settings - UpdateWalletSetting(chain, name, load_on_start, warnings); + UpdateWalletSetting(*context.chain, name, load_on_start, warnings); status = DatabaseStatus::SUCCESS; return wallet; @@ -803,10 +810,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 +1210,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 +1218,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. @@ -1795,9 +1799,9 @@ void CWallet::ResendWalletTransactions() /** @} */ // end of mapWallet -void MaybeResendWalletTxs() +void MaybeResendWalletTxs(WalletContext& context) { - for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { + for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { pwallet->ResendWalletTransactions(); } } @@ -1822,11 +1826,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 +2132,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 +2142,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 +2151,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 +2204,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; } @@ -2502,8 +2506,9 @@ std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, cons return MakeDatabase(wallet_path, options, status, error_string); } -std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) +std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) { + interfaces::Chain* chain = context.chain; const std::string& walletFile = database->Filename(); int64_t nStart = GetTimeMillis(); @@ -2586,19 +2591,21 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain* chain, const std::st } if (!gArgs.GetArg("-addresstype", "").empty()) { - if (!ParseOutputType(gArgs.GetArg("-addresstype", ""), walletInstance->m_default_address_type)) { + std::optional<OutputType> parsed = ParseOutputType(gArgs.GetArg("-addresstype", "")); + if (!parsed) { error = strprintf(_("Unknown address type '%s'"), gArgs.GetArg("-addresstype", "")); return nullptr; } + walletInstance->m_default_address_type = parsed.value(); } if (!gArgs.GetArg("-changetype", "").empty()) { - OutputType out_type; - if (!ParseOutputType(gArgs.GetArg("-changetype", ""), out_type)) { + std::optional<OutputType> parsed = ParseOutputType(gArgs.GetArg("-changetype", "")); + if (!parsed) { error = strprintf(_("Unknown change type '%s'"), gArgs.GetArg("-changetype", "")); return nullptr; } - walletInstance->m_default_change_type = out_type; + walletInstance->m_default_change_type = parsed.value(); } if (gArgs.IsArgSet("-mintxfee")) { @@ -2713,9 +2720,9 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain* chain, const std::st } { - LOCK(cs_wallets); - for (auto& load_wallet : g_load_wallet_fns) { - load_wallet(interfaces::MakeWallet(walletInstance)); + LOCK(context.wallets_mutex); + for (auto& load_wallet : context.wallet_load_fns) { + load_wallet(interfaces::MakeWallet(context, walletInstance)); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3997751f52..a1bbf66136 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -42,6 +42,8 @@ #include <boost/signals2/signal.hpp> +struct WalletContext; + using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>; struct bilingual_str; @@ -53,14 +55,14 @@ struct bilingual_str; //! by the shared pointer deleter. void UnloadWallet(std::shared_ptr<CWallet>&& wallet); -bool AddWallet(const std::shared_ptr<CWallet>& wallet); -bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start, std::vector<bilingual_str>& warnings); -bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start); -std::vector<std::shared_ptr<CWallet>> GetWallets(); -std::shared_ptr<CWallet> GetWallet(const std::string& name); -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); -std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); -std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet); +bool AddWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet); +bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start, std::vector<bilingual_str>& warnings); +bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start); +std::vector<std::shared_ptr<CWallet>> GetWallets(WalletContext& context); +std::shared_ptr<CWallet> GetWallet(WalletContext& context, const std::string& name); +std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); +std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); +std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext& context, LoadWalletFn load_wallet); std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); //! -paytxfee default @@ -183,7 +185,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 +565,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 +667,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); @@ -772,7 +774,7 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr<CWallet> Create(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings); + static std::shared_ptr<CWallet> Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings); /** * Wallet post-init setup @@ -919,7 +921,7 @@ public: * Called periodically by the schedule thread. Prompts individual wallets to resend * their transactions. Actual rebroadcast schedule is managed by the wallets themselves. */ -void MaybeResendWalletTxs(); +void MaybeResendWalletTxs(WalletContext& context); /** RAII object to check and reserve a wallet rescan */ class WalletRescanReserver diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 1e5d8dfa3a..2fabe65a93 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1004,14 +1004,14 @@ DBErrors WalletBatch::ZapSelectTx(std::vector<uint256>& vTxHashIn, std::vector<u return DBErrors::LOAD_OK; } -void MaybeCompactWalletDB() +void MaybeCompactWalletDB(WalletContext& context) { static std::atomic<bool> fOneThread(false); if (fOneThread.exchange(true)) { return; } - for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { + for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { WalletDatabase& dbh = pwallet->GetDatabase(); unsigned int nUpdateCounter = dbh.nUpdateCounter; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 9b775eb481..25c2ec5909 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -31,6 +31,7 @@ static const bool DEFAULT_FLUSHWALLET = true; struct CBlockLocator; +struct WalletContext; class CKeyPool; class CMasterKey; class CScript; @@ -279,7 +280,7 @@ private: }; //! Compacts BDB state so that wallet.dat is self-contained (if there are changes) -void MaybeCompactWalletDB(); +void MaybeCompactWalletDB(WalletContext& context); //! Callback for filtering key types to deserialize in ReadKeyValue using KeyFilterFn = std::function<bool(const std::string&)>; |