Merge pull request #1627 from lightninglabs/multi-rfq-receive-fix

`AddInvoice` correctly handles existing route hints

Author: guggero

asset/asset.go

@@ -604,6 +604,85 @@ func (s *Specifier) AssertNotEmpty() error {
 	return nil
 }
 
+// Equal compares this specifier to another one, returning true if they are
+// equal. If strict is true, then both specifiers need to either have both equal
+// asset IDs and group keys set or only one of those (but matching). If strict
+// is false, then it's enough to have the same group key _or_ the same asset ID
+// (if one has no group key set). This is useful for cases where one specifier
+// only specifies the group key, while the other one specifies both the group
+// key and the asset ID. In that case, we can consider them equal if the group
+// keys match, even if the asset IDs are different (or one is not set).
+func (s *Specifier) Equal(other *Specifier, strict bool) (bool, error) {
+	// If both specifiers are nil, they are equal.
+	if s == nil && other == nil {
+		return true, nil
+	}
+
+	// If one of the specifiers is nil, they are not equal.
+	if s == nil || other == nil {
+		return false, nil
+	}
+
+	// If either of them is empty while the other is not, they are not
+	// equal.
+	if (s.HasId() || s.HasGroupPubKey()) !=
+		(other.HasId() || other.HasGroupPubKey()) {
+
+		return false, nil
+	}
+
+	// If they both have distinct elements set, then they are not equal.
+	if s.HasId() != other.HasId() &&
+		s.HasGroupPubKey() != other.HasGroupPubKey() {
+
+		return false, nil
+	}
+
+	// If both specifiers have a group public key, compare them.
+	if s.HasGroupPubKey() && other.HasGroupPubKey() {
+		groupKeyA := s.UnwrapGroupKeyToPtr()
+		groupKeyB := other.UnwrapGroupKeyToPtr()
+
+		// If any unwrapped element is nil, something's wrong, and we
+		// can't compare them.
+		if groupKeyA == nil || groupKeyB == nil {
+			return false, fmt.Errorf("unable to unwrap group key "+
+				"from specifier: %v vs %v", s, other)
+		}
+
+		if !groupKeyA.IsEqual(groupKeyB) {
+			return false, nil
+		}
+
+		// If we're not doing a strict comparison, then we can return
+		// true here if the group keys match. The group key has higher
+		// priority than the ID, so if they match and the comparison
+		// isn't strict, we can consider the specifiers equal.
+		if !strict {
+			return true, nil
+		}
+	}
+
+	// If both specifiers have an ID, compare them.
+	if s.HasId() && other.HasId() {
+		idA := s.UnwrapIdToPtr()
+		idB := other.UnwrapIdToPtr()
+
+		// If any unwrapped element is nil, something's wrong and we
+		// can't compare them.
+		if idA == nil || idB == nil {
+			return false, fmt.Errorf("unable to unwrap asset ID "+
+				"from specifier: %v vs %v", s, other)
+		}
+
+		if *idA != *idB {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
 // Type denotes the asset types supported by the Taproot Asset protocol.
 type Type uint8
 

asset/asset_test.go

@@ -1247,3 +1247,125 @@ func TestDecodeAsset(t *testing.T) {
 
 	t.Logf("Decoded asset: %v", string(assetJSON))
 }
+
+// TestSpecifierEqual tests the Specifier.Equal method for all possible cases.
+func TestSpecifierEqual(t *testing.T) {
+	id1 := ID{1, 2, 3}
+	id2 := ID{4, 5, 6}
+	pk1, _ := btcec.NewPrivateKey()
+	pubKey1 := pk1.PubKey()
+	pk2, _ := btcec.NewPrivateKey()
+	pubKey2 := pk2.PubKey()
+
+	cases := []struct {
+		name      string
+		s, other  *Specifier
+		strict    bool
+		expects   bool
+		expectErr bool
+	}{
+		{
+			name:    "both nil",
+			s:       nil,
+			other:   nil,
+			strict:  false,
+			expects: true,
+		},
+		{
+			name:    "one nil (s)",
+			s:       nil,
+			other:   &Specifier{},
+			strict:  false,
+			expects: false,
+		},
+		{
+			name:    "one nil (other)",
+			s:       &Specifier{},
+			other:   nil,
+			strict:  false,
+			expects: false,
+		},
+		{
+			name:    "both empty",
+			s:       &Specifier{},
+			other:   &Specifier{},
+			strict:  false,
+			expects: true,
+		},
+		{
+			name:    "both with same ID",
+			s:       &Specifier{id: fn.Some(id1)},
+			other:   &Specifier{id: fn.Some(id1)},
+			strict:  false,
+			expects: true,
+		},
+		{
+			name:    "both with different ID",
+			s:       &Specifier{id: fn.Some(id1)},
+			other:   &Specifier{id: fn.Some(id2)},
+			strict:  false,
+			expects: false,
+		},
+		{
+			name:    "both with same group key",
+			s:       &Specifier{groupKey: fn.Some(*pubKey1)},
+			other:   &Specifier{groupKey: fn.Some(*pubKey1)},
+			strict:  false,
+			expects: true,
+		},
+		{
+			name:    "both with different group key",
+			s:       &Specifier{groupKey: fn.Some(*pubKey1)},
+			other:   &Specifier{groupKey: fn.Some(*pubKey2)},
+			strict:  false,
+			expects: false,
+		},
+		{
+			name:    "one with ID, one with group key",
+			s:       &Specifier{id: fn.Some(id1)},
+			other:   &Specifier{groupKey: fn.Some(*pubKey1)},
+			strict:  false,
+			expects: false,
+		},
+		{
+			name:    "both with ID, strict true",
+			s:       &Specifier{id: fn.Some(id1)},
+			other:   &Specifier{id: fn.Some(id1)},
+			strict:  true,
+			expects: true,
+		},
+		{
+			name:    "both with group key, strict true",
+			s:       &Specifier{groupKey: fn.Some(*pubKey1)},
+			other:   &Specifier{groupKey: fn.Some(*pubKey1)},
+			strict:  true,
+			expects: true,
+		},
+		{
+			name:    "one with ID, one with group key, strict true",
+			s:       &Specifier{id: fn.Some(id1)},
+			other:   &Specifier{groupKey: fn.Some(*pubKey1)},
+			strict:  true,
+			expects: false,
+		},
+		{
+			name:    "both empty, strict true",
+			s:       &Specifier{},
+			other:   &Specifier{},
+			strict:  true,
+			expects: true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			eq, err := tc.s.Equal(tc.other, tc.strict)
+			if tc.expectErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.expects, eq)
+			}
+		})
+	}
+}

rpcserver.go

@@ -7228,67 +7228,48 @@ func (r *rpcServer) AddAssetBuyOffer(_ context.Context,
 	return &rfqrpc.AddAssetBuyOfferResponse{}, nil
 }
 
-// marshalPeerAcceptedBuyQuotes marshals a map of peer accepted asset buy quotes
-// into the RPC form. These are quotes that were requested by our node and have
-// been accepted by our peers.
-func marshalPeerAcceptedBuyQuotes(
-	quotes map[rfq.SerialisedScid]rfqmsg.BuyAccept) (
-	[]*rfqrpc.PeerAcceptedBuyQuote, error) {
+// marshalPeerAcceptedBuyQuote marshals a peer accepted asset buy quote into
+// the RPC form. This is a quote that was requested by our node and has been
+// accepted by our peer.
+func marshalPeerAcceptedBuyQuote(
+	quote rfqmsg.BuyAccept) *rfqrpc.PeerAcceptedBuyQuote {
 
-	// Marshal the accepted quotes into the RPC form.
-	rpcQuotes := make(
-		[]*rfqrpc.PeerAcceptedBuyQuote, 0, len(quotes),
-	)
-	for scid, quote := range quotes {
-		coefficient := quote.AssetRate.Rate.Coefficient.String()
-		rpcAskAssetRate := &rfqrpc.FixedPoint{
-			Coefficient: coefficient,
-			Scale:       uint32(quote.AssetRate.Rate.Scale),
-		}
-
-		rpcQuote := &rfqrpc.PeerAcceptedBuyQuote{
-			Peer:           quote.Peer.String(),
-			Id:             quote.ID[:],
-			Scid:           uint64(scid),
-			AssetMaxAmount: quote.Request.AssetMaxAmt,
-			AskAssetRate:   rpcAskAssetRate,
-			Expiry:         uint64(quote.AssetRate.Expiry.Unix()),
-		}
-		rpcQuotes = append(rpcQuotes, rpcQuote)
+	coefficient := quote.AssetRate.Rate.Coefficient.String()
+	rpcAskAssetRate := &rfqrpc.FixedPoint{
+		Coefficient: coefficient,
+		Scale:       uint32(quote.AssetRate.Rate.Scale),
 	}
 
-	return rpcQuotes, nil
+	return &rfqrpc.PeerAcceptedBuyQuote{
+		Peer:           quote.Peer.String(),
+		Id:             quote.ID[:],
+		Scid:           uint64(quote.ShortChannelId()),
+		AssetMaxAmount: quote.Request.AssetMaxAmt,
+		AskAssetRate:   rpcAskAssetRate,
+		Expiry:         uint64(quote.AssetRate.Expiry.Unix()),
+	}
 }
 
-// marshalPeerAcceptedSellQuotes marshals a map of peer accepted asset sell
-// quotes into the RPC form. These are quotes that were requested by our node
-// and have been accepted by our peers.
-//
-// nolint: lll
-func marshalPeerAcceptedSellQuotes(quotes map[rfq.SerialisedScid]rfqmsg.SellAccept) (
-	[]*rfqrpc.PeerAcceptedSellQuote, error) {
+// marshalPeerAcceptedSellQuote marshals peer accepted asset sell quote into the
+// RPC form. This is a quote that was requested by our node and has been
+// accepted by our peers.
+func marshalPeerAcceptedSellQuote(
+	quote rfqmsg.SellAccept) *rfqrpc.PeerAcceptedSellQuote {
 
-	// Marshal the accepted quotes into the RPC form.
-	rpcQuotes := make([]*rfqrpc.PeerAcceptedSellQuote, 0, len(quotes))
-	for scid, quote := range quotes {
-		rpcAssetRate := &rfqrpc.FixedPoint{
-			Coefficient: quote.AssetRate.Rate.Coefficient.String(),
-			Scale:       uint32(quote.AssetRate.Rate.Scale),
-		}
-
-		// TODO(ffranr): Add SellRequest payment max amount to
-		//  PeerAcceptedSellQuote.
-		rpcQuote := &rfqrpc.PeerAcceptedSellQuote{
-			Peer:         quote.Peer.String(),
-			Id:           quote.ID[:],
-			Scid:         uint64(scid),
-			BidAssetRate: rpcAssetRate,
-			Expiry:       uint64(quote.AssetRate.Expiry.Unix()),
-		}
-		rpcQuotes = append(rpcQuotes, rpcQuote)
+	rpcAssetRate := &rfqrpc.FixedPoint{
+		Coefficient: quote.AssetRate.Rate.Coefficient.String(),
+		Scale:       uint32(quote.AssetRate.Rate.Scale),
 	}
 
-	return rpcQuotes, nil
+	// TODO(ffranr): Add SellRequest payment max amount to
+	//  PeerAcceptedSellQuote.
+	return &rfqrpc.PeerAcceptedSellQuote{
+		Peer:         quote.Peer.String(),
+		Id:           quote.ID[:],
+		Scid:         uint64(quote.ShortChannelId()),
+		BidAssetRate: rpcAssetRate,
+		Expiry:       uint64(quote.AssetRate.Expiry.Unix()),
+	}
 }
 
 // QueryPeerAcceptedQuotes is used to query for quotes that were requested by
@@ -7302,19 +7283,13 @@ func (r *rpcServer) QueryPeerAcceptedQuotes(_ context.Context,
 	peerAcceptedBuyQuotes := r.cfg.RfqManager.PeerAcceptedBuyQuotes()
 	peerAcceptedSellQuotes := r.cfg.RfqManager.PeerAcceptedSellQuotes()
 
-	rpcBuyQuotes, err := marshalPeerAcceptedBuyQuotes(peerAcceptedBuyQuotes)
-	if err != nil {
-		return nil, fmt.Errorf("error marshalling peer accepted buy "+
-			"quotes: %w", err)
-	}
-
-	rpcSellQuotes, err := marshalPeerAcceptedSellQuotes(
-		peerAcceptedSellQuotes,
+	rpcBuyQuotes := fn.Map(
+		maps.Values(peerAcceptedBuyQuotes), marshalPeerAcceptedBuyQuote,
+	)
+	rpcSellQuotes := fn.Map(
+		maps.Values(peerAcceptedSellQuotes),
+		marshalPeerAcceptedSellQuote,
 	)
-	if err != nil {
-		return nil, fmt.Errorf("error marshalling peer accepted sell "+
-			"quotes: %w", err)
-	}
 
 	return &rfqrpc.QueryPeerAcceptedQuotesResponse{
 		BuyQuotes:  rpcBuyQuotes,
@@ -8188,9 +8163,19 @@ func (r *rpcServer) AddInvoice(ctx context.Context,
 	var expensiveQuote *rfqrpc.PeerAcceptedBuyQuote
 	if !existingQuotes {
 		expensiveQuote = acquiredQuotes[0].quote
+	} else {
+		mgr := r.cfg.AuxInvoiceManager
+		buyQuote, err := mgr.GetBuyQuoteFromRouteHints(
+			iReq, specifier,
+		)
+		if err != nil {
+			return nil, fmt.Errorf("failed to find matching buy "+
+				"quote in accepted quotes: %w", err)
+		}
+
+		expensiveQuote = marshalPeerAcceptedBuyQuote(*buyQuote)
 	}
 
-	// replace with above
 	// Now that we have the accepted quote, we know the amount in (milli)
 	// Satoshi that we need to pay. We can now update the invoice with this
 	// amount.

tapchannel/aux_invoice_manager.go

@@ -430,6 +430,38 @@ func IsAssetInvoice(invoice *lnrpc.Invoice, rfqLookup RfqLookup) bool {
 	return false
 }
 
+// GetBuyQuoteFromRouteHints is a helper method that extracts a buy quote from
+// the route hints of an invoice, given that the quote is accepted and has a
+// matching specifier. If no matching quote is found, it returns an error.
+func (s *AuxInvoiceManager) GetBuyQuoteFromRouteHints(invoice *lnrpc.Invoice,
+	specifier asset.Specifier) (*rfqmsg.BuyAccept, error) {
+
+	buyQuotes := s.cfg.RfqManager.PeerAcceptedBuyQuotes()
+	for _, hint := range invoice.RouteHints {
+		for _, h := range hint.HopHints {
+			scid := rfqmsg.SerialisedScid(h.ChanId)
+			buyQuote, ok := buyQuotes[scid]
+			if !ok {
+				continue
+			}
+
+			quoteSpecifier := buyQuote.Request.AssetSpecifier
+			areEqual, err := quoteSpecifier.Equal(&specifier, false)
+			if err != nil {
+				return nil, fmt.Errorf("error comparing "+
+					"specifiers: %w", err)
+			}
+
+			if areEqual {
+				return &buyQuote, nil
+			}
+		}
+	}
+
+	return nil, fmt.Errorf("no buy quote found for specifier %s",
+		specifier.String())
+}
+
 // validateAssetHTLC runs a couple of checks on the provided asset HTLC.
 func (s *AuxInvoiceManager) validateAssetHTLC(ctx context.Context,
 	htlc *rfqmsg.Htlc, circuitKey invoices.CircuitKey) error {