multi: refresh htlcswitch aliases on aliasmgr update

aliasmgr/aliasmgr.go

@@ -11,6 +11,11 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 )
 
+// UpdateLinkAliases is a function type for a function that locates the active
+// link that matches the given shortID and triggers an update based on the
+// latest values of the alias manager.
+type UpdateLinkAliases func(shortID lnwire.ShortChannelID) error
+
 var (
 	// aliasBucket stores aliases as keys and their base SCIDs as values.
 	// This is used to populate the maps that the Manager uses. The keys
@@ -82,6 +87,10 @@ var (
 type Manager struct {
 	backend kvdb.Backend
 
+	// linkAliasUpdater is a function used by the alias manager to
+	// facilitate live update of aliases in other subsystems.
+	linkAliasUpdater UpdateLinkAliases
+
 	// baseToSet is a mapping from the "base" SCID to the set of aliases
 	// for this channel. This mapping includes all channels that
 	// negotiated the option-scid-alias feature bit.
@@ -103,8 +112,14 @@ type Manager struct {
 }
 
 // NewManager initializes an alias Manager from the passed database backend.
-func NewManager(db kvdb.Backend) (*Manager, error) {
-	m := &Manager{backend: db}
+func NewManager(db kvdb.Backend, linkAliasUpdater UpdateLinkAliases) (*Manager,
+	error) {
+
+	m := &Manager{
+		backend:          db,
+		linkAliasUpdater: linkAliasUpdater,
+	}
+
 	m.baseToSet = make(map[lnwire.ShortChannelID][]lnwire.ShortChannelID)
 	m.aliasToBase = make(map[lnwire.ShortChannelID]lnwire.ShortChannelID)
 	m.peerAlias = make(map[lnwire.ChannelID]lnwire.ShortChannelID)
@@ -220,12 +235,22 @@ func (m *Manager) populateMaps() error {
 // AddLocalAlias adds a database mapping from the passed alias to the passed
 // base SCID. The gossip boolean marks whether or not to create a mapping
 // that the gossiper will use. It is set to false for the upgrade path where
-// the feature-bit is toggled on and there are existing channels.
+// the feature-bit is toggled on and there are existing channels. The linkUpdate
+// flag is used to signal whether this function should also trigger an update
+// on the htlcswitch scid alias maps.
 func (m *Manager) AddLocalAlias(alias, baseScid lnwire.ShortChannelID,
-	gossip bool) error {
+	gossip, linkUpdate bool) error {
 
+	// We need to lock the manager for the whole duration of this method,
+	// except for the very last part where we call the link updater. In
+	// order for us to safely use a defer _and_ still be able to manually
+	// unlock, we use a sync.Once.
 	m.Lock()
-	defer m.Unlock()
+	unlockOnce := sync.Once{}
+	unlock := func() {
+		unlockOnce.Do(m.Unlock)
+	}
+	defer unlock()
 
 	err := kvdb.Update(m.backend, func(tx kvdb.RwTx) error {
 		// If the caller does not want to allow the alias to be used
@@ -275,6 +300,18 @@ func (m *Manager) AddLocalAlias(alias, baseScid lnwire.ShortChannelID,
 		m.aliasToBase[alias] = baseScid
 	}
 
+	// We definitely need to unlock the Manager before calling the link
+	// updater. If we don't, we'll deadlock. We use a sync.Once to ensure
+	// that we only unlock once.
+	unlock()
+
+	// Finally, we trigger a htlcswitch update if the flag is set, in order
+	// for any future htlc that references the added alias to be properly
+	// routed.
+	if linkUpdate {
+		return m.linkAliasUpdater(baseScid)
+	}
+
 	return nil
 }
 
@@ -349,8 +386,16 @@ func (m *Manager) DeleteSixConfs(baseScid lnwire.ShortChannelID) error {
 func (m *Manager) DeleteLocalAlias(alias,
 	baseScid lnwire.ShortChannelID) error {
 
+	// We need to lock the manager for the whole duration of this method,
+	// except for the very last part where we call the link updater. In
+	// order for us to safely use a defer _and_ still be able to manually
+	// unlock, we use a sync.Once.
 	m.Lock()
-	defer m.Unlock()
+	unlockOnce := sync.Once{}
+	unlock := func() {
+		unlockOnce.Do(m.Unlock)
+	}
+	defer unlock()
 
 	err := kvdb.Update(m.backend, func(tx kvdb.RwTx) error {
 		aliasToBaseBucket, err := tx.CreateTopLevelBucket(aliasBucket)
@@ -397,7 +442,12 @@ func (m *Manager) DeleteLocalAlias(alias,
 	// cache (but this is only set if we gossip the alias).
 	delete(m.aliasToBase, alias)
 
-	return nil
+	// We definitely need to unlock the Manager before calling the link
+	// updater. If we don't, we'll deadlock. We use a sync.Once to ensure
+	// that we only unlock once.
+	unlock()
+
+	return m.linkAliasUpdater(baseScid)
 }
 
 // PutPeerAlias stores the peer's alias SCID once we learn of it in the

aliasmgr/aliasmgr_test.go

@@ -23,7 +23,11 @@ func TestAliasStorePeerAlias(t *testing.T) {
 	require.NoError(t, err)
 	defer db.Close()
 
-	aliasStore, err := NewManager(db)
+	linkUpdater := func(shortID lnwire.ShortChannelID) error {
+		return nil
+	}
+
+	aliasStore, err := NewManager(db, linkUpdater)
 	require.NoError(t, err)
 
 	var chanID1 [32]byte
@@ -52,7 +56,11 @@ func TestAliasStoreRequest(t *testing.T) {
 	require.NoError(t, err)
 	defer db.Close()
 
-	aliasStore, err := NewManager(db)
+	linkUpdater := func(shortID lnwire.ShortChannelID) error {
+		return nil
+	}
+
+	aliasStore, err := NewManager(db, linkUpdater)
 	require.NoError(t, err)
 
 	// We'll assert that the very first alias we receive is StartingAlias.
@@ -80,7 +88,14 @@ func TestAliasLifecycle(t *testing.T) {
 	require.NoError(t, err)
 	defer db.Close()
 
-	aliasStore, err := NewManager(db)
+	updateChan := make(chan struct{}, 1)
+
+	linkUpdater := func(shortID lnwire.ShortChannelID) error {
+		updateChan <- struct{}{}
+		return nil
+	}
+
+	aliasStore, err := NewManager(db, linkUpdater)
 	require.NoError(t, err)
 
 	const (
@@ -94,18 +109,24 @@ func TestAliasLifecycle(t *testing.T) {
 	aliasScid2 := lnwire.NewShortChanIDFromInt(alias + 1)
 
 	// Add the first alias.
-	err = aliasStore.AddLocalAlias(aliasScid, baseScid, false)
+	err = aliasStore.AddLocalAlias(aliasScid, baseScid, false, true)
 	require.NoError(t, err)
 
+	// The link updater should be called.
+	<-updateChan
+
 	// Query the aliases and verify the results.
 	aliasList := aliasStore.GetAliases(baseScid)
 	require.Len(t, aliasList, 1)
 	require.Contains(t, aliasList, aliasScid)
 
 	// Add the second alias.
-	err = aliasStore.AddLocalAlias(aliasScid2, baseScid, false)
+	err = aliasStore.AddLocalAlias(aliasScid2, baseScid, false, true)
 	require.NoError(t, err)
 
+	// The link updater should be called.
+	<-updateChan
+
 	// Query the aliases and verify the results.
 	aliasList = aliasStore.GetAliases(baseScid)
 	require.Len(t, aliasList, 2)
@@ -116,11 +137,21 @@ func TestAliasLifecycle(t *testing.T) {
 	err = aliasStore.DeleteLocalAlias(aliasScid, baseScid)
 	require.NoError(t, err)
 
+	// The link updater should be called.
+	<-updateChan
+
 	// We expect to get an error if we attempt to delete the same alias
 	// again.
 	err = aliasStore.DeleteLocalAlias(aliasScid, baseScid)
 	require.ErrorIs(t, err, ErrAliasNotFound)
 
+	// The link updater should _not_ be called.
+	select {
+	case <-updateChan:
+		t.Fatal("link alias updater should not have been called")
+	default:
+	}
+
 	// Query the aliases and verify that first one doesn't exist anymore.
 	aliasList = aliasStore.GetAliases(baseScid)
 	require.Len(t, aliasList, 1)
@@ -131,6 +162,9 @@ func TestAliasLifecycle(t *testing.T) {
 	err = aliasStore.DeleteLocalAlias(aliasScid2, baseScid)
 	require.NoError(t, err)
 
+	// The link updater should be called.
+	<-updateChan
+
 	// Query the aliases and verify that none exists.
 	aliasList = aliasStore.GetAliases(baseScid)
 	require.Len(t, aliasList, 0)