diff --git a/rfq/manager.go b/rfq/manager.go index 2a3725752d..f50003313c 100644 --- a/rfq/manager.go +++ b/rfq/manager.go @@ -228,10 +228,11 @@ func (m *Manager) startSubsystems(ctx context.Context) error { // Initialise and start the order handler. m.orderHandler, err = NewOrderHandler(OrderHandlerCfg{ - CleanupInterval: CacheCleanupInterval, - HtlcInterceptor: m.cfg.HtlcInterceptor, - HtlcSubscriber: m.cfg.HtlcSubscriber, - AcceptHtlcEvents: m.acceptHtlcEvents, + CleanupInterval: CacheCleanupInterval, + HtlcInterceptor: m.cfg.HtlcInterceptor, + HtlcSubscriber: m.cfg.HtlcSubscriber, + AcceptHtlcEvents: m.acceptHtlcEvents, + AssetSpecifierChecker: m, }) if err != nil { return fmt.Errorf("error initializing RFQ order handler: %w", diff --git a/rfq/order.go b/rfq/order.go index e1980abd22..f279d4f02e 100644 --- a/rfq/order.go +++ b/rfq/order.go @@ -3,6 +3,7 @@ package rfq import ( "bytes" "context" + "crypto/sha256" "fmt" "sync" "time" @@ -53,12 +54,23 @@ func parseHtlcCustomRecords(customRecords map[uint64][]byte) (*rfqmsg.Htlc, // SerialisedScid is a serialised short channel id (SCID). type SerialisedScid = rfqmsg.SerialisedScid +// AssetSpecifierChecker is an interface that contains methods for checking +// certain properties related to asset specifiers. +type AssetSpecifierChecker interface { + // AssetMatchesSpecifier checks whether the passed specifier and asset + // ID match. If the specifier contains a group key, it will check + // whether the asset belongs to that group. + AssetMatchesSpecifier(ctx context.Context, + specifier asset.Specifier, id asset.ID) (bool, error) +} + // Policy is an interface that abstracts the terms which determine whether an // asset sale/purchase channel HTLC is accepted or rejected. type Policy interface { // CheckHtlcCompliance returns an error if the given HTLC intercept // descriptor does not satisfy the subject policy. - CheckHtlcCompliance(htlc lndclient.InterceptedHtlc) error + CheckHtlcCompliance(htlc lndclient.InterceptedHtlc, + specifierChecker AssetSpecifierChecker) error // Expiry returns the policy's expiry time as a unix timestamp. Expiry() uint64 @@ -146,7 +158,8 @@ func NewAssetSalePolicy(quote rfqmsg.BuyAccept) *AssetSalePolicy { // information used to determine the policy applicable to the HTLC. As a result, // HTLC custom records are not expected to be present. func (c *AssetSalePolicy) CheckHtlcCompliance( - htlc lndclient.InterceptedHtlc) error { + htlc lndclient.InterceptedHtlc, + specifierChecker AssetSpecifierChecker) error { // Since we will be reading CurrentAmountMsat value we acquire a read // lock. @@ -248,11 +261,22 @@ func (c *AssetSalePolicy) GenerateInterceptorResponse( outgoingAmt := rfqmath.DefaultOnChainHtlcMSat - // Unpack asset ID. - assetID, err := c.AssetSpecifier.UnwrapIdOrErr() - if err != nil { - return nil, fmt.Errorf("asset sale policy has no asset ID: %w", - err) + var assetID asset.ID + + switch { + case c.AssetSpecifier.HasGroupPubKey(): + groupKey := c.AssetSpecifier.UnwrapGroupKeyToPtr() + + // We have performed checks for the asset IDs inside the HTLC + // against the specifier's group key in a previous step. Here + // we just need to provide a dummy value as the asset ID. The + // real asset IDs will be carefully picked in a later step in + // the process. What really matters now is the total amount. + assetID = sha256.Sum256(groupKey.SerializeCompressed()) + + case c.AssetSpecifier.HasId(): + specifierID := *c.AssetSpecifier.UnwrapIdToPtr() + copy(assetID[:], specifierID[:]) } // Compute the outgoing asset amount given the msat outgoing amount and @@ -342,7 +366,8 @@ func NewAssetPurchasePolicy(quote rfqmsg.SellAccept) *AssetPurchasePolicy { // CheckHtlcCompliance returns an error if the given HTLC intercept descriptor // does not satisfy the subject policy. func (c *AssetPurchasePolicy) CheckHtlcCompliance( - htlc lndclient.InterceptedHtlc) error { + htlc lndclient.InterceptedHtlc, + specifierChecker AssetSpecifierChecker) error { // Since we will be reading CurrentAmountMsat value we acquire a read // lock. @@ -368,7 +393,9 @@ func (c *AssetPurchasePolicy) CheckHtlcCompliance( } // Sum the asset balance in the HTLC record. - assetAmt, err := htlcRecord.SumAssetBalance(c.AssetSpecifier) + assetAmt, err := htlcRecord.SumAssetBalance( + c.AssetSpecifier, specifierChecker, + ) if err != nil { return fmt.Errorf("error summing asset balance: %w", err) } @@ -524,14 +551,15 @@ func NewAssetForwardPolicy(incoming, outgoing Policy) (*AssetForwardPolicy, // CheckHtlcCompliance returns an error if the given HTLC intercept descriptor // does not satisfy the subject policy. func (a *AssetForwardPolicy) CheckHtlcCompliance( - htlc lndclient.InterceptedHtlc) error { + htlc lndclient.InterceptedHtlc, + sChk AssetSpecifierChecker) error { - if err := a.incomingPolicy.CheckHtlcCompliance(htlc); err != nil { + if err := a.incomingPolicy.CheckHtlcCompliance(htlc, sChk); err != nil { return fmt.Errorf("error checking forward policy, inbound "+ "HTLC does not comply with policy: %w", err) } - if err := a.outgoingPolicy.CheckHtlcCompliance(htlc); err != nil { + if err := a.outgoingPolicy.CheckHtlcCompliance(htlc, sChk); err != nil { return fmt.Errorf("error checking forward policy, outbound "+ "HTLC does not comply with policy: %w", err) } @@ -642,6 +670,10 @@ type OrderHandlerCfg struct { // HtlcSubscriber is a subscriber that is used to retrieve live HTLC // event updates. HtlcSubscriber HtlcSubscriber + + // AssetSpecifierChecker is an interface that contains methods for + // checking certain properties related to asset specifiers. + AssetSpecifierChecker AssetSpecifierChecker } // OrderHandler orchestrates management of accepted quote bundles. It monitors @@ -716,7 +748,7 @@ func (h *OrderHandler) handleIncomingHtlc(_ context.Context, // At this point, we know that a policy exists and has not expired // whilst sitting in the local cache. We can now check that the HTLC // complies with the policy. - err = policy.CheckHtlcCompliance(htlc) + err = policy.CheckHtlcCompliance(htlc, h.cfg.AssetSpecifierChecker) if err != nil { log.Warnf("HTLC does not comply with policy: %v "+ "(HTLC=%v, policy=%v)", err, htlc, policy) diff --git a/rfqmsg/records.go b/rfqmsg/records.go index a702d6b420..5a9a23e8d9 100644 --- a/rfqmsg/records.go +++ b/rfqmsg/records.go @@ -2,6 +2,7 @@ package rfqmsg import ( "bytes" + "context" "encoding/hex" "encoding/json" "errors" @@ -11,6 +12,7 @@ import ( "github.com/lightninglabs/taproot-assets/asset" "github.com/lightninglabs/taproot-assets/fn" "github.com/lightninglabs/taproot-assets/rfqmath" + "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/tlv" ) @@ -82,22 +84,39 @@ func (h *Htlc) Balances() []*AssetBalance { return h.Amounts.Val.Balances } +// SpecifierChecker is an interface that contains methods for checking certain +// properties related to asset specifiers. +type SpecifierChecker interface { + // AssetMatchesSpecifier checks whether the passed specifier and asset + // ID match. If the specifier contains a group key, it will check + // whether the asset belongs to that group. + AssetMatchesSpecifier(ctx context.Context, + specifier asset.Specifier, id asset.ID) (bool, error) +} + // SumAssetBalance returns the sum of the asset balances for the given asset. -func (h *Htlc) SumAssetBalance(assetSpecifier asset.Specifier) (rfqmath.BigInt, +func (h *Htlc) SumAssetBalance(assetSpecifier asset.Specifier, + checker SpecifierChecker) (rfqmath.BigInt, error) { balanceTotal := rfqmath.NewBigIntFromUint64(0) - targetAssetID, err := assetSpecifier.UnwrapIdOrErr() - if err != nil { - return balanceTotal, fmt.Errorf("unable to unwrap asset ID: %w", - err) - } - for idx := range h.Amounts.Val.Balances { balance := h.Amounts.Val.Balances[idx] - if balance.AssetID.Val != targetAssetID { + ctxt, cancel := context.WithTimeout( + context.Background(), wait.DefaultTimeout, + ) + defer cancel() + + match, err := checker.AssetMatchesSpecifier( + ctxt, assetSpecifier, balance.AssetID.Val, + ) + if err != nil { + return balanceTotal, err + } + + if !match { continue } diff --git a/rfqmsg/records_test.go b/rfqmsg/records_test.go index 69c68bcc12..0bf86a611c 100644 --- a/rfqmsg/records_test.go +++ b/rfqmsg/records_test.go @@ -2,6 +2,7 @@ package rfqmsg import ( "bytes" + "context" "encoding/json" "testing" @@ -23,6 +24,22 @@ type htlcTestCase struct { sumBalances map[asset.ID]rfqmath.BigInt } +type DummyChecker struct{} + +func (d *DummyChecker) AssetMatchesSpecifier(_ context.Context, + specifier asset.Specifier, id asset.ID) (bool, error) { + + switch { + case specifier.HasGroupPubKey(): + return true, nil + + case specifier.HasId(): + return *specifier.UnwrapIdToPtr() == id, nil + } + + return false, nil +} + // assetHtlcTestCase is a helper function that asserts different properties of // the test case. func assetHtlcTestCase(t *testing.T, tc htlcTestCase) { @@ -61,9 +78,13 @@ func assetHtlcTestCase(t *testing.T, tc htlcTestCase) { tc.sumBalances = make(map[asset.ID]rfqmath.BigInt) } + dummyChecker := DummyChecker{} + for assetID, expectedBalance := range tc.sumBalances { assetSpecifier := asset.NewSpecifierFromId(assetID) - balance, err := tc.htlc.SumAssetBalance(assetSpecifier) + balance, err := tc.htlc.SumAssetBalance( + assetSpecifier, &dummyChecker, + ) require.NoError(t, err) require.Equal(t, expectedBalance, balance) diff --git a/tapchannel/aux_invoice_manager.go b/tapchannel/aux_invoice_manager.go index 511804233d..ca81236c04 100644 --- a/tapchannel/aux_invoice_manager.go +++ b/tapchannel/aux_invoice_manager.go @@ -42,6 +42,12 @@ type RfqManager interface { // node and have been requested by our peers. These quotes are // exclusively available to our node for the sale of assets. LocalAcceptedSellQuotes() rfq.SellAcceptMap + + // AssetMatchesSpecifier checks if the provided asset satisfies the + // provided specifier. If the specifier includes a group key, we will + // check if the asset belongs to that group. + AssetMatchesSpecifier(ctx context.Context, specifier asset.Specifier, + id asset.ID) (bool, error) } // A compile time assertion to ensure that the rfq.Manager meets the expected @@ -128,7 +134,7 @@ func (s *AuxInvoiceManager) Start() error { // handleInvoiceAccept is the handler that will be called for each invoice that // is accepted. It will intercept the HTLCs that attempt to settle the invoice // and modify them if necessary. -func (s *AuxInvoiceManager) handleInvoiceAccept(_ context.Context, +func (s *AuxInvoiceManager) handleInvoiceAccept(ctx context.Context, req lndclient.InvoiceHtlcModifyRequest) ( *lndclient.InvoiceHtlcModifyResponse, error) { @@ -200,7 +206,7 @@ func (s *AuxInvoiceManager) handleInvoiceAccept(_ context.Context, } // We now run some validation checks on the asset HTLC. - err = s.validateAssetHTLC(htlc) + err = s.validateAssetHTLC(ctx, htlc) if err != nil { log.Errorf("Failed to validate asset HTLC: %v", err) @@ -274,22 +280,23 @@ func (s *AuxInvoiceManager) identifierFromQuote( buyQuote, isBuy := acceptedBuyQuotes[rfqID.Scid()] sellQuote, isSell := acceptedSellQuotes[rfqID.Scid()] + var specifier asset.Specifier + switch { case isBuy: - if buyQuote.Request.AssetSpecifier.HasId() { - req := buyQuote.Request - return req.AssetSpecifier, nil - } + specifier = buyQuote.Request.AssetSpecifier case isSell: - if sellQuote.Request.AssetSpecifier.HasId() { - req := sellQuote.Request - return req.AssetSpecifier, nil - } + specifier = sellQuote.Request.AssetSpecifier + } + + err := specifier.AssertNotEmpty() + if err != nil { + return specifier, fmt.Errorf("rfqID does not match any "+ + "accepted buy or sell quote: %v", err) } - return asset.Specifier{}, fmt.Errorf("rfqID does not match any " + - "accepted buy or sell quote") + return specifier, nil } // priceFromQuote retrieves the price from the accepted quote for the given RFQ @@ -382,7 +389,9 @@ func isAssetInvoice(invoice *lnrpc.Invoice, rfqLookup RfqLookup) bool { } // validateAssetHTLC runs a couple of checks on the provided asset HTLC. -func (s *AuxInvoiceManager) validateAssetHTLC(htlc *rfqmsg.Htlc) error { +func (s *AuxInvoiceManager) validateAssetHTLC(ctx context.Context, + htlc *rfqmsg.Htlc) error { + rfqID := htlc.RfqID.ValOpt().UnsafeFromSome() // Retrieve the asset identifier from the RFQ quote. @@ -392,27 +401,21 @@ func (s *AuxInvoiceManager) validateAssetHTLC(htlc *rfqmsg.Htlc) error { "quote: %v", err) } - if !identifier.HasId() { - return fmt.Errorf("asset specifier has empty assetID") - } - // Check for each of the asset balances of the HTLC that the identifier // matches that of the RFQ quote. for _, v := range htlc.Balances() { - err := fn.MapOptionZ( - identifier.ID(), func(id asset.ID) error { - if v.AssetID.Val != id { - return fmt.Errorf("mismatch between " + - "htlc asset ID and rfq asset " + - "ID") - } - - return nil - }, + match, err := s.cfg.RfqManager.AssetMatchesSpecifier( + ctx, identifier, v.AssetID.Val, ) + if err != nil { return err } + + if !match { + return fmt.Errorf("asset ID %s does not match %s", + v.AssetID.Val.String(), identifier.String()) + } } return nil diff --git a/tapchannel/aux_invoice_manager_test.go b/tapchannel/aux_invoice_manager_test.go index e180ca17b8..49baa1cf43 100644 --- a/tapchannel/aux_invoice_manager_test.go +++ b/tapchannel/aux_invoice_manager_test.go @@ -63,6 +63,22 @@ var ( // The test RFQ SCID that is derived from testRfqID. testScid = testRfqID.Scid() + + // The common asset ID used on test cases. + assetID = dummyAssetID(1) + + // The asset ID of the first asset that is considered part of the group. + groupAssetID_1 = dummyAssetID(41) + + // The asset ID of the second asset that is considered part of the + // group. + groupAssetID_2 = dummyAssetID(42) + + // The specifier based on the common asset ID. + assetSpecifier = asset.NewSpecifierFromId(assetID) + + // The specifier based on a dummy generated group key. + groupSpecifier = asset.NewSpecifierFromGroupKey(*pubKeyFromUint64(1337)) ) // mockRfqManager mocks the interface of the rfq manager required by the aux @@ -73,14 +89,41 @@ type mockRfqManager struct { localSellQuotes rfq.SellAcceptMap } +// PeerAcceptedBuyQuotes returns buy quotes that were requested by our node and +// have been accepted by our peers. These quotes are exclusively available to +// our node for the acquisition of assets. func (m *mockRfqManager) PeerAcceptedBuyQuotes() rfq.BuyAcceptMap { return m.peerBuyQuotes } +// LocalAcceptedSellQuotes returns sell quotes that were accepted by our node +// and have been requested by our peers. These quotes are exclusively available +// to our node for the sale of assets. func (m *mockRfqManager) LocalAcceptedSellQuotes() rfq.SellAcceptMap { return m.localSellQuotes } +// AssetMatchesSpecifier checks if the provided asset satisfies the provided +// specifier. If the specifier includes a group key, we will check if the asset +// belongs to that group. +func (m *mockRfqManager) AssetMatchesSpecifier(ctx context.Context, + specifier asset.Specifier, id asset.ID) (bool, error) { + + switch { + case specifier.HasGroupPubKey(): + if id == groupAssetID_1 || id == groupAssetID_2 { + return true, nil + } + + return false, nil + + case specifier.HasId(): + return *specifier.UnwrapIdToPtr() == id, nil + } + + return false, nil +} + // mockHtlcModifier mocks the HtlcModifier interface that is required by the // AuxInvoiceManager. type mockHtlcModifier struct { @@ -100,12 +143,12 @@ func (m *mockHtlcModifier) HtlcModifier(ctx context.Context, res, err := handler(ctx, r) if err != nil { - return err + m.t.Errorf("Handler error: %v", err) } if m.expectedResQue[i].CancelSet { if !res.CancelSet { - return fmt.Errorf("expected cancel set flag") + m.t.Errorf("expected cancel set flag") } continue @@ -113,7 +156,7 @@ func (m *mockHtlcModifier) HtlcModifier(ctx context.Context, // Check if there's a match with the expected outcome. if res.AmtPaid != m.expectedResQue[i].AmtPaid { - return fmt.Errorf("invoice paid amount does not match "+ + m.t.Errorf("invoice paid amount does not match "+ "expected amount, %v != %v", res.AmtPaid, m.expectedResQue[i].AmtPaid) } @@ -269,11 +312,6 @@ func (m *mockHtlcModifierProperty) HtlcModifier(ctx context.Context, // TestAuxInvoiceManager tests that the htlc modifications of the aux invoice // manager align with our expectations. func TestAuxInvoiceManager(t *testing.T) { - var ( - assetID = dummyAssetID(1) - assetSpecifier = asset.NewSpecifierFromId(assetID) - ) - testCases := []struct { name string buyQuotes rfq.BuyAcceptMap @@ -471,6 +509,94 @@ func TestAuxInvoiceManager(t *testing.T) { }, }, }, + { + name: "asset invoice, group key rfq", + requests: []lndclient.InvoiceHtlcModifyRequest{ + { + Invoice: &lnrpc.Invoice{ + RouteHints: testRouteHints(), + ValueMsat: 20_000_000, + PaymentAddr: []byte{1, 1, 1}, + }, + WireCustomRecords: newWireCustomRecords( + t, []*rfqmsg.AssetBalance{ + // Balance asset ID + // belongs to group. + rfqmsg.NewAssetBalance( + groupAssetID_1, + 3, + ), + // Balance asset ID + // belongs to group. + rfqmsg.NewAssetBalance( + groupAssetID_2, + 4, + ), + }, fn.Some(testRfqID), + ), + }, + }, + responses: []lndclient.InvoiceHtlcModifyResponse{ + { + AmtPaid: 3_000_000 + 4_000_000, + }, + }, + buyQuotes: rfq.BuyAcceptMap{ + testScid: { + Peer: testNodeID, + AssetRate: rfqmsg.NewAssetRate( + testAssetRate, time.Now(), + ), + Request: rfqmsg.BuyRequest{ + AssetSpecifier: groupSpecifier, + }, + }, + }, + }, + { + name: "asset invoice, group key rfq, bad htlc", + requests: []lndclient.InvoiceHtlcModifyRequest{ + { + Invoice: &lnrpc.Invoice{ + RouteHints: testRouteHints(), + ValueMsat: 20_000_000, + PaymentAddr: []byte{1, 1, 1}, + }, + WireCustomRecords: newWireCustomRecords( + t, []*rfqmsg.AssetBalance{ + // Balance asset ID does + // not belong to group. + rfqmsg.NewAssetBalance( + dummyAssetID(2), + 3, + ), + // Balance asset ID does + // not belong to group. + rfqmsg.NewAssetBalance( + dummyAssetID(3), + 4, + ), + }, fn.Some(testRfqID), + ), + }, + }, + responses: []lndclient.InvoiceHtlcModifyResponse{ + { + CancelSet: true, + }, + }, + buyQuotes: rfq.BuyAcceptMap{ + testScid: { + Peer: testNodeID, + AssetRate: rfqmsg.NewAssetRate( + testAssetRate, time.Now(), + ), + Request: rfqmsg.BuyRequest{ + AssetSpecifier: groupSpecifier, + }, + }, + }, + }, } for _, testCase := range testCases {