diff --git a/lnrpc/wtclientrpc/wtclient.go b/lnrpc/wtclientrpc/wtclient.go index ad99f0541..4275ec25a 100644 --- a/lnrpc/wtclientrpc/wtclient.go +++ b/lnrpc/wtclientrpc/wtclient.go @@ -243,12 +243,7 @@ func (c *WatchtowerClient) RemoveTower(ctx context.Context, } } - // TODO(conner): make atomic via multiplexed client - err = c.cfg.Client.RemoveTower(pubKey, addr) - if err != nil { - return nil, err - } - err = c.cfg.AnchorClient.RemoveTower(pubKey, addr) + err = c.cfg.ClientMgr.RemoveTower(pubKey, addr) if err != nil { return nil, err } diff --git a/watchtower/wtclient/client.go b/watchtower/wtclient/client.go index e1749374a..6cea916ee 100644 --- a/watchtower/wtclient/client.go +++ b/watchtower/wtclient/client.go @@ -95,13 +95,6 @@ type RegisteredTower struct { // Client is the primary interface used by the daemon to control a client's // lifecycle and backup revoked states. type Client interface { - // RemoveTower removes a watchtower from being considered for future - // session negotiations and from being used for any subsequent backups - // until it's added again. If an address is provided, then this call - // only serves as a way of removing the address from the watchtower - // instead. - RemoveTower(*btcec.PublicKey, net.Addr) error - // RegisteredTowers retrieves the list of watchtowers registered with // the client. RegisteredTowers(...wtdb.ClientSessionListOption) ([]*RegisteredTower, @@ -153,6 +146,9 @@ type newTowerMsg struct { // staleTowerMsg is an internal message we'll use within the TowerClient to // signal that a tower should no longer be considered. type staleTowerMsg struct { + // id is the unique database identifier for the tower. + id wtdb.TowerID + // pubKey is the identifying public key of the watchtower. pubKey *btcec.PublicKey @@ -1492,17 +1488,18 @@ func (c *TowerClient) handleNewTower(tower *Tower) error { return nil } -// RemoveTower removes a watchtower from being considered for future session +// removeTower removes a watchtower from being considered for future session // negotiations and from being used for any subsequent backups until it's added // again. If an address is provided, then this call only serves as a way of // removing the address from the watchtower instead. -func (c *TowerClient) RemoveTower(pubKey *btcec.PublicKey, +func (c *TowerClient) removeTower(id wtdb.TowerID, pubKey *btcec.PublicKey, addr net.Addr) error { errChan := make(chan error, 1) select { case c.staleTowers <- &staleTowerMsg{ + id: id, pubKey: pubKey, addr: addr, errChan: errChan, @@ -1524,9 +1521,8 @@ func (c *TowerClient) RemoveTower(pubKey *btcec.PublicKey, // inactive and removed as candidates. If the active session queue corresponds // to any of these sessions, a new one will be negotiated. func (c *TowerClient) handleStaleTower(msg *staleTowerMsg) error { - // We'll load the tower before potentially removing it in order to - // retrieve its ID within the database. - dbTower, err := c.cfg.DB.LoadTower(msg.pubKey) + // We'll first update our in-memory state. + err := c.candidateTowers.RemoveCandidate(msg.id, msg.addr) if err != nil { return err } @@ -1534,19 +1530,14 @@ func (c *TowerClient) handleStaleTower(msg *staleTowerMsg) error { // If an address was provided, then we're only meant to remove the // address from the tower. if msg.addr != nil { - return c.removeTowerAddr(dbTower, msg.addr) + return nil } // Otherwise, the tower should no longer be used for future session - // negotiations and backups. First, we'll update our in-memory state - // with the stale tower. - err = c.candidateTowers.RemoveCandidate(dbTower.ID, nil) - if err != nil { - return err - } + // negotiations and backups. pubKey := msg.pubKey.SerializeCompressed() - sessions, err := c.cfg.DB.ListClientSessions(&dbTower.ID) + sessions, err := c.cfg.DB.ListClientSessions(&msg.id) if err != nil { return fmt.Errorf("unable to retrieve sessions for tower %x: "+ "%v", pubKey, err) @@ -1573,40 +1564,6 @@ func (c *TowerClient) handleStaleTower(msg *staleTowerMsg) error { } } - // Finally, we will update our persisted state with the stale tower. - return c.cfg.DB.RemoveTower(msg.pubKey, nil) -} - -// removeTowerAddr removes the given address from the tower. -func (c *TowerClient) removeTowerAddr(tower *wtdb.Tower, addr net.Addr) error { - if addr == nil { - return fmt.Errorf("an address must be provided") - } - - // We'll first update our in-memory state followed by our persisted - // state with the stale tower. The removal of the tower address from - // the in-memory state will fail if the address is currently being used - // for a session negotiation. - err := c.candidateTowers.RemoveCandidate(tower.ID, addr) - if err != nil { - return err - } - - err = c.cfg.DB.RemoveTower(tower.IdentityKey, addr) - if err != nil { - // If the persisted state update fails, re-add the address to - // our in-memory state. - tower, newTowerErr := NewTowerFromDBTower(tower) - if newTowerErr != nil { - log.Errorf("could not create new in-memory tower: %v", - newTowerErr) - } else { - c.candidateTowers.AddCandidate(tower) - } - - return err - } - return nil } diff --git a/watchtower/wtclient/client_test.go b/watchtower/wtclient/client_test.go index 6be1bf496..f1deb697a 100644 --- a/watchtower/wtclient/client_test.go +++ b/watchtower/wtclient/client_test.go @@ -766,7 +766,7 @@ func (h *testHarness) addTower(addr *lnwire.NetAddress) { func (h *testHarness) removeTower(pubKey *btcec.PublicKey, addr net.Addr) { h.t.Helper() - err := h.client.RemoveTower(pubKey, addr) + err := h.clientMgr.RemoveTower(pubKey, addr) require.NoError(h.t, err) } @@ -1718,7 +1718,7 @@ var clientTests = []clientTest{ require.NoError(h.t, err) // Remove the old tower address from the client. - err = h.client.RemoveTower( + err = h.clientMgr.RemoveTower( towerAddr.IdentityKey, oldAddr, ) require.NoError(h.t, err) @@ -1752,7 +1752,7 @@ var clientTests = []clientTest{ // negotiation with the server will be in progress, so // the client should be able to remove the server. err := wait.NoError(func() error { - return h.client.RemoveTower( + return h.clientMgr.RemoveTower( h.server.addr.IdentityKey, nil, ) }, waitTime) @@ -1807,7 +1807,7 @@ var clientTests = []clientTest{ // address currently being locked for session // negotiation. err = wait.Predicate(func() bool { - err = h.client.RemoveTower( + err = h.clientMgr.RemoveTower( h.server.addr.IdentityKey, h.server.addr.Address, ) @@ -1818,7 +1818,7 @@ var clientTests = []clientTest{ // Assert that the second address can be removed since // it is not being used for session negotiation. err = wait.NoError(func() error { - return h.client.RemoveTower( + return h.clientMgr.RemoveTower( h.server.addr.IdentityKey, towerTCPAddr, ) }, waitTime) @@ -1830,7 +1830,7 @@ var clientTests = []clientTest{ // Assert that the client can now remove the first // address. err = wait.NoError(func() error { - return h.client.RemoveTower( + return h.clientMgr.RemoveTower( h.server.addr.IdentityKey, nil, ) }, waitTime) @@ -2223,7 +2223,7 @@ var clientTests = []clientTest{ // Now we can remove the old one. err := wait.Predicate(func() bool { - err := h.client.RemoveTower( + err := h.clientMgr.RemoveTower( h.server.addr.IdentityKey, nil, ) @@ -2309,7 +2309,7 @@ var clientTests = []clientTest{ require.NoError(h.t, err) // Now remove the tower. - err = h.client.RemoveTower( + err = h.clientMgr.RemoveTower( h.server.addr.IdentityKey, nil, ) require.NoError(h.t, err) @@ -2400,7 +2400,7 @@ var clientTests = []clientTest{ h.startClient() // Now remove the tower. - err = h.client.RemoveTower( + err = h.clientMgr.RemoveTower( h.server.addr.IdentityKey, nil, ) require.NoError(h.t, err) diff --git a/watchtower/wtclient/manager.go b/watchtower/wtclient/manager.go index d583d6c52..b28268f62 100644 --- a/watchtower/wtclient/manager.go +++ b/watchtower/wtclient/manager.go @@ -2,9 +2,11 @@ package wtclient import ( "fmt" + "net" "sync" "time" + "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" @@ -24,6 +26,13 @@ type TowerClientManager interface { // any new addresses included will be considered when dialing it for // session negotiations and backups. AddTower(*lnwire.NetAddress) error + + // RemoveTower removes a watchtower from being considered for future + // session negotiations and from being used for any subsequent backups + // until it's added again. If an address is provided, then this call + // only serves as a way of removing the address from the watchtower + // instead. + RemoveTower(*btcec.PublicKey, net.Addr) error } // Config provides the TowerClient with access to the resources it requires to @@ -243,3 +252,50 @@ func (m *Manager) AddTower(address *lnwire.NetAddress) error { return nil } + +// RemoveTower removes a watchtower from being considered for future session +// negotiations and from being used for any subsequent backups until it's added +// again. If an address is provided, then this call only serves as a way of +// removing the address from the watchtower instead. +func (m *Manager) RemoveTower(key *btcec.PublicKey, addr net.Addr) error { + // We'll load the tower before potentially removing it in order to + // retrieve its ID within the database. + dbTower, err := m.cfg.DB.LoadTower(key) + if err != nil { + return err + } + + m.clientsMu.Lock() + defer m.clientsMu.Unlock() + + for _, client := range m.clients { + err := client.removeTower(dbTower.ID, key, addr) + if err != nil { + return err + } + } + + if err := m.cfg.DB.RemoveTower(key, addr); err != nil { + // If the persisted state update fails, re-add the address to + // our client's in-memory state. + tower, newTowerErr := NewTowerFromDBTower(dbTower) + if newTowerErr != nil { + log.Errorf("could not create new in-memory tower: %v", + newTowerErr) + + return err + } + + for _, client := range m.clients { + addTowerErr := client.addTower(tower) + if addTowerErr != nil { + log.Errorf("could not re-add tower: %v", + addTowerErr) + } + } + + return err + } + + return nil +}