lnwallet+funding: properly propagate NewLightningChannel errors

This commit ensures that we now properly handle and propagate errors
that arise when attempting to create a new channel after the funding
transaction is believed to be confirmed.

A previous edge case would arise when a user attempted to create a new
channel, but their corresponding btcd node wasn’t yet fully synced.

fundingmanager.go

@@ -726,7 +726,13 @@ func (f *fundingManager) handleFundingSignComplete(fmsg *fundingSignCompleteMsg)
 	go func() {
 		// TODO(roasbeef): need to persist pending broadcast channels,
 		// send chan open proof during scan of blocks mined while down.
-		openChan, confHeight, confBlockIndex := resCtx.reservation.DispatchChan()
+		openChanDetails, err := resCtx.reservation.DispatchChan()
+		if err != nil {
+			fndgLog.Errorf("Unable to dispatch "+
+				"ChannelPoint(%v): %v", fundingPoint, err)
+			return
+		}
+
 		// This reservation is no longer pending as the funding
 		// transaction has been fully confirmed.
 		f.deleteReservationCtx(peerID, chanID)
@@ -739,19 +745,19 @@ func (f *fundingManager) handleFundingSignComplete(fmsg *fundingSignCompleteMsg)
 
 		// First we send the newly opened channel to the source server
 		// peer.
-		fmsg.peer.newChannels <- openChan
+		fmsg.peer.newChannels <- openChanDetails.Channel
 
 		// Afterwards we send the breach arbiter the new channel so it
 		// can watch for attempts to breach the channel's contract by
 		// the remote party.
-		f.breachAribter.newContracts <- openChan
+		f.breachAribter.newContracts <- openChanDetails.Channel
 
 		// With the block height and the transaction index known, we
 		// can construct the compact chainID which is used on the
 		// network to unique identify channels.
 		chainID := lnwire.ChannelID{
-			BlockHeight: confHeight,
-			TxIndex:     confBlockIndex,
+			BlockHeight: openChanDetails.ConfirmationHeight,
+			TxIndex:     openChanDetails.TransactionIndex,
 			TxPosition:  uint16(fundingPoint.Index),
 		}
 
@@ -767,8 +773,8 @@ func (f *fundingManager) handleFundingSignComplete(fmsg *fundingSignCompleteMsg)
 		// TODO(roasbeef): should include sigs from funding
 		// locked
 		//  * should be moved to after funding locked is recv'd
-		f.announceChannel(fmsg.peer.server, openChan, chainID, f.fakeProof,
-			f.fakeProof)
+		f.announceChannel(fmsg.peer.server, openChanDetails.Channel,
+			chainID, f.fakeProof, f.fakeProof)
 
 		// Finally give the caller a final update notifying them that
 		// the channel is now open.

lnwallet/interface_test.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -479,8 +480,12 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, wallet *lnwallet
 	// Assert that the channel opens after a single block.
 	lnChan := make(chan *lnwallet.LightningChannel, 1)
 	go func() {
-		channel, _, _ := chanReservation.DispatchChan()
-		lnChan <- channel
+		openDetails, err := chanReservation.DispatchChan()
+		if err != nil {
+			t.Fatalf("unable to finalize reservation: %v", err)
+		}
+
+		lnChan <- openDetails.Channel
 	}()
 	lnc := assertChannelOpen(t, miner, uint32(numReqConfs), lnChan)
 
@@ -757,8 +762,12 @@ func testSingleFunderReservationWorkflowInitiator(miner *rpctest.Harness,
 
 	lnChan := make(chan *lnwallet.LightningChannel, 1)
 	go func() {
-		channel, _, _ := chanReservation.DispatchChan()
-		lnChan <- channel
+		openDetails, err := chanReservation.DispatchChan()
+		if err != nil {
+			t.Fatalf("unable to open channel: %v", err)
+		}
+
+		lnChan <- openDetails.Channel
 	}()
 	assertChannelOpen(t, miner, uint32(numReqConfs), lnChan)
 }
@@ -915,7 +924,8 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
 	// Some period of time later, Bob presents us with an SPV proof
 	// attesting to an open channel. At this point Alice recognizes the
 	// channel, saves the state to disk, and creates the channel itself.
-	if _, err := chanReservation.FinalizeReservation(); err != nil {
+	_, err = chanReservation.FinalizeReservation()
+	if err != nil && !strings.Contains(err.Error(), "No information") {
 		t.Fatalf("unable to finalize reservation: %v", err)
 	}
 

lnwallet/reservation.go

@@ -130,7 +130,8 @@ type ChannelReservation struct {
 	// confirmation details will be sent on once the channel is considered
 	// 'open'. A channel is open once the funding transaction has reached a
 	// sufficient number of confirmations.
-	chanOpen chan *openChanDetails
+	chanOpen    chan *openChanDetails
+	chanOpenErr chan error
 
 	wallet *LightningWallet
 }
@@ -213,6 +214,7 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, minFeeRate btcut
 		pushSat:        pushSat,
 		reservationID:  id,
 		chanOpen:       make(chan *openChanDetails, 1),
+		chanOpenErr:    make(chan error, 1),
 		wallet:         wallet,
 	}
 }
@@ -434,6 +436,23 @@ func (r *ChannelReservation) Cancel() error {
 	return <-errChan
 }
 
+// OpenChannelDetails wraps the finalized fully confirmed channel which
+// resulted from a ChannelReservation instance with details concerning exactly
+// _where_ in the chain the channel was ultimately opened.
+type OpenChannelDetails struct {
+	// Channel is the active channel created by an instance of a
+	// ChannelReservation and the required funding workflow.
+	Channel *LightningChannel
+
+	// ConfirmationHeight is the block height within the chain that included
+	// the channel.
+	ConfirmationHeight uint32
+
+	// TransactionIndex is the index within the confirming block that the
+	// transaction resides.
+	TransactionIndex uint32
+}
+
 // DispatchChan returns a channel which will be sent on once the funding
 // transaction for this pending payment channel obtains the configured number
 // of confirmations. Once confirmations have been obtained, a fully initialized
@@ -441,12 +460,17 @@ func (r *ChannelReservation) Cancel() error {
 //
 // NOTE: If this method is called before .CompleteReservation(), it will block
 // indefinitely.
-func (r *ChannelReservation) DispatchChan() (*LightningChannel, uint32, uint32) {
-	// TODO(roasbeef): goroutine sending in wallet should be lifted up into
-	// the fundingMgr
-	openDetails := <-r.chanOpen
+func (r *ChannelReservation) DispatchChan() (*OpenChannelDetails, error) {
+	if err := <-r.chanOpenErr; err != nil {
+		return nil, err
+	}
 
-	return openDetails.channel, openDetails.blockHeight, openDetails.txIndex
+	openDetails := <-r.chanOpen
+	return &OpenChannelDetails{
+		Channel:            openDetails.channel,
+		ConfirmationHeight: openDetails.blockHeight,
+		TransactionIndex:   openDetails.txIndex,
+	}, nil
 }
 
 // FinalizeReservation completes the pending reservation, returning an active
@@ -463,5 +487,9 @@ func (r *ChannelReservation) FinalizeReservation() (*LightningChannel, error) {
 		err:              errChan,
 	}
 
-	return (<-r.chanOpen).channel, <-errChan
+	if err := <-errChan; err != nil {
+		return nil, err
+	}
+
+	return (<-r.chanOpen).channel, nil
 }

lnwallet/wallet.go

@@ -1,6 +1,7 @@
 package lnwallet
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"sync"
@@ -1229,13 +1230,18 @@ func (l *LightningWallet) handleChannelOpen(req *channelOpenMsg) {
 
 	// Finally, create and officially open the payment channel!
 	// TODO(roasbeef): CreationTime once tx is 'open'
-	channel, _ := NewLightningChannel(l.Signer, l.ChainIO, l.chainNotifier,
+	channel, err := NewLightningChannel(l.Signer, l.ChainIO, l.chainNotifier,
 		res.partialState)
+	if err != nil {
+		req.err <- err
+		res.chanOpen <- nil
+		return
+	}
 
+	req.err <- nil
 	res.chanOpen <- &openChanDetails{
 		channel: channel,
 	}
-	req.err <- nil
 }
 
 // openChannelAfterConfirmations creates, and opens a payment channel after
@@ -1265,20 +1271,28 @@ out:
 		// don't count this as the signal that the funding transaction has
 		// been confirmed.
 		if !ok {
+			res.chanOpenErr <- errors.New("wallet shutting down")
 			res.chanOpen <- nil
 			return
 		}
 
 		break out
 	case <-l.quit:
+		res.chanOpenErr <- errors.New("wallet shutting down")
 		res.chanOpen <- nil
 		return
 	}
 
 	// Finally, create and officially open the payment channel!
 	// TODO(roasbeef): CreationTime once tx is 'open'
-	channel, _ := NewLightningChannel(l.Signer, l.ChainIO, l.chainNotifier,
+	channel, err := NewLightningChannel(l.Signer, l.ChainIO, l.chainNotifier,
 		res.partialState)
+	if err != nil {
+		res.chanOpenErr <- err
+		res.chanOpen <- nil
+	}
+
+	res.chanOpenErr <- nil
 	res.chanOpen <- &openChanDetails{
 		channel:     channel,
 		blockHeight: confDetails.BlockHeight,