blockchain: make BIP30 enforcement logic testable

diff --git a/blockchain/bip30_test.go b/blockchain/bip30_test.go
new file mode 100644
index 0000000..c741b32
--- /dev/null
+++ b/blockchain/bip30_test.go
@@ -0,0 +1,86 @@
+package blockchain
+
+import (
+	"testing"
+
+	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/chaincfg/chainhash"
+	"github.com/stretchr/testify/require"
+)
+
+// mustHashFromStr is a helper to parse hashes inside tests.  It fails the
+// test immediately when the string is not a valid hash.
+func mustHashFromStr(t *testing.T, s string) chainhash.Hash {
+	t.Helper()
+
+	h, err := chainhash.NewHashFromStr(s)
+	require.NoError(t, err)
+
+	return *h
+}
+
+// TestBip0030CheckNeededExceptions makes sure the two historical blocks that
+// overwrote earlier coinbases do not trigger the BIP30 check.  This mirrors the
+// exceptions Bitcoin Core ships as part of the original BIP30 fix.
+func TestBip0030CheckNeededExceptions(t *testing.T) {
+	params := chaincfg.MainNetParams
+
+	cases := []struct {
+		height int32
+		hash   string
+	}{{
+		height: 91842,
+		hash:   "00000000000a4d0a398161ffc163c503763b1f4360639393e0e4c8e300e0caec",
+	}, {
+		height: 91880,
+		hash:   "00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721",
+	}}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.hash, func(t *testing.T) {
+			node := &blockNode{
+				height: tc.height,
+				hash:   mustHashFromStr(t, tc.hash),
+			}
+			require.False(t, bip0030CheckNeeded(node, &params))
+		})
+	}
+}
+
+// TestBip0030CheckNeededBeforeBIP34 ensures we still run the BIP30 check prior
+// to the recorded BIP34 activation height.
+func TestBip0030CheckNeededBeforeBIP34(t *testing.T) {
+	params := chaincfg.MainNetParams
+
+	node := &blockNode{
+		height: params.BIP0034Height - 1,
+		hash:   mustHashFromStr(t, "0000000000000000000000000000000000000000000000000000000000000001"),
+	}
+
+	require.True(t, bip0030CheckNeeded(node, &params))
+}
+
+// TestBip0030CheckNeededAfterBIP34 covers the happy-path where we are on a
+// chain that contains the recorded BIP34 activation block.  In that case the
+// expensive duplicate-coinbase check can be skipped.
+func TestBip0030CheckNeededAfterBIP34(t *testing.T) {
+	params := chaincfg.MainNetParams
+
+	ancestor := &blockNode{
+		height: params.BIP0034Height,
+		hash:   mustHashFromStr(t, "000000000000024b89b42a942fe0d9fea3bb44ab7bd1b19115dd6a759c0808b8"),
+	}
+	parent := &blockNode{
+		height: ancestor.height + 1,
+		hash:   mustHashFromStr(t, "0000000000000000000000000000000000000000000000000000000000000002"),
+		parent: ancestor,
+	}
+	node := &blockNode{
+		height: parent.height + 1,
+		hash:   mustHashFromStr(t, "0000000000000000000000000000000000000000000000000000000000000003"),
+		parent: parent,
+	}
+
+	require.False(t, bip0030CheckNeeded(node, &params))
+}
diff --git a/blockchain/validate.go b/blockchain/validate.go
index 1cc0ec3..f650fa9 100644
--- a/blockchain/validate.go
+++ b/blockchain/validate.go
@@ -191,6 +191,41 @@ func isBIP0030Node(node *blockNode) bool {
 	return false
 }
 
+// bip0030CheckNeeded determines if the expensive overwrite check from BIP0030
+// needs to be executed for the provided block node under the supplied network
+// parameters.
+//
+// There are two blocks in the chain which violate this rule, so the check must
+// be skipped for those blocks.  The isBIP0030Node function is used to determine
+// if this block is one of the two blocks that must be skipped.
+//
+// In addition, as of BIP0034, duplicate coinbases are no longer possible due to
+// its requirement for including the block height in the coinbase and thus it is
+// no longer possible to create transactions that 'overwrite' older ones.
+// Therefore, only enforce the rule if BIP0034 is not yet active.  This is a
+// useful optimization because the BIP0030 check is expensive since it involves
+// a ton of cache misses in the utxoset.
+func bip0030CheckNeeded(node *blockNode, params *chaincfg.Params) bool {
+	// Sanity checks for the inputs not to dereference a nil pointer.
+	if node == nil || params == nil {
+		return false
+	}
+
+	// Skip the check for the historical mainnet blocks that overwrote
+	// earlier coinbases before BIP0030 existed.
+	if isBIP0030Node(node) {
+		return false
+	}
+
+	// Once BIP0034 is known to be active on this chain, duplicate coinbases
+	// can no longer occur, so the check can be omitted.
+	if node.height >= params.BIP0034Height {
+		return false
+	}
+
+	return true
+}
+
 // CalcBlockSubsidy returns the subsidy amount a block at the provided height
 // should have. This is mainly used for determining how much the coinbase for
 // newly generated blocks awards as well as validating the coinbase for blocks
@@ -1102,20 +1137,9 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block *btcutil.Block, vi
 	// BIP0030 added a rule to prevent blocks which contain duplicate
 	// transactions that 'overwrite' older transactions which are not fully
 	// spent.  See the documentation for checkBIP0030 for more details.
-	//
-	// There are two blocks in the chain which violate this rule, so the
-	// check must be skipped for those blocks.  The isBIP0030Node function
-	// is used to determine if this block is one of the two blocks that must
-	// be skipped.
-	//
-	// In addition, as of BIP0034, duplicate coinbases are no longer
-	// possible due to its requirement for including the block height in the
-	// coinbase and thus it is no longer possible to create transactions
-	// that 'overwrite' older ones.  Therefore, only enforce the rule if
-	// BIP0034 is not yet active.  This is a useful optimization because the
-	// BIP0030 check is expensive since it involves a ton of cache misses in
-	// the utxoset.
-	if !isBIP0030Node(node) && (node.height < b.chainParams.BIP0034Height) {
+	// Sometimes BIP0030 must to skipped (as an exception) or may be skipped
+	// (as an optimization), see bip0030CheckNeeded for details.
+	if bip0030CheckNeeded(node, b.chainParams) {
 		err := b.checkBIP0030(node, block, view)
 		if err != nil {
 			return err