Skip to content

Commit 39d1889

Browse files
authored
Merge pull request #1192 from lightninglabs/rfq-order-validation
[rfq]: add validation to `AddAssetBuyOrder` and `AddAssetSellOrder` RPCs
2 parents 965edcd + e0d9ea1 commit 39d1889

File tree

5 files changed

+394
-243
lines changed

5 files changed

+394
-243
lines changed

itest/rfq_test.go

+50-30
Original file line numberDiff line numberDiff line change
@@ -87,25 +87,35 @@ func testRfqAssetBuyHtlcIntercept(t *harnessTest) {
8787
bidAmt := uint64(90000)
8888
buyOrderExpiry := uint64(time.Now().Add(24 * time.Hour).Unix())
8989

90-
_, err = ts.CarolTapd.AddAssetBuyOrder(
91-
ctxt, &rfqrpc.AddAssetBuyOrderRequest{
92-
AssetSpecifier: &rfqrpc.AssetSpecifier{
93-
Id: &rfqrpc.AssetSpecifier_AssetId{
94-
AssetId: mintedAssetId,
95-
},
90+
// We first try to add a buy order without specifying the asset skip
91+
// flag. That should result in an error, since we only have a normal
92+
// channel and not an asset channel.
93+
buyReq := &rfqrpc.AddAssetBuyOrderRequest{
94+
AssetSpecifier: &rfqrpc.AssetSpecifier{
95+
Id: &rfqrpc.AssetSpecifier_AssetId{
96+
AssetId: mintedAssetId,
9697
},
97-
AssetMaxAmt: purchaseAssetAmt,
98-
Expiry: buyOrderExpiry,
98+
},
99+
AssetMaxAmt: purchaseAssetAmt,
100+
Expiry: buyOrderExpiry,
99101

100-
// Here we explicitly specify Bob as the destination
101-
// peer for the buy order. This will prompt Carol's tapd
102-
// node to send a request for quote message to Bob's
103-
// node.
104-
PeerPubKey: ts.BobLnd.PubKey[:],
102+
// Here we explicitly specify Bob as the destination
103+
// peer for the buy order. This will prompt Carol's tapd
104+
// node to send a request for quote message to Bob's
105+
// node.
106+
PeerPubKey: ts.BobLnd.PubKey[:],
105107

106-
TimeoutSeconds: uint32(rfqTimeout.Seconds()),
107-
},
108+
TimeoutSeconds: uint32(rfqTimeout.Seconds()),
109+
}
110+
_, err = ts.AliceTapd.AddAssetBuyOrder(ctxt, buyReq)
111+
require.ErrorContains(
112+
t.t, err, "error checking peer channel: error checking asset "+
113+
"channel",
108114
)
115+
116+
// Now we set the skip flag and we shouldn't get an error anymore.
117+
buyReq.SkipAssetChannelCheck = true
118+
_, err = ts.CarolTapd.AddAssetBuyOrder(ctxt, buyReq)
109119
require.NoError(t.t, err, "unable to upsert asset buy order")
110120

111121
// Wait until Carol receives an incoming quote accept message (sent from
@@ -266,25 +276,35 @@ func testRfqAssetSellHtlcIntercept(t *harnessTest) {
266276
askAmt := uint64(42000)
267277
sellOrderExpiry := uint64(time.Now().Add(24 * time.Hour).Unix())
268278

269-
_, err = ts.AliceTapd.AddAssetSellOrder(
270-
ctxt, &rfqrpc.AddAssetSellOrderRequest{
271-
AssetSpecifier: &rfqrpc.AssetSpecifier{
272-
Id: &rfqrpc.AssetSpecifier_AssetId{
273-
AssetId: mintedAssetIdBytes,
274-
},
279+
// We first try to add a sell order without specifying the asset skip
280+
// flag. That should result in an error, since we only have a normal
281+
// channel and not an asset channel.
282+
sellReq := &rfqrpc.AddAssetSellOrderRequest{
283+
AssetSpecifier: &rfqrpc.AssetSpecifier{
284+
Id: &rfqrpc.AssetSpecifier_AssetId{
285+
AssetId: mintedAssetIdBytes,
275286
},
276-
PaymentMaxAmt: askAmt,
277-
Expiry: sellOrderExpiry,
287+
},
288+
PaymentMaxAmt: askAmt,
289+
Expiry: sellOrderExpiry,
278290

279-
// Here we explicitly specify Bob as the destination
280-
// peer for the sell order. This will prompt Alice's
281-
// tapd node to send a request for quote message to
282-
// Bob's node.
283-
PeerPubKey: ts.BobLnd.PubKey[:],
291+
// Here we explicitly specify Bob as the destination
292+
// peer for the sell order. This will prompt Alice's
293+
// tapd node to send a request for quote message to
294+
// Bob's node.
295+
PeerPubKey: ts.BobLnd.PubKey[:],
284296

285-
TimeoutSeconds: uint32(rfqTimeout.Seconds()),
286-
},
297+
TimeoutSeconds: uint32(rfqTimeout.Seconds()),
298+
}
299+
_, err = ts.AliceTapd.AddAssetSellOrder(ctxt, sellReq)
300+
require.ErrorContains(
301+
t.t, err, "error checking peer channel: error checking asset "+
302+
"channel",
287303
)
304+
305+
// Now we set the skip flag and we shouldn't get an error anymore.
306+
sellReq.SkipAssetChannelCheck = true
307+
_, err = ts.AliceTapd.AddAssetSellOrder(ctxt, sellReq)
288308
require.NoError(t.t, err, "unable to upsert asset sell order")
289309

290310
// Wait until Alice receives an incoming sell quote accept message (sent

rpcserver.go

+86-14
Original file line numberDiff line numberDiff line change
@@ -6340,7 +6340,7 @@ func unmarshalAssetBuyOrder(
63406340

63416341
// AddAssetBuyOrder upserts a new buy order for the given asset into the RFQ
63426342
// manager. If the order already exists for the given asset, it will be updated.
6343-
func (r *rpcServer) AddAssetBuyOrder(_ context.Context,
6343+
func (r *rpcServer) AddAssetBuyOrder(ctx context.Context,
63446344
req *rfqrpc.AddAssetBuyOrderRequest) (*rfqrpc.AddAssetBuyOrderResponse,
63456345
error) {
63466346

@@ -6354,13 +6354,24 @@ func (r *rpcServer) AddAssetBuyOrder(_ context.Context,
63546354
return nil, fmt.Errorf("error unmarshalling buy order: %w", err)
63556355
}
63566356

6357-
peerStr := fn.MapOptionZ(
6358-
buyOrder.Peer, func(peerVertex route.Vertex) string {
6359-
return peerVertex.String()
6360-
},
6357+
// Currently, we require the peer to be specified in the buy order.
6358+
peer, err := buyOrder.Peer.UnwrapOrErr(
6359+
fmt.Errorf("buy order peer must be specified"),
6360+
)
6361+
if err != nil {
6362+
return nil, err
6363+
}
6364+
6365+
// Check if we have a channel with the peer.
6366+
err = r.checkPeerChannel(
6367+
ctx, peer, buyOrder.AssetSpecifier, req.SkipAssetChannelCheck,
63616368
)
6369+
if err != nil {
6370+
return nil, fmt.Errorf("error checking peer channel: %w", err)
6371+
}
6372+
63626373
rpcsLog.Debugf("[AddAssetBuyOrder]: upserting buy order "+
6363-
"(dest_peer=%s)", peerStr)
6374+
"(dest_peer=%s)", peer.String())
63646375

63656376
// Register an event listener before actually inserting the order, so we
63666377
// definitely don't miss any responses.
@@ -6402,11 +6413,61 @@ func (r *rpcServer) AddAssetBuyOrder(_ context.Context,
64026413

64036414
case <-timeout:
64046415
return nil, fmt.Errorf("timeout waiting for response "+
6405-
"(peer=%s)", peerStr)
6416+
"(peer=%s)", peer.String())
64066417
}
64076418
}
64086419
}
64096420

6421+
// checkPeerChannel checks if there is a channel with the given peer. If the
6422+
// asset channel check is enabled, it will also check if there is a channel with
6423+
// the given asset with the peer.
6424+
func (r *rpcServer) checkPeerChannel(ctx context.Context, peer route.Vertex,
6425+
specifier asset.Specifier, skipAssetChannelCheck bool) error {
6426+
6427+
// We want to make sure there is at least a channel between us and the
6428+
// peer, otherwise RFQ negotiation doesn't make sense.
6429+
switch {
6430+
// For integration tests, we can't create asset channels, so we allow
6431+
// the asset channel check to be skipped. In this case we simply check
6432+
// that we have any channel with the peer.
6433+
case skipAssetChannelCheck:
6434+
activeChannels, err := r.cfg.Lnd.Client.ListChannels(
6435+
ctx, true, false,
6436+
)
6437+
if err != nil {
6438+
return fmt.Errorf("unable to fetch channels: %w", err)
6439+
}
6440+
peerChannels := fn.Filter(
6441+
activeChannels, func(c lndclient.ChannelInfo) bool {
6442+
return c.PubKeyBytes == peer
6443+
},
6444+
)
6445+
if len(peerChannels) == 0 {
6446+
return fmt.Errorf("no active channel found with peer "+
6447+
"%x", peer[:])
6448+
}
6449+
6450+
// For any other case, we'll want to make sure there is a channel with
6451+
// a non-zero balance of the given asset to carry the order.
6452+
default:
6453+
assetID, err := specifier.UnwrapIdOrErr()
6454+
if err != nil {
6455+
return fmt.Errorf("cannot check asset channel, " +
6456+
"missing asset ID")
6457+
}
6458+
6459+
// If we don't get an error here, it means we do have an asset
6460+
// channel with the peer.
6461+
_, err = r.rfqChannel(ctx, assetID, &peer)
6462+
if err != nil {
6463+
return fmt.Errorf("error checking asset channel: %w",
6464+
err)
6465+
}
6466+
}
6467+
6468+
return nil
6469+
}
6470+
64106471
// unmarshalAssetSellOrder unmarshals an asset sell order from the RPC form.
64116472
func unmarshalAssetSellOrder(
64126473
req *rfqrpc.AddAssetSellOrderRequest) (*rfq.SellOrder, error) {
@@ -6457,7 +6518,7 @@ func unmarshalAssetSellOrder(
64576518

64586519
// AddAssetSellOrder upserts a new sell order for the given asset into the RFQ
64596520
// manager. If the order already exists for the given asset, it will be updated.
6460-
func (r *rpcServer) AddAssetSellOrder(_ context.Context,
6521+
func (r *rpcServer) AddAssetSellOrder(ctx context.Context,
64616522
req *rfqrpc.AddAssetSellOrderRequest) (*rfqrpc.AddAssetSellOrderResponse,
64626523
error) {
64636524

@@ -6472,13 +6533,24 @@ func (r *rpcServer) AddAssetSellOrder(_ context.Context,
64726533
err)
64736534
}
64746535

6475-
// Extract peer identifier as a string for logging.
6476-
peerStr := fn.MapOptionZ(sellOrder.Peer, func(p route.Vertex) string {
6477-
return p.String()
6478-
})
6536+
// Currently, we require the peer to be specified in the buy order.
6537+
peer, err := sellOrder.Peer.UnwrapOrErr(
6538+
fmt.Errorf("sell order peer must be specified"),
6539+
)
6540+
if err != nil {
6541+
return nil, err
6542+
}
6543+
6544+
// Check if we have a channel with the peer.
6545+
err = r.checkPeerChannel(
6546+
ctx, peer, sellOrder.AssetSpecifier, req.SkipAssetChannelCheck,
6547+
)
6548+
if err != nil {
6549+
return nil, fmt.Errorf("error checking peer channel: %w", err)
6550+
}
64796551

64806552
rpcsLog.Debugf("[AddAssetSellOrder]: upserting sell order "+
6481-
"(dest_peer=%s)", peerStr)
6553+
"(dest_peer=%s)", peer.String())
64826554

64836555
// Register an event listener before actually inserting the order, so we
64846556
// definitely don't miss any responses.
@@ -6520,7 +6592,7 @@ func (r *rpcServer) AddAssetSellOrder(_ context.Context,
65206592

65216593
case <-timeout:
65226594
return nil, fmt.Errorf("timeout waiting for response "+
6523-
"from peer %s", peerStr)
6595+
"from peer %s", peer.String())
65246596
}
65256597
}
65266598
}

0 commit comments

Comments
 (0)