chainntnfs: validate conf/spend ntfn registration parameters

A height hint not being set would cause lnd to scan for the
confirmation/spend of a txid/outpoint/address from genesis.

The number of confirmations not being set within a confirmation request
would cause the internal TxNotifier to deadlock when dispatching
updates.

chainntnfs/txnotifier.go

@@ -46,10 +46,22 @@ var (
 	// with the TxNotifier but it been shut down.
 	ErrTxNotifierExiting = errors.New("TxNotifier is exiting")
 
-	// ErrTxMaxConfs signals that the user requested a number of
-	// confirmations beyond the reorg safety limit.
-	ErrTxMaxConfs = fmt.Errorf("too many confirmations requested, max is %d",
-		MaxNumConfs)
+	// ErrNoScript is an error returned when a confirmation/spend
+	// registration is attempted without providing an accompanying output
+	// script.
+	ErrNoScript = errors.New("an output script must be provided")
+
+	// ErrNoHeightHint is an error returned when a confirmation/spend
+	// registration is attempted without providing an accompanying height
+	// hint.
+	ErrNoHeightHint = errors.New("a height hint greater than 0 must be " +
+		"provided")
+
+	// ErrNumConfsOutOfRange is an error returned when a confirmation/spend
+	// registration is attempted and the number of confirmations provided is
+	// out of range.
+	ErrNumConfsOutOfRange = fmt.Errorf("number of confirmations must be "+
+		"between %d and %d", 1, MaxNumConfs)
 )
 
 // rescanState indicates the progression of a registration before the notifier
@@ -531,15 +543,27 @@ func NewTxNotifier(startHeight uint32, reorgSafetyLimit uint32,
 func (n *TxNotifier) newConfNtfn(txid *chainhash.Hash,
 	pkScript []byte, numConfs, heightHint uint32) (*ConfNtfn, error) {
 
-	confRequest, err := NewConfRequest(txid, pkScript)
-	if err != nil {
-		return nil, err
+	// An accompanying output script must always be provided.
+	if len(pkScript) == 0 {
+		return nil, ErrNoScript
 	}
 
 	// Enforce that we will not dispatch confirmations beyond the reorg
 	// safety limit.
-	if numConfs > n.reorgSafetyLimit {
-		return nil, ErrTxMaxConfs
+	if numConfs == 0 || numConfs > n.reorgSafetyLimit {
+		return nil, ErrNumConfsOutOfRange
+	}
+
+	// A height hint must be provided to prevent scanning from the genesis
+	// block.
+	if heightHint == 0 {
+		return nil, ErrNoHeightHint
+	}
+
+	// Ensure the output script is of a supported type.
+	confRequest, err := NewConfRequest(txid, pkScript)
+	if err != nil {
+		return nil, err
 	}
 
 	confID := atomic.AddUint64(&n.confClientCounter, 1)
@@ -910,6 +934,18 @@ func (n *TxNotifier) dispatchConfDetails(
 func (n *TxNotifier) newSpendNtfn(outpoint *wire.OutPoint,
 	pkScript []byte, heightHint uint32) (*SpendNtfn, error) {
 
+	// An accompanying output script must always be provided.
+	if len(pkScript) == 0 {
+		return nil, ErrNoScript
+	}
+
+	// A height hint must be provided to prevent scanning from the genesis
+	// block.
+	if heightHint == 0 {
+		return nil, ErrNoHeightHint
+	}
+
+	// Ensure the output script is of a supported type.
 	spendRequest, err := NewSpendRequest(outpoint, pkScript)
 	if err != nil {
 		return nil, err

chainntnfs/txnotifier_test.go

@@ -124,30 +124,81 @@ func newMockHintCache() *mockHintCache {
 	}
 }
 
-// TestTxNotifierMaxConfs ensures that we are not able to register for more
-// confirmations on a transaction than the maximum supported.
-func TestTxNotifierMaxConfs(t *testing.T) {
+// TestTxNotifierRegistrationValidation ensures that we are not able to register
+// requests with invalid parameters.
+func TestTxNotifierRegistrationValidation(t *testing.T) {
 	t.Parallel()
 
-	hintCache := newMockHintCache()
-	n := chainntnfs.NewTxNotifier(
-		10, chainntnfs.ReorgSafetyLimit, hintCache, hintCache,
-	)
-
-	// Registering one confirmation above the maximum should fail with
-	// ErrTxMaxConfs.
-	_, err := n.RegisterConf(
-		&chainntnfs.ZeroHash, testRawScript, chainntnfs.MaxNumConfs+1, 1,
-	)
-	if err != chainntnfs.ErrTxMaxConfs {
-		t.Fatalf("expected chainntnfs.ErrTxMaxConfs, got %v", err)
+	testCases := []struct {
+		name       string
+		pkScript   []byte
+		numConfs   uint32
+		heightHint uint32
+		checkSpend bool
+		err        error
+	}{
+		{
+			name:       "empty output script",
+			pkScript:   nil,
+			numConfs:   1,
+			heightHint: 1,
+			checkSpend: true,
+			err:        chainntnfs.ErrNoScript,
+		},
+		{
+			name:       "zero num confs",
+			pkScript:   testRawScript,
+			numConfs:   0,
+			heightHint: 1,
+			err:        chainntnfs.ErrNumConfsOutOfRange,
+		},
+		{
+			name:       "exceed max num confs",
+			pkScript:   testRawScript,
+			numConfs:   chainntnfs.MaxNumConfs + 1,
+			heightHint: 1,
+			err:        chainntnfs.ErrNumConfsOutOfRange,
+		},
+		{
+			name:       "empty height hint",
+			pkScript:   testRawScript,
+			numConfs:   1,
+			heightHint: 0,
+			checkSpend: true,
+			err:        chainntnfs.ErrNoHeightHint,
+		},
 	}
 
-	_, err = n.RegisterConf(
-		&chainntnfs.ZeroHash, testRawScript, chainntnfs.MaxNumConfs, 1,
-	)
-	if err != nil {
-		t.Fatalf("unable to register conf ntfn: %v", err)
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.name, func(t *testing.T) {
+			hintCache := newMockHintCache()
+			n := chainntnfs.NewTxNotifier(
+				10, chainntnfs.ReorgSafetyLimit, hintCache, hintCache,
+			)
+
+			_, err := n.RegisterConf(
+				&chainntnfs.ZeroHash, testCase.pkScript,
+				testCase.numConfs, testCase.heightHint,
+			)
+			if err != testCase.err {
+				t.Fatalf("conf registration expected error "+
+					"\"%v\", got \"%v\"", testCase.err, err)
+			}
+
+			if !testCase.checkSpend {
+				return
+			}
+
+			_, err = n.RegisterSpend(
+				&chainntnfs.ZeroOutPoint, testCase.pkScript,
+				testCase.heightHint,
+			)
+			if err != testCase.err {
+				t.Fatalf("spend registration expected error "+
+					"\"%v\", got \"%v\"", testCase.err, err)
+			}
+		})
 	}
 }