Merge pull request #2488 from arkpar/bc

Prevent sync from downloading unknown blocks that are already in the queue

libethereum/BlockChainSync.cpp

@@ -916,26 +916,23 @@ void PV61Sync::requestSubchain(std::shared_ptr<EthereumPeer> _peer)
 	if (syncPeer != m_chainSyncPeers.end())
 	{
 		// Already downoading, request next batch
-		h256s& d = m_downloadingChainMap.at(syncPeer->second);
-		_peer->requestHashes(d.back());
+		SubChain const& s = m_downloadingChainMap.at(syncPeer->second);
+		_peer->requestHashes(s.lastHash);
 	}
 	else if (needsSyncing(_peer))
 	{
 		if (!m_readyChainMap.empty())
 		{
 			clog(NetAllDetail) << "Helping with hashchin download";
-			h256s& d = m_readyChainMap.begin()->second;
-			_peer->requestHashes(d.back());
-			m_downloadingChainMap[m_readyChainMap.begin()->first] = move(d);
+			SubChain& s = m_readyChainMap.begin()->second;
+			_peer->requestHashes(s.lastHash);
+			m_downloadingChainMap[m_readyChainMap.begin()->first] = move(s);
 			m_chainSyncPeers[_peer] = m_readyChainMap.begin()->first;
 			m_readyChainMap.erase(m_readyChainMap.begin());
 		}
 		else if (!m_downloadingChainMap.empty() && m_hashScanComplete)
-		{
 			// Lead syncer is done, just grab whatever we can
-			h256s& d = m_downloadingChainMap.begin()->second;
-			_peer->requestHashes(d.back());
-		}
+			_peer->requestHashes(m_downloadingChainMap.begin()->second.lastHash);
 	}
 }
 
@@ -974,7 +971,7 @@ void PV61Sync::completeSubchain(std::shared_ptr<EthereumPeer> _peer, unsigned _n
 		//Done chain-get
 		m_syncingNeededBlocks.clear();
 		for (auto h = m_completeChainMap.rbegin(); h != m_completeChainMap.rend(); ++h)
-			m_syncingNeededBlocks.insert(m_syncingNeededBlocks.end(), h->second.begin(), h->second.end());
+			m_syncingNeededBlocks.insert(m_syncingNeededBlocks.end(), h->second.hashes.begin(), h->second.hashes.end());
 		m_completeChainMap.clear();
 		m_knownHashes.clear();
 		m_syncingBlockNumber = 0;
@@ -1010,7 +1007,7 @@ void PV61Sync::onPeerHashes(std::shared_ptr<EthereumPeer> _peer, h256s const& _h
 		if (isSyncing(_peer) && _peer->m_syncHashNumber == m_syncingBlockNumber)
 		{
 			// End of hash chain, add last chunk to download
-			m_readyChainMap.insert(make_pair(m_syncingBlockNumber, h256s { _peer->m_latestHash }));
+			m_readyChainMap.insert(make_pair(m_syncingBlockNumber, SubChain{ h256s{ _peer->m_latestHash }, _peer->m_latestHash }));
 			m_hashScanComplete = true;
 			_peer->m_syncHashNumber = 0;
 			requestSubchain(_peer);
@@ -1038,7 +1035,7 @@ void PV61Sync::onPeerHashes(std::shared_ptr<EthereumPeer> _peer, h256s const& _h
 		// Got new subchain marker
 		assert(_hashes.size() == 1);
 		m_knownHashes.insert(_hashes[0]);
-		m_readyChainMap.insert(make_pair(m_syncingBlockNumber, h256s { _hashes[0] }));
+		m_readyChainMap.insert(make_pair(m_syncingBlockNumber, SubChain{ h256s{ _hashes[0] }, _hashes[0] }));
 		if ((m_readyChainMap.size() + m_downloadingChainMap.size() + m_completeChainMap.size()) * c_hashSubchainSize > _peer->m_expectedHashes)
 		{
 			_peer->disable("Too many hashes from lead peer");
@@ -1056,7 +1053,7 @@ void PV61Sync::onPeerHashes(std::shared_ptr<EthereumPeer> _peer, h256s const& _h
 		{
 			//check downlading peers
 			for (auto const& downloader: m_downloadingChainMap)
-				if (downloader.second.back() == _peer->m_syncHash)
+				if (downloader.second.lastHash == _peer->m_syncHash)
 				{
 					number = downloader.first;
 					break;
@@ -1071,7 +1068,7 @@ void PV61Sync::onPeerHashes(std::shared_ptr<EthereumPeer> _peer, h256s const& _h
 		}
 
 		auto downloadingPeer = m_downloadingChainMap.find(number);
-		if (downloadingPeer == m_downloadingChainMap.end() || downloadingPeer->second.back() != _peer->m_syncHash)
+		if (downloadingPeer == m_downloadingChainMap.end() || downloadingPeer->second.lastHash != _peer->m_syncHash)
 		{
 			// Too late, other peer has already downloaded our hashes
 			m_chainSyncPeers.erase(_peer);
@@ -1079,7 +1076,7 @@ void PV61Sync::onPeerHashes(std::shared_ptr<EthereumPeer> _peer, h256s const& _h
 			return;
 		}
 
-		h256s& hashes = downloadingPeer->second;
+		SubChain& subChain = downloadingPeer->second;
 		unsigned knowns = 0;
 		unsigned unknowns = 0;
 		for (unsigned i = 0; i < _hashes.size(); ++i)
@@ -1111,13 +1108,14 @@ void PV61Sync::onPeerHashes(std::shared_ptr<EthereumPeer> _peer, h256s const& _h
 			else if (status == QueueStatus::Unknown)
 			{
 				unknowns++;
-				hashes.push_back(h);
+				subChain.hashes.push_back(h);
 			}
 			else
 				knowns++;
+			subChain.lastHash = h;
 		}
-		clog(NetMessageSummary) << knowns << "knowns," << unknowns << "unknowns; now at" << hashes.back();
-		if (hashes.size() > c_hashSubchainSize)
+		clog(NetMessageSummary) << knowns << "knowns," << unknowns << "unknowns; now at" << subChain.lastHash;
+		if (subChain.hashes.size() > c_hashSubchainSize)
 		{
 			_peer->disable("Too many subchain hashes");
 			restartSync();
@@ -1175,11 +1173,11 @@ SyncStatus PV61Sync::status() const
 	{
 		res.hashesReceived = 0;
 		for (auto const& d : m_readyChainMap)
-			res.hashesReceived += d.second.size();
+			res.hashesReceived += d.second.hashes.size();
 		for (auto const& d : m_downloadingChainMap)
-			res.hashesReceived += d.second.size();
+			res.hashesReceived += d.second.hashes.size();
 		for (auto const& d : m_completeChainMap)
-			res.hashesReceived += d.second.size();
+			res.hashesReceived += d.second.hashes.size();
 	}
 	return res;
 }

libethereum/BlockChainSync.h

@@ -126,7 +126,6 @@ private:
 	void logNewBlock(h256 const& _h);
 
 	EthereumHost& m_host;
-	HashDownloadMan m_hashMan;
 };
 
 
@@ -308,12 +307,18 @@ private:
 	/// Check if downloading hashes in parallel
 	bool isPV61Syncing() const;
 
-	std::map<unsigned, h256s> m_completeChainMap;		///< Fully downloaded subchains
-	std::map<unsigned, h256s> m_readyChainMap;			///< Subchains ready for download
-	std::map<unsigned, h256s> m_downloadingChainMap;	///< Subchains currently being downloading. In sync with m_chainSyncPeers
+	struct SubChain
+	{
+		h256s hashes;	///< List of subchain hashes
+		h256 lastHash;	///< Last requested subchain hash
+	};
+
+	std::map<unsigned, SubChain> m_completeChainMap;		///< Fully downloaded subchains
+	std::map<unsigned, SubChain> m_readyChainMap;			///< Subchains ready for download
+	std::map<unsigned, SubChain> m_downloadingChainMap;		///< Subchains currently being downloading. In sync with m_chainSyncPeers
 	std::map<std::weak_ptr<EthereumPeer>, unsigned, std::owner_less<std::weak_ptr<EthereumPeer>>> m_chainSyncPeers; ///< Peers to m_downloadingSubchain number map
-	h256Hash m_knownHashes;								///< Subchain start markers. Used to track suchain completion
-	unsigned m_syncingBlockNumber = 0;					///< Current subchain marker
+	h256Hash m_knownHashes;									///< Subchain start markers. Used to track suchain completion
+	unsigned m_syncingBlockNumber = 0;						///< Current subchain marker
 	bool m_hashScanComplete = false;						///< True if leading peer completed hashchain scan and we have a list of subchains ready
 };