From bb985a7b6abee503852c61eec74ca3a29f582815 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sat, 2 Jan 2021 15:48:34 +0000 Subject: [PATCH] [net processing] Only relay blocks by cmpctblock and cache for fast relay if segwit is enabled This introduces an early exit in PeerManagerImpl::NewPoWValidBlock() if segwit has not been activated for the block. This means that we won't cache the block/compact block for fast relay and won't relay the cmpctblock immediately to peers that have requested hb compact blocks. This is fine because any block where segwit is not yet activated is buried deep in the chain, and so compact block relay will not be effective. It's ok not to cache the block/compact block for fast relay for the same reason - the block must be very deeply buried in the block chain. ProcessBlockAvailability() also won't get called for all nodes. This is also fine, since that function only updates hashLastUnknownBlock and pindexBestKnownBlock, and is called early in every SendMessages() call. --- src/net_processing.cpp | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a5bd1f086ec..500d59bf822 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -678,7 +678,6 @@ private: std::shared_ptr m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); std::shared_ptr m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex); uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex); - bool m_most_recent_compact_block_has_witnesses GUARDED_BY(m_most_recent_block_mutex){false}; /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ int m_highest_fast_announce{0}; @@ -1630,7 +1629,8 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha return; m_highest_fast_announce = pindex->nHeight; - bool fWitnessEnabled = DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT); + if (!DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) return; + uint256 hashBlock(pblock->GetHash()); const std::shared_future lazy_ser{ std::async(std::launch::deferred, [&] { return msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; @@ -1640,10 +1640,9 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha m_most_recent_block_hash = hashBlock; m_most_recent_block = pblock; m_most_recent_compact_block = pcmpctblock; - m_most_recent_compact_block_has_witnesses = fWitnessEnabled; } - m_connman.ForEachNode([this, pindex, fWitnessEnabled, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); if (pnode->GetCommonVersion() < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) @@ -1847,12 +1846,10 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& { std::shared_ptr a_recent_block; std::shared_ptr a_recent_compact_block; - bool fWitnessesPresentInARecentCompactBlock; { LOCK(m_most_recent_block_mutex); a_recent_block = m_most_recent_block; a_recent_compact_block = m_most_recent_compact_block; - fWitnessesPresentInARecentCompactBlock = m_most_recent_compact_block_has_witnesses; } bool need_activate_chain = false; @@ -1965,17 +1962,15 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // 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 // instead we respond with the full, non-compact block. - bool fPeerWantsWitness = State(pfrom.GetId())->m_provides_cmpctblocks; - int nSendFlags = fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; if (CanDirectFetch() && pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { - if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); + if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); } else { - CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness); - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + CBlockHeaderAndShortTxIDs cmpctblock(*pblock, true); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } } else { - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); } } } @@ -2134,10 +2129,9 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, const CBlock& block, c } resp.txn[i] = block.vtx[req.indexes[i]]; } - LOCK(cs_main); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - int nSendFlags = State(pfrom.GetId())->m_provides_cmpctblocks ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - m_connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCKTXN, resp)); } void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, @@ -3230,9 +3224,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // expensive disk reads, because it will require the peer to // actually receive all the data read from disk over the network. LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block > %i deep\n", pfrom.GetId(), MAX_BLOCKTXN_DEPTH); - CInv inv; - WITH_LOCK(cs_main, inv.type = State(pfrom.GetId())->m_provides_cmpctblocks ? MSG_WITNESS_BLOCK : MSG_BLOCK); - inv.hash = req.blockhash; + CInv inv{MSG_WITNESS_BLOCK, req.blockhash}; WITH_LOCK(peer->m_getdata_requests_mutex, peer->m_getdata_requests.push_back(inv)); // The message processing loop will go around again (without pausing) and we'll respond then return; @@ -3607,12 +3599,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT) && !nodestate->m_provides_cmpctblocks) { - // Don't bother trying to process compact blocks from v1 peers - // after segwit activates. - return; - } - // We want to be a bit conservative just to be extra careful about DoS // possibilities in compact block processing... if (pindex->nHeight <= m_chainman.ActiveChain().Height() + 2) {