fix some lock issues, cleanups

README.md

@@ -10,7 +10,6 @@ Windows 10 with Visual Studio 2019 for build environment and vcpkg for library m
 
 - openssl
 - boost
-- tbb
 - fmt
 - spdlog
 

tiny-lib/Chain.cpp

@@ -39,7 +39,7 @@ std::recursive_mutex Chain::Mutex;
 
 std::atomic_bool Chain::InitialBlockDownloadComplete = false;
 
-int64_t Chain::GetCurrentHeight()
+uint32_t Chain::GetCurrentHeight()
 {
 	std::lock_guard lock(Mutex);
 
@@ -53,15 +53,15 @@ int64_t Chain::GetMedianTimePast(uint32_t numLastBlocks)
 	if (numLastBlocks > ActiveChain.size())
 		return 0;
 
-	const int64_t first_idx = ActiveChain.size() - numLastBlocks;
-	int64_t median_idx = first_idx + (numLastBlocks / 2);
+	const uint32_t first_idx = ActiveChain.size() - numLastBlocks;
+	uint32_t median_idx = first_idx + (numLastBlocks / 2);
 	if (numLastBlocks % 2 == 0)
 		median_idx -= 1;
 
 	return ActiveChain[median_idx]->Timestamp;
 }
 
-int64_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
+uint32_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
 {
 	std::lock_guard lock(Mutex);
 
@@ -110,7 +110,7 @@ int64_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
 	if (block->Timestamp <= GetMedianTimePast(11))
 		throw BlockValidationException("Timestamp is too old");
 
-	int64_t prev_block_chain_idx;
+	uint32_t prev_block_chain_idx;
 	if (block->PrevBlockHash.empty())
 	{
 		prev_block_chain_idx = ActiveChainIdx;
@@ -162,7 +162,7 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 
 	const auto blockId = block->Id();
 
-	std::shared_ptr<Block> located_block; //HACK: this could be a ref
+	std::shared_ptr<Block> located_block;
 	if (!doingReorg)
 	{
 		auto [located_block2, located_block_height, located_block_chain_idx] = LocateBlockInAllChains(block->Id());
@@ -180,7 +180,7 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 		return -1;
 	}
 
-	int64_t chainIdx;
+	uint32_t chainIdx;
 	try
 	{
 		chainIdx = ValidateBlock(block);
@@ -307,12 +307,12 @@ bool Chain::ReorgIfNecessary()
 	bool reorged = false;
 
 	auto frozenSideBranches = SideBranches;
-	int64_t branch_idx = 1;
+	uint32_t branch_idx = 1;
 	for (const auto& chain : frozenSideBranches)
 	{
 		auto [fork_block, fork_heigh] = LocateBlockInActiveChain(chain[0]->PrevBlockHash);
 
-		int64_t branchHeight = chain.size() + fork_heigh;
+		uint32_t branchHeight = chain.size() + fork_heigh;
 		if (branchHeight > GetCurrentHeight())
 		{
 			LOG_INFO("Attempting reorg of idx {} to active chain, new height of {} vs. {}", branch_idx, branchHeight,
@@ -326,7 +326,7 @@ bool Chain::ReorgIfNecessary()
 	return reorged;
 }
 
-bool Chain::TryReorg(const std::vector<std::shared_ptr<Block>>& branch, int64_t branchIdx, int64_t forkIdx)
+bool Chain::TryReorg(const std::vector<std::shared_ptr<Block>>& branch, uint32_t branchIdx, uint32_t forkIdx)
 {
 	std::lock_guard lock(Mutex);
 
@@ -338,8 +338,7 @@ bool Chain::TryReorg(const std::vector<std::shared_ptr<Block>>& branch, int64_t
 
 	for (const auto& block : branch)
 	{
-		const int64_t connectedBlockIdx = ConnectBlock(block, true);
-		if (connectedBlockIdx != ActiveChainIdx)
+		if (ConnectBlock(block, true) != ActiveChainIdx)
 		{
 			RollbackReorg(oldActiveChain, fork_block, branchIdx);
 
@@ -356,7 +355,7 @@ bool Chain::TryReorg(const std::vector<std::shared_ptr<Block>>& branch, int64_t
 }
 
 void Chain::RollbackReorg(const std::vector<std::shared_ptr<Block>>& oldActiveChain,
-                          const std::shared_ptr<Block>& forkBlock, int64_t branchIdx)
+                          const std::shared_ptr<Block>& forkBlock, uint32_t branchIdx)
 {
 	std::lock_guard lock(Mutex);
 
@@ -377,7 +376,7 @@ std::pair<std::shared_ptr<Block>, int64_t> Chain::LocateBlockInChain(const std::
 {
 	std::lock_guard lock(Mutex);
 
-	int64_t height = 0;
+	uint32_t height = 0;
 	for (const auto& block : chain)
 	{
 		if (block->Id() == blockHash)
@@ -393,18 +392,14 @@ std::pair<std::shared_ptr<Block>, int64_t> Chain::LocateBlockInChain(const std::
 
 std::tuple<std::shared_ptr<Block>, int64_t> Chain::LocateBlockInActiveChain(const std::string& blockHash)
 {
-	auto [located_block, located_block_height] = LocateBlockInChain(blockHash, ActiveChain);
-	if (located_block != nullptr)
-		return {located_block, located_block_height};
-
-	return {located_block, located_block_height};
+	return LocateBlockInChain(blockHash, ActiveChain);
 }
 
 std::tuple<std::shared_ptr<Block>, int64_t, int64_t> Chain::LocateBlockInAllChains(const std::string& blockHash)
 {
 	std::lock_guard lock(Mutex);
 
-	int64_t chain_idx = 0;
+	uint32_t chain_idx = 0;
 	auto [located_block, located_block_height] = LocateBlockInActiveChain(blockHash);
 	if (located_block != nullptr)
 		return {located_block, located_block_height, chain_idx};

tiny-lib/Chain.hpp

@@ -28,24 +28,23 @@ class Chain
 
 	static std::recursive_mutex Mutex;
 
-	static constexpr int64_t ActiveChainIdx = 0;
+	static constexpr uint32_t ActiveChainIdx = 0;
 
 	static std::atomic_bool InitialBlockDownloadComplete;
 
-	static int64_t GetCurrentHeight();
-
+	static uint32_t GetCurrentHeight();
 	static int64_t GetMedianTimePast(uint32_t numLastBlocks);
 
-	static int64_t ValidateBlock(const std::shared_ptr<Block>& block);
+	static uint32_t ValidateBlock(const std::shared_ptr<Block>& block);
 
 	static int64_t ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg = false);
 	static std::shared_ptr<Block> DisconnectBlock(const std::shared_ptr<Block>& block);
 	static std::vector<std::shared_ptr<Block>> DisconnectToFork(const std::shared_ptr<Block>& forkBlock);
 
 	static bool ReorgIfNecessary();
-	static bool TryReorg(const std::vector<std::shared_ptr<Block>>& branch, int64_t branchIdx, int64_t forkIdx);
+	static bool TryReorg(const std::vector<std::shared_ptr<Block>>& branch, uint32_t branchIdx, uint32_t forkIdx);
 	static void RollbackReorg(const std::vector<std::shared_ptr<Block>>& oldActiveChain,
-	                          const std::shared_ptr<Block>& forkBlock, int64_t branchIdx);
+	                          const std::shared_ptr<Block>& forkBlock, uint32_t branchIdx);
 
 	static std::pair<std::shared_ptr<Block>, int64_t> LocateBlockInChain(
 		const std::string& blockHash, const std::vector<std::shared_ptr<Block>>& chain);

tiny-lib/Exceptions.cpp

@@ -14,7 +14,7 @@ TxValidationException::TxValidationException(const char* msg)
 {
 }
 
-TxValidationException::TxValidationException(const char* msg, std::shared_ptr<Tx> toOrphan)
+TxValidationException::TxValidationException(const char* msg, const std::shared_ptr<Tx>& toOrphan)
 	: std::exception(msg), ToOrphan(toOrphan)
 {
 }
@@ -24,7 +24,7 @@ BlockValidationException::BlockValidationException(const char* msg)
 {
 }
 
-BlockValidationException::BlockValidationException(const char* msg, std::shared_ptr<Block> toOrphan)
+BlockValidationException::BlockValidationException(const char* msg, const std::shared_ptr<Block>& toOrphan)
 	: std::exception(msg), ToOrphan(toOrphan)
 {
 }

tiny-lib/Exceptions.hpp

@@ -15,7 +15,7 @@ class TxValidationException : public std::exception
 {
 public:
 	TxValidationException(const char* msg);
-	TxValidationException(const char* msg, std::shared_ptr<Tx> toOrphan);
+	TxValidationException(const char* msg, const std::shared_ptr<Tx>& toOrphan);
 
 	std::shared_ptr<Tx> ToOrphan;
 };
@@ -24,7 +24,7 @@ class BlockValidationException : public std::exception
 {
 public:
 	BlockValidationException(const char* msg);
-	BlockValidationException(const char* msg, std::shared_ptr<Block> toOrphan);
+	BlockValidationException(const char* msg, const std::shared_ptr<Block>& toOrphan);
 
 	std::shared_ptr<Block> ToOrphan;
 };

tiny-lib/GetBlockMsg.cpp

@@ -20,18 +20,19 @@ void GetBlockMsg::Handle(std::shared_ptr<Connection>& con)
 	if (height == -1)
 		height = 1;
 
-	Chain::Mutex.lock();
-
 	std::vector<std::shared_ptr<Block>> blocks;
 	blocks.reserve(ChunkSize);
 
 	uint32_t max_height = height + ChunkSize;
-	if (max_height > Chain::ActiveChain.size())
-		max_height = Chain::ActiveChain.size();
-	for (uint32_t i = height; i < max_height - 1; i++)
-		blocks.push_back(Chain::ActiveChain[i]);
 
-	Chain::Mutex.unlock();
+	{
+		std::lock_guard lock(Chain::Mutex);
+
+		if (max_height > Chain::ActiveChain.size())
+			max_height = Chain::ActiveChain.size();
+		for (uint32_t i = height; i < max_height - 1; i++)
+			blocks.push_back(Chain::ActiveChain[i]);
+	}
 
 	LOG_TRACE("Sending {} blocks to {}:{}", blocks.size(), endpoint.address().to_string(), endpoint.port());
 	NetClient::SendMsg(con, InvMsg(blocks));

tiny-lib/Mempool.cpp

@@ -99,15 +99,7 @@ std::shared_ptr<Block> Mempool::TryAddToBlock(std::shared_ptr<Block>& block, con
 	{
 		const auto& toSpend = txIn->ToSpend;
 
-		auto map_it = std::ranges::find_if(UTXO::Map,
-		                                   [&toSpend](
-		                                   const std::pair<std::shared_ptr<TxOutPoint>, std::shared_ptr<UTXO>>&
-		                                   p)
-		                                   {
-			                                   const auto& [txOutPoint, utxo] = p;
-			                                   return *txOutPoint == *toSpend;
-		                                   });
-		if (map_it != UTXO::Map.end())
+		if (UTXO::FindInMap(toSpend) != nullptr)
 			continue;
 
 		const auto& inMempool = Find_UTXO_InMempool(toSpend);

tiny-lib/NetClient.cpp

@@ -46,25 +46,31 @@ void NetClient::RunAsync()
 
 void NetClient::Stop()
 {
-	ConnectionsMutex.lock();
-	for (const auto& con : Connections)
 	{
-		if (con->Socket.is_open())
+		std::lock_guard lock(ConnectionsMutex);
+
+		for (const auto& con : Connections)
 		{
-			con->Socket.shutdown(boost::asio::socket_base::shutdown_both);
-			con->Socket.close();
+			auto& soc = con->Socket;
+
+			if (soc.is_open())
+			{
+				soc.shutdown(boost::asio::socket_base::shutdown_both);
+				soc.close();
+			}
 		}
 	}
-	ConnectionsMutex.unlock();
 
 	IO_Service.stop();
 	if (IO_Thread.joinable())
 		IO_Thread.join();
 
-	ConnectionsMutex.lock();
-	MinerConnections.clear();
-	Connections.clear();
-	ConnectionsMutex.unlock();
+	{
+		std::lock_guard lock(ConnectionsMutex);
+
+		MinerConnections.clear();
+		Connections.clear();
+	}
 }
 
 void NetClient::Connect(const std::string& address, uint16_t port)
@@ -80,9 +86,11 @@ void NetClient::Connect(const std::string& address, uint16_t port)
 		return;
 	}
 	con->Socket.set_option(boost::asio::ip::tcp::no_delay(true));
-	ConnectionsMutex.lock();
-	Connections.push_back(con);
-	ConnectionsMutex.unlock();
+	{
+		std::lock_guard lock(ConnectionsMutex);
+
+		Connections.push_back(con);
+	}
 	SendMsg(con, PeerHelloMsg());
 	DoAsyncRead(con);
 }
@@ -117,19 +125,23 @@ bool NetClient::SendMsgRandom(const IMsg& msg)
 
 void NetClient::BroadcastMsg(const IMsg& msg)
 {
-	ConnectionsMutex.lock();
-	if (MinerConnections.empty())
-		return;
-	ConnectionsMutex.unlock();
+	{
+		std::lock_guard lock(ConnectionsMutex);
+
+		if (MinerConnections.empty())
+			return;
+	}
 
 	const auto msgBuffer = PrepareSendBuffer(msg);
 
-	ConnectionsMutex.lock();
-	for (auto& con : MinerConnections)
 	{
-		Write(con, msgBuffer);
+		std::lock_guard lock(ConnectionsMutex);
+
+		for (auto& con : MinerConnections)
+		{
+			Write(con, msgBuffer);
+		}
 	}
-	ConnectionsMutex.unlock();
 }
 
 std::shared_ptr<Connection> NetClient::GetRandomConnection()
@@ -160,9 +172,11 @@ void NetClient::HandleAccept(std::shared_ptr<Connection>& con, const boost::syst
 		         soc.remote_endpoint().port());
 
 		soc.set_option(boost::asio::ip::tcp::no_delay(true));
-		ConnectionsMutex.lock();
-		Connections.push_back(con);
-		ConnectionsMutex.unlock();
+		{
+			std::lock_guard lock(ConnectionsMutex);
+
+			Connections.push_back(con);
+		}
 		SendMsg(con, PeerHelloMsg());
 		DoAsyncRead(con);
 	}
@@ -357,8 +371,9 @@ void NetClient::RemoveConnection(std::shared_ptr<Connection>& con)
 
 		if (soc.is_open())
 		{
-			LOG_INFO("Peer {}:{} disconnected", soc.remote_endpoint().address().to_string(), soc.remote_endpoint().port());
-
+			LOG_INFO("Peer {}:{} disconnected", soc.remote_endpoint().address().to_string(),
+			         soc.remote_endpoint().port());
+
 			soc.shutdown(boost::asio::socket_base::shutdown_both);
 			soc.close();
 		}

tiny-lib/SendActiveChainMsg.cpp

@@ -13,10 +13,14 @@ BinaryBuffer SendActiveChainMsg::Serialize() const
 {
 	BinaryBuffer buffer;
 
-	buffer.WriteSize(Chain::ActiveChain.size());
-	for (const auto& block : Chain::ActiveChain)
 	{
-		buffer.WriteRaw(block->Serialize().GetBuffer());
+		std::lock_guard lock(Chain::Mutex);
+
+		buffer.WriteSize(Chain::ActiveChain.size());
+		for (const auto& block : Chain::ActiveChain)
+		{
+			buffer.WriteRaw(block->Serialize().GetBuffer());
+		}
 	}
 
 	return buffer;