chanbackup+rpcserver+server: return number of recovered channels

chanbackup/recover.go

@@ -40,9 +40,11 @@ type PeerConnector interface {
 // the channel. In addition a LinkNode will be created for each new peer as
 // well, in order to expose the addressing information required to locate to
 // and connect to each peer in order to initiate the recovery protocol.
+// The number of channels that were successfully restored is returned.
 func Recover(backups []Single, restorer ChannelRestorer,
-	peerConnector PeerConnector) error {
+	peerConnector PeerConnector) (int, error) {
 
+	var numRestored int
 	for i, backup := range backups {
 		log.Infof("Restoring ChannelPoint(%v) to disk: ",
 			backup.FundingOutpoint)
@@ -57,9 +59,10 @@ func Recover(backups []Single, restorer ChannelRestorer,
 			continue
 		}
 		if err != nil {
-			return err
+			return numRestored, err
 		}
 
+		numRestored++
 		log.Infof("Attempting to connect to node=%x (addrs=%v) to "+
 			"restore ChannelPoint(%v)",
 			backup.RemoteNodePub.SerializeCompressed(),
@@ -70,7 +73,7 @@ func Recover(backups []Single, restorer ChannelRestorer,
 			backup.RemoteNodePub, backup.Addresses,
 		)
 		if err != nil {
-			return err
+			return numRestored, err
 		}
 
 		// TODO(roasbeef): to handle case where node has changed addrs,
@@ -80,7 +83,7 @@ func Recover(backups []Single, restorer ChannelRestorer,
 		//  * just to to fresh w/ call to node addrs and de-dup?
 	}
 
-	return nil
+	return numRestored, nil
 }
 
 // TODO(roasbeef): more specific keychain interface?
@@ -88,16 +91,17 @@ func Recover(backups []Single, restorer ChannelRestorer,
 // UnpackAndRecoverSingles is a one-shot method, that given a set of packed
 // single channel backups, will restore the channel state to a channel shell,
 // and also reach out to connect to any of the known node addresses for that
-// channel. It is assumes that after this method exists, if a connection we
-// able to be established, then then PeerConnector will continue to attempt to
-// re-establish a persistent connection in the background.
+// channel. It is assumes that after this method exists, if a connection was
+// established, then the PeerConnector will continue to attempt to re-establish
+// a persistent connection in the background. The number of channels that were
+// successfully restored is returned.
 func UnpackAndRecoverSingles(singles PackedSingles,
 	keyChain keychain.KeyRing, restorer ChannelRestorer,
-	peerConnector PeerConnector) error {
+	peerConnector PeerConnector) (int, error) {
 
 	chanBackups, err := singles.Unpack(keyChain)
 	if err != nil {
-		return err
+		return 0, err
 	}
 
 	return Recover(chanBackups, restorer, peerConnector)
@@ -106,16 +110,17 @@ func UnpackAndRecoverSingles(singles PackedSingles,
 // UnpackAndRecoverMulti is a one-shot method, that given a set of packed
 // multi-channel backups, will restore the channel states to channel shells,
 // and also reach out to connect to any of the known node addresses for that
-// channel. It is assumes that after this method exists, if a connection we
-// able to be established, then then PeerConnector will continue to attempt to
-// re-establish a persistent connection in the background.
+// channel. It is assumes that after this method exists, if a connection was
+// established, then the PeerConnector will continue to attempt to re-establish
+// a persistent connection in the background. The number of channels that were
+// successfully restored is returned.
 func UnpackAndRecoverMulti(packedMulti PackedMulti,
 	keyChain keychain.KeyRing, restorer ChannelRestorer,
-	peerConnector PeerConnector) error {
+	peerConnector PeerConnector) (int, error) {
 
 	chanBackups, err := packedMulti.Unpack(keyChain)
 	if err != nil {
-		return err
+		return 0, err
 	}
 
 	return Recover(chanBackups.StaticBackups, restorer, peerConnector)

chanbackup/recover_test.go

@@ -2,7 +2,7 @@ package chanbackup
 
 import (
 	"bytes"
-	"fmt"
+	"errors"
 	"net"
 	"testing"
 
@@ -11,6 +11,12 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var (
+	errRestoreFail = errors.New("restore fail")
+
+	errConnectFail = errors.New("connect fail")
+)
+
 type mockChannelRestorer struct {
 	fail bool
 
@@ -19,7 +25,7 @@ type mockChannelRestorer struct {
 
 func (m *mockChannelRestorer) RestoreChansFromSingles(...Single) error {
 	if m.fail {
-		return fmt.Errorf("fail")
+		return errRestoreFail
 	}
 
 	m.callCount++
@@ -33,11 +39,11 @@ type mockPeerConnector struct {
 	callCount int
 }
 
-func (m *mockPeerConnector) ConnectPeer(node *btcec.PublicKey,
-	addrs []net.Addr) error {
+func (m *mockPeerConnector) ConnectPeer(_ *btcec.PublicKey,
+	_ []net.Addr) error {
 
 	if m.fail {
-		return fmt.Errorf("fail")
+		return errConnectFail
 	}
 
 	m.callCount++
@@ -59,16 +65,13 @@ func TestUnpackAndRecoverSingles(t *testing.T) {
 	var packedBackups PackedSingles
 	for i := 0; i < numSingles; i++ {
 		channel, err := genRandomOpenChannelShell()
-		if err != nil {
-			t.Fatalf("unable make channel: %v", err)
-		}
+		require.NoError(t, err)
 
 		single := NewSingle(channel, nil)
 
 		var b bytes.Buffer
-		if err := single.PackToWriter(&b, keyRing); err != nil {
-			t.Fatalf("unable to pack single: %v", err)
-		}
+		err = single.PackToWriter(&b, keyRing)
+		require.NoError(t, err)
 
 		backups = append(backups, single)
 		packedBackups = append(packedBackups, b.Bytes())
@@ -83,54 +86,47 @@ func TestUnpackAndRecoverSingles(t *testing.T) {
 	// If we make the channel restore fail, then the entire method should
 	// as well
 	chanRestorer.fail = true
-	err := UnpackAndRecoverSingles(
+	_, err := UnpackAndRecoverSingles(
 		packedBackups, keyRing, &chanRestorer, &peerConnector,
 	)
-	if err == nil {
-		t.Fatalf("restoration should have failed")
-	}
+	require.ErrorIs(t, err, errRestoreFail)
 
 	chanRestorer.fail = false
 
 	// If we make the peer connector fail, then the entire method should as
 	// well
 	peerConnector.fail = true
-	err = UnpackAndRecoverSingles(
+	_, err = UnpackAndRecoverSingles(
 		packedBackups, keyRing, &chanRestorer, &peerConnector,
 	)
-	if err == nil {
-		t.Fatalf("restoration should have failed")
-	}
+	require.ErrorIs(t, err, errConnectFail)
 
 	chanRestorer.callCount--
 	peerConnector.fail = false
 
 	// Next, we'll ensure that if all the interfaces function as expected,
 	// then the channels will properly be unpacked and restored.
-	err = UnpackAndRecoverSingles(
+	numRestored, err := UnpackAndRecoverSingles(
 		packedBackups, keyRing, &chanRestorer, &peerConnector,
 	)
-	require.NoError(t, err, "unable to recover chans")
+	require.NoError(t, err)
+	require.EqualValues(t, numSingles, numRestored)
 
 	// Both the restorer, and connector should have been called 10 times,
 	// once for each backup.
-	if chanRestorer.callCount != numSingles {
-		t.Fatalf("expected %v calls, instead got %v",
-			numSingles, chanRestorer.callCount)
-	}
-	if peerConnector.callCount != numSingles {
-		t.Fatalf("expected %v calls, instead got %v",
-			numSingles, peerConnector.callCount)
-	}
+	require.EqualValues(
+		t, numSingles, chanRestorer.callCount, "restorer call count",
+	)
+	require.EqualValues(
+		t, numSingles, peerConnector.callCount, "peer call count",
+	)
 
 	// If we modify the keyRing, then unpacking should fail.
 	keyRing.Fail = true
-	err = UnpackAndRecoverSingles(
+	_, err = UnpackAndRecoverSingles(
 		packedBackups, keyRing, &chanRestorer, &peerConnector,
 	)
-	if err == nil {
-		t.Fatalf("unpacking should have failed")
-	}
+	require.ErrorContains(t, err, "fail")
 
 	// TODO(roasbeef): verify proper call args
 }
@@ -148,9 +144,7 @@ func TestUnpackAndRecoverMulti(t *testing.T) {
 	backups := make([]Single, 0, numSingles)
 	for i := 0; i < numSingles; i++ {
 		channel, err := genRandomOpenChannelShell()
-		if err != nil {
-			t.Fatalf("unable make channel: %v", err)
-		}
+		require.NoError(t, err)
 
 		single := NewSingle(channel, nil)
 
@@ -162,9 +156,8 @@ func TestUnpackAndRecoverMulti(t *testing.T) {
 	}
 
 	var b bytes.Buffer
-	if err := multi.PackToWriter(&b, keyRing); err != nil {
-		t.Fatalf("unable to pack multi: %v", err)
-	}
+	err := multi.PackToWriter(&b, keyRing)
+	require.NoError(t, err)
 
 	// Next, we'll pack the set of singles into a packed multi, and also
 	// create the set of interfaces we need to carry out the remainder of
@@ -177,54 +170,47 @@ func TestUnpackAndRecoverMulti(t *testing.T) {
 	// If we make the channel restore fail, then the entire method should
 	// as well
 	chanRestorer.fail = true
-	err := UnpackAndRecoverMulti(
+	_, err = UnpackAndRecoverMulti(
 		packedMulti, keyRing, &chanRestorer, &peerConnector,
 	)
-	if err == nil {
-		t.Fatalf("restoration should have failed")
-	}
+	require.ErrorIs(t, err, errRestoreFail)
 
 	chanRestorer.fail = false
 
 	// If we make the peer connector fail, then the entire method should as
 	// well
 	peerConnector.fail = true
-	err = UnpackAndRecoverMulti(
+	_, err = UnpackAndRecoverMulti(
 		packedMulti, keyRing, &chanRestorer, &peerConnector,
 	)
-	if err == nil {
-		t.Fatalf("restoration should have failed")
-	}
+	require.ErrorIs(t, err, errConnectFail)
 
 	chanRestorer.callCount--
 	peerConnector.fail = false
 
 	// Next, we'll ensure that if all the interfaces function as expected,
 	// then the channels will properly be unpacked and restored.
-	err = UnpackAndRecoverMulti(
+	numRestored, err := UnpackAndRecoverMulti(
 		packedMulti, keyRing, &chanRestorer, &peerConnector,
 	)
-	require.NoError(t, err, "unable to recover chans")
+	require.NoError(t, err)
+	require.EqualValues(t, numSingles, numRestored)
 
 	// Both the restorer, and connector should have been called 10 times,
 	// once for each backup.
-	if chanRestorer.callCount != numSingles {
-		t.Fatalf("expected %v calls, instead got %v",
-			numSingles, chanRestorer.callCount)
-	}
-	if peerConnector.callCount != numSingles {
-		t.Fatalf("expected %v calls, instead got %v",
-			numSingles, peerConnector.callCount)
-	}
+	require.EqualValues(
+		t, numSingles, chanRestorer.callCount, "restorer call count",
+	)
+	require.EqualValues(
+		t, numSingles, peerConnector.callCount, "peer call count",
+	)
 
 	// If we modify the keyRing, then unpacking should fail.
 	keyRing.Fail = true
-	err = UnpackAndRecoverMulti(
+	_, err = UnpackAndRecoverMulti(
 		packedMulti, keyRing, &chanRestorer, &peerConnector,
 	)
-	if err == nil {
-		t.Fatalf("unpacking should have failed")
-	}
+	require.ErrorContains(t, err, "fail")
 
 	// TODO(roasbeef): verify proper call args
 }

rpcserver.go

@@ -8159,7 +8159,7 @@ func (r *rpcServer) RestoreChannelBackups(ctx context.Context,
 		// write the new backups to disk, and then attempt to connect
 		// out to any peers that we know of which were our prior
 		// channel peers.
-		err := chanbackup.UnpackAndRecoverSingles(
+		_, err := chanbackup.UnpackAndRecoverSingles(
 			chanbackup.PackedSingles(packedBackups),
 			r.server.cc.KeyRing, chanRestorer, r.server,
 		)
@@ -8176,7 +8176,7 @@ func (r *rpcServer) RestoreChannelBackups(ctx context.Context,
 		// out to any peers that we know of which were our prior
 		// channel peers.
 		packedMulti := chanbackup.PackedMulti(packedMultiBackup)
-		err := chanbackup.UnpackAndRecoverMulti(
+		_, err := chanbackup.UnpackAndRecoverMulti(
 			packedMulti, r.server.cc.KeyRing, chanRestorer,
 			r.server,
 		)

server.go

@@ -2248,7 +2248,7 @@ func (s *server) Start() error {
 			chainArb:   s.chainArb,
 		}
 		if len(s.chansToRestore.PackedSingleChanBackups) != 0 {
-			err := chanbackup.UnpackAndRecoverSingles(
+			_, err := chanbackup.UnpackAndRecoverSingles(
 				s.chansToRestore.PackedSingleChanBackups,
 				s.cc.KeyRing, chanRestorer, s,
 			)
@@ -2259,7 +2259,7 @@ func (s *server) Start() error {
 			}
 		}
 		if len(s.chansToRestore.PackedMultiChanBackup) != 0 {
-			err := chanbackup.UnpackAndRecoverMulti(
+			_, err := chanbackup.UnpackAndRecoverMulti(
 				s.chansToRestore.PackedMultiChanBackup,
 				s.cc.KeyRing, chanRestorer, s,
 			)