From ae23cd14e5c908921ee6fdf296aca121c4403a91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kucha=C5=99?= Date: Thu, 2 Jul 2026 07:44:37 +0000 Subject: [PATCH 1/2] wtclient: prune closable sessions when the tower is unreachable When all of a watchtower session's channels are closed, the session becomes "closable" and the client attempts to tell the tower it may delete the session before removing its own copy. Previously, if the tower could not be reached, deleteSessionFromTower returned an error and the session was never deleted from the client's own database. On every restart the client re-loaded these sessions and re-dialed the dead tower, which could keep lnd busy for a very long time on startup, and the tower could never be fully removed because a tower that still has sessions is only ever marked inactive. Notifying the tower is only a courtesy so that it can reclaim disk space: a closable session's channels are all closed, so the local copy no longer protects anything. We therefore now delete the local session copy on a best-effort basis even when the tower cannot be reached. In addition, if the tower has been deactivated by the user, we skip contacting it entirely and prune the session locally. Fixes #10646. --- watchtower/wtclient/client.go | 25 +++-- watchtower/wtclient/client_test.go | 156 +++++++++++++++++++++++++++++ watchtower/wtclient/errors.go | 5 + watchtower/wtclient/manager.go | 32 ++++-- 4 files changed, 203 insertions(+), 15 deletions(-) diff --git a/watchtower/wtclient/client.go b/watchtower/wtclient/client.go index f8f3d3e618b..cad6b197b27 100644 --- a/watchtower/wtclient/client.go +++ b/watchtower/wtclient/client.go @@ -561,16 +561,25 @@ func (c *client) stopAndRemoveSession(id wtdb.SessionID, final bool) error { // deleteSessionFromTower dials the tower that we created the session with and // attempts to send the tower the DeleteSession message. func (c *client) deleteSessionFromTower(sess *wtdb.ClientSession) error { - // First, we check if we have already loaded this tower in our - // candidate towers iterator. + // Load the tower from the DB so that we can check its status. If the + // tower has been deactivated by the user, then there is no point in + // attempting to reach it to notify it of the session deletion; we + // signal this to the caller so that our local copy of the (closable) + // session can still be pruned. We check the DB tower directly since + // the in-memory Tower does not carry the tower's status. + dbTower, err := c.cfg.DB.LoadTowerByID(sess.TowerID) + if err != nil { + return err + } + if dbTower.Status == wtdb.TowerStatusInactive { + return errTowerInactive + } + + // Use the tower from our candidate iterator if we have already loaded + // it, so that we share its address iterator. Otherwise, construct one + // from the DB tower. tower, err := c.candidateTowers.GetTower(sess.TowerID) if errors.Is(err, ErrTowerNotInIterator) { - // If not, then we attempt to load it from the DB. - dbTower, err := c.cfg.DB.LoadTowerByID(sess.TowerID) - if err != nil { - return err - } - tower, err = NewTowerFromDBTower(dbTower) if err != nil { return err diff --git a/watchtower/wtclient/client_test.go b/watchtower/wtclient/client_test.go index b44d3e2df91..fe30ba69069 100644 --- a/watchtower/wtclient/client_test.go +++ b/watchtower/wtclient/client_test.go @@ -2093,6 +2093,162 @@ var clientTests = []clientTest{ require.NoError(h.t, err) }, }, + { + // Assert that a closable session is still deleted from the + // client's own database even when the tower it was negotiated + // with can no longer be reached. Informing the tower that it + // may delete the session is best-effort: a session is only + // closable once all of its channels are closed, so the local + // copy no longer protects anything. A permanently unreachable + // tower must therefore not prevent local cleanup, otherwise the + // session lingers forever and the client re-dials the dead + // tower on every restart (see issue #10646). + name: "assert that closable sessions are deleted even if " + + "the tower is unreachable", + cfg: harnessCfg{ + localBalance: localBalance, + remoteBalance: remoteBalance, + policy: wtpolicy.Policy{ + TxPolicy: defaultTxPolicy, + MaxUpdates: 5, + }, + }, + fn: func(h *testHarness) { + const numUpdates = 5 + + h.sendUpdatesOn = true + + // Saturate a session with updates for channel 0 and + // ensure the tower receives them all. + hints := h.advanceChannelN(0, numUpdates) + h.backupStates(0, 0, numUpdates, nil) + h.server.waitForUpdates(hints, waitTime) + + // Exactly one session should hold updates for the + // channel. + sessionIDs := h.relevantSessions(0) + require.Len(h.t, sessionIDs, 1) + + // Close the channel so that the now-exhausted session + // becomes closable. + h.closeChannel(0, 1) + + err := wait.Predicate(func() bool { + return h.isSessionClosable(sessionIDs[0]) + }, waitTime) + require.NoError(h.t, err) + + // Simulate the tower disappearing from the network by + // removing its dial callback from the mock net. Any + // dial to it now fails, just like a tower whose host is + // permanently unreachable. + h.net.removeConnCallback(h.server.addr) + + // Mine enough blocks to exceed the session-close-range + // so that the client tries to hand the session off to + // the tower and then delete it. + h.mine(3) + + // Even though the tower can no longer be reached, the + // client must still remove the closable session from + // its own database. + err = wait.Predicate(func() bool { + cs, err := h.clientDB.ListClosableSessions() + require.NoError(h.t, err) + + sessions, err := h.clientDB.ListClientSessions( + nil, + ) + require.NoError(h.t, err) + + _, ok := sessions[sessionIDs[0]] + + return len(cs) == 0 && !ok + }, waitTime) + require.NoError(h.t, err) + }, + }, + { + // Assert that when a closable session belongs to a tower that + // the user has deactivated, the client prunes the session from + // its own database without attempting to contact the tower. We + // verify that no contact is made by asserting that the (still + // reachable) tower server retains the session in its own DB + // (see issue #10646). + name: "assert that closable sessions of a deactivated tower " + + "are deleted without contacting the tower", + cfg: harnessCfg{ + localBalance: localBalance, + remoteBalance: remoteBalance, + policy: wtpolicy.Policy{ + TxPolicy: defaultTxPolicy, + MaxUpdates: 5, + }, + }, + fn: func(h *testHarness) { + const numUpdates = 5 + + h.sendUpdatesOn = true + + // Saturate a session with updates for channel 0 and + // ensure the tower receives them all. + hints := h.advanceChannelN(0, numUpdates) + h.backupStates(0, 0, numUpdates, nil) + h.server.waitForUpdates(hints, waitTime) + + sessionIDs := h.relevantSessions(0) + require.Len(h.t, sessionIDs, 1) + + // The tower server should be aware of the session. + _, err := h.server.db.GetSessionInfo(&sessionIDs[0]) + require.NoError(h.t, err) + + // Close the channel so that the now-exhausted session + // becomes closable. + h.closeChannel(0, 1) + + err = wait.Predicate(func() bool { + return h.isSessionClosable(sessionIDs[0]) + }, waitTime) + require.NoError(h.t, err) + + // Deactivate the tower. The tower server is still + // up and reachable; only the client has marked it + // inactive. + err = h.clientMgr.DeactivateTower( + h.server.addr.IdentityKey, + ) + require.NoError(h.t, err) + + // Mine enough blocks to exceed the session-close-range + // so that the client processes the closable session. + h.mine(3) + + // The client should delete the closable session + // from its own database. + err = wait.Predicate(func() bool { + cs, err := h.clientDB.ListClosableSessions() + require.NoError(h.t, err) + + sessions, err := h.clientDB.ListClientSessions( + nil, + ) + require.NoError(h.t, err) + + _, ok := sessions[sessionIDs[0]] + + return len(cs) == 0 && !ok + }, waitTime) + require.NoError(h.t, err) + + // Crucially, since the tower was deactivated, the + // client should not have contacted it, so the tower + // server must still have the session in its own + // database. + _, err = h.server.db.GetSessionInfo(&sessionIDs[0]) + require.NoError(h.t, err) + }, + }, { // Demonstrate that the client is able to recover after // deleting its database by skipping through key indices until diff --git a/watchtower/wtclient/errors.go b/watchtower/wtclient/errors.go index 2cb070ac45a..6bc87c92721 100644 --- a/watchtower/wtclient/errors.go +++ b/watchtower/wtclient/errors.go @@ -39,4 +39,9 @@ var ( // create a new session with a tower with a session key that has already // been used in the past. ErrSessionKeyAlreadyUsed = errors.New("session key already used") + + // errTowerInactive signals that a session's tower has been deactivated + // by the user, so no attempt should be made to contact it when cleaning + // up a closable session. + errTowerInactive = errors.New("tower is inactive") ) diff --git a/watchtower/wtclient/manager.go b/watchtower/wtclient/manager.go index 03a344a15d8..78b5ecc7f53 100644 --- a/watchtower/wtclient/manager.go +++ b/watchtower/wtclient/manager.go @@ -937,19 +937,37 @@ func (m *Manager) handleClosableSessions( continue } + // Tell the tower it can delete the + // session. This is best-effort: the + // session is closable, meaning all of + // its channels are closed, so our local + // copy no longer protects anything and + // contacting the tower only lets it free + // disk space. We therefore always delete + // our local copy below, even if the tower + // is unreachable or has been deactivated, + // so that a dead or removed tower cannot + // make closable sessions accumulate and + // be re-dialed on every startup (see + // issue #10646). err = client.deleteSessionFromTower(sess) - if err != nil { - log.Errorf("error deleting "+ - "session %s from tower: %v", - sess.ID, err) - - continue + switch { + case errors.Is(err, errTowerInactive): + log.Debugf("Tower for session %s "+ + "is inactive, deleting "+ + "local session copy", sess.ID) + + case err != nil: + log.Warnf("Could not delete "+ + "session %s from tower, "+ + "deleting local copy "+ + "anyway: %v", sess.ID, err) } err = m.cfg.DB.DeleteSession(item.sessionID) if err != nil { log.Errorf("could not delete "+ - "session(%s) from DB: %w", + "session(%s) from DB: %v", sess.ID, err) continue From c3b159ac6861c3802af372b170490c71f1fd7d99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kucha=C5=99?= Date: Thu, 2 Jul 2026 07:44:37 +0000 Subject: [PATCH 2/2] docs: add release note for watchtower dead-tower cleanup fix Add a release note for the fix that prunes closable watchtower sessions locally when the tower is permanently unreachable or has been deactivated, and add the author to the contributor list (#10646). --- docs/release-notes/release-notes-0.22.0.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 260c099f1f2..9030a3fdfc0 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -42,6 +42,17 @@ regardless of peer connectivity. Uptime is now seeded from the peer's actual connection state. +* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/10947) where a + watchtower that became permanently unreachable (for example a tower that + went offline or was deactivated with `lncli wtclient deactivate`) could make + `lnd` spend a very long time cleaning up on startup. Closable watchtower + sessions (those whose channels are all closed) are now pruned from the client + database on a best-effort basis even when the tower cannot be reached, and + the client no longer contacts a deactivated tower when cleaning them up. + Previously such sessions could never be deleted, so the client re-dialed the + dead tower on every restart and the tower could never be fully removed (see + [issue 10646](https://github.com/lightningnetwork/lnd/issues/10646)). + # New Features ## Functional Enhancements @@ -134,3 +145,4 @@ * bitromortac * Boris Nagaev * Erick Cestari +* Jan Kuchaƙ