diff options
Diffstat (limited to 'src')
53 files changed, 597 insertions, 522 deletions
diff --git a/src/addrman.cpp b/src/addrman.cpp index 832c3b3cb9..86f56052c1 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -395,7 +395,7 @@ void AddrManImpl::Unserialize(Stream& s_) } } -AddrInfo* AddrManImpl::Find(const CNetAddr& addr, int* pnId) +AddrInfo* AddrManImpl::Find(const CService& addr, int* pnId) { AssertLockHeld(cs); @@ -553,10 +553,6 @@ void AddrManImpl::Good_(const CService& addr, bool test_before_evict, int64_t nT AddrInfo& info = *pinfo; - // check whether we are talking about the exact same CService (including same port) - if (info != addr) - return; - // update info info.nLastSuccess = nTime; info.nLastTry = nTime; @@ -685,10 +681,6 @@ void AddrManImpl::Attempt_(const CService& addr, bool fCountFailure, int64_t nTi AddrInfo& info = *pinfo; - // check whether we are talking about the exact same CService (including same port) - if (info != addr) - return; - // update info info.nLastTry = nTime; if (fCountFailure && info.nLastCountAttempt < nLastGood) { @@ -711,40 +703,56 @@ std::pair<CAddress, int64_t> AddrManImpl::Select_(bool newOnly) const // use a tried node double fChanceFactor = 1.0; while (1) { + // Pick a tried bucket, and an initial position in that bucket. int nKBucket = insecure_rand.randrange(ADDRMAN_TRIED_BUCKET_COUNT); int nKBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE); - while (vvTried[nKBucket][nKBucketPos] == -1) { - nKBucket = (nKBucket + insecure_rand.randbits(ADDRMAN_TRIED_BUCKET_COUNT_LOG2)) % ADDRMAN_TRIED_BUCKET_COUNT; - nKBucketPos = (nKBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE; + // Iterate over the positions of that bucket, starting at the initial one, + // and looping around. + int i; + for (i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { + if (vvTried[nKBucket][(nKBucketPos + i) % ADDRMAN_BUCKET_SIZE] != -1) break; } - int nId = vvTried[nKBucket][nKBucketPos]; + // If the bucket is entirely empty, start over with a (likely) different one. + if (i == ADDRMAN_BUCKET_SIZE) continue; + // Find the entry to return. + int nId = vvTried[nKBucket][(nKBucketPos + i) % ADDRMAN_BUCKET_SIZE]; const auto it_found{mapInfo.find(nId)}; assert(it_found != mapInfo.end()); const AddrInfo& info{it_found->second}; + // With probability GetChance() * fChanceFactor, return the entry. if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) { LogPrint(BCLog::ADDRMAN, "Selected %s from tried\n", info.ToString()); return {info, info.nLastTry}; } + // Otherwise start over with a (likely) different bucket, and increased chance factor. fChanceFactor *= 1.2; } } else { // use a new node double fChanceFactor = 1.0; while (1) { + // Pick a new bucket, and an initial position in that bucket. int nUBucket = insecure_rand.randrange(ADDRMAN_NEW_BUCKET_COUNT); int nUBucketPos = insecure_rand.randrange(ADDRMAN_BUCKET_SIZE); - while (vvNew[nUBucket][nUBucketPos] == -1) { - nUBucket = (nUBucket + insecure_rand.randbits(ADDRMAN_NEW_BUCKET_COUNT_LOG2)) % ADDRMAN_NEW_BUCKET_COUNT; - nUBucketPos = (nUBucketPos + insecure_rand.randbits(ADDRMAN_BUCKET_SIZE_LOG2)) % ADDRMAN_BUCKET_SIZE; + // Iterate over the positions of that bucket, starting at the initial one, + // and looping around. + int i; + for (i = 0; i < ADDRMAN_BUCKET_SIZE; ++i) { + if (vvNew[nUBucket][(nUBucketPos + i) % ADDRMAN_BUCKET_SIZE] != -1) break; } - int nId = vvNew[nUBucket][nUBucketPos]; + // If the bucket is entirely empty, start over with a (likely) different one. + if (i == ADDRMAN_BUCKET_SIZE) continue; + // Find the entry to return. + int nId = vvNew[nUBucket][(nUBucketPos + i) % ADDRMAN_BUCKET_SIZE]; const auto it_found{mapInfo.find(nId)}; assert(it_found != mapInfo.end()); const AddrInfo& info{it_found->second}; + // With probability GetChance() * fChanceFactor, return the entry. if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) { LogPrint(BCLog::ADDRMAN, "Selected %s from new\n", info.ToString()); return {info, info.nLastTry}; } + // Otherwise start over with a (likely) different bucket, and increased chance factor. fChanceFactor *= 1.2; } } @@ -800,10 +808,6 @@ void AddrManImpl::Connected_(const CService& addr, int64_t nTime) AddrInfo& info = *pinfo; - // check whether we are talking about the exact same CService (including same port) - if (info != addr) - return; - // update info int64_t nUpdateInterval = 20 * 60; if (nTime - info.nTime > nUpdateInterval) @@ -822,10 +826,6 @@ void AddrManImpl::SetServices_(const CService& addr, ServiceFlags nServices) AddrInfo& info = *pinfo; - // check whether we are talking about the exact same CService (including same port) - if (info != addr) - return; - // update info info.nServices = nServices; } diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 1dc7f25f9c..f8191d6b85 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -179,8 +179,8 @@ private: //! table with information about all nIds std::unordered_map<int, AddrInfo> mapInfo GUARDED_BY(cs); - //! find an nId based on its network address - std::unordered_map<CNetAddr, int, CNetAddrHash> mapAddr GUARDED_BY(cs); + //! find an nId based on its network address and port. + std::unordered_map<CService, int, CServiceHash> mapAddr GUARDED_BY(cs); //! randomly-ordered vector of all nIds //! This is mutable because it is unobservable outside the class, so any @@ -225,7 +225,7 @@ private: const std::vector<bool> m_asmap; //! Find an entry. - AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); + AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Create a new entry and add it to the internal data structures mapInfo, mapAddr and vRandom. AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 934b574f8b..fd5145950b 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -33,7 +33,6 @@ static void CoinSelection(benchmark::Bench& bench) NodeContext node; auto chain = interfaces::MakeChain(node); CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); - wallet.SetupLegacyScriptPubKeyMan(); std::vector<std::unique_ptr<CWalletTx>> wtxs; LOCK(wallet.cs_wallet); @@ -65,10 +64,6 @@ static void CoinSelection(benchmark::Bench& bench) } typedef std::set<CInputCoin> CoinSet; -static NodeContext testNode; -static auto testChain = interfaces::MakeChain(testNode); -static CWallet testWallet(testChain.get(), "", CreateDummyWalletDatabase()); -std::vector<std::unique_ptr<CWalletTx>> wtxn; // Copied from src/wallet/test/coinselector_tests.cpp static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>& set) @@ -76,10 +71,9 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup> CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; - std::unique_ptr<CWalletTx> wtx = std::make_unique<CWalletTx>(MakeTransactionRef(std::move(tx))); + CInputCoin coin(MakeTransactionRef(tx), nInput); set.emplace_back(); - set.back().Insert(COutput(testWallet, *wtx, nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false); - wtxn.emplace_back(std::move(wtx)); + set.back().Insert(coin, 0, true, 0, 0, false); } // Copied from src/wallet/test/coinselector_tests.cpp static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool) @@ -97,7 +91,6 @@ static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool) static void BnBExhaustion(benchmark::Bench& bench) { // Setup - testWallet.SetupLegacyScriptPubKeyMan(); std::vector<OutputGroup> utxo_pool; CoinSet selection; CAmount value_ret = 0; diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index f28768efc8..a0a82ea359 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -6,6 +6,7 @@ #include <policy/policy.h> #include <test/util/setup_common.h> #include <txmempool.h> +#include <validation.h> #include <vector> @@ -26,14 +27,8 @@ struct Available { Available(CTransactionRef& ref, size_t tx_count) : ref(ref), tx_count(tx_count){} }; -static void ComplexMemPool(benchmark::Bench& bench) +static std::vector<CTransactionRef> CreateOrderedCoins(FastRandomContext& det_rand, int childTxs, int min_ancestors) { - int childTxs = 800; - if (bench.complexityN() > 1) { - childTxs = static_cast<int>(bench.complexityN()); - } - - FastRandomContext det_rand{true}; std::vector<Available> available_coins; std::vector<CTransactionRef> ordered_coins; // Create some base transactions @@ -58,8 +53,10 @@ static void ComplexMemPool(benchmark::Bench& bench) size_t idx = det_rand.randrange(available_coins.size()); Available coin = available_coins[idx]; uint256 hash = coin.ref->GetHash(); - // biased towards taking just one ancestor, but maybe more - size_t n_to_take = det_rand.randrange(2) == 0 ? 1 : 1+det_rand.randrange(coin.ref->vout.size() - coin.vin_left); + // biased towards taking min_ancestors parents, but maybe more + size_t n_to_take = det_rand.randrange(2) == 0 ? + min_ancestors : + min_ancestors + det_rand.randrange(coin.ref->vout.size() - coin.vin_left); for (size_t i = 0; i < n_to_take; ++i) { tx.vin.emplace_back(); tx.vin.back().prevout = COutPoint(hash, coin.vin_left++); @@ -79,6 +76,17 @@ static void ComplexMemPool(benchmark::Bench& bench) ordered_coins.emplace_back(MakeTransactionRef(tx)); available_coins.emplace_back(ordered_coins.back(), tx_counter++); } + return ordered_coins; +} + +static void ComplexMemPool(benchmark::Bench& bench) +{ + FastRandomContext det_rand{true}; + int childTxs = 800; + if (bench.complexityN() > 1) { + childTxs = static_cast<int>(bench.complexityN()); + } + std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 1); const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN); CTxMemPool pool; LOCK2(cs_main, pool.cs); @@ -91,4 +99,21 @@ static void ComplexMemPool(benchmark::Bench& bench) }); } +static void MempoolCheck(benchmark::Bench& bench) +{ + FastRandomContext det_rand{true}; + const int childTxs = bench.complexityN() > 1 ? static_cast<int>(bench.complexityN()) : 2000; + const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 5); + const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN, {"-checkmempool=1"}); + CTxMemPool pool; + LOCK2(cs_main, pool.cs); + const CCoinsViewCache& coins_tip = testing_setup.get()->m_node.chainman->ActiveChainstate().CoinsTip(); + for (auto& tx : ordered_coins) AddTx(tx, pool); + + bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { + pool.check(coins_tip, /* spendheight */ 2); + }); +} + BENCHMARK(ComplexMemPool); +BENCHMARK(MempoolCheck); diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index a205d8b6e7..166ed16042 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -14,7 +14,7 @@ #include <optional> -static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_watchonly, const bool add_mine) +static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_mine) { const auto test_setup = MakeNoLogFileContext<const TestingSetup>(); @@ -22,13 +22,14 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()}; { - wallet.SetupLegacyScriptPubKeyMan(); + LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet.SetupDescriptorScriptPubKeyMans(); if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); } auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}}); const std::optional<std::string> address_mine{add_mine ? std::optional<std::string>{getnewaddress(wallet)} : std::nullopt}; - if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY); for (int i = 0; i < 100; ++i) { generatetoaddress(test_setup->m_node, address_mine.value_or(ADDRESS_WATCHONLY)); @@ -42,14 +43,13 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b if (set_dirty) wallet.MarkDirty(); bal = GetBalance(wallet); if (add_mine) assert(bal.m_mine_trusted > 0); - if (add_watchonly) assert(bal.m_watchonly_trusted > 0); }); } -static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_watchonly */ true, /* add_mine */ true); } -static void WalletBalanceClean(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ true, /* add_mine */ true); } -static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ false, /* add_mine */ true); } -static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ true, /* add_mine */ false); } +static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_mine */ true); } +static void WalletBalanceClean(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true); } +static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true); } +static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ false); } BENCHMARK(WalletBalanceDirty); BENCHMARK(WalletBalanceClean); diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 43e986a765..b6344ec413 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -551,15 +551,26 @@ public: } // Report peer connection totals by type. - result += " ipv4 ipv6 onion"; - const bool any_i2p_peers = m_counts.at(2).at(3); // false if total i2p peers count is 0, otherwise true - if (any_i2p_peers) result += " i2p"; + result += " "; + std::vector<int8_t> reachable_networks; + for (const UniValue& network : networkinfo["networks"].getValues()) { + if (network["reachable"].get_bool()) { + const std::string& network_name{network["name"].get_str()}; + const int8_t network_id{NetworkStringToId(network_name)}; + if (network_id == UNKNOWN_NETWORK) continue; + result += strprintf("%8s", network_name); // column header + reachable_networks.push_back(network_id); + } + }; result += " total block"; if (m_manual_peers_count) result += " manual"; + const std::array rows{"in", "out", "total"}; - for (uint8_t i = 0; i < 3; ++i) { - result += strprintf("\n%-5s %5i %5i %5i", rows.at(i), m_counts.at(i).at(0), m_counts.at(i).at(1), m_counts.at(i).at(2)); // ipv4/ipv6/onion peers counts - if (any_i2p_peers) result += strprintf(" %5i", m_counts.at(i).at(3)); // i2p peers count + for (size_t i = 0; i < rows.size(); ++i) { + result += strprintf("\n%-5s", rows[i]); // row header + for (int8_t n : reachable_networks) { + result += strprintf("%8i", m_counts.at(i).at(n)); // network peers count + } result += strprintf(" %5i", m_counts.at(i).at(m_networks.size())); // total peers count if (i == 1) { // the outbound row has two extra columns for block relay and manual peer counts result += strprintf(" %5i", m_block_relay_peers_count); diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 765954c92e..21d4df5b01 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -30,6 +30,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman) argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS); argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); + argsman.AddArg("-legacy", "Create legacy wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); diff --git a/src/consensus/amount.h b/src/consensus/amount.h index 8b41a2277d..96566ea13f 100644 --- a/src/consensus/amount.h +++ b/src/consensus/amount.h @@ -11,6 +11,7 @@ /** Amount in satoshis (Can be negative) */ typedef int64_t CAmount; +/** The amount of satoshis in one BTC. */ static constexpr CAmount COIN = 100000000; /** No amount larger than this (in satoshi) is valid. diff --git a/src/crypto/chacha_poly_aead.h b/src/crypto/chacha_poly_aead.h index 0afe8fcc14..6a7998335d 100644 --- a/src/crypto/chacha_poly_aead.h +++ b/src/crypto/chacha_poly_aead.h @@ -117,8 +117,8 @@ static constexpr int AAD_PACKAGES_PER_ROUND = 21; /* 64 / 3 round down*/ class ChaCha20Poly1305AEAD { private: - ChaCha20 m_chacha_main; // payload and poly1305 key-derivation cipher instance - ChaCha20 m_chacha_header; // AAD cipher instance (encrypted length) + ChaCha20 m_chacha_header; // AAD cipher instance (encrypted length) and poly1305 key-derivation cipher instance + ChaCha20 m_chacha_main; // payload unsigned char m_aad_keystream_buffer[CHACHA20_ROUND_OUTPUT]; // aad keystream cache uint64_t m_cached_aad_seqnr; // aad keystream cache hint diff --git a/src/external_signer.cpp b/src/external_signer.cpp index d6388b759a..75070899c6 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -9,6 +9,7 @@ #include <util/system.h> #include <external_signer.h> +#include <algorithm> #include <stdexcept> #include <string> #include <vector> @@ -75,15 +76,14 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str ssTx << psbtx; // Check if signer fingerprint matches any input master key fingerprint - bool match = false; - for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { - const PSBTInput& input = psbtx.inputs[i]; + auto matches_signer_fingerprint = [&](const PSBTInput& input) { for (const auto& entry : input.hd_keypaths) { - if (m_fingerprint == strprintf("%08x", ReadBE32(entry.second.fingerprint))) match = true; + if (m_fingerprint == strprintf("%08x", ReadBE32(entry.second.fingerprint))) return true; } - } + return false; + }; - if (!match) { + if (!std::any_of(psbtx.inputs.begin(), psbtx.inputs.end(), matches_signer_fingerprint)) { error = "Signer fingerprint " + m_fingerprint + " does not match any of the inputs:\n" + EncodeBase64(ssTx.str()); return false; } diff --git a/src/fs.cpp b/src/fs.cpp index 8cae7f32c6..7a99444eef 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -16,6 +16,7 @@ #define NOMINMAX #endif #include <codecvt> +#include <limits> #include <windows.h> #endif diff --git a/src/net.cpp b/src/net.cpp index 7271ff22b2..ad558dd598 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1295,9 +1295,8 @@ void CConnman::NotifyNumConnectionsChanged() } } -bool CConnman::ShouldRunInactivityChecks(const CNode& node, std::optional<int64_t> now_in) const +bool CConnman::ShouldRunInactivityChecks(const CNode& node, int64_t now) const { - const int64_t now = now_in ? now_in.value() : GetTimeSeconds(); return node.nTimeConnected + m_peer_connect_timeout < now; } @@ -942,7 +942,7 @@ public: std::chrono::microseconds PoissonNextSendInbound(std::chrono::microseconds now, std::chrono::seconds average_interval); /** Return true if we should disconnect the peer for failing an inactivity check. */ - bool ShouldRunInactivityChecks(const CNode& node, std::optional<int64_t> now=std::nullopt) const; + bool ShouldRunInactivityChecks(const CNode& node, int64_t secs_now) const; private: struct ListenSocket { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 66b99aa2bb..9f3aa5b4a3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2299,7 +2299,8 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set) break; } } - m_mempool.check(m_chainman.ActiveChainstate()); + CChainState& active_chainstate = m_chainman.ActiveChainstate(); + m_mempool.check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1); } bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, @@ -2960,16 +2961,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, best_block = &inv.hash; } } else if (inv.IsGenTxMsg()) { + if (reject_tx_invs) { + LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId()); + pfrom.fDisconnect = true; + return; + } const GenTxid gtxid = ToGenTxid(inv); const bool fAlreadyHave = AlreadyHaveTx(gtxid); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); pfrom.AddKnownTx(inv.hash); - if (reject_tx_invs) { - LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId()); - pfrom.fDisconnect = true; - return; - } else if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { + if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { AddTxAnnouncement(pfrom, gtxid, current_time); } } else { @@ -3242,12 +3244,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid))) { + if (AlreadyHaveTx(GenTxid::Wtxid(wtxid))) { if (pfrom.HasPermission(NetPermissionFlags::ForceRelay)) { // Always relay transactions received from peers with forcerelay // permission, even if they were already in the mempool, allowing // the node to function as a gateway for nodes hidden behind it. - if (!m_mempool.exists(tx.GetHash())) { + if (!m_mempool.exists(GenTxid::Txid(tx.GetHash()))) { LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); } else { LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); @@ -3261,7 +3263,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, const TxValidationState& state = result.m_state; if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { - m_mempool.check(m_chainman.ActiveChainstate()); + CChainState& active_chainstate = m_chainman.ActiveChainstate(); + m_mempool.check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1); // As this version of the transaction was acceptable, we can forget about any // requests for it. m_txrequest.ForgetTxHash(tx.GetHash()); @@ -3312,7 +3315,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - const GenTxid gtxid{/* is_wtxid=*/false, parent_txid}; + const auto gtxid{GenTxid::Txid(parent_txid)}; pfrom.AddKnownTx(parent_txid); if (!AlreadyHaveTx(gtxid)) AddTxAnnouncement(pfrom, gtxid, current_time); } @@ -4312,8 +4315,11 @@ void PeerManagerImpl::CheckForStaleTipAndEvictPeers() void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now) { - if (m_connman.ShouldRunInactivityChecks(node_to) && peer.m_ping_nonce_sent && + if (m_connman.ShouldRunInactivityChecks(node_to, std::chrono::duration_cast<std::chrono::seconds>(now).count()) && + peer.m_ping_nonce_sent && now > peer.m_ping_start.load() + std::chrono::seconds{TIMEOUT_INTERVAL}) { + // The ping timeout is using mocktime. To disable the check during + // testing, increase -peertimeout. LogPrint(BCLog::NET, "ping timeout: %fs peer=%d\n", 0.000001 * count_microseconds(now - peer.m_ping_start.load()), peer.m_id); node_to.fDisconnect = true; return; diff --git a/src/netaddress.h b/src/netaddress.h index 66c8c48f08..57eb8bc72f 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -253,7 +253,6 @@ public: } } - friend class CNetAddrHash; friend class CSubNet; private: @@ -467,22 +466,6 @@ private: } }; -class CNetAddrHash -{ -public: - size_t operator()(const CNetAddr& a) const noexcept - { - CSipHasher hasher(m_salt_k0, m_salt_k1); - hasher.Write(a.m_net); - hasher.Write(a.m_addr.data(), a.m_addr.size()); - return static_cast<size_t>(hasher.Finalize()); - } - -private: - const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max()); - const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max()); -}; - class CSubNet { protected: @@ -565,6 +548,25 @@ public: READWRITEAS(CNetAddr, obj); READWRITE(Using<BigEndianFormatter<2>>(obj.port)); } + + friend class CServiceHash; +}; + +class CServiceHash +{ +public: + size_t operator()(const CService& a) const noexcept + { + CSipHasher hasher(m_salt_k0, m_salt_k1); + hasher.Write(a.m_net); + hasher.Write(a.port); + hasher.Write(a.m_addr.data(), a.m_addr.size()); + return static_cast<size_t>(hasher.Finalize()); + } + +private: + const uint64_t m_salt_k0 = GetRand(std::numeric_limits<uint64_t>::max()); + const uint64_t m_salt_k1 = GetRand(std::numeric_limits<uint64_t>::max()); }; #endif // BITCOIN_NETADDRESS_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 73f4036057..192caf7994 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -555,7 +555,7 @@ public: { if (!m_node.mempool) return false; LOCK(m_node.mempool->cs); - return m_node.mempool->exists(txid); + return m_node.mempool->exists(GenTxid::Txid(txid)); } bool hasDescendantsInMempool(const uint256& txid) override { diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 7ac2e22006..7e6b0cf245 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -22,7 +22,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) // If this transaction is not in our mempool, then we can't be sure // we will know about all its inputs. - if (!pool.exists(tx.GetHash())) { + if (!pool.exists(GenTxid::Txid(tx.GetHash()))) { return RBFTransactionState::UNKNOWN; } @@ -98,7 +98,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, if (!parents_of_conflicts.count(tx.vin[j].prevout.hash)) { // Rather than check the UTXO set - potentially expensive - it's cheaper to just check // if the new input refers to a tx that's in the mempool. - if (pool.exists(tx.vin[j].prevout.hash)) { + if (pool.exists(GenTxid::Txid(tx.vin[j].prevout.hash))) { return strprintf("replacement %s adds unconfirmed input, idx %d", tx.GetHash().ToString(), j); } diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 46db39f8db..947c1c60bb 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -391,8 +391,11 @@ class GenTxid { bool m_is_wtxid; uint256 m_hash; -public: GenTxid(bool is_wtxid, const uint256& hash) : m_is_wtxid(is_wtxid), m_hash(hash) {} + +public: + static GenTxid Txid(const uint256& hash) { return GenTxid{false, hash}; } + static GenTxid Wtxid(const uint256& hash) { return GenTxid{true, hash}; } bool IsWtxid() const { return m_is_wtxid; } const uint256& GetHash() const { return m_hash; } friend bool operator==(const GenTxid& a, const GenTxid& b) { return a.m_is_wtxid == b.m_is_wtxid && a.m_hash == b.m_hash; } diff --git a/src/protocol.cpp b/src/protocol.cpp index 2e70b41e4c..7506c81815 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -223,5 +223,5 @@ std::vector<std::string> serviceFlagsToStr(uint64_t flags) GenTxid ToGenTxid(const CInv& inv) { assert(inv.IsGenTxMsg()); - return {inv.IsMsgWtx(), inv.hash}; + return inv.IsMsgWtx() ? GenTxid::Wtxid(inv.hash) : GenTxid::Txid(inv.hash); } diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 7de56a648a..5b586b9d89 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -154,10 +154,11 @@ static bool InitSettings() 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)))); + std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"); + std::string error_translated = QCoreApplication::translate("bitcoin-core", error.c_str()).toStdString(); + InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort); + 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?")); @@ -176,10 +177,11 @@ static bool InitSettings() 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)))); + std::string error = QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"); + std::string error_translated = QCoreApplication::translate("bitcoin-core", error.c_str()).toStdString(); + InitError(Untranslated(strprintf("%s:\n%s\n", error, MakeUnorderedList(errors)))); - QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Ok); + 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.*/ diff --git a/src/qt/forms/createwalletdialog.ui b/src/qt/forms/createwalletdialog.ui index b11fb026b0..56adbe17a5 100644 --- a/src/qt/forms/createwalletdialog.ui +++ b/src/qt/forms/createwalletdialog.ui @@ -107,6 +107,9 @@ <property name="text"> <string>Descriptor Wallet</string> </property> + <property name="checked"> + <bool>true</bool> + </property> </widget> </item> <item> diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 2292c01d6a..61b52fd08a 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -184,8 +184,8 @@ static void InitMessage(SplashScreen *splash, const std::string &message) static void ShowProgress(SplashScreen *splash, const std::string &title, int nProgress, bool resume_possible) { InitMessage(splash, title + std::string("\n") + - (resume_possible ? _("(press q to shutdown and continue later)").translated - : _("press q to shutdown").translated) + + (resume_possible ? SplashScreen::tr("(press q to shutdown and continue later)").toStdString() + : SplashScreen::tr("press q to shutdown").toStdString()) + strprintf("\n%d", nProgress) + "%"); } diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 0de781661a..729957699a 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -64,8 +64,12 @@ void TestAddAddressesToSendBook(interfaces::Node& node) test.m_node.wallet_client = wallet_client.get(); node.setContext(&test.m_node); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase()); - wallet->SetupLegacyScriptPubKeyMan(); wallet->LoadWallet(); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + { + LOCK(wallet->cs_wallet); + wallet->SetupDescriptorScriptPubKeyMans(); + } auto build_address = [&wallet]() { CKey key; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 62b135d3f1..c74c8f25b3 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -143,11 +143,20 @@ void TestGUI(interfaces::Node& node) node.setContext(&test.m_node); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); { - auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); - LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); - wallet->SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type), "", "receive"); - spk_man->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); + LOCK(wallet->cs_wallet); + wallet->SetupDescriptorScriptPubKeyMans(); + + // Add the coinbase key + FlatSigningProvider provider; + std::string error; + std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(test.coinbaseKey) + ")", provider, error, /* require_checksum=*/ false); + assert(desc); + WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1); + if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false); + CTxDestination dest = GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type); + wallet->SetAddressBook(dest, "", "receive"); wallet->SetLastBlockProcessed(105, node.context()->chainman->ActiveChain().Tip()->GetBlockHash()); } { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index dadd82e03f..aa7a55e7a9 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -516,7 +516,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool std::set<std::string> setDepends; for (const CTxIn& txin : tx.vin) { - if (pool.exists(txin.prevout.hash)) + if (pool.exists(GenTxid::Txid(txin.prevout.hash))) setDepends.insert(txin.prevout.hash.ToString()); } diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h index 0e31ad3ce3..c4923dc56f 100644 --- a/src/support/allocators/secure.h +++ b/src/support/allocators/secure.h @@ -9,6 +9,7 @@ #include <support/lockedpool.h> #include <support/cleanse.h> +#include <memory> #include <string> // @@ -17,15 +18,13 @@ // template <typename T> struct secure_allocator : public std::allocator<T> { - // MSVC8 default copy constructor is broken - typedef std::allocator<T> base; - typedef typename base::size_type size_type; - typedef typename base::difference_type difference_type; - typedef typename base::pointer pointer; - typedef typename base::const_pointer const_pointer; - typedef typename base::reference reference; - typedef typename base::const_reference const_reference; - typedef typename base::value_type value_type; + using base = std::allocator<T>; + using traits = std::allocator_traits<base>; + using size_type = typename traits::size_type; + using difference_type = typename traits::difference_type; + using pointer = typename traits::pointer; + using const_pointer = typename traits::const_pointer; + using value_type = typename traits::value_type; secure_allocator() noexcept {} secure_allocator(const secure_allocator& a) noexcept : base(a) {} template <typename U> diff --git a/src/support/allocators/zeroafterfree.h b/src/support/allocators/zeroafterfree.h index 418f0ee656..77de4b1e69 100644 --- a/src/support/allocators/zeroafterfree.h +++ b/src/support/allocators/zeroafterfree.h @@ -13,15 +13,13 @@ template <typename T> struct zero_after_free_allocator : public std::allocator<T> { - // MSVC8 default copy constructor is broken - typedef std::allocator<T> base; - typedef typename base::size_type size_type; - typedef typename base::difference_type difference_type; - typedef typename base::pointer pointer; - typedef typename base::const_pointer const_pointer; - typedef typename base::reference reference; - typedef typename base::const_reference const_reference; - typedef typename base::value_type value_type; + using base = std::allocator<T>; + using traits = std::allocator_traits<base>; + using size_type = typename traits::size_type; + using difference_type = typename traits::difference_type; + using pointer = typename traits::pointer; + using const_pointer = typename traits::const_pointer; + using value_type = typename traits::value_type; zero_after_free_allocator() noexcept {} zero_after_free_allocator(const zero_after_free_allocator& a) noexcept : base(a) {} template <typename U> diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index bd6f470219..991bfa5efc 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -89,7 +89,7 @@ public: deterministic = makeDeterministic; } - AddrInfo* Find(const CNetAddr& addr, int* pnId = nullptr) + AddrInfo* Find(const CService& addr, int* pnId = nullptr) { LOCK(m_impl->cs); return m_impl->Find(addr, pnId); @@ -222,15 +222,15 @@ BOOST_AUTO_TEST_CASE(addrman_ports) 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_EQUAL(addrman.size(), 1U); + BOOST_CHECK(addrman.Add({CAddress(addr1_port, NODE_NONE)}, source)); + BOOST_CHECK_EQUAL(addrman.size(), 2U); auto addr_ret2 = addrman.Select().first; - BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333"); + BOOST_CHECK(addr_ret2.ToString() == "250.1.1.1:8333" || addr_ret2.ToString() == "250.1.1.1:8334"); - // Test: Add same IP but diff port to tried table, it doesn't get added. - // Perhaps this is not ideal behavior but it is the current behavior. + // Test: Add same IP but diff port to tried table; this converts the entry with + // the specified port to tried, but not the other. addrman.Good(CAddress(addr1_port, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.size(), 1U); + BOOST_CHECK_EQUAL(addrman.size(), 2U); bool newOnly = true; auto addr_ret3 = addrman.Select(newOnly).first; BOOST_CHECK_EQUAL(addr_ret3.ToString(), "250.1.1.1:8333"); @@ -369,18 +369,18 @@ BOOST_AUTO_TEST_CASE(addrman_find) CNetAddr source2 = ResolveIP("250.1.2.2"); BOOST_CHECK(addrman.Add({addr1}, source1)); - BOOST_CHECK(!addrman.Add({addr2}, source2)); + BOOST_CHECK(addrman.Add({addr2}, source2)); BOOST_CHECK(addrman.Add({addr3}, source1)); - // Test: ensure Find returns an IP matching what we searched on. + // Test: ensure Find returns an IP/port matching what we searched on. AddrInfo* info1 = addrman.Find(addr1); BOOST_REQUIRE(info1); BOOST_CHECK_EQUAL(info1->ToString(), "250.1.2.1:8333"); - // Test 18; Find does not discriminate by port number. + // Test; Find discriminates by port number. AddrInfo* info2 = addrman.Find(addr2); BOOST_REQUIRE(info2); - BOOST_CHECK_EQUAL(info2->ToString(), info1->ToString()); + BOOST_CHECK_EQUAL(info2->ToString(), "250.1.2.1:9999"); // Test: Find returns another IP matching what we searched on. AddrInfo* info3 = addrman.Find(addr3); diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index 5b3b39fdb8..1483bd3cb3 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -694,8 +694,8 @@ BOOST_AUTO_TEST_CASE(chacha20_poly1305_aead_testvector) TestChaCha20Poly1305AEAD(true, 0, /* m */ "0000000000000000000000000000000000000000000000000000000000000000", - /* k1 (payload) */ "0000000000000000000000000000000000000000000000000000000000000000", - /* k2 (AAD) */ "0000000000000000000000000000000000000000000000000000000000000000", + /* k1 (AAD) */ "0000000000000000000000000000000000000000000000000000000000000000", + /* k2 (payload) */ "0000000000000000000000000000000000000000000000000000000000000000", /* AAD keystream */ "76b8e0ada0f13d90405d6ae55386bd28bdd219b8a08ded1aa836efcc8b770dc7da41597c5157488d7724e03fb8d84a376a43b8f41518a11cc387b669b2ee6586", /* encrypted message & MAC */ "76b8e09f07e7be5551387a98ba977c732d080dcb0f29a048e3656912c6533e32d2fc11829c1b6c1df1f551cd6131ff08", /* encrypted message & MAC at sequence 999 */ "b0a03d5bd2855d60699e7d3a3133fa47be740fe4e4c1f967555e2d9271f31c3aaa7aa16ec62c5e24f040c08bb20c3598"); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 0bfe6eecd9..668ff150ee 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -52,6 +52,8 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { const CChainParams& chainparams = Params(); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); + // Disable inactivity checks for this test to avoid interference + static_cast<ConnmanTestMsg*>(connman.get())->SetPeerConnectTimeout(99999); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, false); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 8df3707fc9..c6df6a0e61 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -137,24 +137,29 @@ public: // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. // Keys may be different. - using AddrInfoHasher = std::function<size_t(const AddrInfo&)>; - using AddrInfoEq = std::function<bool(const AddrInfo&, const AddrInfo&)>; - - CNetAddrHash netaddr_hasher; - - AddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const AddrInfo& a) { - return netaddr_hasher(static_cast<CNetAddr>(a)) ^ netaddr_hasher(a.source) ^ - a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried; + auto addrinfo_hasher = [](const AddrInfo& a) { + CSipHasher hasher(0, 0); + auto addr_key = a.GetKey(); + auto source_key = a.source.GetAddrBytes(); + hasher.Write(a.nLastSuccess); + hasher.Write(a.nAttempts); + hasher.Write(a.nRefCount); + hasher.Write(a.fInTried); + hasher.Write(a.GetNetwork()); + hasher.Write(a.source.GetNetwork()); + hasher.Write(addr_key.size()); + hasher.Write(source_key.size()); + hasher.Write(addr_key.data(), addr_key.size()); + hasher.Write(source_key.data(), source_key.size()); + return (size_t)hasher.Finalize(); }; - AddrInfoEq addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& 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; + auto addrinfo_eq = [](const AddrInfo& lhs, const AddrInfo& rhs) { + return std::tie(static_cast<const CService&>(lhs), lhs.source, lhs.nLastSuccess, lhs.nAttempts, lhs.nRefCount, lhs.fInTried) == + std::tie(static_cast<const CService&>(rhs), rhs.source, rhs.nLastSuccess, rhs.nAttempts, rhs.nRefCount, rhs.fInTried); }; - using Addresses = std::unordered_set<AddrInfo, AddrInfoHasher, AddrInfoEq>; + using Addresses = std::unordered_set<AddrInfo, decltype(addrinfo_hasher), decltype(addrinfo_eq)>; const size_t num_addresses{m_impl->mapInfo.size()}; diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 6201cc813c..17b5ef88b9 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -81,7 +81,7 @@ void SetMempoolConstraints(ArgsManager& args, FuzzedDataProvider& fuzzed_data_pr void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, CChainState& chainstate) { - WITH_LOCK(::cs_main, tx_pool.check(chainstate)); + WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1)); { BlockAssembler::Options options; options.nBlockMaxWeight = fuzzed_data_provider.ConsumeIntegralInRange(0U, MAX_BLOCK_WEIGHT); @@ -97,7 +97,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, CCh std::vector<uint256> all_txids; tx_pool.queryHashes(all_txids); assert(all_txids.size() < info_all.size()); - WITH_LOCK(::cs_main, tx_pool.check(chainstate)); + WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1)); } SyncWithValidationInterfaceQueue(); } diff --git a/src/test/fuzz/txrequest.cpp b/src/test/fuzz/txrequest.cpp index 72438ff2d7..a73bbcfc25 100644 --- a/src/test/fuzz/txrequest.cpp +++ b/src/test/fuzz/txrequest.cpp @@ -204,7 +204,7 @@ public: } // Call TxRequestTracker's implementation. - m_tracker.ReceivedInv(peer, GenTxid{is_wtxid, TXHASHES[txhash]}, preferred, reqtime); + m_tracker.ReceivedInv(peer, is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash]), preferred, reqtime); } void RequestedTx(int peer, int txhash, std::chrono::microseconds exptime) @@ -252,7 +252,7 @@ public: for (int peer2 = 0; peer2 < MAX_PEERS; ++peer2) { Announcement& ann2 = m_announcements[txhash][peer2]; if (ann2.m_state == State::REQUESTED && ann2.m_time <= m_now) { - expected_expired.emplace_back(peer2, GenTxid{ann2.m_is_wtxid, TXHASHES[txhash]}); + expected_expired.emplace_back(peer2, ann2.m_is_wtxid ? GenTxid::Wtxid(TXHASHES[txhash]) : GenTxid::Txid(TXHASHES[txhash])); ann2.m_state = State::COMPLETED; break; } diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index bf36f8a6c9..b3497b8ef8 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -444,12 +444,12 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(entry.Fee(5000LL).FromTx(tx2)); pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing - BOOST_CHECK(pool.exists(tx1.GetHash())); - BOOST_CHECK(pool.exists(tx2.GetHash())); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx1.GetHash()))); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx2.GetHash()))); pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // should remove the lower-feerate transaction - BOOST_CHECK(pool.exists(tx1.GetHash())); - BOOST_CHECK(!pool.exists(tx2.GetHash())); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx1.GetHash()))); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx2.GetHash()))); pool.addUnchecked(entry.FromTx(tx2)); CMutableTransaction tx3 = CMutableTransaction(); @@ -462,14 +462,14 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(entry.Fee(20000LL).FromTx(tx3)); pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP) - BOOST_CHECK(!pool.exists(tx1.GetHash())); - BOOST_CHECK(pool.exists(tx2.GetHash())); - BOOST_CHECK(pool.exists(tx3.GetHash())); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx1.GetHash()))); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx2.GetHash()))); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx3.GetHash()))); pool.TrimToSize(GetVirtualTransactionSize(CTransaction(tx1))); // mempool is limited to tx1's size in memory usage, so nothing fits - BOOST_CHECK(!pool.exists(tx1.GetHash())); - BOOST_CHECK(!pool.exists(tx2.GetHash())); - BOOST_CHECK(!pool.exists(tx3.GetHash())); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx1.GetHash()))); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx2.GetHash()))); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx3.GetHash()))); CFeeRate maxFeeRateRemoved(25000, GetVirtualTransactionSize(CTransaction(tx3)) + GetVirtualTransactionSize(CTransaction(tx2))); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); @@ -529,19 +529,19 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) // we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that pool.TrimToSize(pool.DynamicMemoryUsage() - 1); - BOOST_CHECK(pool.exists(tx4.GetHash())); - BOOST_CHECK(pool.exists(tx6.GetHash())); - BOOST_CHECK(!pool.exists(tx7.GetHash())); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx4.GetHash()))); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx6.GetHash()))); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx7.GetHash()))); - if (!pool.exists(tx5.GetHash())) + if (!pool.exists(GenTxid::Txid(tx5.GetHash()))) pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5)); pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7)); pool.TrimToSize(pool.DynamicMemoryUsage() / 2); // should maximize mempool size by only removing 5/7 - BOOST_CHECK(pool.exists(tx4.GetHash())); - BOOST_CHECK(!pool.exists(tx5.GetHash())); - BOOST_CHECK(pool.exists(tx6.GetHash())); - BOOST_CHECK(!pool.exists(tx7.GetHash())); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx4.GetHash()))); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx5.GetHash()))); + BOOST_CHECK(pool.exists(GenTxid::Txid(tx6.GetHash()))); + BOOST_CHECK(!pool.exists(GenTxid::Txid(tx7.GetHash()))); pool.addUnchecked(entry.Fee(1000LL).FromTx(tx5)); pool.addUnchecked(entry.Fee(9000LL).FromTx(tx7)); diff --git a/src/test/txrequest_tests.cpp b/src/test/txrequest_tests.cpp index 1d137b03b1..99d41882c9 100644 --- a/src/test/txrequest_tests.cpp +++ b/src/test/txrequest_tests.cpp @@ -221,7 +221,7 @@ public: /** Generate a random GenTxid; the txhash follows NewTxHash; the is_wtxid flag is random. */ GenTxid NewGTxid(const std::vector<std::vector<NodeId>>& orders = {}) { - return {InsecureRandBool(), NewTxHash(orders)}; + return InsecureRandBool() ? GenTxid::Wtxid(NewTxHash(orders)) : GenTxid::Txid(NewTxHash(orders)); } /** Generate a new random NodeId to use as peer. The same NodeId is never returned twice @@ -494,8 +494,8 @@ void BuildWtxidTest(Scenario& scenario, int config) auto peerT = scenario.NewPeer(); auto peerW = scenario.NewPeer(); auto txhash = scenario.NewTxHash(); - GenTxid txid{false, txhash}; - GenTxid wtxid{true, txhash}; + auto txid{GenTxid::Txid(txhash)}; + auto wtxid{GenTxid::Wtxid(txhash)}; auto reqtimeT = InsecureRandBool() ? MIN_TIME : scenario.Now() + RandomTime8s(); auto reqtimeW = InsecureRandBool() ? MIN_TIME : scenario.Now() + RandomTime8s(); diff --git a/src/test/util/net.h b/src/test/util/net.h index 939ec322ed..d89fc34b75 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -17,6 +17,12 @@ struct ConnmanTestMsg : public CConnman { using CConnman::CConnman; + + void SetPeerConnectTimeout(int64_t timeout) + { + m_peer_connect_timeout = timeout; + } + void AddTestNode(CNode& node) { LOCK(cs_vNodes); diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index 061659818f..76c1bf93a5 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -25,16 +25,4 @@ std::string getnewaddress(CWallet& w) return EncodeDestination(dest); } -void importaddress(CWallet& wallet, const std::string& address) -{ - auto spk_man = wallet.GetLegacyScriptPubKeyMan(); - LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore); - const auto dest = DecodeDestination(address); - assert(IsValidDestination(dest)); - const auto script = GetScriptForDestination(dest); - wallet.MarkDirty(); - assert(!spk_man->HaveWatchOnly(script)); - if (!spk_man->AddWatchOnly(script, 0 /* nCreateTime */)) assert(false); - wallet.SetAddressBook(dest, /* label */ "", "receive"); -} #endif // ENABLE_WALLET diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5a93f30c8a..9bc2377c63 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -5,6 +5,7 @@ #include <txmempool.h> +#include <coins.h> #include <consensus/consensus.h> #include <consensus/tx_verify.h> #include <consensus/validation.h> @@ -671,16 +672,7 @@ void CTxMemPool::clear() _clear(); } -static void CheckInputsAndUpdateCoins(const CTransaction& tx, CCoinsViewCache& mempoolDuplicate, const int64_t spendheight) -{ - TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass - CAmount txfee = 0; - bool fCheckResult = tx.IsCoinBase() || Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee); - assert(fCheckResult); - UpdateCoins(tx, mempoolDuplicate, std::numeric_limits<int>::max()); -} - -void CTxMemPool::check(CChainState& active_chainstate) const +void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const { if (m_check_ratio == 0) return; @@ -693,20 +685,16 @@ void CTxMemPool::check(CChainState& active_chainstate) const uint64_t checkTotal = 0; CAmount check_total_fee{0}; uint64_t innerUsage = 0; + uint64_t prev_ancestor_count{0}; - CCoinsViewCache& active_coins_tip = active_chainstate.CoinsTip(); CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(&active_coins_tip)); - const int64_t spendheight = active_chainstate.m_chain.Height() + 1; - std::list<const CTxMemPoolEntry*> waitingOnDependants; - for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - unsigned int i = 0; + for (const auto& it : GetSortedDepthAndScore()) { checkTotal += it->GetTxSize(); check_total_fee += it->GetFee(); innerUsage += it->DynamicMemoryUsage(); const CTransaction& tx = it->GetTx(); innerUsage += memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst()); - bool fDependsWait = false; CTxMemPoolEntry::Parents setParentCheck; for (const CTxIn &txin : tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. @@ -714,17 +702,17 @@ void CTxMemPool::check(CChainState& active_chainstate) const if (it2 != mapTx.end()) { const CTransaction& tx2 = it2->GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); - fDependsWait = true; setParentCheck.insert(*it2); - } else { - assert(active_coins_tip.HaveCoin(txin.prevout)); } + // We are iterating through the mempool entries sorted in order by ancestor count. + // All parents must have been checked before their children and their coins added to + // the mempoolDuplicate coins cache. + assert(mempoolDuplicate.HaveCoin(txin.prevout)); // Check whether its inputs are marked in mapNextTx. auto it3 = mapNextTx.find(txin.prevout); assert(it3 != mapNextTx.end()); assert(it3->first == &txin.prevout); assert(it3->second == &tx); - i++; } auto comp = [](const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) -> bool { return a.GetTx().GetHash() == b.GetTx().GetHash(); @@ -751,6 +739,9 @@ void CTxMemPool::check(CChainState& active_chainstate) const assert(it->GetSizeWithAncestors() == nSizeCheck); assert(it->GetSigOpCostWithAncestors() == nSigOpCheck); assert(it->GetModFeesWithAncestors() == nFeesCheck); + // Sanity check: we are walking in ascending ancestor count order. + assert(prev_ancestor_count <= it->GetCountWithAncestors()); + prev_ancestor_count = it->GetCountWithAncestors(); // Check children against mapNextTx CTxMemPoolEntry::Children setChildrenCheck; @@ -769,24 +760,12 @@ void CTxMemPool::check(CChainState& active_chainstate) const // just a sanity check, not definitive that this calc is correct... assert(it->GetSizeWithDescendants() >= child_sizes + it->GetTxSize()); - if (fDependsWait) - waitingOnDependants.push_back(&(*it)); - else { - CheckInputsAndUpdateCoins(tx, mempoolDuplicate, spendheight); - } - } - unsigned int stepsSinceLastRemove = 0; - while (!waitingOnDependants.empty()) { - const CTxMemPoolEntry* entry = waitingOnDependants.front(); - waitingOnDependants.pop_front(); - if (!mempoolDuplicate.HaveInputs(entry->GetTx())) { - waitingOnDependants.push_back(entry); - stepsSinceLastRemove++; - assert(stepsSinceLastRemove < waitingOnDependants.size()); - } else { - CheckInputsAndUpdateCoins(entry->GetTx(), mempoolDuplicate, spendheight); - stepsSinceLastRemove = 0; - } + TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass + CAmount txfee = 0; + assert(!tx.IsCoinBase()); + assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee)); + for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout); + AddCoins(mempoolDuplicate, tx, std::numeric_limits<int>::max()); } for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) { uint256 hash = it->second->GetHash(); @@ -895,8 +874,6 @@ TxMempoolInfo CTxMemPool::info(const GenTxid& gtxid) const return GetInfo(i); } -TxMempoolInfo CTxMemPool::info(const uint256& txid) const { return info(GenTxid{false, txid}); } - void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta) { { @@ -969,7 +946,7 @@ CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>& hashes) c bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const { for (unsigned int i = 0; i < tx.vin.size(); i++) - if (exists(tx.vin[i].prevout.hash)) + if (exists(GenTxid::Txid(tx.vin[i].prevout.hash))) return false; return true; } @@ -1140,7 +1117,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends if (pvNoSpendsRemaining) { for (const CTransaction& tx : txn) { for (const CTxIn& txin : tx.vin) { - if (exists(txin.prevout.hash)) continue; + if (exists(GenTxid::Txid(txin.prevout.hash))) continue; pvNoSpendsRemaining->push_back(txin.prevout); } } diff --git a/src/txmempool.h b/src/txmempool.h index 460e9d0ceb..90b2aee371 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -622,7 +622,7 @@ public: * all inputs are in the mapNextTx array). If sanity-checking is turned off, * check does nothing. */ - void check(CChainState& active_chainstate) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); // addUnchecked must updated state for all ancestors of a given transaction, // to track size/count of descendant transactions. First version of @@ -782,7 +782,6 @@ public: } return (mapTx.count(gtxid.GetHash()) != 0); } - bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}); } CTransactionRef get(const uint256& hash) const; txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs) @@ -790,7 +789,6 @@ public: AssertLockHeld(cs); return mapTx.project<0>(mapTx.get<index_by_wtxid>().find(wtxid)); } - TxMempoolInfo info(const uint256& hash) const; TxMempoolInfo info(const GenTxid& gtxid) const; std::vector<TxMempoolInfo> infoAll() const; @@ -802,7 +800,7 @@ public: LOCK(cs); // Sanity check the transaction is in the mempool & insert into // unbroadcast set. - if (exists(txid)) m_unbroadcast_txids.insert(txid); + if (exists(GenTxid::Txid(txid))) m_unbroadcast_txids.insert(txid); }; /** Removes a transaction from the unbroadcast set */ diff --git a/src/txrequest.cpp b/src/txrequest.cpp index f8d7a1ece8..7d478a5b26 100644 --- a/src/txrequest.cpp +++ b/src/txrequest.cpp @@ -300,7 +300,7 @@ std::map<uint256, TxHashInfo> ComputeTxHashInfo(const Index& index, const Priori GenTxid ToGenTxid(const Announcement& ann) { - return {ann.m_is_wtxid, ann.m_txhash}; + return ann.m_is_wtxid ? GenTxid::Wtxid(ann.m_txhash) : GenTxid::Txid(ann.m_txhash); } } // namespace diff --git a/src/validation.cpp b/src/validation.cpp index ff71020ebb..207cdc8233 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -355,7 +355,7 @@ void CChainState::MaybeUpdateMempoolForReorg( // If the transaction doesn't make it in to the mempool, remove any // transactions that depend on it (which would now be orphans). m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG); - } else if (m_mempool->exists((*it)->GetHash())) { + } else if (m_mempool->exists(GenTxid::Txid((*it)->GetHash()))) { vHashUpdate.push_back((*it)->GetHash()); } ++it; @@ -585,10 +585,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (!CheckFinalTx(m_active_chainstate.m_chain.Tip(), tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final"); - if (m_pool.exists(GenTxid(true, tx.GetWitnessHash()))) { + if (m_pool.exists(GenTxid::Wtxid(tx.GetWitnessHash()))) { // Exact transaction already exists in the mempool. return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-already-in-mempool"); - } else if (m_pool.exists(GenTxid(false, tx.GetHash()))) { + } else if (m_pool.exists(GenTxid::Txid(tx.GetHash()))) { // Transaction with the same non-witness data but different witness (same txid, different // wtxid) already exists in the mempool. return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-same-nonwitness-data-in-mempool"); @@ -908,7 +908,7 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) // trim mempool and check if tx was trimmed if (!bypass_limits) { LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip(), gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetIntArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); - if (!m_pool.exists(hash)) + if (!m_pool.exists(GenTxid::Txid(hash))) return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); } return true; @@ -1236,12 +1236,6 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund AddCoins(inputs, tx, nHeight); } -void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight) -{ - CTxUndo txundo; - UpdateCoins(tx, inputs, txundo, nHeight); -} - bool CScriptCheck::operator()() { const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness; @@ -2487,7 +2481,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex // any disconnected transactions back to the mempool. MaybeUpdateMempoolForReorg(disconnectpool, true); } - if (m_mempool) m_mempool->check(*this); + if (m_mempool) m_mempool->check(this->CoinsTip(), this->m_chain.Height() + 1); CheckForkWarningConditions(); @@ -4500,7 +4494,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka // wallet(s) having loaded it while we were processing // mempool transactions; consider these as valid, instead of // failed, but mark them as 'already there' - if (pool.exists(tx->GetHash())) { + if (pool.exists(GenTxid::Txid(tx->GetHash()))) { ++already_there; } else { ++failed; diff --git a/src/validation.h b/src/validation.h index 1c23b3ad5c..4da8ec8d24 100644 --- a/src/validation.h +++ b/src/validation.h @@ -229,9 +229,6 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx const Package& txns, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -/** Apply the effects of this transaction on the UTXO set represented by view */ -void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); - /** Transaction validation functions */ /** diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 2290e119fd..74fc10ab25 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -132,7 +132,7 @@ bool BerkeleyEnvironment::Open(bilingual_str& err) fs::path pathIn = fs::PathFromString(strPath); TryCreateDirectories(pathIn); if (!LockDirectory(pathIn, ".walletlock")) { - LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath); + LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance may be using it.\n", strPath); err = strprintf(_("Error initializing wallet database environment %s!"), fs::quoted(fs::PathToString(Directory()))); return false; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6959466d1d..f2f28c83ff 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1389,8 +1389,9 @@ static const std::vector<RPCResult> TransactionDescriptionString() { return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, - {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if transaction only input is a coinbase one."}, - {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Only present if we consider transaction to be trusted and so safe to spend from."}, + {RPCResult::Type::BOOL, "generated", /* optional */ true, "Only present if the transaction's only input is a coinbase one."}, + {RPCResult::Type::BOOL, "trusted", /* optional */ true, "Whether we consider the transaction to be trusted and safe to spend from.\n" + "Only present when the transaction has 0 confirmations (or negative confirmations, if conflicted)."}, {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "The block hash containing the transaction."}, {RPCResult::Type::NUM, "blockheight", /* optional */ true, "The block height containing the transaction."}, {RPCResult::Type::NUM, "blockindex", /* optional */ true, "The index of the transaction in the block that includes it."}, @@ -1408,7 +1409,7 @@ static const std::vector<RPCResult> TransactionDescriptionString() {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR, "comment", /* optional */ true, "If a comment is associated with the transaction, only present if not empty."}, {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - "may be unknown for unconfirmed transactions not in the mempool"}}; + "may be unknown for unconfirmed transactions not in the mempool."}}; } static RPCHelpMan listtransactions() @@ -2770,7 +2771,7 @@ static RPCHelpMan createwallet() {"blank", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."}, {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, - {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation"}, + {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation"}, {"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."}, {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."}, }, @@ -2812,12 +2813,11 @@ static RPCHelpMan createwallet() if (!request.params[4].isNull() && request.params[4].get_bool()) { flags |= WALLET_FLAG_AVOID_REUSE; } - if (!request.params[5].isNull() && request.params[5].get_bool()) { + if (request.params[5].isNull() || request.params[5].get_bool()) { #ifndef USE_SQLITE throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without sqlite support (required for descriptor wallets)"); #endif flags |= WALLET_FLAG_DESCRIPTORS; - warnings.emplace_back(Untranslated("Wallet is an experimental descriptor wallet")); } if (!request.params[7].isNull() && request.params[7].get_bool()) { #ifdef ENABLE_EXTERNAL_SIGNER diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 650e083e8e..c493b96248 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -228,7 +228,7 @@ void SQLiteDatabase::Open() // Now begin a transaction to acquire the exclusive lock. This lock won't be released until we close because of the exclusive locking mode. int ret = sqlite3_exec(m_db, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr, nullptr); if (ret != SQLITE_OK) { - throw std::runtime_error("SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another bitcoind?\n"); + throw std::runtime_error("SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of " PACKAGE_NAME "?\n"); } ret = sqlite3_exec(m_db, "COMMIT", nullptr, nullptr, nullptr); if (ret != SQLITE_OK) { diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f80c4637b8..8606924bb3 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -28,19 +28,9 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup) typedef std::set<CInputCoin> CoinSet; -static std::vector<COutput> vCoins; -static NodeContext testNode; -static auto testChain = interfaces::MakeChain(testNode); -static CWallet testWallet(testChain.get(), "", CreateDummyWalletDatabase()); -static CAmount balance = 0; - -CoinEligibilityFilter filter_standard(1, 6, 0); -CoinEligibilityFilter filter_confirmed(1, 1, 0); -CoinEligibilityFilter filter_standard_extra(6, 6, 0); -CoinSelectionParams coin_selection_params(/* change_output_size= */ 0, - /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0), - /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), - /* tx_noinputs_size= */ 0, /* avoid_partial= */ false); +static const CoinEligibilityFilter filter_standard(1, 6, 0); +static const CoinEligibilityFilter filter_confirmed(1, 1, 0); +static const CoinEligibilityFilter filter_standard_extra(6, 6, 0); static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) { @@ -62,9 +52,8 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe set.insert(coin); } -static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false) +static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false) { - balance += nValue; static int nextLockTime = 0; CMutableTransaction tx; tx.nLockTime = nextLockTime++; // so all transactions get different hashes @@ -82,24 +71,19 @@ static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bo // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - CWalletTx* wtx = wallet.AddToWallet(MakeTransactionRef(std::move(tx)), /* confirm= */ {}); + uint256 txid = tx.GetHash(); + + LOCK(wallet.cs_wallet); + auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)))); + assert(ret.second); + CWalletTx& wtx = (*ret.first).second; if (fIsFromMe) { - wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); - wtx->m_is_cache_empty = false; + wtx.m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); + wtx.m_is_cache_empty = false; } - COutput output(wallet, *wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); - vCoins.push_back(output); -} -static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0, bool spendable = false) -{ - add_coin(testWallet, nValue, nAge, fIsFromMe, nInput, spendable); -} - -static void empty_wallet(void) -{ - vCoins.clear(); - balance = 0; + COutput output(wallet, wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); + coins.push_back(output); } static bool equal_sets(CoinSet a, CoinSet b) @@ -142,20 +126,20 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins) return static_groups; } -inline std::vector<OutputGroup>& KnapsackGroupOutputs(const CoinEligibilityFilter& filter) +inline std::vector<OutputGroup>& KnapsackGroupOutputs(const std::vector<COutput>& coins, CWallet& wallet, const CoinEligibilityFilter& filter) { + CoinSelectionParams coin_selection_params(/* change_output_size= */ 0, + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_noinputs_size= */ 0, /* avoid_partial= */ false); static std::vector<OutputGroup> static_groups; - static_groups = GroupOutputs(testWallet, vCoins, coin_selection_params, filter, /* positive_only */false); + static_groups = GroupOutputs(wallet, coins, coin_selection_params, filter, /* positive_only */false); return static_groups; } // Branch and bound coin selection tests BOOST_AUTO_TEST_CASE(bnb_search_test) { - - LOCK(testWallet.cs_wallet); - testWallet.SetupLegacyScriptPubKeyMan(); - // Setup std::vector<CInputCoin> utxo_pool; CoinSet selection; @@ -288,196 +272,213 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000), /* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000), /* tx_noinputs_size= */ 0, /* avoid_partial= */ false); - CoinSet setCoinsRet; - CAmount nValueRet; - empty_wallet(); - add_coin(1); - vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(vCoins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); - - // Test fees subtracted from output: - empty_wallet(); - add_coin(1 * CENT); - vCoins.at(0).nInputBytes = 40; - coin_selection_params_bnb.m_subtract_fee_outputs = true; - BOOST_CHECK(SelectCoinsBnB(GroupCoins(vCoins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); - - // Make sure that can use BnB when there are preset inputs - empty_wallet(); { std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); - wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); - add_coin(*wallet, 5 * CENT, 6 * 24, false, 0, true); - add_coin(*wallet, 3 * CENT, 6 * 24, false, 0, true); - add_coin(*wallet, 2 * CENT, 6 * 24, false, 0, true); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + + std::vector<COutput> coins; + CoinSet setCoinsRet; + CAmount nValueRet; + + add_coin(coins, *wallet, 1); + coins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); + + // Test fees subtracted from output: + coins.clear(); + add_coin(coins, *wallet, 1 * CENT); + coins.at(0).nInputBytes = 40; + coin_selection_params_bnb.m_subtract_fee_outputs = true; + BOOST_CHECK(SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); + } + + { + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + + std::vector<COutput> coins; + CoinSet setCoinsRet; + CAmount nValueRet; + + add_coin(coins, *wallet, 5 * CENT, 6 * 24, false, 0, true); + add_coin(coins, *wallet, 3 * CENT, 6 * 24, false, 0, true); + add_coin(coins, *wallet, 2 * CENT, 6 * 24, false, 0, true); CCoinControl coin_control; coin_control.fAllowOtherInputs = true; - coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i)); + coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); - BOOST_CHECK(SelectCoins(*wallet, vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb)); + BOOST_CHECK(SelectCoins(*wallet, coins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb)); } } BOOST_AUTO_TEST_CASE(knapsack_solver_test) { + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + CoinSet setCoinsRet, setCoinsRet2; CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - testWallet.SetupLegacyScriptPubKeyMan(); + std::vector<COutput> coins; // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) { - empty_wallet(); + coins.clear(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); - add_coin(1*CENT, 4); // add a new 1 cent coin + add_coin(coins, *wallet, 1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(1 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); // but we can find a new 1 cent - BOOST_CHECK(KnapsackSolver(1 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(1 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); - add_coin(2*CENT); // add a mature 2 cent coin + add_coin(coins, *wallet, 2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!KnapsackSolver(3 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(3 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); // we can make 3 cents of new coins - BOOST_CHECK(KnapsackSolver(3 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(3 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); - add_coin(5*CENT); // add a mature 5 cent coin, - add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses - add_coin(20*CENT); // and a mature 20 cent coin + add_coin(coins, *wallet, 5*CENT); // add a mature 5 cent coin, + add_coin(coins, *wallet, 10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses + add_coin(coins, *wallet, 20*CENT); // and a mature 20 cent coin // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(filter_standard_extra), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(38 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard_extra), setCoinsRet, nValueRet)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK(KnapsackSolver(37 * CENT, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(37 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK(KnapsackSolver(38 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(38 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK(KnapsackSolver(34 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(34 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK(KnapsackSolver(7 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(7 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK(KnapsackSolver(8 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(8 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK(KnapsackSolver(9 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(9 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - empty_wallet(); + coins.clear(); - add_coin( 6*CENT); - add_coin( 7*CENT); - add_coin( 8*CENT); - add_coin(20*CENT); - add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total + add_coin(coins, *wallet, 6*CENT); + add_coin(coins, *wallet, 7*CENT); + add_coin(coins, *wallet, 8*CENT); + add_coin(coins, *wallet, 20*CENT); + add_coin(coins, *wallet, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK(KnapsackSolver(71 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); - BOOST_CHECK(!KnapsackSolver(72 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(71 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(!KnapsackSolver(72 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total + add_coin(coins, *wallet, 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 + add_coin(coins, *wallet, 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(16 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK(KnapsackSolver(11 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(11 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // check that the smallest bigger coin is used - add_coin( 1*COIN); - add_coin( 2*COIN); - add_coin( 3*COIN); - add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK(KnapsackSolver(95 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + add_coin(coins, *wallet, 1*COIN); + add_coin(coins, *wallet, 2*COIN); + add_coin(coins, *wallet, 3*COIN); + add_coin(coins, *wallet, 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents + BOOST_CHECK(KnapsackSolver(95 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK(KnapsackSolver(195 * CENT, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(195 * CENT, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - empty_wallet(); - add_coin(MIN_CHANGE * 1 / 10); - add_coin(MIN_CHANGE * 2 / 10); - add_coin(MIN_CHANGE * 3 / 10); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 5 / 10); + coins.clear(); + add_coin(coins, *wallet, MIN_CHANGE * 1 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 2 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 3 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 4 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 5 / 10); // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // but if we add a bigger coin, small change is avoided - add_coin(1111*MIN_CHANGE); + add_coin(coins, *wallet, 1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - empty_wallet(); + coins.clear(); for (int j = 0; j < 20; j++) - add_coin(50000 * COIN); + add_coin(coins, *wallet, 50000 * COIN); - BOOST_CHECK(KnapsackSolver(500000 * COIN, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(500000 * COIN, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -485,79 +486,79 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest coin: - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + coins.clear(); + add_coin(coins, *wallet, MIN_CHANGE * 5 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); + add_coin(coins, *wallet, 1111 * MIN_CHANGE); + BOOST_CHECK(KnapsackSolver(1 * MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - empty_wallet(); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 8 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + coins.clear(); + add_coin(coins, *wallet, MIN_CHANGE * 4 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); + add_coin(coins, *wallet, MIN_CHANGE * 8 / 10); + add_coin(coins, *wallet, 1111 * MIN_CHANGE); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 // test avoiding small change - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 100); - add_coin(MIN_CHANGE * 1); - add_coin(MIN_CHANGE * 100); + coins.clear(); + add_coin(coins, *wallet, MIN_CHANGE * 5 / 100); + add_coin(coins, *wallet, MIN_CHANGE * 1); + add_coin(coins, *wallet, MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 10001 / 100, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 10001 / 100, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 9990 / 100, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(MIN_CHANGE * 9990 / 100, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - } - - // test with many inputs - for (CAmount amt=1500; amt < COIN; amt*=10) { - empty_wallet(); - // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) - for (uint16_t j = 0; j < 676; j++) - add_coin(amt); - - // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. - for (int i = 0; i < RUN_TESTS; i++) { - BOOST_CHECK(KnapsackSolver(2000, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)); - - if (amt - 2000 < MIN_CHANGE) { - // needs more than one input: - uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); - CAmount returnValue = amt * returnSize; - BOOST_CHECK_EQUAL(nValueRet, returnValue); - BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); - } else { - // one input is sufficient: - BOOST_CHECK_EQUAL(nValueRet, amt); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - } - } - } - - // test randomness - { - empty_wallet(); - for (int i2 = 0; i2 < 100; i2++) - add_coin(COIN); - - // Again, we only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. - for (int i = 0; i < RUN_TESTS; i++) { + } + + // test with many inputs + for (CAmount amt=1500; amt < COIN; amt*=10) { + coins.clear(); + // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) + for (uint16_t j = 0; j < 676; j++) + add_coin(coins, *wallet, amt); + + // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. + for (int i = 0; i < RUN_TESTS; i++) { + BOOST_CHECK(KnapsackSolver(2000, KnapsackGroupOutputs(coins, *wallet, filter_confirmed), setCoinsRet, nValueRet)); + + if (amt - 2000 < MIN_CHANGE) { + // needs more than one input: + uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); + CAmount returnValue = amt * returnSize; + BOOST_CHECK_EQUAL(nValueRet, returnValue); + BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); + } else { + // one input is sufficient: + BOOST_CHECK_EQUAL(nValueRet, amt); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + } + } + } + + // test randomness + { + coins.clear(); + for (int i2 = 0; i2 < 100; i2++) + add_coin(coins, *wallet, COIN); + + // Again, we only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. + for (int i = 0; i < RUN_TESTS; i++) { // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(vCoins), setCoinsRet, nValueRet)); - BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(vCoins), setCoinsRet2, nValueRet)); + BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(coins), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(coins), setCoinsRet2, nValueRet)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -567,66 +568,67 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // When choosing 1 from 100 identical coins, 1% of the time, this test will choose the same coin twice // which will cause it to fail. // To avoid that issue, run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(vCoins), setCoinsRet, nValueRet)); - BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(vCoins), setCoinsRet2, nValueRet)); + BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(coins), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(coins), setCoinsRet2, nValueRet)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } BOOST_CHECK_NE(fails, RANDOM_REPEATS); - } - - // add 75 cents in small change. not enough to make 90 cents, - // then try making 90 cents. there are multiple competing "smallest bigger" coins, - // one of which should be picked at random - add_coin(5 * CENT); - add_coin(10 * CENT); - add_coin(15 * CENT); - add_coin(20 * CENT); - add_coin(25 * CENT); - - for (int i = 0; i < RUN_TESTS; i++) { + } + + // add 75 cents in small change. not enough to make 90 cents, + // then try making 90 cents. there are multiple competing "smallest bigger" coins, + // one of which should be picked at random + add_coin(coins, *wallet, 5 * CENT); + add_coin(coins, *wallet, 10 * CENT); + add_coin(coins, *wallet, 15 * CENT); + add_coin(coins, *wallet, 20 * CENT); + add_coin(coins, *wallet, 25 * CENT); + + for (int i = 0; i < RUN_TESTS; i++) { int fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) { - BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(vCoins), setCoinsRet, nValueRet)); - BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(vCoins), setCoinsRet2, nValueRet)); + BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(coins), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(coins), setCoinsRet2, nValueRet)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } BOOST_CHECK_NE(fails, RANDOM_REPEATS); - } - } - - empty_wallet(); + } + } } BOOST_AUTO_TEST_CASE(ApproximateBestSubset) { + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + CoinSet setCoinsRet; CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - testWallet.SetupLegacyScriptPubKeyMan(); - - empty_wallet(); + std::vector<COutput> coins; // Test vValue sort order for (int i = 0; i < 1000; i++) - add_coin(1000 * COIN); - add_coin(3 * COIN); + add_coin(coins, *wallet, 1000 * COIN); + add_coin(coins, *wallet, 3 * COIN); - BOOST_CHECK(KnapsackSolver(1003 * COIN, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)); + BOOST_CHECK(KnapsackSolver(1003 * COIN, KnapsackGroupOutputs(coins, *wallet, filter_standard), setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - empty_wallet(); } // Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value BOOST_AUTO_TEST_CASE(SelectCoins_test) { - LOCK(testWallet.cs_wallet); - testWallet.SetupLegacyScriptPubKeyMan(); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); // Random generator stuff std::default_random_engine generator; @@ -636,12 +638,15 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) // Run this test 100 times for (int i = 0; i < 100; ++i) { - empty_wallet(); + std::vector<COutput> coins; + CAmount balance{0}; // Make a wallet with 1000 exponentially distributed random inputs for (int j = 0; j < 1000; ++j) { - add_coin((CAmount)(distribution(generator)*10000000)); + CAmount val = distribution(generator)*10000000; + add_coin(coins, *wallet, val); + balance += val; } // Generate a random fee rate in the range of 100 - 400 @@ -658,7 +663,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CoinSet out_set; CAmount out_value = 0; CCoinControl cc; - BOOST_CHECK(SelectCoins(testWallet, vCoins, target, out_set, out_value, cc, cs_params)); + BOOST_CHECK(SelectCoins(*wallet, coins, target, out_set, out_value, cc, cs_params)); BOOST_CHECK_GE(out_value, target); } } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 8a97f7779d..120a20749e 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -13,10 +13,21 @@ BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup) +static void import_descriptor(CWallet& wallet, const std::string& descriptor) +{ + LOCK(wallet.cs_wallet); + FlatSigningProvider provider; + std::string error; + std::unique_ptr<Descriptor> desc = Parse(descriptor, provider, error, /* require_checksum=*/ false); + assert(desc); + WalletDescriptor w_desc(std::move(desc), 0, 0, 10, 0); + wallet.AddWalletDescriptor(w_desc, provider, "", false); +} + BOOST_AUTO_TEST_CASE(psbt_updater_test) { - auto spk_man = m_wallet.GetOrCreateLegacyScriptPubKeyMan(); - LOCK2(m_wallet.cs_wallet, spk_man->cs_KeyStore); + LOCK(m_wallet.cs_wallet); + m_wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); // Create prevtxs and add to wallet CDataStream s_prev_tx1(ParseHex("0200000000010158e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd7501000000171600145f275f436b09a8cc9a2eb2a2f528485c68a56323feffffff02d8231f1b0100000017a914aed962d6654f9a2b36608eb9d64d2b260db4f1118700c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88702483045022100a22edcc6e5bc511af4cc4ae0de0fcd75c7e04d8c1c3a8aa9d820ed4b967384ec02200642963597b9b1bc22c75e9f3e117284a962188bf5e8a74c895089046a20ad770121035509a48eb623e10aace8bfd0212fdb8a8e5af3c94b0b133b95e114cab89e4f7965000000"), SER_NETWORK, PROTOCOL_VERSION); @@ -29,27 +40,10 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) s_prev_tx2 >> prev_tx2; m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx2->GetHash()), std::forward_as_tuple(prev_tx2)); - // Add scripts - CScript rs1; - CDataStream s_rs1(ParseHex("475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae"), SER_NETWORK, PROTOCOL_VERSION); - s_rs1 >> rs1; - spk_man->AddCScript(rs1); - - CScript rs2; - CDataStream s_rs2(ParseHex("2200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903"), SER_NETWORK, PROTOCOL_VERSION); - s_rs2 >> rs2; - spk_man->AddCScript(rs2); - - CScript ws1; - CDataStream s_ws1(ParseHex("47522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae"), SER_NETWORK, PROTOCOL_VERSION); - s_ws1 >> ws1; - spk_man->AddCScript(ws1); - - // Add hd seed - CKey key = DecodeSecret("5KSSJQ7UNfFGwVgpCZDSHm5rVNhMFcFtvWM3zQ8mW4qNDEN7LFd"); // Mainnet and uncompressed form of cUkG8i1RFfWGWy5ziR11zJ5V4U4W3viSFCfyJmZnvQaUsd1xuF3T - CPubKey master_pub_key = spk_man->DeriveNewSeed(key); - spk_man->SetHDSeed(master_pub_key); - spk_man->NewKeyPool(); + // Import descriptors for keys and scripts + import_descriptor(m_wallet, "sh(multi(2,xprv9s21ZrQH143K2LE7W4Xf3jATf9jECxSb7wj91ZnmY4qEJrS66Qru9RFqq8xbkgT32ya6HqYJweFdJUEDf5Q6JFV7jMiUws7kQfe6Tv4RbfN/0h/0h/0h,xprv9s21ZrQH143K2LE7W4Xf3jATf9jECxSb7wj91ZnmY4qEJrS66Qru9RFqq8xbkgT32ya6HqYJweFdJUEDf5Q6JFV7jMiUws7kQfe6Tv4RbfN/0h/0h/1h))"); + import_descriptor(m_wallet, "sh(wsh(multi(2,xprv9s21ZrQH143K2LE7W4Xf3jATf9jECxSb7wj91ZnmY4qEJrS66Qru9RFqq8xbkgT32ya6HqYJweFdJUEDf5Q6JFV7jMiUws7kQfe6Tv4RbfN/0h/0h/2h,xprv9s21ZrQH143K2LE7W4Xf3jATf9jECxSb7wj91ZnmY4qEJrS66Qru9RFqq8xbkgT32ya6HqYJweFdJUEDf5Q6JFV7jMiUws7kQfe6Tv4RbfN/0h/0h/3h)))"); + import_descriptor(m_wallet, "wpkh(xprv9s21ZrQH143K2LE7W4Xf3jATf9jECxSb7wj91ZnmY4qEJrS66Qru9RFqq8xbkgT32ya6HqYJweFdJUEDf5Q6JFV7jMiUws7kQfe6Tv4RbfN/0h/0h/*h)"); // Call FillPSBT PartiallySignedTransaction psbtx; @@ -71,7 +65,8 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) // Try to sign the mutated input SignatureData sigdata; - BOOST_CHECK(spk_man->FillPSBT(psbtx, PrecomputePSBTData(psbtx), SIGHASH_ALL, true, true) != TransactionError::OK); + BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true, true) != TransactionError::OK); + //BOOST_CHECK(spk_man->FillPSBT(psbtx, PrecomputePSBTData(psbtx), SIGHASH_ALL, true, true) != TransactionError::OK); } BOOST_AUTO_TEST_CASE(parse_hd_keypath) diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index becef70729..d88d8eabdb 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -33,6 +33,8 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) CCoinControl coin_control; coin_control.m_feerate.emplace(10000); coin_control.fOverrideFeeRate = true; + // We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output + coin_control.m_change_type = OutputType::LEGACY; FeeCalculation fee_calc; BOOST_CHECK(CreateTransaction(*wallet, {recipient}, tx, fee, change_pos, error, coin_control, fee_calc)); BOOST_CHECK_EQUAL(tx->vout.size(), 1); diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index c3061b93c0..2990fc8f8d 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -6,6 +6,7 @@ #include <chain.h> #include <key.h> +#include <key_io.h> #include <test/util/setup_common.h> #include <wallet/wallet.h> #include <wallet/walletdb.h> @@ -23,9 +24,16 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc } wallet->LoadWallet(); { - auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); - LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); - spk_man->AddKeyPubKey(key, key.GetPubKey()); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + + FlatSigningProvider provider; + std::string error; + std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(key) + ")", provider, error, /* require_checksum=*/ false); + assert(desc); + WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1); + if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false); } WalletRescanReserver reserver(*wallet); reserver.reserve(); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 94b5abfba7..0965128ade 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -11,6 +11,7 @@ #include <vector> #include <interfaces/chain.h> +#include <key_io.h> #include <node/blockstorage.h> #include <node/context.h> #include <policy/policy.h> @@ -43,6 +44,7 @@ BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) static std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) { DatabaseOptions options; + options.create_flags = WALLET_FLAG_DESCRIPTORS; DatabaseStatus status; bilingual_str error; std::vector<bilingual_str> warnings; @@ -77,9 +79,13 @@ static CMutableTransaction TestSimpleSpend(const CTransaction& from, uint32_t in static void AddKey(CWallet& wallet, const CKey& key) { - auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); - LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore); - spk_man->AddKeyPubKey(key, key.GetPubKey()); + LOCK(wallet.cs_wallet); + FlatSigningProvider provider; + std::string error; + std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(key) + ")", provider, error, /* require_checksum=*/ false); + assert(desc); + WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1); + if (!wallet.AddWalletDescriptor(w_desc, provider, "", false)) assert(false); } BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) @@ -95,6 +101,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } AddKey(wallet, coinbaseKey); @@ -114,6 +121,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } AddKey(wallet, coinbaseKey); @@ -140,6 +148,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } AddKey(wallet, coinbaseKey); @@ -165,6 +174,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } AddKey(wallet, coinbaseKey); @@ -320,10 +330,12 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); - auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); CWalletTx wtx(m_coinbase_txns.back()); - LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore); + LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet.SetupDescriptorScriptPubKeyMans(); + wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash(), 0); @@ -336,7 +348,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) // Invalidate the cached value, add the key, and make sure a new immature // credit amount is calculated. wtx.MarkDirty(); - BOOST_CHECK(spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey())); + AddKey(wallet, coinbaseKey); BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx), 50*COIN); } @@ -593,14 +605,26 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) { - std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); - wallet->SetupLegacyScriptPubKeyMan(); - wallet->SetMinVersion(FEATURE_LATEST); - wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); - BOOST_CHECK(!wallet->TopUpKeyPool(1000)); - CTxDestination dest; - bilingual_str error; - BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); + { + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); + wallet->SetupLegacyScriptPubKeyMan(); + wallet->SetMinVersion(FEATURE_LATEST); + wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + BOOST_CHECK(!wallet->TopUpKeyPool(1000)); + CTxDestination dest; + bilingual_str error; + BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); + } + { + std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetMinVersion(FEATURE_LATEST); + wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + CTxDestination dest; + bilingual_str error; + BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); + } } // Explicit calculation which is used to test the wallet constant diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1b0d55bffa..824f32aeae 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3235,7 +3235,8 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern auto spk_man = m_spk_managers.at(id).get(); spk_mans[type] = spk_man; - if (spk_mans_other[type] == spk_man) { + const auto it = spk_mans_other.find(type); + if (it != spk_mans_other.end() && it->second == spk_man) { spk_mans_other.erase(type); } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a6839f1f78..c920d4af51 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1188,9 +1188,9 @@ std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase() /** Return object for accessing temporary in-memory database. */ std::unique_ptr<WalletDatabase> CreateMockWalletDatabase() { -#ifdef USE_BDB - return std::make_unique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), ""); -#elif USE_SQLITE +#ifdef USE_SQLITE return std::make_unique<SQLiteDatabase>("", "", true); +#elif USE_BDB + return std::make_unique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), ""); #endif } diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 788679bbeb..88f0a2ce20 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -120,6 +120,10 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n"); return false; } + if (args.IsArgSet("-legacy") && command != "create") { + tfm::format(std::cerr, "The -legacy option can only be used with the 'create' command.\n"); + return false; + } if (command == "create" && !args.IsArgSet("-wallet")) { tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n"); return false; @@ -130,7 +134,19 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) if (command == "create") { DatabaseOptions options; options.require_create = true; - if (args.GetBoolArg("-descriptors", false)) { + // If -legacy is set, use it. Otherwise default to false. + bool make_legacy = args.GetBoolArg("-legacy", false); + // If neither -legacy nor -descriptors is set, default to true. If -descriptors is set, use its value. + bool make_descriptors = (!args.IsArgSet("-descriptors") && !args.IsArgSet("-legacy")) || (args.IsArgSet("-descriptors") && args.GetBoolArg("-descriptors", true)); + if (make_legacy && make_descriptors) { + tfm::format(std::cerr, "Only one of -legacy or -descriptors can be set to true, not both\n"); + return false; + } + if (!make_legacy && !make_descriptors) { + tfm::format(std::cerr, "One of -legacy or -descriptors must be set to true (or omitted)\n"); + return false; + } + if (make_descriptors) { options.create_flags |= WALLET_FLAG_DESCRIPTORS; options.require_format = DatabaseFormat::SQLITE; } |