From 80175472d1a9687da704c5180bda173596271b12 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 18 Dec 2016 23:03:16 -0800 Subject: Make CBlockIndex*es in net_processing const --- src/net_processing.cpp | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ccfbb77fcd..4374b5be3e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -101,7 +101,7 @@ namespace { /** Blocks that are in flight, and that are in the queue to be downloaded. Protected by cs_main. */ struct QueuedBlock { uint256 hash; - CBlockIndex* pindex; //!< Optional. + const CBlockIndex* pindex; //!< Optional. bool fValidatedHeaders; //!< Whether this block has validated headers at the time of request. std::unique_ptr partialBlock; //!< Optional, used for CMPCTBLOCK downloads }; @@ -156,13 +156,13 @@ struct CNodeState { //! List of asynchronously-determined block rejections to notify this peer about. std::vector rejects; //! The best known block we know this peer has announced. - CBlockIndex *pindexBestKnownBlock; + const CBlockIndex *pindexBestKnownBlock; //! The hash of the last unknown block this peer has announced. uint256 hashLastUnknownBlock; //! The last full block we both have. - CBlockIndex *pindexLastCommonBlock; + const CBlockIndex *pindexLastCommonBlock; //! The best header we have sent our peer. - CBlockIndex *pindexBestHeaderSent; + const CBlockIndex *pindexBestHeaderSent; //! Length of current-streak of unconnecting headers announcements int nUnconnectingHeaders; //! Whether we've started headers synchronization with this peer. @@ -331,7 +331,7 @@ bool MarkBlockAsReceived(const uint256& hash) { // Requires cs_main. // returns false, still setting pit, if the block was already in flight from the same peer // pit will only be valid as long as the same cs_main lock is being held -bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, CBlockIndex *pindex = NULL, list::iterator **pit = NULL) { +bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, const CBlockIndex *pindex = NULL, list::iterator **pit = NULL) { CNodeState *state = State(nodeid); assert(state != NULL); @@ -432,7 +432,7 @@ bool CanDirectFetch(const Consensus::Params &consensusParams) } // Requires cs_main -bool PeerHasHeader(CNodeState *state, CBlockIndex *pindex) +bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) { if (state->pindexBestKnownBlock && pindex == state->pindexBestKnownBlock->GetAncestor(pindex->nHeight)) return true; @@ -443,7 +443,7 @@ bool PeerHasHeader(CNodeState *state, CBlockIndex *pindex) /** Find the last common ancestor two blocks have. * Both pa and pb must be non-NULL. */ -CBlockIndex* LastCommonAncestor(CBlockIndex* pa, CBlockIndex* pb) { +const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb) { if (pa->nHeight > pb->nHeight) { pa = pa->GetAncestor(pb->nHeight); } else if (pb->nHeight > pa->nHeight) { @@ -462,7 +462,7 @@ CBlockIndex* LastCommonAncestor(CBlockIndex* pa, CBlockIndex* pb) { /** Update pindexLastCommonBlock and add not-in-flight missing successors to vBlocks, until it has * at most count entries. */ -void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector& vBlocks, NodeId& nodeStaller, const Consensus::Params& consensusParams) { +void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector& vBlocks, NodeId& nodeStaller, const Consensus::Params& consensusParams) { if (count == 0) return; @@ -490,8 +490,8 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectorpindexLastCommonBlock == state->pindexBestKnownBlock) return; - std::vector vToFetch; - CBlockIndex *pindexWalk = state->pindexLastCommonBlock; + std::vector vToFetch; + const CBlockIndex *pindexWalk = state->pindexLastCommonBlock; // Never fetch further than the best block we know the peer has, or more than BLOCK_DOWNLOAD_WINDOW + 1 beyond the last // linked block we have in common with this peer. The +1 is so we can detect stalling, namely if we would be able to // download that next block if the window were 1 larger. @@ -514,7 +514,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectorIsValid(BLOCK_VALID_TREE)) { // We consider the chain that this peer is on invalid. return; @@ -1049,7 +1049,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } } -uint32_t GetFetchFlags(CNode* pfrom, CBlockIndex* pprev, const Consensus::Params& chainparams) { +uint32_t GetFetchFlags(CNode* pfrom, const CBlockIndex* pprev, const Consensus::Params& chainparams) { uint32_t nFetchFlags = 0; if ((pfrom->GetLocalServices() & NODE_WITNESS) && State(pfrom->GetId())->fHaveWitness) { nFetchFlags |= MSG_WITNESS_FLAG; @@ -1456,7 +1456,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, LOCK(cs_main); // Find the last block the caller has in the main chain - CBlockIndex* pindex = FindForkInGlobalIndex(chainActive, locator); + const CBlockIndex* pindex = FindForkInGlobalIndex(chainActive, locator); // Send the rest of the chain if (pindex) @@ -1552,7 +1552,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } CNodeState *nodestate = State(pfrom->GetId()); - CBlockIndex* pindex = NULL; + const CBlockIndex* pindex = NULL; if (locator.IsNull()) { // If locator is null, return the hashStop block @@ -1775,7 +1775,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } } - CBlockIndex *pindex = NULL; + const CBlockIndex *pindex = NULL; CValidationState state; if (!ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) { int nDoS; @@ -2051,7 +2051,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, return true; } - CBlockIndex *pindexLast = NULL; + const CBlockIndex *pindexLast = NULL; { LOCK(cs_main); CNodeState *nodestate = State(pfrom->GetId()); @@ -2128,8 +2128,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // If this set of headers is valid and ends in a block with at least as // much work as our tip, download as much as possible. if (fCanDirectFetch && pindexLast->IsValid(BLOCK_VALID_TREE) && chainActive.Tip()->nChainWork <= pindexLast->nChainWork) { - vector vToFetch; - CBlockIndex *pindexWalk = pindexLast; + vector vToFetch; + const CBlockIndex *pindexWalk = pindexLast; // Calculate all the blocks we'd need to switch to pindexLast, up to a limit. while (pindexWalk && !chainActive.Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && @@ -2151,7 +2151,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } else { vector vGetData; // Download as much as possible, from earliest to latest. - BOOST_REVERSE_FOREACH(CBlockIndex *pindex, vToFetch) { + BOOST_REVERSE_FOREACH(const CBlockIndex *pindex, vToFetch) { if (nodestate->nBlocksInFlight >= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { // Can't download any more from this peer break; @@ -2740,7 +2740,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg bool fRevertToInv = ((!state.fPreferHeaders && (!state.fPreferHeaderAndIDs || pto->vBlockHashesToAnnounce.size() > 1)) || pto->vBlockHashesToAnnounce.size() > MAX_BLOCKS_TO_ANNOUNCE); - CBlockIndex *pBestIndex = NULL; // last header queued for delivery + const CBlockIndex *pBestIndex = NULL; // last header queued for delivery ProcessBlockAvailability(pto->id); // ensure pindexBestKnownBlock is up-to-date if (!fRevertToInv) { @@ -2751,7 +2751,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg BOOST_FOREACH(const uint256 &hash, pto->vBlockHashesToAnnounce) { BlockMap::iterator mi = mapBlockIndex.find(hash); assert(mi != mapBlockIndex.end()); - CBlockIndex *pindex = mi->second; + const CBlockIndex *pindex = mi->second; if (chainActive[pindex->nHeight] != pindex) { // Bail out if we reorged away from this block fRevertToInv = true; @@ -2828,7 +2828,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg const uint256 &hashToAnnounce = pto->vBlockHashesToAnnounce.back(); BlockMap::iterator mi = mapBlockIndex.find(hashToAnnounce); assert(mi != mapBlockIndex.end()); - CBlockIndex *pindex = mi->second; + const CBlockIndex *pindex = mi->second; // Warn if we're announcing a block that is not on the main chain. // This should be very rare and could be optimized out. @@ -3013,10 +3013,10 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg // vector vGetData; if (!pto->fClient && (fFetch || !IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { - vector vToDownload; + vector vToDownload; NodeId staller = -1; FindNextBlocksToDownload(pto->GetId(), MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller, consensusParams); - BOOST_FOREACH(CBlockIndex *pindex, vToDownload) { + BOOST_FOREACH(const CBlockIndex *pindex, vToDownload) { uint32_t nFetchFlags = GetFetchFlags(pto, pindex->pprev, consensusParams); vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); MarkBlockAsInFlight(pto->GetId(), pindex->GetBlockHash(), consensusParams, pindex); -- cgit v1.2.3 From c80209214208621b84da10895427e1e5f381d1b8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 5 Jan 2017 10:31:39 -0500 Subject: Relay compact block messages prior to full block connection --- src/net_processing.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4374b5be3e..15bdef86fa 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -752,6 +752,39 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn } } +void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) { + CBlockHeaderAndShortTxIDs cmpctblock(*pblock, true); + CNetMsgMaker msgMaker(PROTOCOL_VERSION); + + LOCK(cs_main); + + static int nHighestFastAnnounce = 0; + if (pindex->nHeight <= nHighestFastAnnounce) + return; + nHighestFastAnnounce = pindex->nHeight; + + bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, Params().GetConsensus()); + uint256 hashBlock(pblock->GetHash()); + + connman->ForEachNode([this, &cmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { + // TODO: Avoid the repeated-serialization here + if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) + return; + ProcessBlockAvailability(pnode->GetId()); + CNodeState &state = *State(pnode->GetId()); + // If the peer has, or we announced to them the previous block already, + // but we don't think they have this one, go ahead and announce it + if (state.fPreferHeaderAndIDs && (!fWitnessEnabled || state.fWantsCmpctWitness) && + !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { + + LogPrint("net", "%s sending header-and-ids %s to peer %d\n", "PeerLogicValidation::NewPoWValidBlock", + hashBlock.ToString(), pnode->id); + connman->PushMessage(pnode, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); + state.pindexBestHeaderSent = pindex; + } + }); +} + void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { const int nNewHeight = pindexNew->nHeight; connman->SetBestHeight(nNewHeight); -- cgit v1.2.3 From 9eaec08dd236a9e73c7993d25274ec913c999c63 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 14 Dec 2016 16:28:22 -0800 Subject: Cache most-recently-announced block's shared_ptr --- src/net_processing.cpp | 51 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 15bdef86fa..5de103d4ef 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -752,6 +752,10 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn } } +static CCriticalSection cs_most_recent_block; +static std::shared_ptr most_recent_block; +static uint256 most_recent_block_hash; + void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) { CBlockHeaderAndShortTxIDs cmpctblock(*pblock, true); CNetMsgMaker msgMaker(PROTOCOL_VERSION); @@ -766,6 +770,12 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, Params().GetConsensus()); uint256 hashBlock(pblock->GetHash()); + { + LOCK(cs_most_recent_block); + most_recent_block_hash = hashBlock; + most_recent_block = pblock; + } + connman->ForEachNode([this, &cmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { // TODO: Avoid the repeated-serialization here if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) @@ -1090,6 +1100,23 @@ uint32_t GetFetchFlags(CNode* pfrom, const CBlockIndex* pprev, const Consensus:: return nFetchFlags; } +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()) { + LOCK(cs_main); + Misbehaving(pfrom->GetId(), 100); + LogPrintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom->id); + return; + } + resp.txn[i] = block.vtx[req.indexes[i]]; + } + LOCK(cs_main); + CNetMsgMaker msgMaker(pfrom->GetSendVersion()); + int nSendFlags = State(pfrom->GetId())->fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; + connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); +} + bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman& connman, std::atomic& interruptMsgProc) { unsigned int nMaxSendBufferSize = connman.GetSendBufferSize(); @@ -1529,6 +1556,18 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, BlockTransactionsRequest req; vRecv >> req; + std::shared_ptr recent_block; + { + LOCK(cs_most_recent_block); + if (most_recent_block_hash == req.blockhash) + recent_block = most_recent_block; + // Unlock cs_most_recent_block to avoid cs_main lock inversion + } + if (recent_block) { + SendBlockTransactions(*recent_block, req, pfrom, connman); + return true; + } + LOCK(cs_main); BlockMap::iterator it = mapBlockIndex.find(req.blockhash); @@ -1558,17 +1597,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, bool ret = ReadBlockFromDisk(block, it->second, chainparams.GetConsensus()); assert(ret); - BlockTransactions resp(req); - for (size_t i = 0; i < req.indexes.size(); i++) { - if (req.indexes[i] >= block.vtx.size()) { - Misbehaving(pfrom->GetId(), 100); - LogPrintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom->id); - return true; - } - resp.txn[i] = block.vtx[req.indexes[i]]; - } - int nSendFlags = State(pfrom->GetId())->fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); + SendBlockTransactions(block, req, pfrom, connman); } -- cgit v1.2.3 From 5749a853b9f46cc58d51aae8b5d4dcbd110a20ca Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Dec 2016 15:29:06 -0800 Subject: Cache most-recently-connected compact block --- src/net_processing.cpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5de103d4ef..669be6e89b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -754,10 +754,11 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn static CCriticalSection cs_most_recent_block; static std::shared_ptr most_recent_block; +static std::shared_ptr most_recent_compact_block; static uint256 most_recent_block_hash; void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) { - CBlockHeaderAndShortTxIDs cmpctblock(*pblock, true); + std::shared_ptr pcmpctblock = std::make_shared (*pblock, true); CNetMsgMaker msgMaker(PROTOCOL_VERSION); LOCK(cs_main); @@ -774,9 +775,10 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: LOCK(cs_most_recent_block); most_recent_block_hash = hashBlock; most_recent_block = pblock; + most_recent_compact_block = pcmpctblock; } - connman->ForEachNode([this, &cmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { + connman->ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { // TODO: Avoid the repeated-serialization here if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) return; @@ -789,7 +791,7 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: LogPrint("net", "%s sending header-and-ids %s to peer %d\n", "PeerLogicValidation::NewPoWValidBlock", hashBlock.ToString(), pnode->id); - connman->PushMessage(pnode, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); + connman->PushMessage(pnode, msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock)); state.pindexBestHeaderSent = pindex; } }); @@ -2859,13 +2861,24 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg // probably means we're doing an initial-ish-sync or they're slow LogPrint("net", "%s sending header-and-ids %s to peer %d\n", __func__, vHeaders.front().GetHash().ToString(), pto->id); - //TODO: Shouldn't need to reload block from disk, but requires refactor - CBlock block; - bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams); - assert(ret); - CBlockHeaderAndShortTxIDs cmpctblock(block, state.fWantsCmpctWitness); + int nSendFlags = state.fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + + LOCK(cs_most_recent_block); + if (most_recent_block_hash == pBestIndex->GetBlockHash()) { + if (state.fWantsCmpctWitness) + connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *most_recent_compact_block)); + else { + CBlockHeaderAndShortTxIDs cmpctblock(*most_recent_block, state.fWantsCmpctWitness); + connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + } + } else { + CBlock block; + bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams); + assert(ret); + CBlockHeaderAndShortTxIDs cmpctblock(block, state.fWantsCmpctWitness); + connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + } state.pindexBestHeaderSent = pBestIndex; } else if (state.fPreferHeaders) { if (vHeaders.size() > 1) { -- cgit v1.2.3 From 9eb67f50008970d09a8253475d591cdad872692e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 31 Dec 2016 18:16:08 +0100 Subject: Ensure we meet the BIP 152 old-relay-types response requirements In order to do this, we must call ActivateBestChain prior to responding getdata requests for blocks which we announced using compact blocks. For getheaders responses we dont need code changes, but do note that we must reset the bestHeaderSent so that the SendMessages call re-announces the header in question. While we could do something smarter for getblocks, calling ActivateBestChain is simple and more obviously correct, instead of doing something more similar to getheaders. See-also the BIP clarifications at https://github.com/bitcoin/bips/pull/486 --- src/net_processing.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 669be6e89b..fdb70d6578 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -957,6 +957,16 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam BlockMap::iterator mi = mapBlockIndex.find(inv.hash); if (mi != mapBlockIndex.end()) { + if (mi->second->nChainTx && !mi->second->IsValid(BLOCK_VALID_SCRIPTS) && + mi->second->IsValid(BLOCK_VALID_TREE)) { + // If we have the block and all of its parents, but have not yet validated it, + // we might be in the middle of connecting it (ie in the unlock of cs_main + // before ActivateBestChain but after AcceptBlock). + // In this case, we need to run ActivateBestChain prior to checking the relay + // conditions below. + CValidationState dummy; + ActivateBestChain(dummy, Params()); + } if (chainActive.Contains(mi->second)) { send = true; } else { @@ -1517,6 +1527,18 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, LOCK(cs_main); + // We might have announced the currently-being-connected tip using a + // compact block, which resulted in the peer sending a getblocks + // request, which we would otherwise respond to without the new block. + // To avoid this situation we simply verify that we are on our best + // known chain now. This is super overkill, but we handle it better + // for getheaders requests, and there are no known nodes which support + // compact blocks but still use getblocks to request blocks. + { + CValidationState dummy; + ActivateBestChain(dummy, Params()); + } + // Find the last block the caller has in the main chain const CBlockIndex* pindex = FindForkInGlobalIndex(chainActive, locator); @@ -1647,6 +1669,14 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // if our peer has chainActive.Tip() (and thus we are sending an empty // headers message). In both cases it's safe to update // pindexBestHeaderSent to be our tip. + // + // It is important that we simply reset the BestHeaderSent value here, + // and not max(BestHeaderSent, newHeaderSent). We might have announced + // the currently-being-connected tip using a compact block, which + // resulted in the peer sending a headers request, which we respond to + // without the new block. By resetting the BestHeaderSent, we ensure we + // 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)); } -- cgit v1.2.3 From c1ae4fcf7d5d85c182e36ff6f7a529f8a84aa372 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 5 Jan 2017 15:15:40 -0500 Subject: Avoid holding cs_most_recent_block while calling ReadBlockFromDisk --- src/net_processing.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index fdb70d6578..440696ae55 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2894,15 +2894,20 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg int nSendFlags = state.fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - LOCK(cs_most_recent_block); - if (most_recent_block_hash == pBestIndex->GetBlockHash()) { - if (state.fWantsCmpctWitness) - connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *most_recent_compact_block)); - else { - CBlockHeaderAndShortTxIDs cmpctblock(*most_recent_block, state.fWantsCmpctWitness); - connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + bool fGotBlockFromCache = false; + { + LOCK(cs_most_recent_block); + if (most_recent_block_hash == pBestIndex->GetBlockHash()) { + if (state.fWantsCmpctWitness) + connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *most_recent_compact_block)); + else { + CBlockHeaderAndShortTxIDs cmpctblock(*most_recent_block, state.fWantsCmpctWitness); + connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); + } + fGotBlockFromCache = true; } - } else { + } + if (!fGotBlockFromCache) { CBlock block; bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams); assert(ret); -- cgit v1.2.3 From 962f7f054fd9692f92d40c771cf3ba8ef0cde891 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 11 Jan 2017 21:15:28 -0800 Subject: Call ActivateBestChain without cs_main/with most_recent_block There is still a call to ActivateBestChain with cs_main if a peer requests the block prior to it being validated, but this one is more specifically-gated, so should be less of an issue. --- src/net_processing.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 440696ae55..015921ebe9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -964,8 +964,13 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // before ActivateBestChain but after AcceptBlock). // In this case, we need to run ActivateBestChain prior to checking the relay // conditions below. + std::shared_ptr a_recent_block; + { + LOCK(cs_most_recent_block); + a_recent_block = most_recent_block; + } CValidationState dummy; - ActivateBestChain(dummy, Params()); + ActivateBestChain(dummy, Params(), a_recent_block); } if (chainActive.Contains(mi->second)) { send = true; @@ -1525,8 +1530,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, uint256 hashStop; vRecv >> locator >> hashStop; - LOCK(cs_main); - // We might have announced the currently-being-connected tip using a // compact block, which resulted in the peer sending a getblocks // request, which we would otherwise respond to without the new block. @@ -1535,10 +1538,17 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // for getheaders requests, and there are no known nodes which support // compact blocks but still use getblocks to request blocks. { + std::shared_ptr a_recent_block; + { + LOCK(cs_most_recent_block); + a_recent_block = most_recent_block; + } CValidationState dummy; - ActivateBestChain(dummy, Params()); + ActivateBestChain(dummy, Params(), a_recent_block); } + LOCK(cs_main); + // Find the last block the caller has in the main chain const CBlockIndex* pindex = FindForkInGlobalIndex(chainActive, locator); -- cgit v1.2.3 From 02ee4eb2637db9077aefa1f5e59864218e016b93 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 13 Jan 2017 15:49:31 -0500 Subject: Make most_recent_compact_block a pointer to a const --- 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 015921ebe9..8bcb590675 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -754,11 +754,11 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn static CCriticalSection cs_most_recent_block; static std::shared_ptr most_recent_block; -static std::shared_ptr most_recent_compact_block; +static std::shared_ptr most_recent_compact_block; static uint256 most_recent_block_hash; void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) { - std::shared_ptr pcmpctblock = std::make_shared (*pblock, true); + std::shared_ptr pcmpctblock = std::make_shared (*pblock, true); CNetMsgMaker msgMaker(PROTOCOL_VERSION); LOCK(cs_main); -- cgit v1.2.3