From df30b481c6a9fc50be64cd5dfd374b11cfb6ba68 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 4 Dec 2024 08:56:59 +0100 Subject: [PATCH 1/7] input: export NUMS key parser. --- input/script_utils.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/input/script_utils.go b/input/script_utils.go index 000efe9585..90e368dd12 100644 --- a/input/script_utils.go +++ b/input/script_utils.go @@ -29,9 +29,9 @@ var ( SequenceLockTimeSeconds = uint32(1 << 22) ) -// mustParsePubKey parses a hex encoded public key string into a public key and +// MustParsePubKey parses a hex encoded public key string into a public key and // panic if parsing fails. -func mustParsePubKey(pubStr string) btcec.PublicKey { +func MustParsePubKey(pubStr string) btcec.PublicKey { pubBytes, err := hex.DecodeString(pubStr) if err != nil { panic(err) @@ -55,7 +55,7 @@ var ( // https://github.com/lightninglabs/lightning-node-connect/tree/ // master/mailbox/numsgen, with the seed phrase "Lightning Simple // Taproot". - TaprootNUMSKey = mustParsePubKey(TaprootNUMSHex) + TaprootNUMSKey = MustParsePubKey(TaprootNUMSHex) ) // Signature is an interface for objects that can populate signatures during From e47024b7907d37b7c35ad5d341e60ae654722c7b Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 9 Dec 2024 22:08:43 +0100 Subject: [PATCH 2/7] routing: Use NUMS point for blinded paths To be able to do MPP payment to multiple blinded routes we need to add a constant dummy hop as a final hop to every blined path. This is used when sending or querying a blinded path, to let the pathfinder be able to send MPP payments over different blinded routes. For this dummy final hop we use a NUMS key so that we are sure no other node can use this blinded pubkey either in a normal or blinded route. Moreover this helps us handling the mission control data for blinded paths correctly because we always consider the blinded pubkey pairs which are registered with mission control when a payment to a blinded path fails. --- routing/blinding.go | 203 ++++++++++++++++++++++++++++++--------- routing/blinding_test.go | 66 ++++++++++++- routing/pathfind.go | 42 ++++++-- routing/pathfind_test.go | 4 +- 4 files changed, 257 insertions(+), 58 deletions(-) diff --git a/routing/blinding.go b/routing/blinding.go index 0c27e87439..70e1a22cb3 100644 --- a/routing/blinding.go +++ b/routing/blinding.go @@ -1,17 +1,27 @@ package routing import ( + "bytes" "errors" "fmt" "github.com/btcsuite/btcd/btcec/v2" + "github.com/decred/dcrd/dcrec/secp256k1/v4" sphinx "github.com/lightningnetwork/lightning-onion" - "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph/db/models" + "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) +// BlindedPathNUMSHex is the hex encoded version of the blinded path target +// NUMs key (in compressed format) which has no known private key. +// This was generated using the following script: +// https://github.com/lightninglabs/lightning-node-connect/tree/master/ +// mailbox/numsgen, with the seed phrase "Lightning Blinded Path". +const BlindedPathNUMSHex = "02667a98ef82ecb522f803b17a74f14508a48b25258f9831" + + "dd6e95f5e299dfd54e" + var ( // ErrNoBlindedPath is returned when the blinded path in a blinded // payment is missing. @@ -25,6 +35,14 @@ var ( // ErrHTLCRestrictions is returned when a blinded path has invalid // HTLC maximum and minimum values. ErrHTLCRestrictions = errors.New("invalid htlc minimum and maximum") + + // BlindedPathNUMSKey is a NUMS key (nothing up my sleeves number) that + // has no known private key. + BlindedPathNUMSKey = input.MustParsePubKey(BlindedPathNUMSHex) + + // CompressedBlindedPathNUMSKey is the compressed version of the + // BlindedPathNUMSKey. + CompressedBlindedPathNUMSKey = BlindedPathNUMSKey.SerializeCompressed() ) // BlindedPaymentPathSet groups the data we need to handle sending to a set of @@ -70,7 +88,9 @@ type BlindedPaymentPathSet struct { } // NewBlindedPaymentPathSet constructs a new BlindedPaymentPathSet from a set of -// BlindedPayments. +// BlindedPayments. For blinded paths which have more than one single hop a +// dummy hop via a NUMS key is appeneded to allow for MPP path finding via +// multiple blinded paths. func NewBlindedPaymentPathSet(paths []*BlindedPayment) (*BlindedPaymentPathSet, error) { @@ -103,36 +123,53 @@ func NewBlindedPaymentPathSet(paths []*BlindedPayment) (*BlindedPaymentPathSet, } } - // Derive an ephemeral target priv key that will be injected into each - // blinded path final hop. - targetPriv, err := btcec.NewPrivateKey() - if err != nil { - return nil, err + // Deep copy the paths to avoid mutating the original paths. + pathSet := make([]*BlindedPayment, len(paths)) + for i, path := range paths { + pathSet[i] = path.deepCopy() } - targetPub := targetPriv.PubKey() - var ( - pathSet = paths - finalCLTVDelta uint16 - ) - // If any provided blinded path only has a single hop (ie, the - // destination node is also the introduction node), then we discard all - // other paths since we know the real pub key of the destination node. - // We also then set the final CLTV delta to the path's delta since - // there are no other edge hints that will account for it. For a single - // hop path, there is also no need for the pseudo target pub key - // replacement, so our target pub key in this case just remains the - // real introduction node ID. - for _, path := range paths { - if len(path.BlindedPath.BlindedHops) != 1 { - continue + // For blinded paths we use the NUMS key as a target if the blinded + // path has more hops than just the introduction node. + targetPub := &BlindedPathNUMSKey + + var finalCLTVDelta uint16 + + // In case the paths do NOT include a single hop route we append a + // dummy hop via a NUMS key to allow for MPP path finding via multiple + // blinded paths. A unified target is needed to use all blinded paths + // during the payment lifecycle. A dummy hop is solely added for the + // path finding process and is removed after the path is found. This + // ensures that we still populate the mission control with the correct + // data and also respect these mc entries when looking for a path. + for _, path := range pathSet { + pathLength := len(path.BlindedPath.BlindedHops) + + // If any provided blinded path only has a single hop (ie, the + // destination node is also the introduction node), then we + // discard all other paths since we know the real pub key of the + // destination node. We also then set the final CLTV delta to + // the path's delta since there are no other edge hints that + // will account for it. + if pathLength == 1 { + pathSet = []*BlindedPayment{path} + finalCLTVDelta = path.CltvExpiryDelta + targetPub = path.BlindedPath.IntroductionPoint + + break } - pathSet = []*BlindedPayment{path} - finalCLTVDelta = path.CltvExpiryDelta - targetPub = path.BlindedPath.IntroductionPoint - - break + lastHop := path.BlindedPath.BlindedHops[pathLength-1] + path.BlindedPath.BlindedHops = append( + path.BlindedPath.BlindedHops, + &sphinx.BlindedHopInfo{ + BlindedNodePub: &BlindedPathNUMSKey, + // We add the last hop's cipher text so that + // the payload size of the final hop is equal + // to the real last hop. + CipherText: lastHop.CipherText, + }, + ) } return &BlindedPaymentPathSet{ @@ -222,7 +259,7 @@ func (s *BlindedPaymentPathSet) ToRouteHints() (RouteHints, error) { hints := make(RouteHints) for _, path := range s.paths { - pathHints, err := path.toRouteHints(fn.Some(s.targetPubKey)) + pathHints, err := path.toRouteHints() if err != nil { return nil, err } @@ -239,6 +276,12 @@ func (s *BlindedPaymentPathSet) ToRouteHints() (RouteHints, error) { return hints, nil } +// IsBlindedRouteNUMSTargetKey returns true if the given public key is the +// NUMS key used as a target for blinded path final hops. +func IsBlindedRouteNUMSTargetKey(pk []byte) bool { + return bytes.Equal(pk, CompressedBlindedPathNUMSKey) +} + // BlindedPayment provides the path and payment parameters required to send a // payment along a blinded path. type BlindedPayment struct { @@ -291,6 +334,22 @@ func (b *BlindedPayment) Validate() error { b.HtlcMaximum, b.HtlcMinimum) } + for _, hop := range b.BlindedPath.BlindedHops { + // The first hop of the blinded path does not necessarily have + // blinded node pub key because it is the introduction point. + if hop.BlindedNodePub == nil { + continue + } + + if IsBlindedRouteNUMSTargetKey( + hop.BlindedNodePub.SerializeCompressed(), + ) { + + return fmt.Errorf("blinded path cannot include NUMS "+ + "key: %s", BlindedPathNUMSHex) + } + } + return nil } @@ -301,11 +360,8 @@ func (b *BlindedPayment) Validate() error { // effectively the final_cltv_delta for the receiving introduction node). In // the case of multiple blinded hops, CLTV delta is fully accounted for in the // hints (both for intermediate hops and the final_cltv_delta for the receiving -// node). The pseudoTarget, if provided, will be used to override the pub key -// of the destination node in the path. -func (b *BlindedPayment) toRouteHints( - pseudoTarget fn.Option[*btcec.PublicKey]) (RouteHints, error) { - +// node). +func (b *BlindedPayment) toRouteHints() (RouteHints, error) { // If we just have a single hop in our blinded route, it just contains // an introduction node (this is a valid path according to the spec). // Since we have the un-blinded node ID for the introduction node, we @@ -393,16 +449,77 @@ func (b *BlindedPayment) toRouteHints( hints[fromNode] = []AdditionalEdge{lastEdge} } - pseudoTarget.WhenSome(func(key *btcec.PublicKey) { - // For the very last hop on the path, switch out the ToNodePub - // for the pseudo target pub key. - lastEdge.policy.ToNodePubKey = func() route.Vertex { - return route.NewVertex(key) + return hints, nil +} + +// deepCopy returns a deep copy of the BlindedPayment. +func (b *BlindedPayment) deepCopy() *BlindedPayment { + if b == nil { + return nil + } + + cpyPayment := &BlindedPayment{ + BaseFee: b.BaseFee, + ProportionalFeeRate: b.ProportionalFeeRate, + CltvExpiryDelta: b.CltvExpiryDelta, + HtlcMinimum: b.HtlcMinimum, + HtlcMaximum: b.HtlcMaximum, + } + + // Deep copy the BlindedPath if it exists + if b.BlindedPath != nil { + cpyPayment.BlindedPath = &sphinx.BlindedPath{ + BlindedHops: make([]*sphinx.BlindedHopInfo, + len(b.BlindedPath.BlindedHops)), } - // Then override the final hint with this updated edge. - hints[fromNode] = []AdditionalEdge{lastEdge} - }) + if b.BlindedPath.IntroductionPoint != nil { + cpyPayment.BlindedPath.IntroductionPoint = + copyPublicKey(b.BlindedPath.IntroductionPoint) + } - return hints, nil + if b.BlindedPath.BlindingPoint != nil { + cpyPayment.BlindedPath.BlindingPoint = + copyPublicKey(b.BlindedPath.BlindingPoint) + } + + // Copy each blinded hop info. + for i, hop := range b.BlindedPath.BlindedHops { + if hop == nil { + continue + } + + cpyHop := &sphinx.BlindedHopInfo{ + CipherText: hop.CipherText, + } + + if hop.BlindedNodePub != nil { + cpyHop.BlindedNodePub = + copyPublicKey(hop.BlindedNodePub) + } + + cpyHop.CipherText = make([]byte, len(hop.CipherText)) + copy(cpyHop.CipherText, hop.CipherText) + + cpyPayment.BlindedPath.BlindedHops[i] = cpyHop + } + } + + // Deep copy the Features if they exist + if b.Features != nil { + cpyPayment.Features = b.Features.Clone() + } + + return cpyPayment +} + +// copyPublicKey makes a deep copy of a public key. +// +// TODO(ziggie): Remove this function if this is available in the btcec library. +func copyPublicKey(pk *btcec.PublicKey) *btcec.PublicKey { + var result secp256k1.JacobianPoint + pk.AsJacobian(&result) + result.ToAffine() + + return btcec.NewPublicKey(&result.X, &result.Y) } diff --git a/routing/blinding_test.go b/routing/blinding_test.go index 8f83f7fd82..0a8846adb7 100644 --- a/routing/blinding_test.go +++ b/routing/blinding_test.go @@ -2,11 +2,11 @@ package routing import ( "bytes" + "reflect" "testing" "github.com/btcsuite/btcd/btcec/v2" sphinx "github.com/lightningnetwork/lightning-onion" - "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" @@ -129,7 +129,7 @@ func TestBlindedPaymentToHints(t *testing.T) { HtlcMaximum: htlcMax, Features: features, } - hints, err := blindedPayment.toRouteHints(fn.None[*btcec.PublicKey]()) + hints, err := blindedPayment.toRouteHints() require.NoError(t, err) require.Nil(t, hints) @@ -184,7 +184,7 @@ func TestBlindedPaymentToHints(t *testing.T) { }, } - actual, err := blindedPayment.toRouteHints(fn.None[*btcec.PublicKey]()) + actual, err := blindedPayment.toRouteHints() require.NoError(t, err) require.Equal(t, len(expected), len(actual)) @@ -218,3 +218,63 @@ func TestBlindedPaymentToHints(t *testing.T) { require.Equal(t, expectedHint[0], actualHint[0]) } } + +// TestBlindedPaymentDeepCopy tests the deep copy method of the BLindedPayment +// struct. +// +// TODO(ziggie): Make this a property test instead. +func TestBlindedPaymentDeepCopy(t *testing.T) { + _, pkBlind1 := btcec.PrivKeyFromBytes([]byte{1}) + _, blindingPoint := btcec.PrivKeyFromBytes([]byte{2}) + _, pkBlind2 := btcec.PrivKeyFromBytes([]byte{3}) + + // Create a test BlindedPayment with non-nil fields + original := &BlindedPayment{ + BaseFee: 1000, + ProportionalFeeRate: 2000, + CltvExpiryDelta: 144, + HtlcMinimum: 1000, + HtlcMaximum: 1000000, + Features: lnwire.NewFeatureVector(nil, nil), + BlindedPath: &sphinx.BlindedPath{ + IntroductionPoint: pkBlind1, + BlindingPoint: blindingPoint, + BlindedHops: []*sphinx.BlindedHopInfo{ + { + BlindedNodePub: pkBlind2, + CipherText: []byte("test cipher"), + }, + }, + }, + } + + // Make a deep copy + cpyPayment := original.deepCopy() + + // Test 1: Verify the copy is not the same pointer + if cpyPayment == original { + t.Fatal("deepCopy returned same pointer") + } + + // Verify all fields are equal + if !reflect.DeepEqual(original, cpyPayment) { + t.Fatal("copy is not equal to original") + } + + // Modify the copy and verify it doesn't affect the original + cpyPayment.BaseFee = 2000 + cpyPayment.BlindedPath.BlindedHops[0].CipherText = []byte("modified") + + require.NotEqual(t, original.BaseFee, cpyPayment.BaseFee) + + require.NotEqual( + t, + original.BlindedPath.BlindedHops[0].CipherText, + cpyPayment.BlindedPath.BlindedHops[0].CipherText, + ) + + // Verify nil handling. + var nilPayment *BlindedPayment + nilCopy := nilPayment.deepCopy() + require.Nil(t, nilCopy) +} diff --git a/routing/pathfind.go b/routing/pathfind.go index 80f4b1e68f..5b5bbe9f27 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -158,6 +158,32 @@ func newRoute(sourceVertex route.Vertex, ) pathLength := len(pathEdges) + + // When paying to a blinded route we might have appended a dummy hop at + // the end to make MPP payments possible via all paths of the blinded + // route set. We always append a dummy hop when the internal pathfiner + // looks for a route to a blinded path which is at least one hop long + // (excluding the introduction point). We add this dummy hop so that + // we search for a universal target but also respect potential mc + // entries which might already be present for a particular blinded path. + // However when constructing the Sphinx packet we need to remove this + // dummy hop again which we do here. + // + // NOTE: The path length is always at least 1 because there must be one + // edge from the source to the destination. However we check for > 0 + // just for robustness here. + if blindedPathSet != nil && pathLength > 0 { + finalBlindedPubKey := pathEdges[pathLength-1].policy. + ToNodePubKey() + + if IsBlindedRouteNUMSTargetKey(finalBlindedPubKey[:]) { + // If the last hop is the NUMS key for blinded paths, we + // remove the dummy hop from the route. + pathEdges = pathEdges[:pathLength-1] + pathLength-- + } + } + for i := pathLength - 1; i >= 0; i-- { // Now we'll start to calculate the items within the per-hop // payload for the hop this edge is leading to. @@ -319,10 +345,6 @@ func newRoute(sourceVertex route.Vertex, dataIndex = 0 blindedPath = blindedPayment.BlindedPath - numHops = len(blindedPath.BlindedHops) - realFinal = blindedPath.BlindedHops[numHops-1]. - BlindedNodePub - introVertex = route.NewVertex( blindedPath.IntroductionPoint, ) @@ -350,11 +372,6 @@ func newRoute(sourceVertex route.Vertex, if i != len(hops)-1 { hop.AmtToForward = 0 hop.OutgoingTimeLock = 0 - } else { - // For the final hop, we swap out the pub key - // bytes to the original destination node pub - // key for that payment path. - hop.PubKeyBytes = route.NewVertex(realFinal) } dataIndex++ @@ -901,6 +918,13 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // included. If we are coming from the source hop, the payload // size is zero, because the original htlc isn't in the onion // blob. + // + // NOTE: For blinded paths with the NUMS key as the last hop, + // the payload size accounts for this dummy hop which is of + // the same size as the real last hop. So we account for a + // bigger size than the route is however we accept this + // little inaccuracy here because we are over estimating by + // 1 hop. var payloadSize uint64 if fromVertex != source { // In case the unifiedEdge does not have a payload size diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index c463b8135b..2adeca134b 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -3284,9 +3284,7 @@ func TestBlindedRouteConstruction(t *testing.T) { // that make up the graph we'll give to route construction. The hints // map is keyed by source node, so we can retrieve our blinded edges // accordingly. - blindedEdges, err := blindedPayment.toRouteHints( - fn.None[*btcec.PublicKey](), - ) + blindedEdges, err := blindedPayment.toRouteHints() require.NoError(t, err) carolDaveEdge := blindedEdges[carolVertex][0] From 3cec72ae9c349a4c7a21e3a4a8bac1b832b106fe Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 9 Dec 2024 22:15:21 +0100 Subject: [PATCH 3/7] routing: improve lasthoppaylaod size calculation Fixes a bug and makes the function more robust. Before we would always return the encrypted data size of last hop of the last path. Now we return the greatest last hop payload not always the one of the last path. --- routing/blinding.go | 16 +++++++- routing/pathfind.go | 17 ++++++--- routing/pathfind_test.go | 81 +++++++++++++++++++++++++++------------- 3 files changed, 81 insertions(+), 33 deletions(-) diff --git a/routing/blinding.go b/routing/blinding.go index 70e1a22cb3..a343c15da8 100644 --- a/routing/blinding.go +++ b/routing/blinding.go @@ -235,21 +235,33 @@ func (s *BlindedPaymentPathSet) FinalCLTVDelta() uint16 { // LargestLastHopPayloadPath returns the BlindedPayment in the set that has the // largest last-hop payload. This is to be used for onion size estimation in // path finding. -func (s *BlindedPaymentPathSet) LargestLastHopPayloadPath() *BlindedPayment { +func (s *BlindedPaymentPathSet) LargestLastHopPayloadPath() (*BlindedPayment, + error) { + var ( largestPath *BlindedPayment currentMax int ) + + if len(s.paths) == 0 { + return nil, fmt.Errorf("no blinded paths in the set") + } + + // We set the largest path to make sure we always return a path even + // if the cipher text is empty. + largestPath = s.paths[0] + for _, path := range s.paths { numHops := len(path.BlindedPath.BlindedHops) lastHop := path.BlindedPath.BlindedHops[numHops-1] if len(lastHop.CipherText) > currentMax { largestPath = path + currentMax = len(lastHop.CipherText) } } - return largestPath + return largestPath, nil } // ToRouteHints converts the blinded path payment set into a RouteHints map so diff --git a/routing/pathfind.go b/routing/pathfind.go index 5b5bbe9f27..3f1d0ba092 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -700,7 +700,10 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // The payload size of the final hop differ from intermediate hops // and depends on whether the destination is blinded or not. - lastHopPayloadSize := lastHopPayloadSize(r, finalHtlcExpiry, amt) + lastHopPayloadSize, err := lastHopPayloadSize(r, finalHtlcExpiry, amt) + if err != nil { + return nil, 0, err + } // We can't always assume that the end destination is publicly // advertised to the network so we'll manually include the target node. @@ -1433,11 +1436,15 @@ func getProbabilityBasedDist(weight int64, probability float64, // It depends on the tlv types which are present and also whether the hop is // part of a blinded route or not. func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32, - amount lnwire.MilliSatoshi) uint64 { + amount lnwire.MilliSatoshi) (uint64, error) { if r.BlindedPaymentPathSet != nil { - paymentPath := r.BlindedPaymentPathSet. + paymentPath, err := r.BlindedPaymentPathSet. LargestLastHopPayloadPath() + if err != nil { + return 0, err + } + blindedPath := paymentPath.BlindedPath.BlindedHops blindedPoint := paymentPath.BlindedPath.BlindingPoint @@ -1452,7 +1459,7 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32, } // The final hop does not have a short chanID set. - return finalHop.PayloadSize(0) + return finalHop.PayloadSize(0), nil } var mpp *record.MPP @@ -1478,7 +1485,7 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32, } // The final hop does not have a short chanID set. - return finalHop.PayloadSize(0) + return finalHop.PayloadSize(0), nil } // overflowSafeAdd adds two MilliSatoshi values and returns the result. If an diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 2adeca134b..4579abc6a9 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -3413,32 +3413,48 @@ func TestLastHopPayloadSize(t *testing.T) { customRecords = map[uint64][]byte{ record.CustomTypeStart: {1, 2, 3}, } - sizeEncryptedData = 100 - encrypedData = bytes.Repeat( - []byte{1}, sizeEncryptedData, + + encrypedDataSmall = bytes.Repeat( + []byte{1}, 5, + ) + encrypedDataLarge = bytes.Repeat( + []byte{1}, 100, ) - _, blindedPoint = btcec.PrivKeyFromBytes([]byte{5}) - paymentAddr = &[32]byte{1} - ampOptions = &Options{} - amtToForward = lnwire.MilliSatoshi(10000) - finalHopExpiry int32 = 144 + _, blindedPoint = btcec.PrivKeyFromBytes([]byte{5}) + paymentAddr = &[32]byte{1} + ampOptions = &Options{} + amtToForward = lnwire.MilliSatoshi(10000) + emptyEncryptedData = []byte{} + finalHopExpiry int32 = 144 oneHopPath = &sphinx.BlindedPath{ BlindedHops: []*sphinx.BlindedHopInfo{ { - CipherText: encrypedData, + CipherText: emptyEncryptedData, + }, + }, + BlindingPoint: blindedPoint, + } + + twoHopPathSmallHopSize = &sphinx.BlindedPath{ + BlindedHops: []*sphinx.BlindedHopInfo{ + { + CipherText: encrypedDataLarge, + }, + { + CipherText: encrypedDataLarge, }, }, BlindingPoint: blindedPoint, } - twoHopPath = &sphinx.BlindedPath{ + twoHopPathLargeHopSize = &sphinx.BlindedPath{ BlindedHops: []*sphinx.BlindedHopInfo{ { - CipherText: encrypedData, + CipherText: encrypedDataSmall, }, { - CipherText: encrypedData, + CipherText: encrypedDataSmall, }, }, BlindingPoint: blindedPoint, @@ -3451,15 +3467,19 @@ func TestLastHopPayloadSize(t *testing.T) { require.NoError(t, err) twoHopBlindedPayment, err := NewBlindedPaymentPathSet( - []*BlindedPayment{{BlindedPath: twoHopPath}}, + []*BlindedPayment{ + {BlindedPath: twoHopPathLargeHopSize}, + {BlindedPath: twoHopPathSmallHopSize}, + }, ) require.NoError(t, err) testCases := []struct { - name string - restrictions *RestrictParams - finalHopExpiry int32 - amount lnwire.MilliSatoshi + name string + restrictions *RestrictParams + finalHopExpiry int32 + amount lnwire.MilliSatoshi + expectedEncryptedData []byte }{ { name: "Non blinded final hop", @@ -3477,16 +3497,18 @@ func TestLastHopPayloadSize(t *testing.T) { restrictions: &RestrictParams{ BlindedPaymentPathSet: oneHopBlindedPayment, }, - amount: amtToForward, - finalHopExpiry: finalHopExpiry, + amount: amtToForward, + finalHopExpiry: finalHopExpiry, + expectedEncryptedData: emptyEncryptedData, }, { name: "Blinded final hop of a two hop payment", restrictions: &RestrictParams{ BlindedPaymentPathSet: twoHopBlindedPayment, }, - amount: amtToForward, - finalHopExpiry: finalHopExpiry, + amount: amtToForward, + finalHopExpiry: finalHopExpiry, + expectedEncryptedData: encrypedDataLarge, }, } @@ -3510,16 +3532,23 @@ func TestLastHopPayloadSize(t *testing.T) { var finalHop route.Hop if tc.restrictions.BlindedPaymentPathSet != nil { - path := tc.restrictions.BlindedPaymentPathSet. - LargestLastHopPayloadPath() + bPSet := tc.restrictions.BlindedPaymentPathSet + path, err := bPSet.LargestLastHopPayloadPath() + require.NotNil(t, path) + + require.NoError(t, err) + blindedPath := path.BlindedPath.BlindedHops blindedPoint := path.BlindedPath.BlindingPoint + lastHop := blindedPath[len(blindedPath)-1] + require.Equal(t, lastHop.CipherText, + tc.expectedEncryptedData) //nolint:ll finalHop = route.Hop{ AmtToForward: tc.amount, OutgoingTimeLock: uint32(tc.finalHopExpiry), - EncryptedData: blindedPath[len(blindedPath)-1].CipherText, + EncryptedData: lastHop.CipherText, } if len(blindedPath) == 1 { finalHop.BlindingPoint = blindedPoint @@ -3539,11 +3568,11 @@ func TestLastHopPayloadSize(t *testing.T) { payLoad, err := createHopPayload(finalHop, 0, true) require.NoErrorf(t, err, "failed to create hop payload") - expectedPayloadSize := lastHopPayloadSize( + expectedPayloadSize, err := lastHopPayloadSize( tc.restrictions, tc.finalHopExpiry, tc.amount, ) - + require.NoError(t, err) require.Equal( t, expectedPayloadSize, uint64(payLoad.NumBytes()), From 9327940ac4eeb1b1b276068f4955b3b93dca5070 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 9 Dec 2024 22:17:22 +0100 Subject: [PATCH 4/7] routing: add pathfinding test We add a test where we add duplicate hops in a route and verify that the pathfinding engine can handle this edge case. --- routing/pathfind_test.go | 104 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 4579abc6a9..f91d95d6b0 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -765,6 +765,9 @@ func TestPathFinding(t *testing.T) { }, { name: "path finding with additional edges", fn: runPathFindingWithAdditionalEdges, + }, { + name: "path finding with duplicate blinded hop", + fn: runPathFindingWithBlindedPathDuplicateHop, }, { name: "path finding with redundant additional edges", fn: runPathFindingWithRedundantAdditionalEdges, @@ -1265,6 +1268,107 @@ func runPathFindingWithAdditionalEdges(t *testing.T, useCache bool) { assertExpectedPath(t, graph.aliasMap, path, "songoku", "doge") } +// runPathFindingWithBlindedPathDuplicateHop tests that in case a blinded path +// has duplicate hops that the path finding algorithm does not fail or behave +// incorrectly. This can happen because the creator of the blinded path can +// specify the same hop multiple times and this will only be detected at the +// forwarding nodes, so it is important that we can handle this case. +func runPathFindingWithBlindedPathDuplicateHop(t *testing.T, useCache bool) { + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) + require.NoError(t, err, "unable to create graph") + + sourceNode, err := graph.graph.SourceNode() + require.NoError(t, err, "unable to fetch source node") + + paymentAmt := lnwire.NewMSatFromSatoshis(100) + + songokuPubKeyBytes := graph.aliasMap["songoku"] + songokuPubKey, err := btcec.ParsePubKey(songokuPubKeyBytes[:]) + require.NoError(t, err, "unable to parse public key from bytes") + + _, pkb1 := btcec.PrivKeyFromBytes([]byte{2}) + _, pkb2 := btcec.PrivKeyFromBytes([]byte{3}) + _, blindedPoint := btcec.PrivKeyFromBytes([]byte{5}) + + sizeEncryptedData := 100 + cipherText := bytes.Repeat( + []byte{1}, sizeEncryptedData, + ) + + vb1 := route.NewVertex(pkb1) + vb2 := route.NewVertex(pkb2) + + // Payments to blinded paths always pay to the NUMS target key. + dummyTarget := route.NewVertex(&BlindedPathNUMSKey) + + graph.aliasMap["pkb1"] = vb1 + graph.aliasMap["pkb2"] = vb2 + graph.aliasMap["dummyTarget"] = dummyTarget + + // Create a blinded payment with duplicate hops and make sure the + // path finding algorithm can cope with that. We add blinded hop 2 + // 3 times. The path finding algorithm should create a path with a + // single hop to pkb2 (the first entry). + blindedPayment := &BlindedPayment{ + BlindedPath: &sphinx.BlindedPath{ + IntroductionPoint: songokuPubKey, + BlindingPoint: blindedPoint, + BlindedHops: []*sphinx.BlindedHopInfo{ + { + CipherText: cipherText, + }, + { + BlindedNodePub: pkb2, + CipherText: cipherText, + }, + { + BlindedNodePub: pkb1, + CipherText: cipherText, + }, + { + BlindedNodePub: pkb2, + CipherText: cipherText, + }, + { + BlindedNodePub: &BlindedPathNUMSKey, + CipherText: cipherText, + }, + { + BlindedNodePub: pkb2, + CipherText: cipherText, + }, + }, + }, + HtlcMinimum: 1, + HtlcMaximum: 100_000_000, + CltvExpiryDelta: 140, + } + + blindedPath, err := blindedPayment.toRouteHints() + require.NoError(t, err) + + find := func(r *RestrictParams) ( + []*unifiedEdge, error) { + + return dbFindPath( + graph.graph, blindedPath, &mockBandwidthHints{}, + r, testPathFindingConfig, + sourceNode.PubKeyBytes, dummyTarget, paymentAmt, + 0, 0, + ) + } + + // We should now be able to find a path however not the chained path + // of the blinded hops. + path, err := find(noRestrictions) + require.NoError(t, err, "unable to create route to blinded path") + + // The path should represent the following hops: + // source node -> songoku -> pkb2 -> dummyTarget + assertExpectedPath(t, graph.aliasMap, path, "songoku", "pkb2", + "dummyTarget") +} + // runPathFindingWithRedundantAdditionalEdges asserts that we are able to find // paths to nodes ignoring additional edges that are already known by self node. func runPathFindingWithRedundantAdditionalEdges(t *testing.T, useCache bool) { From decfdb68ac3ec090cbbdc568f2b72004b62f7a8c Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 9 Dec 2024 22:18:27 +0100 Subject: [PATCH 5/7] routing: bugfix for mc reporting of blinded paths When reporting an error or a success case of a payment to a blinded paths, the amounts to forward for intermediate hops are set to 0 so we need to use the receiver amount instead. --- routing/result_interpretation.go | 79 ++++++++++++++++++++++++--- routing/result_interpretation_test.go | 63 +++++++++++++++++---- 2 files changed, 123 insertions(+), 19 deletions(-) diff --git a/routing/result_interpretation.go b/routing/result_interpretation.go index bc1749dfb1..60dd22c438 100644 --- a/routing/result_interpretation.go +++ b/routing/result_interpretation.go @@ -100,8 +100,27 @@ func interpretResult(rt *mcRoute, // processSuccess processes a successful payment attempt. func (i *interpretedResult) processSuccess(route *mcRoute) { - // For successes, all nodes must have acted in the right way. Therefore - // we mark all of them with a success result. + // For successes, all nodes must have acted in the right way. + // Therefore we mark all of them with a success result. However we need + // to handle the blinded route part separately because for intermediate + // blinded nodes the amount field is set to zero so we use the receiver + // amount. + introIdx, isBlinded := introductionPointIndex(route) + if isBlinded { + // Report success for all the pairs until the introduction + // point. + i.successPairRange(route, 0, introIdx-1) + + // Handle the blinded route part. + // + // NOTE: The introIdx index here does describe the node after + // the introduction point. + i.markBlindedRouteSuccess(route, introIdx) + + return + } + + // Mark nodes as successful in the non-blinded case of the payment. i.successPairRange(route, 0, len(route.hops.Val)-1) } @@ -517,11 +536,22 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate(route *mcRoute, if introIdx == len(route.hops.Val)-1 { i.finalFailureReason = &reasonError } else { - // If there are other hops between the recipient and - // introduction node, then we just penalize the last - // hop in the blinded route to minimize the storage of - // results for ephemeral keys. - i.failPairBalance(route, len(route.hops.Val)-1) + // We penalize the final hop of the blinded route which + // is sufficient to not reuse this route again and is + // also more memory efficient because the other hops + // of the blinded path are ephemeral and will only be + // used in conjunction with the final hop. Moreover we + // don't want to punish the introduction node because + // the blinded failure does not necessarily mean that + // the introduction node was at fault. + // + // TODO(ziggie): Make sure we only keep mc data for + // blinded paths, in both the success and failure case, + // in memory during the time of the payment and remove + // it afterwards. Blinded paths and their blinded hop + // keys are always changing per blinded route so there + // is no point in persisting this data. + i.failBlindedRoute(route) } // In all other cases, we penalize the reporting node. These are all @@ -828,6 +858,41 @@ func (i *interpretedResult) successPairRange(rt *mcRoute, fromIdx, toIdx int) { } } +// failBlindedRoute marks a blinded route as failed for the specific amount to +// send by only punishing the last pair. +func (i *interpretedResult) failBlindedRoute(rt *mcRoute) { + // We fail the last pair of the route, in order to fail the complete + // blinded route. This is because the combination of ephemeral pubkeys + // is unique to the route. We fail the last pair in order to not punish + // the introduction node, since we don't want to disincentivize them + // from providing that service. + pair, _ := getPair(rt, len(rt.hops.Val)-1) + + // Since all the hops along a blinded path don't have any amount set, we + // extract the minimal amount to punish from the value that is tried to + // be sent to the receiver. + amt := rt.hops.Val[len(rt.hops.Val)-1].amtToFwd.Val + + i.pairResults[pair] = failPairResult(amt) +} + +// markBlindedRouteSuccess marks the hops of the blinded route AFTER the +// introduction node as successful. +// +// NOTE: The introIdx must be the index of the first hop of the blinded route +// AFTER the introduction node. +func (i *interpretedResult) markBlindedRouteSuccess(rt *mcRoute, introIdx int) { + // For blinded hops we do not have the forwarding amount so we take the + // minimal amount which went through the route by looking at the last + // hop. + successAmt := rt.hops.Val[len(rt.hops.Val)-1].amtToFwd.Val + for idx := introIdx; idx < len(rt.hops.Val); idx++ { + pair, _ := getPair(rt, idx) + + i.pairResults[pair] = successPairResult(successAmt) + } +} + // getPair returns a node pair from the route and the amount passed between that // pair. func getPair(rt *mcRoute, channelIdx int) (DirectedNodePair, diff --git a/routing/result_interpretation_test.go b/routing/result_interpretation_test.go index 8c67bdeea9..a0c3e9062e 100644 --- a/routing/result_interpretation_test.go +++ b/routing/result_interpretation_test.go @@ -96,13 +96,17 @@ var ( AmtToForward: 99, }, { - PubKeyBytes: hops[2], - AmtToForward: 95, + PubKeyBytes: hops[2], + // Intermediate blinded hops don't have an + // amount set. + AmtToForward: 0, BlindingPoint: genTestPubKey(), }, { - PubKeyBytes: hops[3], - AmtToForward: 88, + PubKeyBytes: hops[3], + // Intermediate blinded hops don't have an + // amount set. + AmtToForward: 0, }, { PubKeyBytes: hops[4], @@ -122,8 +126,10 @@ var ( AmtToForward: 99, }, { - PubKeyBytes: hops[2], - AmtToForward: 95, + PubKeyBytes: hops[2], + // Intermediate blinded hops don't have an + // amount set. + AmtToForward: 0, BlindingPoint: genTestPubKey(), }, { @@ -140,13 +146,17 @@ var ( TotalAmount: 100, Hops: []*route.Hop{ { - PubKeyBytes: hops[1], - AmtToForward: 90, + PubKeyBytes: hops[1], + // Intermediate blinded hops don't have an + // amount set. + AmtToForward: 0, BlindingPoint: genTestPubKey(), }, { - PubKeyBytes: hops[2], - AmtToForward: 75, + PubKeyBytes: hops[2], + // Intermediate blinded hops don't have an + // amount set. + AmtToForward: 0, }, { PubKeyBytes: hops[3], @@ -552,7 +562,12 @@ var resultTestCases = []resultTestCase{ pairResults: map[DirectedNodePair]pairResult{ getTestPair(0, 1): successPairResult(100), getTestPair(1, 2): successPairResult(99), - getTestPair(3, 4): failPairResult(88), + + // The amount for the last hop is always the + // receiver amount because the amount to forward + // is always set to 0 for intermediate blinded + // hops. + getTestPair(3, 4): failPairResult(77), }, }, }, @@ -567,7 +582,12 @@ var resultTestCases = []resultTestCase{ expectedResult: &interpretedResult{ pairResults: map[DirectedNodePair]pairResult{ getTestPair(0, 1): successPairResult(100), - getTestPair(2, 3): failPairResult(75), + + // The amount for the last hop is always the + // receiver amount because the amount to forward + // is always set to 0 for intermediate blinded + // hops. + getTestPair(2, 3): failPairResult(58), }, }, }, @@ -682,6 +702,25 @@ var resultTestCases = []resultTestCase{ finalFailureReason: &reasonError, }, }, + // Test a multi-hop blinded route and that in a success case the amounts + // for the blinded route part are correctly set to the receiver amount. + { + name: "blinded multi-hop success", + route: blindedMultiToIntroduction, + success: true, + expectedResult: &interpretedResult{ + pairResults: map[DirectedNodePair]pairResult{ + getTestPair(0, 1): successPairResult(100), + + // For the route blinded part of the route the + // success amount is determined by the receiver + // amount because the intermediate blinded hops + // set the forwarded amount to 0. + getTestPair(1, 2): successPairResult(58), + getTestPair(2, 3): successPairResult(58), + }, + }, + }, } // TestResultInterpretation executes a list of test cases that test the result From 2610663658f6cff674db997df2bcc99d80ea5195 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 10 Dec 2024 11:41:10 +0100 Subject: [PATCH 6/7] docs: add release-notes for 18.4 --- docs/release-notes/release-notes-0.18.4.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release-notes/release-notes-0.18.4.md b/docs/release-notes/release-notes-0.18.4.md index ab72c01c31..e52dbc3d1c 100644 --- a/docs/release-notes/release-notes-0.18.4.md +++ b/docs/release-notes/release-notes-0.18.4.md @@ -26,6 +26,9 @@ * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9324) to prevent potential deadlocks when LND depends on external components (e.g. aux components, hooks). + +* [Make sure blinded payment failures are handled correctly in the mission +controller](https://github.com/lightningnetwork/lnd/pull/9316). # New Features @@ -121,4 +124,5 @@ types in a series of changes: * George Tsagkarelis * Olaoluwa Osuntokun * Oliver Gugger +* Ziggie From 101311debbd1fef6717e1867e0e4232f7eef7c80 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 10 Dec 2024 11:41:28 +0100 Subject: [PATCH 7/7] docs: fix typos in release-notes 19.0 --- docs/release-notes/release-notes-0.19.0.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index ee0b0c1360..6f91a8fc87 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -60,7 +60,7 @@ * [Make the contract resolutions for the channel arbitrator optional]( https://github.com/lightningnetwork/lnd/pull/9253) - * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9322) that caused +* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9322) that caused estimateroutefee to ignore the default payment timeout. # New Features @@ -80,7 +80,7 @@ ## RPC Additions * [Add a new rpc endpoint](https://github.com/lightningnetwork/lnd/pull/8843) - `BumpForceCloseFee` which moves the functionality soley available in the + `BumpForceCloseFee` which moves the functionality solely available in the `lncli` to LND hence making it more universal. * [The `walletrpc.FundPsbt` RPC method now has an option to specify the fee as