From 39f1dc944554218911b0945fff7e6d06f3dab284 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 5 Aug 2020 17:46:31 +0200 Subject: p2p: remove nFetchFlags from NetMsgType TX and INV processing The nFetchFlags code can be removed here because GetFetchFlags() can only add the MSG_WITNESS_FLAG, which is added to the CInv::type field. That CInv is only passed to AlreadyHave() or ToGenTxid(), and neither of those functions do anything different depending on whether the CInv type is MSG_TX or MSG_WITNESS_TX. Co-authored by: John Newbery --- src/net_processing.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 60bdfbe9f..a01514185 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2654,14 +2654,12 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty LOCK(cs_main); - uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime(); uint256* best_block{nullptr}; for (CInv &inv : vInv) { - if (interruptMsgProc) - return; + if (interruptMsgProc) return; // Ignore INVs that don't match wtxidrelay setting. // Note that orphan parent fetching always uses MSG_TX GETDATAs regardless of the wtxidrelay setting. @@ -2675,10 +2673,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty bool fAlreadyHave = AlreadyHave(inv, m_mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - if (inv.IsMsgTx()) { - inv.type |= nFetchFlags; - } - if (inv.type == MSG_BLOCK) { UpdateBlockAvailability(pfrom.GetId(), inv.hash); if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) { @@ -3013,7 +3007,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } } if (!fRejectedParents) { - uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime(); for (const uint256& parent_txid : unique_parents) { @@ -3022,7 +3015,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - CInv _inv(MSG_TX | nFetchFlags, parent_txid); + CInv _inv(MSG_TX, parent_txid); pfrom.AddKnownTx(parent_txid); if (!AlreadyHave(_inv, m_mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); } -- cgit v1.2.3 From 42ca5618cae0fd9ef97d2006b17d896bc58cc17c Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 30 Jul 2020 10:30:24 +0100 Subject: [net processing] Split AlreadyHave() into separate block and tx functions --- src/net_processing.cpp | 78 +++++++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 42 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a01514185..3a0613dee 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1422,47 +1422,38 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidatio // -bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool static AlreadyHaveTx(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - switch (inv.type) + assert(recentRejects); + if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) { + // If the chain tip has changed previously rejected transactions + // might be now valid, e.g. due to a nLockTime'd tx becoming valid, + // or a double-spend. Reset the rejects filter and give those + // txs a second chance. + hashRecentRejectsChainTip = ::ChainActive().Tip()->GetBlockHash(); + recentRejects->reset(); + } + { - case MSG_TX: - case MSG_WITNESS_TX: - case MSG_WTX: - { - assert(recentRejects); - if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) - { - // If the chain tip has changed previously rejected transactions - // might be now valid, e.g. due to a nLockTime'd tx becoming valid, - // or a double-spend. Reset the rejects filter and give those - // txs a second chance. - hashRecentRejectsChainTip = ::ChainActive().Tip()->GetBlockHash(); - recentRejects->reset(); - } + LOCK(g_cs_orphans); + if (!inv.IsMsgWtx() && mapOrphanTransactions.count(inv.hash)) { + return true; + } else if (inv.IsMsgWtx() && g_orphans_by_wtxid.count(inv.hash)) { + return true; + } + } - { - LOCK(g_cs_orphans); - if (!inv.IsMsgWtx() && mapOrphanTransactions.count(inv.hash)) { - return true; - } else if (inv.IsMsgWtx() && g_orphans_by_wtxid.count(inv.hash)) { - return true; - } - } + { + LOCK(g_cs_recent_confirmed_transactions); + if (g_recent_confirmed_transactions->contains(inv.hash)) return true; + } - { - LOCK(g_cs_recent_confirmed_transactions); - if (g_recent_confirmed_transactions->contains(inv.hash)) return true; - } + return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); +} - return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); - } - case MSG_BLOCK: - case MSG_WITNESS_BLOCK: - return LookupBlockIndex(inv.hash) != nullptr; - } - // Don't know what it is, just say we already got one - return true; +bool static AlreadyHaveBlock(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + return LookupBlockIndex(inv.hash) != nullptr; } void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) @@ -2670,10 +2661,10 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (inv.IsMsgWtx()) continue; } - bool fAlreadyHave = AlreadyHave(inv, m_mempool); - LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - if (inv.type == MSG_BLOCK) { + bool fAlreadyHave = AlreadyHaveBlock(inv, m_mempool); + LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); + UpdateBlockAvailability(pfrom.GetId(), inv.hash); if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) { // Headers-first is the primary method of announcement on @@ -2684,6 +2675,9 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty best_block = &inv.hash; } } else { + bool fAlreadyHave = AlreadyHaveTx(inv, mempool); + LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); + pfrom.AddKnownTx(inv.hash); if (fBlocksOnly) { LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId()); @@ -2963,7 +2957,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (!AlreadyHave(CInv(MSG_WTX, wtxid), m_mempool) && + if (!AlreadyHaveTx(CInv(MSG_WTX, wtxid), m_mempool) && AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { m_mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman); @@ -3017,7 +3011,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // protocol for getting all unconfirmed parents. CInv _inv(MSG_TX, parent_txid); pfrom.AddKnownTx(parent_txid); - if (!AlreadyHave(_inv, m_mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); + if (!AlreadyHaveTx(_inv, m_mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); } AddOrphanTx(ptx, pfrom.GetId()); @@ -4568,7 +4562,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); CInv inv(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), gtxid.GetHash()); - if (!AlreadyHave(inv, m_mempool)) { + if (!AlreadyHaveTx(inv, m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. const auto last_request_time = GetTxRequestTime(gtxid); -- cgit v1.2.3 From 430e183b89d00b4148f0b77a6fcacca2cd948202 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 30 Jul 2020 11:01:11 +0100 Subject: [net processing] Remove mempool argument from AlreadyHaveBlock() --- src/net_processing.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3a0613dee..3fb8907b5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1451,7 +1451,7 @@ bool static AlreadyHaveTx(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_ return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); } -bool static AlreadyHaveBlock(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool static AlreadyHaveBlock(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return LookupBlockIndex(inv.hash) != nullptr; } @@ -2662,7 +2662,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } if (inv.type == MSG_BLOCK) { - bool fAlreadyHave = AlreadyHaveBlock(inv, m_mempool); + bool fAlreadyHave = AlreadyHaveBlock(inv); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); UpdateBlockAvailability(pfrom.GetId(), inv.hash); -- cgit v1.2.3 From 5fdfb80b861e0de3fcf8a57163b3f52af4b2df3b Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 30 Jul 2020 11:02:40 +0100 Subject: [net processing] Change AlreadyHaveBlock() to take block_hash argument --- src/net_processing.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3fb8907b5..82d41d737 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1451,9 +1451,9 @@ bool static AlreadyHaveTx(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_ return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); } -bool static AlreadyHaveBlock(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - return LookupBlockIndex(inv.hash) != nullptr; + return LookupBlockIndex(block_hash) != nullptr; } void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) @@ -2662,7 +2662,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } if (inv.type == MSG_BLOCK) { - bool fAlreadyHave = AlreadyHaveBlock(inv); + bool fAlreadyHave = AlreadyHaveBlock(inv.hash); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); UpdateBlockAvailability(pfrom.GetId(), inv.hash); -- cgit v1.2.3 From acd66421671e42a58e8e067868e1ab86268e3231 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 30 Jul 2020 11:18:45 +0100 Subject: [net processing] Change AlreadyHaveTx() to take a GenTxid --- src/net_processing.cpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 82d41d737..490774358 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1422,7 +1422,7 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidatio // -bool static AlreadyHaveTx(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool static AlreadyHaveTx(const GenTxid& gtxid, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { assert(recentRejects); if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) { @@ -1436,19 +1436,19 @@ bool static AlreadyHaveTx(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_ { LOCK(g_cs_orphans); - if (!inv.IsMsgWtx() && mapOrphanTransactions.count(inv.hash)) { + if (!gtxid.IsWtxid() && mapOrphanTransactions.count(gtxid.GetHash())) { return true; - } else if (inv.IsMsgWtx() && g_orphans_by_wtxid.count(inv.hash)) { + } else if (gtxid.IsWtxid() && g_orphans_by_wtxid.count(gtxid.GetHash())) { return true; } } { LOCK(g_cs_recent_confirmed_transactions); - if (g_recent_confirmed_transactions->contains(inv.hash)) return true; + if (g_recent_confirmed_transactions->contains(gtxid.GetHash())) return true; } - return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); + return recentRejects->contains(gtxid.GetHash()) || mempool.exists(gtxid); } bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -2675,7 +2675,8 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty best_block = &inv.hash; } } else { - bool fAlreadyHave = AlreadyHaveTx(inv, mempool); + GenTxid gtxid = ToGenTxid(inv); + bool fAlreadyHave = AlreadyHaveTx(gtxid, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); pfrom.AddKnownTx(inv.hash); @@ -2684,7 +2685,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty pfrom.fDisconnect = true; return; } else if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { - RequestTx(State(pfrom.GetId()), ToGenTxid(inv), current_time); + RequestTx(State(pfrom.GetId()), gtxid, current_time); } } } @@ -2957,7 +2958,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (!AlreadyHaveTx(CInv(MSG_WTX, wtxid), m_mempool) && + if (!AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool) && AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { m_mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman); @@ -3009,9 +3010,9 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - CInv _inv(MSG_TX, parent_txid); + GenTxid gtxid{/* is_wtxid=*/false, parent_txid}; pfrom.AddKnownTx(parent_txid); - if (!AlreadyHaveTx(_inv, m_mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); + if (!AlreadyHaveTx(gtxid, m_mempool)) RequestTx(State(pfrom.GetId()), gtxid, current_time); } AddOrphanTx(ptx, pfrom.GetId()); @@ -4562,7 +4563,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); CInv inv(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), gtxid.GetHash()); - if (!AlreadyHaveTx(inv, m_mempool)) { + if (!AlreadyHaveTx(ToGenTxid(inv), m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. const auto last_request_time = GetTxRequestTime(gtxid); -- cgit v1.2.3 From b1c855453bf2634e7fd9b53c4a76a8536fc9865d Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 5 Aug 2020 17:35:05 +0200 Subject: p2p: use CInv block message helpers in net_processing.cpp --- src/net_processing.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 490774358..290853e0a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1555,7 +1555,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // disconnect node in case we have reached the outbound limit for serving historical blocks if (send && connman.OutboundTargetReached(true) && - (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && + (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); @@ -1581,7 +1581,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c std::shared_ptr pblock; if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { pblock = a_recent_block; - } else if (inv.type == MSG_WITNESS_BLOCK) { + } else if (inv.IsMsgWitnessBlk()) { // Fast-path: in this case it is possible to serve the block directly from disk, // as the network format matches the format on disk std::vector block_data; @@ -1598,12 +1598,11 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c pblock = pblockRead; } if (pblock) { - if (inv.type == MSG_BLOCK) + if (inv.IsMsgBlk()) { connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_WITNESS_BLOCK) + } else if (inv.IsMsgWitnessBlk()) { connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_FILTERED_BLOCK) - { + } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; if (pfrom.m_tx_relay != nullptr) { @@ -1627,9 +1626,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } // else // no response - } - else if (inv.type == MSG_CMPCT_BLOCK) - { + } else if (inv.IsMsgCmpctBlk()) { // If a peer is asking for old blocks, we're almost guaranteed // they won't have a useful mempool to match against a compact block, // and we don't feel like constructing the object for them, so @@ -1757,7 +1754,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm // expensive to process. if (it != pfrom.vRecvGetData.end() && !pfrom.fPauseSend) { const CInv &inv = *it++; - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK || inv.type == MSG_WITNESS_BLOCK) { + if (inv.IsGenBlkMsg()) { ProcessGetBlockData(pfrom, chainparams, inv, connman); } // else: If the first item on the queue is an unknown type, we erase it @@ -2661,7 +2658,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (inv.IsMsgWtx()) continue; } - if (inv.type == MSG_BLOCK) { + if (inv.IsMsgBlk()) { bool fAlreadyHave = AlreadyHaveBlock(inv.hash); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); -- cgit v1.2.3 From 24ee4f01eadb870435712950a1364cf0def06e9f Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sat, 8 Aug 2020 14:36:26 +0200 Subject: p2p: make gtxid(.hash) and fAlreadyHave localvars const --- src/net_processing.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 290853e0a..b9f0e1f98 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1434,21 +1434,23 @@ bool static AlreadyHaveTx(const GenTxid& gtxid, const CTxMemPool& mempool) EXCLU recentRejects->reset(); } + const uint256& hash = gtxid.GetHash(); + { LOCK(g_cs_orphans); - if (!gtxid.IsWtxid() && mapOrphanTransactions.count(gtxid.GetHash())) { + if (!gtxid.IsWtxid() && mapOrphanTransactions.count(hash)) { return true; - } else if (gtxid.IsWtxid() && g_orphans_by_wtxid.count(gtxid.GetHash())) { + } else if (gtxid.IsWtxid() && g_orphans_by_wtxid.count(hash)) { return true; } } { LOCK(g_cs_recent_confirmed_transactions); - if (g_recent_confirmed_transactions->contains(gtxid.GetHash())) return true; + if (g_recent_confirmed_transactions->contains(hash)) return true; } - return recentRejects->contains(gtxid.GetHash()) || mempool.exists(gtxid); + return recentRejects->contains(hash) || mempool.exists(gtxid); } bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -2645,8 +2647,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty const auto current_time = GetTime(); uint256* best_block{nullptr}; - for (CInv &inv : vInv) - { + for (CInv& inv : vInv) { if (interruptMsgProc) return; // Ignore INVs that don't match wtxidrelay setting. @@ -2659,7 +2660,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } if (inv.IsMsgBlk()) { - bool fAlreadyHave = AlreadyHaveBlock(inv.hash); + const bool fAlreadyHave = AlreadyHaveBlock(inv.hash); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); UpdateBlockAvailability(pfrom.GetId(), inv.hash); @@ -2672,8 +2673,8 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty best_block = &inv.hash; } } else { - GenTxid gtxid = ToGenTxid(inv); - bool fAlreadyHave = AlreadyHaveTx(gtxid, mempool); + const GenTxid gtxid = ToGenTxid(inv); + const bool fAlreadyHave = AlreadyHaveTx(gtxid, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); pfrom.AddKnownTx(inv.hash); @@ -3007,7 +3008,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - GenTxid gtxid{/* is_wtxid=*/false, parent_txid}; + const GenTxid gtxid{/* is_wtxid=*/false, parent_txid}; pfrom.AddKnownTx(parent_txid); if (!AlreadyHaveTx(gtxid, m_mempool)) RequestTx(State(pfrom.GetId()), gtxid, current_time); } -- cgit v1.2.3 From fb56d37612dea6666e7da73d671311a697570dae Mon Sep 17 00:00:00 2001 From: John Newbery Date: Fri, 21 Aug 2020 16:10:48 +0200 Subject: p2p: ensure inv is GenMsgTx before ToGenTxid in inv processing and otherwise log that an unknown INV type was received. In INV processing, when handling transaction type inv messages, ToGenTxid() expects that we constructed the CInv ourselves or that we verified that it is for a transaction type CInv. Therefore, change this `else` branch into an `else if (inv.GenMsgTx())` to make this safer and log any INVs that fall through. --- src/net_processing.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b9f0e1f98..0bd5e56dd 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2672,7 +2672,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // then fetch the blocks we need to catch up. best_block = &inv.hash; } - } else { + } else if (inv.IsGenTxMsg()) { const GenTxid gtxid = ToGenTxid(inv); const bool fAlreadyHave = AlreadyHaveTx(gtxid, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); @@ -2685,6 +2685,8 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } else if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { RequestTx(State(pfrom.GetId()), gtxid, current_time); } + } else { + LogPrint(BCLog::NET, "Unknown inv type \"%s\" received from peer=%d\n", inv.ToString(), pfrom.GetId()); } } -- cgit v1.2.3