blockchain: Split block and header validation.

This commit refactors the consensus rule checks for block headers and
blocks in the blockchain package into separate functions.  These changes
contain no modifications to consensus rules and the code still passes all
block consensus tests.  It is only a refactoring.

This is being done to help pave the way toward supporting concurrent
downloads.  While the package already supports headers-first mode up
through the latest checkpoint through the use of the BFFastAdd flag and
hard-coded checkpoints, it currently only works when downloading from a
single peer.  In order to support concurrent downloads from multiple
peers, the ability for the caller to do things such as independently
checking a block header (both context-free and full-context checks) will
be needed.

There are several more changes that will be necessary to support
concurrent downloads as well, such as making the package concurrent safe,
modifying it to make use of the new database API, etc.  Those changes are
planned for future commits.

blockchain/accept.go

@@ -4,11 +4,7 @@
 
 package blockchain
 
-import (
+import "github.com/btcsuite/btcutil"
-	"fmt"
-
-	"github.com/btcsuite/btcutil"
-)
 
 // maybeAcceptBlock potentially accepts a block into the memory block chain.
 // It performs several validation checks which depend on its position within
@@ -16,11 +12,12 @@ import (
 // through ProcessBlock before calling this function with it.
 //
 // The flags modify the behavior of this function as follows:
-//  - BFFastAdd: The somewhat expensive BIP0034 validation is not performed.
 //  - BFDryRun: The memory chain index will not be pruned and no accept
 //    notification will be sent since the block is not being accepted.
+//
+// The flags are also passed to checkBlockContext and connectBestChain.  See
+// their documentation for how the flags modify their behavior.
 func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags) error {
-	fastAdd := flags&BFFastAdd == BFFastAdd
 	dryRun := flags&BFDryRun == BFDryRun
 
 	// Get a block node for the block previous to this one.  Will be nil
@@ -39,112 +36,12 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags)
 	}
 	block.SetHeight(blockHeight)
 
-	blockHeader := &block.MsgBlock().Header
+	// The block must pass all of the validation rules which depend on the
-	if !fastAdd {
+	// position of the block within the block chain.
-		// Ensure the difficulty specified in the block header matches
+	err = b.checkBlockContext(block, prevNode, flags)
-		// the calculated difficulty based on the previous block and
-		// difficulty retarget rules.
-		expectedDifficulty, err := b.calcNextRequiredDifficulty(prevNode,
-			block.MsgBlock().Header.Timestamp)
 	if err != nil {
 		return err
 	}
-		blockDifficulty := blockHeader.Bits
-		if blockDifficulty != expectedDifficulty {
-			str := "block difficulty of %d is not the expected value of %d"
-			str = fmt.Sprintf(str, blockDifficulty, expectedDifficulty)
-			return ruleError(ErrUnexpectedDifficulty, str)
-		}
-
-		// Ensure the timestamp for the block header is after the
-		// median time of the last several blocks (medianTimeBlocks).
-		medianTime, err := b.calcPastMedianTime(prevNode)
-		if err != nil {
-			log.Errorf("calcPastMedianTime: %v", err)
-			return err
-		}
-		if !blockHeader.Timestamp.After(medianTime) {
-			str := "block timestamp of %v is not after expected %v"
-			str = fmt.Sprintf(str, blockHeader.Timestamp,
-				medianTime)
-			return ruleError(ErrTimeTooOld, str)
-		}
-
-		// Ensure all transactions in the block are finalized.
-		for _, tx := range block.Transactions() {
-			if !IsFinalizedTransaction(tx, blockHeight,
-				blockHeader.Timestamp) {
-				str := fmt.Sprintf("block contains "+
-					"unfinalized transaction %v", tx.Sha())
-				return ruleError(ErrUnfinalizedTx, str)
-			}
-		}
-
-	}
-
-	// Ensure chain matches up to predetermined checkpoints.
-	blockHash := block.Sha()
-	if !b.verifyCheckpoint(blockHeight, blockHash) {
-		str := fmt.Sprintf("block at height %d does not match "+
-			"checkpoint hash", blockHeight)
-		return ruleError(ErrBadCheckpoint, str)
-	}
-
-	// Find the previous checkpoint and prevent blocks which fork the main
-	// chain before it.  This prevents storage of new, otherwise valid,
-	// blocks which build off of old blocks that are likely at a much easier
-	// difficulty and therefore could be used to waste cache and disk space.
-	checkpointBlock, err := b.findPreviousCheckpoint()
-	if err != nil {
-		return err
-	}
-	if checkpointBlock != nil && blockHeight < checkpointBlock.Height() {
-		str := fmt.Sprintf("block at height %d forks the main chain "+
-			"before the previous checkpoint at height %d",
-			blockHeight, checkpointBlock.Height())
-		return ruleError(ErrForkTooOld, str)
-	}
-
-	if !fastAdd {
-		// Reject version 2 blocks once a majority of the network has
-		// upgraded.  This is part of BIP0066.
-		if blockHeader.Version < 3 && b.isMajorityVersion(3, prevNode,
-			b.chainParams.BlockRejectNumRequired) {
-
-			str := "new blocks with version %d are no longer valid"
-			str = fmt.Sprintf(str, blockHeader.Version)
-			return ruleError(ErrBlockVersionTooOld, str)
-		}
-
-		// Reject version 1 blocks once a majority of the network has
-		// upgraded.  This is part of BIP0034.
-		if blockHeader.Version < 2 && b.isMajorityVersion(2, prevNode,
-			b.chainParams.BlockRejectNumRequired) {
-
-			str := "new blocks with version %d are no longer valid"
-			str = fmt.Sprintf(str, blockHeader.Version)
-			return ruleError(ErrBlockVersionTooOld, str)
-		}
-
-		// Ensure coinbase starts with serialized block heights for
-		// blocks whose version is the serializedHeightVersion or
-		// newer once a majority of the network has upgraded.  This is
-		// part of BIP0034.
-		if ShouldHaveSerializedBlockHeight(blockHeader) &&
-			b.isMajorityVersion(serializedHeightVersion, prevNode,
-				b.chainParams.BlockEnforceNumRequired) {
-
-			expectedHeight := int64(0)
-			if prevNode != nil {
-				expectedHeight = prevNode.height + 1
-			}
-			coinbaseTx := block.Transactions()[0]
-			err := checkSerializedHeight(coinbaseTx, expectedHeight)
-			if err != nil {
-				return err
-			}
-		}
-	}
 
 	// Prune block nodes which are no longer needed before creating
 	// a new node.
@@ -157,7 +54,8 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags)
 
 	// Create a new block node for the block and add it to the in-memory
 	// block chain (could be either a side chain or the main chain).
-	newNode := newBlockNode(blockHeader, blockHash, blockHeight)
+	blockHeader := &block.MsgBlock().Header
+	newNode := newBlockNode(blockHeader, block.Sha(), blockHeight)
 	if prevNode != nil {
 		newNode.parent = prevNode
 		newNode.height = blockHeight

blockchain/validate.go

@@ -304,13 +304,12 @@ func CheckTransactionSanity(tx *btcutil.Tx) error {
 // difficulty is in min/max range and that the block hash is less than the
 // target difficulty as claimed.
 //
-//
 // The flags modify the behavior of this function as follows:
 //  - BFNoPoWCheck: The check to ensure the block hash is less than the target
 //    difficulty is not performed.
-func checkProofOfWork(block *btcutil.Block, powLimit *big.Int, flags BehaviorFlags) error {
+func checkProofOfWork(header *wire.BlockHeader, powLimit *big.Int, flags BehaviorFlags) error {
 	// The target difficulty must be larger than zero.
-	target := CompactToBig(block.MsgBlock().Header.Bits)
+	target := CompactToBig(header.Bits)
 	if target.Sign() <= 0 {
 		str := fmt.Sprintf("block target difficulty of %064x is too low",
 			target)
@@ -328,7 +327,8 @@ func checkProofOfWork(block *btcutil.Block, powLimit *big.Int, flags BehaviorFla
 	// to avoid proof of work checks is set.
 	if flags&BFNoPoWCheck != BFNoPoWCheck {
 		// The block hash must be less than the claimed target.
-		hashNum := ShaHashToBig(block.Sha())
+		hash := header.BlockSha()
+		hashNum := ShaHashToBig(&hash)
 		if hashNum.Cmp(target) > 0 {
 			str := fmt.Sprintf("block hash of %064x is higher than "+
 				"expected max of %064x", hashNum, target)
@@ -343,7 +343,7 @@ func checkProofOfWork(block *btcutil.Block, powLimit *big.Int, flags BehaviorFla
 // difficulty is in min/max range and that the block hash is less than the
 // target difficulty as claimed.
 func CheckProofOfWork(block *btcutil.Block, powLimit *big.Int) error {
-	return checkProofOfWork(block, powLimit, BFNone)
+	return checkProofOfWork(&block.MsgBlock().Header, powLimit, BFNone)
 }
 
 // CountSigOps returns the number of signature operations for all transaction
@@ -436,14 +436,58 @@ func CountP2SHSigOps(tx *btcutil.Tx, isCoinBaseTx bool, txStore TxStore) (int, e
 	return totalSigOps, nil
 }
 
+// checkBlockHeaderSanity performs some preliminary checks on a block header to
+// ensure it is sane before continuing with processing.  These checks are
+// context free.
+//
+// The flags do not modify the behavior of this function directly, however they
+// are needed to pass along to checkProofOfWork.
+func checkBlockHeaderSanity(header *wire.BlockHeader, powLimit *big.Int, timeSource MedianTimeSource, flags BehaviorFlags) error {
+	// Ensure the proof of work bits in the block header is in min/max range
+	// and the block hash is less than the target value described by the
+	// bits.
+	err := checkProofOfWork(header, powLimit, flags)
+	if err != nil {
+		return err
+	}
+
+	// A block timestamp must not have a greater precision than one second.
+	// This check is necessary because Go time.Time values support
+	// nanosecond precision whereas the consensus rules only apply to
+	// seconds and it's much nicer to deal with standard Go time values
+	// instead of converting to seconds everywhere.
+	if !header.Timestamp.Equal(time.Unix(header.Timestamp.Unix(), 0)) {
+		str := fmt.Sprintf("block timestamp of %v has a higher "+
+			"precision than one second", header.Timestamp)
+		return ruleError(ErrInvalidTime, str)
+	}
+
+	// Ensure the block time is not too far in the future.
+	maxTimestamp := timeSource.AdjustedTime().Add(time.Second *
+		MaxTimeOffsetSeconds)
+	if header.Timestamp.After(maxTimestamp) {
+		str := fmt.Sprintf("block timestamp of %v is too far in the "+
+			"future", header.Timestamp)
+		return ruleError(ErrTimeTooNew, str)
+	}
+
+	return nil
+}
+
 // checkBlockSanity performs some preliminary checks on a block to ensure it is
 // sane before continuing with block processing.  These checks are context free.
 //
 // The flags do not modify the behavior of this function directly, however they
-// are needed to pass along to checkProofOfWork.
+// are needed to pass along to checkBlockHeaderSanity.
 func checkBlockSanity(block *btcutil.Block, powLimit *big.Int, timeSource MedianTimeSource, flags BehaviorFlags) error {
-	// A block must have at least one transaction.
 	msgBlock := block.MsgBlock()
+	header := &msgBlock.Header
+	err := checkBlockHeaderSanity(header, powLimit, timeSource, flags)
+	if err != nil {
+		return err
+	}
+
+	// A block must have at least one transaction.
 	numTx := len(msgBlock.Transactions)
 	if numTx == 0 {
 		return ruleError(ErrNoTransactions, "block does not contain "+
@@ -466,35 +510,6 @@ func checkBlockSanity(block *btcutil.Block, powLimit *big.Int, timeSource Median
 		return ruleError(ErrBlockTooBig, str)
 	}
 
-	// Ensure the proof of work bits in the block header is in min/max range
-	// and the block hash is less than the target value described by the
-	// bits.
-	err := checkProofOfWork(block, powLimit, flags)
-	if err != nil {
-		return err
-	}
-
-	// A block timestamp must not have a greater precision than one second.
-	// This check is necessary because Go time.Time values support
-	// nanosecond precision whereas the consensus rules only apply to
-	// seconds and it's much nicer to deal with standard Go time values
-	// instead of converting to seconds everywhere.
-	header := &block.MsgBlock().Header
-	if !header.Timestamp.Equal(time.Unix(header.Timestamp.Unix(), 0)) {
-		str := fmt.Sprintf("block timestamp of %v has a higher "+
-			"precision than one second", header.Timestamp)
-		return ruleError(ErrInvalidTime, str)
-	}
-
-	// Ensure the block time is not too far in the future.
-	maxTimestamp := timeSource.AdjustedTime().Add(time.Second *
-		MaxTimeOffsetSeconds)
-	if header.Timestamp.After(maxTimestamp) {
-		str := fmt.Sprintf("block timestamp of %v is too far in the "+
-			"future", header.Timestamp)
-		return ruleError(ErrTimeTooNew, str)
-	}
-
 	// The first transaction in a block must be a coinbase.
 	transactions := block.Transactions()
 	if !IsCoinBase(transactions[0]) {
@@ -574,10 +589,162 @@ func CheckBlockSanity(block *btcutil.Block, powLimit *big.Int, timeSource Median
 	return checkBlockSanity(block, powLimit, timeSource, BFNone)
 }
 
-// ExtractCoinbaseHeight attempts to extract the height of the block
+// checkBlockHeaderContext peforms several validation checks on the block header
-// from the scriptSig of a coinbase transaction. Coinbase heights
+// which depend on its position within the block chain.
-// are only present in blocks of version 2 or later. This was added as part of
+//
-// BIP0034.
+// The flags modify the behavior of this function as follows:
+//  - BFFastAdd: All checks except those involving comparing the header against
+//    the checkpoints are not performed.
+func (b *BlockChain) checkBlockHeaderContext(header *wire.BlockHeader, prevNode *blockNode, flags BehaviorFlags) error {
+	// The genesis block is valid by definition.
+	if prevNode == nil {
+		return nil
+	}
+
+	fastAdd := flags&BFFastAdd == BFFastAdd
+	if !fastAdd {
+		// Ensure the difficulty specified in the block header matches
+		// the calculated difficulty based on the previous block and
+		// difficulty retarget rules.
+		expectedDifficulty, err := b.calcNextRequiredDifficulty(prevNode,
+			header.Timestamp)
+		if err != nil {
+			return err
+		}
+		blockDifficulty := header.Bits
+		if blockDifficulty != expectedDifficulty {
+			str := "block difficulty of %d is not the expected value of %d"
+			str = fmt.Sprintf(str, blockDifficulty, expectedDifficulty)
+			return ruleError(ErrUnexpectedDifficulty, str)
+		}
+
+		// Ensure the timestamp for the block header is after the
+		// median time of the last several blocks (medianTimeBlocks).
+		medianTime, err := b.calcPastMedianTime(prevNode)
+		if err != nil {
+			log.Errorf("calcPastMedianTime: %v", err)
+			return err
+		}
+		if !header.Timestamp.After(medianTime) {
+			str := "block timestamp of %v is not after expected %v"
+			str = fmt.Sprintf(str, header.Timestamp, medianTime)
+			return ruleError(ErrTimeTooOld, str)
+		}
+	}
+
+	// The height of this block is one more than the referenced previous
+	// block.
+	blockHeight := prevNode.height + 1
+
+	// Ensure chain matches up to predetermined checkpoints.
+	blockHash := header.BlockSha()
+	if !b.verifyCheckpoint(blockHeight, &blockHash) {
+		str := fmt.Sprintf("block at height %d does not match "+
+			"checkpoint hash", blockHeight)
+		return ruleError(ErrBadCheckpoint, str)
+	}
+
+	// Find the previous checkpoint and prevent blocks which fork the main
+	// chain before it.  This prevents storage of new, otherwise valid,
+	// blocks which build off of old blocks that are likely at a much easier
+	// difficulty and therefore could be used to waste cache and disk space.
+	checkpointBlock, err := b.findPreviousCheckpoint()
+	if err != nil {
+		return err
+	}
+	if checkpointBlock != nil && blockHeight < checkpointBlock.Height() {
+		str := fmt.Sprintf("block at height %d forks the main chain "+
+			"before the previous checkpoint at height %d",
+			blockHeight, checkpointBlock.Height())
+		return ruleError(ErrForkTooOld, str)
+	}
+
+	if !fastAdd {
+		// Reject version 2 blocks once a majority of the network has
+		// upgraded.  This is part of BIP0066.
+		if header.Version < 3 && b.isMajorityVersion(3, prevNode,
+			b.chainParams.BlockRejectNumRequired) {
+
+			str := "new blocks with version %d are no longer valid"
+			str = fmt.Sprintf(str, header.Version)
+			return ruleError(ErrBlockVersionTooOld, str)
+		}
+
+		// Reject version 1 blocks once a majority of the network has
+		// upgraded.  This is part of BIP0034.
+		if header.Version < 2 && b.isMajorityVersion(2, prevNode,
+			b.chainParams.BlockRejectNumRequired) {
+
+			str := "new blocks with version %d are no longer valid"
+			str = fmt.Sprintf(str, header.Version)
+			return ruleError(ErrBlockVersionTooOld, str)
+		}
+	}
+
+	return nil
+}
+
+// checkBlockContext peforms several validation checks on the block which depend
+// on its position within the block chain.
+//
+// The flags modify the behavior of this function as follows:
+//  - BFFastAdd: The transaction are not checked to see if they are finalized
+//    and the somewhat expensive BIP0034 validation is not performed.
+//
+// The flags are also passed to checkBlockHeaderContext.  See its documentation
+// for how the flags modify its behavior.
+func (b *BlockChain) checkBlockContext(block *btcutil.Block, prevNode *blockNode, flags BehaviorFlags) error {
+	// The genesis block is valid by definition.
+	if prevNode == nil {
+		return nil
+	}
+
+	// Perform all block header related validation checks.
+	header := &block.MsgBlock().Header
+	err := b.checkBlockHeaderContext(header, prevNode, flags)
+	if err != nil {
+		return err
+	}
+
+	fastAdd := flags&BFFastAdd == BFFastAdd
+	if !fastAdd {
+		// The height of this block is one more than the referenced
+		// previous block.
+		blockHeight := prevNode.height + 1
+
+		// Ensure all transactions in the block are finalized.
+		for _, tx := range block.Transactions() {
+			if !IsFinalizedTransaction(tx, blockHeight,
+				header.Timestamp) {
+
+				str := fmt.Sprintf("block contains unfinalized "+
+					"transaction %v", tx.Sha())
+				return ruleError(ErrUnfinalizedTx, str)
+			}
+		}
+
+		// Ensure coinbase starts with serialized block heights for
+		// blocks whose version is the serializedHeightVersion or newer
+		// once a majority of the network has upgraded.  This is part of
+		// BIP0034.
+		if ShouldHaveSerializedBlockHeight(header) &&
+			b.isMajorityVersion(serializedHeightVersion, prevNode,
+				b.chainParams.BlockEnforceNumRequired) {
+
+			coinbaseTx := block.Transactions()[0]
+			err := checkSerializedHeight(coinbaseTx, blockHeight)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
+// ExtractCoinbaseHeight attempts to extract the height of the block from the
+// scriptSig of a coinbase transaction.  Coinbase heights are only present in
+// blocks of version 2 or later.  This was added as part of BIP0034.
 func ExtractCoinbaseHeight(coinbaseTx *btcutil.Tx) (int64, error) {
 	sigScript := coinbaseTx.MsgTx().TxIn[0].SignatureScript
 	if len(sigScript) < 1 {