From f128e67ea3d21874773e6e2081f035ea366877a4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 23 Jun 2023 18:11:08 -0400 Subject: [PATCH 01/13] Merge bitcoin/bitcoin#26969: net, refactor: net_processing, add `ProcessCompactBlockTxns` 77d6d89d43cc5969c98d9b4b56a1e877b473e731 net: net_processing, add `ProcessCompactBlockTxns` (brunoerg) Pull request description: When processing `CMPCTBLOCK` message, at some moments we can need to process compact block txns / `BLOCKTXN`, since all messages are handled by `ProcessMessage`, so we call `ProcessMessage` all over again. https://github.com/bitcoin/bitcoin/blob/ab98673f058853e00c310afad57925f54c1ecfae/src/net_processing.cpp#L4331-L4348 This PR creates a function called `ProcessCompactBlockTxns` to process it to avoid calling `ProcessMessage` for it - this function is also called when processing `BLOCKTXN` msg. ACKs for top commit: instagibbs: reACK 77d6d89d43cc5969c98d9b4b56a1e877b473e731 ajtowns: utACK 77d6d89d43cc5969c98d9b4b56a1e877b473e731 achow101: ACK 77d6d89d43cc5969c98d9b4b56a1e877b473e731 Tree-SHA512: 4b73c189487b999a04a8f15608a2ac1966d0f5c6db3ae0782641e68b9e95cb0807bd065d124c1f316b25b04d522a765addcd7d82c541702695113d4e54db4fda --- src/net_processing.cpp | 175 ++++++++++++++++++++++------------------- 1 file changed, 96 insertions(+), 79 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2816e0ec91a6..e48fa3a13e16 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3706,12 +3706,96 @@ MessageProcessingResult PeerManagerImpl::ProcessPlatformBanMessage(NodeId node, return ret; } -void PeerManagerImpl::ProcessMessage( - CNode& pfrom, - const std::string& msg_type, - CDataStream& vRecv, - const std::chrono::microseconds time_received, - const std::atomic& interruptMsgProc) +void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const BlockTransactions& block_transactions) +{ + std::shared_ptr pblock = std::make_shared(); + bool fBlockRead{false}; + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); + { + LOCK(cs_main); + + auto range_flight = mapBlocksInFlight.equal_range(block_transactions.blockhash); + size_t already_in_flight = std::distance(range_flight.first, range_flight.second); + bool requested_block_from_this_peer{false}; + + // Multimap ensures ordering of outstanding requests. It's either empty or first in line. + bool first_in_flight = already_in_flight == 0 || (range_flight.first->second.first == pfrom.GetId()); + + while (range_flight.first != range_flight.second) { + auto [node_id, block_it] = range_flight.first->second; + if (node_id == pfrom.GetId() && block_it->partialBlock) { + requested_block_from_this_peer = true; + break; + } + range_flight.first++; + } + + if (!requested_block_from_this_peer) { + LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId()); + return; + } + + PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock; + ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn); + if (status == READ_STATUS_INVALID) { + RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect + Misbehaving(peer, 100, "invalid compact block/non-matching block transactions"); + return; + } else if (status == READ_STATUS_FAILED) { + if (first_in_flight) { + // Might have collided, fall back to getdata now :( + std::vector invs; + invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); + } else { + RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); + LogPrint(BCLog::NET, "Peer %d sent us a compact block but it failed to reconstruct, waiting on first download to complete\n", pfrom.GetId()); + return; + } + } else { + // Block is either okay, or possibly we received + // READ_STATUS_CHECKBLOCK_FAILED. + // Note that CheckBlock can only fail for one of a few reasons: + // 1. bad-proof-of-work (impossible here, because we've already + // accepted the header) + // 2. merkleroot doesn't match the transactions given (already + // caught in FillBlock with READ_STATUS_FAILED, so + // impossible here) + // 3. the block is otherwise invalid (eg invalid coinbase, + // block is too big, too many legacy sigops, etc). + // So if CheckBlock failed, #3 is the only possibility. + // Under BIP 152, we don't discourage the peer unless proof of work is + // invalid (we don't require all the stateless checks to have + // been run). This is handled below, so just treat this as + // though the block was successfully read, and rely on the + // handling in ProcessNewBlock to ensure the block index is + // updated, etc. + RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // it is now an empty pointer + fBlockRead = true; + // mapBlockSource is used for potentially punishing peers and + // updating which peers send us compact blocks, so the race + // between here and cs_main in ProcessNewBlock is fine. + // BIP 152 permits peers to relay compact blocks after validating + // the header only; we should not punish peers if the block turns + // out to be invalid. + mapBlockSource.emplace(block_transactions.blockhash, std::make_pair(pfrom.GetId(), false)); + } + } // Don't hold cs_main when we call into ProcessNewBlock + if (fBlockRead) { + // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, + // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) + // This bypasses some anti-DoS logic in AcceptBlock (eg to prevent + // disk-space attacks), but this should be safe due to the + // protections in the compact block handler -- see related comment + // in compact block optimistic reconstruction handling. + ProcessBlock(pfrom, pblock, /*force_processing=*/true, /*min_pow_checked=*/true); + } + return; +} + +void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, + const std::chrono::microseconds time_received, + const std::atomic& interruptMsgProc) { AssertLockHeld(g_msgproc_mutex); @@ -4832,12 +4916,7 @@ void PeerManagerImpl::ProcessMessage( blockhash.ToString(), pfrom.GetId()); } - // When we succeed in decoding a block's txids from a cmpctblock - // message we typically jump to the BLOCKTXN handling code, with a - // dummy (empty) BLOCKTXN message, to re-use the logic there in - // completing processing of the putative block (without cs_main). bool fProcessBLOCKTXN = false; - CDataStream blockTxnMsg(SER_NETWORK, PROTOCOL_VERSION); // If we end up treating this as a plain headers message, call that as well // without cs_main. @@ -4920,10 +4999,6 @@ void PeerManagerImpl::ProcessMessage( req.indexes.push_back(i); } if (req.indexes.empty()) { - // Dirty hack to jump to BLOCKTXN code (TODO: move message handling into their own functions) - BlockTransactions txn; - txn.blockhash = blockhash; - blockTxnMsg << txn; fProcessBLOCKTXN = true; } else { req.blockhash = pindex->GetBlockHash(); @@ -4962,8 +5037,11 @@ void PeerManagerImpl::ProcessMessage( } } // cs_main - if (fProcessBLOCKTXN) - return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, interruptMsgProc); + if (fProcessBLOCKTXN) { + BlockTransactions txn; + txn.blockhash = blockhash; + return ProcessCompactBlockTxns(pfrom, *peer, txn); + } if (fRevertToHeaderProcessing) { // Headers received from HB compact block peers are permitted to be @@ -5014,68 +5092,7 @@ void PeerManagerImpl::ProcessMessage( BlockTransactions resp; vRecv >> resp; - std::shared_ptr pblock = std::make_shared(); - bool fBlockRead = false; - { - LOCK(cs_main); - - std::map::iterator> >::iterator it = mapBlocksInFlight.find(resp.blockhash); - if (it == mapBlocksInFlight.end() || !it->second.second->partialBlock || - it->second.first != pfrom.GetId()) { - LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId()); - return; - } - - PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; - ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); - if (status == READ_STATUS_INVALID) { - RemoveBlockRequest(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); - return; - } else if (status == READ_STATUS_FAILED) { - // Might have collided, fall back to getdata now :( - std::vector invs; - invs.push_back(CInv(MSG_BLOCK, resp.blockhash)); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); - } else { - // Block is either okay, or possibly we received - // READ_STATUS_CHECKBLOCK_FAILED. - // Note that CheckBlock can only fail for one of a few reasons: - // 1. bad-proof-of-work (impossible here, because we've already - // accepted the header) - // 2. merkleroot doesn't match the transactions given (already - // caught in FillBlock with READ_STATUS_FAILED, so - // impossible here) - // 3. the block is otherwise invalid (eg invalid coinbase, - // block is too big, too many legacy sigops, etc). - // So if CheckBlock failed, #3 is the only possibility. - // Under BIP 152, we don't discourage the peer unless proof of work is - // invalid (we don't require all the stateless checks to have - // been run). This is handled below, so just treat this as - // though the block was successfully read, and rely on the - // handling in ProcessNewBlock to ensure the block index is - // updated, etc. - RemoveBlockRequest(resp.blockhash); // it is now an empty pointer - fBlockRead = true; - // mapBlockSource is used for potentially punishing peers and - // updating which peers send us compact blocks, so the race - // between here and cs_main in ProcessNewBlock is fine. - // BIP 152 permits peers to relay compact blocks after validating - // the header only; we should not punish peers if the block turns - // out to be invalid. - mapBlockSource.emplace(resp.blockhash, std::make_pair(pfrom.GetId(), false)); - } - } // Don't hold cs_main when we call into ProcessNewBlock - if (fBlockRead) { - // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, - // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) - // This bypasses some anti-DoS logic in AcceptBlock (eg to prevent - // disk-space attacks), but this should be safe due to the - // protections in the compact block handler -- see related comment - // in compact block optimistic reconstruction handling. - ProcessBlock(pfrom, pblock, /*force_processing=*/true); - } - return; + return ProcessCompactBlockTxns(pfrom, *peer, resp); } if (msg_type == NetMsgType::HEADERS || msg_type == NetMsgType::HEADERS2) { From fa4fd88e7e272ad21a3145b91414f16bfd172927 Mon Sep 17 00:00:00 2001 From: Shailendra Kumar Gupta Date: Thu, 5 Feb 2026 21:48:03 +0800 Subject: [PATCH 02/13] Revert "Merge bitcoin/bitcoin#26969: net, refactor: net_processing, add `ProcessCompactBlockTxns`" This reverts commit f128e67ea3d21874773e6e2081f035ea366877a4. --- src/net_processing.cpp | 175 +++++++++++++++++++---------------------- 1 file changed, 79 insertions(+), 96 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e48fa3a13e16..2816e0ec91a6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3706,96 +3706,12 @@ MessageProcessingResult PeerManagerImpl::ProcessPlatformBanMessage(NodeId node, return ret; } -void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const BlockTransactions& block_transactions) -{ - std::shared_ptr pblock = std::make_shared(); - bool fBlockRead{false}; - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - { - LOCK(cs_main); - - auto range_flight = mapBlocksInFlight.equal_range(block_transactions.blockhash); - size_t already_in_flight = std::distance(range_flight.first, range_flight.second); - bool requested_block_from_this_peer{false}; - - // Multimap ensures ordering of outstanding requests. It's either empty or first in line. - bool first_in_flight = already_in_flight == 0 || (range_flight.first->second.first == pfrom.GetId()); - - while (range_flight.first != range_flight.second) { - auto [node_id, block_it] = range_flight.first->second; - if (node_id == pfrom.GetId() && block_it->partialBlock) { - requested_block_from_this_peer = true; - break; - } - range_flight.first++; - } - - if (!requested_block_from_this_peer) { - LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId()); - return; - } - - PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock; - ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn); - if (status == READ_STATUS_INVALID) { - RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(peer, 100, "invalid compact block/non-matching block transactions"); - return; - } else if (status == READ_STATUS_FAILED) { - if (first_in_flight) { - // Might have collided, fall back to getdata now :( - std::vector invs; - invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash)); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); - } else { - RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); - LogPrint(BCLog::NET, "Peer %d sent us a compact block but it failed to reconstruct, waiting on first download to complete\n", pfrom.GetId()); - return; - } - } else { - // Block is either okay, or possibly we received - // READ_STATUS_CHECKBLOCK_FAILED. - // Note that CheckBlock can only fail for one of a few reasons: - // 1. bad-proof-of-work (impossible here, because we've already - // accepted the header) - // 2. merkleroot doesn't match the transactions given (already - // caught in FillBlock with READ_STATUS_FAILED, so - // impossible here) - // 3. the block is otherwise invalid (eg invalid coinbase, - // block is too big, too many legacy sigops, etc). - // So if CheckBlock failed, #3 is the only possibility. - // Under BIP 152, we don't discourage the peer unless proof of work is - // invalid (we don't require all the stateless checks to have - // been run). This is handled below, so just treat this as - // though the block was successfully read, and rely on the - // handling in ProcessNewBlock to ensure the block index is - // updated, etc. - RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // it is now an empty pointer - fBlockRead = true; - // mapBlockSource is used for potentially punishing peers and - // updating which peers send us compact blocks, so the race - // between here and cs_main in ProcessNewBlock is fine. - // BIP 152 permits peers to relay compact blocks after validating - // the header only; we should not punish peers if the block turns - // out to be invalid. - mapBlockSource.emplace(block_transactions.blockhash, std::make_pair(pfrom.GetId(), false)); - } - } // Don't hold cs_main when we call into ProcessNewBlock - if (fBlockRead) { - // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, - // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) - // This bypasses some anti-DoS logic in AcceptBlock (eg to prevent - // disk-space attacks), but this should be safe due to the - // protections in the compact block handler -- see related comment - // in compact block optimistic reconstruction handling. - ProcessBlock(pfrom, pblock, /*force_processing=*/true, /*min_pow_checked=*/true); - } - return; -} - -void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, - const std::chrono::microseconds time_received, - const std::atomic& interruptMsgProc) +void PeerManagerImpl::ProcessMessage( + CNode& pfrom, + const std::string& msg_type, + CDataStream& vRecv, + const std::chrono::microseconds time_received, + const std::atomic& interruptMsgProc) { AssertLockHeld(g_msgproc_mutex); @@ -4916,7 +4832,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, blockhash.ToString(), pfrom.GetId()); } + // When we succeed in decoding a block's txids from a cmpctblock + // message we typically jump to the BLOCKTXN handling code, with a + // dummy (empty) BLOCKTXN message, to re-use the logic there in + // completing processing of the putative block (without cs_main). bool fProcessBLOCKTXN = false; + CDataStream blockTxnMsg(SER_NETWORK, PROTOCOL_VERSION); // If we end up treating this as a plain headers message, call that as well // without cs_main. @@ -4999,6 +4920,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, req.indexes.push_back(i); } if (req.indexes.empty()) { + // Dirty hack to jump to BLOCKTXN code (TODO: move message handling into their own functions) + BlockTransactions txn; + txn.blockhash = blockhash; + blockTxnMsg << txn; fProcessBLOCKTXN = true; } else { req.blockhash = pindex->GetBlockHash(); @@ -5037,11 +4962,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } } // cs_main - if (fProcessBLOCKTXN) { - BlockTransactions txn; - txn.blockhash = blockhash; - return ProcessCompactBlockTxns(pfrom, *peer, txn); - } + if (fProcessBLOCKTXN) + return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, interruptMsgProc); if (fRevertToHeaderProcessing) { // Headers received from HB compact block peers are permitted to be @@ -5092,7 +5014,68 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, BlockTransactions resp; vRecv >> resp; - return ProcessCompactBlockTxns(pfrom, *peer, resp); + std::shared_ptr pblock = std::make_shared(); + bool fBlockRead = false; + { + LOCK(cs_main); + + std::map::iterator> >::iterator it = mapBlocksInFlight.find(resp.blockhash); + if (it == mapBlocksInFlight.end() || !it->second.second->partialBlock || + it->second.first != pfrom.GetId()) { + LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId()); + return; + } + + PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; + ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); + if (status == READ_STATUS_INVALID) { + RemoveBlockRequest(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect + Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); + return; + } else if (status == READ_STATUS_FAILED) { + // Might have collided, fall back to getdata now :( + std::vector invs; + invs.push_back(CInv(MSG_BLOCK, resp.blockhash)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); + } else { + // Block is either okay, or possibly we received + // READ_STATUS_CHECKBLOCK_FAILED. + // Note that CheckBlock can only fail for one of a few reasons: + // 1. bad-proof-of-work (impossible here, because we've already + // accepted the header) + // 2. merkleroot doesn't match the transactions given (already + // caught in FillBlock with READ_STATUS_FAILED, so + // impossible here) + // 3. the block is otherwise invalid (eg invalid coinbase, + // block is too big, too many legacy sigops, etc). + // So if CheckBlock failed, #3 is the only possibility. + // Under BIP 152, we don't discourage the peer unless proof of work is + // invalid (we don't require all the stateless checks to have + // been run). This is handled below, so just treat this as + // though the block was successfully read, and rely on the + // handling in ProcessNewBlock to ensure the block index is + // updated, etc. + RemoveBlockRequest(resp.blockhash); // it is now an empty pointer + fBlockRead = true; + // mapBlockSource is used for potentially punishing peers and + // updating which peers send us compact blocks, so the race + // between here and cs_main in ProcessNewBlock is fine. + // BIP 152 permits peers to relay compact blocks after validating + // the header only; we should not punish peers if the block turns + // out to be invalid. + mapBlockSource.emplace(resp.blockhash, std::make_pair(pfrom.GetId(), false)); + } + } // Don't hold cs_main when we call into ProcessNewBlock + if (fBlockRead) { + // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, + // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) + // This bypasses some anti-DoS logic in AcceptBlock (eg to prevent + // disk-space attacks), but this should be safe due to the + // protections in the compact block handler -- see related comment + // in compact block optimistic reconstruction handling. + ProcessBlock(pfrom, pblock, /*force_processing=*/true); + } + return; } if (msg_type == NetMsgType::HEADERS || msg_type == NetMsgType::HEADERS2) { From d7daf6929b6723c7fc4c1ef9bebcc60d53708c06 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 10 May 2023 09:52:59 +0100 Subject: [PATCH 03/13] Merge bitcoin/bitcoin#27608: p2p: Avoid prematurely clearing download state for other peers 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar) Pull request description: Avoid letting one peer send us data that clears out the download request (and related timers etc) from another peer. The exception is if a block is definitely stored to disk, in which case we'll clear the download state (just as we do for compact blocks). ACKs for top commit: jamesob: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 ([`jamesob/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl`](https://github.com/jamesob/bitcoin/tree/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl)) instagibbs: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 fjahr: Code review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 mzumsande: Code Review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 Tree-SHA512: 3ee92507edc3303c16c70ca44ba6c28c104afe95196e4b9167032590ed23d4f569f654f8eb8758940bd6536bc9ca810d2a77d2739db386b927e8b3f3cf55cb16 --- src/net_processing.cpp | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2816e0ec91a6..425ee525f426 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1233,7 +1233,7 @@ bool PeerManagerImpl::IsBlockRequested(const uint256& hash) return mapBlocksInFlight.find(hash) != mapBlocksInFlight.end(); } -void PeerManagerImpl::RemoveBlockRequest(const uint256& hash) +void PeerManagerImpl::RemoveBlockRequest(const uint256& hash, std::optional from_peer) { auto it = mapBlocksInFlight.find(hash); if (it == mapBlocksInFlight.end()) { @@ -1242,6 +1242,12 @@ void PeerManagerImpl::RemoveBlockRequest(const uint256& hash) } auto [node_id, list_it] = it->second; + + if (from_peer && node_id != *from_peer) { + // Block was requested by another peer + return; + } + CNodeState *state = State(node_id); assert(state != nullptr); @@ -1277,7 +1283,7 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st } // Make sure it's not listed somewhere already. - RemoveBlockRequest(hash); + RemoveBlockRequest(hash, std::nullopt); std::list::iterator it = state->vBlocksInFlight.insert(state->vBlocksInFlight.end(), {&block, std::unique_ptr(pit ? new PartiallyDownloadedBlock(&m_mempool) : nullptr)}); @@ -3609,6 +3615,11 @@ void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr(); + // In case this block came from a different peer than we requested + // from, we can erase the block request now anyway (as we just stored + // this block to disk). + LOCK(cs_main); + RemoveBlockRequest(block->GetHash(), std::nullopt); } else { LOCK(cs_main); mapBlockSource.erase(block->GetHash()); @@ -4903,8 +4914,8 @@ void PeerManagerImpl::ProcessMessage( PartiallyDownloadedBlock& partialBlock = *(*queuedBlockIt)->partialBlock; ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { - RemoveBlockRequest(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, "invalid compact block"); + RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect + Misbehaving(*peer, 100, "invalid compact block"); return; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it @@ -4997,7 +5008,7 @@ void PeerManagerImpl::ProcessMessage( // process from some other peer. We do this after calling // ProcessNewBlock so that a malleated cmpctblock announcement // can't be used to interfere with block relay. - RemoveBlockRequest(pblock->GetHash()); + RemoveBlockRequest(pblock->GetHash(), std::nullopt); } } return; @@ -5029,8 +5040,8 @@ void PeerManagerImpl::ProcessMessage( PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { - RemoveBlockRequest(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); + RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect + Misbehaving(*peer, 100, "invalid compact block/non-matching block transactions"); return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( @@ -5055,7 +5066,7 @@ void PeerManagerImpl::ProcessMessage( // though the block was successfully read, and rely on the // handling in ProcessNewBlock to ensure the block index is // updated, etc. - RemoveBlockRequest(resp.blockhash); // it is now an empty pointer + RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // it is now an empty pointer fBlockRead = true; // mapBlockSource is used for potentially punishing peers and // updating which peers send us compact blocks, so the race @@ -5137,7 +5148,7 @@ void PeerManagerImpl::ProcessMessage( // Always process the block if we requested it, since we may // need it even when it's not a candidate for a new best tip. forceProcessing = IsBlockRequested(hash); - RemoveBlockRequest(hash); + RemoveBlockRequest(hash, pfrom.GetId()); // mapBlockSource is only used for punishing peers and setting // which peers send us compact blocks, so the race between here and // cs_main in ProcessNewBlock is fine. From fee05987fffe8309c0ab03875f86d5534b529e9d Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 10 May 2023 09:52:59 +0100 Subject: [PATCH 04/13] Merge bitcoin/bitcoin#27608: p2p: Avoid prematurely clearing download state for other peers 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar) Pull request description: Avoid letting one peer send us data that clears out the download request (and related timers etc) from another peer. The exception is if a block is definitely stored to disk, in which case we'll clear the download state (just as we do for compact blocks). ACKs for top commit: jamesob: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 ([`jamesob/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl`](https://github.com/jamesob/bitcoin/tree/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl)) instagibbs: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 fjahr: Code review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 mzumsande: Code Review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 Tree-SHA512: 3ee92507edc3303c16c70ca44ba6c28c104afe95196e4b9167032590ed23d4f569f654f8eb8758940bd6536bc9ca810d2a77d2739db386b927e8b3f3cf55cb16 --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 425ee525f426..dbf17c94bf94 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1035,7 +1035,7 @@ class PeerManagerImpl final : public PeerManager * - the block has been recieved from a peer * - the request for the block has timed out */ - void RemoveBlockRequest(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void RemoveBlockRequest(const uint256& hash, std::optional from_peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /* Mark a block as in flight * Returns false, still setting pit, if the block was already in flight from the same peer From ca348fa79d40e8472cd1d52b5d80c8c8f2b9c285 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 10 May 2023 09:52:59 +0100 Subject: [PATCH 05/13] Merge bitcoin/bitcoin#27608: p2p: Avoid prematurely clearing download state for other peers 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar) Pull request description: Avoid letting one peer send us data that clears out the download request (and related timers etc) from another peer. The exception is if a block is definitely stored to disk, in which case we'll clear the download state (just as we do for compact blocks). ACKs for top commit: jamesob: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 ([`jamesob/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl`](https://github.com/jamesob/bitcoin/tree/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl)) instagibbs: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 fjahr: Code review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 mzumsande: Code Review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 Tree-SHA512: 3ee92507edc3303c16c70ca44ba6c28c104afe95196e4b9167032590ed23d4f569f654f8eb8758940bd6536bc9ca810d2a77d2739db386b927e8b3f3cf55cb16 --- src/net_processing.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index dbf17c94bf94..25485c085608 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4915,7 +4915,7 @@ void PeerManagerImpl::ProcessMessage( ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(*peer, 100, "invalid compact block"); + Misbehaving(pfrom.GetId(), 100, "invalid compact block"); return; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it @@ -5041,7 +5041,7 @@ void PeerManagerImpl::ProcessMessage( ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(*peer, 100, "invalid compact block/non-matching block transactions"); + Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( From 469aa99ef6ac586dc6ddee1d9cd3f18273cb9e32 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 10 May 2023 09:52:59 +0100 Subject: [PATCH 06/13] Merge bitcoin/bitcoin#27608: p2p: Avoid prematurely clearing download state for other peers 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 p2p: Avoid prematurely clearing download state for other peers (Suhas Daftuar) Pull request description: Avoid letting one peer send us data that clears out the download request (and related timers etc) from another peer. The exception is if a block is definitely stored to disk, in which case we'll clear the download state (just as we do for compact blocks). ACKs for top commit: jamesob: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 ([`jamesob/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl`](https://github.com/jamesob/bitcoin/tree/ackr/27608.1.sdaftuar.p2p_avoid_prematurely_cl)) instagibbs: ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 fjahr: Code review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 mzumsande: Code Review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4 Tree-SHA512: 3ee92507edc3303c16c70ca44ba6c28c104afe95196e4b9167032590ed23d4f569f654f8eb8758940bd6536bc9ca810d2a77d2739db386b927e8b3f3cf55cb16 --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 25485c085608..2700927318e6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5041,7 +5041,7 @@ void PeerManagerImpl::ProcessMessage( ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); + Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); // Test return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( From 58197b2586d341b93c32b8b57ff6a107e769170c Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 May 2023 14:57:27 +0100 Subject: [PATCH 07/13] Merge bitcoin/bitcoin#27594: refactor: Remove unused GetTimeMillis fae1d9cdede0e88cabbf8130869be31276457c34 refactor: Remove unused GetTimeMillis (MarcoFalke) Pull request description: The function is unused, not type-safe, and does not denote the underlying clock type. So remove it. ACKs for top commit: willcl-ark: tACK fae1d9cded Tree-SHA512: 41ea7125d1964192b85a94265be974d02bf1e79b1feb61bff11486dc0ac811745156940ec5cad2ad1f94b653936f8ae563c959c1c4142203a55645fcb83203e8 --- src/util/time.cpp | 12 ------------ src/util/time.h | 2 -- 2 files changed, 14 deletions(-) diff --git a/src/util/time.cpp b/src/util/time.cpp index bab8759b90bd..b7e1c40f60d5 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -35,13 +35,6 @@ NodeClock::time_point NodeClock::now() noexcept return time_point{ret}; }; -template -static T GetSystemTime() -{ - const auto now = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()); - assert(now.count() > 0); - return now; -} void SetMockTime(int64_t nMockTimeIn) { SetMockTime(std::chrono::seconds{nMockTimeIn}); } void SetMockTime(std::chrono::seconds mock_time_in) @@ -55,11 +48,6 @@ std::chrono::seconds GetMockTime() return g_mock_time.load(std::memory_order_relaxed); } -int64_t GetTimeMicros() -{ - return int64_t{GetSystemTime().count()}; -} - int64_t GetTime() { return GetTime().count(); } std::string FormatISO8601DateTime(int64_t nTime) diff --git a/src/util/time.h b/src/util/time.h index 99e1fc77e9f2..1cf91cd499ad 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -104,8 +104,6 @@ using SecondsDouble = std::chrono::duration Date: Fri, 13 Feb 2026 14:12:13 +0530 Subject: [PATCH 08/13] Revert "Merge bitcoin/bitcoin#27594: refactor: Remove unused GetTimeMillis" This reverts commit 58197b2586d341b93c32b8b57ff6a107e769170c. --- src/util/time.cpp | 12 ++++++++++++ src/util/time.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/src/util/time.cpp b/src/util/time.cpp index b7e1c40f60d5..bab8759b90bd 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -35,6 +35,13 @@ NodeClock::time_point NodeClock::now() noexcept return time_point{ret}; }; +template +static T GetSystemTime() +{ + const auto now = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()); + assert(now.count() > 0); + return now; +} void SetMockTime(int64_t nMockTimeIn) { SetMockTime(std::chrono::seconds{nMockTimeIn}); } void SetMockTime(std::chrono::seconds mock_time_in) @@ -48,6 +55,11 @@ std::chrono::seconds GetMockTime() return g_mock_time.load(std::memory_order_relaxed); } +int64_t GetTimeMicros() +{ + return int64_t{GetSystemTime().count()}; +} + int64_t GetTime() { return GetTime().count(); } std::string FormatISO8601DateTime(int64_t nTime) diff --git a/src/util/time.h b/src/util/time.h index 1cf91cd499ad..99e1fc77e9f2 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -104,6 +104,8 @@ using SecondsDouble = std::chrono::duration Date: Mon, 16 Oct 2023 14:56:54 +0100 Subject: [PATCH 09/13] Merge bitcoin/bitcoin#28583: refactor: [tidy] modernize-use-emplace fa05a726c225dc65dee79367bb67f099ae4f99e6 tidy: modernize-use-emplace (MarcoFalke) Pull request description: Constructing a temporary unnamed object only to copy or move it into a container seems both verbose in code and a strict performance penalty. Fix both issues via the `modernize-use-emplace` tidy check. ACKs for top commit: Sjors: re-utACK fa05a726c2 hebasto: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6. TheCharlatan: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6 Tree-SHA512: 4408a094f406e7bf6c1468c2b0798f68f4d952a1253cf5b20bdc648ad7eea4a2c070051fed46d66fd37bce2ce6f85962484a1d32826b7ab8c9baba431eaa2765 --- src/.bear-tidy-config | 2 ++ src/.clang-tidy | 1 + src/bench/block_assemble.cpp | 2 +- src/bench/wallet_loading.cpp | 4 ++-- src/external_signer.cpp | 2 +- src/httpserver.cpp | 12 +++++----- src/net_permissions.cpp | 14 +++++------ src/net_processing.cpp | 14 +++++------ src/node/blockstorage.cpp | 2 +- src/qt/rpcconsole.cpp | 8 +++---- src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 2 +- src/rpc/rawtransaction.cpp | 8 +++---- src/rpc/server.cpp | 2 +- src/script/sign.cpp | 8 +++---- src/test/addrman_tests.cpp | 4 ++-- src/test/bip32_tests.cpp | 2 +- src/test/net_tests.cpp | 4 ++-- src/test/rpc_tests.cpp | 36 ++++++++++++++--------------- src/test/settings_tests.cpp | 16 ++++++------- src/test/sighash_tests.cpp | 4 ++-- src/test/txpackage_tests.cpp | 22 +++++++++--------- src/test/util/setup_common.cpp | 6 ++--- src/test/util_tests.cpp | 4 ++-- src/test/util_threadnames_tests.cpp | 2 +- src/test/validation_block_tests.cpp | 4 ++-- src/validation.cpp | 2 +- src/wallet/rpc/backup.cpp | 6 ++--- src/wallet/salvage.cpp | 2 +- src/wallet/spend.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 10 ++++---- 31 files changed, 106 insertions(+), 103 deletions(-) diff --git a/src/.bear-tidy-config b/src/.bear-tidy-config index fd12f3d130ba..3512e59a781b 100644 --- a/src/.bear-tidy-config +++ b/src/.bear-tidy-config @@ -12,6 +12,8 @@ "src/leveldb", "src/minisketch", "src/univalue", + "src/bench/nanobench.cpp", + "src/bench/nanobench.h", "src/secp256k1" ] }, diff --git a/src/.clang-tidy b/src/.clang-tidy index ad82694d875d..3acf5d25f32f 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -4,6 +4,7 @@ bugprone-argument-comment, bugprone-use-after-move, misc-unused-using-decls, modernize-use-default-member-init, +modernize-use-emplace, modernize-use-nullptr, performance-for-range-copy, performance-move-const-arg, diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 01832712b706..2d885d056e87 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -32,7 +32,7 @@ static void AssembleBlock(benchmark::Bench& bench) std::array txs; for (size_t b{0}; b < NUM_BLOCKS; ++b) { CMutableTransaction tx; - tx.vin.push_back(MineBlock(test_setup->m_node, SCRIPT_PUB)); + tx.vin.emplace_back(MineBlock(test_setup->m_node, SCRIPT_PUB)); tx.vin.back().scriptSig = scriptSig; tx.vout.emplace_back(1337, SCRIPT_PUB); if (NUM_BLOCKS - b >= COINBASE_MATURITY) diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index 1a2bd7eceb32..bc405cfb09dc 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -46,8 +46,8 @@ static void BenchUnloadWallet(std::shared_ptr&& wallet) static void AddTx(CWallet& wallet) { CMutableTransaction mtx; - mtx.vout.push_back({COIN, GetScriptForDestination(*Assert(wallet.GetNewDestination("")))}); - mtx.vin.push_back(CTxIn()); + mtx.vout.emplace_back({COIN, GetScriptForDestination(*Assert(wallet.GetNewDestination("")))}); + mtx.vin.emplace_back(CTxIn()); wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); } diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 9659cc1a06dc..cda34a14fd2f 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -54,7 +54,7 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector NetPermissions::ToStrings(NetPermissionFlags flags) { std::vector strings; - if (NetPermissions::HasFlag(flags, NetPermissionFlags::BloomFilter)) strings.push_back("bloomfilter"); - if (NetPermissions::HasFlag(flags, NetPermissionFlags::NoBan)) strings.push_back("noban"); - if (NetPermissions::HasFlag(flags, NetPermissionFlags::ForceRelay)) strings.push_back("forcerelay"); - if (NetPermissions::HasFlag(flags, NetPermissionFlags::Relay)) strings.push_back("relay"); - if (NetPermissions::HasFlag(flags, NetPermissionFlags::Mempool)) strings.push_back("mempool"); - if (NetPermissions::HasFlag(flags, NetPermissionFlags::Download)) strings.push_back("download"); - if (NetPermissions::HasFlag(flags, NetPermissionFlags::Addr)) strings.push_back("addr"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::BloomFilter)) strings.emplace_back("bloomfilter"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::NoBan)) strings.emplace_back("noban"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::ForceRelay)) strings.emplace_back("forcerelay"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Relay)) strings.emplace_back("relay"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Mempool)) strings.emplace_back("mempool"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Download)) strings.emplace_back("download"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Addr)) strings.emplace_back("addr"); return strings; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2700927318e6..1e2f9621c5df 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2781,7 +2781,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // and we want it right after the last block so they don't // wait for other stuff first. std::vector vInv; - vInv.push_back(CInv(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash())); + vInv.emplace_back(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); peer.m_continuation_block.SetNull(); } @@ -3182,7 +3182,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c // Can't download any more from this peer break; } - vGetData.push_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); + vGetData.emplace_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); BlockRequested(pfrom.GetId(), *pindex); LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", pindex->GetBlockHash().ToString(), pfrom.GetId()); @@ -4596,7 +4596,7 @@ void PeerManagerImpl::ProcessMessage( const auto send_headers = [this /* for m_connman */, &hashStop, &pindex, &nodestate, &pfrom, &msgMaker](auto msg_type_internal, auto& v_headers, auto callback) { int nLimit = GetHeadersLimit(pfrom, msg_type_internal == NetMsgType::HEADERS2); for (; pindex; pindex = m_chainman.ActiveChain().Next(pindex)) { - v_headers.push_back(callback(pindex)); + v_headers.emplace_back(callback(pindex)); if (--nLimit <= 0 || pindex->GetBlockHash() == hashStop) break; @@ -6145,14 +6145,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } if (fFoundStartingHeader) { // add this to the headers message - vHeaders.push_back(pindex->GetBlockHeader()); + vHeaders.emplace_back(pindex->GetBlockHeader()); } else if (PeerHasHeader(&state, pindex)) { continue; // keep looking for the first new block } else if (pindex->pprev == nullptr || PeerHasHeader(&state, pindex->pprev) || isPrevDevnetGenesisBlock) { // Peer doesn't have this header but they do have the prior one. // Start sending headers. fFoundStartingHeader = true; - vHeaders.push_back(pindex->GetBlockHeader()); + vHeaders.emplace_back(pindex->GetBlockHeader()); } else { // Peer doesn't have this header or the prior one -- nothing will // connect, so bail out. @@ -6260,7 +6260,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Add blocks for (const uint256& hash : peer->m_blocks_for_inv_relay) { - vInv.push_back(CInv(MSG_BLOCK, hash)); + vInv.emplace_back(MSG_BLOCK, hash); if (vInv.size() == MAX_INV_SZ) { m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); @@ -6505,7 +6505,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) NodeId staller = -1; FindNextBlocksToDownload(*peer, MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller); for (const CBlockIndex *pindex : vToDownload) { - vGetData.push_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); + vGetData.emplace_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); BlockRequested(pto->GetId(), *pindex); LogPrint(BCLog::NET, "Requesting block %s (%d) peer=%d\n", pindex->GetBlockHash().ToString(), pindex->nHeight, pto->GetId()); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 76fd09843ac7..35b896b645db 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -331,7 +331,7 @@ bool BlockManager::WriteBlockIndexDB() std::vector> vFiles; vFiles.reserve(m_dirty_fileinfo.size()); for (std::set::iterator it = m_dirty_fileinfo.begin(); it != m_dirty_fileinfo.end();) { - vFiles.push_back(std::make_pair(*it, &m_blockfile_info[*it])); + vFiles.emplace_back(*it, &m_blockfile_info[*it]); m_dirty_fileinfo.erase(it++); } std::vector vBlocks; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 9248e1c406a9..4bdef806da09 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -179,7 +179,7 @@ class PeerIdViewDelegate : public QStyledItemDelegate bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strResult, const std::string &strCommand, const bool fExecute, std::string * const pstrFilteredOut, const WalletModel* wallet_model) { std::vector< std::vector > stack; - stack.push_back(std::vector()); + stack.emplace_back(); enum CmdParseState { @@ -207,7 +207,7 @@ bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strRes } // Make sure stack is not empty before adding something if (stack.empty()) { - stack.push_back(std::vector()); + stack.emplace_back(); } stack.back().push_back(strArg); }; @@ -216,7 +216,7 @@ bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strRes if (nDepthInsideSensitive) { if (!--nDepthInsideSensitive) { assert(filter_begin_pos); - filter_ranges.push_back(std::make_pair(filter_begin_pos, chpos)); + filter_ranges.emplace_back(filter_begin_pos, chpos); filter_begin_pos = 0; } } @@ -316,7 +316,7 @@ bool RPCConsole::RPCParseCommandLine(interfaces::Node* node, std::string &strRes if (nDepthInsideSensitive) { ++nDepthInsideSensitive; } - stack.push_back(std::vector()); + stack.emplace_back(); } // don't allow commands after executed commands on baselevel diff --git a/src/rest.cpp b/src/rest.cpp index 207a2f247400..45985f97e6f7 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -760,7 +760,7 @@ static bool rest_getutxos(const CoreContext& context, HTTPRequest* req, const st return RESTERR(req, HTTP_BAD_REQUEST, "Parse error"); txid.SetHex(strTxid); - vOutPoints.push_back(COutPoint(txid, (uint32_t)nOutput)); + vOutPoints.emplace_back(txid, (uint32_t)nOutput); } if (vOutPoints.size() > 0) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ab74d75431b9..8be55c3a05df 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2192,7 +2192,7 @@ static RPCHelpMan getblockstats() CAmount feerate = tx_size ? txfee / tx_size : 0; if (do_feerate_percentiles) { - feerate_array.emplace_back(std::make_pair(feerate, tx_size)); + feerate_array.emplace_back(feerate, weight); } maxfeerate = std::max(maxfeerate, feerate); minfeerate = std::min(minfeerate, feerate); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 554f4bd3d53e..ffa2d54fdbe7 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1583,10 +1583,10 @@ static RPCHelpMan createpsbt() PartiallySignedTransaction psbtx; psbtx.tx = rawTx; for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { - psbtx.inputs.push_back(PSBTInput()); + psbtx.inputs.emplace_back(); } for (unsigned int i = 0; i < rawTx.vout.size(); ++i) { - psbtx.outputs.push_back(PSBTOutput()); + psbtx.outputs.emplace_back(); } // Serialize the PSBT @@ -1640,10 +1640,10 @@ static RPCHelpMan converttopsbt() PartiallySignedTransaction psbtx; psbtx.tx = tx; for (unsigned int i = 0; i < tx.vin.size(); ++i) { - psbtx.inputs.push_back(PSBTInput()); + psbtx.inputs.emplace_back(); } for (unsigned int i = 0; i < tx.vout.size(); ++i) { - psbtx.outputs.push_back(PSBTOutput()); + psbtx.outputs.emplace_back(); } // Serialize the PSBT diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9cfecfe23bd1..e025e34e042b 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -87,7 +87,7 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& std::vector > vCommands; for (const auto& entry : mapCommands) - vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front())); + vCommands.emplace_back(entry.second.front()->category + entry.first, entry.second.front()); sort(vCommands.begin(), vCommands.end()); JSONRPCRequest jreq = helpreq; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 55ab8e248c41..2622862f4d67 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -137,7 +137,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator case TxoutType::SCRIPTHASH: { uint160 h160{vSolutions[0]}; if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) { - ret.push_back(std::vector(scriptRet.begin(), scriptRet.end())); + ret.emplace_back(scriptRet.begin(), scriptRet.end()); return true; } // Could not find redeemScript, add to missing @@ -146,7 +146,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator } case TxoutType::MULTISIG: { size_t required = vSolutions.front()[0]; - ret.push_back(valtype()); // workaround CHECKMULTISIG bug + ret.emplace_back(); // workaround CHECKMULTISIG bug for (size_t i = 1; i < vSolutions.size() - 1; ++i) { CPubKey pubkey = CPubKey(vSolutions[i]); // We need to always call CreateSig in order to fill sigdata with all @@ -160,7 +160,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator } bool ok = ret.size() == required + 1; for (size_t i = 0; i + ret.size() < required + 1; ++i) { - ret.push_back(valtype()); + ret.emplace_back(); } return ok; } @@ -207,7 +207,7 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato } if (P2SH) { - result.push_back(std::vector(subscript.begin(), subscript.end())); + result.emplace_back(subscript.begin(), subscript.end()); } sigdata.scriptSig = PushAll(result); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 8022dd8c2643..7088c0f9bc55 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -97,8 +97,8 @@ BOOST_AUTO_TEST_CASE(addrman_simple) // Test: reset addrman and test AddrMan::Add multiple addresses works as expected addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); std::vector vAddr; - vAddr.push_back(CAddress(ResolveService("250.1.1.3", 8333), NODE_NONE)); - vAddr.push_back(CAddress(ResolveService("250.1.1.4", 8333), NODE_NONE)); + vAddr.emplace_back(ResolveService("250.1.1.3", 8333), NODE_NONE); + vAddr.emplace_back(ResolveService("250.1.1.4", 8333), NODE_NONE); BOOST_CHECK(addrman->Add(vAddr, source)); BOOST_CHECK(addrman->Size() >= 1); } diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index 3a52630a5d56..e3a71c963f43 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -29,7 +29,7 @@ struct TestVector { explicit TestVector(std::string strHexMasterIn) : strHexMaster(strHexMasterIn) {} TestVector& operator()(std::string pub, std::string prv, unsigned int nChild) { - vDerive.push_back(TestDerivation()); + vDerive.emplace_back(); TestDerivation &der = vDerive.back(); der.pub = pub; der.prv = prv; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 3c77fed9bd8f..e950061d1d6e 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1096,9 +1096,9 @@ class V2TransportTester bool reject{false}; auto msg = m_transport.GetReceivedMessage({}, reject); if (reject) { - ret.push_back(std::nullopt); + ret.emplace_back(std::nullopt); } else { - ret.push_back(std::move(msg)); + ret.emplace_back(std::move(msg)); } progress = true; } diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 6ebc56e250e1..8bf2a44ab387 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -445,11 +445,11 @@ BOOST_AUTO_TEST_CASE(rpc_getblockstats_calculate_percentiles_by_size) CAmount result[NUM_GETBLOCKSTATS_PERCENTILES] = { 0 }; for (int64_t i = 0; i < 100; i++) { - feerates.emplace_back(std::make_pair(1 ,1)); + feerates.emplace_back(1 ,1); } for (int64_t i = 0; i < 100; i++) { - feerates.emplace_back(std::make_pair(2 ,1)); + feerates.emplace_back(2 ,1); } CalculatePercentilesBySize(result, feerates, total_size); @@ -464,11 +464,11 @@ BOOST_AUTO_TEST_CASE(rpc_getblockstats_calculate_percentiles_by_size) CAmount result2[NUM_GETBLOCKSTATS_PERCENTILES] = { 0 }; feerates.clear(); - feerates.emplace_back(std::make_pair(1, 9)); - feerates.emplace_back(std::make_pair(2 , 16)); //10th + 25th percentile - feerates.emplace_back(std::make_pair(4 ,50)); //50th + 75th percentile - feerates.emplace_back(std::make_pair(5 ,10)); - feerates.emplace_back(std::make_pair(9 ,15)); // 90th percentile + feerates.emplace_back(1, 9); + feerates.emplace_back(2 , 16); //10th + 25th percentile + feerates.emplace_back(4 ,50); //50th + 75th percentile + feerates.emplace_back(5 ,10); + feerates.emplace_back(9 ,15); // 90th percentile CalculatePercentilesBySize(result2, feerates, total_size); @@ -483,12 +483,12 @@ BOOST_AUTO_TEST_CASE(rpc_getblockstats_calculate_percentiles_by_size) CAmount result3[NUM_GETBLOCKSTATS_PERCENTILES] = { 0 }; feerates.clear(); - feerates.emplace_back(std::make_pair(1, 9)); - feerates.emplace_back(std::make_pair(2 , 11)); // 10th percentile - feerates.emplace_back(std::make_pair(2 , 5)); // 25th percentile - feerates.emplace_back(std::make_pair(4 ,50)); //50th + 75th percentile - feerates.emplace_back(std::make_pair(5 ,10)); - feerates.emplace_back(std::make_pair(9 ,15)); // 90th percentile + feerates.emplace_back(1, 9); + feerates.emplace_back(2 , 11); // 10th percentile + feerates.emplace_back(2 , 5); // 25th percentile + feerates.emplace_back(4 ,50); //50th + 75th percentile + feerates.emplace_back(5 ,10); + feerates.emplace_back(9 ,15); // 90th percentile CalculatePercentilesBySize(result3, feerates, total_size); @@ -503,11 +503,11 @@ BOOST_AUTO_TEST_CASE(rpc_getblockstats_calculate_percentiles_by_size) CAmount result4[NUM_GETBLOCKSTATS_PERCENTILES] = { 0 }; feerates.clear(); - feerates.emplace_back(std::make_pair(1, 100)); - feerates.emplace_back(std::make_pair(2, 1)); - feerates.emplace_back(std::make_pair(3, 1)); - feerates.emplace_back(std::make_pair(3, 1)); - feerates.emplace_back(std::make_pair(999999, 1)); + feerates.emplace_back(1, 100); + feerates.emplace_back(2, 1); + feerates.emplace_back(3, 1); + feerates.emplace_back(3, 1); + feerates.emplace_back(999999, 1); CalculatePercentilesBySize(result4, feerates, total_size); diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index ad12c4656106..9bd9c7942043 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -119,16 +119,16 @@ static void CheckValues(const util::Settings& settings, const std::string& singl BOOST_AUTO_TEST_CASE(Simple) { util::Settings settings; - settings.command_line_options["name"].push_back("val1"); - settings.command_line_options["name"].push_back("val2"); - settings.ro_config["section"]["name"].push_back(2); + settings.command_line_options["name"].emplace_back("val1"); + settings.command_line_options["name"].emplace_back("val2"); + settings.ro_config["section"]["name"].emplace_back(2); // The last given arg takes precedence when specified via commandline. CheckValues(settings, R"("val2")", R"(["val1","val2",2])"); util::Settings settings2; - settings2.ro_config["section"]["name"].push_back("val2"); - settings2.ro_config["section"]["name"].push_back("val3"); + settings2.ro_config["section"]["name"].emplace_back("val2"); + settings2.ro_config["section"]["name"].emplace_back("val3"); // The first given arg takes precedence when specified via config file. CheckValues(settings2, R"("val2")", R"(["val2","val3"])"); @@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(Simple) BOOST_AUTO_TEST_CASE(NullOverride) { util::Settings settings; - settings.command_line_options["name"].push_back("value"); + settings.command_line_options["name"].emplace_back("value"); BOOST_CHECK_EQUAL(R"("value")", GetSetting(settings, "section", "name", false, false, false).write().c_str()); settings.forced_settings["name"] = {}; BOOST_CHECK_EQUAL(R"(null)", GetSetting(settings, "section", "name", false, false, false).write().c_str()); @@ -202,11 +202,11 @@ BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup) std::vector& dest) { if (action == SET || action == SECTION_SET) { for (int i = 0; i < 2; ++i) { - dest.push_back(value_prefix + ToString(++value_suffix)); + dest.emplace_back(value_prefix + ToString(++value_suffix)); desc += " " + name_prefix + name + "=" + dest.back().get_str(); } } else if (action == NEGATE || action == SECTION_NEGATE) { - dest.push_back(false); + dest.emplace_back(false); desc += " " + name_prefix + "no" + name; } }; diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index f1d8e6d0ad90..b765f9a808db 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -96,7 +96,7 @@ void static RandomTransaction(CMutableTransaction &tx, bool fSingle) int ins = (InsecureRandBits(2)) + 1; int outs = fSingle ? ins : (InsecureRandBits(2)) + 1; for (int in = 0; in < ins; in++) { - tx.vin.push_back(CTxIn()); + tx.vin.emplace_back(); CTxIn &txin = tx.vin.back(); txin.prevout.hash = InsecureRand256(); txin.prevout.n = InsecureRandBits(2); @@ -104,7 +104,7 @@ void static RandomTransaction(CMutableTransaction &tx, bool fSingle) txin.nSequence = (InsecureRandBool()) ? InsecureRand32() : std::numeric_limits::max(); } for (int out = 0; out < outs; out++) { - tx.vout.push_back(CTxOut()); + tx.vout.emplace_back(); CTxOut &txout = tx.vout.back(); txout.nValue = InsecureRandRange(100000000); RandomScript(txout.scriptPubKey); diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 312bf3894248..55e2c5fa6502 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -159,9 +159,9 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100NoDIP0001Setup) auto parent = MakeTransactionRef(CreateValidMempoolTransaction(m_coinbase_txns[i + 1], 0, 0, coinbaseKey, spk, CAmount(48 * COIN), false)); package.emplace_back(parent); - child.vin.push_back(CTxIn(COutPoint(parent->GetHash(), 0))); + child.vin.emplace_back(COutPoint(parent->GetHash(), 0)); } - child.vout.push_back(CTxOut(47 * COIN, spk2)); + child.vout.emplace_back(47 * COIN, spk2); // The child must be in the package. BOOST_CHECK(!IsChildWithParents(package)); @@ -186,20 +186,20 @@ BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100NoDIP0001Setup) // 2 Parents and 1 Child where one parent depends on the other. { CMutableTransaction mtx_parent; - mtx_parent.vin.push_back(CTxIn(COutPoint(m_coinbase_txns[0]->GetHash(), 0))); - mtx_parent.vout.push_back(CTxOut(20 * COIN, spk)); - mtx_parent.vout.push_back(CTxOut(20 * COIN, spk2)); + mtx_parent.vin.emplace_back(COutPoint(m_coinbase_txns[0]->GetHash(), 0)); + mtx_parent.vout.emplace_back(20 * COIN, spk); + mtx_parent.vout.emplace_back(20 * COIN, spk2); CTransactionRef tx_parent = MakeTransactionRef(mtx_parent); CMutableTransaction mtx_parent_also_child; - mtx_parent_also_child.vin.push_back(CTxIn(COutPoint(tx_parent->GetHash(), 0))); - mtx_parent_also_child.vout.push_back(CTxOut(20 * COIN, spk)); + mtx_parent_also_child.vin.emplace_back(COutPoint(tx_parent->GetHash(), 0)); + mtx_parent_also_child.vout.emplace_back(20 * COIN, spk); CTransactionRef tx_parent_also_child = MakeTransactionRef(mtx_parent_also_child); CMutableTransaction mtx_child; - mtx_child.vin.push_back(CTxIn(COutPoint(tx_parent->GetHash(), 1))); - mtx_child.vin.push_back(CTxIn(COutPoint(tx_parent_also_child->GetHash(), 0))); - mtx_child.vout.push_back(CTxOut(39 * COIN, spk)); + mtx_child.vin.emplace_back(COutPoint(tx_parent->GetHash(), 1)); + mtx_child.vin.emplace_back(COutPoint(tx_parent_also_child->GetHash(), 0)); + mtx_child.vout.emplace_back(39 * COIN, spk); CTransactionRef tx_child = MakeTransactionRef(mtx_child); PackageValidationState state; @@ -284,7 +284,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100NoDIP0001Setup) } // Child with missing parent. - mtx_child.vin.push_back(CTxIn(COutPoint(package_unrelated[0]->GetHash(), 0))); + mtx_child.vin.emplace_back(COutPoint(package_unrelated[0]->GetHash(), 0)); Package package_missing_parent; package_missing_parent.push_back(tx_parent); package_missing_parent.push_back(MakeTransactionRef(mtx_child)); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2d98de87a1bd..adcd43b3abc4 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -645,7 +645,7 @@ std::vector TestChainSetup::PopulateMempool(FastRandomContext& for (size_t n{0}; n < num_inputs; ++n) { if (unspent_prevouts.empty()) break; const auto& [prevout, amount] = unspent_prevouts.front(); - mtx.vin.push_back(CTxIn(prevout, CScript())); + mtx.vin.emplace_back(prevout, CScript()); total_in += amount; unspent_prevouts.pop_front(); } @@ -654,7 +654,7 @@ std::vector TestChainSetup::PopulateMempool(FastRandomContext& const CAmount amount_per_output = (total_in - 1000) / num_outputs; for (size_t n{0}; n < num_outputs; ++n) { CScript spk = CScript() << CScriptNum(num_transactions + n); - mtx.vout.push_back(CTxOut(amount_per_output, spk)); + mtx.vout.emplace_back(amount_per_output, spk); } CTransactionRef ptx = MakeTransactionRef(mtx); mempool_transactions.push_back(ptx); @@ -663,7 +663,7 @@ std::vector TestChainSetup::PopulateMempool(FastRandomContext& // it can be used to build a more complex transaction graph. Insert randomly into // unspent_prevouts for extra randomness in the resulting structures. for (size_t n{0}; n < num_outputs; ++n) { - unspent_prevouts.push_back(std::make_pair(COutPoint(ptx->GetHash(), n), amount_per_output)); + unspent_prevouts.emplace_back(COutPoint(ptx->GetHash(), n), amount_per_output); std::swap(unspent_prevouts.back(), unspent_prevouts[det_rand.randrange(unspent_prevouts.size())]); } } diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 2da64caf6576..f78a8e0c264a 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1037,9 +1037,9 @@ BOOST_AUTO_TEST_CASE(test_FormatParagraph) BOOST_AUTO_TEST_CASE(test_FormatSubVersion) { std::vector comments; - comments.push_back(std::string("comment1")); + comments.emplace_back("comment1"); std::vector comments2; - comments2.push_back(std::string("comment1")); + comments2.emplace_back("comment1"); comments2.push_back(SanitizeString(std::string("Comment2; .,_?@-; !\"#$%&'()*+/<=>[]\\^`{|}~"), SAFE_CHARS_UA_COMMENT)); // Semicolon is discouraged but not forbidden by BIP-0014 BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, std::vector()),std::string("/Test:9.99.0/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments),std::string("/Test:9.99.0(comment1)/")); diff --git a/src/test/util_threadnames_tests.cpp b/src/test/util_threadnames_tests.cpp index 3ddf3390615f..53987ae856fa 100644 --- a/src/test/util_threadnames_tests.cpp +++ b/src/test/util_threadnames_tests.cpp @@ -35,7 +35,7 @@ std::set RenameEnMasse(int num_threads) }; for (int i = 0; i < num_threads; ++i) { - threads.push_back(std::thread(RenameThisThread, i)); + threads.emplace_back(RenameThisThread, i); } for (std::thread& thread : threads) thread.join(); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 84daeb098541..ec7c037ab878 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -116,7 +116,7 @@ std::shared_ptr MinerTestingSetup::BadBlock(const uint256& prev_ha auto pblock = Block(prev_hash); CMutableTransaction coinbase_spend; - coinbase_spend.vin.push_back(CTxIn(COutPoint(pblock->vtx[0]->GetHash(), 0), CScript(), 0)); + coinbase_spend.vin.emplace_back(COutPoint(pblock->vtx[0]->GetHash(), 0), CScript(), 0); coinbase_spend.vout.push_back(pblock->vtx[0]->vout[0]); CTransactionRef tx = MakeTransactionRef(coinbase_spend); @@ -243,7 +243,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) std::vector txs; for (int num_txs = 22; num_txs > 0; --num_txs) { CMutableTransaction mtx; - mtx.vin.push_back( + mtx.vin.emplace_back( CTxIn(COutPoint(last_mined->vtx[0]->GetHash(), 1), CScript() << ToByteVector(CScript() << OP_TRUE))); // Two outputs to make sure the transaction is larger than 100 bytes diff --git a/src/validation.cpp b/src/validation.cpp index f13113ba36dc..ad08f0aa4e3b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2572,7 +2572,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, CTxUndo undoDummy; if (i > 0) { - blockundo.vtxundo.push_back(CTxUndo()); + blockundo.vtxundo.emplace_back(); } UpdateCoins(tx, view, i == 0 ? undoDummy : blockundo.vtxundo.back(), pindex->nHeight); } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index caca41089fd2..892dc07328e6 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -573,12 +573,12 @@ RPCHelpMan importwallet() fLabel = true; } } - keys.push_back(std::make_tuple(key, nTime, fLabel, strLabel)); + keys.emplace_back(std::make_tuple(key, nTime, fLabel, strLabel)); } else if(IsHex(vstr[0])) { std::vector vData(ParseHex(vstr[0])); CScript script = CScript(vData.begin(), vData.end()); int64_t birth_time = ParseISO8601DateTime(vstr[1]); - scripts.push_back(std::pair(script, birth_time)); + scripts.emplace_back(std::pair(script, birth_time)); } } file.close(); @@ -995,7 +995,7 @@ RPCHelpMan dumpwallet() // sort time/key pairs std::vector > vKeyBirth; for (const auto& entry : mapKeyBirth) { - vKeyBirth.push_back(std::make_pair(entry.second, entry.first)); + vKeyBirth.emplace_back(entry.second, entry.first); } mapKeyBirth.clear(); std::sort(vKeyBirth.begin(), vKeyBirth.end()); diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 1ee0edd00e8f..d7df7f2d1876 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -103,7 +103,7 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil warnings.push_back(Untranslated("Salvage: WARNING: Number of keys in data does not match number of values.")); break; } - salvagedData.push_back(make_pair(ParseHex(keyHex), ParseHex(valueHex))); + salvagedData.emplace_back(ParseHex(keyHex), ParseHex(valueHex)); } } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 823c7a37bfd6..54df0446c9f8 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -960,7 +960,7 @@ static util::Result CreateTransactionInternal( // works. const uint32_t nSequence{CTxIn::SEQUENCE_FINAL - 1}; for (const auto& coin : result->GetInputSet()) { - txNew.vin.push_back(CTxIn(coin.outpoint, CScript(), nSequence)); + txNew.vin.emplace_back(CTxIn(coin.outpoint, CScript(), nSequence)); } DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight()); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index cf398e4df3fd..59e3ef6dbcfc 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -80,7 +80,7 @@ static void TestUnloadWallet(WalletContext& context, std::shared_ptr&& static CMutableTransaction TestSimpleSpend(const CTransaction& from, uint32_t index, const CKey& key, const CScript& pubkey) { CMutableTransaction mtx; - mtx.vout.push_back({from.vout[index].nValue - DEFAULT_TRANSACTION_MAXFEE, pubkey}); + mtx.vout.emplace_back(from.vout[index].nValue - DEFAULT_TRANSACTION_MAXFEE, pubkey); mtx.vin.push_back({CTxIn{from.GetHash(), index}}); FillableSigningProvider keystore; keystore.AddKey(key); @@ -1539,8 +1539,8 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) BOOST_ASSERT(op_dest); CMutableTransaction mtx; - mtx.vout.push_back({COIN, GetScriptForDestination(*op_dest)}); - mtx.vin.push_back(CTxIn(g_insecure_rand_ctx.rand256(), 0)); + mtx.vout.emplace_back({COIN, GetScriptForDestination(*op_dest)}); + mtx.vin.emplace_back(CTxIn(g_insecure_rand_ctx.rand256(), 0)); const auto& tx_id_to_spend = wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInMempool{})->GetHash(); { @@ -1555,7 +1555,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) // 2) Verify that the available balance of this new tx and the old one is updated (prev tx is marked dirty) mtx.vin.clear(); - mtx.vin.push_back(CTxIn(tx_id_to_spend, 0)); + mtx.vin.emplace_back(CTxIn(tx_id_to_spend, 0)); wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0, 0); const uint256& good_tx_id = mtx.GetHash(); @@ -1576,7 +1576,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) // verify that we are not moving forward if the wallet cannot store it static_cast(wallet.GetDatabase()).m_pass = false; mtx.vin.clear(); - mtx.vin.push_back(CTxIn(good_tx_id, 0)); + mtx.vin.emplace_back(CTxIn(good_tx_id, 0)); BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0, 0), std::runtime_error, HasReason("DB error adding transaction to wallet, write failed")); From 6a1490310bd56cd4c913b1b79b778759398295b0 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 16 Oct 2023 14:56:54 +0100 Subject: [PATCH 10/13] Merge bitcoin/bitcoin#28583: refactor: [tidy] modernize-use-emplace fa05a726c225dc65dee79367bb67f099ae4f99e6 tidy: modernize-use-emplace (MarcoFalke) Pull request description: Constructing a temporary unnamed object only to copy or move it into a container seems both verbose in code and a strict performance penalty. Fix both issues via the `modernize-use-emplace` tidy check. ACKs for top commit: Sjors: re-utACK fa05a726c2 hebasto: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6. TheCharlatan: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6 Tree-SHA512: 4408a094f406e7bf6c1468c2b0798f68f4d952a1253cf5b20bdc648ad7eea4a2c070051fed46d66fd37bce2ce6f85962484a1d32826b7ab8c9baba431eaa2765 --- src/rpc/blockchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8be55c3a05df..d41f8526b9a6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2192,7 +2192,7 @@ static RPCHelpMan getblockstats() CAmount feerate = tx_size ? txfee / tx_size : 0; if (do_feerate_percentiles) { - feerate_array.emplace_back(feerate, weight); + feerate_array.emplace_back(feerate, tx_size); } maxfeerate = std::max(maxfeerate, feerate); minfeerate = std::min(minfeerate, feerate); From 05bd1716bdf9fd174d30dbf401d0b7291e6345a8 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 16 Oct 2023 14:56:54 +0100 Subject: [PATCH 11/13] Merge bitcoin/bitcoin#28583: refactor: [tidy] modernize-use-emplace fa05a726c225dc65dee79367bb67f099ae4f99e6 tidy: modernize-use-emplace (MarcoFalke) Pull request description: Constructing a temporary unnamed object only to copy or move it into a container seems both verbose in code and a strict performance penalty. Fix both issues via the `modernize-use-emplace` tidy check. ACKs for top commit: Sjors: re-utACK fa05a726c2 hebasto: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6. TheCharlatan: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6 Tree-SHA512: 4408a094f406e7bf6c1468c2b0798f68f4d952a1253cf5b20bdc648ad7eea4a2c070051fed46d66fd37bce2ce6f85962484a1d32826b7ab8c9baba431eaa2765 --- src/wallet/test/wallet_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 59e3ef6dbcfc..17746823965d 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -1539,7 +1539,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) BOOST_ASSERT(op_dest); CMutableTransaction mtx; - mtx.vout.emplace_back({COIN, GetScriptForDestination(*op_dest)}); + mtx.vout.emplace_back(COIN, GetScriptForDestination(*op_dest)); mtx.vin.emplace_back(CTxIn(g_insecure_rand_ctx.rand256(), 0)); const auto& tx_id_to_spend = wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInMempool{})->GetHash(); From 956edfc0d826924046dc601e0e9ba3bb6cf86e2b Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 16 Oct 2023 14:56:54 +0100 Subject: [PATCH 12/13] Merge bitcoin/bitcoin#28583: refactor: [tidy] modernize-use-emplace fa05a726c225dc65dee79367bb67f099ae4f99e6 tidy: modernize-use-emplace (MarcoFalke) Pull request description: Constructing a temporary unnamed object only to copy or move it into a container seems both verbose in code and a strict performance penalty. Fix both issues via the `modernize-use-emplace` tidy check. ACKs for top commit: Sjors: re-utACK fa05a726c2 hebasto: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6. TheCharlatan: ACK fa05a726c225dc65dee79367bb67f099ae4f99e6 Tree-SHA512: 4408a094f406e7bf6c1468c2b0798f68f4d952a1253cf5b20bdc648ad7eea4a2c070051fed46d66fd37bce2ce6f85962484a1d32826b7ab8c9baba431eaa2765 --- src/bench/wallet_loading.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index bc405cfb09dc..21df3057143f 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -46,7 +46,7 @@ static void BenchUnloadWallet(std::shared_ptr&& wallet) static void AddTx(CWallet& wallet) { CMutableTransaction mtx; - mtx.vout.emplace_back({COIN, GetScriptForDestination(*Assert(wallet.GetNewDestination("")))}); + mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet.GetNewDestination("")))); mtx.vin.emplace_back(CTxIn()); wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); From db4cc0bd5e2661830de05dda602249f7b0d2ce17 Mon Sep 17 00:00:00 2001 From: Shailendra Kumar Gupta Date: Tue, 24 Feb 2026 16:43:43 +0530 Subject: [PATCH 13/13] removing mistakenly added comment --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1e2f9621c5df..6df52989af86 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5041,7 +5041,7 @@ void PeerManagerImpl::ProcessMessage( ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(resp.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); // Test + Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :(