From 496e29d02e819f91a79d1acab8e9139e858c4a0b Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Mon, 15 Jun 2020 17:41:44 -0700
Subject: [PATCH] chanbackup: refuse to start the SubSwapper if we can't read
 the SCB file

In this commit, we add an additional defense against starting up with an
invalid SCB state. With this commit, we'll now fail to start up if we're
unable to update or read the existing SCB state from disk. This will
prevent an lnd node from starting up with an invalid SCB file, an SCB
file it can't decrypt, or an SCB left over from another node.
---
 chanbackup/pubsub.go      | 28 ++++++++++++++++++----------
 chanbackup/pubsub_test.go |  2 +-
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/chanbackup/pubsub.go b/chanbackup/pubsub.go
index 87d61299..4eef45e1 100644
--- a/chanbackup/pubsub.go
+++ b/chanbackup/pubsub.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"net"
+	"os"
 	"sync"
 
 	"github.com/btcsuite/btcd/wire"
@@ -134,13 +135,24 @@ func NewSubSwapper(startingChans []Single, chanNotifier ChannelNotifier,
 
 // Start starts the chanbackup.SubSwapper.
 func (s *SubSwapper) Start() error {
+	var startErr error
 	s.started.Do(func() {
 		log.Infof("Starting chanbackup.SubSwapper")
 
+		// Before we enter our main loop, we'll update the on-disk
+		// state with the latest Single state, as nodes may have new
+		// advertised addresses.
+		if err := s.updateBackupFile(); err != nil {
+			startErr = fmt.Errorf("unable to refresh backup "+
+				"file: %v", err)
+			return
+		}
+
 		s.wg.Add(1)
 		go s.backupUpdater()
 	})
-	return nil
+
+	return startErr
 }
 
 // Stop signals the SubSwapper to being a graceful shutdown.
@@ -162,7 +174,11 @@ func (s *SubSwapper) updateBackupFile(closedChans ...wire.OutPoint) error {
 	// already have on-disk, to make sure we can decode it (proper seed)
 	// and that we're able to combine it with our new data.
 	diskMulti, err := s.Swapper.ExtractMulti(s.keyRing)
-	if err != nil {
+
+	// If the file doesn't exist on disk, then that's OK as it was never
+	// created. In this case we'll continue onwards as it isn't a critical
+	// error.
+	if err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("unable to extract on disk encrypted "+
 			"SCB: %v", err)
 	}
@@ -235,14 +251,6 @@ func (s *SubSwapper) backupUpdater() {
 
 	log.Debugf("SubSwapper's backupUpdater is active!")
 
-	// Before we enter our main loop, we'll update the on-disk state with
-	// the latest Single state, as nodes may have new advertised addresses.
-	//
-	// TODO(roasbeef): do in Start() so we don't start with an invalid SCB?
-	if err := s.updateBackupFile(); err != nil {
-		log.Errorf("Unable to refresh backup file: %v", err)
-	}
-
 	for {
 		select {
 		// The channel state has been modified! We'll evaluate all
diff --git a/chanbackup/pubsub_test.go b/chanbackup/pubsub_test.go
index 8f4b73c6..aca52df2 100644
--- a/chanbackup/pubsub_test.go
+++ b/chanbackup/pubsub_test.go
@@ -21,7 +21,7 @@ type mockSwapper struct {
 
 func newMockSwapper(keychain keychain.KeyRing) *mockSwapper {
 	return &mockSwapper{
-		swaps:     make(chan PackedMulti),
+		swaps:     make(chan PackedMulti, 1),
 		keyChain:  keychain,
 		swapState: &Multi{},
 	}