diff options
Diffstat (limited to 'src')
56 files changed, 842 insertions, 442 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index feed4a0061..e542a067a4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -191,6 +191,7 @@ BITCOIN_CORE_H = \ kernel/mempool_options.h \ kernel/mempool_persist.h \ kernel/mempool_removal_reason.h \ + kernel/messagestartchars.h \ kernel/notifications_interface.h \ kernel/validation_cache_sizes.h \ key.h \ diff --git a/src/addrdb.cpp b/src/addrdb.cpp index b5e35bf8f6..8b85b77e2b 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -90,10 +90,10 @@ void DeserializeDB(Stream& stream, Data&& data, bool fCheckSum = true) { HashVerifier verifier{stream}; // de-serialize file header (network specific magic number) and .. - unsigned char pchMsgTmp[4]; + MessageStartChars pchMsgTmp; verifier >> pchMsgTmp; // ... verify the network matches ours - if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) { + if (pchMsgTmp != Params().MessageStart()) { throw std::runtime_error{"Invalid network magic number"}; } diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 4d032cefc5..0737144b84 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -6,6 +6,7 @@ #include <consensus/validation.h> #include <crypto/sha256.h> #include <node/miner.h> +#include <random.h> #include <test/util/mining.h> #include <test/util/script.h> #include <test/util/setup_common.h> diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 0e110a653a..249b76ee85 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -79,7 +79,7 @@ static void CoinSelection(benchmark::Bench& bench) }; auto group = wallet::GroupOutputs(wallet, available_coins, coin_selection_params, {{filter_standard}})[filter_standard]; bench.run([&] { - auto result = AttemptSelection(1003 * COIN, group, coin_selection_params, /*allow_mixed_output_types=*/true); + auto result = AttemptSelection(wallet.chain(), 1003 * COIN, group, coin_selection_params, /*allow_mixed_output_types=*/true); assert(result); assert(result->GetSelectedValue() == 1003 * COIN); assert(result->GetInputSet().size() == 2); diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index b3799ad1b7..a1c8d4d942 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -7,6 +7,7 @@ #include <consensus/merkle.h> #include <consensus/validation.h> #include <pow.h> +#include <random.h> #include <test/util/setup_common.h> #include <txmempool.h> #include <validation.h> diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 1f94461d19..993db66653 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -5,6 +5,7 @@ #include <bench/bench.h> #include <kernel/mempool_entry.h> #include <policy/policy.h> +#include <random.h> #include <test/util/setup_common.h> #include <txmempool.h> #include <util/chaintype.h> diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp index 77f5940926..22b8f1b356 100644 --- a/src/bench/streams_findbyte.cpp +++ b/src/bench/streams_findbyte.cpp @@ -16,7 +16,7 @@ static void FindByte(benchmark::Bench& bench) data[file_size-1] = 1; fwrite(&data, sizeof(uint8_t), file_size, file); rewind(file); - CBufferedFile bf(file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size, 0, 0); + BufferedFile bf{file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size, 0}; bench.run([&] { bf.SetPos(0); diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 19c4d36126..fc83a4ad3a 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -22,6 +22,7 @@ #include <node/blockstorage.h> #include <node/caches.h> #include <node/chainstate.h> +#include <random.h> #include <scheduler.h> #include <script/sigcache.h> #include <util/chaintype.h> diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index 582c18c9c6..8b68d04b2b 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -17,6 +17,7 @@ #include <core_io.h> #include <streams.h> #include <util/exception.h> +#include <util/strencodings.h> #include <util/translation.h> #include <version.h> diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index c95937ba75..775496e21b 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -4,7 +4,6 @@ #include <dbwrapper.h> -#include <clientversion.h> #include <logging.h> #include <random.h> #include <serialize.h> @@ -156,9 +155,9 @@ struct CDBBatch::WriteBatchImpl { leveldb::WriteBatch batch; }; -CDBBatch::CDBBatch(const CDBWrapper& _parent) : parent(_parent), - m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()}, - ssValue(SER_DISK, CLIENT_VERSION){}; +CDBBatch::CDBBatch(const CDBWrapper& _parent) + : parent{_parent}, + m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()} {}; CDBBatch::~CDBBatch() = default; @@ -168,7 +167,7 @@ void CDBBatch::Clear() size_estimate = 0; } -void CDBBatch::WriteImpl(Span<const std::byte> key, CDataStream& ssValue) +void CDBBatch::WriteImpl(Span<const std::byte> key, DataStream& ssValue) { leveldb::Slice slKey(CharCast(key.data()), key.size()); ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent)); diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 2f7448e878..63c2f99d2a 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -80,11 +80,11 @@ private: const std::unique_ptr<WriteBatchImpl> m_impl_batch; DataStream ssKey{}; - CDataStream ssValue; + DataStream ssValue{}; size_t size_estimate{0}; - void WriteImpl(Span<const std::byte> key, CDataStream& ssValue); + void WriteImpl(Span<const std::byte> key, DataStream& ssValue); void EraseImpl(Span<const std::byte> key); public: diff --git a/src/headerssync.cpp b/src/headerssync.cpp index a3adfb4f70..f891063cd2 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -7,6 +7,7 @@ #include <pow.h> #include <timedata.h> #include <util/check.h> +#include <util/vector.h> // The two constants below are computed using the simulation script on // https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1 @@ -51,9 +52,9 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus void HeadersSyncState::Finalize() { Assume(m_download_state != State::FINAL); - m_header_commitments = {}; + ClearShrink(m_header_commitments); m_last_header_received.SetNull(); - m_redownloaded_headers = {}; + ClearShrink(m_redownloaded_headers); m_redownload_buffer_last_hash.SetNull(); m_redownload_buffer_first_prev_hash.SetNull(); m_process_all_remaining_headers = false; diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 3c9ec84e24..6b38e19d81 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -79,7 +79,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe return false; } - CAutoFile file(m_chainstate->m_blockman.OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION); + CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true), CLIENT_VERSION}; if (file.IsNull()) { return error("%s: OpenBlockFile failed", __func__); } diff --git a/src/init.cpp b/src/init.cpp index 1515007c54..6dd3d5970b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1223,7 +1223,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.connman); node.connman = std::make_unique<CConnman>(GetRand<uint64_t>(), GetRand<uint64_t>(), - *node.addrman, *node.netgroupman, args.GetBoolArg("-networkactive", true)); + *node.addrman, *node.netgroupman, chainparams, args.GetBoolArg("-networkactive", true)); assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index dd664165d3..b5243725ad 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -216,6 +216,43 @@ public: //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) = 0; + //! For each outpoint, calculate the fee-bumping cost to spend this outpoint at the specified + // feerate, including bumping its ancestors. For example, if the target feerate is 10sat/vbyte + // and this outpoint refers to a mempool transaction at 3sat/vbyte, the bump fee includes the + // cost to bump the mempool transaction to 10sat/vbyte (i.e. 7 * mempooltx.vsize). If that + // transaction also has, say, an unconfirmed parent with a feerate of 1sat/vbyte, the bump fee + // includes the cost to bump the parent (i.e. 9 * parentmempooltx.vsize). + // + // If the outpoint comes from an unconfirmed transaction that is already above the target + // feerate or bumped by its descendant(s) already, it does not need to be bumped. Its bump fee + // is 0. Likewise, if any of the transaction's ancestors are already bumped by a transaction + // in our mempool, they are not included in the transaction's bump fee. + // + // Also supported is bump-fee calculation in the case of replacements. If an outpoint + // conflicts with another transaction in the mempool, it is assumed that the goal is to replace + // that transaction. As such, the calculation will exclude the to-be-replaced transaction, but + // will include the fee-bumping cost. If bump fees of descendants of the to-be-replaced + // transaction are requested, the value will be 0. Fee-related RBF rules are not included as + // they are logically distinct. + // + // Any outpoints that are otherwise unavailable from the mempool (e.g. UTXOs from confirmed + // transactions or transactions not yet broadcast by the wallet) are given a bump fee of 0. + // + // If multiple outpoints come from the same transaction (which would be very rare because + // it means that one transaction has multiple change outputs or paid the same wallet using multiple + // outputs in the same transaction) or have shared ancestry, the bump fees are calculated + // independently, i.e. as if only one of them is spent. This may result in double-fee-bumping. This + // caveat can be rectified per use of the sister-function CalculateCombinedBumpFee(…). + virtual std::map<COutPoint, CAmount> CalculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) = 0; + + //! Calculate the combined bump fee for an input set per the same strategy + // as in CalculateIndividualBumpFees(…). + // Unlike CalculateIndividualBumpFees(…), this does not return individual + // bump fees per outpoint, but a single bump fee for the shared ancestry. + // The combined bump fee may be used to correct overestimation due to + // shared ancestry by multiple UTXOs after coin selection. + virtual std::optional<CAmount> CalculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) = 0; + //! Get the node's package limits. //! Currently only returns the ancestor and descendant count limits, but could be enhanced to //! return more policy settings. diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 733a3339b3..6cee379faf 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -10,6 +10,7 @@ #include <consensus/merkle.h> #include <consensus/params.h> #include <hash.h> +#include <kernel/messagestartchars.h> #include <logging.h> #include <primitives/block.h> #include <primitives/transaction.h> @@ -359,7 +360,7 @@ public: HashWriter h{}; h << consensus.signet_challenge; uint256 hash = h.GetHash(); - memcpy(pchMessageStart, hash.begin(), 4); + std::copy_n(hash.begin(), 4, pchMessageStart.begin()); nDefaultPort = 38333; nPruneAfterHeight = 1000; diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index 2d38af609c..63837bb23e 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -7,9 +7,8 @@ #define BITCOIN_KERNEL_CHAINPARAMS_H #include <consensus/params.h> -#include <netaddress.h> +#include <kernel/messagestartchars.h> #include <primitives/block.h> -#include <protocol.h> #include <uint256.h> #include <util/chaintype.h> #include <util/hash_type.h> @@ -87,17 +86,8 @@ public: }; const Consensus::Params& GetConsensus() const { return consensus; } - const CMessageHeader::MessageStartChars& MessageStart() const { return pchMessageStart; } + const MessageStartChars& MessageStart() const { return pchMessageStart; } uint16_t GetDefaultPort() const { return nDefaultPort; } - uint16_t GetDefaultPort(Network net) const - { - return net == NET_I2P ? I2P_SAM31_PORT : GetDefaultPort(); - } - uint16_t GetDefaultPort(const std::string& addr) const - { - CNetAddr a; - return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : GetDefaultPort(); - } const CBlock& GenesisBlock() const { return genesis; } /** Default value for -checkmempool and -checkblockindex argument */ @@ -165,7 +155,7 @@ protected: CChainParams() {} Consensus::Params consensus; - CMessageHeader::MessageStartChars pchMessageStart; + MessageStartChars pchMessageStart; uint16_t nDefaultPort; uint64_t nPruneAfterHeight; uint64_t m_assumed_blockchain_size; diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index 6be07da222..ff655c5ffa 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -41,7 +41,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active if (load_path.empty()) return false; FILE* filestr{opts.mockable_fopen_function(load_path, "rb")}; - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + CAutoFile file{filestr, CLIENT_VERSION}; if (file.IsNull()) { LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); return false; @@ -157,7 +157,7 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock return false; } - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + CAutoFile file{filestr, CLIENT_VERSION}; uint64_t version = MEMPOOL_DUMP_VERSION; file << version; diff --git a/src/kernel/messagestartchars.h b/src/kernel/messagestartchars.h new file mode 100644 index 0000000000..829616dc8b --- /dev/null +++ b/src/kernel/messagestartchars.h @@ -0,0 +1,13 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_KERNEL_MESSAGESTARTCHARS_H +#define BITCOIN_KERNEL_MESSAGESTARTCHARS_H + +#include <array> +#include <cstdint> + +using MessageStartChars = std::array<uint8_t, 4>; + +#endif // BITCOIN_KERNEL_MESSAGESTARTCHARS_H diff --git a/src/key_io.cpp b/src/key_io.cpp index a061165613..1a0b51a28b 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -8,6 +8,7 @@ #include <bech32.h> #include <script/interpreter.h> #include <script/solver.h> +#include <tinyformat.h> #include <util/strencodings.h> #include <algorithm> diff --git a/src/net.cpp b/src/net.cpp index af2855932d..ef1135bb8c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -35,6 +35,7 @@ #include <util/threadinterrupt.h> #include <util/trace.h> #include <util/translation.h> +#include <util/vector.h> #ifdef WIN32 #include <string.h> @@ -470,8 +471,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo Ticks<HoursDouble>(pszDest ? 0h : Now<NodeSeconds>() - addrConnect.nTime)); // Resolve - const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) : - Params().GetDefaultPort()}; + const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) : + m_params.GetDefaultPort()}; if (pszDest) { const std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; if (!resolved.empty()) { @@ -730,7 +731,7 @@ V1Transport::V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noex m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn) { assert(std::size(Params().MessageStart()) == std::size(m_magic_bytes)); - std::copy(std::begin(Params().MessageStart()), std::end(Params().MessageStart()), m_magic_bytes); + m_magic_bytes = Params().MessageStart(); LOCK(m_recv_mutex); Reset(); } @@ -759,7 +760,7 @@ int V1Transport::readHeader(Span<const uint8_t> msg_bytes) } // Check start string, network magic - if (memcmp(hdr.pchMessageStart, m_magic_bytes, CMessageHeader::MESSAGE_START_SIZE) != 0) { + if (hdr.pchMessageStart != m_magic_bytes) { LogPrint(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id); return -1; } @@ -899,8 +900,7 @@ void V1Transport::MarkBytesSent(size_t bytes_sent) noexcept m_bytes_sent = 0; } else if (!m_sending_header && m_bytes_sent == m_message_to_send.data.size()) { // We're done sending a message's data. Wipe the data vector to reduce memory consumption. - m_message_to_send.data.clear(); - m_message_to_send.data.shrink_to_fit(); + ClearShrink(m_message_to_send.data); m_bytes_sent = 0; } } @@ -1123,8 +1123,8 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept SetReceiveState(RecvState::V1); SetSendState(SendState::V1); // Reset v2 transport buffers to save memory. - m_recv_buffer = {}; - m_send_buffer = {}; + ClearShrink(m_recv_buffer); + ClearShrink(m_send_buffer); } else { // We have not received enough to distinguish v1 from v2 yet. Wait until more bytes come. } @@ -1144,7 +1144,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept // they receive our uniformly random key and garbage, but detecting this case specially // means we can log it. static constexpr std::array<uint8_t, 12> MATCH = {'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0}; - static constexpr size_t OFFSET = sizeof(CMessageHeader::MessageStartChars); + static constexpr size_t OFFSET = std::tuple_size_v<MessageStartChars>; if (!m_initiating && m_recv_buffer.size() >= OFFSET + MATCH.size()) { if (std::equal(MATCH.begin(), MATCH.end(), m_recv_buffer.begin() + OFFSET)) { LogPrint(BCLog::NET, "V2 transport error: V1 peer with wrong MessageStart %s\n", @@ -1184,8 +1184,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept /*ignore=*/false, /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION)); // We no longer need the garbage. - m_send_garbage.clear(); - m_send_garbage.shrink_to_fit(); + ClearShrink(m_send_garbage); // Construct version packet in the send buffer. m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size()); @@ -1275,7 +1274,7 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept // Ignore flag does not matter for garbage authentication. Any valid packet functions // as authentication. Receive and process the version packet next. SetReceiveState(RecvState::VERSION); - m_recv_garbage = {}; + ClearShrink(m_recv_garbage); break; case RecvState::VERSION: if (!ignore) { @@ -1295,9 +1294,9 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept Assume(false); } // Wipe the receive buffer where the next packet will be received into. - m_recv_buffer = {}; + ClearShrink(m_recv_buffer); // In all but APP_READY state, we can wipe the decoded contents. - if (m_recv_state != RecvState::APP_READY) m_recv_decode_buffer = {}; + if (m_recv_state != RecvState::APP_READY) ClearShrink(m_recv_decode_buffer); } else { // We either have less than 3 bytes, so we don't know the packet's length yet, or more // than 3 bytes but less than the packet's full ciphertext. Wait until those arrive. @@ -1511,7 +1510,7 @@ CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool LogPrint(BCLog::NET, "V2 transport error: invalid message type (%u bytes contents), peer=%d\n", m_recv_decode_buffer.size(), m_nodeid); reject_message = true; } - m_recv_decode_buffer = {}; + ClearShrink(m_recv_decode_buffer); SetReceiveState(RecvState::APP); return msg; @@ -1545,7 +1544,7 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept m_cipher.Encrypt(MakeByteSpan(contents), {}, false, MakeWritableByteSpan(m_send_buffer)); m_send_type = msg.m_type; // Release memory - msg.data = {}; + ClearShrink(msg.data); return true; } @@ -1577,7 +1576,7 @@ void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept // Wipe the buffer when everything is sent. if (m_send_pos == m_send_buffer.size()) { m_send_pos = 0; - m_send_buffer = {}; + ClearShrink(m_send_buffer); } } @@ -2184,7 +2183,7 @@ void CConnman::WakeMessageHandler() void CConnman::ThreadDNSAddressSeed() { FastRandomContext rng; - std::vector<std::string> seeds = Params().DNSSeeds(); + std::vector<std::string> seeds = m_params.DNSSeeds(); Shuffle(seeds.begin(), seeds.end(), rng); int seeds_right_now = 0; // Number of seeds left before testing if we have enough connections int found = 0; @@ -2275,7 +2274,7 @@ void CConnman::ThreadDNSAddressSeed() const auto addresses{LookupHost(host, nMaxIPs, true)}; if (!addresses.empty()) { for (const CNetAddr& ip : addresses) { - CAddress addr = CAddress(CService(ip, Params().GetDefaultPort()), requiredServiceBits); + CAddress addr = CAddress(CService(ip, m_params.GetDefaultPort()), requiredServiceBits); addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - 3 * 24h, -4 * 24h); // use a random age between 3 and 7 days old vAdd.push_back(addr); found++; @@ -2480,7 +2479,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) } if (add_fixed_seeds_now) { - std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())}; + std::vector<CAddress> seed_addrs{ConvertSeeds(m_params.FixedSeeds())}; // We will not make outgoing connections to peers that are unreachable // (e.g. because of -onlynet configuration). // Therefore, we do not add them to addrman in the first place. @@ -2769,7 +2768,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() const } for (const std::string& strAddNode : lAddresses) { - CService service(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode))); + CService service(LookupNumeric(strAddNode, GetDefaultPort(strAddNode))); AddedNodeInfo addedNode{strAddNode, CService(), false, false}; if (service.IsValid()) { // strAddNode is an IP:port @@ -3075,11 +3074,12 @@ void CConnman::SetNetworkActive(bool active) } CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, - const NetGroupManager& netgroupman, bool network_active) + const NetGroupManager& netgroupman, const CChainParams& params, bool network_active) : addrman(addrman_in) , m_netgroupman{netgroupman} , nSeed0(nSeed0In) , nSeed1(nSeed1In) + , m_params(params) { SetTryNewOutboundPeer(false); @@ -3093,6 +3093,16 @@ NodeId CConnman::GetNewNodeId() return nLastNodeId.fetch_add(1, std::memory_order_relaxed); } +uint16_t CConnman::GetDefaultPort(Network net) const +{ + return net == NET_I2P ? I2P_SAM31_PORT : m_params.GetDefaultPort(); +} + +uint16_t CConnman::GetDefaultPort(const std::string& addr) const +{ + CNetAddr a; + return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : m_params.GetDefaultPort(); +} bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlags permissions) { @@ -10,15 +10,16 @@ #include <chainparams.h> #include <common/bloom.h> #include <compat/compat.h> -#include <node/connection_types.h> #include <consensus/amount.h> #include <crypto/siphash.h> #include <hash.h> #include <i2p.h> +#include <kernel/messagestartchars.h> #include <net_permissions.h> #include <netaddress.h> #include <netbase.h> #include <netgroup.h> +#include <node/connection_types.h> #include <policy/feerate.h> #include <protocol.h> #include <random.h> @@ -46,6 +47,7 @@ class AddrMan; class BanMan; +class CChainParams; class CNode; class CScheduler; struct bilingual_str; @@ -360,7 +362,7 @@ public: class V1Transport final : public Transport { private: - CMessageHeader::MessageStartChars m_magic_bytes; + MessageStartChars m_magic_bytes; const NodeId m_node_id; // Only for logging mutable Mutex m_recv_mutex; //!< Lock for receive state mutable CHash256 hasher GUARDED_BY(m_recv_mutex); @@ -1080,7 +1082,7 @@ public: } CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman, - bool network_active = true); + const CChainParams& params, bool network_active = true); ~CConnman(); @@ -1356,6 +1358,9 @@ private: // Whether the node should be passed out in ForEach* callbacks static bool NodeFullyConnected(const CNode* pnode); + uint16_t GetDefaultPort(Network net) const; + uint16_t GetDefaultPort(const std::string& addr) const; + // Network usage totals mutable Mutex m_total_bytes_sent_mutex; std::atomic<uint64_t> nTotalBytesRecv{0}; @@ -1565,6 +1570,8 @@ private: std::vector<CNode*> m_nodes_copy; }; + const CChainParams& m_params; + friend struct ConnmanTestMsg; }; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 70f11be586..f21c94c0a0 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -11,6 +11,7 @@ #include <flatfile.h> #include <hash.h> #include <kernel/chainparams.h> +#include <kernel/messagestartchars.h> #include <logging.h> #include <pow.h> #include <reverse_iterator.h> @@ -21,6 +22,7 @@ #include <util/batchpriority.h> #include <util/fs.h> #include <util/signalinterrupt.h> +#include <util/strencodings.h> #include <util/translation.h> #include <validation.h> @@ -822,7 +824,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const { // Open history file to append - CAutoFile fileout(OpenBlockFile(pos), SER_DISK, CLIENT_VERSION); + CAutoFile fileout{OpenBlockFile(pos), CLIENT_VERSION}; if (fileout.IsNull()) { return error("WriteBlockToDisk: OpenBlockFile failed"); } @@ -878,7 +880,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons block.SetNull(); // Open history file to read - CAutoFile filein(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION); + CAutoFile filein{OpenBlockFile(pos, true), CLIENT_VERSION}; if (filein.IsNull()) { return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString()); } @@ -927,12 +929,12 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF } try { - CMessageHeader::MessageStartChars blk_start; + MessageStartChars blk_start; unsigned int blk_size; filein >> blk_start >> blk_size; - if (memcmp(blk_start, GetParams().MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) { + if (blk_start != GetParams().MessageStart()) { return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart())); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index c79fd2c6a1..b251ece31f 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -11,7 +11,7 @@ #include <kernel/blockmanager_opts.h> #include <kernel/chainparams.h> #include <kernel/cs_main.h> -#include <protocol.h> +#include <kernel/messagestartchars.h> #include <sync.h> #include <util/fs.h> #include <util/hasher.h> @@ -73,7 +73,7 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB /** Size of header written by WriteBlockToDisk before a serialized CBlock */ -static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int); +static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v<MessageStartChars> + sizeof(unsigned int); extern std::atomic_bool fReindex; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index a6d84555c0..e0c40036d9 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -28,6 +28,7 @@ #include <node/coin.h> #include <node/context.h> #include <node/interface_ui.h> +#include <node/mini_miner.h> #include <node/transaction.h> #include <policy/feerate.h> #include <policy/fees.h> @@ -665,6 +666,26 @@ public: if (!m_node.mempool) return; m_node.mempool->GetTransactionAncestry(txid, ancestors, descendants, ancestorsize, ancestorfees); } + + std::map<COutPoint, CAmount> CalculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override + { + if (!m_node.mempool) { + std::map<COutPoint, CAmount> bump_fees; + for (const auto& outpoint : outpoints) { + bump_fees.emplace(std::make_pair(outpoint, 0)); + } + return bump_fees; + } + return MiniMiner(*m_node.mempool, outpoints).CalculateBumpFees(target_feerate); + } + + std::optional<CAmount> CalculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override + { + if (!m_node.mempool) { + return 0; + } + return MiniMiner(*m_node.mempool, outpoints).CalculateTotalBumpFees(target_feerate); + } void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) override { const CTxMemPool::Limits default_limits{}; diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index 6f253eddfa..2827242f96 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -7,9 +7,7 @@ #include <consensus/amount.h> #include <policy/feerate.h> #include <primitives/transaction.h> -#include <timedata.h> #include <util/check.h> -#include <util/moneystr.h> #include <algorithm> #include <numeric> @@ -171,9 +169,8 @@ void MiniMiner::DeleteAncestorPackage(const std::set<MockEntryMap::iterator, Ite for (auto& descendant : it->second) { // If these fail, we must be double-deducting. Assume(descendant->second.GetModFeesWithAncestors() >= anc->second.GetModifiedFee()); - Assume(descendant->second.vsize_with_ancestors >= anc->second.GetTxSize()); - descendant->second.fee_with_ancestors -= anc->second.GetModifiedFee(); - descendant->second.vsize_with_ancestors -= anc->second.GetTxSize(); + Assume(descendant->second.GetSizeWithAncestors() >= anc->second.GetTxSize()); + descendant->second.UpdateAncestorState(-anc->second.GetTxSize(), -anc->second.GetModifiedFee()); } } // Delete these entries. diff --git a/src/node/mini_miner.h b/src/node/mini_miner.h index db07e6d1bf..9d9d66bf0b 100644 --- a/src/node/mini_miner.h +++ b/src/node/mini_miner.h @@ -19,12 +19,13 @@ class MiniMinerMempoolEntry const CAmount fee_individual; const CTransactionRef tx; const int64_t vsize_individual; + CAmount fee_with_ancestors; + int64_t vsize_with_ancestors; // This class must be constructed while holding mempool.cs. After construction, the object's // methods can be called without holding that lock. + public: - CAmount fee_with_ancestors; - int64_t vsize_with_ancestors; explicit MiniMinerMempoolEntry(CTxMemPool::txiter entry) : fee_individual{entry->GetModifiedFee()}, tx{entry->GetSharedTx()}, @@ -38,6 +39,10 @@ public: int64_t GetTxSize() const { return vsize_individual; } int64_t GetSizeWithAncestors() const { return vsize_with_ancestors; } const CTransaction& GetTx() const LIFETIMEBOUND { return *tx; } + void UpdateAncestorState(int64_t vsize_change, CAmount fee_change) { + vsize_with_ancestors += vsize_change; + fee_with_ancestors += fee_change; + } }; // Comparator needed for std::set<MockEntryMap::iterator> diff --git a/src/protocol.cpp b/src/protocol.cpp index 2105480c72..cb956191e4 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -92,7 +92,7 @@ const static std::vector<std::string> g_all_net_message_types{ CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn) { - memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE); + pchMessageStart = pchMessageStartIn; // Copy the command name size_t i = 0; diff --git a/src/protocol.h b/src/protocol.h index e1caba766f..56668898e4 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_PROTOCOL_H #define BITCOIN_PROTOCOL_H +#include <kernel/messagestartchars.h> // IWYU pragma: export #include <netaddress.h> #include <primitives/transaction.h> #include <serialize.h> @@ -13,6 +14,7 @@ #include <uint256.h> #include <util/time.h> +#include <array> #include <cstdint> #include <limits> #include <string> @@ -26,14 +28,12 @@ class CMessageHeader { public: - static constexpr size_t MESSAGE_START_SIZE = 4; static constexpr size_t COMMAND_SIZE = 12; static constexpr size_t MESSAGE_SIZE_SIZE = 4; static constexpr size_t CHECKSUM_SIZE = 4; - static constexpr size_t MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE; + static constexpr size_t MESSAGE_SIZE_OFFSET = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE; static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE; - static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE; - typedef unsigned char MessageStartChars[MESSAGE_START_SIZE]; + static constexpr size_t HEADER_SIZE = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE; explicit CMessageHeader() = default; @@ -47,7 +47,7 @@ public: SERIALIZE_METHODS(CMessageHeader, obj) { READWRITE(obj.pchMessageStart, obj.pchCommand, obj.nMessageSize, obj.pchChecksum); } - char pchMessageStart[MESSAGE_START_SIZE]{}; + MessageStartChars pchMessageStart{}; char pchCommand[COMMAND_SIZE]{}; uint32_t nMessageSize{std::numeric_limits<uint32_t>::max()}; uint8_t pchChecksum[CHECKSUM_SIZE]{}; diff --git a/src/random.cpp b/src/random.cpp index 51b8b3ad9d..9bd8ff9f1a 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -5,6 +5,7 @@ #include <random.h> +#include <compat/compat.h> #include <compat/cpuid.h> #include <crypto/chacha20.h> #include <crypto/sha256.h> diff --git a/src/randomenv.cpp b/src/randomenv.cpp index 581612bccf..da81a61651 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -10,6 +10,7 @@ #include <randomenv.h> #include <clientversion.h> +#include <compat/compat.h> #include <compat/cpuid.h> #include <crypto/sha512.h> #include <support/cleanse.h> diff --git a/src/serialize.h b/src/serialize.h index 04b29d1ded..1ad8ac4373 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -285,6 +285,7 @@ template<typename Stream> inline void Serialize(Stream& s, int64_t a ) { ser_wri template<typename Stream> inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } template<typename Stream, int N> inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); } template<typename Stream, int N> inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); } +template <typename Stream, typename B, std::size_t N> void Serialize(Stream& s, const std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.write(MakeByteSpan(a)); } template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); } #ifndef CHAR_EQUALS_INT8 @@ -301,6 +302,7 @@ template<typename Stream> inline void Unserialize(Stream& s, int64_t& a ) { a = template<typename Stream> inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } template<typename Stream, int N> inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } template<typename Stream, int N> inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } +template <typename Stream, typename B, std::size_t N> void Unserialize(Stream& s, std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.read(MakeWritableByteSpan(a)); } template <typename Stream, typename B> void Unserialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.read(AsWritableBytes(span)); } template <typename Stream> inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); } diff --git a/src/streams.h b/src/streams.h index 5ff952be76..f9a817c9b6 100644 --- a/src/streams.h +++ b/src/streams.h @@ -550,12 +550,10 @@ public: class CAutoFile : public AutoFile { private: - const int nType; const int nVersion; public: - explicit CAutoFile(std::FILE* file, int type, int version, std::vector<std::byte> data_xor = {}) : AutoFile{file, std::move(data_xor)}, nType{type}, nVersion{version} {} - int GetType() const { return nType; } + explicit CAutoFile(std::FILE* file, int version, std::vector<std::byte> data_xor = {}) : AutoFile{file, std::move(data_xor)}, nVersion{version} {} int GetVersion() const { return nVersion; } template<typename T> @@ -579,10 +577,9 @@ public: * Will automatically close the file when it goes out of scope if not null. * If you need to close the file early, use file.fclose() instead of fclose(file). */ -class CBufferedFile +class BufferedFile { private: - const int nType; const int nVersion; FILE *src; //!< source file @@ -603,7 +600,7 @@ private: return false; size_t nBytes = fread((void*)&vchBuf[pos], 1, readNow, src); if (nBytes == 0) { - throw std::ios_base::failure(feof(src) ? "CBufferedFile::Fill: end of file" : "CBufferedFile::Fill: fread failed"); + throw std::ios_base::failure(feof(src) ? "BufferedFile::Fill: end of file" : "BufferedFile::Fill: fread failed"); } nSrcPos += nBytes; return true; @@ -632,25 +629,24 @@ private: } public: - CBufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) - : nType(nTypeIn), nVersion(nVersionIn), nReadLimit(std::numeric_limits<uint64_t>::max()), nRewind(nRewindIn), vchBuf(nBufSize, std::byte{0}) + BufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nVersionIn) + : nVersion{nVersionIn}, nReadLimit{std::numeric_limits<uint64_t>::max()}, nRewind{nRewindIn}, vchBuf(nBufSize, std::byte{0}) { if (nRewindIn >= nBufSize) throw std::ios_base::failure("Rewind limit must be less than buffer size"); src = fileIn; } - ~CBufferedFile() + ~BufferedFile() { fclose(); } // Disallow copies - CBufferedFile(const CBufferedFile&) = delete; - CBufferedFile& operator=(const CBufferedFile&) = delete; + BufferedFile(const BufferedFile&) = delete; + BufferedFile& operator=(const BufferedFile&) = delete; int GetVersion() const { return nVersion; } - int GetType() const { return nType; } void fclose() { @@ -715,7 +711,7 @@ public: } template<typename T> - CBufferedFile& operator>>(T&& obj) { + BufferedFile& operator>>(T&& obj) { ::Unserialize(*this, obj); return (*this); } diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 13cb1cc314..636f5b9388 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -8,10 +8,12 @@ #include <node/context.h> #include <node/kernel_notifications.h> #include <script/solver.h> +#include <primitives/block.h> #include <util/chaintype.h> #include <validation.h> #include <boost/test/unit_test.hpp> +#include <test/util/logging.h> #include <test/util/setup_common.h> using node::BLOCK_SERIALIZATION_HEADER_SIZE; @@ -130,4 +132,73 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) BOOST_CHECK(!blockman.CheckBlockDataAvailability(tip, *last_pruned_block)); } +BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) +{ + KernelNotifications notifications{m_node.exit_status}; + node::BlockManager::Options blockman_opts{ + .chainparams = Params(), + .blocks_dir = m_args.GetBlocksDirPath(), + .notifications = notifications, + }; + BlockManager blockman{m_node.kernel->interrupt, blockman_opts}; + + // Test blocks with no transactions, not even a coinbase + CBlock block1; + block1.nVersion = 1; + CBlock block2; + block2.nVersion = 2; + CBlock block3; + block3.nVersion = 3; + + // They are 80 bytes header + 1 byte 0x00 for vtx length + constexpr int TEST_BLOCK_SIZE{81}; + + // Blockstore is empty + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), 0); + + // Write the first block; dbp=nullptr means this block doesn't already have a disk + // location, so allocate a free location and write it there. + FlatFilePos pos1{blockman.SaveBlockToDisk(block1, /*nHeight=*/1, /*dbp=*/nullptr)}; + + // Write second block + FlatFilePos pos2{blockman.SaveBlockToDisk(block2, /*nHeight=*/2, /*dbp=*/nullptr)}; + + // Two blocks in the file + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); + + // First two blocks are written as expected + // Errors are expected because block data is junk, thrown AFTER successful read + CBlock read_block; + BOOST_CHECK_EQUAL(read_block.nVersion, 0); + { + ASSERT_DEBUG_LOG("ReadBlockFromDisk: Errors in block header"); + BOOST_CHECK(!blockman.ReadBlockFromDisk(read_block, pos1)); + BOOST_CHECK_EQUAL(read_block.nVersion, 1); + } + { + ASSERT_DEBUG_LOG("ReadBlockFromDisk: Errors in block header"); + BOOST_CHECK(!blockman.ReadBlockFromDisk(read_block, pos2)); + BOOST_CHECK_EQUAL(read_block.nVersion, 2); + } + + // When FlatFilePos* dbp is given, SaveBlockToDisk() will not write or + // overwrite anything to the flat file block storage. It will, however, + // update the blockfile metadata. This is to facilitate reindexing + // when the user has the blocks on disk but the metadata is being rebuilt. + // Verify this behavior by attempting (and failing) to write block 3 data + // to block 2 location. + CBlockFileInfo* block_data = blockman.GetBlockFileInfo(0); + BOOST_CHECK_EQUAL(block_data->nBlocks, 2); + BOOST_CHECK(blockman.SaveBlockToDisk(block3, /*nHeight=*/3, /*dbp=*/&pos2) == pos2); + // Metadata is updated... + BOOST_CHECK_EQUAL(block_data->nBlocks, 3); + // ...but there are still only two blocks in the file + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); + + // Block 2 was not overwritten: + // SaveBlockToDisk() did not call WriteBlockToDisk() because `FlatFilePos* dbp` was non-null + blockman.ReadBlockFromDisk(read_block, pos2); + BOOST_CHECK_EQUAL(read_block.nVersion, 2); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 8c1182b5e1..6e740a4f53 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -142,7 +142,7 @@ static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerM BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { NodeId id{0}; - auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS; @@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(block_relay_only_eviction) { NodeId id{0}; - auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); constexpr int max_outbound_block_relay{MAX_BLOCK_RELAY_ONLY_CONNECTIONS}; @@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) LOCK(NetEventsInterface::g_msgproc_mutex); auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {}); CNetAddr tor_netaddr; @@ -407,7 +407,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) LOCK(NetEventsInterface::g_msgproc_mutex); auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {}); banman->ClearBanned(); diff --git a/src/test/fuzz/buffered_file.cpp b/src/test/fuzz/buffered_file.cpp index 2f7ce60c7f..1116274e3d 100644 --- a/src/test/fuzz/buffered_file.cpp +++ b/src/test/fuzz/buffered_file.cpp @@ -18,10 +18,10 @@ FUZZ_TARGET(buffered_file) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; FuzzedFileProvider fuzzed_file_provider = ConsumeFile(fuzzed_data_provider); - std::optional<CBufferedFile> opt_buffered_file; + std::optional<BufferedFile> opt_buffered_file; FILE* fuzzed_file = fuzzed_file_provider.open(); try { - opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegral<int>(), fuzzed_data_provider.ConsumeIntegral<int>()); + opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegral<int>()); } catch (const std::ios_base::failure&) { if (fuzzed_file != nullptr) { fclose(fuzzed_file); @@ -62,7 +62,6 @@ FUZZ_TARGET(buffered_file) }); } opt_buffered_file->GetPos(); - opt_buffered_file->GetType(); opt_buffered_file->GetVersion(); } } diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index cdf240dc59..e46e085ee7 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -36,6 +36,7 @@ FUZZ_TARGET(connman, .init = initialize_connman) fuzzed_data_provider.ConsumeIntegral<uint64_t>(), *g_setup->m_node.addrman, *g_setup->m_node.netgroupman, + Params(), fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index 26c219d6c8..57129a60b8 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -8,6 +8,7 @@ #include <script/descriptor.h> #include <test/fuzz/fuzz.h> #include <util/chaintype.h> +#include <util/strencodings.h> //! Types are raw (un)compressed pubkeys, raw xonly pubkeys, raw privkeys (WIF), xpubs, xprvs. static constexpr uint8_t KEY_TYPES_COUNT{6}; diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index c20cbde05f..32bd00ec03 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -29,6 +29,10 @@ #include <utility> #include <vector> +#ifdef __AFL_FUZZ_INIT +__AFL_FUZZ_INIT(); +#endif + const std::function<void(const std::string&)> G_TEST_LOG_FUN{}; /** @@ -188,21 +192,13 @@ int main(int argc, char** argv) { initialize(); static const auto& test_one_input = *Assert(g_test_one_input); -#ifdef __AFL_INIT - // Enable AFL deferred forkserver mode. Requires compilation using - // afl-clang-fast++. See fuzzing.md for details. - __AFL_INIT(); -#endif - #ifdef __AFL_LOOP // Enable AFL persistent mode. Requires compilation using afl-clang-fast++. // See fuzzing.md for details. - while (__AFL_LOOP(1000)) { - std::vector<uint8_t> buffer; - if (!read_stdin(buffer)) { - continue; - } - test_one_input(buffer); + const uint8_t* buffer = __AFL_FUZZ_TESTCASE_BUF; + while (__AFL_LOOP(100000)) { + size_t buffer_len = __AFL_FUZZ_TESTCASE_LEN; + test_one_input({buffer, buffer_len}); } #else std::vector<uint8_t> buffer; diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index da724f8d7b..f65356936b 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -77,66 +77,66 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) const CAmount normal_fee{CENT/200}; const CAmount high_fee{CENT/10}; - // Create a parent tx1 and child tx2 with normal fees: - const auto tx1 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); + // Create a parent tx0 and child tx1 with normal fees: + const auto tx0 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx0)); + const auto tx1 = make_tx({COutPoint{tx0->GetHash(), 0}}, /*num_outputs=*/1); pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx1)); - const auto tx2 = make_tx({COutPoint{tx1->GetHash(), 0}}, /*num_outputs=*/1); - pool.addUnchecked(entry.Fee(normal_fee).FromTx(tx2)); - // Create a low-feerate parent tx3 and high-feerate child tx4 (cpfp) - const auto tx3 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(low_fee).FromTx(tx3)); - const auto tx4 = make_tx({COutPoint{tx3->GetHash(), 0}}, /*num_outputs=*/1); - pool.addUnchecked(entry.Fee(high_fee).FromTx(tx4)); + // Create a low-feerate parent tx2 and high-feerate child tx3 (cpfp) + const auto tx2 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx2)); + const auto tx3 = make_tx({COutPoint{tx2->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx3)); - // Create a parent tx5 and child tx6 where both have low fees - const auto tx5 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); + // Create a parent tx4 and child tx5 where both have low fees + const auto tx4 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx4)); + const auto tx5 = make_tx({COutPoint{tx4->GetHash(), 0}}, /*num_outputs=*/1); pool.addUnchecked(entry.Fee(low_fee).FromTx(tx5)); - const auto tx6 = make_tx({COutPoint{tx5->GetHash(), 0}}, /*num_outputs=*/1); - pool.addUnchecked(entry.Fee(low_fee).FromTx(tx6)); - // Make tx6's modified fee much higher than its base fee. This should cause it to pass + // Make tx5's modified fee much higher than its base fee. This should cause it to pass // the fee-related checks despite being low-feerate. - pool.PrioritiseTransaction(tx6->GetHash(), CENT/100); + pool.PrioritiseTransaction(tx5->GetHash(), CENT/100); - // Create a high-feerate parent tx7, low-feerate child tx8 - const auto tx7 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(high_fee).FromTx(tx7)); - const auto tx8 = make_tx({COutPoint{tx7->GetHash(), 0}}, /*num_outputs=*/1); - pool.addUnchecked(entry.Fee(low_fee).FromTx(tx8)); + // Create a high-feerate parent tx6, low-feerate child tx7 + const auto tx6 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx6)); + const auto tx7 = make_tx({COutPoint{tx6->GetHash(), 0}}, /*num_outputs=*/1); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx7)); std::vector<COutPoint> all_unspent_outpoints({ - COutPoint{tx1->GetHash(), 1}, - COutPoint{tx2->GetHash(), 0}, - COutPoint{tx3->GetHash(), 1}, - COutPoint{tx4->GetHash(), 0}, - COutPoint{tx5->GetHash(), 1}, - COutPoint{tx6->GetHash(), 0}, - COutPoint{tx7->GetHash(), 1}, - COutPoint{tx8->GetHash(), 0} - }); - for (const auto& outpoint : all_unspent_outpoints) BOOST_CHECK(!pool.isSpent(outpoint)); - - std::vector<COutPoint> all_spent_outpoints({ + COutPoint{tx0->GetHash(), 1}, COutPoint{tx1->GetHash(), 0}, + COutPoint{tx2->GetHash(), 1}, COutPoint{tx3->GetHash(), 0}, + COutPoint{tx4->GetHash(), 1}, COutPoint{tx5->GetHash(), 0}, + COutPoint{tx6->GetHash(), 1}, COutPoint{tx7->GetHash(), 0} }); + for (const auto& outpoint : all_unspent_outpoints) BOOST_CHECK(!pool.isSpent(outpoint)); + + std::vector<COutPoint> all_spent_outpoints({ + COutPoint{tx0->GetHash(), 0}, + COutPoint{tx2->GetHash(), 0}, + COutPoint{tx4->GetHash(), 0}, + COutPoint{tx6->GetHash(), 0} + }); for (const auto& outpoint : all_spent_outpoints) BOOST_CHECK(pool.GetConflictTx(outpoint) != nullptr); std::vector<COutPoint> all_parent_outputs({ - COutPoint{tx1->GetHash(), 0}, - COutPoint{tx1->GetHash(), 1}, - COutPoint{tx3->GetHash(), 0}, - COutPoint{tx3->GetHash(), 1}, - COutPoint{tx5->GetHash(), 0}, - COutPoint{tx5->GetHash(), 1}, - COutPoint{tx7->GetHash(), 0}, - COutPoint{tx7->GetHash(), 1} + COutPoint{tx0->GetHash(), 0}, + COutPoint{tx0->GetHash(), 1}, + COutPoint{tx2->GetHash(), 0}, + COutPoint{tx2->GetHash(), 1}, + COutPoint{tx4->GetHash(), 0}, + COutPoint{tx4->GetHash(), 1}, + COutPoint{tx6->GetHash(), 0}, + COutPoint{tx6->GetHash(), 1} }); - std::vector<CTransactionRef> all_transactions{tx1, tx2, tx3, tx4, tx5, tx6, tx7, tx8}; + std::vector<CTransactionRef> all_transactions{tx0, tx1, tx2, tx3, tx4, tx5, tx6, tx7}; struct TxDimensions { int32_t vsize; CAmount mod_fee; CFeeRate feerate; }; @@ -178,47 +178,47 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) BOOST_CHECK(sanity_check(all_transactions, bump_fees)); BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); - // Check tx1 bumpfee: no other bumper. - const TxDimensions& tx1_dimensions = tx_dims.find(tx1->GetHash())->second; - CAmount bumpfee1 = Find(bump_fees, COutPoint{tx1->GetHash(), 1}); - if (target_feerate <= tx1_dimensions.feerate) { - BOOST_CHECK_EQUAL(bumpfee1, 0); + // Check tx0 bumpfee: no other bumper. + const TxDimensions& tx0_dimensions = tx_dims.find(tx0->GetHash())->second; + CAmount bumpfee0 = Find(bump_fees, COutPoint{tx0->GetHash(), 1}); + if (target_feerate <= tx0_dimensions.feerate) { + BOOST_CHECK_EQUAL(bumpfee0, 0); } else { - // Difference is fee to bump tx1 from current to target feerate. - BOOST_CHECK_EQUAL(bumpfee1, target_feerate.GetFee(tx1_dimensions.vsize) - tx1_dimensions.mod_fee); + // Difference is fee to bump tx0 from current to target feerate. + BOOST_CHECK_EQUAL(bumpfee0, target_feerate.GetFee(tx0_dimensions.vsize) - tx0_dimensions.mod_fee); } - // Check tx3 bumpfee: assisted by tx4. + // Check tx2 bumpfee: assisted by tx3. + const TxDimensions& tx2_dimensions = tx_dims.find(tx2->GetHash())->second; const TxDimensions& tx3_dimensions = tx_dims.find(tx3->GetHash())->second; - const TxDimensions& tx4_dimensions = tx_dims.find(tx4->GetHash())->second; - const CFeeRate tx3_feerate = CFeeRate(tx3_dimensions.mod_fee + tx4_dimensions.mod_fee, tx3_dimensions.vsize + tx4_dimensions.vsize); - CAmount bumpfee3 = Find(bump_fees, COutPoint{tx3->GetHash(), 1}); - if (target_feerate <= tx3_feerate) { - // As long as target feerate is below tx4's ancestor feerate, there is no bump fee. - BOOST_CHECK_EQUAL(bumpfee3, 0); + const CFeeRate tx2_feerate = CFeeRate(tx2_dimensions.mod_fee + tx3_dimensions.mod_fee, tx2_dimensions.vsize + tx3_dimensions.vsize); + CAmount bumpfee2 = Find(bump_fees, COutPoint{tx2->GetHash(), 1}); + if (target_feerate <= tx2_feerate) { + // As long as target feerate is below tx3's ancestor feerate, there is no bump fee. + BOOST_CHECK_EQUAL(bumpfee2, 0); } else { - // Difference is fee to bump tx3 from current to target feerate, without tx4. - BOOST_CHECK_EQUAL(bumpfee3, target_feerate.GetFee(tx3_dimensions.vsize) - tx3_dimensions.mod_fee); + // Difference is fee to bump tx2 from current to target feerate, without tx3. + BOOST_CHECK_EQUAL(bumpfee2, target_feerate.GetFee(tx2_dimensions.vsize) - tx2_dimensions.mod_fee); } - // If tx6’s modified fees are sufficient for tx5 and tx6 to be picked + // If tx5’s modified fees are sufficient for tx4 and tx5 to be picked // into the block, our prospective new transaction would not need to - // bump tx5 when using tx5’s second output. If however even tx6’s + // bump tx4 when using tx4’s second output. If however even tx5’s // modified fee (which essentially indicates "effective feerate") is - // not sufficient to bump tx5, using the second output of tx5 would - // require our transaction to bump tx5 from scratch since we evaluate + // not sufficient to bump tx4, using the second output of tx4 would + // require our transaction to bump tx4 from scratch since we evaluate // transaction packages per ancestor sets and do not consider multiple // children’s fees. + const TxDimensions& tx4_dimensions = tx_dims.find(tx4->GetHash())->second; const TxDimensions& tx5_dimensions = tx_dims.find(tx5->GetHash())->second; - const TxDimensions& tx6_dimensions = tx_dims.find(tx6->GetHash())->second; - const CFeeRate tx5_feerate = CFeeRate(tx5_dimensions.mod_fee + tx6_dimensions.mod_fee, tx5_dimensions.vsize + tx6_dimensions.vsize); - CAmount bumpfee5 = Find(bump_fees, COutPoint{tx5->GetHash(), 1}); - if (target_feerate <= tx5_feerate) { - // As long as target feerate is below tx6's ancestor feerate, there is no bump fee. - BOOST_CHECK_EQUAL(bumpfee5, 0); + const CFeeRate tx4_feerate = CFeeRate(tx4_dimensions.mod_fee + tx5_dimensions.mod_fee, tx4_dimensions.vsize + tx5_dimensions.vsize); + CAmount bumpfee4 = Find(bump_fees, COutPoint{tx4->GetHash(), 1}); + if (target_feerate <= tx4_feerate) { + // As long as target feerate is below tx5's ancestor feerate, there is no bump fee. + BOOST_CHECK_EQUAL(bumpfee4, 0); } else { - // Difference is fee to bump tx5 from current to target feerate, without tx6. - BOOST_CHECK_EQUAL(bumpfee5, target_feerate.GetFee(tx5_dimensions.vsize) - tx5_dimensions.mod_fee); + // Difference is fee to bump tx4 from current to target feerate, without tx5. + BOOST_CHECK_EQUAL(bumpfee4, target_feerate.GetFee(tx4_dimensions.vsize) - tx4_dimensions.mod_fee); } } // Spent outpoints should usually not be requested as they would not be @@ -240,36 +240,36 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) // even though only one of them is in a to-be-replaced transaction. BOOST_CHECK(sanity_check(all_transactions, bump_fees)); - // Check tx1 bumpfee: no other bumper. - const TxDimensions& tx1_dimensions = tx_dims.find(tx1->GetHash())->second; - CAmount it1_spent = Find(bump_fees, COutPoint{tx1->GetHash(), 0}); - if (target_feerate <= tx1_dimensions.feerate) { - BOOST_CHECK_EQUAL(it1_spent, 0); + // Check tx0 bumpfee: no other bumper. + const TxDimensions& tx0_dimensions = tx_dims.find(tx0->GetHash())->second; + CAmount it0_spent = Find(bump_fees, COutPoint{tx0->GetHash(), 0}); + if (target_feerate <= tx0_dimensions.feerate) { + BOOST_CHECK_EQUAL(it0_spent, 0); } else { - // Difference is fee to bump tx1 from current to target feerate. - BOOST_CHECK_EQUAL(it1_spent, target_feerate.GetFee(tx1_dimensions.vsize) - tx1_dimensions.mod_fee); + // Difference is fee to bump tx0 from current to target feerate. + BOOST_CHECK_EQUAL(it0_spent, target_feerate.GetFee(tx0_dimensions.vsize) - tx0_dimensions.mod_fee); } - // Check tx3 bumpfee: no other bumper, because tx4 is to-be-replaced. - const TxDimensions& tx3_dimensions = tx_dims.find(tx3->GetHash())->second; - const CFeeRate tx3_feerate_unbumped = tx3_dimensions.feerate; - auto it3_spent = Find(bump_fees, COutPoint{tx3->GetHash(), 0}); - if (target_feerate <= tx3_feerate_unbumped) { - BOOST_CHECK_EQUAL(it3_spent, 0); + // Check tx2 bumpfee: no other bumper, because tx3 is to-be-replaced. + const TxDimensions& tx2_dimensions = tx_dims.find(tx2->GetHash())->second; + const CFeeRate tx2_feerate_unbumped = tx2_dimensions.feerate; + auto it2_spent = Find(bump_fees, COutPoint{tx2->GetHash(), 0}); + if (target_feerate <= tx2_feerate_unbumped) { + BOOST_CHECK_EQUAL(it2_spent, 0); } else { - // Difference is fee to bump tx3 from current to target feerate, without tx4. - BOOST_CHECK_EQUAL(it3_spent, target_feerate.GetFee(tx3_dimensions.vsize) - tx3_dimensions.mod_fee); + // Difference is fee to bump tx2 from current to target feerate, without tx3. + BOOST_CHECK_EQUAL(it2_spent, target_feerate.GetFee(tx2_dimensions.vsize) - tx2_dimensions.mod_fee); } - // Check tx5 bumpfee: no other bumper, because tx6 is to-be-replaced. - const TxDimensions& tx5_dimensions = tx_dims.find(tx5->GetHash())->second; - const CFeeRate tx5_feerate_unbumped = tx5_dimensions.feerate; - auto it5_spent = Find(bump_fees, COutPoint{tx5->GetHash(), 0}); - if (target_feerate <= tx5_feerate_unbumped) { - BOOST_CHECK_EQUAL(it5_spent, 0); + // Check tx4 bumpfee: no other bumper, because tx5 is to-be-replaced. + const TxDimensions& tx4_dimensions = tx_dims.find(tx4->GetHash())->second; + const CFeeRate tx4_feerate_unbumped = tx4_dimensions.feerate; + auto it4_spent = Find(bump_fees, COutPoint{tx4->GetHash(), 0}); + if (target_feerate <= tx4_feerate_unbumped) { + BOOST_CHECK_EQUAL(it4_spent, 0); } else { - // Difference is fee to bump tx5 from current to target feerate, without tx6. - BOOST_CHECK_EQUAL(it5_spent, target_feerate.GetFee(tx5_dimensions.vsize) - tx5_dimensions.mod_fee); + // Difference is fee to bump tx4 from current to target feerate, without tx5. + BOOST_CHECK_EQUAL(it4_spent, target_feerate.GetFee(tx4_dimensions.vsize) - tx4_dimensions.mod_fee); } } } @@ -277,145 +277,178 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) { +/* Tx graph for `miniminer_overlap` unit test: + * + * coinbase_tx [mined] ... block-chain + * ------------------------------------------------- + * / | \ \ ... mempool + * / | \ | + * tx0 tx1 tx2 tx4 + * [low] [med] [high] [high] + * \ | / | + * \ | / tx5 + * \ | / [low] + * tx3 / \ + * [high] tx6 tx7 + * [med] [high] + * + * NOTE: + * -> "low"/"med"/"high" denote the _absolute_ fee of each tx + * -> tx3 has 3 inputs and 3 outputs, all other txs have 1 input and 2 outputs + * -> tx3's feerate is lower than tx2's, as tx3 has more weight (due to having more inputs and outputs) + * + * -> tx2_FR = high / tx2_vsize + * -> tx3_FR = high / tx3_vsize + * -> tx3_ASFR = (low+med+high+high) / (tx0_vsize + tx1_vsize + tx2_vsize + tx3_vsize) + * -> tx4_FR = high / tx4_vsize + * -> tx6_ASFR = (high+low+med) / (tx4_vsize + tx5_vsize + tx6_vsize) + * -> tx7_ASFR = (high+low+high) / (tx4_vsize + tx5_vsize + tx7_vsize) */ + CTxMemPool& pool = *Assert(m_node.mempool); LOCK2(::cs_main, pool.cs); TestMemPoolEntryHelper entry; - const CAmount low_fee{CENT/2000}; - const CAmount med_fee{CENT/200}; - const CAmount high_fee{CENT/10}; - - // Create 3 parents of different feerates, and 1 child spending from all 3. - const auto tx1 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(low_fee).FromTx(tx1)); - const auto tx2 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(med_fee).FromTx(tx2)); - const auto tx3 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); + const CAmount low_fee{CENT/2000}; // 500 ṩ + const CAmount med_fee{CENT/200}; // 5000 ṩ + const CAmount high_fee{CENT/10}; // 100_000 ṩ + + // Create 3 parents of different feerates, and 1 child spending outputs from all 3 parents. + const auto tx0 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx0)); + const auto tx1 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(med_fee).FromTx(tx1)); + const auto tx2 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx2)); + const auto tx3 = make_tx({COutPoint{tx0->GetHash(), 0}, COutPoint{tx1->GetHash(), 0}, COutPoint{tx2->GetHash(), 0}}, /*num_outputs=*/3); pool.addUnchecked(entry.Fee(high_fee).FromTx(tx3)); - const auto tx4 = make_tx({COutPoint{tx1->GetHash(), 0}, COutPoint{tx2->GetHash(), 0}, COutPoint{tx3->GetHash(), 0}}, /*num_outputs=*/3); - pool.addUnchecked(entry.Fee(high_fee).FromTx(tx4)); // Create 1 grandparent and 1 parent, then 2 children. - const auto tx5 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(high_fee).FromTx(tx5)); - const auto tx6 = make_tx({COutPoint{tx5->GetHash(), 0}}, /*num_outputs=*/3); - pool.addUnchecked(entry.Fee(low_fee).FromTx(tx6)); - const auto tx7 = make_tx({COutPoint{tx6->GetHash(), 0}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(med_fee).FromTx(tx7)); - const auto tx8 = make_tx({COutPoint{tx6->GetHash(), 1}}, /*num_outputs=*/2); - pool.addUnchecked(entry.Fee(high_fee).FromTx(tx8)); - - std::vector<CTransactionRef> all_transactions{tx1, tx2, tx3, tx4, tx5, tx6, tx7, tx8}; + const auto tx4 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx4)); + const auto tx5 = make_tx({COutPoint{tx4->GetHash(), 0}}, /*num_outputs=*/3); + pool.addUnchecked(entry.Fee(low_fee).FromTx(tx5)); + const auto tx6 = make_tx({COutPoint{tx5->GetHash(), 0}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(med_fee).FromTx(tx6)); + const auto tx7 = make_tx({COutPoint{tx5->GetHash(), 1}}, /*num_outputs=*/2); + pool.addUnchecked(entry.Fee(high_fee).FromTx(tx7)); + + std::vector<CTransactionRef> all_transactions{tx0, tx1, tx2, tx3, tx4, tx5, tx6, tx7}; std::vector<int64_t> tx_vsizes; tx_vsizes.reserve(all_transactions.size()); for (const auto& tx : all_transactions) tx_vsizes.push_back(GetVirtualTransactionSize(*tx)); std::vector<COutPoint> all_unspent_outpoints({ + COutPoint{tx0->GetHash(), 1}, COutPoint{tx1->GetHash(), 1}, COutPoint{tx2->GetHash(), 1}, + COutPoint{tx3->GetHash(), 0}, COutPoint{tx3->GetHash(), 1}, - COutPoint{tx4->GetHash(), 0}, + COutPoint{tx3->GetHash(), 2}, COutPoint{tx4->GetHash(), 1}, - COutPoint{tx4->GetHash(), 2}, - COutPoint{tx5->GetHash(), 1}, - COutPoint{tx6->GetHash(), 2}, - COutPoint{tx7->GetHash(), 0}, - COutPoint{tx8->GetHash(), 0} + COutPoint{tx5->GetHash(), 2}, + COutPoint{tx6->GetHash(), 0}, + COutPoint{tx7->GetHash(), 0} }); for (const auto& outpoint : all_unspent_outpoints) BOOST_CHECK(!pool.isSpent(outpoint)); - const auto tx3_feerate = CFeeRate(high_fee, tx_vsizes[2]); - const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[3]); - // tx4's feerate is lower than tx3's. same fee, different weight. - BOOST_CHECK(tx3_feerate > tx4_feerate); - const auto tx4_anc_feerate = CFeeRate(low_fee + med_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[3]); - const auto tx5_feerate = CFeeRate(high_fee, tx_vsizes[4]); - const auto tx7_anc_feerate = CFeeRate(low_fee + med_fee, tx_vsizes[5] + tx_vsizes[6]); - const auto tx8_anc_feerate = CFeeRate(low_fee + high_fee, tx_vsizes[5] + tx_vsizes[7]); - BOOST_CHECK(tx5_feerate > tx7_anc_feerate); - BOOST_CHECK(tx5_feerate > tx8_anc_feerate); + const auto tx2_feerate = CFeeRate(high_fee, tx_vsizes[2]); + const auto tx3_feerate = CFeeRate(high_fee, tx_vsizes[3]); + // tx3's feerate is lower than tx2's. same fee, different weight. + BOOST_CHECK(tx2_feerate > tx3_feerate); + const auto tx3_anc_feerate = CFeeRate(low_fee + med_fee + high_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]); + const auto tx3_iter = pool.GetIter(tx3->GetHash()); + BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_iter.value()->GetModFeesWithAncestors(), tx3_iter.value()->GetSizeWithAncestors())); + const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]); + const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]); + const auto tx6_iter = pool.GetIter(tx6->GetHash()); + BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_iter.value()->GetModFeesWithAncestors(), tx6_iter.value()->GetSizeWithAncestors())); + const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]); + const auto tx7_iter = pool.GetIter(tx7->GetHash()); + BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_iter.value()->GetModFeesWithAncestors(), tx7_iter.value()->GetSizeWithAncestors())); + BOOST_CHECK(tx4_feerate > tx6_anc_feerate); + BOOST_CHECK(tx4_feerate > tx7_anc_feerate); // Extremely high feerate: everybody's bumpfee is from their full ancestor set. { node::MiniMiner mini_miner(pool, all_unspent_outpoints); const CFeeRate very_high_feerate(COIN); - BOOST_CHECK(tx4_anc_feerate < very_high_feerate); + BOOST_CHECK(tx3_anc_feerate < very_high_feerate); BOOST_CHECK(mini_miner.IsReadyToCalculate()); auto bump_fees = mini_miner.CalculateBumpFees(very_high_feerate); BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); BOOST_CHECK(!mini_miner.IsReadyToCalculate()); BOOST_CHECK(sanity_check(all_transactions, bump_fees)); - const auto tx1_bumpfee = bump_fees.find(COutPoint{tx1->GetHash(), 1}); - BOOST_CHECK(tx1_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx1_bumpfee->second, very_high_feerate.GetFee(tx_vsizes[0]) - low_fee); - const auto tx4_bumpfee = bump_fees.find(COutPoint{tx4->GetHash(), 0}); - BOOST_CHECK(tx4_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx4_bumpfee->second, + const auto tx0_bumpfee = bump_fees.find(COutPoint{tx0->GetHash(), 1}); + BOOST_CHECK(tx0_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx0_bumpfee->second, very_high_feerate.GetFee(tx_vsizes[0]) - low_fee); + const auto tx3_bumpfee = bump_fees.find(COutPoint{tx3->GetHash(), 0}); + BOOST_CHECK(tx3_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx3_bumpfee->second, very_high_feerate.GetFee(tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]) - (low_fee + med_fee + high_fee + high_fee)); + const auto tx6_bumpfee = bump_fees.find(COutPoint{tx6->GetHash(), 0}); + BOOST_CHECK(tx6_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx6_bumpfee->second, + very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]) - (high_fee + low_fee + med_fee)); const auto tx7_bumpfee = bump_fees.find(COutPoint{tx7->GetHash(), 0}); BOOST_CHECK(tx7_bumpfee != bump_fees.end()); BOOST_CHECK_EQUAL(tx7_bumpfee->second, - very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]) - (high_fee + low_fee + med_fee)); - const auto tx8_bumpfee = bump_fees.find(COutPoint{tx8->GetHash(), 0}); - BOOST_CHECK(tx8_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx8_bumpfee->second, very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]) - (high_fee + low_fee + high_fee)); - // Total fees: if spending multiple outputs from tx4 don't double-count fees. - node::MiniMiner mini_miner_total_tx4(pool, {COutPoint{tx4->GetHash(), 0}, COutPoint{tx4->GetHash(), 1}}); - BOOST_CHECK(mini_miner_total_tx4.IsReadyToCalculate()); - const auto tx4_bump_fee = mini_miner_total_tx4.CalculateTotalBumpFees(very_high_feerate); - BOOST_CHECK(!mini_miner_total_tx4.IsReadyToCalculate()); - BOOST_CHECK(tx4_bump_fee.has_value()); - BOOST_CHECK_EQUAL(tx4_bump_fee.value(), + // Total fees: if spending multiple outputs from tx3 don't double-count fees. + node::MiniMiner mini_miner_total_tx3(pool, {COutPoint{tx3->GetHash(), 0}, COutPoint{tx3->GetHash(), 1}}); + BOOST_CHECK(mini_miner_total_tx3.IsReadyToCalculate()); + const auto tx3_bump_fee = mini_miner_total_tx3.CalculateTotalBumpFees(very_high_feerate); + BOOST_CHECK(!mini_miner_total_tx3.IsReadyToCalculate()); + BOOST_CHECK(tx3_bump_fee.has_value()); + BOOST_CHECK_EQUAL(tx3_bump_fee.value(), very_high_feerate.GetFee(tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]) - (low_fee + med_fee + high_fee + high_fee)); - // Total fees: if spending both tx7 and tx8, don't double-count fees. - node::MiniMiner mini_miner_tx7_tx8(pool, {COutPoint{tx7->GetHash(), 0}, COutPoint{tx8->GetHash(), 0}}); - BOOST_CHECK(mini_miner_tx7_tx8.IsReadyToCalculate()); - const auto tx7_tx8_bumpfee = mini_miner_tx7_tx8.CalculateTotalBumpFees(very_high_feerate); - BOOST_CHECK(!mini_miner_tx7_tx8.IsReadyToCalculate()); - BOOST_CHECK(tx7_tx8_bumpfee.has_value()); - BOOST_CHECK_EQUAL(tx7_tx8_bumpfee.value(), + // Total fees: if spending both tx6 and tx7, don't double-count fees. + node::MiniMiner mini_miner_tx6_tx7(pool, {COutPoint{tx6->GetHash(), 0}, COutPoint{tx7->GetHash(), 0}}); + BOOST_CHECK(mini_miner_tx6_tx7.IsReadyToCalculate()); + const auto tx6_tx7_bumpfee = mini_miner_tx6_tx7.CalculateTotalBumpFees(very_high_feerate); + BOOST_CHECK(!mini_miner_tx6_tx7.IsReadyToCalculate()); + BOOST_CHECK(tx6_tx7_bumpfee.has_value()); + BOOST_CHECK_EQUAL(tx6_tx7_bumpfee.value(), very_high_feerate.GetFee(tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6] + tx_vsizes[7]) - (high_fee + low_fee + med_fee + high_fee)); } - // Feerate just below tx5: tx7 and tx8 have different bump fees. + // Feerate just below tx4: tx6 and tx7 have different bump fees. { - const auto just_below_tx5 = CFeeRate(tx5_feerate.GetFeePerK() - 5); + const auto just_below_tx4 = CFeeRate(tx4_feerate.GetFeePerK() - 5); node::MiniMiner mini_miner(pool, all_unspent_outpoints); BOOST_CHECK(mini_miner.IsReadyToCalculate()); - auto bump_fees = mini_miner.CalculateBumpFees(just_below_tx5); + auto bump_fees = mini_miner.CalculateBumpFees(just_below_tx4); BOOST_CHECK(!mini_miner.IsReadyToCalculate()); BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + const auto tx6_bumpfee = bump_fees.find(COutPoint{tx6->GetHash(), 0}); + BOOST_CHECK(tx6_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx6_bumpfee->second, just_below_tx4.GetFee(tx_vsizes[5] + tx_vsizes[6]) - (low_fee + med_fee)); const auto tx7_bumpfee = bump_fees.find(COutPoint{tx7->GetHash(), 0}); BOOST_CHECK(tx7_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx7_bumpfee->second, just_below_tx5.GetFee(tx_vsizes[5] + tx_vsizes[6]) - (low_fee + med_fee)); - const auto tx8_bumpfee = bump_fees.find(COutPoint{tx8->GetHash(), 0}); - BOOST_CHECK(tx8_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx8_bumpfee->second, just_below_tx5.GetFee(tx_vsizes[5] + tx_vsizes[7]) - (low_fee + high_fee)); - // Total fees: if spending both tx7 and tx8, don't double-count fees. - node::MiniMiner mini_miner_tx7_tx8(pool, {COutPoint{tx7->GetHash(), 0}, COutPoint{tx8->GetHash(), 0}}); - BOOST_CHECK(mini_miner_tx7_tx8.IsReadyToCalculate()); - const auto tx7_tx8_bumpfee = mini_miner_tx7_tx8.CalculateTotalBumpFees(just_below_tx5); - BOOST_CHECK(!mini_miner_tx7_tx8.IsReadyToCalculate()); - BOOST_CHECK(tx7_tx8_bumpfee.has_value()); - BOOST_CHECK_EQUAL(tx7_tx8_bumpfee.value(), just_below_tx5.GetFee(tx_vsizes[5] + tx_vsizes[6]) - (low_fee + med_fee)); + BOOST_CHECK_EQUAL(tx7_bumpfee->second, just_below_tx4.GetFee(tx_vsizes[5] + tx_vsizes[7]) - (low_fee + high_fee)); + // Total fees: if spending both tx6 and tx7, don't double-count fees. + node::MiniMiner mini_miner_tx6_tx7(pool, {COutPoint{tx6->GetHash(), 0}, COutPoint{tx7->GetHash(), 0}}); + BOOST_CHECK(mini_miner_tx6_tx7.IsReadyToCalculate()); + const auto tx6_tx7_bumpfee = mini_miner_tx6_tx7.CalculateTotalBumpFees(just_below_tx4); + BOOST_CHECK(!mini_miner_tx6_tx7.IsReadyToCalculate()); + BOOST_CHECK(tx6_tx7_bumpfee.has_value()); + BOOST_CHECK_EQUAL(tx6_tx7_bumpfee.value(), just_below_tx4.GetFee(tx_vsizes[5] + tx_vsizes[6]) - (low_fee + med_fee)); } - // Feerate between tx7 and tx8's ancestor feerates: don't need to bump tx6 because tx8 already does. + // Feerate between tx6 and tx7's ancestor feerates: don't need to bump tx5 because tx7 already does. { - const auto just_above_tx7 = CFeeRate(med_fee + 10, tx_vsizes[6]); - BOOST_CHECK(just_above_tx7 <= CFeeRate(low_fee + high_fee, tx_vsizes[5] + tx_vsizes[7])); + const auto just_above_tx6 = CFeeRate(med_fee + 10, tx_vsizes[6]); + BOOST_CHECK(just_above_tx6 <= CFeeRate(low_fee + high_fee, tx_vsizes[5] + tx_vsizes[7])); node::MiniMiner mini_miner(pool, all_unspent_outpoints); BOOST_CHECK(mini_miner.IsReadyToCalculate()); - auto bump_fees = mini_miner.CalculateBumpFees(just_above_tx7); + auto bump_fees = mini_miner.CalculateBumpFees(just_above_tx6); BOOST_CHECK(!mini_miner.IsReadyToCalculate()); BOOST_CHECK_EQUAL(bump_fees.size(), all_unspent_outpoints.size()); BOOST_CHECK(sanity_check(all_transactions, bump_fees)); + const auto tx6_bumpfee = bump_fees.find(COutPoint{tx6->GetHash(), 0}); + BOOST_CHECK(tx6_bumpfee != bump_fees.end()); + BOOST_CHECK_EQUAL(tx6_bumpfee->second, just_above_tx6.GetFee(tx_vsizes[6]) - (med_fee)); const auto tx7_bumpfee = bump_fees.find(COutPoint{tx7->GetHash(), 0}); BOOST_CHECK(tx7_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx7_bumpfee->second, just_above_tx7.GetFee(tx_vsizes[6]) - (med_fee)); - const auto tx8_bumpfee = bump_fees.find(COutPoint{tx8->GetHash(), 0}); - BOOST_CHECK(tx8_bumpfee != bump_fees.end()); - BOOST_CHECK_EQUAL(tx8_bumpfee->second, 0); + BOOST_CHECK_EQUAL(tx7_bumpfee->second, 0); } } BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) @@ -445,12 +478,12 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) const auto cluster_501 = pool.GatherClusters({tx_501->GetHash()}); BOOST_CHECK_EQUAL(cluster_501.size(), 0); - // Zig Zag cluster: - // txp0 txp1 txp2 ... txp48 txp49 - // \ / \ / \ \ / - // txc0 txc1 txc2 ... txc48 - // Note that each transaction's ancestor size is 1 or 3, and each descendant size is 1, 2 or 3. - // However, all of these transactions are in the same cluster. + /* Zig Zag cluster: + * txp0 txp1 txp2 ... txp48 txp49 + * \ / \ / \ \ / + * txc0 txc1 txc2 ... txc48 + * Note that each transaction's ancestor size is 1 or 3, and each descendant size is 1, 2 or 3. + * However, all of these transactions are in the same cluster. */ std::vector<uint256> zigzag_txids; for (auto p{0}; p < 50; ++p) { const auto txp = make_tx({COutPoint{GetRandHash(), 0}}, /*num_outputs=*/2); diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 036355b5f1..a29d96dc76 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1114,7 +1114,7 @@ public: } /** Send V1 version message header to the transport. */ - void SendV1Version(const CMessageHeader::MessageStartChars& magic) + void SendV1Version(const MessageStartChars& magic) { CMessageHeader hdr(magic, "version", 126 + InsecureRandRange(11)); CDataStream ser(SER_NETWORK, CLIENT_VERSION); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 589a2fd766..99740ee779 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -260,7 +260,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) // The buffer size (second arg) must be greater than the rewind // amount (third arg). try { - CBufferedFile bfbad(file, 25, 25, 222, 333); + BufferedFile bfbad{file, 25, 25, 333}; BOOST_CHECK(false); } catch (const std::exception& e) { BOOST_CHECK(strstr(e.what(), @@ -268,11 +268,10 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) } // The buffer is 25 bytes, allow rewinding 10 bytes. - CBufferedFile bf(file, 25, 10, 222, 333); + BufferedFile bf{file, 25, 10, 333}; BOOST_CHECK(!bf.eof()); - // These two members have no functional effect. - BOOST_CHECK_EQUAL(bf.GetType(), 222); + // This member has no functional effect. BOOST_CHECK_EQUAL(bf.GetVersion(), 333); uint8_t i; @@ -357,7 +356,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) BOOST_CHECK(false); } catch (const std::exception& e) { BOOST_CHECK(strstr(e.what(), - "CBufferedFile::Fill: end of file") != nullptr); + "BufferedFile::Fill: end of file") != nullptr); } // Attempting to read beyond the end sets the EOF indicator. BOOST_CHECK(bf.eof()); @@ -392,7 +391,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip) rewind(file); // The buffer is 25 bytes, allow rewinding 10 bytes. - CBufferedFile bf(file, 25, 10, 222, 333); + BufferedFile bf{file, 25, 10, 333}; uint8_t i; // This is like bf >> (7-byte-variable), in that it will cause data @@ -446,7 +445,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) size_t bufSize = InsecureRandRange(300) + 1; size_t rewindSize = InsecureRandRange(bufSize); - CBufferedFile bf(file, bufSize, rewindSize, 222, 333); + BufferedFile bf{file, bufSize, rewindSize, 333}; size_t currentPos = 0; size_t maxPos = 0; for (int step = 0; step < 100; ++step) { diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index c1f6226982..790eabc7c1 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -4,6 +4,7 @@ #include <consensus/validation.h> #include <key.h> +#include <random.h> #include <script/sign.h> #include <script/signingprovider.h> #include <test/util/setup_common.h> diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 331199709e..2947bc3fcb 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -253,7 +253,7 @@ TestingSetup::TestingSetup( /*deterministic=*/false, m_node.args->GetIntArg("-checkaddrman", 0)); m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - m_node.connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests. + m_node.connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); // Deterministic randomness for tests. PeerManager::Options peerman_opts; ApplyArgsManOptions(*m_node.args, peerman_opts); m_node.peerman = PeerManager::make(*m_node.connman, *m_node.addrman, diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 26677bfa55..67f71bd266 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1791,4 +1791,29 @@ BOOST_AUTO_TEST_CASE(util_WriteBinaryFile) BOOST_CHECK(valid); BOOST_CHECK_EQUAL(actual_text, expected_text); } + +BOOST_AUTO_TEST_CASE(clearshrink_test) +{ + { + std::vector<uint8_t> v = {1, 2, 3}; + ClearShrink(v); + BOOST_CHECK_EQUAL(v.size(), 0); + BOOST_CHECK_EQUAL(v.capacity(), 0); + } + + { + std::vector<bool> v = {false, true, false, false, true, true}; + ClearShrink(v); + BOOST_CHECK_EQUAL(v.size(), 0); + BOOST_CHECK_EQUAL(v.capacity(), 0); + } + + { + std::deque<int> v = {1, 3, 3, 7}; + ClearShrink(v); + BOOST_CHECK_EQUAL(v.size(), 0); + // std::deque has no capacity() we can observe. + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/time.h b/src/util/time.h index b6aab615ba..6aa776137c 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -6,8 +6,6 @@ #ifndef BITCOIN_UTIL_TIME_H #define BITCOIN_UTIL_TIME_H -#include <compat/compat.h> - #include <chrono> // IWYU pragma: export #include <cstdint> #include <string> diff --git a/src/util/vector.h b/src/util/vector.h index 9b9218e54f..40ff73c293 100644 --- a/src/util/vector.h +++ b/src/util/vector.h @@ -49,4 +49,22 @@ inline V Cat(V v1, const V& v2) return v1; } +/** Clear a vector (or std::deque) and release its allocated memory. */ +template<typename V> +inline void ClearShrink(V& v) noexcept +{ + // There are various ways to clear a vector and release its memory: + // + // 1. V{}.swap(v) + // 2. v = V{} + // 3. v = {}; v.shrink_to_fit(); + // 4. v.clear(); v.shrink_to_fit(); + // + // (2) does not appear to release memory in glibc debug mode, even if v.shrink_to_fit() + // follows. (3) and (4) rely on std::vector::shrink_to_fit, which is only a non-binding + // request. Therefore, we use method (1). + + V{}.swap(v); +} + #endif // BITCOIN_UTIL_VECTOR_H diff --git a/src/validation.cpp b/src/validation.cpp index 6e0addc877..1d4786bb17 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -23,6 +23,7 @@ #include <hash.h> #include <kernel/chainparams.h> #include <kernel/mempool_entry.h> +#include <kernel/messagestartchars.h> #include <kernel/notifications_interface.h> #include <logging.h> #include <logging/timer.h> @@ -4588,8 +4589,8 @@ void ChainstateManager::LoadExternalBlockFile( int nLoaded = 0; try { - // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor - CBufferedFile blkdat(fileIn, 2*MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE+8, SER_DISK, CLIENT_VERSION); + // This takes over fileIn and calls fclose() on it in the BufferedFile destructor + BufferedFile blkdat{fileIn, 2 * MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE + 8, CLIENT_VERSION}; // nRewind indicates where to resume scanning in case something goes wrong, // such as a block fails to deserialize. uint64_t nRewind = blkdat.GetPos(); @@ -4602,11 +4603,11 @@ void ChainstateManager::LoadExternalBlockFile( unsigned int nSize = 0; try { // locate a header - unsigned char buf[CMessageHeader::MESSAGE_START_SIZE]; + MessageStartChars buf; blkdat.FindByte(std::byte(params.MessageStart()[0])); nRewind = blkdat.GetPos() + 1; blkdat >> buf; - if (memcmp(buf, params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) { + if (buf != params.MessageStart()) { continue; } // read size diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index d6b9b68e1f..391e120932 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -7,6 +7,7 @@ #include <common/system.h> #include <consensus/amount.h> #include <consensus/consensus.h> +#include <interfaces/chain.h> #include <logging.h> #include <policy/feerate.h> #include <util/check.h> @@ -449,19 +450,19 @@ void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool in } } -CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) +CAmount SelectionResult::GetSelectionWaste(CAmount change_cost, CAmount target, bool use_effective_value) { // This function should not be called with empty inputs as that would mean the selection failed - assert(!inputs.empty()); + assert(!m_selected_inputs.empty()); // Always consider the cost of spending an input now vs in the future. CAmount waste = 0; - CAmount selected_effective_value = 0; - for (const auto& coin_ptr : inputs) { + for (const auto& coin_ptr : m_selected_inputs) { const COutput& coin = *coin_ptr; waste += coin.GetFee() - coin.long_term_fee; - selected_effective_value += use_effective_value ? coin.GetEffectiveValue() : coin.txout.nValue; } + // Bump fee of whole selection may diverge from sum of individual bump fees + waste -= bump_fee_group_discount; if (change_cost) { // Consider the cost of making change and spending it in the future @@ -470,6 +471,7 @@ CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmo waste += change_cost; } else { // When we are not making change (change_cost == 0), consider the excess we are throwing away to fees + CAmount selected_effective_value = use_effective_value ? GetSelectedEffectiveValue() : GetSelectedValue(); assert(selected_effective_value >= target); waste += selected_effective_value - target; } @@ -488,14 +490,22 @@ CAmount GenerateChangeTarget(const CAmount payment_value, const CAmount change_f } } +void SelectionResult::SetBumpFeeDiscount(const CAmount discount) +{ + // Overlapping ancestry can only lower the fees, not increase them + assert (discount >= 0); + bump_fee_group_discount = discount; +} + + void SelectionResult::ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee) { const CAmount change = GetChange(min_viable_change, change_fee); if (change > 0) { - m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective); + m_waste = GetSelectionWaste(change_cost, m_target, m_use_effective); } else { - m_waste = GetSelectionWaste(m_selected_inputs, 0, m_target, m_use_effective); + m_waste = GetSelectionWaste(0, m_target, m_use_effective); } } @@ -511,7 +521,12 @@ CAmount SelectionResult::GetSelectedValue() const CAmount SelectionResult::GetSelectedEffectiveValue() const { - return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->GetEffectiveValue(); }); + return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->GetEffectiveValue(); }) + bump_fee_group_discount; +} + +CAmount SelectionResult::GetTotalBumpFees() const +{ + return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->ancestor_bump_fees; }) - bump_fee_group_discount; } void SelectionResult::Clear() diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index afd868fc89..20b2461c04 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -17,6 +17,7 @@ #include <optional> + namespace wallet { //! lower bound for randomly-chosen target change amount static constexpr CAmount CHANGE_LOWER{50000}; @@ -26,10 +27,10 @@ static constexpr CAmount CHANGE_UPPER{1000000}; /** A UTXO under consideration for use in funding a new transaction. */ struct COutput { private: - /** The output's value minus fees required to spend it.*/ + /** The output's value minus fees required to spend it and bump its unconfirmed ancestors to the target feerate. */ std::optional<CAmount> effective_value; - /** The fee required to spend this output at the transaction's target feerate. */ + /** The fee required to spend this output at the transaction's target feerate and to bump its unconfirmed ancestors to the target feerate. */ std::optional<CAmount> fee; public: @@ -71,6 +72,9 @@ public: /** The fee required to spend this output at the consolidation feerate. */ CAmount long_term_fee{0}; + /** The fee necessary to bump this UTXO's ancestor transactions to the target feerate */ + CAmount ancestor_bump_fees{0}; + COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me, const std::optional<CFeeRate> feerate = std::nullopt) : outpoint{outpoint}, txout{txout}, @@ -83,6 +87,7 @@ public: from_me{from_me} { if (feerate) { + // base fee without considering potential unconfirmed ancestors fee = input_bytes < 0 ? 0 : feerate.value().GetFee(input_bytes); effective_value = txout.nValue - fee.value(); } @@ -104,6 +109,16 @@ public: return outpoint < rhs.outpoint; } + void ApplyBumpFee(CAmount bump_fee) + { + assert(bump_fee >= 0); + ancestor_bump_fees = bump_fee; + assert(fee); + *fee += bump_fee; + // Note: assert(effective_value - bump_fee == nValue - fee.value()); + effective_value = txout.nValue - fee.value(); + } + CAmount GetFee() const { assert(fee.has_value()); @@ -275,26 +290,6 @@ struct OutputGroupTypeMap typedef std::map<CoinEligibilityFilter, OutputGroupTypeMap> FilteredOutputGroups; -/** Compute the waste for this result given the cost of change - * and the opportunity cost of spending these inputs now vs in the future. - * If change exists, waste = change_cost + inputs * (effective_feerate - long_term_feerate) - * If no change, waste = excess + inputs * (effective_feerate - long_term_feerate) - * where excess = selected_effective_value - target - * change_cost = effective_feerate * change_output_size + long_term_feerate * change_spend_size - * - * Note this function is separate from SelectionResult for the tests. - * - * @param[in] inputs The selected inputs - * @param[in] change_cost The cost of creating change and spending it in the future. - * Only used if there is change, in which case it must be positive. - * Must be 0 if there is no change. - * @param[in] target The amount targeted by the coin selection algorithm. - * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false). - * @return The waste - */ -[[nodiscard]] CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); - - /** Choose a random change target for each transaction to make it harder to fingerprint the Core * wallet based on the change output values of transactions it creates. * Change target covers at least change fees and adds a random value on top of it. @@ -336,6 +331,8 @@ private: std::optional<CAmount> m_waste; /** Total weight of the selected inputs */ int m_weight{0}; + /** How much individual inputs overestimated the bump fees for the shared ancestry */ + CAmount bump_fee_group_discount{0}; template<typename T> void InsertInputs(const T& inputs) @@ -348,6 +345,22 @@ private: } } + /** Compute the waste for this result given the cost of change + * and the opportunity cost of spending these inputs now vs in the future. + * If change exists, waste = change_cost + inputs * (effective_feerate - long_term_feerate) + * If no change, waste = excess + inputs * (effective_feerate - long_term_feerate) + * where excess = selected_effective_value - target + * change_cost = effective_feerate * change_output_size + long_term_feerate * change_spend_size + * + * @param[in] change_cost The cost of creating change and spending it in the future. + * Only used if there is change, in which case it must be positive. + * Must be 0 if there is no change. + * @param[in] target The amount targeted by the coin selection algorithm. + * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false). + * @return The waste + */ + [[nodiscard]] CAmount GetSelectionWaste(CAmount change_cost, CAmount target, bool use_effective_value = true); + public: explicit SelectionResult(const CAmount target, SelectionAlgorithm algo) : m_target(target), m_algo(algo) {} @@ -359,11 +372,16 @@ public: [[nodiscard]] CAmount GetSelectedEffectiveValue() const; + [[nodiscard]] CAmount GetTotalBumpFees() const; + void Clear(); void AddInput(const OutputGroup& group); void AddInputs(const std::set<std::shared_ptr<COutput>>& inputs, bool subtract_fee_outputs); + /** How much individual inputs overestimated the bump fees for shared ancestries */ + void SetBumpFeeDiscount(const CAmount discount); + /** Calculates and stores the waste for this selection via GetSelectionWaste */ void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee); [[nodiscard]] CAmount GetWaste() const; diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 0c24920516..0ee39d2b5a 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -136,7 +136,7 @@ bool IsSQLiteFile(const fs::path& path) } // Check the application id matches our network magic - return memcmp(Params().MessageStart(), app_id, 4) == 0; + return memcmp(Params().MessageStart().data(), app_id, 4) == 0; } void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 3720d144eb..f99da926bc 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -63,7 +63,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet } //! Check if the user provided a valid feeRate -static feebumper::Result CheckFeeRate(const CWallet& wallet, const CFeeRate& newFeerate, const int64_t maxTxSize, CAmount old_fee, std::vector<bilingual_str>& errors) +static feebumper::Result CheckFeeRate(const CWallet& wallet, const CMutableTransaction& mtx, const CFeeRate& newFeerate, const int64_t maxTxSize, CAmount old_fee, std::vector<bilingual_str>& errors) { // check that fee rate is higher than mempool's minimum fee // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) @@ -80,7 +80,17 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CFeeRate& new return feebumper::Result::WALLET_ERROR; } - CAmount new_total_fee = newFeerate.GetFee(maxTxSize); + std::vector<COutPoint> reused_inputs; + reused_inputs.reserve(mtx.vin.size()); + for (const CTxIn& txin : mtx.vin) { + reused_inputs.push_back(txin.prevout); + } + + std::optional<CAmount> combined_bump_fee = wallet.chain().CalculateCombinedBumpFee(reused_inputs, newFeerate); + if (!combined_bump_fee.has_value()) { + errors.push_back(strprintf(Untranslated("Failed to calculate bump fees, because unconfirmed UTXOs depend on enormous cluster of unconfirmed transactions."))); + } + CAmount new_total_fee = newFeerate.GetFee(maxTxSize) + combined_bump_fee.value(); CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE)); @@ -283,7 +293,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo } temp_mtx.vout = txouts; const int64_t maxTxSize{CalculateMaximumSignedTxSize(CTransaction(temp_mtx), &wallet, &new_coin_control).vsize}; - Result res = CheckFeeRate(wallet, *new_coin_control.m_feerate, maxTxSize, old_fee, errors); + Result res = CheckFeeRate(wallet, temp_mtx, *new_coin_control.m_feerate, maxTxSize, old_fee, errors); if (res != Result::OK) { return res; } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index fd7f279505..96c9a3dc16 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -259,6 +259,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const { PreSelectedInputs result; const bool can_grind_r = wallet.CanGrindR(); + std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().CalculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate); for (const COutPoint& outpoint : coin_control.ListSelected()) { int input_bytes = -1; CTxOut txout; @@ -294,6 +295,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */ COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate); + output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint)); result.Insert(output, coin_selection_params.m_subtract_fee_outputs); } return result; @@ -314,6 +316,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, const int max_depth = {coinControl ? coinControl->m_max_depth : DEFAULT_MAX_DEPTH}; const bool only_safe = {coinControl ? !coinControl->m_include_unsafe_inputs : true}; const bool can_grind_r = wallet.CanGrindR(); + std::vector<COutPoint> outpoints; std::set<uint256> trusted_parents; for (const auto& entry : wallet.mapWallet) @@ -433,6 +436,8 @@ CoinsResult AvailableCoins(const CWallet& wallet, result.Add(GetOutputType(type, is_from_p2sh), COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate)); + outpoints.push_back(outpoint); + // Checks the sum amount of all UTXO's. if (params.min_sum_amount != MAX_MONEY) { if (result.GetTotalAmount() >= params.min_sum_amount) { @@ -447,6 +452,16 @@ CoinsResult AvailableCoins(const CWallet& wallet, } } + if (feerate.has_value()) { + std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().CalculateIndividualBumpFees(outpoints, feerate.value()); + + for (auto& [_, outputs] : result.coins) { + for (auto& output : outputs) { + output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint)); + } + } + } + return result; } @@ -628,13 +643,13 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, // Returns true if the result contains an error and the message is not empty static bool HasErrorMsg(const util::Result<SelectionResult>& res) { return !util::ErrorString(res).empty(); } -util::Result<SelectionResult> AttemptSelection(const CAmount& nTargetValue, OutputGroupTypeMap& groups, +util::Result<SelectionResult> AttemptSelection(interfaces::Chain& chain, const CAmount& nTargetValue, OutputGroupTypeMap& groups, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types) { // Run coin selection on each OutputType and compute the Waste Metric std::vector<SelectionResult> results; for (auto& [type, group] : groups.groups_by_type) { - auto result{ChooseSelectionResult(nTargetValue, group, coin_selection_params)}; + auto result{ChooseSelectionResult(chain, nTargetValue, group, coin_selection_params)}; // If any specific error message appears here, then something particularly wrong happened. if (HasErrorMsg(result)) return result; // So let's return the specific error. // Append the favorable result. @@ -648,14 +663,14 @@ util::Result<SelectionResult> AttemptSelection(const CAmount& nTargetValue, Outp // over all available coins, which would allow mixing. // If TypesCount() <= 1, there is nothing to mix. if (allow_mixed_output_types && groups.TypesCount() > 1) { - return ChooseSelectionResult(nTargetValue, groups.all_groups, coin_selection_params); + return ChooseSelectionResult(chain, nTargetValue, groups.all_groups, coin_selection_params); } // Either mixing is not allowed and we couldn't find a solution from any single OutputType, or mixing was allowed and we still couldn't // find a solution using all available coins return util::Error(); }; -util::Result<SelectionResult> ChooseSelectionResult(const CAmount& nTargetValue, Groups& groups, const CoinSelectionParams& coin_selection_params) +util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, const CAmount& nTargetValue, Groups& groups, const CoinSelectionParams& coin_selection_params) { // Vector of results. We will choose the best one based on waste. std::vector<SelectionResult> results; @@ -680,12 +695,10 @@ util::Result<SelectionResult> ChooseSelectionResult(const CAmount& nTargetValue, // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_inputs_weight)}) { - knapsack_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*knapsack_result); } else append_error(knapsack_result); if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.m_change_fee, coin_selection_params.rng_fast, max_inputs_weight)}) { - srd_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*srd_result); } else append_error(srd_result); @@ -695,6 +708,27 @@ util::Result<SelectionResult> ChooseSelectionResult(const CAmount& nTargetValue, return errors.empty() ? util::Error() : errors.front(); } + // If the chosen input set has unconfirmed inputs, check for synergies from overlapping ancestry + for (auto& result : results) { + std::vector<COutPoint> outpoints; + std::set<std::shared_ptr<COutput>> coins = result.GetInputSet(); + CAmount summed_bump_fees = 0; + for (auto& coin : coins) { + if (coin->depth > 0) continue; // Bump fees only exist for unconfirmed inputs + outpoints.push_back(coin->outpoint); + summed_bump_fees += coin->ancestor_bump_fees; + } + std::optional<CAmount> combined_bump_fee = chain.CalculateCombinedBumpFee(outpoints, coin_selection_params.m_effective_feerate); + if (!combined_bump_fee.has_value()) { + return util::Error{_("Failed to calculate bump fees, because unconfirmed UTXOs depend on enormous cluster of unconfirmed transactions.")}; + } + CAmount bump_fee_overestimate = summed_bump_fees - combined_bump_fee.value(); + if (bump_fee_overestimate) { + result.SetBumpFeeDiscount(bump_fee_overestimate); + } + result.ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); + } + // Choose the result with the least waste // If the waste is the same, choose the one which spends more inputs. return *std::min_element(results.begin(), results.end()); @@ -824,7 +858,7 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin for (const auto& select_filter : ordered_filters) { auto it = filtered_groups.find(select_filter.filter); if (it == filtered_groups.end()) continue; - if (auto res{AttemptSelection(value_to_select, it->second, + if (auto res{AttemptSelection(wallet.chain(), value_to_select, it->second, coin_selection_params, select_filter.allow_mixed_output_types)}) { return res; // result found } else { @@ -1120,7 +1154,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( if (nBytes == -1) { return util::Error{_("Missing solving data for estimating transaction size")}; } - CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); + CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes) + result.GetTotalBumpFees(); const CAmount output_value = CalculateOutputValue(txNew); Assume(recipients_sum + change_amount == output_value); CAmount current_fee = result.GetSelectedValue() - output_value; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index cc9ccf3011..407627b5f1 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -123,6 +123,7 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, * the solution (according to the waste metric) will be chosen. If a valid input cannot be found from any * single OutputType, fallback to running `ChooseSelectionResult()` over all available coins. * + * param@[in] chain The chain interface to get information on unconfirmed UTXOs bump fees * param@[in] nTargetValue The target value * param@[in] groups The grouped outputs mapped by coin eligibility filters * param@[in] coin_selection_params Parameters for the coin selection @@ -132,7 +133,7 @@ FilteredOutputGroups GroupOutputs(const CWallet& wallet, * or (2) an specific error message if there was something particularly wrong (e.g. a selection * result that surpassed the tx max weight size). */ -util::Result<SelectionResult> AttemptSelection(const CAmount& nTargetValue, OutputGroupTypeMap& groups, +util::Result<SelectionResult> AttemptSelection(interfaces::Chain& chain, const CAmount& nTargetValue, OutputGroupTypeMap& groups, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types); /** @@ -140,6 +141,7 @@ util::Result<SelectionResult> AttemptSelection(const CAmount& nTargetValue, Outp * Multiple coin selection algorithms will be run and the input set that produces the least waste * (according to the waste metric) will be chosen. * + * param@[in] chain The chain interface to get information on unconfirmed UTXOs bump fees * param@[in] nTargetValue The target value * param@[in] groups The struct containing the outputs grouped by script and divided by (1) positive only outputs and (2) all outputs (positive + negative). * param@[in] coin_selection_params Parameters for the coin selection @@ -148,7 +150,7 @@ util::Result<SelectionResult> AttemptSelection(const CAmount& nTargetValue, Outp * or (2) an specific error message if there was something particularly wrong (e.g. a selection * result that surpassed the tx max weight size). */ -util::Result<SelectionResult> ChooseSelectionResult(const CAmount& nTargetValue, Groups& groups, const CoinSelectionParams& coin_selection_params); +util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, const CAmount& nTargetValue, Groups& groups, const CoinSelectionParams& coin_selection_params); // User manually selected inputs that must be part of the transaction struct PreSelectedInputs diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index ecd34bb96a..db9989163d 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -193,7 +193,7 @@ bool SQLiteDatabase::Verify(bilingual_str& error) auto read_result = ReadPragmaInteger(m_db, "application_id", "the application id", error); if (!read_result.has_value()) return false; uint32_t app_id = static_cast<uint32_t>(read_result.value()); - uint32_t net_magic = ReadBE32(Params().MessageStart()); + uint32_t net_magic = ReadBE32(Params().MessageStart().data()); if (app_id != net_magic) { error = strprintf(_("SQLiteDatabase: Unexpected application id. Expected %u, got %u"), net_magic, app_id); return false; @@ -324,7 +324,7 @@ void SQLiteDatabase::Open() } // Set the application id - uint32_t app_id = ReadBE32(Params().MessageStart()); + uint32_t app_id = ReadBE32(Params().MessageStart().data()); SetPragma(m_db, "application_id", strprintf("%d", static_cast<int32_t>(app_id)), "Failed to set the application id"); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index c8283f453a..9569210ba0 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -58,15 +58,17 @@ static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result) result.AddInput(group); } -static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fee = 0, CAmount long_term_fee = 0) +static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result, CAmount fee, CAmount long_term_fee) { CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; tx.nLockTime = nextLockTime++; // so all transactions get different hashes - COutput coin(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ 148, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, fee); - coin.long_term_fee = long_term_fee; - set.insert(std::make_shared<COutput>(coin)); + std::shared_ptr<COutput> coin = std::make_shared<COutput>(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ 148, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, fee); + OutputGroup group; + group.Insert(coin, /*ancestors=*/ 0, /*descendants=*/ 0); + coin->long_term_fee = long_term_fee; // group.Insert() will modify long_term_fee, so we need to set it afterwards + result.AddInput(group); } static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = CFeeRate(0), int nAge = 6*24, bool fIsFromMe = false, int nInput =0, bool spendable = false, int custom_size = 0) @@ -827,7 +829,6 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) BOOST_AUTO_TEST_CASE(waste_test) { - CoinSet selection; const CAmount fee{100}; const CAmount change_cost{125}; const CAmount fee_diff{40}; @@ -835,92 +836,179 @@ BOOST_AUTO_TEST_CASE(waste_test) const CAmount target{2 * COIN}; const CAmount excess{in_amt - fee * 2 - target}; - // Waste with change is the change cost and difference between fee and long term fee - add_coin(1 * COIN, 1, selection, fee, fee - fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee - fee_diff); - const CAmount waste1 = GetSelectionWaste(selection, change_cost, target); - BOOST_CHECK_EQUAL(fee_diff * 2 + change_cost, waste1); - selection.clear(); - - // Waste without change is the excess and difference between fee and long term fee - add_coin(1 * COIN, 1, selection, fee, fee - fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee - fee_diff); - const CAmount waste_nochange1 = GetSelectionWaste(selection, 0, target); - BOOST_CHECK_EQUAL(fee_diff * 2 + excess, waste_nochange1); - selection.clear(); - - // Waste with change and fee == long term fee is just cost of change - add_coin(1 * COIN, 1, selection, fee, fee); - add_coin(2 * COIN, 2, selection, fee, fee); - BOOST_CHECK_EQUAL(change_cost, GetSelectionWaste(selection, change_cost, target)); - selection.clear(); - - // Waste without change and fee == long term fee is just the excess - add_coin(1 * COIN, 1, selection, fee, fee); - add_coin(2 * COIN, 2, selection, fee, fee); - BOOST_CHECK_EQUAL(excess, GetSelectionWaste(selection, 0, target)); - selection.clear(); - - // Waste will be greater when fee is greater, but long term fee is the same - add_coin(1 * COIN, 1, selection, fee * 2, fee - fee_diff); - add_coin(2 * COIN, 2, selection, fee * 2, fee - fee_diff); - const CAmount waste2 = GetSelectionWaste(selection, change_cost, target); - BOOST_CHECK_GT(waste2, waste1); - selection.clear(); - - // Waste with change is the change cost and difference between fee and long term fee - // With long term fee greater than fee, waste should be less than when long term fee is less than fee - add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - const CAmount waste3 = GetSelectionWaste(selection, change_cost, target); - BOOST_CHECK_EQUAL(fee_diff * -2 + change_cost, waste3); - BOOST_CHECK_LT(waste3, waste1); - selection.clear(); - - // Waste without change is the excess and difference between fee and long term fee - // With long term fee greater than fee, waste should be less than when long term fee is less than fee - add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - const CAmount waste_nochange2 = GetSelectionWaste(selection, 0, target); - BOOST_CHECK_EQUAL(fee_diff * -2 + excess, waste_nochange2); - BOOST_CHECK_LT(waste_nochange2, waste_nochange1); - selection.clear(); - - // No Waste when fee == long_term_fee, no change, and no excess - add_coin(1 * COIN, 1, selection, fee, fee); - add_coin(2 * COIN, 2, selection, fee, fee); - const CAmount exact_target{in_amt - fee * 2}; - BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/0, exact_target)); - selection.clear(); - - // No Waste when (fee - long_term_fee) == (-cost_of_change), and no excess - const CAmount new_change_cost{fee_diff * 2}; - add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, new_change_cost, target)); - selection.clear(); - - // No Waste when (fee - long_term_fee) == (-excess), no change cost - const CAmount new_target{in_amt - fee * 2 - fee_diff * 2}; - add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/ 0, new_target)); - selection.clear(); - - // Negative waste when the long term fee is greater than the current fee and the selected value == target - const CAmount exact_target1{3 * COIN - 2 * fee}; - const CAmount target_waste1{-2 * fee_diff}; // = (2 * fee) - (2 * (fee + fee_diff)) - add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - BOOST_CHECK_EQUAL(target_waste1, GetSelectionWaste(selection, /*change_cost=*/ 0, exact_target1)); - selection.clear(); - - // Negative waste when the long term fee is greater than the current fee and change_cost < - (inputs * (fee - long_term_fee)) - const CAmount large_fee_diff{90}; - const CAmount target_waste2{-2 * large_fee_diff + change_cost}; // = (2 * fee) - (2 * (fee + large_fee_diff)) + change_cost - add_coin(1 * COIN, 1, selection, fee, fee + large_fee_diff); - add_coin(2 * COIN, 2, selection, fee, fee + large_fee_diff); - BOOST_CHECK_EQUAL(target_waste2, GetSelectionWaste(selection, change_cost, target)); + // The following tests that the waste is calculated correctly in various scenarios. + // ComputeAndSetWaste will first determine the size of the change output. We don't really + // care about the change and just want to use the variant that always includes the change_cost, + // so min_viable_change and change_fee are set to 0 to ensure that. + { + // Waste with change is the change cost and difference between fee and long term fee + SelectionResult selection1{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection1, fee, fee - fee_diff); + add_coin(2 * COIN, 2, selection1, fee, fee - fee_diff); + selection1.ComputeAndSetWaste(/*min_viable_change=*/0, change_cost, /*change_fee=*/0); + BOOST_CHECK_EQUAL(fee_diff * 2 + change_cost, selection1.GetWaste()); + + // Waste will be greater when fee is greater, but long term fee is the same + SelectionResult selection2{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection2, fee * 2, fee - fee_diff); + add_coin(2 * COIN, 2, selection2, fee * 2, fee - fee_diff); + selection2.ComputeAndSetWaste(/*min_viable_change=*/0, change_cost, /*change_fee=*/0); + BOOST_CHECK_GT(selection2.GetWaste(), selection1.GetWaste()); + + // Waste with change is the change cost and difference between fee and long term fee + // With long term fee greater than fee, waste should be less than when long term fee is less than fee + SelectionResult selection3{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection3, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection3, fee, fee + fee_diff); + selection3.ComputeAndSetWaste(/*min_viable_change=*/0, change_cost, /*change_fee=*/0); + BOOST_CHECK_EQUAL(fee_diff * -2 + change_cost, selection3.GetWaste()); + BOOST_CHECK_LT(selection3.GetWaste(), selection1.GetWaste()); + } + + { + // Waste without change is the excess and difference between fee and long term fee + SelectionResult selection_nochange1{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection_nochange1, fee, fee - fee_diff); + add_coin(2 * COIN, 2, selection_nochange1, fee, fee - fee_diff); + selection_nochange1.ComputeAndSetWaste(/*min_viable_change=*/0, /*change_cost=*/0, /*change_fee=*/0); + BOOST_CHECK_EQUAL(fee_diff * 2 + excess, selection_nochange1.GetWaste()); + + // Waste without change is the excess and difference between fee and long term fee + // With long term fee greater than fee, waste should be less than when long term fee is less than fee + SelectionResult selection_nochange2{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection_nochange2, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection_nochange2, fee, fee + fee_diff); + selection_nochange2.ComputeAndSetWaste(/*min_viable_change=*/0, /*change_cost=*/0, /*change_fee=*/0); + BOOST_CHECK_EQUAL(fee_diff * -2 + excess, selection_nochange2.GetWaste()); + BOOST_CHECK_LT(selection_nochange2.GetWaste(), selection_nochange1.GetWaste()); + } + + { + // Waste with change and fee == long term fee is just cost of change + SelectionResult selection{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection, fee, fee); + add_coin(2 * COIN, 2, selection, fee, fee); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, change_cost, /*change_fee=*/0); + BOOST_CHECK_EQUAL(change_cost, selection.GetWaste()); + } + + { + // Waste without change and fee == long term fee is just the excess + SelectionResult selection{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection, fee, fee); + add_coin(2 * COIN, 2, selection, fee, fee); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, /*change_cost=*/0, /*change_fee=*/0); + BOOST_CHECK_EQUAL(excess, selection.GetWaste()); + } + + { + // No Waste when fee == long_term_fee, no change, and no excess + const CAmount exact_target{in_amt - fee * 2}; + SelectionResult selection{exact_target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection, fee, fee); + add_coin(2 * COIN, 2, selection, fee, fee); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, /*change_cost=*/0, /*change_fee=*/0); + BOOST_CHECK_EQUAL(0, selection.GetWaste()); + } + + { + // No Waste when (fee - long_term_fee) == (-cost_of_change), and no excess + SelectionResult selection{target, SelectionAlgorithm::MANUAL}; + const CAmount new_change_cost{fee_diff * 2}; + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, new_change_cost, /*change_fee=*/0); + BOOST_CHECK_EQUAL(0, selection.GetWaste()); + } + + { + // No Waste when (fee - long_term_fee) == (-excess), no change cost + const CAmount new_target{in_amt - fee * 2 - fee_diff * 2}; + SelectionResult selection{new_target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, /*change_cost=*/0, /*change_fee=*/0); + BOOST_CHECK_EQUAL(0, selection.GetWaste()); + } + + { + // Negative waste when the long term fee is greater than the current fee and the selected value == target + const CAmount exact_target{3 * COIN - 2 * fee}; + SelectionResult selection{exact_target, SelectionAlgorithm::MANUAL}; + const CAmount target_waste1{-2 * fee_diff}; // = (2 * fee) - (2 * (fee + fee_diff)) + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, /*change_cost=*/0, /*change_fee=*/0); + BOOST_CHECK_EQUAL(target_waste1, selection.GetWaste()); + } + + { + // Negative waste when the long term fee is greater than the current fee and change_cost < - (inputs * (fee - long_term_fee)) + SelectionResult selection{target, SelectionAlgorithm::MANUAL}; + const CAmount large_fee_diff{90}; + const CAmount target_waste2{-2 * large_fee_diff + change_cost}; // = (2 * fee) - (2 * (fee + large_fee_diff)) + change_cost + add_coin(1 * COIN, 1, selection, fee, fee + large_fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + large_fee_diff); + selection.ComputeAndSetWaste(/*min_viable_change=*/0, change_cost, /*change_fee=*/0); + BOOST_CHECK_EQUAL(target_waste2, selection.GetWaste()); + } +} + + +BOOST_AUTO_TEST_CASE(bump_fee_test) +{ + const CAmount fee{100}; + const CAmount min_viable_change{200}; + const CAmount change_cost{125}; + const CAmount change_fee{35}; + const CAmount fee_diff{40}; + const CAmount target{2 * COIN}; + + { + SelectionResult selection{target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection, /*fee=*/fee, /*long_term_fee=*/fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + const std::vector<std::shared_ptr<COutput>> inputs = selection.GetShuffledInputVector(); + + for (size_t i = 0; i < inputs.size(); ++i) { + inputs[i]->ApplyBumpFee(20*(i+1)); + } + + selection.ComputeAndSetWaste(min_viable_change, change_cost, change_fee); + CAmount expected_waste = fee_diff * -2 + change_cost + /*bump_fees=*/60; + BOOST_CHECK_EQUAL(expected_waste, selection.GetWaste()); + + selection.SetBumpFeeDiscount(30); + selection.ComputeAndSetWaste(min_viable_change, change_cost, change_fee); + expected_waste = fee_diff * -2 + change_cost + /*bump_fees=*/60 - /*group_discount=*/30; + BOOST_CHECK_EQUAL(expected_waste, selection.GetWaste()); + } + + { + // Test with changeless transaction + // + // Bump fees and excess both contribute fully to the waste score, + // therefore, a bump fee group discount will not change the waste + // score as long as we do not create change in both instances. + CAmount changeless_target = 3 * COIN - 2 * fee - 100; + SelectionResult selection{changeless_target, SelectionAlgorithm::MANUAL}; + add_coin(1 * COIN, 1, selection, /*fee=*/fee, /*long_term_fee=*/fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + const std::vector<std::shared_ptr<COutput>> inputs = selection.GetShuffledInputVector(); + + for (size_t i = 0; i < inputs.size(); ++i) { + inputs[i]->ApplyBumpFee(20*(i+1)); + } + + selection.ComputeAndSetWaste(min_viable_change, change_cost, change_fee); + CAmount expected_waste = fee_diff * -2 + /*bump_fees=*/60 + /*excess = 100 - bump_fees*/40; + BOOST_CHECK_EQUAL(expected_waste, selection.GetWaste()); + + selection.SetBumpFeeDiscount(30); + selection.ComputeAndSetWaste(min_viable_change, change_cost, change_fee); + expected_waste = fee_diff * -2 + /*bump_fees=*/60 - /*group_discount=*/30 + /*excess = 100 - bump_fees + group_discount*/70; + BOOST_CHECK_EQUAL(expected_waste, selection.GetWaste()); + } } BOOST_AUTO_TEST_CASE(effective_value_test) |