diff --git a/docs/release-notes/release-notes-0.21.0.md b/docs/release-notes/release-notes-0.21.0.md index 23b93a0fd0d..f7b99398579 100644 --- a/docs/release-notes/release-notes-0.21.0.md +++ b/docs/release-notes/release-notes-0.21.0.md @@ -88,6 +88,10 @@ RBF close state machine does not yet thread through the `AuxCloser` hook that overlay channels rely on to build aux-aware close transactions. +* [Fixed `EstimateRouteFee`](https://github.com/lightningnetwork/lnd/pull/10771) + to use independent probe payment hashes when probing multiple LSPs, preventing + later probes from reusing the first probe's CLTV delta. + # New Features - [Basic Support](https://github.com/lightningnetwork/lnd/pull/9868) for onion diff --git a/lnrpc/routerrpc/router_server.go b/lnrpc/routerrpc/router_server.go index e3ac207e597..43981764449 100644 --- a/lnrpc/routerrpc/router_server.go +++ b/lnrpc/routerrpc/router_server.go @@ -524,6 +524,25 @@ func (s *Server) probeDestination(dest []byte, amtSat int64) (*RouteFeeResponse, func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string, timeout uint32) (*RouteFeeResponse, error) { + return s.probePaymentRequestWithSender( + ctx, paymentRequest, timeout, s.sendProbePayment, + ) +} + +// probePaymentSender dispatches a probe payment request and returns the +// resulting fee estimate. It exists as a test seam so tests can inject a stub +// sender and inspect generated probe requests without running the payment +// lifecycle. +type probePaymentSender func(context.Context, + *SendPaymentRequest) (*RouteFeeResponse, error) + +// probePaymentRequestWithSender contains the implementation of +// probePaymentRequest. The sender is injected so tests can inspect generated +// probe requests without invoking the full payment lifecycle. +func (s *Server) probePaymentRequestWithSender(ctx context.Context, + paymentRequest string, timeout uint32, + sendProbePayment probePaymentSender) (*RouteFeeResponse, error) { + payReq, err := zpay32.Decode( paymentRequest, s.cfg.RouterBackend.ActiveNetParams, ) @@ -576,7 +595,8 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string, probeRequest.Dest) probeRequest.RouteHints = invoicesrpc.CreateRPCRouteHints(hints) - return s.sendProbePayment(ctx, probeRequest) + + return sendProbePayment(ctx, probeRequest) } // If the heuristic indicates an LSP, we filter and group route hints by @@ -612,6 +632,16 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string, lspHint := group.LspHopHint + // Each LSP probe must use a unique payment hash, otherwise the + // payment lifecycle will treat later probes as attempts on the + // first probe's payment and reuse its payment-level parameters. + var lspPaymentHash lntypes.Hash + _, err := crand.Read(lspPaymentHash[:]) + if err != nil { + return nil, fmt.Errorf("cannot generate random probe "+ + "preimage: %w", err) + } + log.Infof("Probing LSP with destination: %v", lspKey) // Create a new probe request for this LSP. @@ -621,7 +651,7 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string, MaxParts: probeRequest.MaxParts, AllowSelfPayment: probeRequest.AllowSelfPayment, AmtMsat: amtMsat, - PaymentHash: probeRequest.PaymentHash, + PaymentHash: lspPaymentHash[:], FeeLimitSat: probeRequest.FeeLimitSat, FinalCltvDelta: int32(lspHint.CLTVExpiryDelta), DestFeatures: probeRequest.DestFeatures, @@ -652,7 +682,7 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string, lspProbeRequest.AmtMsat += int64(hopFee) // Dispatch the payment probe for this LSP. - resp, err := s.sendProbePayment(ctx, lspProbeRequest) + resp, err := sendProbePayment(ctx, lspProbeRequest) if err != nil { log.Warnf("Failed to probe LSP %v: %v", lspKey, err) continue diff --git a/lnrpc/routerrpc/router_server_test.go b/lnrpc/routerrpc/router_server_test.go index a46a129400e..08bdbe29869 100644 --- a/lnrpc/routerrpc/router_server_test.go +++ b/lnrpc/routerrpc/router_server_test.go @@ -7,6 +7,9 @@ import ( "time" "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcec/v2/ecdsa" + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnwire" paymentsdb "github.com/lightningnetwork/lnd/payments/db" @@ -764,3 +767,135 @@ func TestPrepareLspRouteHints(t *testing.T) { require.Contains(t, err.Error(), "no public LSP nodes found") }) } + +// TestProbePaymentRequestUsesUniqueHashPerLSP verifies that each LSP probe is +// isolated from the other probes by using a distinct payment hash. +func TestProbePaymentRequestUsesUniqueHashPerLSP(t *testing.T) { + // Arrange: create three public LSPs and an invoice whose private + // destination can be reached through any of them. + destPrivKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + bobPrivKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + bobPubKey := bobPrivKey.PubKey() + + evePrivKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + evePubKey := evePrivKey.PubKey() + + davePrivKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + davePubKey := davePrivKey.PubKey() + + bobVertex := route.NewVertex(bobPubKey) + eveVertex := route.NewVertex(evePubKey) + daveVertex := route.NewVertex(davePubKey) + + publicNodes := map[route.Vertex]struct{}{ + bobVertex: {}, + eveVertex: {}, + daveVertex: {}, + } + hasNode := func(nodePub route.Vertex) (bool, error) { + _, ok := publicNodes[nodePub] + + return ok, nil + } + + server := &Server{ + cfg: &Config{ + RouterBackend: &RouterBackend{ + ActiveNetParams: &chaincfg.RegressionNetParams, + HasNode: hasNode, + }, + }, + } + + lspHint := func(pubKey *btcec.PublicKey, chanID uint64, + cltv uint16) zpay32.HopHint { + + return zpay32.HopHint{ + NodeID: pubKey, + ChannelID: chanID, + FeeBaseMSat: uint32(chanID * 1_000), + FeeProportionalMillionths: uint32(chanID), + CLTVExpiryDelta: cltv, + } + } + + bobHint := lspHint(bobPubKey, 1, 100) + eveHint := lspHint(evePubKey, 2, 200) + daveHint := lspHint(davePubKey, 3, 120) + + var paymentHash [32]byte + paymentHash[0] = 1 + invoice, err := zpay32.NewInvoice( + &chaincfg.RegressionNetParams, paymentHash, time.Unix(1, 0), + zpay32.Amount(lnwire.MilliSatoshi(100_000)), + zpay32.Description("multi lsp probe"), + zpay32.Destination(destPrivKey.PubKey()), + zpay32.RouteHint([]zpay32.HopHint{bobHint}), + zpay32.RouteHint([]zpay32.HopHint{eveHint}), + zpay32.RouteHint([]zpay32.HopHint{daveHint}), + ) + require.NoError(t, err) + + signer := zpay32.MessageSigner{ + SignCompact: func(msg []byte) ([]byte, error) { + hash := chainhash.HashB(msg) + + return ecdsa.SignCompact(destPrivKey, hash, true), nil + }, + } + payReq, err := invoice.Encode(signer) + require.NoError(t, err) + + seenHashes := make(map[[32]byte]struct{}) + probedDests := make(map[route.Vertex]struct{}) + expectedCltv := map[route.Vertex]int32{ + bobVertex: int32(bobHint.CLTVExpiryDelta), + eveVertex: int32(eveHint.CLTVExpiryDelta), + daveVertex: int32(daveHint.CLTVExpiryDelta), + } + + sendProbe := func(_ context.Context, + req *SendPaymentRequest) (*RouteFeeResponse, error) { + + require.Len(t, req.PaymentHash, 32) + + var reqHash [32]byte + copy(reqHash[:], req.PaymentHash) + _, hashExists := seenHashes[reqHash] + require.False(t, hashExists, "reused payment hash") + seenHashes[reqHash] = struct{}{} + + var dest route.Vertex + copy(dest[:], req.Dest) + probedDests[dest] = struct{}{} + + require.Equal(t, expectedCltv[dest], req.FinalCltvDelta) + + return &RouteFeeResponse{ + RoutingFeeMsat: int64(req.FinalCltvDelta), + TimeLockDelay: int64(req.FinalCltvDelta), + FailureReason: lnrpc. + PaymentFailureReason_FAILURE_REASON_NONE, + }, nil + } + + // Act: estimate the route fee with a stubbed probe sender that records + // the generated per-LSP probe requests. + _, err = server.probePaymentRequestWithSender( + t.Context(), payReq, 1, sendProbe, + ) + + // Assert: all LSPs were probed, each probe had a unique payment hash, + // and the probe request kept the CLTV delta for its target LSP. + require.NoError(t, err) + require.Len(t, seenHashes, MaxLspsToProbe) + require.Len(t, probedDests, MaxLspsToProbe) + require.Contains(t, probedDests, bobVertex) + require.Contains(t, probedDests, eveVertex) + require.Contains(t, probedDests, daveVertex) +}