diff --git a/chanbackup/recover.go b/chanbackup/recover.go index 7f5502c4a..033bd695f 100644 --- a/chanbackup/recover.go +++ b/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) diff --git a/chanbackup/recover_test.go b/chanbackup/recover_test.go index e3e607737..c8719cb3f 100644 --- a/chanbackup/recover_test.go +++ b/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 } diff --git a/rpcserver.go b/rpcserver.go index ddd31224e..f10c07fdb 100644 --- a/rpcserver.go +++ b/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, ) diff --git a/server.go b/server.go index 0fc95dc75..73456735a 100644 --- a/server.go +++ b/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, )