diff options
42 files changed, 374 insertions, 329 deletions
diff --git a/doc/JSON-RPC-interface.md b/doc/JSON-RPC-interface.md index 6cbb6ebd72..ec332d23eb 100644 --- a/doc/JSON-RPC-interface.md +++ b/doc/JSON-RPC-interface.md @@ -128,7 +128,7 @@ RPC interface will be abused. Instead, expose it only on the host system's localhost, for example: `-p 127.0.0.1:8332:8332` -- **Secure authentication:** By default, Bitcoin Core generates unique +- **Secure authentication:** By default, when no `rpcpassword` is specified, Bitcoin Core generates unique login credentials each time it restarts and puts them into a file readable only by the user that started Bitcoin Core, allowing any of that user's RPC clients with read access to the file to login diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 322fa987ae..791c049a4a 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -950,7 +950,9 @@ Threads and synchronization internal to a class (private or protected) rather than public. - Combine annotations in function declarations with run-time asserts in - function definitions: + function definitions (`AssertLockNotHeld()` can be omitted if `LOCK()` is + called unconditionally after it because `LOCK()` does the same check as + `AssertLockNotHeld()` internally, for non-recursive mutexes): ```C++ // txmempool.h diff --git a/src/Makefile.am b/src/Makefile.am index 8905c0ad1c..8508d13b34 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -328,6 +328,7 @@ BITCOIN_CORE_H = \ util/time.h \ util/tokenpipe.h \ util/trace.h \ + util/transaction_identifier.h \ util/translation.h \ util/types.h \ util/ui_change_type.h \ diff --git a/src/addresstype.h b/src/addresstype.h index d3422c6813..0152858bad 100644 --- a/src/addresstype.h +++ b/src/addresstype.h @@ -5,23 +5,26 @@ #ifndef BITCOIN_ADDRESSTYPE_H #define BITCOIN_ADDRESSTYPE_H +#include <attributes.h> #include <pubkey.h> #include <script/script.h> #include <uint256.h> #include <util/hash_type.h> -#include <variant> #include <algorithm> +#include <variant> +#include <vector> -class CNoDestination { +class CNoDestination +{ private: CScript m_script; public: CNoDestination() = default; - CNoDestination(const CScript& script) : m_script(script) {} + explicit CNoDestination(const CScript& script) : m_script(script) {} - const CScript& GetScript() const { return m_script; } + const CScript& GetScript() const LIFETIMEBOUND { return m_script; } friend bool operator==(const CNoDestination& a, const CNoDestination& b) { return a.GetScript() == b.GetScript(); } friend bool operator<(const CNoDestination& a, const CNoDestination& b) { return a.GetScript() < b.GetScript(); } @@ -32,7 +35,7 @@ private: CPubKey m_pubkey; public: - PubKeyDestination(const CPubKey& pubkey) : m_pubkey(pubkey) {} + explicit PubKeyDestination(const CPubKey& pubkey) : m_pubkey(pubkey) {} const CPubKey& GetPubKey() const LIFETIMEBOUND { return m_pubkey; } diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index 160534b63c..632918c0ca 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -94,13 +94,14 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type } // Generate destinations - CScript dest = GetScriptForDestination(getNewDestination(wallet, output_type)); + const auto dest{getNewDestination(wallet, output_type)}; // Generate chain; each coinbase will have two outputs to fill-up the wallet const auto& params = Params(); + const CScript coinbase_out{GetScriptForDestination(dest)}; unsigned int chain_size = 5000; // 5k blocks means 10k UTXO for the wallet (minus 200 due COINBASE_MATURITY) for (unsigned int i = 0; i < chain_size; ++i) { - generateFakeBlock(params, test_setup->m_node, wallet, dest); + generateFakeBlock(params, test_setup->m_node, wallet, coinbase_out); } // Check available balance @@ -185,4 +186,4 @@ static void WalletAvailableCoins(benchmark::Bench& bench) { AvailableCoins(bench BENCHMARK(WalletCreateTxUseOnlyPresetInputs, benchmark::PriorityLevel::LOW) BENCHMARK(WalletCreateTxUsePresetInputsAndCoinSelection, benchmark::PriorityLevel::LOW) -BENCHMARK(WalletAvailableCoins, benchmark::PriorityLevel::LOW);
\ No newline at end of file +BENCHMARK(WalletAvailableCoins, benchmark::PriorityLevel::LOW); diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 647e36adb3..4c14c5b939 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -224,8 +224,7 @@ static bool InitHTTPAllowList() rpc_allow_subnets.emplace_back(LookupHost("127.0.0.1", false).value(), 8); // always allow IPv4 local subnet rpc_allow_subnets.emplace_back(LookupHost("::1", false).value()); // always allow IPv6 localhost for (const std::string& strAllow : gArgs.GetArgs("-rpcallowip")) { - CSubNet subnet; - LookupSubNet(strAllow, subnet); + const CSubNet subnet{LookupSubNet(strAllow)}; if (!subnet.IsValid()) { uiInterface.ThreadSafeMessageBox( strprintf(Untranslated("Invalid -rpcallowip subnet specification: %s. Valid are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24)."), strAllow), diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index cf6b58e08d..a134a55264 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -111,8 +111,7 @@ bool NetWhitelistPermissions::TryParse(const std::string& str, NetWhitelistPermi if (!TryParsePermissionFlags(str, flags, offset, error)) return false; const std::string net = str.substr(offset); - CSubNet subnet; - LookupSubNet(net, subnet); + const CSubNet subnet{LookupSubNet(net)}; if (!subnet.IsValid()) { error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net); return false; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3bfb606037..7fcc399151 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1947,9 +1947,9 @@ void PeerManagerImpl::BlockConnected( { LOCK(m_recent_confirmed_transactions_mutex); for (const auto& ptx : pblock->vtx) { - m_recent_confirmed_transactions.insert(ptx->GetHash()); - if (ptx->GetHash() != ptx->GetWitnessHash()) { - m_recent_confirmed_transactions.insert(ptx->GetWitnessHash()); + m_recent_confirmed_transactions.insert(ptx->GetHash().ToUint256()); + if (ptx->HasWitness()) { + m_recent_confirmed_transactions.insert(ptx->GetWitnessHash().ToUint256()); } } } @@ -3003,8 +3003,8 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id)) { const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx); const TxValidationState& state = result.m_state; - const uint256& orphanHash = porphanTx->GetHash(); - const uint256& orphan_wtxid = porphanTx->GetWitnessHash(); + const Txid& orphanHash = porphanTx->GetHash(); + const Wtxid& orphan_wtxid = porphanTx->GetWitnessHash(); if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::TXPACKAGES, " accepted orphan tx %s (wtxid=%s)\n", orphanHash.ToString(), orphan_wtxid.ToString()); @@ -3052,7 +3052,7 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 // for concerns around weakening security of unupgraded nodes // if we start doing this too early. - m_recent_rejects.insert(porphanTx->GetWitnessHash()); + m_recent_rejects.insert(porphanTx->GetWitnessHash().ToUint256()); // If the transaction failed for TX_INPUTS_NOT_STANDARD, // then we know that the witness was irrelevant to the policy // failure, since this check depends only on the txid @@ -3061,10 +3061,10 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) // processing of this transaction in the event that child // transactions are later received (resulting in // parent-fetching by txid via the orphan-handling logic). - if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && porphanTx->GetWitnessHash() != porphanTx->GetHash()) { + if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && porphanTx->HasWitness()) { // We only add the txid if it differs from the wtxid, to // avoid wasting entries in the rolling bloom filter. - m_recent_rejects.insert(porphanTx->GetHash()); + m_recent_rejects.insert(porphanTx->GetHash().ToUint256()); } } m_orphanage.EraseTx(orphanHash); @@ -4319,8 +4319,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // regardless of what witness is provided, we will not accept // this, so we don't need to allow for redownload of this txid // from any of our non-wtxidrelay peers. - m_recent_rejects.insert(tx.GetHash()); - m_recent_rejects.insert(tx.GetWitnessHash()); + m_recent_rejects.insert(tx.GetHash().ToUint256()); + m_recent_rejects.insert(tx.GetWitnessHash().ToUint256()); m_txrequest.ForgetTxHash(tx.GetHash()); m_txrequest.ForgetTxHash(tx.GetWitnessHash()); } @@ -4339,7 +4339,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 // for concerns around weakening security of unupgraded nodes // if we start doing this too early. - m_recent_rejects.insert(tx.GetWitnessHash()); + m_recent_rejects.insert(tx.GetWitnessHash().ToUint256()); m_txrequest.ForgetTxHash(tx.GetWitnessHash()); // If the transaction failed for TX_INPUTS_NOT_STANDARD, // then we know that the witness was irrelevant to the policy @@ -4349,8 +4349,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // processing of this transaction in the event that child // transactions are later received (resulting in // parent-fetching by txid via the orphan-handling logic). - if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && tx.GetWitnessHash() != tx.GetHash()) { - m_recent_rejects.insert(tx.GetHash()); + if (state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD && tx.HasWitness()) { + m_recent_rejects.insert(tx.GetHash().ToUint256()); m_txrequest.ForgetTxHash(tx.GetHash()); } if (RecursiveDynamicUsage(*ptx) < 100000) { @@ -5780,9 +5780,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LOCK(tx_relay->m_bloom_filter_mutex); for (const auto& txinfo : vtxinfo) { - const uint256& hash = peer->m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash(); - CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash); - tx_relay->m_tx_inventory_to_send.erase(hash); + CInv inv{ + peer->m_wtxid_relay ? MSG_WTX : MSG_TX, + peer->m_wtxid_relay ? + txinfo.tx->GetWitnessHash().ToUint256() : + txinfo.tx->GetHash().ToUint256(), + }; + tx_relay->m_tx_inventory_to_send.erase(inv.hash); + // Don't send transactions that peers will not put into their mempool if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; @@ -5790,7 +5795,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (tx_relay->m_bloom_filter) { if (!tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; } - tx_relay->m_tx_inventory_known_filter.insert(hash); + tx_relay->m_tx_inventory_known_filter.insert(inv.hash); vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); diff --git a/src/net_types.cpp b/src/net_types.cpp index 2cdc10d8c9..fd6ad80404 100644 --- a/src/net_types.cpp +++ b/src/net_types.cpp @@ -63,9 +63,9 @@ void BanMapFromJson(const UniValue& bans_json, banmap_t& bans) LogPrintf("Dropping entry with unknown version (%s) from ban list\n", version); continue; } - CSubNet subnet; const auto& subnet_str = ban_entry_json[BANMAN_JSON_ADDR_KEY].get_str(); - if (!LookupSubNet(subnet_str, subnet)) { + const CSubNet subnet{LookupSubNet(subnet_str)}; + if (!subnet.IsValid()) { LogPrintf("Dropping entry with unparseable address or subnet (%s) from ban list\n", subnet_str); continue; } diff --git a/src/netbase.cpp b/src/netbase.cpp index 5e1e121bfe..5a609a872e 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -645,10 +645,12 @@ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_ return true; } -bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) +CSubNet LookupSubNet(const std::string& subnet_str) { + CSubNet subnet; + assert(!subnet.IsValid()); if (!ContainsNoNUL(subnet_str)) { - return false; + return subnet; } const size_t slash_pos{subnet_str.find_last_of('/')}; @@ -662,23 +664,21 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) uint8_t netmask; if (ParseUInt8(netmask_str, &netmask)) { // Valid number; assume CIDR variable-length subnet masking. - subnet_out = CSubNet{addr.value(), netmask}; - return subnet_out.IsValid(); + subnet = CSubNet{addr.value(), netmask}; } else { // Invalid number; try full netmask syntax. Never allow lookup for netmask. const std::optional<CNetAddr> full_netmask{LookupHost(netmask_str, /*fAllowLookup=*/false)}; if (full_netmask.has_value()) { - subnet_out = CSubNet{addr.value(), full_netmask.value()}; - return subnet_out.IsValid(); + subnet = CSubNet{addr.value(), full_netmask.value()}; } } } else { // Single IP subnet (<ipv4>/32 or <ipv6>/128). - subnet_out = CSubNet{addr.value()}; - return subnet_out.IsValid(); + subnet = CSubNet{addr.value()}; } } - return false; + + return subnet; } void InterruptSocks5(bool interrupt) diff --git a/src/netbase.h b/src/netbase.h index 8b7da4109f..d51f63fd81 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -227,11 +227,9 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLoo * @param[in] subnet_str A string representation of a subnet of the form * `network address [ "/", ( CIDR-style suffix | netmask ) ]` * e.g. "2001:db8::/32", "192.0.2.0/255.255.255.0" or "8.8.8.8". - * @param[out] subnet_out Internal subnet representation, if parsable/resolvable - * from `subnet_str`. - * @returns whether the operation succeeded or not. + * @returns a CSubNet object (that may or may not be valid). */ -bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out); +CSubNet LookupSubNet(const std::string& subnet_str); /** * Create a TCP socket in the given address family. diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 2c913bf432..1ad8345fcb 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -12,6 +12,7 @@ #include <tinyformat.h> #include <uint256.h> #include <util/strencodings.h> +#include <util/transaction_identifier.h> #include <version.h> #include <cassert> @@ -65,22 +66,23 @@ std::string CTxOut::ToString() const CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {} CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime) {} -uint256 CMutableTransaction::GetHash() const +Txid CMutableTransaction::GetHash() const { - return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash(); + return Txid::FromUint256((CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash()); } -uint256 CTransaction::ComputeHash() const +Txid CTransaction::ComputeHash() const { - return (CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash(); + return Txid::FromUint256((CHashWriter{SERIALIZE_TRANSACTION_NO_WITNESS} << *this).GetHash()); } -uint256 CTransaction::ComputeWitnessHash() const +Wtxid CTransaction::ComputeWitnessHash() const { if (!HasWitness()) { - return hash; + return Wtxid::FromUint256(hash.ToUint256()); } - return (CHashWriter{0} << *this).GetHash(); + + return Wtxid::FromUint256((CHashWriter{0} << *this).GetHash()); } CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {} diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index bd7eb16bec..89deb9de4d 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -6,11 +6,12 @@ #ifndef BITCOIN_PRIMITIVES_TRANSACTION_H #define BITCOIN_PRIMITIVES_TRANSACTION_H +#include <attributes.h> #include <consensus/amount.h> -#include <prevector.h> #include <script/script.h> #include <serialize.h> #include <uint256.h> +#include <util/transaction_identifier.h> // IWYU pragma: export #include <cstddef> #include <cstdint> @@ -309,11 +310,11 @@ public: private: /** Memory only. */ - const uint256 hash; - const uint256 m_witness_hash; + const Txid hash; + const Wtxid m_witness_hash; - uint256 ComputeHash() const; - uint256 ComputeWitnessHash() const; + Txid ComputeHash() const; + Wtxid ComputeWitnessHash() const; public: /** Convert a CMutableTransaction into a CTransaction. */ @@ -334,8 +335,8 @@ public: return vin.empty() && vout.empty(); } - const uint256& GetHash() const { return hash; } - const uint256& GetWitnessHash() const { return m_witness_hash; }; + const Txid& GetHash() const LIFETIMEBOUND { return hash; } + const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return m_witness_hash; }; // Return sum of txouts. CAmount GetValueOut() const; @@ -405,7 +406,7 @@ struct CMutableTransaction /** Compute the hash of this CMutableTransaction. This is computed on the * fly, as opposed to GetHash() in CTransaction, which uses a cached result. */ - uint256 GetHash() const; + Txid GetHash() const; bool HasWitness() const { @@ -432,7 +433,7 @@ 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; } + const uint256& GetHash() const LIFETIMEBOUND { 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; } friend bool operator<(const GenTxid& a, const GenTxid& b) { return std::tie(a.m_is_wtxid, a.m_hash) < std::tie(b.m_is_wtxid, b.m_hash); } }; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index ee3327530c..a45579fa0d 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -187,8 +187,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact setAddress.insert(rcp.address); ++nAddresses; - CScript scriptPubKey = GetScriptForDestination(DecodeDestination(rcp.address.toStdString())); - CRecipient recipient = {scriptPubKey, rcp.amount, rcp.fSubtractFeeFromAmount}; + CRecipient recipient{DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount}; vecSend.push_back(recipient); total += rcp.amount; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 63788c3a03..1a9052e008 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -734,7 +734,7 @@ static RPCHelpMan setban() } } else - LookupSubNet(request.params[0].get_str(), subNet); + subNet = LookupSubNet(request.params[0].get_str()); if (! (isSubnet ? subNet.IsValid() : netAddr.IsValid()) ) throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Invalid IP/Subnet"); diff --git a/src/script/script.cpp b/src/script/script.cpp index 1594d3cc79..80e8d26bcf 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -1,11 +1,14 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2021 The Bitcoin Core developers +// Copyright (c) 2009-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <script/script.h> +#include <crypto/common.h> #include <hash.h> +#include <uint256.h> +#include <util/hash_type.h> #include <util/strencodings.h> #include <string> diff --git a/src/script/script.h b/src/script/script.h index c329a2afd6..66d63fae89 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2022 The Bitcoin Core developers +// Copyright (c) 2009-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -8,18 +8,19 @@ #include <attributes.h> #include <crypto/common.h> -#include <prevector.h> +#include <prevector.h> // IWYU pragma: export #include <serialize.h> #include <uint256.h> #include <util/hash_type.h> -#include <assert.h> -#include <climits> +#include <cassert> +#include <cstdint> +#include <cstring> #include <limits> #include <stdexcept> -#include <stdint.h> -#include <string.h> #include <string> +#include <type_traits> +#include <utility> #include <vector> // Maximum number of bytes pushable to the stack diff --git a/src/sync.cpp b/src/sync.cpp index 4621805653..58752a9f18 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -246,7 +246,7 @@ void LeaveCritical() pop_lock(); } -std::string LocksHeld() +static std::string LocksHeld() { LockData& lockdata = GetLockData(); std::lock_guard<std::mutex> lock(lockdata.dd_mutex); diff --git a/src/sync.h b/src/sync.h index 45d40b5fdc..dc63e3f2d0 100644 --- a/src/sync.h +++ b/src/sync.h @@ -57,7 +57,6 @@ template <typename MutexType> void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false); void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); -std::string LocksHeld(); template <typename MutexType> void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs); template <typename MutexType> diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp index dcf500acc3..ba31315297 100644 --- a/src/test/fuzz/netbase_dns_lookup.cpp +++ b/src/test/fuzz/netbase_dns_lookup.cpp @@ -59,9 +59,6 @@ FUZZ_TARGET(netbase_dns_lookup) assert(!resolved_service.IsInternal()); } { - CSubNet resolved_subnet; - if (LookupSubNet(name, resolved_subnet)) { - assert(resolved_subnet.IsValid()); - } + (void)LookupSubNet(name); } } diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 4c81c0b679..7220c5d997 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -238,7 +238,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) } if (fuzzed_data_provider.ConsumeBool()) { const auto& txid = fuzzed_data_provider.ConsumeBool() ? - txs.back()->GetHash() : + txs.back()->GetHash().ToUint256() : PickValue(fuzzed_data_provider, mempool_outpoints).hash; const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN); tx_pool.PrioritiseTransaction(txid, delta); diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index ee73f67f66..5ec3e89d1e 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -227,7 +227,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool) } if (fuzzed_data_provider.ConsumeBool()) { const auto& txid = fuzzed_data_provider.ConsumeBool() ? - tx->GetHash() : + tx->GetHash().ToUint256() : PickValue(fuzzed_data_provider, outpoints_rbf).hash; const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN); tx_pool.PrioritiseTransaction(txid, delta); @@ -343,8 +343,8 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool) tx_pool.RollingFeeUpdate(); } if (fuzzed_data_provider.ConsumeBool()) { - const auto& txid = fuzzed_data_provider.ConsumeBool() ? - mut_tx.GetHash() : + const auto txid = fuzzed_data_provider.ConsumeBool() ? + mut_tx.GetHash().ToUint256() : PickValue(fuzzed_data_provider, txids); const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN); tx_pool.PrioritiseTransaction(txid, delta); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 74ff531cd9..233c741d19 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -27,13 +27,6 @@ static CNetAddr ResolveIP(const std::string& ip) return LookupHost(ip, false).value_or(CNetAddr{}); } -static CSubNet ResolveSubNet(const std::string& subnet) -{ - CSubNet ret; - LookupSubNet(subnet, ret); - return ret; -} - static CNetAddr CreateInternal(const std::string& host) { CNetAddr addr; @@ -159,49 +152,49 @@ BOOST_AUTO_TEST_CASE(embedded_test) BOOST_AUTO_TEST_CASE(subnet_test) { - BOOST_CHECK(ResolveSubNet("1.2.3.0/24") == ResolveSubNet("1.2.3.0/255.255.255.0")); - BOOST_CHECK(ResolveSubNet("1.2.3.0/24") != ResolveSubNet("1.2.4.0/255.255.255.0")); - BOOST_CHECK(ResolveSubNet("1.2.3.0/24").Match(ResolveIP("1.2.3.4"))); - BOOST_CHECK(!ResolveSubNet("1.2.2.0/24").Match(ResolveIP("1.2.3.4"))); - BOOST_CHECK(ResolveSubNet("1.2.3.4").Match(ResolveIP("1.2.3.4"))); - BOOST_CHECK(ResolveSubNet("1.2.3.4/32").Match(ResolveIP("1.2.3.4"))); - BOOST_CHECK(!ResolveSubNet("1.2.3.4").Match(ResolveIP("5.6.7.8"))); - BOOST_CHECK(!ResolveSubNet("1.2.3.4/32").Match(ResolveIP("5.6.7.8"))); - BOOST_CHECK(ResolveSubNet("::ffff:127.0.0.1").Match(ResolveIP("127.0.0.1"))); - BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:8").Match(ResolveIP("1:2:3:4:5:6:7:8"))); - BOOST_CHECK(!ResolveSubNet("1:2:3:4:5:6:7:8").Match(ResolveIP("1:2:3:4:5:6:7:9"))); - BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:0/112").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); - BOOST_CHECK(ResolveSubNet("192.168.0.1/24").Match(ResolveIP("192.168.0.2"))); - BOOST_CHECK(ResolveSubNet("192.168.0.20/29").Match(ResolveIP("192.168.0.18"))); - BOOST_CHECK(ResolveSubNet("1.2.2.1/24").Match(ResolveIP("1.2.2.4"))); - BOOST_CHECK(ResolveSubNet("1.2.2.110/31").Match(ResolveIP("1.2.2.111"))); - BOOST_CHECK(ResolveSubNet("1.2.2.20/26").Match(ResolveIP("1.2.2.63"))); + BOOST_CHECK(LookupSubNet("1.2.3.0/24") == LookupSubNet("1.2.3.0/255.255.255.0")); + BOOST_CHECK(LookupSubNet("1.2.3.0/24") != LookupSubNet("1.2.4.0/255.255.255.0")); + BOOST_CHECK(LookupSubNet("1.2.3.0/24").Match(ResolveIP("1.2.3.4"))); + BOOST_CHECK(!LookupSubNet("1.2.2.0/24").Match(ResolveIP("1.2.3.4"))); + BOOST_CHECK(LookupSubNet("1.2.3.4").Match(ResolveIP("1.2.3.4"))); + BOOST_CHECK(LookupSubNet("1.2.3.4/32").Match(ResolveIP("1.2.3.4"))); + BOOST_CHECK(!LookupSubNet("1.2.3.4").Match(ResolveIP("5.6.7.8"))); + BOOST_CHECK(!LookupSubNet("1.2.3.4/32").Match(ResolveIP("5.6.7.8"))); + BOOST_CHECK(LookupSubNet("::ffff:127.0.0.1").Match(ResolveIP("127.0.0.1"))); + BOOST_CHECK(LookupSubNet("1:2:3:4:5:6:7:8").Match(ResolveIP("1:2:3:4:5:6:7:8"))); + BOOST_CHECK(!LookupSubNet("1:2:3:4:5:6:7:8").Match(ResolveIP("1:2:3:4:5:6:7:9"))); + BOOST_CHECK(LookupSubNet("1:2:3:4:5:6:7:0/112").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); + BOOST_CHECK(LookupSubNet("192.168.0.1/24").Match(ResolveIP("192.168.0.2"))); + BOOST_CHECK(LookupSubNet("192.168.0.20/29").Match(ResolveIP("192.168.0.18"))); + BOOST_CHECK(LookupSubNet("1.2.2.1/24").Match(ResolveIP("1.2.2.4"))); + BOOST_CHECK(LookupSubNet("1.2.2.110/31").Match(ResolveIP("1.2.2.111"))); + BOOST_CHECK(LookupSubNet("1.2.2.20/26").Match(ResolveIP("1.2.2.63"))); // All-Matching IPv6 Matches arbitrary IPv6 - BOOST_CHECK(ResolveSubNet("::/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); + BOOST_CHECK(LookupSubNet("::/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); // But not `::` or `0.0.0.0` because they are considered invalid addresses - BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("::"))); - BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("0.0.0.0"))); + BOOST_CHECK(!LookupSubNet("::/0").Match(ResolveIP("::"))); + BOOST_CHECK(!LookupSubNet("::/0").Match(ResolveIP("0.0.0.0"))); // Addresses from one network (IPv4) don't belong to subnets of another network (IPv6) - BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("1.2.3.4"))); + BOOST_CHECK(!LookupSubNet("::/0").Match(ResolveIP("1.2.3.4"))); // All-Matching IPv4 does not Match IPv6 - BOOST_CHECK(!ResolveSubNet("0.0.0.0/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); + BOOST_CHECK(!LookupSubNet("0.0.0.0/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); // Invalid subnets Match nothing (not even invalid addresses) BOOST_CHECK(!CSubNet().Match(ResolveIP("1.2.3.4"))); - BOOST_CHECK(!ResolveSubNet("").Match(ResolveIP("4.5.6.7"))); - BOOST_CHECK(!ResolveSubNet("bloop").Match(ResolveIP("0.0.0.0"))); - BOOST_CHECK(!ResolveSubNet("bloop").Match(ResolveIP("hab"))); + BOOST_CHECK(!LookupSubNet("").Match(ResolveIP("4.5.6.7"))); + BOOST_CHECK(!LookupSubNet("bloop").Match(ResolveIP("0.0.0.0"))); + BOOST_CHECK(!LookupSubNet("bloop").Match(ResolveIP("hab"))); // Check valid/invalid - BOOST_CHECK(ResolveSubNet("1.2.3.0/0").IsValid()); - BOOST_CHECK(!ResolveSubNet("1.2.3.0/-1").IsValid()); - BOOST_CHECK(ResolveSubNet("1.2.3.0/32").IsValid()); - BOOST_CHECK(!ResolveSubNet("1.2.3.0/33").IsValid()); - BOOST_CHECK(!ResolveSubNet("1.2.3.0/300").IsValid()); - BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:8/0").IsValid()); - BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:8/33").IsValid()); - BOOST_CHECK(!ResolveSubNet("1:2:3:4:5:6:7:8/-1").IsValid()); - BOOST_CHECK(ResolveSubNet("1:2:3:4:5:6:7:8/128").IsValid()); - BOOST_CHECK(!ResolveSubNet("1:2:3:4:5:6:7:8/129").IsValid()); - BOOST_CHECK(!ResolveSubNet("fuzzy").IsValid()); + BOOST_CHECK(LookupSubNet("1.2.3.0/0").IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/-1").IsValid()); + BOOST_CHECK(LookupSubNet("1.2.3.0/32").IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/33").IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/300").IsValid()); + BOOST_CHECK(LookupSubNet("1:2:3:4:5:6:7:8/0").IsValid()); + BOOST_CHECK(LookupSubNet("1:2:3:4:5:6:7:8/33").IsValid()); + BOOST_CHECK(!LookupSubNet("1:2:3:4:5:6:7:8/-1").IsValid()); + BOOST_CHECK(LookupSubNet("1:2:3:4:5:6:7:8/128").IsValid()); + BOOST_CHECK(!LookupSubNet("1:2:3:4:5:6:7:8/129").IsValid()); + BOOST_CHECK(!LookupSubNet("fuzzy").IsValid()); //CNetAddr constructor test BOOST_CHECK(CSubNet(ResolveIP("127.0.0.1")).IsValid()); @@ -247,85 +240,85 @@ BOOST_AUTO_TEST_CASE(subnet_test) BOOST_CHECK(!CSubNet(tor_addr, 200).IsValid()); BOOST_CHECK(!CSubNet(tor_addr, ResolveIP("255.0.0.0")).IsValid()); - subnet = ResolveSubNet("1.2.3.4/255.255.255.255"); + subnet = LookupSubNet("1.2.3.4/255.255.255.255"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.4/32"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.254"); + subnet = LookupSubNet("1.2.3.4/255.255.255.254"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.4/31"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.252"); + subnet = LookupSubNet("1.2.3.4/255.255.255.252"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.4/30"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.248"); + subnet = LookupSubNet("1.2.3.4/255.255.255.248"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.0/29"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.240"); + subnet = LookupSubNet("1.2.3.4/255.255.255.240"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.0/28"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.224"); + subnet = LookupSubNet("1.2.3.4/255.255.255.224"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.0/27"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.192"); + subnet = LookupSubNet("1.2.3.4/255.255.255.192"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.0/26"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.128"); + subnet = LookupSubNet("1.2.3.4/255.255.255.128"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.0/25"); - subnet = ResolveSubNet("1.2.3.4/255.255.255.0"); + subnet = LookupSubNet("1.2.3.4/255.255.255.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.3.0/24"); - subnet = ResolveSubNet("1.2.3.4/255.255.254.0"); + subnet = LookupSubNet("1.2.3.4/255.255.254.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.2.0/23"); - subnet = ResolveSubNet("1.2.3.4/255.255.252.0"); + subnet = LookupSubNet("1.2.3.4/255.255.252.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/22"); - subnet = ResolveSubNet("1.2.3.4/255.255.248.0"); + subnet = LookupSubNet("1.2.3.4/255.255.248.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/21"); - subnet = ResolveSubNet("1.2.3.4/255.255.240.0"); + subnet = LookupSubNet("1.2.3.4/255.255.240.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/20"); - subnet = ResolveSubNet("1.2.3.4/255.255.224.0"); + subnet = LookupSubNet("1.2.3.4/255.255.224.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/19"); - subnet = ResolveSubNet("1.2.3.4/255.255.192.0"); + subnet = LookupSubNet("1.2.3.4/255.255.192.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/18"); - subnet = ResolveSubNet("1.2.3.4/255.255.128.0"); + subnet = LookupSubNet("1.2.3.4/255.255.128.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/17"); - subnet = ResolveSubNet("1.2.3.4/255.255.0.0"); + subnet = LookupSubNet("1.2.3.4/255.255.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/16"); - subnet = ResolveSubNet("1.2.3.4/255.254.0.0"); + subnet = LookupSubNet("1.2.3.4/255.254.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.2.0.0/15"); - subnet = ResolveSubNet("1.2.3.4/255.252.0.0"); + subnet = LookupSubNet("1.2.3.4/255.252.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/14"); - subnet = ResolveSubNet("1.2.3.4/255.248.0.0"); + subnet = LookupSubNet("1.2.3.4/255.248.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/13"); - subnet = ResolveSubNet("1.2.3.4/255.240.0.0"); + subnet = LookupSubNet("1.2.3.4/255.240.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/12"); - subnet = ResolveSubNet("1.2.3.4/255.224.0.0"); + subnet = LookupSubNet("1.2.3.4/255.224.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/11"); - subnet = ResolveSubNet("1.2.3.4/255.192.0.0"); + subnet = LookupSubNet("1.2.3.4/255.192.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/10"); - subnet = ResolveSubNet("1.2.3.4/255.128.0.0"); + subnet = LookupSubNet("1.2.3.4/255.128.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/9"); - subnet = ResolveSubNet("1.2.3.4/255.0.0.0"); + subnet = LookupSubNet("1.2.3.4/255.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "1.0.0.0/8"); - subnet = ResolveSubNet("1.2.3.4/254.0.0.0"); + subnet = LookupSubNet("1.2.3.4/254.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/7"); - subnet = ResolveSubNet("1.2.3.4/252.0.0.0"); + subnet = LookupSubNet("1.2.3.4/252.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/6"); - subnet = ResolveSubNet("1.2.3.4/248.0.0.0"); + subnet = LookupSubNet("1.2.3.4/248.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/5"); - subnet = ResolveSubNet("1.2.3.4/240.0.0.0"); + subnet = LookupSubNet("1.2.3.4/240.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/4"); - subnet = ResolveSubNet("1.2.3.4/224.0.0.0"); + subnet = LookupSubNet("1.2.3.4/224.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/3"); - subnet = ResolveSubNet("1.2.3.4/192.0.0.0"); + subnet = LookupSubNet("1.2.3.4/192.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/2"); - subnet = ResolveSubNet("1.2.3.4/128.0.0.0"); + subnet = LookupSubNet("1.2.3.4/128.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/1"); - subnet = ResolveSubNet("1.2.3.4/0.0.0.0"); + subnet = LookupSubNet("1.2.3.4/0.0.0.0"); BOOST_CHECK_EQUAL(subnet.ToString(), "0.0.0.0/0"); - subnet = ResolveSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); + subnet = LookupSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); BOOST_CHECK_EQUAL(subnet.ToString(), "1:2:3:4:5:6:7:8/128"); - subnet = ResolveSubNet("1:2:3:4:5:6:7:8/ffff:0000:0000:0000:0000:0000:0000:0000"); + subnet = LookupSubNet("1:2:3:4:5:6:7:8/ffff:0000:0000:0000:0000:0000:0000:0000"); BOOST_CHECK_EQUAL(subnet.ToString(), "1::/16"); - subnet = ResolveSubNet("1:2:3:4:5:6:7:8/0000:0000:0000:0000:0000:0000:0000:0000"); + subnet = LookupSubNet("1:2:3:4:5:6:7:8/0000:0000:0000:0000:0000:0000:0000:0000"); BOOST_CHECK_EQUAL(subnet.ToString(), "::/0"); // Invalid netmasks (with 1-bits after 0-bits) - subnet = ResolveSubNet("1.2.3.4/255.255.232.0"); + subnet = LookupSubNet("1.2.3.4/255.255.232.0"); BOOST_CHECK(!subnet.IsValid()); - subnet = ResolveSubNet("1.2.3.4/255.0.255.255"); + subnet = LookupSubNet("1.2.3.4/255.0.255.255"); BOOST_CHECK(!subnet.IsValid()); - subnet = ResolveSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f"); + subnet = LookupSubNet("1:2:3:4:5:6:7:8/ffff:ffff:ffff:fffe:ffff:ffff:ffff:ff0f"); BOOST_CHECK(!subnet.IsValid()); } @@ -479,15 +472,15 @@ BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) BOOST_CHECK(!LookupHost("127.0.0.1\0"s, false).has_value()); BOOST_CHECK(!LookupHost("127.0.0.1\0example.com"s, false).has_value()); BOOST_CHECK(!LookupHost("127.0.0.1\0example.com\0"s, false).has_value()); - CSubNet ret; - BOOST_CHECK(LookupSubNet("1.2.3.0/24"s, ret)); - BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0"s, ret)); - BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com"s, ret)); - BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com\0"s, ret)); - BOOST_CHECK(LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"s, ret)); - BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0"s, ret)); - BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0example.com"s, ret)); - BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0example.com\0"s, ret)); + + BOOST_CHECK(LookupSubNet("1.2.3.0/24"s).IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0"s).IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com"s).IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com\0"s).IsValid()); + BOOST_CHECK(LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"s).IsValid()); + BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0"s).IsValid()); + BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0example.com"s).IsValid()); + BOOST_CHECK(!LookupSubNet("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion\0example.com\0"s).IsValid()); } // Since CNetAddr (un)ser is tested separately in net_tests.cpp here we only diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp index af53737fec..d374497a45 100644 --- a/src/test/orphanage_tests.cpp +++ b/src/test/orphanage_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <arith_uint256.h> +#include <primitives/transaction.h> #include <pubkey.h> #include <script/sign.h> #include <script/signingprovider.h> @@ -29,8 +30,8 @@ public: CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { LOCK(m_mutex); - std::map<uint256, OrphanTx>::iterator it; - it = m_orphans.lower_bound(InsecureRand256()); + std::map<Txid, OrphanTx>::iterator it; + it = m_orphans.lower_bound(Txid::FromUint256(InsecureRand256())); if (it == m_orphans.end()) it = m_orphans.begin(); return it->second.tx; diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 7455d914e8..16f54644c2 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -7,6 +7,7 @@ #include <consensus/validation.h> #include <logging.h> #include <policy/policy.h> +#include <primitives/transaction.h> #include <cassert> @@ -20,8 +21,8 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) { LOCK(m_mutex); - const uint256& hash = tx->GetHash(); - const uint256& wtxid = tx->GetWitnessHash(); + const Txid& hash = tx->GetHash(); + const Wtxid& wtxid = tx->GetWitnessHash(); if (m_orphans.count(hash)) return false; @@ -53,16 +54,16 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) return true; } -int TxOrphanage::EraseTx(const uint256& txid) +int TxOrphanage::EraseTx(const Txid& txid) { LOCK(m_mutex); return EraseTxNoLock(txid); } -int TxOrphanage::EraseTxNoLock(const uint256& txid) +int TxOrphanage::EraseTxNoLock(const Txid& txid) { AssertLockHeld(m_mutex); - std::map<uint256, OrphanTx>::iterator it = m_orphans.find(txid); + std::map<Txid, OrphanTx>::iterator it = m_orphans.find(txid); if (it == m_orphans.end()) return 0; for (const CTxIn& txin : it->second.tx->vin) @@ -100,10 +101,10 @@ void TxOrphanage::EraseForPeer(NodeId peer) m_peer_work_set.erase(peer); int nErased = 0; - std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin(); + std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin(); while (iter != m_orphans.end()) { - std::map<uint256, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid + std::map<Txid, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { nErased += EraseTxNoLock(maybeErase->second.tx->GetHash()); @@ -123,10 +124,10 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans) // Sweep out expired orphan pool entries: int nErased = 0; int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL; - std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin(); + std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin(); while (iter != m_orphans.end()) { - std::map<uint256, OrphanTx>::iterator maybeErase = iter++; + std::map<Txid, OrphanTx>::iterator maybeErase = iter++; if (maybeErase->second.nTimeExpire <= nNow) { nErased += EraseTxNoLock(maybeErase->second.tx->GetHash()); } else { @@ -159,7 +160,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx) for (const auto& elem : it_by_prev->second) { // Get this source peer's work set, emplacing an empty set if it didn't exist // (note: if this peer wasn't still connected, we would have removed the orphan tx already) - std::set<uint256>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second; + std::set<Txid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second; // Add this tx to the work set orphan_work_set.insert(elem->first); LogPrint(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n", @@ -173,9 +174,9 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const { LOCK(m_mutex); if (gtxid.IsWtxid()) { - return m_wtxid_to_orphan_it.count(gtxid.GetHash()); + return m_wtxid_to_orphan_it.count(Wtxid::FromUint256(gtxid.GetHash())); } else { - return m_orphans.count(gtxid.GetHash()); + return m_orphans.count(Txid::FromUint256(gtxid.GetHash())); } } @@ -187,7 +188,7 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer) if (work_set_it != m_peer_work_set.end()) { auto& work_set = work_set_it->second; while (!work_set.empty()) { - uint256 txid = *work_set.begin(); + Txid txid = *work_set.begin(); work_set.erase(work_set.begin()); const auto orphan_it = m_orphans.find(txid); @@ -215,7 +216,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) { LOCK(m_mutex); - std::vector<uint256> vOrphanErase; + std::vector<Txid> vOrphanErase; for (const CTransactionRef& ptx : block.vtx) { const CTransaction& tx = *ptx; @@ -226,7 +227,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) if (itByPrev == m_outpoint_to_orphan_it.end()) continue; for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { const CTransaction& orphanTx = *(*mi)->second.tx; - const uint256& orphanHash = orphanTx.GetHash(); + const auto& orphanHash = orphanTx.GetHash(); vOrphanErase.push_back(orphanHash); } } @@ -235,7 +236,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block) // Erase orphan transactions included or precluded by this block if (vOrphanErase.size()) { int nErased = 0; - for (const uint256& orphanHash : vOrphanErase) { + for (const auto& orphanHash : vOrphanErase) { nErased += EraseTxNoLock(orphanHash); } LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx included or conflicted by block\n", nErased); diff --git a/src/txorphanage.h b/src/txorphanage.h index a4705bf382..2196ed4c85 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -34,7 +34,7 @@ public: CTransactionRef GetTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase an orphan by txid */ - int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + int EraseTx(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); /** Erase all orphans announced by a peer (eg, after that peer disconnects) */ void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); @@ -71,10 +71,10 @@ protected: /** Map from txid to orphan transaction record. Limited by * -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */ - std::map<uint256, OrphanTx> m_orphans GUARDED_BY(m_mutex); + std::map<Txid, OrphanTx> m_orphans GUARDED_BY(m_mutex); /** Which peer provided the orphans that need to be reconsidered */ - std::map<NodeId, std::set<uint256>> m_peer_work_set GUARDED_BY(m_mutex); + std::map<NodeId, std::set<Txid>> m_peer_work_set GUARDED_BY(m_mutex); using OrphanMap = decltype(m_orphans); @@ -96,10 +96,10 @@ protected: /** Index from wtxid into the m_orphans to lookup orphan * transactions using their witness ids. */ - std::map<uint256, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(m_mutex); + std::map<Wtxid, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(m_mutex); /** Erase an orphan by txid */ - int EraseTxNoLock(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); + int EraseTxNoLock(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex); }; #endif // BITCOIN_TXORPHANAGE_H diff --git a/src/util/transaction_identifier.h b/src/util/transaction_identifier.h new file mode 100644 index 0000000000..4fb9b49966 --- /dev/null +++ b/src/util/transaction_identifier.h @@ -0,0 +1,68 @@ +#ifndef BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H +#define BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H + +#include <attributes.h> +#include <uint256.h> +#include <util/types.h> + +/** transaction_identifier represents the two canonical transaction identifier + * types (txid, wtxid).*/ +template <bool has_witness> +class transaction_identifier +{ + uint256 m_wrapped; + + // Note: Use FromUint256 externally instead. + transaction_identifier(const uint256& wrapped) : m_wrapped{wrapped} {} + + // TODO: Comparisons with uint256 should be disallowed once we have + // converted most of the code to using the new txid types. + constexpr int Compare(const uint256& other) const { return m_wrapped.Compare(other); } + constexpr int Compare(const transaction_identifier<has_witness>& other) const { return m_wrapped.Compare(other.m_wrapped); } + template <typename Other> + constexpr int Compare(const Other& other) const + { + static_assert(ALWAYS_FALSE<Other>, "Forbidden comparison type"); + return 0; + } + +public: + transaction_identifier() : m_wrapped{} {} + + template <typename Other> + bool operator==(const Other& other) const { return Compare(other) == 0; } + template <typename Other> + bool operator!=(const Other& other) const { return Compare(other) != 0; } + template <typename Other> + bool operator<(const Other& other) const { return Compare(other) < 0; } + + const uint256& ToUint256() const LIFETIMEBOUND { return m_wrapped; } + static transaction_identifier FromUint256(const uint256& id) { return {id}; } + + /** Wrapped `uint256` methods. */ + constexpr bool IsNull() const { return m_wrapped.IsNull(); } + constexpr void SetNull() { m_wrapped.SetNull(); } + std::string GetHex() const { return m_wrapped.GetHex(); } + std::string ToString() const { return m_wrapped.ToString(); } + constexpr const std::byte* data() const { return reinterpret_cast<const std::byte*>(m_wrapped.data()); } + constexpr const std::byte* begin() const { return reinterpret_cast<const std::byte*>(m_wrapped.begin()); } + constexpr const std::byte* end() const { return reinterpret_cast<const std::byte*>(m_wrapped.end()); } + template <typename Stream> void Serialize(Stream& s) const { m_wrapped.Serialize(s); } + template <typename Stream> void Unserialize(Stream& s) { m_wrapped.Unserialize(s); } + + /** Conversion function to `uint256`. + * + * Note: new code should use `ToUint256`. + * + * TODO: This should be removed once the majority of the code has switched + * to using the Txid and Wtxid types. Until then it makes for a smoother + * transition to allow this conversion. */ + operator const uint256&() const LIFETIMEBOUND { return m_wrapped; } +}; + +/** Txid commits to all transaction fields except the witness. */ +using Txid = transaction_identifier<false>; +/** Wtxid commits to all transaction fields including the witness. */ +using Wtxid = transaction_identifier<true>; + +#endif // BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H diff --git a/src/validation.cpp b/src/validation.cpp index a6cab6b095..589e023aaf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -618,7 +618,7 @@ private: const CTransactionRef& m_ptx; /** Txid. */ - const uint256& m_hash; + const Txid& m_hash; TxValidationState m_state; /** A temporary cache containing serialized transaction data for signature verification. * Reused across PolicyScriptChecks and ConsensusScriptChecks. */ @@ -1870,7 +1870,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, // transaction). uint256 hashCacheEntry; CSHA256 hasher = g_scriptExecutionCacheHasher; - hasher.Write(tx.GetWitnessHash().begin(), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin()); + hasher.Write(UCharCast(tx.GetWitnessHash().begin()), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin()); AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks if (g_scriptExecutionCache.contains(hashCacheEntry, !cacheFullScriptStore)) { return true; diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp index e6b25cc216..b32dc8ed5a 100644 --- a/src/wallet/test/group_outputs_tests.cpp +++ b/src/wallet/test/group_outputs_tests.cpp @@ -40,7 +40,7 @@ static void addCoin(CoinsResult& coins, tx.vout[0].nValue = nValue; tx.vout[0].scriptPubKey = GetScriptForDestination(dest); - const uint256& txid = tx.GetHash(); + const auto txid{tx.GetHash().ToUint256()}; 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)), TxStateInactive{})); assert(ret.second); diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 68c98ae6b9..5926d88129 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -78,7 +78,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup) // Try to create a tx that spends more than what preset inputs + wallet selected inputs are covering for. // The wallet can cover up to 200 BTC, and the tx target is 299 BTC. - std::vector<CRecipient> recipients = {{GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::BECH32, "dummy"))), + std::vector<CRecipient> recipients{{*Assert(wallet->GetNewDestination(OutputType::BECH32, "dummy")), /*nAmount=*/299 * COIN, /*fSubtractFeeFromAmount=*/true}}; CCoinControl coin_control; coin_control.m_allow_other_inputs = true; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index ad4bb3a9d2..bcbc31ed3e 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -605,7 +605,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) // returns the coin associated with the change address underneath the // coinbaseKey pubkey, even though the change address has a different // pubkey. - AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, /*subtract_fee=*/false}); + AddTx(CRecipient{PubKeyDestination{{}}, 1 * COIN, /*subtract_fee=*/false}); { LOCK(wallet->cs_wallet); list = ListCoins(*wallet); @@ -963,7 +963,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup) mtx.vin.clear(); mtx.vin.emplace_back(tx_id_to_spend, 0); wallet.transactionAddedToMempool(MakeTransactionRef(mtx)); - const uint256& good_tx_id = mtx.GetHash(); + const auto good_tx_id{mtx.GetHash().ToUint256()}; { // Verify balance update for the new tx and the old one diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 0d0821e857..db858fa5ba 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -5,19 +5,20 @@ #ifndef BITCOIN_WALLET_TRANSACTION_H #define BITCOIN_WALLET_TRANSACTION_H -#include <bitset> -#include <cstdint> +#include <attributes.h> #include <consensus/amount.h> #include <primitives/transaction.h> -#include <serialize.h> -#include <wallet/types.h> -#include <threadsafety.h> #include <tinyformat.h> +#include <uint256.h> #include <util/overloaded.h> #include <util/strencodings.h> #include <util/string.h> +#include <wallet/types.h> -#include <list> +#include <bitset> +#include <cstdint> +#include <map> +#include <utility> #include <variant> #include <vector> @@ -330,8 +331,8 @@ public: bool isInactive() const { return state<TxStateInactive>(); } bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } bool isConfirmed() const { return state<TxStateConfirmed>(); } - const uint256& GetHash() const { return tx->GetHash(); } - const uint256& GetWitnessHash() const { return tx->GetWitnessHash(); } + const Txid& GetHash() const LIFETIMEBOUND { return tx->GetHash(); } + const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return tx->GetWitnessHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } private: diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 9f1557f65a..1fd938d18a 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -39,7 +39,6 @@ from test_framework.util import ( assert_greater_than, assert_greater_than_or_equal, assert_raises_rpc_error, - find_output, find_vout_for_address, ) from test_framework.wallet_util import ( @@ -417,16 +416,17 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].converttopsbt(hexstring=signedtx['hex'], permitsigdata=True) # Create outputs to nodes 1 and 2 + # (note that we intentionally create two different txs here, as we want + # to check that each node is missing prevout data for one of the two + # utxos, see "should only have data for one input" test below) node1_addr = self.nodes[1].getnewaddress() node2_addr = self.nodes[2].getnewaddress() - txid1 = self.nodes[0].sendtoaddress(node1_addr, 13) - txid2 = self.nodes[0].sendtoaddress(node2_addr, 13) - blockhash = self.generate(self.nodes[0], 6)[0] - vout1 = find_output(self.nodes[1], txid1, 13, blockhash=blockhash) - vout2 = find_output(self.nodes[2], txid2, 13, blockhash=blockhash) + utxo1 = self.create_outpoints(self.nodes[0], outputs=[{node1_addr: 13}])[0] + utxo2 = self.create_outpoints(self.nodes[0], outputs=[{node2_addr: 13}])[0] + self.generate(self.nodes[0], 6)[0] # Create a psbt spending outputs from nodes 1 and 2 - psbt_orig = self.nodes[0].createpsbt([{"txid":txid1, "vout":vout1}, {"txid":txid2, "vout":vout2}], {self.nodes[0].getnewaddress():25.999}) + psbt_orig = self.nodes[0].createpsbt([utxo1, utxo2], {self.nodes[0].getnewaddress():25.999}) # Update psbts, should only have data for one input and not the other psbt1 = self.nodes[1].walletprocesspsbt(psbt_orig, False, "ALL")['psbt'] @@ -603,14 +603,9 @@ class PSBTTest(BitcoinTestFramework): # Send to all types of addresses addr1 = self.nodes[1].getnewaddress("", "bech32") - txid1 = self.nodes[0].sendtoaddress(addr1, 11) - vout1 = find_output(self.nodes[0], txid1, 11) addr2 = self.nodes[1].getnewaddress("", "legacy") - txid2 = self.nodes[0].sendtoaddress(addr2, 11) - vout2 = find_output(self.nodes[0], txid2, 11) addr3 = self.nodes[1].getnewaddress("", "p2sh-segwit") - txid3 = self.nodes[0].sendtoaddress(addr3, 11) - vout3 = find_output(self.nodes[0], txid3, 11) + utxo1, utxo2, utxo3 = self.create_outpoints(self.nodes[1], outputs=[{addr1: 11}, {addr2: 11}, {addr3: 11}]) self.sync_all() def test_psbt_input_keys(psbt_input, keys): @@ -618,7 +613,7 @@ class PSBTTest(BitcoinTestFramework): assert_equal(set(keys), set(psbt_input.keys())) # Create a PSBT. None of the inputs are filled initially - psbt = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1},{"txid":txid2, "vout":vout2},{"txid":txid3, "vout":vout3}], {self.nodes[0].getnewaddress():32.999}) + psbt = self.nodes[1].createpsbt([utxo1, utxo2, utxo3], {self.nodes[0].getnewaddress():32.999}) decoded = self.nodes[1].decodepsbt(psbt) test_psbt_input_keys(decoded['inputs'][0], []) test_psbt_input_keys(decoded['inputs'][1], []) @@ -641,15 +636,14 @@ class PSBTTest(BitcoinTestFramework): test_psbt_input_keys(decoded['inputs'][2], ['non_witness_utxo','witness_utxo', 'bip32_derivs', 'redeem_script']) # Two PSBTs with a common input should not be joinable - psbt1 = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1}], {self.nodes[0].getnewaddress():Decimal('10.999')}) + psbt1 = self.nodes[1].createpsbt([utxo1], {self.nodes[0].getnewaddress():Decimal('10.999')}) assert_raises_rpc_error(-8, "exists in multiple PSBTs", self.nodes[1].joinpsbts, [psbt1, updated]) # Join two distinct PSBTs addr4 = self.nodes[1].getnewaddress("", "p2sh-segwit") - txid4 = self.nodes[0].sendtoaddress(addr4, 5) - vout4 = find_output(self.nodes[0], txid4, 5) + utxo4 = self.create_outpoints(self.nodes[0], outputs=[{addr4: 5}])[0] self.generate(self.nodes[0], 6) - psbt2 = self.nodes[1].createpsbt([{"txid":txid4, "vout":vout4}], {self.nodes[0].getnewaddress():Decimal('4.999')}) + psbt2 = self.nodes[1].createpsbt([utxo4], {self.nodes[0].getnewaddress():Decimal('4.999')}) psbt2 = self.nodes[1].walletprocesspsbt(psbt2)['psbt'] psbt2_decoded = self.nodes[0].decodepsbt(psbt2) assert "final_scriptwitness" in psbt2_decoded['inputs'][0] and "final_scriptSig" in psbt2_decoded['inputs'][0] @@ -669,11 +663,10 @@ class PSBTTest(BitcoinTestFramework): # Newly created PSBT needs UTXOs and updating addr = self.nodes[1].getnewaddress("", "p2sh-segwit") - txid = self.nodes[0].sendtoaddress(addr, 7) + utxo = self.create_outpoints(self.nodes[0], outputs=[{addr: 7}])[0] addrinfo = self.nodes[1].getaddressinfo(addr) - blockhash = self.generate(self.nodes[0], 6)[0] - vout = find_output(self.nodes[0], txid, 7, blockhash=blockhash) - psbt = self.nodes[1].createpsbt([{"txid":txid, "vout":vout}], {self.nodes[0].getnewaddress("", "p2sh-segwit"):Decimal('6.999')}) + self.generate(self.nodes[0], 6)[0] + psbt = self.nodes[1].createpsbt([utxo], {self.nodes[0].getnewaddress("", "p2sh-segwit"):Decimal('6.999')}) analyzed = self.nodes[0].analyzepsbt(psbt) assert not analyzed['inputs'][0]['has_utxo'] and not analyzed['inputs'][0]['is_final'] and analyzed['inputs'][0]['next'] == 'updater' and analyzed['next'] == 'updater' @@ -872,9 +865,8 @@ class PSBTTest(BitcoinTestFramework): self.log.info("Test that walletprocesspsbt both updates and signs a non-updated psbt containing Taproot inputs") addr = self.nodes[0].getnewaddress("", "bech32m") - txid = self.nodes[0].sendtoaddress(addr, 1) - vout = find_vout_for_address(self.nodes[0], txid, addr) - psbt = self.nodes[0].createpsbt([{"txid": txid, "vout": vout}], [{self.nodes[0].getnewaddress(): 0.9999}]) + utxo = self.create_outpoints(self.nodes[0], outputs=[{addr: 1}])[0] + psbt = self.nodes[0].createpsbt([utxo], [{self.nodes[0].getnewaddress(): 0.9999}]) signed = self.nodes[0].walletprocesspsbt(psbt) rawtx = signed["hex"] self.nodes[0].sendrawtransaction(rawtx) @@ -962,11 +954,10 @@ class PSBTTest(BitcoinTestFramework): descriptor = descsum_create(f"wpkh({key})") - txid = self.nodes[0].sendtoaddress(address, 1) + utxo = self.create_outpoints(self.nodes[0], outputs=[{address: 1}])[0] self.sync_all() - vout = find_output(self.nodes[0], txid, 1) - psbt = self.nodes[2].createpsbt([{"txid": txid, "vout": vout}], {self.nodes[0].getnewaddress(): 0.99999}) + psbt = self.nodes[2].createpsbt([utxo], {self.nodes[0].getnewaddress(): 0.99999}) decoded = self.nodes[2].decodepsbt(psbt) test_psbt_input_keys(decoded['inputs'][0], []) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 4e6d245b5f..70b3943478 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -30,6 +30,7 @@ from .util import ( PortSeed, assert_equal, check_json_precision, + find_vout_for_address, get_datadir_path, initialize_datadir, p2p_port, @@ -697,6 +698,22 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): sync_fun() if sync_fun else self.sync_all() return blocks + def create_outpoints(self, node, *, outputs): + """Send funds to a given list of `{address: amount}` targets using the bitcoind + wallet and return the corresponding outpoints as a list of dictionaries + `[{"txid": txid, "vout": vout1}, {"txid": txid, "vout": vout2}, ...]`. + The result can be used to specify inputs for RPCs like `createrawtransaction`, + `createpsbt`, `lockunspent` etc.""" + assert all(len(output.keys()) == 1 for output in outputs) + send_res = node.send(outputs) + assert send_res["complete"] + utxos = [] + for output in outputs: + address = list(output.keys())[0] + vout = find_vout_for_address(node, send_res["txid"], address) + utxos.append({"txid": send_res["txid"], "vout": vout}) + return utxos + def sync_blocks(self, nodes=None, wait=1, timeout=60): """ Wait until everybody has the same tip. diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index efb1dbd554..104f158d92 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -483,18 +483,6 @@ def check_node_connections(*, node, num_in, num_out): ############################# -def find_output(node, txid, amount, *, blockhash=None): - """ - Return index to output of txid with value amount - Raises exception if there is none. - """ - txdata = node.getrawtransaction(txid, 1, blockhash) - for i in range(len(txdata["vout"])): - if txdata["vout"][i]["value"] == amount: - return i - raise RuntimeError("find_output txid %s : %s not found" % (txid, str(amount))) - - # Create large OP_RETURN txouts that can be appended to a transaction # to make it large (helper for constructing large transactions). The # total serialized size of the txouts is about 66k vbytes. diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 01149a0977..78bfa97212 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -18,7 +18,6 @@ from test_framework.util import ( assert_equal, assert_fee_amount, assert_raises_rpc_error, - find_vout_for_address, ) from test_framework.wallet_util import test_address from test_framework.wallet import MiniWallet @@ -471,10 +470,9 @@ class WalletTest(BitcoinTestFramework): # Import address and private key to check correct behavior of spendable unspents # 1. Send some coins to generate new UTXO address_to_import = self.nodes[2].getnewaddress() - txid = self.nodes[0].sendtoaddress(address_to_import, 1) + utxo = self.create_outpoints(self.nodes[0], outputs=[{address_to_import: 1}])[0] self.sync_mempools(self.nodes[0:3]) - vout = find_vout_for_address(self.nodes[2], txid, address_to_import) - self.nodes[2].lockunspent(False, [{"txid": txid, "vout": vout}]) + self.nodes[2].lockunspent(False, [utxo]) self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3])) self.log.info("Test sendtoaddress with fee_rate param (explicit fee rate in sat/vB)") diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 77611649ac..a331ba997e 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -22,7 +22,6 @@ from test_framework.util import ( assert_greater_than_or_equal, assert_raises_rpc_error, count_bytes, - find_vout_for_address, get_fee, ) from test_framework.wallet_util import generate_keypair, WalletUnlock @@ -85,14 +84,13 @@ class RawTransactionsTest(BitcoinTestFramework): Unlock all UTXOs except the watchonly one """ to_keep = [] - if self.watchonly_txid is not None and self.watchonly_vout is not None: - to_keep.append({"txid": self.watchonly_txid, "vout": self.watchonly_vout}) + if self.watchonly_utxo is not None: + to_keep.append(self.watchonly_utxo) wallet.lockunspent(True) wallet.lockunspent(False, to_keep) def run_test(self): - self.watchonly_txid = None - self.watchonly_vout = None + self.watchonly_utxo = None self.log.info("Connect nodes, set fees, generate blocks, and sync") self.min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] # This test is not meant to test fee estimation and we'd like @@ -163,11 +161,10 @@ class RawTransactionsTest(BitcoinTestFramework): watchonly_pubkey = self.nodes[0].getaddressinfo(watchonly_address)["pubkey"] self.watchonly_amount = Decimal(200) wwatch.importpubkey(watchonly_pubkey, "", True) - self.watchonly_txid = self.nodes[0].sendtoaddress(watchonly_address, self.watchonly_amount) + self.watchonly_utxo = self.create_outpoints(self.nodes[0], outputs=[{watchonly_address: self.watchonly_amount}])[0] # Lock UTXO so nodes[0] doesn't accidentally spend it - self.watchonly_vout = find_vout_for_address(self.nodes[0], self.watchonly_txid, watchonly_address) - self.nodes[0].lockunspent(False, [{"txid": self.watchonly_txid, "vout": self.watchonly_vout}]) + self.nodes[0].lockunspent(False, [self.watchonly_utxo]) self.nodes[0].sendtoaddress(self.nodes[3].get_wallet_rpc(self.default_wallet_name).getnewaddress(), self.watchonly_amount / 10) @@ -738,7 +735,7 @@ class RawTransactionsTest(BitcoinTestFramework): result = wwatch.fundrawtransaction(rawtx, True) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 1) - assert_equal(res_dec["vin"][0]["txid"], self.watchonly_txid) + assert_equal(res_dec["vin"][0]["txid"], self.watchonly_utxo['txid']) assert "fee" in result.keys() assert_greater_than(result["changepos"], -1) @@ -758,7 +755,7 @@ class RawTransactionsTest(BitcoinTestFramework): result = wwatch.fundrawtransaction(rawtx, includeWatching=True, changeAddress=w3.getrawchangeaddress(), subtractFeeFromOutputs=[0]) res_dec = self.nodes[0].decoderawtransaction(result["hex"]) assert_equal(len(res_dec["vin"]), 1) - assert res_dec["vin"][0]["txid"] == self.watchonly_txid + assert res_dec["vin"][0]["txid"] == self.watchonly_utxo['txid'] assert_greater_than(result["fee"], 0) assert_equal(result["changepos"], -1) @@ -970,10 +967,9 @@ class RawTransactionsTest(BitcoinTestFramework): self.log.info("Test fundrawtxn subtract fee from outputs with preset inputs that are sufficient") addr = self.nodes[0].getnewaddress() - txid = self.nodes[0].sendtoaddress(addr, 10) - vout = find_vout_for_address(self.nodes[0], txid, addr) + utxo = self.create_outpoints(self.nodes[0], outputs=[{addr: 10}])[0] - rawtx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], [{self.nodes[0].getnewaddress(): 5}]) + rawtx = self.nodes[0].createrawtransaction([utxo], [{self.nodes[0].getnewaddress(): 5}]) fundedtx = self.nodes[0].fundrawtransaction(rawtx, subtractFeeFromOutputs=[0]) signedtx = self.nodes[0].signrawtransactionwithwallet(fundedtx['hex']) self.nodes[0].sendrawtransaction(signedtx['hex']) @@ -1264,14 +1260,12 @@ class RawTransactionsTest(BitcoinTestFramework): addr = wallet.getnewaddress(address_type="bech32") ext_addr = self.nodes[0].getnewaddress(address_type="bech32") - txid = self.nodes[0].send([{addr: 5}, {ext_addr: 5}])["txid"] - vout = find_vout_for_address(self.nodes[0], txid, addr) - ext_vout = find_vout_for_address(self.nodes[0], txid, ext_addr) + utxo, ext_utxo = self.create_outpoints(self.nodes[0], outputs=[{addr: 5}, {ext_addr: 5}]) self.nodes[0].sendtoaddress(wallet.getnewaddress(address_type="bech32"), 5) self.generate(self.nodes[0], 1) - rawtx = wallet.createrawtransaction([{'txid': txid, 'vout': vout}], [{self.nodes[0].getnewaddress(address_type="bech32"): 8}]) + rawtx = wallet.createrawtransaction([utxo], [{self.nodes[0].getnewaddress(address_type="bech32"): 8}]) fundedtx = wallet.fundrawtransaction(rawtx, fee_rate=10, change_type="bech32") # with 71-byte signatures we should expect following tx size # tx overhead (10) + 2 inputs (41 each) + 2 p2wpkh (31 each) + (segwit marker and flag (2) + 2 p2wpkh 71 byte sig witnesses (107 each)) / witness scaling factor (4) @@ -1279,7 +1273,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(fundedtx['fee'] * COIN, tx_size * 10) # Using the other output should have 72 byte sigs - rawtx = wallet.createrawtransaction([{'txid': txid, 'vout': ext_vout}], [{self.nodes[0].getnewaddress(): 13}]) + rawtx = wallet.createrawtransaction([ext_utxo], [{self.nodes[0].getnewaddress(): 13}]) ext_desc = self.nodes[0].getaddressinfo(ext_addr)["desc"] fundedtx = wallet.fundrawtransaction(rawtx, fee_rate=10, change_type="bech32", solving_data={"descriptors": [ext_desc]}) # tx overhead (10) + 3 inputs (41 each) + 2 p2wpkh(31 each) + (segwit marker and flag (2) + 2 p2wpkh 71 bytes sig witnesses (107 each) + p2wpkh 72 byte sig witness (108)) / witness scaling factor (4) @@ -1298,10 +1292,9 @@ class RawTransactionsTest(BitcoinTestFramework): addr = wallet.getnewaddress() inputs = [] for i in range(0, 2): - txid = self.nodes[2].sendtoaddress(addr, 5) - self.sync_mempools() - vout = find_vout_for_address(wallet, txid, addr) - inputs.append((txid, vout)) + utxo = self.create_outpoints(self.nodes[2], outputs=[{addr: 5}])[0] + inputs.append((utxo['txid'], utxo['vout'])) + self.sync_mempools() # Unsafe inputs are ignored by default. rawtx = wallet.createrawtransaction([], [{self.nodes[2].getnewaddress(): 7.5}]) diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index ad5ae111aa..1f1f92589c 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -24,7 +24,6 @@ from test_framework.descriptors import descsum_create from test_framework.util import ( assert_equal, assert_raises_rpc_error, - find_vout_for_address, ) from test_framework.wallet_util import ( get_generate_key, @@ -493,12 +492,10 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 999) # generate some utxos for next tests - txid = w0.sendtoaddress(addr, 10) - vout = find_vout_for_address(self.nodes[0], txid, addr) + utxo = self.create_outpoints(w0, outputs=[{addr: 10}])[0] addr2 = wmulti_pub.getnewaddress('', 'bech32') - txid2 = w0.sendtoaddress(addr2, 10) - vout2 = find_vout_for_address(self.nodes[0], txid2, addr2) + utxo2 = self.create_outpoints(w0, outputs=[{addr2: 10}])[0] self.generate(self.nodes[0], 6) assert_equal(wmulti_pub.getbalance(), wmulti_priv.getbalance()) @@ -554,7 +551,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(res[1]['success'], True) assert_equal(res[1]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors') - rawtx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {w0.getnewaddress(): 9.999}) + rawtx = self.nodes[1].createrawtransaction([utxo], {w0.getnewaddress(): 9.999}) tx_signed_1 = wmulti_priv1.signrawtransactionwithwallet(rawtx) assert_equal(tx_signed_1['complete'], False) tx_signed_2 = wmulti_priv2.signrawtransactionwithwallet(tx_signed_1['hex']) @@ -648,7 +645,7 @@ class ImportDescriptorsTest(BitcoinTestFramework): }]) assert_equal(res[0]['success'], True) - rawtx = self.nodes[1].createrawtransaction([{'txid': txid2, 'vout': vout2}], {w0.getnewaddress(): 9.999}) + rawtx = self.nodes[1].createrawtransaction([utxo2], {w0.getnewaddress(): 9.999}) tx = wmulti_priv3.signrawtransactionwithwallet(rawtx) assert_equal(tx['complete'], True) self.nodes[1].sendrawtransaction(tx['hex']) diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index aede9281d5..e2edaef4da 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -23,7 +23,6 @@ from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_scrip from test_framework.util import ( assert_equal, assert_raises_rpc_error, - find_vout_for_address, sha256sum_file, ) from test_framework.wallet_util import ( @@ -310,14 +309,14 @@ class WalletMigrationTest(BitcoinTestFramework): # Received watchonly tx that is then spent import_sent_addr = default.getnewaddress() imports0.importaddress(import_sent_addr) - received_sent_watchonly_txid = default.sendtoaddress(import_sent_addr, 10) - received_sent_watchonly_vout = find_vout_for_address(self.nodes[0], received_sent_watchonly_txid, import_sent_addr) - send = default.sendall(recipients=[default.getnewaddress()], inputs=[{"txid": received_sent_watchonly_txid, "vout": received_sent_watchonly_vout}]) + received_sent_watchonly_utxo = self.create_outpoints(node=default, outputs=[{import_sent_addr: 10}])[0] + + send = default.sendall(recipients=[default.getnewaddress()], inputs=[received_sent_watchonly_utxo]) sent_watchonly_txid = send["txid"] self.generate(self.nodes[0], 1) received_watchonly_tx_info = imports0.gettransaction(received_watchonly_txid, True) - received_sent_watchonly_tx_info = imports0.gettransaction(received_sent_watchonly_txid, True) + received_sent_watchonly_tx_info = imports0.gettransaction(received_sent_watchonly_utxo["txid"], True) balances = imports0.getbalances() spendable_bal = balances["mine"]["trusted"] @@ -332,7 +331,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(imports0.getwalletinfo()["descriptors"], True) self.assert_is_sqlite("imports0") assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_watchonly_txid) - assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_sent_watchonly_txid) + assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_sent_watchonly_utxo['txid']) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, sent_watchonly_txid) assert_equal(len(imports0.listtransactions(include_watchonly=True)), 1) imports0.gettransaction(received_txid) @@ -347,7 +346,7 @@ class WalletMigrationTest(BitcoinTestFramework): received_migrated_watchonly_tx_info = watchonly.gettransaction(received_watchonly_txid) assert_equal(received_watchonly_tx_info["time"], received_migrated_watchonly_tx_info["time"]) assert_equal(received_watchonly_tx_info["timereceived"], received_migrated_watchonly_tx_info["timereceived"]) - received_sent_migrated_watchonly_tx_info = watchonly.gettransaction(received_sent_watchonly_txid) + received_sent_migrated_watchonly_tx_info = watchonly.gettransaction(received_sent_watchonly_utxo["txid"]) assert_equal(received_sent_watchonly_tx_info["time"], received_sent_migrated_watchonly_tx_info["time"]) assert_equal(received_sent_watchonly_tx_info["timereceived"], received_sent_migrated_watchonly_tx_info["timereceived"]) watchonly.gettransaction(sent_watchonly_txid) diff --git a/test/functional/wallet_signrawtransactionwithwallet.py b/test/functional/wallet_signrawtransactionwithwallet.py index d560dfdc11..b0517f951d 100755 --- a/test/functional/wallet_signrawtransactionwithwallet.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -14,7 +14,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - find_vout_for_address, ) from test_framework.messages import ( CTxInWitness, @@ -194,13 +193,12 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): address = script_to_p2wsh(script) # Fund that address and make the spend - txid = self.nodes[0].sendtoaddress(address, 1) - vout = find_vout_for_address(self.nodes[0], txid, address) + utxo1 = self.create_outpoints(self.nodes[0], outputs=[{address: 1}])[0] self.generate(self.nodes[0], 1) - utxo = self.nodes[0].listunspent()[0] - amt = Decimal(1) + utxo["amount"] - Decimal(0.00001) + utxo2 = self.nodes[0].listunspent()[0] + amt = Decimal(1) + utxo2["amount"] - Decimal(0.00001) tx = self.nodes[0].createrawtransaction( - [{"txid": txid, "vout": vout, "sequence": 1},{"txid": utxo["txid"], "vout": utxo["vout"]}], + [{**utxo1, "sequence": 1},{"txid": utxo2["txid"], "vout": utxo2["vout"]}], [{self.nodes[0].getnewaddress(): amt}], self.nodes[0].getblockcount() ) @@ -229,13 +227,12 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): address = script_to_p2wsh(script) # Fund that address and make the spend - txid = self.nodes[0].sendtoaddress(address, 1) - vout = find_vout_for_address(self.nodes[0], txid, address) + utxo1 = self.create_outpoints(self.nodes[0], outputs=[{address: 1}])[0] self.generate(self.nodes[0], 1) - utxo = self.nodes[0].listunspent()[0] - amt = Decimal(1) + utxo["amount"] - Decimal(0.00001) + utxo2 = self.nodes[0].listunspent()[0] + amt = Decimal(1) + utxo2["amount"] - Decimal(0.00001) tx = self.nodes[0].createrawtransaction( - [{"txid": txid, "vout": vout},{"txid": utxo["txid"], "vout": utxo["vout"]}], + [utxo1, {"txid": utxo2["txid"], "vout": utxo2["vout"]}], [{self.nodes[0].getnewaddress(): amt}], self.nodes[0].getblockcount() ) diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index d8ef66d83a..1f3b6f2ce9 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -7,7 +7,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - find_vout_for_address ) from test_framework.messages import ( COIN, @@ -35,8 +34,8 @@ class TxnMallTest(BitcoinTestFramework): super().setup_network() self.disconnect_nodes(1, 2) - def spend_txid(self, txid, vout, outputs): - inputs = [{"txid": txid, "vout": vout}] + def spend_utxo(self, utxo, outputs): + inputs = [utxo] tx = self.nodes[0].createrawtransaction(inputs, outputs) tx = self.nodes[0].fundrawtransaction(tx) tx = self.nodes[0].signrawtransactionwithwallet(tx['hex']) @@ -56,13 +55,13 @@ class TxnMallTest(BitcoinTestFramework): self.nodes[0].settxfee(.001) node0_address1 = self.nodes[0].getnewaddress(address_type=output_type) - node0_txid1 = self.nodes[0].sendtoaddress(node0_address1, 1219) - node0_tx1 = self.nodes[0].gettransaction(node0_txid1) - self.nodes[0].lockunspent(False, [{"txid":node0_txid1, "vout": find_vout_for_address(self.nodes[0], node0_txid1, node0_address1)}]) + node0_utxo1 = self.create_outpoints(self.nodes[0], outputs=[{node0_address1: 1219}])[0] + node0_tx1 = self.nodes[0].gettransaction(node0_utxo1['txid']) + self.nodes[0].lockunspent(False, [node0_utxo1]) node0_address2 = self.nodes[0].getnewaddress(address_type=output_type) - node0_txid2 = self.nodes[0].sendtoaddress(node0_address2, 29) - node0_tx2 = self.nodes[0].gettransaction(node0_txid2) + node0_utxo2 = self.create_outpoints(self.nodes[0], outputs=[{node0_address2: 29}])[0] + node0_tx2 = self.nodes[0].gettransaction(node0_utxo2['txid']) assert_equal(self.nodes[0].getbalance(), starting_balance + node0_tx1["fee"] + node0_tx2["fee"]) @@ -71,8 +70,8 @@ class TxnMallTest(BitcoinTestFramework): node1_address = self.nodes[1].getnewaddress() # Send tx1, and another transaction tx2 that won't be cloned - txid1 = self.spend_txid(node0_txid1, find_vout_for_address(self.nodes[0], node0_txid1, node0_address1), {node1_address: 40}) - txid2 = self.spend_txid(node0_txid2, find_vout_for_address(self.nodes[0], node0_txid2, node0_address2), {node1_address: 20}) + txid1 = self.spend_utxo(node0_utxo1, {node1_address: 40}) + txid2 = self.spend_utxo(node0_utxo2, {node1_address: 20}) # Construct a clone of tx1, to be malleated rawtx1 = self.nodes[0].getrawtransaction(txid1, 1) diff --git a/test/functional/wallet_txn_doublespend.py b/test/functional/wallet_txn_doublespend.py index 38ebfe0d7a..3cd0cd3207 100755 --- a/test/functional/wallet_txn_doublespend.py +++ b/test/functional/wallet_txn_doublespend.py @@ -8,8 +8,6 @@ from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - find_output, - find_vout_for_address ) @@ -31,8 +29,8 @@ class TxnMallTest(BitcoinTestFramework): super().setup_network() self.disconnect_nodes(1, 2) - def spend_txid(self, txid, vout, outputs): - inputs = [{"txid": txid, "vout": vout}] + def spend_utxo(self, utxo, outputs): + inputs = [utxo] tx = self.nodes[0].createrawtransaction(inputs, outputs) tx = self.nodes[0].fundrawtransaction(tx) tx = self.nodes[0].signrawtransactionwithwallet(tx['hex']) @@ -54,13 +52,13 @@ class TxnMallTest(BitcoinTestFramework): # Assign coins to foo and bar addresses: node0_address_foo = self.nodes[0].getnewaddress() - fund_foo_txid = self.nodes[0].sendtoaddress(node0_address_foo, 1219) - fund_foo_tx = self.nodes[0].gettransaction(fund_foo_txid) - self.nodes[0].lockunspent(False, [{"txid":fund_foo_txid, "vout": find_vout_for_address(self.nodes[0], fund_foo_txid, node0_address_foo)}]) + fund_foo_utxo = self.create_outpoints(self.nodes[0], outputs=[{node0_address_foo: 1219}])[0] + fund_foo_tx = self.nodes[0].gettransaction(fund_foo_utxo['txid']) + self.nodes[0].lockunspent(False, [fund_foo_utxo]) node0_address_bar = self.nodes[0].getnewaddress() - fund_bar_txid = self.nodes[0].sendtoaddress(node0_address_bar, 29) - fund_bar_tx = self.nodes[0].gettransaction(fund_bar_txid) + fund_bar_utxo = self.create_outpoints(node=self.nodes[0], outputs=[{node0_address_bar: 29}])[0] + fund_bar_tx = self.nodes[0].gettransaction(fund_bar_utxo['txid']) assert_equal(self.nodes[0].getbalance(), starting_balance + fund_foo_tx["fee"] + fund_bar_tx["fee"]) @@ -71,13 +69,7 @@ class TxnMallTest(BitcoinTestFramework): # First: use raw transaction API to send 1240 BTC to node1_address, # but don't broadcast: doublespend_fee = Decimal('-.02') - rawtx_input_0 = {} - rawtx_input_0["txid"] = fund_foo_txid - rawtx_input_0["vout"] = find_output(self.nodes[0], fund_foo_txid, 1219) - rawtx_input_1 = {} - rawtx_input_1["txid"] = fund_bar_txid - rawtx_input_1["vout"] = find_output(self.nodes[0], fund_bar_txid, 29) - inputs = [rawtx_input_0, rawtx_input_1] + inputs = [fund_foo_utxo, fund_bar_utxo] change_address = self.nodes[0].getnewaddress() outputs = {} outputs[node1_address] = 1240 @@ -87,8 +79,8 @@ class TxnMallTest(BitcoinTestFramework): assert_equal(doublespend["complete"], True) # Create two spends using 1 50 BTC coin each - txid1 = self.spend_txid(fund_foo_txid, find_vout_for_address(self.nodes[0], fund_foo_txid, node0_address_foo), {node1_address: 40}) - txid2 = self.spend_txid(fund_bar_txid, find_vout_for_address(self.nodes[0], fund_bar_txid, node0_address_bar), {node1_address: 20}) + txid1 = self.spend_utxo(fund_foo_utxo, {node1_address: 40}) + txid2 = self.spend_utxo(fund_bar_utxo, {node1_address: 20}) # Have node0 mine a block: if (self.options.mine_block): |