[NOD-164 + NOD-167] AcceptedIDMerkleRoot validation and newBlockTemplate (#295)

* [NOD-164] Added validation routine

* [NOD-167] Extracted acceptedIDMerkleRoot calculation to its own method and implemented NextAcceptedIDMerkleRoot.

* [NOD-164] Fixed TestValidateFeeTransaction.

* [NOD-164] Fixed TestFinality.

* [NOD-164] Fixed blk_ tests.

* [NOD-164] Fixed if -> iff in a comment.

* [NOD-164] Minor style changes in comments.

* [NOD-164] Moved validateAcceptedIDMerkleRoot to before its population with the block's own transactions.
Replaced heavy call to verifyAndBuildUTXO in NextBlockFeeTransaction and NextAcceptedIDMerkleRoot with a call to pastUTXO on the virtual.

* [NOD-164] Fixed erroneous comment.

* [NOD-164] Inserted the logic from buildAndSortAcceptedTxs into calculateAcceptedIDMerkleRoot, since the former was meaningless on its own.

* [NOD-164] Changed looping over txsAcceptanceData instead of over node.blues.

blockdag/dag.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"sort"
 	"sync"
 	"time"
 
@@ -507,6 +508,40 @@ func (dag *BlockDAG) addBlock(node *blockNode, parentNodes blockSet, block *util
 	return err
 }
 
+func calculateAcceptedIDMerkleRoot(txsAcceptanceData MultiBlockTxsAcceptanceData) *daghash.Hash {
+	var acceptedTxs []*util.Tx
+	for _, blockTxsAcceptanceData := range txsAcceptanceData {
+		for _, txAcceptance := range blockTxsAcceptanceData {
+			if !txAcceptance.IsAccepted {
+				continue
+			}
+			acceptedTxs = append(acceptedTxs, txAcceptance.Tx)
+		}
+	}
+	sort.Slice(acceptedTxs, func(i, j int) bool {
+		return daghash.LessTxID(acceptedTxs[i].ID(), acceptedTxs[j].ID())
+	})
+
+	acceptedIDMerkleTree := BuildIDMerkleTreeStore(acceptedTxs)
+	return acceptedIDMerkleTree.Root()
+}
+
+func (node *blockNode) validateAcceptedIDMerkleRoot(dag *BlockDAG, txsAcceptanceData MultiBlockTxsAcceptanceData) error {
+	if node.isGenesis() {
+		return nil
+	}
+
+	calculatedAccepetedIDMerkleRoot := calculateAcceptedIDMerkleRoot(txsAcceptanceData)
+	header := node.Header()
+	if !header.AcceptedIDMerkleRoot.IsEqual(calculatedAccepetedIDMerkleRoot) {
+		str := fmt.Sprintf("block accepted ID merkle root is invalid - block "+
+			"header indicates %s, but calculated value is %s",
+			header.AcceptedIDMerkleRoot, calculatedAccepetedIDMerkleRoot)
+		return ruleError(ErrBadMerkleRoot, str)
+	}
+	return nil
+}
+
 // connectBlock handles connecting the passed node/block to the DAG.
 //
 // This function MUST be called with the DAG state lock held (for writes).
@@ -712,7 +747,7 @@ func (dag *BlockDAG) updateFinalityPoint() {
 
 // NextBlockFeeTransaction prepares the fee transaction for the next mined block
 //
-// This function CAN'T be called with the DAG lock not held.
+// This function CAN'T be called with the DAG lock held.
 func (dag *BlockDAG) NextBlockFeeTransaction() (*wire.MsgTx, error) {
 	dag.dagLock.RLock()
 	defer dag.dagLock.RUnlock()
@@ -724,13 +759,35 @@ func (dag *BlockDAG) NextBlockFeeTransaction() (*wire.MsgTx, error) {
 //
 // This function MUST be called with the DAG read-lock held
 func (dag *BlockDAG) NextBlockFeeTransactionNoLock() (*wire.MsgTx, error) {
-	_, txsAcceptanceData, _, err := dag.virtual.blockNode.verifyAndBuildUTXO(dag, nil, true)
+	_, txsAcceptanceData, err := dag.pastUTXO(&dag.virtual.blockNode)
 	if err != nil {
 		return nil, err
 	}
 	return dag.virtual.blockNode.buildFeeTransaction(dag, txsAcceptanceData)
 }
 
+// NextAcceptedIDMerkleRoot prepares the acceptedIDMerkleRoot for the next mined block
+//
+// This function CAN'T be called with the DAG lock held.
+func (dag *BlockDAG) NextAcceptedIDMerkleRoot() (*daghash.Hash, error) {
+	dag.dagLock.RLock()
+	defer dag.dagLock.RUnlock()
+
+	return dag.NextAcceptedIDMerkleRootNoLock()
+}
+
+// NextAcceptedIDMerkleRootNoLock prepares the acceptedIDMerkleRoot for the next mined block
+//
+// This function MUST be called with the DAG read-lock held
+func (dag *BlockDAG) NextAcceptedIDMerkleRootNoLock() (*daghash.Hash, error) {
+	_, txsAcceptanceData, err := dag.pastUTXO(&dag.virtual.blockNode)
+	if err != nil {
+		return nil, err
+	}
+
+	return calculateAcceptedIDMerkleRoot(txsAcceptanceData), nil
+}
+
 // applyDAGChanges does the following:
 // 1. Connects each of the new block's parents to the block.
 // 2. Adds the new block to the DAG's tips.
@@ -820,6 +877,11 @@ func (node *blockNode) verifyAndBuildUTXO(dag *BlockDAG, transactions []*util.Tx
 		return nil, nil, nil, err
 	}
 
+	err = node.validateAcceptedIDMerkleRoot(dag, txsAcceptanceData)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+
 	feeData, err := dag.checkConnectToPastUTXO(node, pastUTXO, transactions, fastAdd)
 	if err != nil {
 		return nil, nil, nil, err

blockdag/dag_test.go

@@ -192,7 +192,7 @@ func TestHaveBlock(t *testing.T) {
 		{hash: dagconfig.SimNetParams.GenesisHash.String(), want: true},
 
 		// Block 3b should be present (as a second child of Block 2).
-		{hash: "0a20af2fa5ce154a60faca96e9fa125fc52e0ca8f98484708d0413203626edaf", want: true},
+		{hash: "6733a86f4e3a46c0d4df76d6fbfab88eacadf8e44f54eb544a18d6b95570510c", want: true},
 
 		// Block 100000 should be present (as an orphan).
 		{hash: "18bcf45b8c0dbccd7690a728f3486c6d5fc84971688f89f4554297b6a278e554", want: true},
@@ -901,6 +901,18 @@ func TestValidateFeeTransaction(t *testing.T) {
 		for i, tx := range transactions {
 			utilTxs[i] = util.NewTx(tx)
 		}
+
+		newVirtual, err := GetVirtualFromParentsForTest(dag, parentHashes)
+		if err != nil {
+			t.Fatalf("block %v: unexpected error when setting virtual for test: %v", blockName, err)
+		}
+		oldVirtual := SetVirtualForTest(dag, newVirtual)
+		acceptedIDMerkleRoot, err := dag.NextAcceptedIDMerkleRoot()
+		if err != nil {
+			t.Fatalf("block %v: unexpected error when getting next acceptIDMerkleRoot: %v", blockName, err)
+		}
+		SetVirtualForTest(dag, oldVirtual)
+
 		daghash.Sort(parentHashes)
 		msgBlock := &wire.MsgBlock{
 			Header: wire.BlockHeader{
@@ -908,7 +920,7 @@ func TestValidateFeeTransaction(t *testing.T) {
 				ParentHashes:         parentHashes,
 				HashMerkleRoot:       BuildHashMerkleTreeStore(utilTxs).Root(),
 				IDMerkleRoot:         BuildIDMerkleTreeStore(utilTxs).Root(),
-				AcceptedIDMerkleRoot: &daghash.ZeroHash,
+				AcceptedIDMerkleRoot: acceptedIDMerkleRoot,
 				UTXOCommitment:       &daghash.ZeroHash,
 			},
 			Transactions: transactions,

mining/mining.go

@@ -653,13 +653,17 @@ func (g *BlkTmplGenerator) NewBlockTemplate(payToAddress util.Address) (*BlockTe
 	// Create a new block ready to be solved.
 	hashMerkleTree := blockdag.BuildHashMerkleTreeStore(blockTxns)
 	idMerkleTree := blockdag.BuildIDMerkleTreeStore(blockTxns)
+	acceptedIDMerkleRoot, err := g.dag.NextAcceptedIDMerkleRoot()
+	if err != nil {
+		return nil, err
+	}
 	var msgBlock wire.MsgBlock
 	msgBlock.Header = wire.BlockHeader{
 		Version:              nextBlockVersion,
 		ParentHashes:         g.dag.TipHashes(),
 		HashMerkleRoot:       hashMerkleTree.Root(),
 		IDMerkleRoot:         idMerkleTree.Root(),
-		AcceptedIDMerkleRoot: &daghash.ZeroHash,
+		AcceptedIDMerkleRoot: acceptedIDMerkleRoot,
 		UTXOCommitment:       &daghash.ZeroHash,
 		Timestamp:            ts,
 		Bits:                 reqDifficulty,

mining/test_utils.go

@@ -116,6 +116,12 @@ func PrepareBlockForTest(dag *blockdag.BlockDAG, params *dagconfig.Params, paren
 		}
 		template.Block.Header.HashMerkleRoot = blockdag.BuildHashMerkleTreeStore(utilTxs).Root()
 		template.Block.Header.IDMerkleRoot = blockdag.BuildIDMerkleTreeStore(utilTxs).Root()
+
+		acceptedIDMerkleRoot, err := dag.NextAcceptedIDMerkleRoot()
+		if err != nil {
+			return nil, err
+		}
+		template.Block.Header.AcceptedIDMerkleRoot = acceptedIDMerkleRoot
 	}
 	return template.Block, nil
 }

util/daghash/hash.go

@@ -227,12 +227,17 @@ func (hash *Hash) Cmp(target *Hash) int {
 	return HashToBig(hash).Cmp(HashToBig(target))
 }
 
-//Less returns true iff hash a is less than hash b
+// Less returns true iff hash a is less than hash b
 func Less(a *Hash, b *Hash) bool {
 	return a.Cmp(b) < 0
 }
 
-//JoinHashesStrings joins all the stringified hashes separated by a separator
+// LessTxID returns true if tx ID a is less than tx ID b
+func LessTxID(a *TxID, b *TxID) bool {
+	return Less((*Hash)(a), (*Hash)(b))
+}
+
+// JoinHashesStrings joins all the stringified hashes separated by a separator
 func JoinHashesStrings(hashes []*Hash, separator string) string {
 	return strings.Join(Strings(hashes), separator)
 }