Adding re-try RFQ negotiation on SCID collsion

docs/release-notes/release-notes-0.8.0.md

@@ -102,6 +102,10 @@
   fixes inverted sort direction in `AssetRoots`, `AssetLeafKeys`, and
   `QueryEvents` universe RPCs.
 
+* [PR#2040](https://github.com/lightninglabs/taproot-assets/pull/2040)
+  handles SCID alias collisions during incoming buy-accept processing by
+  retrying RFQ negotiation with a fresh RFQ ID instead of failing the flow.
+
 # New Features
 
 ## Functional Enhancements

rfq/manager.go

@@ -456,6 +456,41 @@ func (m *Manager) handleIncomingMessage(ctx context.Context,
 				return
 			}
 
+			// Since we're going to buy assets from our peer, we
+			// need to make sure we can identify the incoming asset
+			// payment by the SCID alias through which it comes in
+			// and compare it to the one in the invoice.
+			err := m.addScidAlias(
+				uint64(msg.ShortChannelId()),
+				msg.Request.AssetSpecifier, msg.Peer,
+			)
+			if err != nil {
+				if isAliasCollisionErr(err) {
+					m.Goroutine(func() error {
+						return m.retryBuyNegotiation(
+							ctx, msg,
+						)
+					}, func(retryErr error) {
+						m.handleError(
+							fmt.Errorf("error "+
+								"retrying buy "+
+								"negotiation "+
+								"after alias "+
+								"collision: %w",
+								retryErr,
+							),
+						)
+					})
+					return
+				}
+
+				m.handleError(
+					fmt.Errorf("error adding local alias: "+
+						"%w", err),
+				)
+				return
+			}
+
 			// The quote request has been accepted. Persist to
 			// DB first so that on restart the quote is not
 			// silently lost.
@@ -465,6 +500,15 @@ func (m *Manager) handleIncomingMessage(ctx context.Context,
 				ctx, msg,
 			)
 			if storeErr != nil {
+				rollbackErr := m.deleteScidAlias(uint64(scid))
+				if rollbackErr != nil {
+					m.handleError(fmt.Errorf("failed to "+
+						"rollback alias after buy "+
+						"quote persistence error for "+
+						"SCID %d: %w", scid,
+						rollbackErr))
+				}
+
 				m.handleError(fmt.Errorf("failed to "+
 					"persist peer buy quote for "+
 					"SCID %d: %w", scid, storeErr))
@@ -475,23 +519,6 @@ func (m *Manager) handleIncomingMessage(ctx context.Context,
 			// DB write succeeded; populate the in-memory
 			// cache.
 			m.orderHandler.peerBuyQuotes.Store(scid, msg)
-
-			// Since we're going to buy assets from our peer, we
-			// need to make sure we can identify the incoming asset
-			// payment by the SCID alias through which it comes in
-			// and compare it to the one in the invoice.
-			err := m.addScidAlias(
-				uint64(msg.ShortChannelId()),
-				msg.Request.AssetSpecifier, msg.Peer,
-			)
-			if err != nil {
-				m.handleError(
-					fmt.Errorf("error adding local alias: "+
-						"%w", err),
-				)
-				return
-			}
-
 			// Notify subscribers of the incoming peer accepted
 			// asset buy quote.
 			event := NewPeerAcceptedBuyQuoteEvent(&msg)
@@ -577,28 +604,40 @@ func (m *Manager) handleOutgoingMessage(ctx context.Context,
 	// Perform type specific handling of the outgoing message.
 	switch msg := outgoingMsg.(type) {
 	case *rfqmsg.BuyAccept:
+		err := m.addScidAlias(
+			uint64(msg.ShortChannelId()),
+			msg.Request.AssetSpecifier, msg.Peer,
+		)
+		if err != nil {
+			if isAliasCollisionErr(err) {
+				return fn.NewCriticalError(fmt.Errorf("error "+
+					"adding local alias for outgoing "+
+					"buy accept collision: %w", err))
+			}
+
+			return fmt.Errorf("error adding local alias: %w", err)
+		}
+
 		// A peer sent us an asset buy quote request in an attempt to
 		// buy an asset from us. Having accepted the request, but before
 		// we inform our peer of our decision, we inform the order
 		// handler that we are willing to sell the asset subject to a
 		// sale policy.
-		err := m.orderHandler.RegisterAssetSalePolicy(ctx, *msg)
+		err = m.orderHandler.RegisterAssetSalePolicy(ctx, *msg)
 		if err != nil {
+			rollbackErr := m.deleteScidAlias(
+				uint64(msg.ShortChannelId()),
+			)
+			if rollbackErr != nil {
+				return fmt.Errorf("registering asset sale "+
+					"policy failed and alias rollback "+
+					"failed: %w", rollbackErr)
+			}
+
 			return fmt.Errorf("registering asset sale "+
 				"policy: %w", err)
 		}
 
-		// Since our peer is going to buy assets from us, we need to
-		// make sure we can identify the forwarded asset payment by the
-		// outgoing SCID alias within the onion packet.
-		err = m.addScidAlias(
-			uint64(msg.ShortChannelId()),
-			msg.Request.AssetSpecifier, msg.Peer,
-		)
-		if err != nil {
-			return fmt.Errorf("error adding local alias: %w", err)
-		}
-
 	case *rfqmsg.SellAccept:
 		// A peer sent us an asset sell quote request in an attempt to
 		// sell an asset to us. Having accepted the request, but before
@@ -672,6 +711,13 @@ func (m *Manager) addScidAlias(scidAlias uint64, assetSpecifier asset.Specifier,
 		lnwire.NewShortChanIDFromInt(baseSCID),
 	)
 	if err != nil {
+		if isAliasCollisionErr(err) {
+			return fmt.Errorf(
+				"add alias: scid alias already exists: %w",
+				err,
+			)
+		}
+
 		// Not being able to call lnd to add the alias is a critical
 		// error, which warrants shutting down, as something is wrong.
 		return fn.NewCriticalError(
@@ -683,6 +729,71 @@ func (m *Manager) addScidAlias(scidAlias uint64, assetSpecifier asset.Specifier,
 	return nil
 }
 
+// deleteScidAlias removes a SCID alias from lnd's alias manager.
+func (m *Manager) deleteScidAlias(scidAlias uint64) error {
+	alias := lnwire.NewShortChanIDFromInt(scidAlias)
+	baseScid, err := m.cfg.AliasManager.FetchBaseAlias(
+		context.Background(), alias,
+	)
+	if err != nil {
+		return fmt.Errorf("fetching base SCID for alias %d: %w",
+			scidAlias, err)
+	}
+
+	err = m.cfg.AliasManager.DeleteLocalAlias(
+		context.Background(), alias, baseScid,
+	)
+	if err != nil {
+		return fmt.Errorf("deleting alias %d: %w", scidAlias, err)
+	}
+
+	return nil
+}
+
+// isAliasCollisionErr returns true if the error indicates an alias collision.
+func isAliasCollisionErr(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	lowerErr := strings.ToLower(err.Error())
+	return strings.Contains(lowerErr, "alias already exists")
+}
+
+// retryBuyNegotiation requests a new quote with a fresh RFQ ID while preserving
+// the original order parameters.
+func (m *Manager) retryBuyNegotiation(ctx context.Context,
+	msg rfqmsg.BuyAccept) error {
+
+	req := msg.Request
+	order := BuyOrder{
+		AssetSpecifier:  req.AssetSpecifier,
+		AssetMaxAmt:     req.AssetMaxAmt,
+		AssetMinAmt:     req.AssetMinAmt,
+		AssetRateLimit:  req.AssetRateLimit,
+		ExecutionPolicy: req.ExecutionPolicy,
+		AssetRateHint:   req.AssetRateHint,
+		Expiry: time.Now().Add(
+			rfqmsg.DefaultQuoteLifetime,
+		),
+		Peer:                fn.Some(msg.Peer),
+		PriceOracleMetadata: req.PriceOracleMetadata,
+	}
+
+	newID, err := m.negotiator.HandleOutgoingBuyOrder(ctx, order)
+	if err != nil {
+		return fmt.Errorf(
+			"unable to retry buy RFQ negotiation: %w", err,
+		)
+	}
+
+	log.Warnf("Retrying buy RFQ negotiation due to SCID alias collision "+
+		"(old_rfq_id=%x, new_rfq_id=%x, peer=%x)", msg.ID[:], newID[:],
+		msg.Peer[:])
+
+	return nil
+}
+
 // mainEventLoop is the main event loop of the RFQ manager.
 func (m *Manager) mainEventLoop(ctx context.Context) {
 	for {