From 4ffb5daa3713c372207a49aa3c2d0e23cc3f54d3 Mon Sep 17 00:00:00 2001
From: Ori Newman <orinewman1@gmail.com>
Date: Mon, 3 Feb 2020 13:42:40 +0200
Subject: [PATCH] [NOD-622] Fix populateTemplateFromCandidates to sort
 txsForBlockTemplate.txMasses and txsForBlockTemplate.txFees (#617)

* [NOD-622] Fix populateTemplateFromCandidates to sort txsForBlockTemplate.txMasses and txsForBlockTemplate.txFees

* [NOD-622] Sort transactions in PrepareBlockForTest

* [NOD-622] Remove duplicate append of selected transactions
---
 blockdag/mining.go                | 13 -------------
 blockdag/test_utils.go            | 12 ++++++++++++
 mining/txselection.go             | 15 ++++++++++++---
 util/subnetworkid/subnetworkid.go |  8 --------
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/blockdag/mining.go b/blockdag/mining.go
index 5a6006a4d..a79e710df 100644
--- a/blockdag/mining.go
+++ b/blockdag/mining.go
@@ -5,9 +5,7 @@ import (
 	"encoding/binary"
 	"github.com/kaspanet/kaspad/txscript"
 	"github.com/kaspanet/kaspad/util"
-	"github.com/kaspanet/kaspad/util/subnetworkid"
 	"github.com/kaspanet/kaspad/wire"
-	"sort"
 	"time"
 )
 
@@ -25,17 +23,6 @@ func (dag *BlockDAG) BlockForMining(transactions []*util.Tx) (*wire.MsgBlock, er
 		return nil, err
 	}
 
-	// Sort transactions by subnetwork ID before building Merkle tree
-	sort.Slice(transactions, func(i, j int) bool {
-		if transactions[i].MsgTx().SubnetworkID.IsEqual(subnetworkid.SubnetworkIDCoinbase) {
-			return true
-		}
-		if transactions[j].MsgTx().SubnetworkID.IsEqual(subnetworkid.SubnetworkIDCoinbase) {
-			return false
-		}
-		return subnetworkid.Less(&transactions[i].MsgTx().SubnetworkID, &transactions[j].MsgTx().SubnetworkID)
-	})
-
 	// Create a new block ready to be solved.
 	hashMerkleTree := BuildHashMerkleTreeStore(transactions)
 	acceptedIDMerkleRoot, err := dag.NextAcceptedIDMerkleRootNoLock()
diff --git a/blockdag/test_utils.go b/blockdag/test_utils.go
index 1b6a4b241..38f6e76dd 100644
--- a/blockdag/test_utils.go
+++ b/blockdag/test_utils.go
@@ -10,6 +10,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"sync"
 
@@ -277,6 +278,17 @@ func PrepareBlockForTest(dag *BlockDAG, parentHashes []*daghash.Hash, transactio
 		blockTransactions[i+1] = util.NewTx(tx)
 	}
 
+	// Sort transactions by subnetwork ID
+	sort.Slice(blockTransactions, func(i, j int) bool {
+		if blockTransactions[i].MsgTx().SubnetworkID.IsEqual(subnetworkid.SubnetworkIDCoinbase) {
+			return true
+		}
+		if blockTransactions[j].MsgTx().SubnetworkID.IsEqual(subnetworkid.SubnetworkIDCoinbase) {
+			return false
+		}
+		return subnetworkid.Less(&blockTransactions[i].MsgTx().SubnetworkID, &blockTransactions[j].MsgTx().SubnetworkID)
+	})
+
 	block, err := dag.BlockForMining(blockTransactions)
 	if err != nil {
 		return nil, err
diff --git a/mining/txselection.go b/mining/txselection.go
index a82eb6083..e3fdabb85 100644
--- a/mining/txselection.go
+++ b/mining/txselection.go
@@ -234,6 +234,7 @@ func (g *BlkTmplGenerator) populateTemplateFromCandidates(candidateTxs []*candid
 		usedP += candidateTx.p
 	}
 
+	selectedTxs := make([]*candidateTx, 0)
 	for len(candidateTxs)-usedCount > 0 {
 		// Rebalance the candidates if it's required
 		if usedP >= rebalanceThreshold*totalP {
@@ -301,17 +302,25 @@ func (g *BlkTmplGenerator) populateTemplateFromCandidates(candidateTxs []*candid
 		// Add the transaction to the result, increment counters, and
 		// save the masses, fees, and signature operation counts to the
 		// result.
-		txsForBlockTemplate.selectedTxs = append(txsForBlockTemplate.selectedTxs, tx)
+		selectedTxs = append(selectedTxs, selectedTx)
 		txsForBlockTemplate.totalMass += selectedTx.txMass
 		txsForBlockTemplate.totalFees += selectedTx.txDesc.Fee
-		txsForBlockTemplate.txMasses = append(txsForBlockTemplate.txMasses, selectedTx.txMass)
-		txsForBlockTemplate.txFees = append(txsForBlockTemplate.txFees, selectedTx.txDesc.Fee)
 
 		log.Tracef("Adding tx %s (feePerKB %.2f)",
 			tx.ID(), selectedTx.txDesc.FeePerKB)
 
 		markCandidateTxForDeletion(selectedTx)
 	}
+
+	sort.Slice(selectedTxs, func(i, j int) bool {
+		return subnetworkid.Less(&selectedTxs[i].txDesc.Tx.MsgTx().SubnetworkID,
+			&selectedTxs[j].txDesc.Tx.MsgTx().SubnetworkID)
+	})
+	for _, selectedTx := range selectedTxs {
+		txsForBlockTemplate.selectedTxs = append(txsForBlockTemplate.selectedTxs, selectedTx.txDesc.Tx)
+		txsForBlockTemplate.txMasses = append(txsForBlockTemplate.txMasses, selectedTx.txMass)
+		txsForBlockTemplate.txFees = append(txsForBlockTemplate.txFees, selectedTx.txDesc.Fee)
+	}
 }
 
 func rebalanceCandidates(oldCandidateTxs []*candidateTx, isFirstRun bool) (
diff --git a/util/subnetworkid/subnetworkid.go b/util/subnetworkid/subnetworkid.go
index 3ac9f5170..d4d964fd7 100644
--- a/util/subnetworkid/subnetworkid.go
+++ b/util/subnetworkid/subnetworkid.go
@@ -9,7 +9,6 @@ import (
 	"encoding/hex"
 	"github.com/pkg/errors"
 	"math/big"
-	"sort"
 )
 
 // IDLength of array used to store the subnetwork ID. See SubnetworkID.
@@ -201,10 +200,3 @@ func (id *SubnetworkID) IsBuiltIn() bool {
 func Less(a *SubnetworkID, b *SubnetworkID) bool {
 	return a.Cmp(b) < 0
 }
-
-// Sort sorts a slice of ids
-func Sort(ids []SubnetworkID) {
-	sort.Slice(ids, func(i, j int) bool {
-		return Less(&ids[i], &ids[j])
-	})
-}