multi: refresh htlcswitch aliases on aliasmgr update

Author: GeorgeTsagk

aliasmgr/aliasmgr.go

@@ -10,6 +10,11 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 )
 
+// UpdateLinkAliases 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
@@ -77,6 +82,10 @@ var (
 type Manager struct {
 	backend kvdb.Backend
 
+	// LinkUpdater is a function used by aliasmgr to facilitate live update
+	// of aliases in other subsystems.
+	LinkUpdater 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.
@@ -98,11 +107,14 @@ type Manager struct {
 }
 
 // NewManager initializes an alias Manager from the passed database backend.
-func NewManager(db kvdb.Backend) (*Manager, error) {
+func NewManager(db kvdb.Backend,
+	linkUpdater UpdateLinkAliases) (*Manager, error) {
+
 	m := &Manager{backend: db}
 	m.baseToSet = make(map[lnwire.ShortChannelID][]lnwire.ShortChannelID)
 	m.aliasToBase = make(map[lnwire.ShortChannelID]lnwire.ShortChannelID)
 	m.peerAlias = make(map[lnwire.ChannelID]lnwire.ShortChannelID)
+	m.LinkUpdater = linkUpdater
 
 	err := m.populateMaps()
 	return m, err
@@ -215,12 +227,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
@@ -270,6 +292,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.LinkUpdater(baseScid)
+	}
+
 	return nil
 }
 
@@ -344,8 +378,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)
@@ -390,7 +432,12 @@ func (m *Manager) DeleteLocalAlias(alias,
 	// cache.
 	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.LinkUpdater(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.

funding/interfaces.go

@@ -36,7 +36,8 @@ type aliasHandler interface {
 	GetPeerAlias(lnwire.ChannelID) (lnwire.ShortChannelID, error)
 
 	// AddLocalAlias persists an alias to an underlying alias store.
-	AddLocalAlias(lnwire.ShortChannelID, lnwire.ShortChannelID, bool) error
+	AddLocalAlias(lnwire.ShortChannelID, lnwire.ShortChannelID, bool,
+		bool) error
 
 	// GetAliases returns the set of aliases given the main SCID of a
 	// channel. This SCID will be an alias for zero-conf channels and will

funding/manager.go

@@ -1249,7 +1249,7 @@ func (f *Manager) advancePendingChannelState(
 		// Persist the alias to the alias database.
 		baseScid := channel.ShortChannelID
 		err := f.cfg.AliasManager.AddLocalAlias(
-			baseScid, baseScid, true,
+			baseScid, baseScid, true, false,
 		)
 		if err != nil {
 			return fmt.Errorf("error adding local alias to "+
@@ -3108,7 +3108,7 @@ func (f *Manager) handleFundingConfirmation(
 		}
 
 		err = f.cfg.AliasManager.AddLocalAlias(
-			aliasScid, confChannel.shortChanID, true,
+			aliasScid, confChannel.shortChanID, true, false,
 		)
 		if err != nil {
 			return fmt.Errorf("unable to request alias: %w", err)
@@ -3274,7 +3274,7 @@ func (f *Manager) sendChannelReady(completeChan *channeldb.OpenChannel,
 
 				err = f.cfg.AliasManager.AddLocalAlias(
 					alias, completeChan.ShortChannelID,
-					false,
+					false, false,
 				)
 				if err != nil {
 					return err
@@ -3853,7 +3853,7 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, //nolint:funlen
 			}
 
 			err = f.cfg.AliasManager.AddLocalAlias(
-				alias, channel.ShortChannelID, false,
+				alias, channel.ShortChannelID, false, false,
 			)
 			if err != nil {
 				log.Errorf("unable to add local alias: %v",

funding/manager_test.go

@@ -161,7 +161,7 @@ func (m *mockAliasMgr) GetPeerAlias(lnwire.ChannelID) (lnwire.ShortChannelID,
 }
 
 func (m *mockAliasMgr) AddLocalAlias(lnwire.ShortChannelID,
-	lnwire.ShortChannelID, bool) error {
+	lnwire.ShortChannelID, bool, bool) error {
 
 	return nil
 }

htlcswitch/switch.go

@@ -2343,6 +2343,26 @@ func (s *Switch) addLiveLink(link ChannelLink) {
 	}
 	s.interfaceIndex[peerPub][link.ChanID()] = link
 
+	s.updateLinkAliases(link)
+}
+
+// UpdateLinkAliases is the externally exposed wrapper for updating link
+// aliases. It acquires the indexMtx and calls the internal method.
+func (s *Switch) UpdateLinkAliases(link ChannelLink) {
+	s.indexMtx.Lock()
+	defer s.indexMtx.Unlock()
+
+	s.updateLinkAliases(link)
+}
+
+// updateLinkAliases updates the aliases for a given link. This will cause the
+// htlcswitch to consult the alias manager on the up to date values of its
+// alias maps.
+//
+// NOTE: this MUST be called with the indexMtx held.
+func (s *Switch) updateLinkAliases(link ChannelLink) {
+	linkScid := link.ShortChanID()
+
 	aliases := link.getAliases()
 	if link.isZeroConf() {
 		if link.zeroConfConfirmed() {
@@ -2367,6 +2387,21 @@ func (s *Switch) addLiveLink(link ChannelLink) {
 			s.baseIndex[alias] = linkScid
 		}
 	} else if link.negotiatedAliasFeature() {
+		// First, we flush any alias mappings for this link's scid
+		// before we populate the map again, in order to get rid of old
+		// values that no longer exist.
+		for alias, real := range s.aliasToReal {
+			if real == linkScid {
+				delete(s.aliasToReal, alias)
+			}
+		}
+
+		for alias, real := range s.baseIndex {
+			if real == linkScid {
+				delete(s.baseIndex, alias)
+			}
+		}
+
 		// The link's SCID is the confirmed SCID for non-zero-conf
 		// option-scid-alias feature bit channels.
 		for _, alias := range aliases {
@@ -2464,11 +2499,16 @@ func (s *Switch) getLinkByMapping(pkt *htlcPacket) (ChannelLink, error) {
 	chanID := pkt.outgoingChanID
 	aliasID := s.cfg.IsAlias(chanID)
 
+	// Custom alias mappings (that aren't communicated over the wire) are
+	// allowed to be outside the range of allowed SCID aliases (so they
+	// return false for IsAlias()).
+	_, haveAliasMapping := s.aliasToReal[chanID]
+
 	// Set the originalOutgoingChanID so the proper channel_update can be
 	// sent back if the option-scid-alias feature bit was negotiated.
 	pkt.originalOutgoingChanID = chanID
 
-	if aliasID {
+	if aliasID || haveAliasMapping {
 		// Since outgoingChanID is an alias, we'll fetch the link via
 		// baseIndex.
 		baseScid, ok := s.baseIndex[chanID]
@@ -2858,14 +2898,21 @@ func (s *Switch) failMailboxUpdate(outgoingScid,
 // the associated channel is not one of these, this function will return nil
 // and the caller is expected to handle this properly. In this case, a return
 // to the original non-alias behavior is expected.
+//
+//nolint:nestif
 func (s *Switch) failAliasUpdate(scid lnwire.ShortChannelID,
 	incoming bool) *lnwire.ChannelUpdate {
 
 	// This function does not defer the unlocking because of the database
 	// lookups for ChannelUpdate.
 	s.indexMtx.RLock()
 
-	if s.cfg.IsAlias(scid) {
+	// Custom alias mappings (that aren't communicated over the wire) are
+	// allowed to be outside the range of allowed SCID aliases (so they
+	// return false for IsAlias()).
+	_, haveAliasMapping := s.aliasToReal[scid]
+
+	if s.cfg.IsAlias(scid) || haveAliasMapping {
 		// The alias SCID was used. In the incoming case this means
 		// the channel is zero-conf as the link sets the scid. In the
 		// outgoing case, the sender set the scid to use and may be

peer/brontide.go

@@ -357,7 +357,7 @@ type Config struct {
 
 	// AddLocalAlias persists an alias to an underlying alias store.
 	AddLocalAlias func(alias, base lnwire.ShortChannelID,
-		gossip bool) error
+		gossip, liveUpdate bool) error
 
 	// AuxLeafStore is an optional store that can be used to store auxiliary
 	// leaves for certain custom channel types.
@@ -838,6 +838,7 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) (
 
 				err = p.cfg.AddLocalAlias(
 					aliasScid, dbChan.ShortChanID(), false,
+					false,
 				)
 				if err != nil {
 					return nil, err

server.go

@@ -639,7 +639,18 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 	thresholdSats := btcutil.Amount(cfg.DustThreshold)
 	thresholdMSats := lnwire.NewMSatFromSatoshis(thresholdSats)
 
-	s.aliasMgr, err = aliasmgr.NewManager(dbs.ChanStateDB)
+	linkUpdater := func(shortID lnwire.ShortChannelID) error {
+		link, err := s.htlcSwitch.GetLinkByShortID(shortID)
+		if err != nil {
+			return err
+		}
+
+		s.htlcSwitch.UpdateLinkAliases(link)
+
+		return nil
+	}
+
+	s.aliasMgr, err = aliasmgr.NewManager(dbs.ChanStateDB, linkUpdater)
 	if err != nil {
 		return nil, err
 	}