blockchain: Consistency pass on subscribe code.

This takes care of a few minor nits on the recently merged subscribe
code.  In particular:

- Avoid extra unnecessary allocation on notifications slice
- Avoid defer overhead on notification mutex in sendNotifications
- Make test function comment start with the name of the function per
  Effective Go guidelines
- Use constant for number of subscribers in test
- Don't exceed column 80 in test print

blockchain/chain.go

@@ -1380,7 +1380,6 @@ func New(config *Config) (*BlockChain, error) {
 		db:                  config.DB,
 		chainParams:         params,
 		timeSource:          config.TimeSource,
-		notifications:       make([]NotificationCallback, 0),
 		sigCache:            config.SigCache,
 		indexManager:        config.IndexManager,
 		minRetargetTimespan: targetTimespan / adjustmentFactor,

blockchain/notifications.go

@@ -71,12 +71,11 @@ func (b *BlockChain) Subscribe(callback NotificationCallback) {
 // caller requested notifications by providing a callback function in the call
 // to New.
 func (b *BlockChain) sendNotification(typ NotificationType, data interface{}) {
-	b.notificationsLock.RLock()
-	defer b.notificationsLock.RUnlock()
-
 	// Generate and send the notification.
 	n := Notification{Type: typ, Data: data}
+	b.notificationsLock.RLock()
 	for _, callback := range b.notifications {
 		callback(&n)
 	}
+	b.notificationsLock.RUnlock()
 }

blockchain/notifications_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/btcsuite/btcd/chaincfg"
 )
 
-// Test that notification callbacks are fired on events.
+// TestNotifications ensures that notification callbacks are fired on events.
 func TestNotifications(t *testing.T) {
 	blocks, err := loadBlocks("blk_0_to_4.dat.bz2")
 	if err != nil {
@@ -19,33 +19,34 @@ func TestNotifications(t *testing.T) {
 	}
 
 	// Create a new database and chain instance to run tests against.
-	chain, teardownFunc, err :=
+	chain, teardownFunc, err := chainSetup("notifications",
-		chainSetup("notifications", &chaincfg.MainNetParams)
+		&chaincfg.MainNetParams)
 	if err != nil {
 		t.Fatalf("Failed to setup chain instance: %v", err)
 	}
 	defer teardownFunc()
 
 	notificationCount := 0
-
 	callback := func(notification *blockchain.Notification) {
 		if notification.Type == blockchain.NTBlockAccepted {
 			notificationCount++
 		}
 	}
 
-	// Register callback 3 times then assert it is called three times
+	// Register callback multiple times then assert it is called that many
-	chain.Subscribe(callback)
+	// times.
-	chain.Subscribe(callback)
+	const numSubscribers = 3
-	chain.Subscribe(callback)
+	for i := 0; i < numSubscribers; i++ {
+		chain.Subscribe(callback)
+	}
 
 	_, _, err = chain.ProcessBlock(blocks[1], blockchain.BFNone)
 	if err != nil {
 		t.Fatalf("ProcessBlock fail on block 1: %v\n", err)
 	}
 
-	if notificationCount != 3 {
+	if notificationCount != numSubscribers {
-		t.Fatalf("Expected notification callback to be executed 3 times, found %d",
+		t.Fatalf("Expected notification callback to be executed %d "+
-			notificationCount)
+			"times, found %d", numSubscribers, notificationCount)
 	}
 }