Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi rfq receive (AddInvoice multiple hop hints) #1457

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6665348
rfqmsg: Htlc.SumAssetBalance requires specifier checker
GeorgeTsagk Feb 27, 2025
4ddf460
rfq: policies use specifier checker
GeorgeTsagk Mar 26, 2025
08752b8
rfq: populate assetID field in interceptor response
GeorgeTsagk Mar 26, 2025
8a22cc0
multi: move rfq marshal code into rfq package
GeorgeTsagk Mar 26, 2025
3ebcce5
tapchannel: aux invoice manager uses specifier checker
GeorgeTsagk Feb 27, 2025
b61c791
tapchannel: add group key tests to AuxInvoiceManager
GeorgeTsagk Feb 27, 2025
3d704c2
rfq: don't return error on empty groupkey lookup
GeorgeTsagk Mar 5, 2025
33f55b1
rfq: asset specifier matcher checks against groupkey hash
GeorgeTsagk Mar 5, 2025
32f4184
tapchannel: make ProduceHtlcExtraData groupkey aware
GeorgeTsagk Mar 5, 2025
49a0de8
taprpc: add groupkey to SendPaymentRequest
GeorgeTsagk Mar 5, 2025
5a1ab5f
rpcserver: add asset specifier marshaller
GeorgeTsagk Mar 26, 2025
a8ebcaf
rpcserver: SendPayment uses groupkey
GeorgeTsagk Mar 5, 2025
a486a7b
taprpc: add groupkey to AddInvoice
GeorgeTsagk Mar 5, 2025
99e85a7
rpcserver: AddInvoice uses groupkey
GeorgeTsagk Mar 5, 2025
46a2f28
multi: move rfq channel helpers to rfq manager
GeorgeTsagk Mar 27, 2025
f5f7ae4
rfq+rpcserver: RfqChannel returns map of peers to channels
GeorgeTsagk Mar 27, 2025
b4eeae1
rfq: add RfqToHopHint helper in manager
GeorgeTsagk Mar 27, 2025
ea6446a
rpcserver: add AcquireBuyOrder helper
GeorgeTsagk Mar 27, 2025
f97fef1
rpcserver: AddInvoices supports multi-rfq
GeorgeTsagk Mar 27, 2025
c5d02b1
taprpc: update AddInvoice documentation
GeorgeTsagk Mar 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
246 changes: 244 additions & 2 deletions rfq/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,26 @@
"context"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"sort"
"sync"
"time"

"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcec/v2/schnorr"
"github.com/lightninglabs/lndclient"
"github.com/lightninglabs/taproot-assets/address"
"github.com/lightninglabs/taproot-assets/asset"
"github.com/lightninglabs/taproot-assets/fn"
"github.com/lightninglabs/taproot-assets/rfqmsg"
"github.com/lightninglabs/taproot-assets/taprpc/rfqrpc"
lfn "github.com/lightningnetwork/lnd/fn/v2"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnutils"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/lightningnetwork/lnd/routing/route"
"github.com/lightningnetwork/lnd/zpay32"
)

const (
Expand Down Expand Up @@ -232,6 +239,7 @@
HtlcInterceptor: m.cfg.HtlcInterceptor,
HtlcSubscriber: m.cfg.HtlcSubscriber,
AcceptHtlcEvents: m.acceptHtlcEvents,
SpecifierChecker: m.AssetMatchesSpecifier,
})
if err != nil {
return fmt.Errorf("error initializing RFQ order handler: %w",
Expand Down Expand Up @@ -948,6 +956,10 @@
// Perform the DB query.
group, err := m.cfg.GroupLookup.QueryAssetGroup(ctx, id)
if err != nil {
if errors.Is(err, address.ErrAssetGroupUnknown) {
return fn.None[btcec.PublicKey](), nil
}

return fn.None[btcec.PublicKey](), err
}

Expand All @@ -971,6 +983,18 @@

switch {
case specifier.HasGroupPubKey():
specifierGK := specifier.UnwrapGroupKeyToPtr()

// Let's directly check if the ID is equal to the hash of the
// group key. This is used by the sender to indicate that any
// asset that belongs to this group may be used.
groupKeyX := schnorr.SerializePubKey(specifierGK)
if asset.ID(groupKeyX) == id {
return true, nil
}

// Now let's make an actual query to find this assetID's group,
// if it exists.
group, err := m.getAssetGroupKey(ctx, id)
if err != nil {
return false, err
Expand All @@ -980,8 +1004,6 @@
return false, nil
}

specifierGK := specifier.UnwrapGroupKeyToPtr()

return group.UnwrapToPtr().IsEqual(specifierGK), nil

case specifier.HasId():
Expand Down Expand Up @@ -1028,6 +1050,226 @@
return true, nil
}

// ChannelWithSpecifier is a helper struct that combines the information of an
// asset specifier that is satisfied by a channel with the channels' general
// information.
type ChannelWithSpecifier struct {
// Specifier is the asset Specifier that is satisfied by this channels'
// assets.
Specifier asset.Specifier

// ChannelInfo is the information about the channel the asset is
// committed to.
ChannelInfo lndclient.ChannelInfo

// AssetInfo contains the asset related info of the channel.
AssetInfo rfqmsg.JsonAssetChanInfo
}

// ComputeChannelAssetBalance computes the total local and remote balance for
// each asset channel that matches the provided asset specifier.
func (m *Manager) ComputeChannelAssetBalance(ctx context.Context,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code move is also a good opportunity to add some test coverage to these functions. Once we add varying RFQ selection strategies, the scope of the functions will expand, so good to nail down some test coverage now while we're at it.

a lil aider can prob go a long way here

activeChannels []lndclient.ChannelInfo,
specifier asset.Specifier) (map[route.Vertex][]ChannelWithSpecifier,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: can use a type def here to give the data structure a more descriptive name/type, and also cut down on the amt of chars needed to ref it a bit.

error) {

peerChanMap := make(map[route.Vertex][]ChannelWithSpecifier)

for chanIdx := range activeChannels {
openChan := activeChannels[chanIdx]
if len(openChan.CustomChannelData) == 0 {
continue
}

var assetData rfqmsg.JsonAssetChannel
err := json.Unmarshal(openChan.CustomChannelData, &assetData)
if err != nil {
return nil, fmt.Errorf("unable to unmarshal asset "+
"data: %w", err)
}

// Check if the assets of this channel match the provided
// specifier.
pass, err := m.ChannelCompatible(
ctx, assetData.Assets, specifier,
)
if err != nil {
return nil, err
}

if pass {
// Since the assets of the channel passed the above
// filter, we're safe to aggregate their info to be
// represented as a single entity.
var aggrInfo rfqmsg.JsonAssetChanInfo

// TODO(george): refactor when JSON gets fixed
for _, info := range assetData.Assets {
aggrInfo.Capacity += info.Capacity
aggrInfo.LocalBalance += info.LocalBalance
aggrInfo.RemoteBalance += info.RemoteBalance
}

_, ok := peerChanMap[openChan.PubKeyBytes]
if !ok {
peerChanMap[openChan.PubKeyBytes] =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since this isn't a nested map, you can actually just ignore ok here and append to the return value, which will automatically allocate space as needed.

make([]ChannelWithSpecifier, 0)
}

chanMap := peerChanMap[openChan.PubKeyBytes]

chanMap = append(chanMap, ChannelWithSpecifier{
Specifier: specifier,
ChannelInfo: openChan,
AssetInfo: aggrInfo,
})

peerChanMap[openChan.PubKeyBytes] = chanMap
}
}

return peerChanMap, nil
}

// ChanLister is a helper that is able to list the channels of the node.
type ChanLister func(ctx context.Context, activeOnly,
publicOnly bool) ([]lndclient.ChannelInfo, error)

// chanIntention defines the intention of calling rfqChannel. This helps with
// returning the channel that is most suitable for what we want to do.
type ChanIntention uint8

const (
// NoIntention defines the absence of any intention, signalling that we
// don't really care which channel is returned.
NoIntention ChanIntention = iota

// SendIntention defines the intention to send over an asset channel.
SendIntention

// ReceiveIntention defines the intention to receive over an asset
// channel.
ReceiveIntention
)

// RfqChannel returns the channel to use for RFQ operations. It returns a map of
// peers and their eligible channels. If a peerPubKey is specified then the map
// will only contain one entry for that peer.
func (m *Manager) RfqChannel(ctx context.Context,
chanLister ChanLister, specifier asset.Specifier,
peerPubKey *route.Vertex, intention ChanIntention) (map[route.Vertex][]ChannelWithSpecifier,

Check failure on line 1160 in rfq/manager.go

View workflow job for this annotation

GitHub Actions / Lint check

The line is 100 characters long, which exceeds the maximum of 80 characters. (lll)
error) {

activeChannels, err := chanLister(ctx, true, false)
if err != nil {
return nil, err
}

balancesMap, err := m.ComputeChannelAssetBalance(
ctx, activeChannels, specifier,
)
if err != nil {
return nil, fmt.Errorf("error computing available asset "+
"channel balance: %w", err)
}

if len(balancesMap) == 0 {
return nil, fmt.Errorf("no asset channel balance found for %s",
&specifier)
}

switch intention {
case SendIntention:
// When sending we care about the volume of our local balances,
// so we sort by local balances in descending order.
for _, v := range balancesMap {
sort.Slice(v, func(i, j int) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, will this actually mutate the underlying value here (the slice) captured by the loop?

Would favor making the re-assignment explicit here, which may guard against future foot guns.

return v[i].AssetInfo.LocalBalance >
v[j].AssetInfo.LocalBalance
})
}
case ReceiveIntention:
// When sending we care about the volume of the remote balances,
// so we sort by remote balances in descending order.
for _, v := range balancesMap {
sort.Slice(v, func(i, j int) bool {
return v[i].AssetInfo.RemoteBalance >
v[j].AssetInfo.RemoteBalance
})
}
case NoIntention:
// We don't care about sending or receiving, this means that
// the method was called as a dry check. Do nothing.
}

// If a peer public key was specified, we always want to use that to
// filter the asset channels.
if peerPubKey != nil {
_, ok := balancesMap[*peerPubKey]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if !ok {
return nil, fmt.Errorf("no asset channels found for "+
"%s and peer=%s", &specifier, peerPubKey)
}
}

return balancesMap, nil
}

// InboundPolicyFetcher is a helper that fetches the inbound policy of a channel
// based on its chanID.
type InboundPolicyFetcher func(ctx context.Context, chanID uint64,
remotePubStr string) (*lnrpc.RoutingPolicy, error)

// RfqToHopHint creates the hop hint representation which encapsulates certain
// quote information along with some other data required by the payment to
// succeed.
func (m *Manager) RfqToHopHint(ctx context.Context,
policyFetcher InboundPolicyFetcher, channelID uint64,
peerPubKey route.Vertex, quote *rfqrpc.PeerAcceptedBuyQuote,
hold bool) (*lnrpc.HopHint, []zpay32.HopHint, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do we return a single RPC hop hint, but a slice of normal hop hints?

I think the godoc comment could use some expansion. Going a bit futher, we may want to consider making a new wrapper type here depending on the end use case (does the client sometimes need to map between the two versions?).


inboundPolicy, err := policyFetcher(ctx, channelID, peerPubKey.String())
if err != nil {
return nil, nil, fmt.Errorf("unable to get inbound channel "+
"policy for channel with ID %d: %w", channelID, err)
}

if hold {
peerPub, err := btcec.ParsePubKey(peerPubKey[:])
if err != nil {
return nil, nil, fmt.Errorf("error parsing peer "+
"pubkey: %w", err)
}
hopHint := []zpay32.HopHint{
{
NodeID: peerPub,
ChannelID: quote.Scid,
FeeBaseMSat: uint32(inboundPolicy.FeeBaseMsat),
FeeProportionalMillionths: uint32(
inboundPolicy.FeeRateMilliMsat,
),
CLTVExpiryDelta: uint16(
inboundPolicy.TimeLockDelta,
),
},
}

return nil, hopHint, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we only ever return a single hop hint here (right now), seems we can just return it directly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we return that directly, then maybe we can make a thin wrapper struct which can take our "rfq" hop hint, and map it into either the zpay32 version (just return the embedded version), or map it into the RPC version.

}

hopHint := &lnrpc.HopHint{
NodeId: peerPubKey.String(),
ChanId: quote.Scid,
FeeBaseMsat: uint32(inboundPolicy.FeeBaseMsat),
FeeProportionalMillionths: uint32(
inboundPolicy.FeeRateMilliMsat,
),
CltvExpiryDelta: inboundPolicy.TimeLockDelta,
}

return hopHint, nil, nil
}

// publishSubscriberEvent publishes an event to all subscribers.
func (m *Manager) publishSubscriberEvent(event fn.Event) {
// Iterate over the subscribers and deliver the event to each one.
Expand Down
Loading
Loading