chainntnfs: add cross interface implementation tests

This commit refactors the existing chainntnfns package in order to
allow more easily allow integration into the main system, by allowing
one to gain access to a set of end-to-end tests for a particular
ChainNotifier implementation.

In order to achieve this, the existing set of tests for the only
concrete implementation (`BtcdNoitifer`) have been refactored to test
against all “registered” notifier interfaces registered. This is
achieved by creating the concept of a “driver” for each concrete
`ChainNotifer` implementation. Once a the package of a particular
driver is imported, solely for the side effects, the init() method
automatically registers the driver.

Additionally, the documentation in various areas of the package have
been cleaned up a bit.

chainntfs/chainntfs_test.go

@@ -1 +0,0 @@
-package chainntnfs

chainntnfs/btcdnotify/btcd.go

@@ -7,13 +7,20 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/lightningnetwork/lnd/chainntfs"
+	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/roasbeef/btcd/btcjson"
 	"github.com/roasbeef/btcd/wire"
 	"github.com/roasbeef/btcrpcclient"
 	"github.com/roasbeef/btcutil"
 )
 
+const (
+
+	// notifierType uniquely identifies this concrete implementation of the
+	// ChainNotifier interface.
+	notifierType = "btcd"
+)
+
 // BtcdNotifier implements the ChainNotifier interface using btcd's websockets
 // notifications. Multiple concurrent clients are supported. All notifications
 // are achieved via non-blocking sends on client channels.
@@ -42,10 +49,10 @@ type BtcdNotifier struct {
 // Ensure BtcdNotifier implements the ChainNotifier interface at compile time.
 var _ chainntnfs.ChainNotifier = (*BtcdNotifier)(nil)
 
-// NewBtcdNotifier returns a new BtcdNotifier instance. This function assumes
-// the btcd node detailed in the passed configuration is already running, and
+// New returns a new BtcdNotifier instance. This function assumes the btcd node
+// detailed in the passed configuration is already running, and
 // willing to accept new websockets clients.
-func NewBtcdNotifier(config *btcrpcclient.ConnConfig) (*BtcdNotifier, error) {
+func New(config *btcrpcclient.ConnConfig) (*BtcdNotifier, error) {
 	notifier := &BtcdNotifier{
 		notificationRegistry: make(chan interface{}),
 
@@ -66,8 +73,8 @@ func NewBtcdNotifier(config *btcrpcclient.ConnConfig) (*BtcdNotifier, error) {
 		OnRedeemingTx:       notifier.onRedeemingTx,
 	}
 
-	// Disable connecting to btcd within the btcrpcclient.New method. We defer
-	// establishing the connection to our .Start() method.
+	// Disable connecting to btcd within the btcrpcclient.New method. We
+	// defer establishing the connection to our .Start() method.
 	config.DisableConnectOnNew = true
 	config.DisableAutoReconnect = false
 	chainConn, err := btcrpcclient.New(config, ntfnCallbacks)

chainntnfs/btcdnotify/confheap.go

@@ -1,17 +1,21 @@
 package btcdnotify
 
-// confEntry...
+// confEntry represents an entry in the min-confirmation heap. .
 type confEntry struct {
 	*confirmationsNotification
 
 	triggerHeight uint32
 }
 
-// confirmationHeap...
+// confirmationHeap is a list of confEntries sorted according to nearest
+// "confirmation" height.Each entry within the min-confirmation heap is sorted
+// according to the smallest dleta from the current blockheight to the
+// triggerHeight of the next entry confirmationHeap
 type confirmationHeap struct {
 	items []*confEntry
 }
 
+// newConfirmationHeap returns a new confirmationHeap with zero items.
 func newConfirmationHeap() *confirmationHeap {
 	var confItems []*confEntry
 	return &confirmationHeap{confItems}

chainntnfs/btcdnotify/driver.go

@@ -0,0 +1,40 @@
+package btcdnotify
+
+import (
+	"fmt"
+
+	"github.com/lightningnetwork/lnd/chainntnfs"
+	"github.com/roasbeef/btcrpcclient"
+)
+
+// createNewNotifier creates a new instance of the ChainNotifier interface
+// implemented by BtcdNotifier.
+func createNewNotifier(args ...interface{}) (chainntnfs.ChainNotifier, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("incorrect number of arguments to .New(...), "+
+			"expected 1, instead passed %v", len(args))
+	}
+
+	config, ok := args[0].(*btcrpcclient.ConnConfig)
+	if !ok {
+		return nil, fmt.Errorf("first argument to btcdnotifier.New is " +
+			"incorrect, expected a *btcrpcclient.ConnConfig")
+	}
+
+	return New(config)
+}
+
+// init registers a driver for the BtcdNotifier concrete implementation of the
+// chainntnfs.ChainNotifier interface.
+func init() {
+	// Register the driver.
+	notifier := &chainntnfs.NotifierDriver{
+		NotifierType: notifierType,
+		New:          createNewNotifier,
+	}
+
+	if err := chainntnfs.RegisterNotifier(notifier); err != nil {
+		panic(fmt.Sprintf("failed to register notifier driver '%s': %v",
+			notifierType, err))
+	}
+}

chainntnfs/interface.go

@@ -1,6 +1,11 @@
 package chainntnfs
 
-import "github.com/roasbeef/btcd/wire"
+import (
+	"fmt"
+	"sync"
+
+	"github.com/roasbeef/btcd/wire"
+)
 
 // ChainNotifier represents a trusted source to receive notifications concerning
 // targeted events on the Bitcoin blockchain. The interface specification is
@@ -104,3 +109,74 @@ type BlockEpoch struct {
 type BlockEpochEvent struct {
 	Epochs chan *BlockEpoch // MUST be buffered.
 }
+
+// NotifierDriver represents a "driver" for a particular interface. A driver is
+// indentified by a globally unique string identifier along with a 'New()'
+// method which is responsible for initializing a particular ChainNotifier
+// concrete implementation.
+type NotifierDriver struct {
+	// NotifierType is a string which uniquely identifes the ChainNotifier
+	// that this driver, drives.
+	NotifierType string
+
+	// New creates a new instance of a concrete ChainNotifier
+	// implementation given a variadic set up arguments. The function takes
+	// a varidaic number of interface paramters in order to provide
+	// initialization flexibility, thereby accomodating several potential
+	// ChainNotifier implementations.
+	New func(args ...interface{}) (ChainNotifier, error)
+}
+
+var (
+	notifiers   = make(map[string]*NotifierDriver)
+	registerMtx sync.Mutex
+)
+
+// RegisteredNotifiers returns a slice of all currently registered notifiers.
+//
+// NOTE: This function is safe for concurrent access.
+func RegisteredNotifiers() []*NotifierDriver {
+	registerMtx.Lock()
+	defer registerMtx.Unlock()
+
+	drivers := make([]*NotifierDriver, 0, len(notifiers))
+	for _, driver := range notifiers {
+		drivers = append(drivers, driver)
+	}
+
+	return drivers
+}
+
+// RegisterNotifier registers a NotifierDriver which is capable of driving a
+// concrete ChainNotifier interface. In the case that this driver has already
+// been registered, an error is returned.
+//
+// NOTE: This function is safe for concurrent access.
+func RegisterNotifier(driver *NotifierDriver) error {
+	registerMtx.Lock()
+	defer registerMtx.Unlock()
+
+	if _, ok := notifiers[driver.NotifierType]; ok {
+		return fmt.Errorf("notifier already registered")
+	}
+
+	notifiers[driver.NotifierType] = driver
+
+	return nil
+}
+
+// SupportedNotifiers returns a slice of strings that represent the database
+// drivers that have been registered and are therefore supported.
+//
+// NOTE: This function is safe for concurrent access.
+func SupportedNotifiers() []string {
+	registerMtx.Lock()
+	defer registerMtx.Unlock()
+
+	supportedNotifiers := make([]string, 0, len(notifiers))
+	for driverName := range notifiers {
+		supportedNotifiers = append(supportedNotifiers, driverName)
+	}
+
+	return supportedNotifiers
+}

chainntnfs/interface_test.go

@@ -1,11 +1,13 @@
-package btcdnotify
+package chainntnfs_test
 
 import (
 	"bytes"
 	"testing"
 	"time"
 
-	"github.com/lightningnetwork/lnd/chainntfs"
+	"github.com/lightningnetwork/lnd/chainntnfs"
+	_ "github.com/lightningnetwork/lnd/chainntnfs/btcdnotify"
+
 	"github.com/roasbeef/btcd/btcec"
 	"github.com/roasbeef/btcd/chaincfg"
 	"github.com/roasbeef/btcd/rpctest"
@@ -181,7 +183,7 @@ func testSpendNotification(miner *rpctest.Harness,
 	notifier chainntnfs.ChainNotifier, t *testing.T) {
 
 	// We'd like to test the spend notifiations for all
-	// chainntnfs.ChainNotifier concrete implemenations.
+	// ChainNotifier concrete implemenations.
 	//
 	// To do so, we first create a new output to our test target
 	// address.
@@ -286,15 +288,22 @@ var ntfnTests = []func(node *rpctest.Harness, notifier chainntnfs.ChainNotifier,
 	testSpendNotification,
 }
 
-// TODO(roasbeef): make test generic across all interfaces?
-//  * indeed!
-//  * requires interface implementation registration
-func TestBtcdNotifier(t *testing.T) {
-
+// TestInterfaces tests all registered interfaces with a unified set of tests
+// which excersie each of the required methods found within the ChainNotifier
+// interface.
+//
+// NOTE: In the future, when additional implementations of the ChainNotifier
+// interface have been implemented, in order to ensure the new concrete
+// implementation is automatically tested, two steps must be undertaken. First,
+// one needs add a "non-captured" (_) import from the new sub-package. This
+// import should trigger an init() method within the package which registeres
+// the interface. Second, an additional case in the switch within the main loop
+// below needs to be added which properly initializes the interface.
+func TestInterfaces(t *testing.T) {
 	// Initialize the harness around a btcd node which will serve as our
-	// dedicated miner to generate blocks, cause re-orgs, etc. We'll set
-	// up this node with a chain length of 125, so we have plentyyy of BTC
-	// to play around with.
+	// dedicated miner to generate blocks, cause re-orgs, etc. We'll set up
+	// this node with a chain length of 125, so we have plentyyy of BTC to
+	// play around with.
 	miner, err := rpctest.New(netParams, nil, nil)
 	if err != nil {
 		t.Fatalf("unable to create mining node: %v", err)
@@ -304,17 +313,30 @@ func TestBtcdNotifier(t *testing.T) {
 		t.Fatalf("unable to set up mining node: %v", err)
 	}
 
-	nodeConfig := miner.RPCConfig()
-	notifier, err := NewBtcdNotifier(&nodeConfig)
-	if err != nil {
-		t.Fatalf("unable to create notifier: %v", err)
-	}
-	if err := notifier.Start(); err != nil {
-		t.Fatalf("unable to start notifier: %v", err)
-	}
-	defer notifier.Stop()
+	rpcConfig := miner.RPCConfig()
 
-	for _, ntfnTest := range ntfnTests {
-		ntfnTest(miner, notifier, t)
+	var notifier chainntnfs.ChainNotifier
+	for _, notifierDriver := range chainntnfs.RegisteredNotifiers() {
+		notifierType := notifierDriver.NotifierType
+
+		switch notifierType {
+		case "btcd":
+			notifier, err = notifierDriver.New(&rpcConfig)
+			if err != nil {
+				t.Fatalf("unable to create %v notifier: %v",
+					notifierType, err)
+			}
+		}
+
+		if err := notifier.Start(); err != nil {
+			t.Fatalf("unable to start notifier %v: %v",
+				notifierType, err)
+		}
+
+		for _, ntfnTest := range ntfnTests {
+			ntfnTest(miner, notifier, t)
+		}
+
+		notifier.Stop()
 	}
 }