From 2ca428bbbb54e366b5b31da37fcc85a26c1a5d54 Mon Sep 17 00:00:00 2001
From: SChernykh <sergey.v.chernykh@gmail.com>
Date: Tue, 22 Nov 2022 22:19:40 +0100
Subject: [PATCH] BlockTemplate: non-ambiguous transaction order

Different nodes could pick different transactions with the same fee/byte which hurted compact broadcasts efficiency
---
 src/block_template.cpp | 14 ++------------
 src/common.h           | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/block_template.cpp b/src/block_template.cpp
index 7249276..fd6506e 100644
--- a/src/block_template.cpp
+++ b/src/block_template.cpp
@@ -298,11 +298,7 @@ void BlockTemplate::update(const MinerData& data, const Mempool& mempool, Wallet
 			m_mempoolTxs.clear();
 		}
 		else if (m_mempoolTxs.size() > max_transactions) {
-			std::nth_element(m_mempoolTxs.begin(), m_mempoolTxs.begin() + max_transactions, m_mempoolTxs.end(),
-				[](const TxMempoolData& tx_a, const TxMempoolData& tx_b)
-				{
-					return tx_a.fee * tx_b.weight > tx_b.fee * tx_a.weight;
-				});
+			std::nth_element(m_mempoolTxs.begin(), m_mempoolTxs.begin() + max_transactions, m_mempoolTxs.end());
 			m_mempoolTxs.resize(max_transactions);
 		}
 
@@ -387,13 +383,7 @@ void BlockTemplate::update(const MinerData& data, const Mempool& mempool, Wallet
 		// Sometimes it even finds the optimal solution
 
 		// Sort all transactions by fee per byte (highest to lowest)
-		std::sort(m_mempoolTxsOrder.begin(), m_mempoolTxsOrder.end(),
-			[this](int a, int b)
-			{
-				const TxMempoolData& tx_a = m_mempoolTxs[a];
-				const TxMempoolData& tx_b = m_mempoolTxs[b];
-				return tx_a.fee * tx_b.weight > tx_b.fee * tx_a.weight;
-			});
+		std::sort(m_mempoolTxsOrder.begin(), m_mempoolTxsOrder.end(), [this](int a, int b) { return m_mempoolTxs[a] < m_mempoolTxs[b]; });
 
 		final_reward = base_reward;
 		final_fees = 0;
diff --git a/src/common.h b/src/common.h
index 59e0ed9..c00950e 100644
--- a/src/common.h
+++ b/src/common.h
@@ -329,6 +329,23 @@ struct TxMempoolData
 {
 	FORCEINLINE TxMempoolData() : id(), blob_size(0), weight(0), fee(0), time_received(0) {}
 
+	FORCEINLINE bool operator<(const TxMempoolData& tx) const
+	{
+		const uint64_t a = fee * tx.weight;
+		const uint64_t b = tx.fee * weight;
+
+		// Prefer transactions with higher fee/byte
+		if (a > b) return true;
+		if (a < b) return false;
+
+		// If fee/byte is the same, prefer smaller transactions (they give smaller penalty when going above the median block size limit)
+		if (weight < tx.weight) return true;
+		if (weight > tx.weight) return false;
+
+		// If two transactions have exactly the same fee and weight, just order them by id
+		return id < tx.id;
+	}
+
 	hash id;
 	uint64_t blob_size;
 	uint64_t weight;