[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

@@ -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()

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

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) (

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])
-	})
-}