diff options
Diffstat (limited to 'src/net_processing.cpp')
| -rw-r--r-- | src/net_processing.cpp | 511 |
1 files changed, 349 insertions, 162 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4321005212..f63abca847 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -38,7 +38,9 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; /** How long to cache transactions in mapRelay for normal relay */ -static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME{15 * 60}; +static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME = std::chrono::minutes{15}; +/** How long a transaction has to be in the mempool before it can unconditionally be relayed (even when not in mapRelay). */ +static constexpr std::chrono::seconds UNCONDITIONAL_RELAY_DELAY = std::chrono::minutes{2}; /** Headers download timeout expressed in microseconds * Timeout = base + per_header * (expected number of headers) */ static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes @@ -73,6 +75,8 @@ static const unsigned int MAX_INV_SZ = 50000; static constexpr int32_t MAX_PEER_TX_IN_FLIGHT = 100; /** Maximum number of announced transactions from a peer */ static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 2 * MAX_INV_SZ; +/** How many microseconds to delay requesting transactions via txids, if we have wtxid-relaying peers */ +static constexpr std::chrono::microseconds TXID_RELAY_DELAY{std::chrono::seconds{2}}; /** How many microseconds to delay requesting transactions from inbound peers */ static constexpr std::chrono::microseconds INBOUND_PEER_TX_DELAY{std::chrono::seconds{2}}; /** How long to wait (in microseconds) before downloading a transaction from an additional peer */ @@ -117,11 +121,20 @@ static constexpr std::chrono::hours AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24}; /** Average delay between peer address broadcasts */ static constexpr std::chrono::seconds AVG_ADDRESS_BROADCAST_INTERVAL{30}; /** Average delay between trickled inventory transmissions in seconds. - * Blocks and whitelisted receivers bypass this, outbound peers get half this delay. */ + * Blocks and peers with noban permission bypass this, outbound peers get half this delay. */ static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5; -/** Maximum number of inventory items to send per transmission. +/** Maximum rate of inventory items to send per second. * Limits the impact of low-fee transaction floods. */ -static constexpr unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_INTERVAL; +static constexpr unsigned int INVENTORY_BROADCAST_PER_SECOND = 7; +/** Maximum number of inventory items to send per transmission. */ +static constexpr unsigned int INVENTORY_BROADCAST_MAX = INVENTORY_BROADCAST_PER_SECOND * INVENTORY_BROADCAST_INTERVAL; +/** The number of most recently announced transactions a peer can request. */ +static constexpr unsigned int INVENTORY_MAX_RECENT_RELAY = 3500; +/** Verify that INVENTORY_MAX_RECENT_RELAY is enough to cache everything typically + * relayed before unconditional relay from the mempool kicks in. This is only a + * lower bound, and it should be larger to account for higher inv rate to outbound + * peers, and random variations in the broadcast mechanism. */ +static_assert(INVENTORY_MAX_RECENT_RELAY >= INVENTORY_BROADCAST_PER_SECOND * UNCONDITIONAL_RELAY_DELAY / std::chrono::seconds{1}, "INVENTORY_RELAY_MAX too low"); /** Average delay between feefilter broadcasts in seconds. */ static constexpr unsigned int AVG_FEEFILTER_BROADCAST_INTERVAL = 10 * 60; /** Maximum feefilter broadcast delay after significant change. */ @@ -140,6 +153,7 @@ struct COrphanTx { }; RecursiveMutex g_cs_orphans; std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans); +std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans); void EraseOrphansFor(NodeId peer); @@ -176,6 +190,15 @@ namespace { * million to make it highly unlikely for users to have issues with this * filter. * + * We only need to add wtxids to this filter. For non-segwit + * transactions, the txid == wtxid, so this only prevents us from + * re-downloading non-segwit transactions when communicating with + * non-wtxidrelay peers -- which is important for avoiding malleation + * attacks that could otherwise interfere with transaction relay from + * non-wtxidrelay peers. For communicating with wtxidrelay peers, having + * the reject filter store wtxids is exactly what we want to avoid + * redownload of a rejected transaction. + * * Memory used: 1.3 MB */ std::unique_ptr<CRollingBloomFilter> recentRejects GUARDED_BY(cs_main); @@ -207,6 +230,9 @@ namespace { /** Number of peers from which we're downloading blocks. */ int nPeersWithValidatedDownloads GUARDED_BY(cs_main) = 0; + /** Number of peers with wtxid relay. */ + int g_wtxid_relay_peers GUARDED_BY(cs_main) = 0; + /** Number of outbound peers with m_chain_sync.m_protect. */ int g_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; @@ -249,7 +275,7 @@ struct CNodeState { bool fCurrentlyConnected; //! Accumulated misbehaviour score for this peer. int nMisbehavior; - //! Whether this peer should be disconnected and marked as discouraged (unless whitelisted with noban). + //! Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). bool m_should_discourage; //! String name of this peer (debugging/logging purposes). const std::string name; @@ -395,6 +421,12 @@ struct CNodeState { //! Whether this peer is a manual connection bool m_is_manual_connection; + //! A rolling bloom filter of all announced tx CInvs to this peer. + CRollingBloomFilter m_recently_announced_invs = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; + + //! Whether this peer relays txs via wtxid + bool m_wtxid_relay{false}; + CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) : address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound), m_is_manual_connection (is_manual) @@ -422,6 +454,7 @@ struct CNodeState { fSupportsDesiredCmpctVersion = false; m_chain_sync = { 0, nullptr, false, false }; m_last_block_announcement = 0; + m_recently_announced_invs.reset(); } }; @@ -448,7 +481,7 @@ static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUS nPreferredDownload += state->fPreferredDownload; } -static void PushNodeVersion(CNode& pnode, CConnman* connman, int64_t nTime) +static void PushNodeVersion(CNode& pnode, CConnman& connman, int64_t nTime) { // Note that pnode->GetLocalServices() is a reflection of the local // services we were offering when the CNode object was created for this @@ -462,7 +495,7 @@ static void PushNodeVersion(CNode& pnode, CConnman* connman, int64_t nTime) CAddress addrYou = (addr.IsRoutable() && !IsProxy(addr) ? addr : CAddress(CService(), addr.nServices)); CAddress addrMe = CAddress(CService(), nLocalNodeServices); - connman->PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe, + connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe, nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && pnode.m_tx_relay != nullptr)); if (fLogIPs) { @@ -573,7 +606,7 @@ static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) EXCLUSIV * lNodesAnnouncingHeaderAndIDs, and keeping that list under a certain size by * removing the first element if necessary. */ -static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); CNodeState* nodestate = State(nodeid); @@ -589,20 +622,20 @@ static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connma return; } } - connman->ForNode(nodeid, [connman](CNode* pfrom){ + connman.ForNode(nodeid, [&connman](CNode* pfrom){ AssertLockHeld(cs_main); uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1; if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { // As per BIP152, we only get 3 of our peers to announce // blocks using compact encodings. - connman->ForNode(lNodesAnnouncingHeaderAndIDs.front(), [connman, nCMPCTBLOCKVersion](CNode* pnodeStop){ + connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [&connman, nCMPCTBLOCKVersion](CNode* pnodeStop){ AssertLockHeld(cs_main); - connman->PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); + connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); return true; }); lNodesAnnouncingHeaderAndIDs.pop_front(); } - connman->PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/true, nCMPCTBLOCKVersion)); + connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/true, nCMPCTBLOCKVersion)); lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); return true; }); @@ -745,7 +778,7 @@ void UpdateTxRequestTime(const uint256& txid, std::chrono::microseconds request_ } } -std::chrono::microseconds CalculateTxGetDataTime(const uint256& txid, std::chrono::microseconds current_time, bool use_inbound_delay) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +std::chrono::microseconds CalculateTxGetDataTime(const uint256& txid, std::chrono::microseconds current_time, bool use_inbound_delay, bool use_txid_delay) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::chrono::microseconds process_time; const auto last_request_time = GetTxRequestTime(txid); @@ -761,6 +794,9 @@ std::chrono::microseconds CalculateTxGetDataTime(const uint256& txid, std::chron // We delay processing announcements from inbound peers if (use_inbound_delay) process_time += INBOUND_PEER_TX_DELAY; + // We delay processing announcements from peers that use txid-relay (instead of wtxid) + if (use_txid_delay) process_time += TXID_RELAY_DELAY; + return process_time; } @@ -778,7 +814,7 @@ void RequestTx(CNodeState* state, const uint256& txid, std::chrono::microseconds // Calculate the time to try requesting this transaction. Use // fPreferredDownload as a proxy for outbound peers. - const auto process_time = CalculateTxGetDataTime(txid, current_time, !state->fPreferredDownload); + const auto process_time = CalculateTxGetDataTime(txid, current_time, !state->fPreferredDownload, !state->m_wtxid_relay && g_wtxid_relay_peers > 0); peer_download_state.m_tx_process_time.emplace(process_time, txid); } @@ -810,19 +846,20 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->fInbound, pnode->m_manual_connection)); } if(!pnode->fInbound) - PushNodeVersion(*pnode, connman, GetTime()); + PushNodeVersion(*pnode, *connman, GetTime()); } void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const { - std::set<uint256> unbroadcast_txids = m_mempool.GetUnbroadcastTxs(); + std::map<uint256, uint256> unbroadcast_txids = m_mempool.GetUnbroadcastTxs(); - for (const uint256& txid : unbroadcast_txids) { + for (const auto& elem : unbroadcast_txids) { // Sanity check: all unbroadcast txns should exist in the mempool - if (m_mempool.exists(txid)) { - RelayTransaction(txid, *connman); + if (m_mempool.exists(elem.first)) { + LOCK(cs_main); + RelayTransaction(elem.first, elem.second, *connman); } else { - m_mempool.RemoveUnbroadcastTx(txid, true); + m_mempool.RemoveUnbroadcastTx(elem.first, true); } } @@ -854,6 +891,8 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim assert(nPeersWithValidatedDownloads >= 0); g_outbound_peers_with_protect_from_disconnect -= state->m_chain_sync.m_protect; assert(g_outbound_peers_with_protect_from_disconnect >= 0); + g_wtxid_relay_peers -= state->m_wtxid_relay; + assert(g_wtxid_relay_peers >= 0); mapNodeState.erase(nodeid); @@ -863,6 +902,7 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim assert(nPreferredDownload == 0); assert(nPeersWithValidatedDownloads == 0); assert(g_outbound_peers_with_protect_from_disconnect == 0); + assert(g_wtxid_relay_peers == 0); } LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } @@ -921,6 +961,8 @@ bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRE auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, g_orphan_list.size()}); assert(ret.second); g_orphan_list.push_back(ret.first); + // Allow for lookups in the orphan pool by wtxid, as well as txid + g_orphans_by_wtxid.emplace(tx->GetWitnessHash(), ret.first); for (const CTxIn& txin : tx->vin) { mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); } @@ -957,6 +999,7 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) it_last->second.list_pos = old_pos; } g_orphan_list.pop_back(); + g_orphans_by_wtxid.erase(it->second.tx->GetWitnessHash()); mapOrphanTransactions.erase(it); return 1; @@ -1016,7 +1059,8 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) } /** - * Increment peer's misbehavior score. If the new value surpasses banscore (specified on startup or by default), mark node to be discouraged, meaning the peer might be disconnected & added to the discouragement filter. + * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node + * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. */ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -1028,9 +1072,8 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV return; state->nMisbehavior += howmuch; - int banscore = gArgs.GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD); std::string message_prefixed = message.empty() ? "" : (": " + message); - if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore) + if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) { LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); state->m_should_discourage = true; @@ -1126,6 +1169,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, case TxValidationResult::TX_MISSING_INPUTS: case TxValidationResult::TX_PREMATURE_SPEND: case TxValidationResult::TX_WITNESS_MUTATED: + case TxValidationResult::TX_WITNESS_STRIPPED: case TxValidationResult::TX_CONFLICT: case TxValidationResult::TX_MEMPOOL_POLICY: break; @@ -1166,14 +1210,15 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CS recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); // Blocks don't typically have more than 4000 transactions, so this should - // be at least six blocks (~1 hr) worth of transactions that we can store. + // be at least six blocks (~1 hr) worth of transactions that we can store, + // inserting both a txid and wtxid for every observed transaction. // If the number of transactions appearing in a block goes up, or if we are // seeing getdata requests more than an hour after initial announcement, we // can increase this number. // The false positive rate of 1/1M should come out to less than 1 // transaction per day that would be inadvertently ignored (which is the // same probability that we have in the reject filter). - g_recent_confirmed_transactions.reset(new CRollingBloomFilter(24000, 0.000001)); + g_recent_confirmed_transactions.reset(new CRollingBloomFilter(48000, 0.000001)); const Consensus::Params& consensusParams = Params().GetConsensus(); // Stale tip checking and peer eviction are on two different timers, but we @@ -1229,6 +1274,9 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pb LOCK(g_cs_recent_confirmed_transactions); for (const auto& ptx : pblock->vtx) { g_recent_confirmed_transactions->insert(ptx->GetHash()); + if (ptx->GetHash() != ptx->GetWitnessHash()) { + g_recent_confirmed_transactions->insert(ptx->GetWitnessHash()); + } } } } @@ -1363,7 +1411,7 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidatio !::ChainstateActive().IsInitialBlockDownload() && mapBlocksInFlight.count(hash) == mapBlocksInFlight.size()) { if (it != mapBlockSource.end()) { - MaybeSetPeerAsAnnouncingHeaderAndIDs(it->second.first, connman); + MaybeSetPeerAsAnnouncingHeaderAndIDs(it->second.first, *connman); } } if (it != mapBlockSource.end()) @@ -1382,6 +1430,7 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO { case MSG_TX: case MSG_WITNESS_TX: + case MSG_WTX: { assert(recentRejects); if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) @@ -1396,7 +1445,11 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO { LOCK(g_cs_orphans); - if (mapOrphanTransactions.count(inv.hash)) return true; + if (inv.type != MSG_WTX && mapOrphanTransactions.count(inv.hash)) { + return true; + } else if (inv.type == MSG_WTX && g_orphans_by_wtxid.count(inv.hash)) { + return true; + } } { @@ -1404,8 +1457,8 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO if (g_recent_confirmed_transactions->contains(inv.hash)) return true; } - return recentRejects->contains(inv.hash) || - mempool.exists(inv.hash); + const bool by_wtxid = (inv.type == MSG_WTX); + return recentRejects->contains(inv.hash) || mempool.exists(inv.hash, by_wtxid); } case MSG_BLOCK: case MSG_WITNESS_BLOCK: @@ -1415,11 +1468,17 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO return true; } -void RelayTransaction(const uint256& txid, const CConnman& connman) +void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) { - connman.ForEachNode([&txid](CNode* pnode) + connman.ForEachNode([&txid, &wtxid](CNode* pnode) { - pnode->PushTxInventory(txid); + AssertLockHeld(cs_main); + CNodeState &state = *State(pnode->GetId()); + if (state.m_wtxid_relay) { + pnode->PushTxInventory(wtxid); + } else { + pnode->PushTxInventory(txid); + } }); } @@ -1459,7 +1518,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& connman.ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); } -void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, const CInv& inv, CConnman* connman) +void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, const CInv& inv, CConnman& connman) { bool send = false; std::shared_ptr<const CBlock> a_recent_block; @@ -1507,7 +1566,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); // disconnect node in case we have reached the outbound limit for serving historical blocks if (send && - connman->OutboundTargetReached(true) && + connman.OutboundTargetReached(true) && (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target ) { @@ -1541,7 +1600,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c if (!ReadRawBlockFromDisk(block_data, pindex, chainparams.MessageStart())) { assert(!"cannot load block from disk"); } - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, MakeSpan(block_data))); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, MakeSpan(block_data))); // Don't set pblock as we've sent the block } else { // Send block from disk @@ -1552,9 +1611,9 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } if (pblock) { if (inv.type == MSG_BLOCK) - connman->PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock)); + connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock)); else if (inv.type == MSG_WITNESS_BLOCK) - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); else if (inv.type == MSG_FILTERED_BLOCK) { bool sendMerkleBlock = false; @@ -1567,7 +1626,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } } if (sendMerkleBlock) { - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see // This avoids hurting performance by pointlessly requiring a round-trip // Note that there is currently no way for a node to request any single transactions we didn't send here - @@ -1576,7 +1635,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // however we MUST always provide at least what the remote peer needs typedef std::pair<unsigned int, uint256> PairType; for (PairType& pair : merkleBlock.vMatchedTxn) - connman->PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, *pblock->vtx[pair.first])); + connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, *pblock->vtx[pair.first])); } // else // no response @@ -1591,13 +1650,13 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c int nSendFlags = fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; if (CanDirectFetch(consensusParams) && pindex->nHeight >= ::ChainActive().Height() - MAX_CMPCTBLOCK_DEPTH) { if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { - connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); + connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); } else { CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness); - connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); } } else { - connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); + connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); } } } @@ -1610,44 +1669,41 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // wait for other stuff first. std::vector<CInv> vInv; vInv.push_back(CInv(MSG_BLOCK, ::ChainActive().Tip()->GetBlockHash())); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); pfrom.hashContinue.SetNull(); } } } //! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). -CTransactionRef static FindTxForGetData(CNode& peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) LOCKS_EXCLUDED(cs_main) +CTransactionRef static FindTxForGetData(const CNode& peer, const uint256& txid_or_wtxid, bool use_wtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main) { - // Check if the requested transaction is so recent that we're just - // about to announce it to the peer; if so, they certainly shouldn't - // know we already have it. - { - LOCK(peer.m_tx_relay->cs_tx_inventory); - if (peer.m_tx_relay->setInventoryTxToSend.count(txid)) return {}; + auto txinfo = mempool.info(txid_or_wtxid, use_wtxid); + if (txinfo.tx) { + // If a TX could have been INVed in reply to a MEMPOOL request, + // or is older than UNCONDITIONAL_RELAY_DELAY, permit the request + // unconditionally. + if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) { + return std::move(txinfo.tx); + } } { LOCK(cs_main); - // Look up transaction in relay pool - auto mi = mapRelay.find(txid); - if (mi != mapRelay.end()) return mi->second; - } - - auto txinfo = mempool.info(txid); - if (txinfo.tx) { - // To protect privacy, do not answer getdata using the mempool when - // that TX couldn't have been INVed in reply to a MEMPOOL request, - // or when it's too recent to have expired from mapRelay. - if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= longlived_mempool_time) { - return txinfo.tx; + // Otherwise, the transaction must have been announced recently. + if (State(peer.GetId())->m_recently_announced_invs.contains(txid_or_wtxid)) { + // If it was, it can be relayed from either the mempool... + if (txinfo.tx) return std::move(txinfo.tx); + // ... or the relay pool. + auto mi = mapRelay.find(txid_or_wtxid); + if (mi != mapRelay.end()) return mi->second; } } return {}; } -void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnman* connman, CTxMemPool& mempool, const std::atomic<bool>& interruptMsgProc) LOCKS_EXCLUDED(cs_main) +void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnman& connman, CTxMemPool& mempool, const std::atomic<bool>& interruptMsgProc) LOCKS_EXCLUDED(cs_main) { AssertLockNotHeld(cs_main); @@ -1655,8 +1711,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm std::vector<CInv> vNotFound; const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); - // mempool entries added before this time have likely expired from mapRelay - const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME; + const std::chrono::seconds now = GetTime<std::chrono::seconds>(); // Get last mempool request time const std::chrono::seconds mempool_req = pfrom.m_tx_relay != nullptr ? pfrom.m_tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); @@ -1664,7 +1719,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm // Process as many TX items from the front of the getdata queue as // possible, since they're common and it's efficient to batch process // them. - while (it != pfrom.vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) { + while (it != pfrom.vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX || it->type == MSG_WTX)) { if (interruptMsgProc) return; // The send buffer provides backpressure. If there's no space in // the buffer, pause processing until the next call. @@ -1677,11 +1732,23 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm continue; } - CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, mempool_req, longlived_mempool_time); + CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, inv.type == MSG_WTX, mempool_req, now); if (tx) { + // WTX and WITNESS_TX imply we serialize with witness int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); - connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); - mempool.RemoveUnbroadcastTx(inv.hash); + connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); + mempool.RemoveUnbroadcastTx(tx->GetHash()); + // As we're going to send tx, make sure its unconfirmed parents are made requestable. + for (const auto& txin : tx->vin) { + auto txinfo = mempool.info(txin.prevout.hash); + if (txinfo.tx && txinfo.m_time > now - UNCONDITIONAL_RELAY_DELAY) { + // Relaying a transaction with a recent but unconfirmed parent. + if (WITH_LOCK(pfrom.m_tx_relay->cs_tx_inventory, return !pfrom.m_tx_relay->filterInventoryKnown.contains(txin.prevout.hash))) { + LOCK(cs_main); + State(pfrom.GetId())->m_recently_announced_invs.insert(txin.prevout.hash); + } + } + } } else { vNotFound.push_back(inv); } @@ -1715,7 +1782,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm // In normal operation, we often send NOTFOUND messages for parents of // transactions that we relay; if a peer is missing a parent, they may // assume we have them and request the parents from us. - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::NOTFOUND, vNotFound)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::NOTFOUND, vNotFound)); } } @@ -1727,7 +1794,7 @@ static uint32_t GetFetchFlags(const CNode& pfrom) EXCLUSIVE_LOCKS_REQUIRED(cs_ma return nFetchFlags; } -inline void static SendBlockTransactions(const CBlock& block, const BlockTransactionsRequest& req, CNode& pfrom, CConnman* connman) { +inline void static SendBlockTransactions(const CBlock& block, const BlockTransactionsRequest& req, CNode& pfrom, CConnman& connman) { BlockTransactions resp(req); for (size_t i = 0; i < req.indexes.size(); i++) { if (req.indexes[i] >= block.vtx.size()) { @@ -1740,10 +1807,10 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac LOCK(cs_main); const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); int nSendFlags = State(pfrom.GetId())->fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); + connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); } -static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block) +static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block) { const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); size_t nCount = headers.size(); @@ -1769,7 +1836,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan // nUnconnectingHeaders gets reset back to 0. if (!LookupBlockIndex(headers[0].hashPrevBlock) && nCount < MAX_BLOCKS_TO_ANNOUNCE) { nodestate->nUnconnectingHeaders++; - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", headers[0].GetHash().ToString(), headers[0].hashPrevBlock.ToString(), @@ -1834,7 +1901,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan // TODO: optimize: if pindexLast is an ancestor of ::ChainActive().Tip or pindexBestHeader, continue // from there instead. LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom.GetId(), pfrom.nStartingHeight); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexLast), uint256())); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexLast), uint256())); } bool fCanDirectFetch = CanDirectFetch(chainparams.GetConsensus()); @@ -1884,7 +1951,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vGetData)); } } } @@ -1895,8 +1962,8 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan // headers to fetch from this peer. if (nodestate->pindexBestKnownBlock && nodestate->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { // This peer has too little work on their headers chain to help - // us sync -- disconnect if using an outbound slot (unless - // whitelisted or addnode). + // us sync -- disconnect if it is an outbound disconnection + // candidate. // Note: We compare their tip to nMinimumChainWork (rather than // ::ChainActive().Tip()) because we won't start block download // until we have a headers chain that has at least @@ -1925,7 +1992,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan return; } -void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uint256>& orphan_work_set, std::list<CTransactionRef>& removed_txn) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) +void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uint256>& orphan_work_set, std::list<CTransactionRef>& removed_txn) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) { AssertLockHeld(cs_main); AssertLockHeld(g_cs_orphans); @@ -1949,7 +2016,7 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uin if (setMisbehaving.count(fromPeer)) continue; if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); - RelayTransaction(orphanHash, *connman); + RelayTransaction(orphanHash, porphanTx->GetWitnessHash(), connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(orphanHash, i)); if (it_by_prev != mapOrphanTransactionsByPrev.end()) { @@ -1966,17 +2033,30 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uin if (MaybePunishNodeForTx(fromPeer, orphan_state)) { setMisbehaving.insert(fromPeer); } - LogPrint(BCLog::MEMPOOL, " invalid orphan tx %s\n", orphanHash.ToString()); + LogPrint(BCLog::MEMPOOL, " invalid orphan tx %s from peer=%d. %s\n", + orphanHash.ToString(), + fromPeer, + orphan_state.ToString()); } // Has inputs but not accepted to mempool // Probably non-standard or insufficient fee LogPrint(BCLog::MEMPOOL, " removed orphan tx %s\n", orphanHash.ToString()); - if (!orphanTx.HasWitness() && orphan_state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { - // Do not use rejection cache for witness transactions or - // witness-stripped transactions, as they can have been malleated. - // See https://github.com/bitcoin/bitcoin/issues/8279 for details. + if (orphan_state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { + // We can add the wtxid of this transaction to our reject filter. + // Do not add txids of witness transactions or witness-stripped + // transactions to the filter, as they can have been malleated; + // adding such txids to the reject filter would potentially + // interfere with relay of valid transactions from peers that + // do not support wtxid-based relay. See + // https://github.com/bitcoin/bitcoin/issues/8279 for details. + // We can remove this restriction (and always add wtxids to + // the filter even for witness stripped transactions) once + // wtxid-based relay is broadly deployed. + // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 + // for concerns around weakening security of unupgraded nodes + // if we start doing this too early. assert(recentRejects); - recentRejects->insert(orphanHash); + recentRejects->insert(orphanTx.GetWitnessHash()); } EraseOrphanTx(orphanHash); done = true; @@ -2210,7 +2290,7 @@ void ProcessMessage( const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, - CConnman* connman, + CConnman& connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc) { @@ -2248,7 +2328,7 @@ void ProcessMessage( nServices = ServiceFlags(nServiceInt); if (!pfrom.fInbound) { - connman->SetServices(pfrom.addr, nServices); + connman.SetServices(pfrom.addr, nServices); } if (!pfrom.fInbound && !pfrom.fFeeler && !pfrom.m_manual_connection && !HasAllDesirableServiceFlags(nServices)) { @@ -2277,7 +2357,7 @@ void ProcessMessage( if (!vRecv.empty()) vRecv >> fRelay; // Disconnect if we connected to ourself - if (pfrom.fInbound && !connman->CheckIncomingNonce(nNonce)) + if (pfrom.fInbound && !connman.CheckIncomingNonce(nNonce)) { LogPrintf("connected to self at %s, disconnecting\n", pfrom.addr.ToString()); pfrom.fDisconnect = true; @@ -2293,7 +2373,11 @@ void ProcessMessage( if (pfrom.fInbound) PushNodeVersion(pfrom, connman, GetAdjustedTime()); - connman->PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); + if (nVersion >= WTXID_RELAY_VERSION) { + connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::WTXIDRELAY)); + } + + connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); pfrom.nServices = nServices; pfrom.SetAddrLocal(addrMe); @@ -2349,9 +2433,9 @@ void ProcessMessage( } // Get recent addresses - connman->PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); + connman.PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); pfrom.fGetAddr = true; - connman->MarkAddressGood(pfrom.addr); + connman.MarkAddressGood(pfrom.addr); } std::string remoteAddr; @@ -2370,7 +2454,7 @@ void ProcessMessage( // If the peer is old enough to have the old alert system, send it the final alert. if (pfrom.nVersion <= 70012) { CDataStream finalAlert(ParseHex("60010000000000000000000000ffffff7f00000000ffffff7ffeffff7f01ffffff7f00000000ffffff7f00ffffff7f002f555247454e543a20416c657274206b657920636f6d70726f6d697365642c2075706772616465207265717569726564004630440220653febd6410f470f6bae11cad19c48413becb1ac2c17f908fd0fd53bdc3abd5202206d0e9c96fe88d4a0f01ed9dedae2b6f9e00da94cad0fecaae66ecf689bf71b50"), SER_NETWORK, PROTOCOL_VERSION); - connman->PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make("alert", finalAlert)); + connman.PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make("alert", finalAlert)); } // Feeler connections exist only to verify if address is online. @@ -2410,7 +2494,7 @@ void ProcessMessage( // We send this to non-NODE NETWORK peers as well, because even // non-NODE NETWORK peers can announce blocks (such as pruning // nodes) - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDHEADERS)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDHEADERS)); } if (pfrom.nVersion >= SHORT_IDS_BLOCKS_VERSION) { // Tell our peer we are willing to provide version 1 or 2 cmpctblocks @@ -2421,14 +2505,33 @@ void ProcessMessage( bool fAnnounceUsingCMPCTBLOCK = false; uint64_t nCMPCTBLOCKVersion = 2; if (pfrom.GetLocalServices() & NODE_WITNESS) - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); nCMPCTBLOCKVersion = 1; - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); } pfrom.fSuccessfullyConnected = true; return; } + // Feature negotiation of wtxidrelay should happen between VERSION and + // VERACK, to avoid relay problems from switching after a connection is up + if (msg_type == NetMsgType::WTXIDRELAY) { + if (pfrom.fSuccessfullyConnected) { + // Disconnect peers that send wtxidrelay message after VERACK; this + // must be negotiated between VERSION and VERACK. + pfrom.fDisconnect = true; + return; + } + if (pfrom.nVersion >= WTXID_RELAY_VERSION) { + LOCK(cs_main); + if (!State(pfrom.GetId())->m_wtxid_relay) { + State(pfrom.GetId())->m_wtxid_relay = true; + g_wtxid_relay_peers++; + } + } + return; + } + if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else LOCK(cs_main); @@ -2468,19 +2571,21 @@ void ProcessMessage( if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60) addr.nTime = nNow - 5 * 24 * 60 * 60; pfrom.AddAddressKnown(addr); - if (banman->IsDiscouraged(addr)) continue; // Do not process banned/discouraged addresses beyond remembering we received them - if (banman->IsBanned(addr)) continue; + if (banman && (banman->IsDiscouraged(addr) || banman->IsBanned(addr))) { + // Do not process banned/discouraged addresses beyond remembering we received them + continue; + } bool fReachable = IsReachable(addr); if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable()) { // Relay to a limited number of other nodes - RelayAddress(addr, fReachable, *connman); + RelayAddress(addr, fReachable, connman); } // Do not store addresses outside our network if (fReachable) vAddrOk.push_back(addr); } - connman->AddNewAddresses(vAddrOk, pfrom.addr, 2 * 60 * 60); + connman.AddNewAddresses(vAddrOk, pfrom.addr, 2 * 60 * 60); if (vAddr.size() < 1000) pfrom.fGetAddr = false; if (pfrom.fOneShot) @@ -2531,9 +2636,10 @@ void ProcessMessage( // block-relay-only peer bool fBlocksOnly = !g_relay_txes || (pfrom.m_tx_relay == nullptr); - // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true - if (pfrom.HasPermission(PF_RELAY)) + // Allow peers with relay permission to send data other than blocks in blocks only mode + if (pfrom.HasPermission(PF_RELAY)) { fBlocksOnly = false; + } LOCK(cs_main); @@ -2546,6 +2652,13 @@ void ProcessMessage( if (interruptMsgProc) return; + // ignore INVs that don't match wtxidrelay setting + if (State(pfrom.GetId())->m_wtxid_relay) { + if (inv.type == MSG_TX) continue; + } else { + if (inv.type == MSG_WTX) continue; + } + bool fAlreadyHave = AlreadyHave(inv, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); @@ -2564,7 +2677,7 @@ void ProcessMessage( best_block = &inv.hash; } } else { - pfrom.AddInventoryKnown(inv); + 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()); pfrom.fDisconnect = true; @@ -2576,7 +2689,7 @@ void ProcessMessage( } if (best_block != nullptr) { - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), *best_block)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), *best_block)); LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, best_block->ToString(), pfrom.GetId()); } @@ -2784,7 +2897,7 @@ void ProcessMessage( // will re-announce the new block via headers (or compact blocks again) // in the SendMessages logic. nodestate->pindexBestHeaderSent = pindex ? pindex : ::ChainActive().Tip(); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, vHeaders)); return; } @@ -2803,26 +2916,50 @@ void ProcessMessage( vRecv >> ptx; const CTransaction& tx = *ptx; - CInv inv(MSG_TX, tx.GetHash()); - pfrom.AddInventoryKnown(inv); + const uint256& txid = ptx->GetHash(); + const uint256& wtxid = ptx->GetWitnessHash(); LOCK2(cs_main, g_cs_orphans); + CNodeState* nodestate = State(pfrom.GetId()); + + const uint256& hash = nodestate->m_wtxid_relay ? wtxid : txid; + pfrom.AddKnownTx(hash); + if (nodestate->m_wtxid_relay && txid != wtxid) { + // Insert txid into filterInventoryKnown, even for + // wtxidrelay peers. This prevents re-adding of + // unconfirmed parents to the recently_announced + // filter, when a child tx is requested. See + // ProcessGetData(). + pfrom.AddKnownTx(txid); + } + TxValidationState state; - CNodeState* nodestate = State(pfrom.GetId()); - nodestate->m_tx_download.m_tx_announced.erase(inv.hash); - nodestate->m_tx_download.m_tx_in_flight.erase(inv.hash); - EraseTxRequest(inv.hash); + nodestate->m_tx_download.m_tx_announced.erase(hash); + nodestate->m_tx_download.m_tx_in_flight.erase(hash); + EraseTxRequest(hash); std::list<CTransactionRef> lRemovedTxn; - if (!AlreadyHave(inv, mempool) && + // We do the AlreadyHave() check using wtxid, rather than txid - in the + // absence of witness malleation, this is strictly better, because the + // recent rejects filter may contain the wtxid but will never contain + // the txid of a segwit transaction that has been rejected. + // In the presence of witness malleation, it's possible that by only + // doing the check with wtxid, we could overlook a transaction which + // was confirmed with a different witness, or exists in our mempool + // with a different witness, but this has limited downside: + // mempool validation does its own lookup of whether we have the txid + // 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), mempool) && AcceptToMemoryPool(mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { mempool.check(&::ChainstateActive().CoinsTip()); - RelayTransaction(tx.GetHash(), *connman); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), connman); for (unsigned int i = 0; i < tx.vout.size(); i++) { - auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(inv.hash, i)); + auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(txid, i)); if (it_by_prev != mapOrphanTransactionsByPrev.end()) { for (const auto& elem : it_by_prev->second) { pfrom.orphan_work_set.insert(elem->first); @@ -2853,10 +2990,17 @@ void ProcessMessage( uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime<std::chrono::microseconds>(); - for (const CTxIn& txin : tx.vin) { - CInv _inv(MSG_TX | nFetchFlags, txin.prevout.hash); - pfrom.AddInventoryKnown(_inv); - if (!AlreadyHave(_inv, mempool)) RequestTx(State(pfrom.GetId()), _inv.hash, current_time); + if (!State(pfrom.GetId())->m_wtxid_relay) { + for (const CTxIn& txin : tx.vin) { + // Here, we only have the txid (and not wtxid) of the + // inputs, so we only request parents from + // non-wtxid-relay peers. + // Eventually we should replace this with an improved + // protocol for getting all unconfirmed parents. + CInv _inv(MSG_TX | nFetchFlags, txin.prevout.hash); + pfrom.AddKnownTx(txin.prevout.hash); + if (!AlreadyHave(_inv, mempool)) RequestTx(State(pfrom.GetId()), _inv.hash, current_time); + } } AddOrphanTx(ptx, pfrom.GetId()); @@ -2870,15 +3014,30 @@ void ProcessMessage( LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString()); // We will continue to reject this tx since it has rejected // parents so avoid re-requesting it from other peers. + // Here we add both the txid and the wtxid, as we know that + // regardless of what witness is provided, we will not accept + // this, so we don't need to allow for redownload of this txid + // from any of our non-wtxidrelay peers. recentRejects->insert(tx.GetHash()); + recentRejects->insert(tx.GetWitnessHash()); } } else { - if (!tx.HasWitness() && state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { - // Do not use rejection cache for witness transactions or - // witness-stripped transactions, as they can have been malleated. - // See https://github.com/bitcoin/bitcoin/issues/8279 for details. + if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { + // We can add the wtxid of this transaction to our reject filter. + // Do not add txids of witness transactions or witness-stripped + // transactions to the filter, as they can have been malleated; + // adding such txids to the reject filter would potentially + // interfere with relay of valid transactions from peers that + // do not support wtxid-based relay. See + // https://github.com/bitcoin/bitcoin/issues/8279 for details. + // We can remove this restriction (and always add wtxids to + // the filter even for witness stripped transactions) once + // wtxid-based relay is broadly deployed. + // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 + // for concerns around weakening security of unupgraded nodes + // if we start doing this too early. assert(recentRejects); - recentRejects->insert(tx.GetHash()); + recentRejects->insert(tx.GetWitnessHash()); if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); } @@ -2887,15 +3046,15 @@ void ProcessMessage( } if (pfrom.HasPermission(PF_FORCERELAY)) { - // Always relay transactions received from whitelisted peers, even + // Always relay transactions received from peers with forcerelay permission, even // if they were already in the mempool, // allowing the node to function as a gateway for // nodes hidden behind it. if (!mempool.exists(tx.GetHash())) { - LogPrintf("Not relaying non-mempool transaction %s from whitelisted peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); + LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); } else { - LogPrintf("Force relaying tx %s from whitelisted peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); - RelayTransaction(tx.GetHash(), *connman); + LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), connman); } } } @@ -2920,8 +3079,7 @@ void ProcessMessage( // peer simply for relaying a tx that our recentRejects has caught, // regardless of false positives. - if (state.IsInvalid()) - { + if (state.IsInvalid()) { LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom.GetId(), state.ToString()); @@ -2949,7 +3107,7 @@ void ProcessMessage( if (!LookupBlockIndex(cmpctblock.header.hashPrevBlock)) { // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers if (!::ChainstateActive().IsInitialBlockDownload()) - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), uint256())); return; } @@ -3010,7 +3168,7 @@ void ProcessMessage( // so we just grab the block via normal getdata std::vector<CInv> vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); } return; } @@ -3044,14 +3202,14 @@ void ProcessMessage( PartiallyDownloadedBlock& partialBlock = *(*queuedBlockIt)->partialBlock; ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { - MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case of whitelist + MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom.GetId())); return; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it std::vector<CInv> vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); return; } @@ -3068,7 +3226,7 @@ void ProcessMessage( fProcessBLOCKTXN = true; } else { req.blockhash = pindex->GetBlockHash(); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req)); } } else { // This block is either already in flight from a different @@ -3094,7 +3252,7 @@ void ProcessMessage( // mempool will probably be useless - request the block normally std::vector<CInv> vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); return; } else { // If this was an announce-cmpctblock, we want the same treatment as a header message @@ -3177,14 +3335,14 @@ void ProcessMessage( PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { - MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case of whitelist + MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom.GetId())); return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( std::vector<CInv> invs; invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom), resp.blockhash)); - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); } else { // Block is either okay, or possibly we received // READ_STATUS_CHECKBLOCK_FAILED. @@ -3319,10 +3477,11 @@ void ProcessMessage( pfrom.fSentAddr = true; pfrom.vAddrToSend.clear(); - std::vector<CAddress> vAddr = connman->GetAddresses(); + std::vector<CAddress> vAddr = connman.GetAddresses(); FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { - if (!banman->IsDiscouraged(addr) && !banman->IsBanned(addr)) { + bool banned_or_discouraged = banman && (banman->IsDiscouraged(addr) || banman->IsBanned(addr)); + if (!banned_or_discouraged) { pfrom.PushAddress(addr, insecure_rand); } } @@ -3340,7 +3499,7 @@ void ProcessMessage( return; } - if (connman->OutboundTargetReached(false) && !pfrom.HasPermission(PF_MEMPOOL)) + if (connman.OutboundTargetReached(false) && !pfrom.HasPermission(PF_MEMPOOL)) { if (!pfrom.HasPermission(PF_NOBAN)) { @@ -3373,7 +3532,7 @@ void ProcessMessage( // it, if the remote node sends a ping once per second and this node takes 5 // seconds to respond to each, the 5th ping the remote sends would appear to // return very quickly. - connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); + connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); } return; } @@ -3513,17 +3672,17 @@ void ProcessMessage( } if (msg_type == NetMsgType::GETCFILTERS) { - ProcessGetCFilters(pfrom, vRecv, chainparams, *connman); + ProcessGetCFilters(pfrom, vRecv, chainparams, connman); return; } if (msg_type == NetMsgType::GETCFHEADERS) { - ProcessGetCFHeaders(pfrom, vRecv, chainparams, *connman); + ProcessGetCFHeaders(pfrom, vRecv, chainparams, connman); return; } if (msg_type == NetMsgType::GETCFCHECKPT) { - ProcessGetCFCheckPt(pfrom, vRecv, chainparams, *connman); + ProcessGetCFCheckPt(pfrom, vRecv, chainparams, connman); return; } @@ -3535,7 +3694,7 @@ void ProcessMessage( vRecv >> vInv; if (vInv.size() <= MAX_PEER_TX_IN_FLIGHT + MAX_BLOCKS_IN_TRANSIT_PER_PEER) { for (CInv &inv : vInv) { - if (inv.type == MSG_TX || inv.type == MSG_WITNESS_TX) { + if (inv.type == MSG_TX || inv.type == MSG_WITNESS_TX || inv.type == MSG_WTX) { // If we receive a NOTFOUND message for a txid we requested, erase // it from our data structures for this peer. auto in_flight_it = state->m_tx_download.m_tx_in_flight.find(inv.hash); @@ -3616,12 +3775,12 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter bool fMoreWork = false; if (!pfrom->vRecvGetData.empty()) - ProcessGetData(*pfrom, chainparams, connman, m_mempool, interruptMsgProc); + ProcessGetData(*pfrom, chainparams, *connman, m_mempool, interruptMsgProc); if (!pfrom->orphan_work_set.empty()) { std::list<CTransactionRef> removed_txn; LOCK2(cs_main, g_cs_orphans); - ProcessOrphanTx(connman, m_mempool, pfrom->orphan_work_set, removed_txn); + ProcessOrphanTx(*connman, m_mempool, pfrom->orphan_work_set, removed_txn); for (const CTransactionRef& removedTx : removed_txn) { AddToCompactExtraTransactions(removedTx); } @@ -3681,7 +3840,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter } try { - ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, m_chainman, m_mempool, connman, m_banman, interruptMsgProc); + ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, m_chainman, m_mempool, *connman, m_banman, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) @@ -3834,17 +3993,19 @@ namespace { class CompareInvMempoolOrder { CTxMemPool *mp; + bool m_wtxid_relay; public: - explicit CompareInvMempoolOrder(CTxMemPool *_mempool) + explicit CompareInvMempoolOrder(CTxMemPool *_mempool, bool use_wtxid) { mp = _mempool; + m_wtxid_relay = use_wtxid; } bool operator()(std::set<uint256>::iterator a, std::set<uint256>::iterator b) { /* As std::make_heap produces a max-heap, we want the entries with the * fewest ancestors/highest fee to sort later. */ - return mp->CompareDepthAndScore(*b, *a); + return mp->CompareDepthAndScore(*b, *a, m_wtxid_relay); } }; } @@ -4152,8 +4313,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto) LOCK(pto->m_tx_relay->cs_filter); for (const auto& txinfo : vtxinfo) { - const uint256& hash = txinfo.tx->GetHash(); - CInv inv(MSG_TX, hash); + const uint256& hash = state.m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash(); + CInv inv(state.m_wtxid_relay ? MSG_WTX : MSG_TX, hash); pto->m_tx_relay->setInventoryTxToSend.erase(hash); // Don't send transactions that peers will not put into their mempool if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { @@ -4163,6 +4324,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (!pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; } pto->m_tx_relay->filterInventoryKnown.insert(hash); + // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter. vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); @@ -4187,7 +4349,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. // A heap is used so that not all items need sorting if only a few are being sent. - CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool); + CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool, state.m_wtxid_relay); std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. @@ -4206,17 +4368,20 @@ bool PeerLogicValidation::SendMessages(CNode* pto) continue; } // Not in the mempool anymore? don't bother sending it. - auto txinfo = m_mempool.info(hash); + auto txinfo = m_mempool.info(hash, state.m_wtxid_relay); if (!txinfo.tx) { continue; } + auto txid = txinfo.tx->GetHash(); + auto wtxid = txinfo.tx->GetWitnessHash(); // Peer told you to not send transactions at that feerate? Don't bother sending it. if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; } if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send - vInv.push_back(CInv(MSG_TX, hash)); + State(pto->GetId())->m_recently_announced_invs.insert(hash); + vInv.push_back(CInv(state.m_wtxid_relay ? MSG_WTX : MSG_TX, hash)); nRelayedTransactions++; { // Expire old relay messages @@ -4226,9 +4391,14 @@ bool PeerLogicValidation::SendMessages(CNode* pto) vRelayExpiration.pop_front(); } - auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx))); + auto ret = mapRelay.emplace(txid, std::move(txinfo.tx)); if (ret.second) { - vRelayExpiration.push_back(std::make_pair(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first)); + vRelayExpiration.emplace_back(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first); + } + // Add wtxid-based lookup into mapRelay as well, so that peers can request by wtxid + auto ret2 = mapRelay.emplace(wtxid, ret.first->second); + if (ret2.second) { + vRelayExpiration.emplace_back(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret2.first); } } if (vInv.size() == MAX_INV_SZ) { @@ -4236,6 +4406,14 @@ bool PeerLogicValidation::SendMessages(CNode* pto) vInv.clear(); } pto->m_tx_relay->filterInventoryKnown.insert(hash); + if (hash != txid) { + // Insert txid into filterInventoryKnown, even for + // wtxidrelay peers. This prevents re-adding of + // unconfirmed parents to the recently_announced + // filter, when a child tx is requested. See + // ProcessGetData(). + pto->m_tx_relay->filterInventoryKnown.insert(txid); + } } } } @@ -4273,9 +4451,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Check for headers sync timeouts if (state.fSyncStarted && state.nHeadersSyncTimeout < std::numeric_limits<int64_t>::max()) { // Detect whether this is a stalling initial-headers-sync peer - if (pindexBestHeader->GetBlockTime() <= GetAdjustedTime() - 24*60*60) { + if (pindexBestHeader->GetBlockTime() <= GetAdjustedTime() - 24 * 60 * 60) { if (nNow > state.nHeadersSyncTimeout && nSyncStarted == 1 && (nPreferredDownload - state.fPreferredDownload >= 1)) { - // Disconnect a (non-whitelisted) peer if it is our only sync peer, + // Disconnect a peer (without the noban permission) if it is our only sync peer, // and we have others we could be using instead. // Note: If all our peers are inbound, then we won't // disconnect our sync peer for stalling; we have bigger @@ -4285,7 +4463,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) pto->fDisconnect = true; return true; } else { - LogPrintf("Timeout downloading headers from whitelisted peer=%d, not disconnecting\n", pto->GetId()); + LogPrintf("Timeout downloading headers from noban peer=%d, not disconnecting\n", pto->GetId()); // Reset the headers sync state so that we have a // chance to try downloading from a different peer. // Note: this will also result in at least one more @@ -4360,7 +4538,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Erase this entry from tx_process_time (it may be added back for // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); - CInv inv(MSG_TX | GetFetchFlags(*pto), txid); + CInv inv(state.m_wtxid_relay ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), txid); if (!AlreadyHave(inv, m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. @@ -4379,7 +4557,15 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // up processing to happen after the download times out // (with a slight delay for inbound peers, to prefer // requests to outbound peers). - const auto next_process_time = CalculateTxGetDataTime(txid, current_time, !state.fPreferredDownload); + // Don't apply the txid-delay to re-requests of a + // transaction; the heuristic of delaying requests to + // txid-relay peers is to save bandwidth on initial + // announcement of a transaction, and doesn't make sense + // for a followup request if our first peer times out (and + // would open us up to an attacker using inbound + // wtxid-relay to prevent us from requesting transactions + // from outbound txid-relay peers). + const auto next_process_time = CalculateTxGetDataTime(txid, current_time, !state.fPreferredDownload, false); tx_process_time.emplace(next_process_time, txid); } } else { @@ -4443,6 +4629,7 @@ public: // orphan transactions mapOrphanTransactions.clear(); mapOrphanTransactionsByPrev.clear(); + g_orphans_by_wtxid.clear(); } }; static CNetProcessingCleanup instance_of_cnetprocessingcleanup; |