From 712399c0db4441b2e4d159461a92fc90e8ad5fa6 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Fri, 2 Dec 2016 22:54:04 -0600
Subject: [PATCH] blockchain: Correct startup threshold state warns.

The thresholdState and deploymentState functions expect the block node
for the block prior to which the threshold state is calculated, however
the startup code which checked the threshold states was using the
current best node instead of its parent.

While here, also update the comments and rename a couple of variables to
help make this fact more clear.
---
 blockchain/chainio.go        | 13 +++++++++++--
 blockchain/thresholdstate.go |  8 ++++----
 blockchain/versionbits.go    | 11 ++++++++++-
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/blockchain/chainio.go b/blockchain/chainio.go
index 21624159..e510fd5d 100644
--- a/blockchain/chainio.go
+++ b/blockchain/chainio.go
@@ -1905,6 +1905,15 @@ func (b *BlockChain) initThresholdCaches() error {
 			"change.  This might take a while...")
 	}
 
+	// Get the previous block node.  This function is used over simply
+	// accessing b.bestNode.parent directly as it will dynamically create
+	// previous block nodes as needed.  This helps allow only the pieces of
+	// the chain that are needed to remain in memory.
+	prevNode, err := b.getPrevNodeFromNode(b.bestNode)
+	if err != nil {
+		return err
+	}
+
 	// Initialize the warning and deployment caches by calculating the
 	// threshold state for each of them.  This will ensure the caches are
 	// populated and any states that needed to be recalculated due to
@@ -1912,7 +1921,7 @@ func (b *BlockChain) initThresholdCaches() error {
 	for bit := uint32(0); bit < vbNumBits; bit++ {
 		checker := bitConditionChecker{bit: bit, chain: b}
 		cache := &b.warningCaches[bit]
-		_, err := b.thresholdState(b.bestNode, checker, cache)
+		_, err := b.thresholdState(prevNode, checker, cache)
 		if err != nil {
 			return err
 		}
@@ -1921,7 +1930,7 @@ func (b *BlockChain) initThresholdCaches() error {
 		deployment := &b.chainParams.Deployments[id]
 		cache := &b.deploymentCaches[id]
 		checker := deploymentChecker{deployment: deployment, chain: b}
-		_, err := b.thresholdState(b.bestNode, checker, cache)
+		_, err := b.thresholdState(prevNode, checker, cache)
 		if err != nil {
 			return err
 		}
diff --git a/blockchain/thresholdstate.go b/blockchain/thresholdstate.go
index e7620be9..f1ca8d19 100644
--- a/blockchain/thresholdstate.go
+++ b/blockchain/thresholdstate.go
@@ -141,9 +141,9 @@ func newThresholdCaches(numCaches uint32) []thresholdStateCache {
 	return caches
 }
 
-// thresholdState returns the current rule change threshold state for the given
-// node and deployment ID.  The cache is used to ensure the threshold states for
-// previous windows are only calculated once.
+// thresholdState returns the current rule change threshold state for the block
+// AFTER the given node and deployment ID.  The cache is used to ensure the
+// threshold states for previous windows are only calculated once.
 //
 // This function MUST be called with the chain state lock held (for writes).
 func (b *BlockChain) thresholdState(prevNode *blockNode, checker thresholdConditionChecker, cache *thresholdStateCache) (ThresholdState, error) {
@@ -305,7 +305,7 @@ func (b *BlockChain) thresholdState(prevNode *blockNode, checker thresholdCondit
 }
 
 // ThresholdState returns the current rule change threshold state of the given
-// deployment ID for the end of the current best chain.
+// deployment ID for the block AFTER then end of the current best chain.
 //
 // This function is safe for concurrent access.
 func (b *BlockChain) ThresholdState(deploymentID uint32) (ThresholdState, error) {
diff --git a/blockchain/versionbits.go b/blockchain/versionbits.go
index c2ad2cde..4da737d9 100644
--- a/blockchain/versionbits.go
+++ b/blockchain/versionbits.go
@@ -244,12 +244,21 @@ func (b *BlockChain) CalcNextBlockVersion() (int32, error) {
 //
 // This function MUST be called with the chain state lock held (for writes)
 func (b *BlockChain) warnUnknownRuleActivations(node *blockNode) error {
+	// Get the previous block node.  This function is used over simply
+	// accessing node.parent directly as it will dynamically create previous
+	// block nodes as needed.  This helps allow only the pieces of the chain
+	// that are needed to remain in memory.
+	prevNode, err := b.getPrevNodeFromNode(node)
+	if err != nil {
+		return err
+	}
+
 	// Warn if any unknown new rules are either about to activate or have
 	// already been activated.
 	for bit := uint32(0); bit < vbNumBits; bit++ {
 		checker := bitConditionChecker{bit: bit, chain: b}
 		cache := &b.warningCaches[bit]
-		state, err := b.thresholdState(node, checker, cache)
+		state, err := b.thresholdState(prevNode, checker, cache)
 		if err != nil {
 			return err
 		}