fee work, locks for utxo and mempool maps, cleanups and minor fixes

README.md

@@ -73,7 +73,6 @@ Windows 10 with Visual Studio 2019 for build environment and vcpkg for library m
 
 - Replace by fee
 - Transaction locking
-- Mempool sorting by fee
 - Orphan blocks
 - Chainwork
 - Peer discovery

tiny-lib/Chain.cpp

@@ -54,7 +54,7 @@ int64_t Chain::GetMedianTimePast(uint32_t numLastBlocks)
 		return 0;
 
 	const uint32_t first_idx = ActiveChain.size() - numLastBlocks;
-	uint32_t median_idx = first_idx + (numLastBlocks / 2);
+	uint32_t median_idx = first_idx + numLastBlocks / 2;
 	if (numLastBlocks % 2 == 0)
 		median_idx -= 1;
 
@@ -68,10 +68,10 @@ uint32_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
 	const auto& txs = block->Txs;
 
 	if (txs.empty())
-		throw BlockValidationException("Transactions are empty");
+		throw BlockValidationException("Transactions empty");
 
 	if (block->Timestamp - Utils::GetUnixTimestamp() > static_cast<int64_t>(NetParams::MAX_FUTURE_BLOCK_TIME_IN_SECS))
-		throw BlockValidationException("Block timestamp is too far in future");
+		throw BlockValidationException("Block timestamp too far in future");
 
 	BIGNUM* target_bn = HashChecker::TargetBitsToBN(block->Bits);
 	if (!HashChecker::IsValid(block->Id(), target_bn))
@@ -98,17 +98,17 @@ uint32_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
 		}
 		catch (const TxValidationException&)
 		{
-			LOG_ERROR("Transaction {} in block {} failed to validate", txs[i]->Id(), block->Id());
+			LOG_ERROR("Transaction {} in block {} failed validation", txs[i]->Id(), block->Id());
 
-			throw BlockValidationException(fmt::format("Transaction {} is invalid", txs[i]->Id()).c_str());
+			throw BlockValidationException(fmt::format("Transaction {} invalid", txs[i]->Id()).c_str());
 		}
 	}
 
 	if (MerkleTree::GetRootOfTxs(txs)->Value != block->MerkleHash)
-		throw BlockValidationException("Merkle hash is invalid");
+		throw BlockValidationException("Merkle hash invalid");
 
 	if (block->Timestamp <= GetMedianTimePast(11))
-		throw BlockValidationException("Timestamp is too old");
+		throw BlockValidationException("Timestamp too old");
 
 	uint32_t prev_block_chain_idx;
 	if (block->PrevBlockHash.empty())
@@ -120,7 +120,7 @@ uint32_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
 		auto [prev_block, prev_block_height, prev_block_chain_idx2] = LocateBlockInAllChains(block->PrevBlockHash);
 		if (prev_block == nullptr)
 			throw BlockValidationException(
-				fmt::format("Previous block {} is not found in any chain", block->PrevBlockHash).c_str(), block);
+				fmt::format("Previous block {} not found in any chain", block->PrevBlockHash).c_str(), block);
 
 		if (prev_block_chain_idx2 != ActiveChainIdx)
 			return prev_block_chain_idx2;
@@ -131,7 +131,7 @@ uint32_t Chain::ValidateBlock(const std::shared_ptr<Block>& block)
 	}
 
 	if (PoW::GetNextWorkRequired(block->PrevBlockHash) != block->Bits)
-		throw BlockValidationException("Bits are incorrect");
+		throw BlockValidationException("Bits incorrect");
 
 	std::vector nonCoinbaseTxs(block->Txs.begin() + 1, block->Txs.end());
 	Tx::ValidateRequest req;
@@ -175,7 +175,7 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 	}
 	if (located_block != nullptr)
 	{
-		LOG_TRACE("Ignore already seen block {}", blockId);
+		LOG_INFO("Ignore already seen block {}", blockId);
 
 		return -1;
 	}
@@ -190,7 +190,7 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 		LOG_ERROR("Block {} failed validation", blockId);
 		if (ex.ToOrphan != nullptr)
 		{
-			LOG_INFO("Saw orphan block {}", blockId);
+			LOG_INFO("Found orphan block {}", blockId);
 
 			OrphanBlocks.push_back(ex.ToOrphan);
 		}
@@ -200,7 +200,7 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 
 	if (chainIdx != ActiveChainIdx && SideBranches.size() < chainIdx)
 	{
-		LOG_INFO("Creating a new side branch {} for block {}", chainIdx, blockId);
+		LOG_INFO("Creating a new side branch with idx {} for block {}", chainIdx, blockId);
 
 		SideBranches.emplace_back();
 	}
@@ -216,8 +216,12 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 		{
 			const auto txId = tx->Id();
 
-			if (Mempool::Map.contains(txId))
-				Mempool::Map.erase(txId);
+			{
+				std::lock_guard lock_mempool(Mempool::Mutex);
+
+				if (Mempool::Map.contains(txId))
+					Mempool::Map.erase(txId);
+			}
 
 			if (!tx->IsCoinbase())
 			{
@@ -233,11 +237,11 @@ int64_t Chain::ConnectBlock(const std::shared_ptr<Block>& block, bool doingReorg
 		}
 	}
 
-	if ((!doingReorg && ReorgIfNecessary()) || chainIdx == ActiveChainIdx)
+	if (!doingReorg && ReorgIfNecessary() || chainIdx == ActiveChainIdx)
 	{
 		PoW::MineInterrupt = true;
 
-		LOG_INFO("Block accepted with height {} and txs {}", ActiveChain.size() - 1, block->Txs.size());
+		LOG_INFO("Block accepted at height {} with {} txs", ActiveChain.size() - 1, block->Txs.size());
 	}
 
 	NetClient::SendMsgRandom(BlockInfoMsg(block));
@@ -259,7 +263,11 @@ std::shared_ptr<Block> Chain::DisconnectBlock(const std::shared_ptr<Block>& bloc
 	{
 		const auto txId = tx->Id();
 
-		Mempool::Map[txId] = tx;
+		{
+			std::lock_guard lock_mempool(Mempool::Mutex);
+
+			Mempool::Map[txId] = tx;
+		}
 
 		for (const auto& txIn : tx->TxIns)
 		{
@@ -349,7 +357,7 @@ bool Chain::TryReorg(const std::vector<std::shared_ptr<Block>>& branch, uint32_t
 	SideBranches.erase(SideBranches.begin() + branchIdx - 1);
 	SideBranches.push_back(oldActiveChain);
 
-	LOG_INFO("Chain reorg -> new height {}, tip {}", ActiveChain.size(), ActiveChain.back()->Id());
+	LOG_INFO("Chain reorganized, new height {} with tip {}", ActiveChain.size(), ActiveChain.back()->Id());
 
 	return true;
 }
@@ -359,7 +367,7 @@ void Chain::RollbackReorg(const std::vector<std::shared_ptr<Block>>& oldActiveCh
 {
 	std::lock_guard lock(Mutex);
 
-	LOG_INFO("Reorg of idx {} to active chain failed", branchIdx);
+	LOG_ERROR("Reorg of idx {} to active chain failed", branchIdx);
 
 	DisconnectToFork(forkBlock);
 

tiny-lib/ECDSA.cpp

@@ -12,7 +12,7 @@ std::pair<std::vector<uint8_t>, std::vector<uint8_t>> ECDSA::Generate()
 	{
 		EC_KEY_free(ec_key);
 
-		return {std::vector<uint8_t>(), std::vector<uint8_t>()};
+		return {{}, {}};
 	}
 
 	const BIGNUM* priv_key = EC_KEY_get0_private_key(ec_key);
@@ -21,7 +21,7 @@ std::pair<std::vector<uint8_t>, std::vector<uint8_t>> ECDSA::Generate()
 	{
 		EC_KEY_free(ec_key);
 
-		return {std::vector<uint8_t>(), std::vector<uint8_t>()};
+		return {{}, {}};
 	}
 
 	const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key);
@@ -30,7 +30,7 @@ std::pair<std::vector<uint8_t>, std::vector<uint8_t>> ECDSA::Generate()
 	{
 		EC_KEY_free(ec_key);
 
-		return {std::vector<uint8_t>(), std::vector<uint8_t>()};
+		return {{}, {}};
 	}
 
 	EC_KEY_free(ec_key);
@@ -46,7 +46,7 @@ std::vector<uint8_t> ECDSA::GetPubKeyFromPrivKey(const std::vector<uint8_t>& pri
 	{
 		EC_KEY_free(ec_key);
 
-		return std::vector<uint8_t>();
+		return {};
 	}
 
 	const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key);
@@ -55,7 +55,7 @@ std::vector<uint8_t> ECDSA::GetPubKeyFromPrivKey(const std::vector<uint8_t>& pri
 	{
 		EC_KEY_free(ec_key);
 
-		return std::vector<uint8_t>();
+		return {};
 	}
 
 	EC_KEY_free(ec_key);
@@ -71,7 +71,7 @@ std::vector<uint8_t> ECDSA::SignMsg(const std::vector<uint8_t>& msg, const std::
 	{
 		EC_KEY_free(ec_key);
 
-		return std::vector<uint8_t>();
+		return {};
 	}
 
 	std::vector<uint8_t> sig(ECDSA_size(ec_key));
@@ -80,7 +80,7 @@ std::vector<uint8_t> ECDSA::SignMsg(const std::vector<uint8_t>& msg, const std::
 	{
 		EC_KEY_free(ec_key);
 
-		return std::vector<uint8_t>();
+		return {};
 	}
 	sig.resize(siglen);
 
@@ -179,23 +179,23 @@ std::vector<uint8_t> ECDSA::GetPubKeyFromPrivKey(EC_KEY* ec_key, const EC_GROUP*
 		{
 			BN_CTX_free(bn_ctx);
 
-			return std::vector<uint8_t>();
+			return {};
 		}
 		if (!EC_KEY_set_public_key(ec_key, pub_key2))
 		{
 			BN_CTX_free(bn_ctx);
 
 			EC_POINT_free(pub_key2);
 
-			return std::vector<uint8_t>();
+			return {};
 		}
 		if (!EC_KEY_check_key(ec_key))
 		{
 			BN_CTX_free(bn_ctx);
 
 			EC_POINT_free(pub_key2);
 
-			return std::vector<uint8_t>();
+			return {};
 		}
 		pub_key = pub_key2;
 	}
@@ -206,15 +206,15 @@ std::vector<uint8_t> ECDSA::GetPubKeyFromPrivKey(EC_KEY* ec_key, const EC_GROUP*
 		BN_free(pub_key_bn);
 		BN_CTX_free(bn_ctx);
 
-		return std::vector<uint8_t>();
+		return {};
 	}
 	std::vector<uint8_t> pub_key_buffer(BN_num_bytes(pub_key_bn));
 	if (!BN_bn2bin(pub_key_bn, pub_key_buffer.data()))
 	{
 		BN_free(pub_key_bn);
 		BN_CTX_free(bn_ctx);
 
-		return std::vector<uint8_t>();
+		return {};
 	}
 	BN_free(pub_key_bn);
 	BN_CTX_free(bn_ctx);

tiny-lib/Mempool.cpp

@@ -2,27 +2,33 @@
 #include "Mempool.hpp"
 
 #include <ranges>
+#include <utility>
 
 #include "BinaryBuffer.hpp"
 #include "Exceptions.hpp"
 #include "Log.hpp"
 #include "NetClient.hpp"
 #include "NetParams.hpp"
+#include "PoW.hpp"
 #include "TxInfoMsg.hpp"
 
 std::unordered_map<std::string, std::shared_ptr<Tx>> Mempool::Map;
 
 std::vector<std::shared_ptr<Tx>> Mempool::OrphanedTxs;
 
+std::recursive_mutex Mempool::Mutex;
+
 std::shared_ptr<UTXO> Mempool::Find_UTXO_InMempool(const std::shared_ptr<TxOutPoint>& txOutPoint)
 {
+	std::lock_guard lock(Mutex);
+
 	if (!Map.contains(txOutPoint->TxId))
 		return nullptr;
 
 	const auto& tx = Map[txOutPoint->TxId];
 	if (tx->TxOuts.size() - 1 < txOutPoint->TxOutIdx)
 	{
-		LOG_TRACE("Couldn't find UTXO in mempool for {}", txOutPoint->TxId);
+		LOG_ERROR("Unable to find UTXO in mempool for {}", txOutPoint->TxId);
 
 		return nullptr;
 	}
@@ -34,17 +40,31 @@ std::shared_ptr<UTXO> Mempool::Find_UTXO_InMempool(const std::shared_ptr<TxOutPo
 
 std::shared_ptr<Block> Mempool::SelectFromMempool(const std::shared_ptr<Block>& block)
 {
+	std::lock_guard lock(Mutex);
+
 	auto newBlock = std::make_shared<Block>(*block);
 
+	std::vector<std::pair<std::string, std::shared_ptr<Tx>>> map_vector(Map.begin(), Map.end());
+	std::ranges::sort(map_vector,
+	                  [](const std::pair<std::string, std::shared_ptr<Tx>>& a,
+	                     const std::pair<std::string, std::shared_ptr<Tx>>& b) -> bool
+	                  {
+		                  const auto& [a_txId, a_tx] = a;
+		                  const auto& [b_txId, b_tx] = b;
+		                  return PoW::CalculateFees(a_tx) < PoW::CalculateFees(b_tx);
+	                  });
+
 	std::set<std::string> addedToBlock;
-	for (const auto& txId : Map | std::ranges::views::keys)
+	for (const auto& txId : map_vector | std::views::keys)
 		newBlock = TryAddToBlock(newBlock, txId, addedToBlock);
 
 	return newBlock;
 }
 
 void Mempool::AddTxToMempool(const std::shared_ptr<Tx>& tx)
 {
+	std::lock_guard lock(Mutex);
+
 	const auto txId = tx->Id();
 	if (Map.contains(txId))
 	{
@@ -74,7 +94,7 @@ void Mempool::AddTxToMempool(const std::shared_ptr<Tx>& tx)
 
 	Map[txId] = tx;
 
-	LOG_INFO("Transaction {} added to mempool", txId);
+	LOG_TRACE("Transaction {} added to mempool", txId);
 
 	NetClient::SendMsgRandom(TxInfoMsg(tx));
 }
@@ -87,6 +107,8 @@ bool Mempool::CheckBlockSize(const std::shared_ptr<Block>& block)
 std::shared_ptr<Block> Mempool::TryAddToBlock(std::shared_ptr<Block>& block, const std::string& txId,
                                               std::set<std::string>& addedToBlock)
 {
+	std::lock_guard lock(Mutex);
+
 	if (addedToBlock.contains(txId))
 		return block;
 
@@ -105,15 +127,15 @@ std::shared_ptr<Block> Mempool::TryAddToBlock(std::shared_ptr<Block>& block, con
 		const auto& inMempool = Find_UTXO_InMempool(toSpend);
 		if (inMempool == nullptr)
 		{
-			LOG_TRACE("Couldn't find UTXO for {}", txIn->ToSpend->TxId);
+			LOG_ERROR("Unable to find UTXO for {}", txIn->ToSpend->TxId);
 
 			return nullptr;
 		}
 
 		block = TryAddToBlock(block, inMempool->TxOutPoint->TxId, addedToBlock);
 		if (block == nullptr)
 		{
-			LOG_TRACE("Couldn't add parent");
+			LOG_ERROR("Unable to add parent");
 
 			return nullptr;
 		}

tiny-lib/Mempool.hpp

@@ -1,5 +1,6 @@
 #pragma once
 #include <memory>
+#include <mutex>
 #include <set>
 #include <string>
 #include <unordered_map>
@@ -13,11 +14,12 @@
 class Mempool
 {
 public:
-	//TODO: map mutex?
 	static std::unordered_map<std::string, std::shared_ptr<Tx>> Map;
 
 	static std::vector<std::shared_ptr<Tx>> OrphanedTxs;
 
+	static std::recursive_mutex Mutex;
+
 	static std::shared_ptr<UTXO> Find_UTXO_InMempool(const std::shared_ptr<TxOutPoint>& txOutPoint);
 
 	static std::shared_ptr<Block> SelectFromMempool(const std::shared_ptr<Block>& block);