diff --git a/contrib/tracing/mempool_monitor.py b/contrib/tracing/mempool_monitor.py index 7e8790dd233f..79a89b7e1eb3 100755 --- a/contrib/tracing/mempool_monitor.py +++ b/contrib/tracing/mempool_monitor.py @@ -27,7 +27,7 @@ struct added_event { u8 hash[HASH_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; }; @@ -35,7 +35,7 @@ { u8 hash[HASH_LENGTH]; char reason[MAX_REMOVAL_REASON_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; u64 entry_time; }; diff --git a/doc/tracing.md b/doc/tracing.md index 16ee467ad9de..d7a1139cac88 100644 --- a/doc/tracing.md +++ b/doc/tracing.md @@ -220,7 +220,7 @@ about the transaction. Arguments passed: 1. Transaction ID (hash) as `pointer to unsigned chars` (i.e. 32 bytes in little-endian) -2. Transaction virtual size as `uint64` +2. Transaction virtual size as `int32` 3. Transaction fee as `int64` #### Tracepoint `mempool:removed` @@ -231,7 +231,7 @@ about the transaction. Arguments passed: 1. Transaction ID (hash) as `pointer to unsigned chars` (i.e. 32 bytes in little-endian) 2. Removal reason as `pointer to C-style String` (max. length 9 characters) -3. Transaction virtual size as `uint64` +3. Transaction virtual size as `int32` 4. Transaction fee as `int64` 5. Transaction mempool entry time (epoch) as `uint64` diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp index 2ec7b8d59423..33667b02cc5e 100644 --- a/src/bench/load_external.cpp +++ b/src/bench/load_external.cpp @@ -34,7 +34,7 @@ static void LoadExternalBlockFile(benchmark::Bench& bench) ss << static_cast(benchmark::data::block813851.size()); // We can't use the streaming serialization (ss << benchmark::data::block813851) // because that first writes a compact size. - ss.write(MakeByteSpan(benchmark::data::block813851)); + ss << Span{benchmark::data::block813851}; // Create the test file. { diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 8465eef35b26..a44d9de9f7b4 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -58,7 +58,7 @@ struct CompareIteratorByHash { * ("descendant" transactions). * * When a new entry is added to the mempool, we update the descendant state - * (nCountWithDescendants, nSizeWithDescendants, and nModFeesWithDescendants) for + * (m_count_with_descendants, nSizeWithDescendants, and nModFeesWithDescendants) for * all ancestors of the newly added transaction. * */ @@ -80,7 +80,7 @@ class CTxMemPoolEntry mutable Parents m_parents; mutable Children m_children; const CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups - const size_t nTxSize; //!< ... and avoid recomputing tx size + const int32_t nTxSize; //!< ... and avoid recomputing tx size const size_t nUsageSize; //!< ... and total memory usage const int64_t nTime; //!< Local time when entering the mempool const unsigned int entryHeight; //!< Chain height when entering the mempool @@ -92,13 +92,15 @@ class CTxMemPoolEntry // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these // descendants as well. - uint64_t nCountWithDescendants{1}; //!< number of descendant transactions - uint64_t nSizeWithDescendants; //!< ... and size + int64_t m_count_with_descendants{1}; //!< number of descendant transactions + // Using int64_t instead of int32_t to avoid signed integer overflow issues. + int64_t nSizeWithDescendants; //!< ... and size CAmount nModFeesWithDescendants; //!< ... and total fees (all including us) // Analogous statistics for ancestor transactions - uint64_t nCountWithAncestors{1}; - uint64_t nSizeWithAncestors; + int64_t m_count_with_ancestors{1}; + // Using int64_t instead of int32_t to avoid signed integer overflow issues. + int64_t nSizeWithAncestors; CAmount nModFeesWithAncestors; int64_t nSigOpCountWithAncestors; @@ -137,7 +139,7 @@ class CTxMemPoolEntry tx = std::move(new_tx); } const CAmount& GetFee() const { return nFee; } - size_t GetTxSize() const + int32_t GetTxSize() const { return GetVirtualTransactionSize(nTxSize, sigOpCount, ::nBytesPerSigOp); } @@ -149,9 +151,9 @@ class CTxMemPoolEntry const LockPoints& GetLockPoints() const { return lockPoints; } // Adjusts the descendant state. - void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); + void UpdateDescendantState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount); // Adjusts the ancestor state - void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps); + void UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps); // Updates the modified fees with descendants/ancestors. void UpdateModifiedFee(CAmount fee_diff) { @@ -166,14 +168,14 @@ class CTxMemPoolEntry lockPoints = lp; } - uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } - uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } + uint64_t GetCountWithDescendants() const { return m_count_with_descendants; } + int64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; } bool GetSpendsCoinbase() const { return spendsCoinbase; } - uint64_t GetCountWithAncestors() const { return nCountWithAncestors; } - uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } + uint64_t GetCountWithAncestors() const { return m_count_with_ancestors; } + int64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } int64_t GetSigOpCountWithAncestors() const { return nSigOpCountWithAncestors; } diff --git a/src/net.cpp b/src/net.cpp index dbf34dbbe5ce..d072c33aa305 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -5069,13 +5069,13 @@ static void CaptureMessageToFile(const CAddress& addr, AutoFile f{fsbridge::fopen(path, "ab")}; ser_writedata64(f, now.count()); - f.write(MakeByteSpan(msg_type)); + f << Span{msg_type}; for (auto i = msg_type.length(); i < CMessageHeader::COMMAND_SIZE; ++i) { f << uint8_t{'\0'}; } uint32_t size = data.size(); ser_writedata32(f, size); - f.write(AsBytes(data)); + f << data; } std::function m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); std::shared_ptr m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex); uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex); + std::unique_ptr> m_most_recent_block_txs GUARDED_BY(m_most_recent_block_mutex); /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ int m_highest_fast_announce GUARDED_BY(::cs_main){0}; @@ -1117,7 +1116,7 @@ class PeerManagerImpl final : public PeerManager /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) - EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex, NetEventsInterface::g_msgproc_mutex) @@ -1130,12 +1129,6 @@ class PeerManagerImpl final : public PeerManager void ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const BlockTransactions& block_transactions) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex, !m_most_recent_block_mutex); - /** Relay map (txid -> CTransactionRef) */ - typedef std::map MapRelay; - MapRelay mapRelay GUARDED_BY(NetEventsInterface::g_msgproc_mutex); - /** Expiration-time ordered list of (expire time, relay map entry) pairs. */ - std::deque> g_relay_expiration GUARDED_BY(NetEventsInterface::g_msgproc_mutex); - /** * When a peer sends us a valid block, instruct it to announce blocks to us * using CMPCTBLOCK if possible by adding its nodeid to the end of @@ -2306,10 +2299,20 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha std::async(std::launch::deferred, [&] { return msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; { + auto most_recent_block_txs = std::make_unique>(); + for (const auto& tx : pblock->vtx) { + most_recent_block_txs->emplace(tx->GetHash(), tx); + // A MSG_ASSET_UNLOCK getdata identifies a version 2 asset unlock by its instance hash + if (IsAssetUnlockWithStableTxid(*tx)) { + most_recent_block_txs->emplace(tx->GetInstanceHash(), tx); + } + } + LOCK(m_most_recent_block_mutex); m_most_recent_block_hash = hashBlock; m_most_recent_block = pblock; m_most_recent_compact_block = pcmpctblock; + m_most_recent_block_txs = std::move(most_recent_block_txs); } m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { @@ -2987,13 +2990,17 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, } } - // Otherwise, the transaction must have been announced recently. - if (tx_relay.m_recently_announced_invs.contains(txid)) { - // If it was, it can be relayed from either the mempool... - if (txinfo.tx) return std::move(txinfo.tx); - // ... or the relay pool. - auto mi = mapRelay.find(txid); - if (mi != mapRelay.end()) return mi->second; + // Otherwise, the transaction might have been announced recently. + bool recent = tx_relay.m_recently_announced_invs.contains(txid); + if (recent && txinfo.tx) return std::move(txinfo.tx); + + // Or it might be from the most recent block + { + LOCK(m_most_recent_block_mutex); + if (m_most_recent_block_txs != nullptr) { + auto it = m_most_recent_block_txs->find(txid); + if (it != m_most_recent_block_txs->end()) return it->second; + } } return {}; @@ -6832,19 +6839,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Send tx_relay->m_recently_announced_invs.insert(announce_hash); nRelayedTransactions++; - { - // Expire old relay messages - while (!g_relay_expiration.empty() && g_relay_expiration.front().first < current_time) - { - mapRelay.erase(g_relay_expiration.front().second); - g_relay_expiration.pop_front(); - } - - auto ret = mapRelay.emplace(announce_hash, std::move(txinfo.tx)); - if (ret.second) { - g_relay_expiration.emplace_back(current_time + RELAY_TX_CACHE_TIME, ret.first); - } - } tx_relay->m_tx_inventory_known_filter.insert(announce_hash); queueAndMaybePushInv(CInv(nInvType, announce_hash)); } diff --git a/src/pubkey.h b/src/pubkey.h index 1ea26f84c9c7..a877e866c58a 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -141,14 +141,14 @@ class CPubKey { unsigned int len = size(); ::WriteCompactSize(s, len); - s.write(AsBytes(Span{vch, len})); + s << Span{vch, len}; } template void Unserialize(Stream& s) { const unsigned int len(::ReadCompactSize(s)); if (len <= SIZE) { - s.read(AsWritableBytes(Span{vch, len})); + s >> Span{vch, len}; if (len != size()) { Invalidate(); } diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 10953cf169e5..cf168942c6a0 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -84,7 +84,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial assert(msg.m_time == m_time); std::vector header; - auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_type, MakeUCharSpan(msg.m_recv)); + auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_type, Span{msg.m_recv}); bool queued = send_transport.SetMessageToSend(msg2); assert(queued); std::optional known_more; diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index df4c8bdd4933..890db816f3b0 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -239,32 +239,32 @@ BOOST_AUTO_TEST_CASE(noncanonical) std::vector::size_type n; // zero encoded with three bytes: - ss.write(MakeByteSpan("\xfd\x00\x00").first(3)); + ss << Span{"\xfd\x00\x00"}.first(3); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xfc encoded with three bytes: - ss.write(MakeByteSpan("\xfd\xfc\x00").first(3)); + ss << Span{"\xfd\xfc\x00"}.first(3); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xfd encoded with three bytes is OK: - ss.write(MakeByteSpan("\xfd\xfd\x00").first(3)); + ss << Span{"\xfd\xfd\x00"}.first(3); n = ReadCompactSize(ss); BOOST_CHECK(n == 0xfd); // zero encoded with five bytes: - ss.write(MakeByteSpan("\xfe\x00\x00\x00\x00").first(5)); + ss << Span{"\xfe\x00\x00\x00\x00"}.first(5); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xffff encoded with five bytes: - ss.write(MakeByteSpan("\xfe\xff\xff\x00\x00").first(5)); + ss << Span{"\xfe\xff\xff\x00\x00"}.first(5); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // zero encoded with nine bytes: - ss.write(MakeByteSpan("\xff\x00\x00\x00\x00\x00\x00\x00\x00").first(9)); + ss << Span{"\xff\x00\x00\x00\x00\x00\x00\x00\x00"}.first(9); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0x01ffffff encoded with nine bytes: - ss.write(MakeByteSpan("\xff\xff\xff\xff\x01\x00\x00\x00\x00").first(9)); + ss << Span{"\xff\xff\xff\xff\x01\x00\x00\x00\x00"}.first(9); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 26eb67e7b6e4..3f9323a22784 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -83,7 +83,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan } // descendants now contains all in-mempool descendants of updateIt. // Update and add to cached descendant map - int64_t modifySize = 0; + int32_t modifySize = 0; CAmount modifyFee = 0; int64_t modifyCount = 0; for (const CTxMemPoolEntry& descendant : descendants) { @@ -99,7 +99,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan // Don't directly remove the transaction here -- doing so would // invalidate iterators in cachedDescendants. Mark it for removal // by inserting into descendants_to_remove. - if (descendant.GetCountWithAncestors() > uint64_t(m_limits.ancestor_count) || descendant.GetSizeWithAncestors() > uint64_t(m_limits.ancestor_size_vbytes)) { + if (descendant.GetCountWithAncestors() > uint64_t(m_limits.ancestor_count) || descendant.GetSizeWithAncestors() > m_limits.ancestor_size_vbytes) { descendants_to_remove.insert(descendant.GetTx().GetHash()); } } @@ -284,8 +284,8 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors for (const CTxMemPoolEntry& parent : parents) { UpdateChild(mapTx.iterator_to(parent), it, add); } - const int64_t updateCount = (add ? 1 : -1); - const int64_t updateSize = updateCount * it->GetTxSize(); + const int32_t updateCount = (add ? 1 : -1); + const int32_t updateSize{updateCount * it->GetTxSize()}; const CAmount updateFee = updateCount * it->GetModifiedFee(); for (txiter ancestorIt : setAncestors) { mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(updateSize, updateFee, updateCount); }); @@ -329,7 +329,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b setEntries setDescendants; CalculateDescendants(removeIt, setDescendants); setDescendants.erase(removeIt); // don't update state for self - int64_t modifySize = -((int64_t)removeIt->GetTxSize()); + int32_t modifySize = -removeIt->GetTxSize(); CAmount modifyFee = -removeIt->GetModifiedFee(); int modifySigOps = -removeIt->GetSigOpCount(); for (txiter dit : setDescendants) { @@ -371,22 +371,22 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b } } -void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) +void CTxMemPoolEntry::UpdateDescendantState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount) { nSizeWithDescendants += modifySize; - assert(int64_t(nSizeWithDescendants) > 0); + assert(nSizeWithDescendants > 0); nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee); - nCountWithDescendants += modifyCount; - assert(int64_t(nCountWithDescendants) > 0); + m_count_with_descendants += modifyCount; + assert(m_count_with_descendants > 0); } -void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps) +void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps) { nSizeWithAncestors += modifySize; - assert(int64_t(nSizeWithAncestors) > 0); + assert(nSizeWithAncestors > 0); nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee); - nCountWithAncestors += modifyCount; - assert(int64_t(nCountWithAncestors) > 0); + m_count_with_ancestors += modifyCount; + assert(m_count_with_ancestors > 0); nSigOpCountWithAncestors += modifySigOps; assert(int(nSigOpCountWithAncestors) >= 0); } @@ -1449,7 +1449,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei // Verify ancestor state is correct. auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits())}; uint64_t nCountCheck = ancestors.size() + 1; - uint64_t nSizeCheck = it->GetTxSize(); + int32_t nSizeCheck = it->GetTxSize(); CAmount nFeesCheck = it->GetModifiedFee(); unsigned int nSigOpCheck = it->GetSigOpCount(); @@ -1470,7 +1470,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei // Check children against mapNextTx CTxMemPoolEntry::Children setChildrenCheck; auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); - uint64_t child_sizes = 0; + int32_t child_sizes{0}; for (; iter != mapNextTx.end() && iter->first->hash == it->GetTx().GetHash(); ++iter) { txiter childit = mapTx.find(iter->second->GetHash()); assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions diff --git a/src/txmempool.h b/src/txmempool.h index c2fea79b0c7f..212531a59f33 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -222,7 +222,7 @@ struct TxMempoolInfo CAmount fee; /** Virtual size of the transaction. */ - size_t vsize; + int32_t vsize; /** The fee delta. */ int64_t nFeeDelta; diff --git a/src/uint256.h b/src/uint256.h index e32612c5e911..db629dff2ea6 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -79,7 +79,7 @@ class base_blob template void Serialize(Stream& s) const { - s.write(MakeByteSpan(m_data)); + s << Span(m_data); } template diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 47c160fd64d5..6863c38f5c51 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -200,7 +200,7 @@ BOOST_AUTO_TEST_CASE(db_cursor_prefix_byte_test) for (const auto& database : TestDatabases(m_path_root)) { std::unique_ptr batch = database->MakeBatch(); for (const auto& [k, v] : {e, p, ps, f, fs, ff, ffs}) { - batch->Write(MakeUCharSpan(k), MakeUCharSpan(v)); + batch->Write(Span{k}, Span{v}); } CheckPrefix(*batch, StringBytes(""), {e, p, ps, f, fs, ff, ffs}); CheckPrefix(*batch, StringBytes("prefix"), {p, ps}); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 683a34e60293..0fb7fcf1a801 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4889,7 +4889,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) bool began = batch->TxnBegin(); assert(began); // This is a critical error, the new db could not be written to. The original db exists as a backup, but we should not continue execution. for (const auto& [key, value] : records) { - if (!batch->Write(MakeUCharSpan(key), MakeUCharSpan(value))) { + if (!batch->Write(Span{key}, Span{value})) { batch->TxnAbort(); m_database->Close(); fs::remove(m_database->Filename()); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index b8e7af1a7e26..7e1e0dbaa653 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1273,7 +1273,7 @@ bool WalletBatch::EraseRecords(const std::unordered_set& types) } // Make a copy of key to avoid data being deleted by the following read of the type - Span key_data = MakeUCharSpan(key); + Span key_data{key}; std::string type; key >> type; diff --git a/test/functional/interface_usdt_mempool.py b/test/functional/interface_usdt_mempool.py index 802153099f35..10b244150cc6 100755 --- a/test/functional/interface_usdt_mempool.py +++ b/test/functional/interface_usdt_mempool.py @@ -35,7 +35,7 @@ struct added_event { u8 hash[HASH_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; }; @@ -43,7 +43,7 @@ { u8 hash[HASH_LENGTH]; char reason[MAX_REMOVAL_REASON_LENGTH]; - u64 vsize; + s32 vsize; s64 fee; u64 entry_time; }; @@ -111,8 +111,7 @@ def added_test(self): """Add a transaction to the mempool and make sure the tracepoint returns the expected txid, vsize, and fee.""" - EXPECTED_ADDED_EVENTS = 1 - handled_added_events = 0 + events = [] self.log.info("Hooking into mempool:added tracepoint...") node = self.nodes[0] @@ -121,21 +120,13 @@ def added_test(self): bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0) def handle_added_event(_, data, __): - nonlocal handled_added_events - event = bpf["added_events"].event(data) - assert_equal(txid, bytes(event.hash)[::-1].hex()) - assert_equal(vsize, event.vsize) - assert_equal(fee, event.fee) - handled_added_events += 1 + events.append(bpf["added_events"].event(data)) bpf["added_events"].open_perf_buffer(handle_added_event) self.log.info("Sending transaction...") fee = Decimal(31200) tx = self.wallet.send_self_transfer(from_node=node, fee=fee / COIN) - # expected data - txid = tx["txid"] - vsize = tx["tx"].get_vsize() self.log.info("Polling buffer...") bpf.perf_buffer_poll(timeout=200) @@ -143,17 +134,20 @@ def handle_added_event(_, data, __): self.log.info("Cleaning up mempool...") self.generate(node, 1) - bpf.cleanup() - self.log.info("Ensuring mempool:added event was handled successfully...") - assert_equal(EXPECTED_ADDED_EVENTS, handled_added_events) + assert_equal(1, len(events)) + event = events[0] + assert_equal(bytes(event.hash)[::-1].hex(), tx["txid"]) + assert_equal(event.vsize, tx["tx"].get_vsize()) + assert_equal(event.fee, fee) + + bpf.cleanup() def removed_test(self): """Expire a transaction from the mempool and make sure the tracepoint returns the expected txid, expiry reason, vsize, and fee.""" - EXPECTED_REMOVED_EVENTS = 1 - handled_removed_events = 0 + events = [] self.log.info("Hooking into mempool:removed tracepoint...") node = self.nodes[0] @@ -162,24 +156,14 @@ def removed_test(self): bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0) def handle_removed_event(_, data, __): - nonlocal handled_removed_events - event = bpf["removed_events"].event(data) - assert_equal(txid, bytes(event.hash)[::-1].hex()) - assert_equal(reason, event.reason.decode("UTF-8")) - assert_equal(vsize, event.vsize) - assert_equal(fee, event.fee) - assert_equal(entry_time, event.entry_time) - handled_removed_events += 1 + events.append(bpf["removed_events"].event(data)) bpf["removed_events"].open_perf_buffer(handle_removed_event) self.log.info("Sending transaction...") fee = Decimal(31200) tx = self.wallet.send_self_transfer(from_node=node, fee=fee / COIN) - # expected data txid = tx["txid"] - reason = "expiry" - vsize = tx["tx"].get_vsize() self.log.info("Fast-forwarding time to mempool expiry...") entry_time = node.getmempoolentry(txid)["time"] @@ -193,17 +177,22 @@ def handle_removed_event(_, data, __): self.log.info("Polling buffer...") bpf.perf_buffer_poll(timeout=200) - bpf.cleanup() - self.log.info("Ensuring mempool:removed event was handled successfully...") - assert_equal(EXPECTED_REMOVED_EVENTS, handled_removed_events) + assert_equal(1, len(events)) + event = events[0] + assert_equal(bytes(event.hash)[::-1].hex(), txid) + assert_equal(event.reason.decode("UTF-8"), "expiry") + assert_equal(event.vsize, tx["tx"].get_vsize()) + assert_equal(event.fee, fee) + assert_equal(event.entry_time, entry_time) + + bpf.cleanup() def rejected_test(self): """Create an invalid transaction and make sure the tracepoint returns the expected txid, rejection reason, peer id, and peer address.""" - EXPECTED_REJECTED_EVENTS = 1 - handled_rejected_events = 0 + events = [] self.log.info("Adding P2P connection...") node = self.nodes[0] @@ -215,11 +204,7 @@ def rejected_test(self): bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0) def handle_rejected_event(_, data, __): - nonlocal handled_rejected_events - event = bpf["rejected_events"].event(data) - assert_equal(txid, bytes(event.hash)[::-1].hex()) - assert_equal(reason, event.reason.decode("UTF-8")) - handled_rejected_events += 1 + events.append(bpf["rejected_events"].event(data)) bpf["rejected_events"].open_perf_buffer(handle_rejected_event) @@ -227,17 +212,19 @@ def handle_rejected_event(_, data, __): tx = self.wallet.create_self_transfer(fee_rate=Decimal(0)) node.p2ps[0].send_txs_and_test([tx["tx"]], node, success=False) - # expected data - txid = tx["tx"].hash - reason = "min relay fee not met" - self.log.info("Polling buffer...") bpf.perf_buffer_poll(timeout=200) - bpf.cleanup() - self.log.info("Ensuring mempool:rejected event was handled successfully...") - assert_equal(EXPECTED_REJECTED_EVENTS, handled_rejected_events) + assert_equal(1, len(events)) + event = events[0] + assert_equal(bytes(event.hash)[::-1].hex(), tx["tx"].hash) + # The next test is already known to fail, so disable it to avoid + # wasting CPU time and developer time. See + # https://github.com/bitcoin/bitcoin/issues/27380 + #assert_equal(event.reason.decode("UTF-8"), "min relay fee not met") + + bpf.cleanup() def run_test(self): """Tests the mempool:added, mempool:removed, and mempool:rejected diff --git a/test/functional/interface_usdt_net.py b/test/functional/interface_usdt_net.py index c55a7976bff4..ca6151973660 100755 --- a/test/functional/interface_usdt_net.py +++ b/test/functional/interface_usdt_net.py @@ -121,11 +121,11 @@ def __repr__(self): checked_outbound_version_msg = 0 events = [] - def check_p2p_message(event, inbound): + def check_p2p_message(event, is_inbound): nonlocal checked_inbound_version_msg, checked_outbound_version_msg if event.msg_type.decode("utf-8") == "version": self.log.info( - f"check_p2p_message(): {'inbound' if inbound else 'outbound'} {event}") + f"check_p2p_message(): {'inbound' if is_inbound else 'outbound'} {event}") peer = self.nodes[0].getpeerinfo()[0] msg = msg_version() msg.deserialize(BytesIO(bytes(event.msg[:event.msg_size]))) @@ -133,13 +133,12 @@ def check_p2p_message(event, inbound): assert_equal(peer["addr"], event.peer_addr.decode("utf-8")) assert_equal(peer["connection_type"], event.peer_conn_type.decode("utf-8")) - if inbound: + if is_inbound: checked_inbound_version_msg += 1 else: checked_outbound_version_msg += 1 def handle_inbound(_, data, __): - nonlocal events event = ctypes.cast(data, ctypes.POINTER(P2PMessage)).contents events.append((event, True)) @@ -157,8 +156,8 @@ def handle_outbound(_, data, __): self.log.info( "check receipt and content of in- and outbound version messages") - for event, inbound in events: - check_p2p_message(event, inbound) + for event, is_inbound in events: + check_p2p_message(event, is_inbound) assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG, checked_inbound_version_msg) assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG, diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index e5103e184b73..28caffd19255 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -252,43 +252,30 @@ def test_add_spent(self): # that the handle_* functions succeeded. EXPECTED_HANDLE_ADD_SUCCESS = 2 EXPECTED_HANDLE_SPENT_SUCCESS = 1 - handle_add_succeeds = 0 - handle_spent_succeeds = 0 - expected_utxocache_spents = [] expected_utxocache_adds = [] + expected_utxocache_spents = [] + + actual_utxocache_adds = [] + actual_utxocache_spents = [] + + def compare_utxo_with_event(utxo, event): + """Compare a utxo dict to the event produced by BPF""" + assert_equal(utxo["txid"], bytes(event.txid[::-1]).hex()) + assert_equal(utxo["index"], event.index) + assert_equal(utxo["height"], event.height) + assert_equal(utxo["value"], event.value) + assert_equal(utxo["is_coinbase"], event.is_coinbase) def handle_utxocache_add(_, data, __): - nonlocal handle_add_succeeds event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents self.log.info(f"handle_utxocache_add(): {event}") - add = expected_utxocache_adds.pop(0) - try: - assert_equal(add["txid"], bytes(event.txid[::-1]).hex()) - assert_equal(add["index"], event.index) - assert_equal(add["height"], event.height) - assert_equal(add["value"], event.value) - assert_equal(add["is_coinbase"], event.is_coinbase) - except AssertionError: - self.log.exception("Assertion failed") - else: - handle_add_succeeds += 1 + actual_utxocache_adds.append(event) def handle_utxocache_spent(_, data, __): - nonlocal handle_spent_succeeds event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents self.log.info(f"handle_utxocache_spent(): {event}") - spent = expected_utxocache_spents.pop(0) - try: - assert_equal(spent["txid"], bytes(event.txid[::-1]).hex()) - assert_equal(spent["index"], event.index) - assert_equal(spent["height"], event.height) - assert_equal(spent["value"], event.value) - assert_equal(spent["is_coinbase"], event.is_coinbase) - except AssertionError: - self.log.exception("Assertion failed") - else: - handle_spent_succeeds += 1 + actual_utxocache_spents.append(event) bpf["utxocache_add"].open_perf_buffer(handle_utxocache_add) bpf["utxocache_spent"].open_perf_buffer(handle_utxocache_spent) @@ -324,19 +311,18 @@ def handle_utxocache_spent(_, data, __): "is_coinbase": block_index == 0, }) - assert_equal(EXPECTED_HANDLE_ADD_SUCCESS, len(expected_utxocache_adds)) - assert_equal(EXPECTED_HANDLE_SPENT_SUCCESS, - len(expected_utxocache_spents)) - bpf.perf_buffer_poll(timeout=200) - bpf.cleanup() + + assert_equal(EXPECTED_HANDLE_ADD_SUCCESS, len(expected_utxocache_adds), len(actual_utxocache_adds)) + assert_equal(EXPECTED_HANDLE_SPENT_SUCCESS, len(expected_utxocache_spents), len(actual_utxocache_spents)) self.log.info( f"check that we successfully traced {EXPECTED_HANDLE_ADD_SUCCESS} adds and {EXPECTED_HANDLE_SPENT_SUCCESS} spent") - assert_equal(0, len(expected_utxocache_adds)) - assert_equal(0, len(expected_utxocache_spents)) - assert_equal(EXPECTED_HANDLE_ADD_SUCCESS, handle_add_succeeds) - assert_equal(EXPECTED_HANDLE_SPENT_SUCCESS, handle_spent_succeeds) + for expected_utxo, actual_event in zip(expected_utxocache_adds + expected_utxocache_spents, + actual_utxocache_adds + actual_utxocache_spents): + compare_utxo_with_event(expected_utxo, actual_event) + + bpf.cleanup() def test_flush(self): """ Tests the utxocache:flush tracepoint API. @@ -367,9 +353,13 @@ def handle_utxocache_flush(_, data, __): "size": event.size }) # sanity checks only - assert event.memory > 0 - assert event.duration > 0 - handle_flush_succeeds += 1 + try: + assert event.memory > 0 + assert event.duration > 0 + except AssertionError: + self.log.exception("Assertion error") + else: + handle_flush_succeeds += 1 bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) diff --git a/test/functional/interface_usdt_validation.py b/test/functional/interface_usdt_validation.py index 88dfd4d53e57..d02ca9df5354 100755 --- a/test/functional/interface_usdt_validation.py +++ b/test/functional/interface_usdt_validation.py @@ -86,7 +86,6 @@ def __repr__(self): self.duration) BLOCKS_EXPECTED = 2 - blocks_checked = 0 expected_blocks = dict() events = [] @@ -98,20 +97,9 @@ def __repr__(self): usdt_contexts=[ctx], debug=0, cflags=["-Wno-error=implicit-function-declaration"]) def handle_blockconnected(_, data, __): - nonlocal events, blocks_checked event = ctypes.cast(data, ctypes.POINTER(Block)).contents self.log.info(f"handle_blockconnected(): {event}") - block_hash = bytes(event.hash[::-1]).hex() - block = expected_blocks[block_hash] - assert_equal(block["hash"], block_hash) - assert_equal(block["height"], event.height) - assert_equal(len(block["tx"]), event.transactions) - assert_equal(len([tx["vin"] for tx in block["tx"]]), event.inputs) - assert_equal(0, event.sigops) # no sigops in coinbase tx - # only plausibility checks - assert event.duration > 0 - del expected_blocks[block_hash] - blocks_checked += 1 + events.append(event) bpf["block_connected"].open_perf_buffer( handle_blockconnected) @@ -136,7 +124,7 @@ def handle_blockconnected(_, data, __): # only plausibility checks assert event.duration > 0 del expected_blocks[block_hash] - assert_equal(BLOCKS_EXPECTED, blocks_checked) + assert_equal(BLOCKS_EXPECTED, len(events)) assert_equal(0, len(expected_blocks)) bpf.cleanup() diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py index 69b6246c30c2..65a6925a8db8 100755 --- a/test/functional/p2p_leak_tx.py +++ b/test/functional/p2p_leak_tx.py @@ -48,6 +48,7 @@ def test_tx_in_block(self): def test_notfound_on_unannounced_tx(self): self.log.info("Check that we don't leak txs to inbound peers that we haven't yet announced to") + self.gen_node.disconnect_p2ps() inbound_peer = self.gen_node.add_p2p_connection(P2PNode()) # An "attacking" inbound peer MAX_REPEATS = 100 diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index c20a58eec210..518a7447034f 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -62,7 +62,6 @@ unsigned-integer-overflow:GetQuorumQuarterMembersBySnapshot unsigned-integer-overflow:policy/fees.cpp unsigned-integer-overflow:prevector.h unsigned-integer-overflow:EvalScript -unsigned-integer-overflow:txmempool.cpp unsigned-integer-overflow:xoroshiro128plusplus.h unsigned-integer-overflow:bitset_detail::PopCount implicit-integer-sign-change:addrman.h @@ -76,7 +75,6 @@ implicit-integer-sign-change:prevector.h implicit-integer-sign-change:script/bitcoinconsensus.cpp implicit-integer-sign-change:script/interpreter.cpp implicit-integer-sign-change:serialize.h -implicit-integer-sign-change:txmempool.cpp implicit-integer-sign-change:util/strencodings.cpp implicit-integer-sign-change:util/strencodings.h implicit-integer-sign-change:validation.cpp