watchtower: prevent removal of last tower addr

This addresses a potential panic when a tower has one of its candidate
sessions chosen, but its only reachable address was removed by a
user-initiated RPC before the fact.

watchtower/wtclient/candidate_iterator.go

@@ -19,7 +19,7 @@ type TowerCandidateIterator interface {
 	// iterator. An optional address can be provided to indicate a stale
 	// tower address to remove it. If it isn't provided, then the tower is
 	// completely removed from the iterator.
-	RemoveCandidate(wtdb.TowerID, net.Addr)
+	RemoveCandidate(wtdb.TowerID, net.Addr) error
 
 	// IsActive determines whether a given tower is exists within the
 	// iterator.
@@ -131,19 +131,26 @@ func (t *towerListIterator) AddCandidate(candidate *wtdb.Tower) {
 // optional address can be provided to indicate a stale tower address to remove
 // it. If it isn't provided, then the tower is completely removed from the
 // iterator.
-func (t *towerListIterator) RemoveCandidate(candidate wtdb.TowerID, addr net.Addr) {
+func (t *towerListIterator) RemoveCandidate(candidate wtdb.TowerID,
+	addr net.Addr) error {
+
 	t.mu.Lock()
 	defer t.mu.Unlock()
 
 	tower, ok := t.candidates[candidate]
 	if !ok {
-		return
+		return nil
 	}
 	if addr != nil {
 		tower.RemoveAddress(addr)
+		if len(tower.Addresses) == 0 {
+			return wtdb.ErrLastTowerAddr
+		}
 	} else {
 		delete(t.candidates, candidate)
 	}
+
+	return nil
 }
 
 // IsActive determines whether a given tower is exists within the iterator.

watchtower/wtclient/client.go

@@ -1124,7 +1124,10 @@ func (c *TowerClient) handleStaleTower(msg *staleTowerMsg) error {
 	if err := c.cfg.DB.RemoveTower(msg.pubKey, msg.addr); err != nil {
 		return err
 	}
-	c.candidateTowers.RemoveCandidate(tower.ID, msg.addr)
+	err = c.candidateTowers.RemoveCandidate(tower.ID, msg.addr)
+	if err != nil {
+		return err
+	}
 
 	// If an address was provided, then we're only meant to remove the
 	// address from the tower, so there's nothing left for us to do.

watchtower/wtdb/client_db.go

@@ -108,6 +108,10 @@ var (
 	// created because session key index differs from the reserved key
 	// index.
 	ErrIncorrectKeyIndex = errors.New("incorrect key index")
+
+	// ErrLastTowerAddr is an error returned when the last address of a
+	// watchtower is attempted to be removed.
+	ErrLastTowerAddr = errors.New("cannot remove last tower address")
 )
 
 // ClientDB is single database providing a persistent storage engine for the
@@ -333,7 +337,13 @@ func (c *ClientDB) RemoveTower(pubKey *btcec.PublicKey, addr net.Addr) error {
 			if err != nil {
 				return err
 			}
+
+			// Towers should always have at least one address saved.
 			tower.RemoveAddress(addr)
+			if len(tower.Addresses) == 0 {
+				return ErrLastTowerAddr
+			}
+
 			return putTower(towers, tower)
 		}
 

watchtower/wtdb/client_db_test.go

@@ -445,7 +445,7 @@ func testRemoveTower(h *clientDBHarness) {
 
 	// We'll then remove the first address. We should now see that the tower
 	// has no addresses left.
-	h.removeTower(pk, addr1, false, nil)
+	h.removeTower(pk, addr1, false, wtdb.ErrLastTowerAddr)
 
 	// Removing the tower as a whole from the database should succeed since
 	// there aren't any active sessions for it.

watchtower/wtmock/client_db.go

@@ -101,6 +101,9 @@ func (m *ClientDB) RemoveTower(pubKey *btcec.PublicKey, addr net.Addr) error {
 
 	if addr != nil {
 		tower.RemoveAddress(addr)
+		if len(tower.Addresses) == 0 {
+			return wtdb.ErrLastTowerAddr
+		}
 		m.towers[tower.ID] = tower
 		return nil
 	}