From 4d1a1d2799bc2b8c5e41d043b0176d0afaecf5b8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 3 Aug 2016 22:13:10 -0700 Subject: [PATCH] chainntnfs: add cross interface implementation tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 - {chainntfs => chainntnfs}/btcdnotify/btcd.go | 19 +++-- .../btcdnotify/confheap.go | 8 +- chainntnfs/btcdnotify/driver.go | 40 ++++++++++ .../chainntfs.go => chainntnfs/interface.go | 78 ++++++++++++++++++- .../interface_test.go | 66 ++++++++++------ {chainntfs => chainntnfs}/log.go | 0 7 files changed, 180 insertions(+), 32 deletions(-) delete mode 100644 chainntfs/chainntfs_test.go rename {chainntfs => chainntnfs}/btcdnotify/btcd.go (96%) rename {chainntfs => chainntnfs}/btcdnotify/confheap.go (77%) create mode 100644 chainntnfs/btcdnotify/driver.go rename chainntfs/chainntfs.go => chainntnfs/interface.go (67%) rename chainntfs/btcdnotify/btcdnotify_test.go => chainntnfs/interface_test.go (84%) rename {chainntfs => chainntnfs}/log.go (100%) diff --git a/chainntfs/chainntfs_test.go b/chainntfs/chainntfs_test.go deleted file mode 100644 index 54a8429f..00000000 --- a/chainntfs/chainntfs_test.go +++ /dev/null @@ -1 +0,0 @@ -package chainntnfs diff --git a/chainntfs/btcdnotify/btcd.go b/chainntnfs/btcdnotify/btcd.go similarity index 96% rename from chainntfs/btcdnotify/btcd.go rename to chainntnfs/btcdnotify/btcd.go index 9a5fd015..ae83d220 100644 --- a/chainntfs/btcdnotify/btcd.go +++ b/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) diff --git a/chainntfs/btcdnotify/confheap.go b/chainntnfs/btcdnotify/confheap.go similarity index 77% rename from chainntfs/btcdnotify/confheap.go rename to chainntnfs/btcdnotify/confheap.go index 0ac454cf..35345ce4 100644 --- a/chainntfs/btcdnotify/confheap.go +++ b/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} diff --git a/chainntnfs/btcdnotify/driver.go b/chainntnfs/btcdnotify/driver.go new file mode 100644 index 00000000..1d3777fa --- /dev/null +++ b/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)) + } +} diff --git a/chainntfs/chainntfs.go b/chainntnfs/interface.go similarity index 67% rename from chainntfs/chainntfs.go rename to chainntnfs/interface.go index 50571478..58d54d6a 100644 --- a/chainntfs/chainntfs.go +++ b/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 +} diff --git a/chainntfs/btcdnotify/btcdnotify_test.go b/chainntnfs/interface_test.go similarity index 84% rename from chainntfs/btcdnotify/btcdnotify_test.go rename to chainntnfs/interface_test.go index e2c21bbc..e5febffc 100644 --- a/chainntfs/btcdnotify/btcdnotify_test.go +++ b/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() } } diff --git a/chainntfs/log.go b/chainntnfs/log.go similarity index 100% rename from chainntfs/log.go rename to chainntnfs/log.go