Merge pull request #1175 from lightninglabs/buyorder-refactor

Refactor RFQ BuyOrder struct

Author: guggero

itest/rfq_test.go

@@ -94,9 +94,8 @@ func testRfqAssetBuyHtlcIntercept(t *harnessTest) {
 					AssetId: mintedAssetId,
 				},
 			},
-			MinAssetAmount: purchaseAssetAmt,
-			MaxBid:         bidAmt,
-			Expiry:         buyOrderExpiry,
+			AssetMaxAmt: purchaseAssetAmt,
+			Expiry:      buyOrderExpiry,
 
 			// Here we explicitly specify Bob as the destination
 			// peer for the buy order. This will prompt Carol's tapd

rfq/manager.go

@@ -688,28 +688,42 @@ func (m *Manager) UpsertAssetBuyOffer(offer BuyOffer) error {
 	return nil
 }
 
-// BuyOrder is a struct that represents a buy order.
+// BuyOrder instructs the RFQ (Request For Quote) system to request a quote from
+// a peer for the acquisition of an asset.
+//
+// The normal use of a buy order is as follows:
+//  1. Alice, operating a wallet node, wants to receive a Tap asset as payment
+//     by issuing a Lightning invoice.
+//  2. Alice has an asset channel established with Bob's edge node.
+//  3. Before issuing the invoice, Alice needs to agree on an exchange rate with
+//     Bob, who will facilitate the asset transfer.
+//  4. To obtain the best exchange rate, Alice creates a buy order specifying
+//     the desired asset.
+//  5. Alice's RFQ subsystem processes the buy order and sends buy requests to
+//     relevant peers to find the best rate. In this example, Bob is the only
+//     available peer.
+//  6. Once Bob provides a satisfactory quote, Alice accepts it.
+//  7. Alice issues the Lightning invoice, which Charlie will pay.
+//  8. Instead of paying Alice directly, Charlie pays Bob.
+//  9. Bob then forwards the agreed amount of the Tap asset to Alice over their
+//     asset channel.
 type BuyOrder struct {
-	// AssetID is the ID of the asset that the buyer is interested in.
-	AssetID *asset.ID
-
-	// AssetGroupKey is the public key of the asset group that the buyer is
-	// interested in.
-	AssetGroupKey *btcec.PublicKey
+	// AssetSpecifier is the asset that the buyer is interested in.
+	AssetSpecifier asset.Specifier
 
-	// MinAssetAmount is the minimum amount of the asset that the buyer is
-	// willing to accept.
-	MinAssetAmount uint64
-
-	// MaxBid is the maximum bid price that the buyer is willing to pay.
-	MaxBid lnwire.MilliSatoshi
+	// AssetMaxAmt is the maximum amount of the asset that the provider must
+	// be willing to offer.
+	AssetMaxAmt uint64
 
 	// Expiry is the unix timestamp at which the buy order expires.
 	Expiry uint64
 
 	// Peer is the peer that the buy order is intended for. This field is
 	// optional.
-	Peer *route.Vertex
+	//
+	// TODO(ffranr): Currently, this field must be specified. In the future,
+	//  the negotiator should be able to determine the optimal peer.
+	Peer fn.Option[route.Vertex]
 }
 
 // UpsertAssetBuyOrder upserts an asset buy order for management.
@@ -718,7 +732,7 @@ func (m *Manager) UpsertAssetBuyOrder(order BuyOrder) error {
 	//
 	// TODO(ffranr): Add support for peerless buy orders. The negotiator
 	//  should be able to determine the optimal peer.
-	if order.Peer == nil {
+	if order.Peer.IsNone() {
 		return fmt.Errorf("buy order peer must be specified")
 	}
 

rfq/negotiator.go

@@ -159,31 +159,27 @@ func (n *Negotiator) HandleOutgoingBuyOrder(buyOrder BuyOrder) error {
 	go func() {
 		defer n.Wg.Done()
 
+		// Unwrap the peer from the buy order. For now, we can assume
+		// that the peer is always specified.
+		peer, err := buyOrder.Peer.UnwrapOrErr(
+			fmt.Errorf("buy order peer must be specified"),
+		)
+		if err != nil {
+			n.cfg.ErrChan <- err
+		}
+
 		// We calculate a proposed bid price for our peer's
 		// consideration. If a price oracle is not specified we will
 		// skip this step.
 		var assetRateHint fn.Option[rfqmsg.AssetRate]
 
-		// Construct an asset specifier from the order.
-		// TODO(ffranr): The order should have an asset specifier field
-		//  rather than an asset ID and group key.
-		assetSpecifier, err := asset.NewSpecifier(
-			buyOrder.AssetID, buyOrder.AssetGroupKey, nil,
-			true,
-		)
-		if err != nil {
-			log.Warnf("failed to construct asset "+
-				"specifier from buy order: %v", err)
-		}
+		if n.cfg.PriceOracle != nil &&
+			buyOrder.AssetSpecifier.IsSome() {
 
-		if n.cfg.PriceOracle != nil && assetSpecifier.IsSome() {
 			// Query the price oracle for a bid price.
-			//
-			// TODO(ffranr): Add assetMaxAmt to BuyOrder and use as
-			//  arg here.
 			assetRate, err := n.queryBidFromPriceOracle(
-				*buyOrder.Peer, assetSpecifier,
-				fn.None[uint64](),
+				peer, buyOrder.AssetSpecifier,
+				fn.Some(buyOrder.AssetMaxAmt),
 				fn.None[lnwire.MilliSatoshi](),
 				fn.None[rfqmsg.AssetRate](),
 			)
@@ -199,10 +195,10 @@ func (n *Negotiator) HandleOutgoingBuyOrder(buyOrder BuyOrder) error {
 			assetRateHint = fn.Some[rfqmsg.AssetRate](*assetRate)
 		}
 
+		// Construct a new buy request to send to the peer.
 		request, err := rfqmsg.NewBuyRequest(
-			*buyOrder.Peer, buyOrder.AssetID,
-			buyOrder.AssetGroupKey, buyOrder.MinAssetAmount,
-			assetRateHint,
+			peer, buyOrder.AssetSpecifier,
+			buyOrder.AssetMaxAmt, assetRateHint,
 		)
 		if err != nil {
 			err := fmt.Errorf("unable to create buy request "+

rfqmsg/buy_request.go

@@ -45,24 +45,16 @@ type BuyRequest struct {
 }
 
 // NewBuyRequest creates a new asset buy quote request.
-func NewBuyRequest(peer route.Vertex, assetID *asset.ID,
-	assetGroupKey *btcec.PublicKey, assetMaxAmt uint64,
-	assetRateHint fn.Option[AssetRate]) (*BuyRequest, error) {
+func NewBuyRequest(peer route.Vertex, assetSpecifier asset.Specifier,
+	assetMaxAmt uint64, assetRateHint fn.Option[AssetRate]) (*BuyRequest,
+	error) {
 
 	id, err := NewID()
 	if err != nil {
 		return nil, fmt.Errorf("unable to generate random "+
 			"quote request id: %w", err)
 	}
 
-	assetSpecifier, err := asset.NewSpecifier(
-		assetID, assetGroupKey, nil, true,
-	)
-	if err != nil {
-		return nil, fmt.Errorf("unable to create asset specifier: %w",
-			err)
-	}
-
 	return &BuyRequest{
 		Peer:           peer,
 		Version:        latestBuyRequestVersion,

rpcserver.go

@@ -6302,13 +6302,20 @@ func unmarshalAssetBuyOrder(
 		peer = &pv
 	}
 
+	// Construct an asset specifier from the asset ID and/or group key.
+	assetSpecifier, err := asset.NewSpecifier(
+		assetId, assetGroupKey, nil, true,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("error creating asset specifier: %w",
+			err)
+	}
+
 	return &rfq.BuyOrder{
-		AssetID:        assetId,
-		AssetGroupKey:  assetGroupKey,
-		MinAssetAmount: req.MinAssetAmount,
-		MaxBid:         lnwire.MilliSatoshi(req.MaxBid),
+		AssetSpecifier: assetSpecifier,
+		AssetMaxAmt:    req.AssetMaxAmt,
 		Expiry:         req.Expiry,
-		Peer:           peer,
+		Peer:           fn.MaybeSome(peer),
 	}, nil
 }
 
@@ -6328,12 +6335,13 @@ func (r *rpcServer) AddAssetBuyOrder(_ context.Context,
 		return nil, fmt.Errorf("error unmarshalling buy order: %w", err)
 	}
 
-	var peer string
-	if buyOrder.Peer != nil {
-		peer = buyOrder.Peer.String()
-	}
+	peerStr := fn.MapOptionZ(
+		buyOrder.Peer, func(peerVertex route.Vertex) string {
+			return peerVertex.String()
+		},
+	)
 	rpcsLog.Debugf("[AddAssetBuyOrder]: upserting buy order "+
-		"(dest_peer=%s)", peer)
+		"(dest_peer=%s)", peerStr)
 
 	// Register an event listener before actually inserting the order, so we
 	// definitely don't miss any responses.
@@ -6375,7 +6383,7 @@ func (r *rpcServer) AddAssetBuyOrder(_ context.Context,
 
 		case <-timeout:
 			return nil, fmt.Errorf("timeout waiting for response "+
-				"from peer %x", buyOrder.Peer[:])
+				"(peer=%s)", peerStr)
 		}
 	}
 }
@@ -7148,9 +7156,9 @@ func (r *rpcServer) AddInvoice(ctx context.Context,
 				AssetId: assetID[:],
 			},
 		},
-		MinAssetAmount: req.AssetAmount,
-		Expiry:         uint64(expiryTimestamp.Unix()),
-		PeerPubKey:     peerPubKey[:],
+		AssetMaxAmt: req.AssetAmount,
+		Expiry:      uint64(expiryTimestamp.Unix()),
+		PeerPubKey:  peerPubKey[:],
 		TimeoutSeconds: uint32(
 			rfq.DefaultTimeout.Seconds(),
 		),