improve speed of retrieval of payments

cmd/commands/cmd_payments.go

@@ -1497,6 +1497,11 @@ var listPaymentsCommand = cli.Command{
 				"payments with creation date less than or " +
 				"equal to it",
 		},
+		cli.BoolFlag{
+			Name: "omit_hops",
+			Usage: "if set, omit hop-level route data to " +
+				"reduce query cost and response size",
+		},
 	},
 	Action: actionDecorator(listPayments),
 }
@@ -1514,6 +1519,7 @@ func listPayments(ctx *cli.Context) error {
 		CountTotalPayments: ctx.Bool("count_total_payments"),
 		CreationDateStart:  ctx.Uint64("creation_date_start"),
 		CreationDateEnd:    ctx.Uint64("creation_date_end"),
+		OmitHops:           ctx.Bool("omit_hops"),
 	}
 
 	payments, err := client.ListPayments(ctxc, req)

config_builder.go

@@ -1134,6 +1134,16 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 
 				// Set the invoice bucket tombstone to indicate
 				// that the migration has been completed.
+				//
+				// TODO(ziggie): The tombstone is currently
+				// set inside the SQL transaction callback,
+				// which is fragile: if the SQL transaction
+				// is retried (e.g. on a serialization
+				// error), the KV tombstone is written before
+				// the SQL commit is confirmed. Move this to
+				// run after ApplyAllMigrations returns so
+				// the tombstone is only set once the
+				// migration is durably committed.
 				d.logger.Debugf("Setting invoice bucket " +
 					"tombstone")
 
@@ -1173,15 +1183,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 						"payments to SQL: %w", err)
 				}
 
-				// Set the payments bucket tombstone to
-				// indicate that the migration has been
-				// completed.
-				d.logger.Debugf("Setting payments bucket " +
-					"tombstone")
-
-				return paymentsdb.SetPaymentsBucketTombstone(
-					dbs.ChanStateDB.Backend,
-				)
+				return nil
 			}
 
 			// Make sure we attach the custom migration function to
@@ -1260,6 +1262,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 			graphExecutor, graphDBOptions...,
 		)
 		if err != nil {
+			cleanUp()
 			err = fmt.Errorf("unable to get graph store: %w", err)
 			d.logger.Error(err)
 
@@ -1275,6 +1278,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 			baseDB, dbs.ChanStateDB.Backend,
 		)
 		if err != nil {
+			cleanUp()
 			err = fmt.Errorf("unable to get payments store: %w",
 				err)
 
@@ -1286,49 +1290,35 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 		// Check if the invoice bucket tombstone is set. If it is, we
 		// need to return and ask the user switch back to using the
 		// native SQL store.
+		//
+		// NOTE: The invoice bucket tombstone acts as the system-wide
+		// guard against switching back to KV mode.
 		ripInvoices, err := dbs.ChanStateDB.GetInvoiceBucketTombstone()
 		if err != nil {
+			cleanUp()
 			err = fmt.Errorf("unable to check invoice bucket "+
 				"tombstone: %w", err)
 			d.logger.Error(err)
 
 			return nil, nil, err
 		}
 		if ripInvoices {
+			cleanUp()
 			err = fmt.Errorf("invoices bucket tombstoned, please " +
 				"switch back to native SQL")
 			d.logger.Error(err)
 
 			return nil, nil, err
 		}
 
-		// Check if the payments bucket tombstone is set. If it is, we
-		// need to return and ask the user switch back to using the
-		// native SQL store.
-		ripPayments, err := paymentsdb.GetPaymentsBucketTombstone(
-			dbs.ChanStateDB.Backend,
-		)
-		if err != nil {
-			err = fmt.Errorf("unable to check payments bucket "+
-				"tombstone: %w", err)
-			d.logger.Error(err)
-
-			return nil, nil, err
-		}
-		if ripPayments {
-			err = fmt.Errorf("payments bucket tombstoned, please " +
-				"switch back to native SQL")
-			d.logger.Error(err)
-
-			return nil, nil, err
-		}
-
 		dbs.InvoiceDB = dbs.ChanStateDB
 
 		graphStore, err = graphdb.NewKVStore(
 			databaseBackends.GraphDB, graphDBOptions...,
 		)
 		if err != nil {
+			cleanUp()
+
 			return nil, nil, err
 		}
 

docs/release-notes/release-notes-0.21.0.md

@@ -177,6 +177,17 @@
 
 ## Deprecations
 
+### ⚠️ **Warning:** Deprecated fields in `lnrpc.Hop` will be removed in release version **0.22**
+
+  The following deprecated fields in the [`lnrpc.Hop`](https://lightning.engineering/api-docs/api/lnd/lightning/send-to-route-sync/#lnrpchop)
+  message will be removed:
+
+  | Field | Deprecated Since | Replacement |
+  |-------|------------------|-------------|
+  | `chan_capacity` | 0.7.1 | None |
+  | `amt_to_forward` | 0.7.1 | `amt_to_forward_msat` |
+  | `fee` | 0.7.1 | `fee_msat` |
+
 ### ⚠️ **Warning:** The deprecated fee rate option `--sat_per_byte` will be removed in release version **0.22**
 
   The deprecated `--sat_per_byte` option will be fully removed. This flag was
@@ -247,6 +258,12 @@
     migration](https://github.com/lightningnetwork/lnd/pull/10485) with
     comprehensive tests. The migration is currently dev-only, compiled behind
     the `test_db_postgres`, `test_db_sqlite`, or `test_native_sql` build tags.
+  * Various [SQL payment store
+    improvements](https://github.com/lightningnetwork/lnd/pull/10535):
+    optimize schema indexes, improve query performance for payment filtering
+    and failed attempt cleanup, fix cross-database timestamp handling, add
+    `omit_hops` option to `ListPayments` to reduce response size, and increase
+    the default SQLite cache size.
 
 
 ## Code Health

lnrpc/lightning.proto

@@ -4554,6 +4554,10 @@ message ListPaymentsRequest {
     // If set, returns all payments with a creation date less than or equal to
     // it. Measured in seconds since the unix epoch.
     uint64 creation_date_end = 7;
+
+    // If set, omit hop-level route data for HTLC attempts to reduce query
+    // cost and response size.
+    bool omit_hops = 8;
 }
 
 message ListPaymentsResponse {

lnrpc/lightning.swagger.json

@@ -2449,6 +2449,13 @@
             "required": false,
             "type": "string",
             "format": "uint64"
+          },
+          {
+            "name": "omit_hops",
+            "description": "If set, omit hop-level route data for HTLC attempts to reduce query\ncost and response size.",
+            "in": "query",
+            "required": false,
+            "type": "boolean"
           }
         ],
         "tags": [

lnrpc/routerrpc/router_backend.go

@@ -655,16 +655,11 @@ func (r *RouterBackend) MarshallRoute(route *route.Route) (*lnrpc.Route, error)
 	for i, hop := range route.Hops {
 		fee := route.HopFee(i)
 
-		// Channel capacity is not a defining property of a route. For
-		// backwards RPC compatibility, we retrieve it here from the
-		// graph.
-		chanCapacity, err := r.FetchChannelCapacity(hop.ChannelID)
-		if err != nil {
-			// If capacity cannot be retrieved, this may be a
-			// not-yet-received or private channel. Then report
-			// amount that is sent through the channel as capacity.
-			chanCapacity = incomingAmt.ToSatoshis()
-		}
+		// Avoid per-hop graph lookups by using the incoming amount as a
+		// lower bound for the capacity. This is not the actual channel
+		// capacity, but it is a reasonable approximation that avoids
+		// slow graph lookups and works for closed/private channels too.
+		chanCapacity := incomingAmt.ToSatoshis()
 
 		// Extract the MPP fields if present on this hop.
 		var mpp *lnrpc.MPPRecord
@@ -713,6 +708,7 @@ func (r *RouterBackend) MarshallRoute(route *route.Route) (*lnrpc.Route, error)
 			blinding := hop.BlindingPoint.SerializeCompressed()
 			resp.Hops[i].BlindingPoint = blinding
 		}
+
 		incomingAmt = hop.AmtToForward
 	}
 

lnrpc/routerrpc/router_backend_test.go

@@ -937,3 +937,55 @@ func TestExtractIntentFromSendRequest(t *testing.T) {
 		})
 	}
 }
+
+// TestMarshallRouteChanCapacity verifies that MarshallRoute correctly sets the
+// ChanCapacity for each hop based on the incoming amount at that hop, not
+// the total route amount. This is a regression test to ensure the
+// incomingAmt is updated per hop.
+func TestMarshallRouteChanCapacity(t *testing.T) {
+	t.Parallel()
+
+	// Build a two-hop route: source -> hop1 -> hop2 -> dest.
+	//
+	// TotalAmount (incoming to hop1) = 1000 msat
+	// hop1.AmtToForward (incoming to hop2) = 900 msat (after fee)
+	const (
+		totalAmtMsat = lnwire.MilliSatoshi(1000)
+		hop1Forward  = lnwire.MilliSatoshi(900)
+		hop2Forward  = lnwire.MilliSatoshi(900)
+	)
+
+	hops := []*route.Hop{
+		{
+			ChannelID:    1,
+			AmtToForward: hop1Forward,
+			PubKeyBytes:  node1,
+		},
+		{
+			ChannelID:    2,
+			AmtToForward: hop2Forward,
+			PubKeyBytes:  node2,
+		},
+	}
+
+	r, err := route.NewRouteFromHops(totalAmtMsat, 100, sourceKey, hops)
+	require.NoError(t, err)
+
+	backend := &RouterBackend{}
+	rpcRoute, err := backend.MarshallRoute(r)
+	require.NoError(t, err)
+	require.Len(t, rpcRoute.Hops, 2)
+
+	// The first hop's capacity should reflect the total incoming amount
+	// (route.TotalAmount), converted to satoshis.
+	require.EqualValues(
+		t, totalAmtMsat.ToSatoshis(), rpcRoute.Hops[0].ChanCapacity,
+	)
+
+	// The second hop's capacity should reflect hop1's forwarded amount, not
+	// the total route amount. Before the fix, both hops incorrectly used
+	// the total route amount.
+	require.EqualValues(
+		t, hop1Forward.ToSatoshis(), rpcRoute.Hops[1].ChanCapacity,
+	)
+}

payments/db/kv_duplicate_payments.go

@@ -228,7 +228,7 @@ func fetchDuplicatePayments(paymentHashBucket kvdb.RBucket) ([]*MPPayment,
 		subBucket := dup.NestedReadBucket(k)
 		if subBucket == nil {
 			// We one bucket for each duplicate to be found.
-			return fmt.Errorf("non bucket element" +
+			return fmt.Errorf("non bucket element " +
 				"in duplicate bucket")
 		}