Merge pull request #6016 from guggero/tor-watchtower

tor+healthcheck: fix healthcheck for multiple services

docs/release-notes/release-notes-0.14.1.md

@@ -2,10 +2,18 @@
 
 ## Bug Fixes
 
-* [Fixed an inaccurate log message during a compaction failure](https://github.com/lightningnetwork/lnd/issues/5937)
-* [A bug has been fixed in channeldb that uses the return value without checking error](https://github.com/lightningnetwork/lnd/pull/6012)
+* [Fixed an inaccurate log message during a compaction
+  failure](https://github.com/lightningnetwork/lnd/pull/5961).
+
+* [Fixed a bug in the Tor controller that would cause the health check to fail
+  if there was more than one hidden service
+  configured](https://github.com/lightningnetwork/lnd/pull/6016).
+
+* [A bug has been fixed in channeldb that uses the return value without checking
+  the returned error first](https://github.com/lightningnetwork/lnd/pull/6012).
 
 # Contributors (Alphabetical Order)
 
 * Jamie Turley
-* nayuta-ueno
+* nayuta-ueno
+* Oliver Gugger

healthcheck/tor_connection.go

@@ -48,7 +48,7 @@ func CheckTorServiceStatus(tc *tor.Controller,
 		return restartTorController(tc, createService)
 
 	// If this is not a connection layer error, such as
-	// ErrServiceNotCreated or ErrServiceIDUnmatch, there's little we can
+	// ErrServiceNotCreated or ErrServiceIDMismatch, there's little we can
 	// do but to report the error to the user.
 	default:
 		return err

tor/cmd_info.go

@@ -3,6 +3,7 @@ package tor
 import (
 	"errors"
 	"fmt"
+	"strings"
 )
 
 var (
@@ -10,9 +11,9 @@ var (
 	// service while it's not been created yet.
 	ErrServiceNotCreated = errors.New("onion service hasn't been created")
 
-	// ErrServiceIDUnmatch is used when the serviceID the controller has
+	// ErrServiceIDMismatch is used when the serviceID the controller has
 	// doesn't match the serviceID the Tor daemon has.
-	ErrServiceIDUnmatch = errors.New("onion serviceIDs not match")
+	ErrServiceIDMismatch = errors.New("onion serviceIDs don't match")
 
 	// ErrNoServiceFound is used when the Tor daemon replies no active
 	// onion services found for the current control connection while we
@@ -62,10 +63,14 @@ func (c *Controller) CheckOnionService() error {
 	}
 
 	// Check that our active service is indeed the service acknowledged by
-	// Tor daemon.
-	if c.activeServiceID != serviceID {
+	// Tor daemon. The controller is only aware of a single service but the
+	// Tor daemon might have multiple services registered (for example for
+	// the watchtower as well as the node p2p connections). So we just want
+	// to check that our current controller's ID is contained in the list of
+	// registered services.
+	if !strings.Contains(serviceID, c.activeServiceID) {
 		return fmt.Errorf("%w: controller has: %v, Tor daemon has: %v",
-			ErrServiceIDUnmatch, c.activeServiceID, serviceID)
+			ErrServiceIDMismatch, c.activeServiceID, serviceID)
 	}
 
 	return nil

tor/cmd_info_test.go

@@ -61,7 +61,30 @@ func TestCheckOnionServiceFailOnServiceIDNotMatch(t *testing.T) {
 	require.NoError(t, err, "server failed to write")
 
 	// Check the error returned from GetServiceInfo is expected.
-	require.ErrorIs(t, c.CheckOnionService(), ErrServiceIDUnmatch)
+	require.ErrorIs(t, c.CheckOnionService(), ErrServiceIDMismatch)
+}
+
+func TestCheckOnionServiceSucceedOnMultipleServices(t *testing.T) {
+	t.Parallel()
+
+	// Create mock server and client connection.
+	proxy := createTestProxy(t)
+	defer proxy.cleanUp()
+	server := proxy.serverConn
+
+	// Assign a fake service ID to the controller.
+	c := &Controller{conn: proxy.clientConn, activeServiceID: "fakeID"}
+
+	// Mock a response with a different serviceID.
+	serverResp := "250-onions/current=service1,fakeID,service2\n250 OK\n"
+
+	// Let the server mocks a given response.
+	_, err := server.Write([]byte(serverResp))
+	require.NoError(t, err, "server failed to write")
+
+	// No error is expected, the controller's ID is contained within the
+	// list of active services.
+	require.NoError(t, c.CheckOnionService())
 }
 
 func TestCheckOnionServiceFailOnClosedConnection(t *testing.T) {