xpay: add maxdelay parameter

michael1011 · michael1011 · commit b302d3b4a023 · 2024-12-27T11:31:42.000+01:00
Changelog-Added: Plugins: `xpay` now supports a `maxdelay` parameter for better `xpay-handle-pay` compatibility.
diff --git a/.msggen.json b/.msggen.json
@@ -1772,6 +1772,7 @@
             "GetRoutes.final_cltv": 7,
             "GetRoutes.finalcltv": 6,
             "GetRoutes.layers[]": 4,
+            "GetRoutes.maxdelay": 8,
             "GetRoutes.maxfee_msat": 5,
             "GetRoutes.source": 1
         },
@@ -3728,6 +3729,7 @@
             "Xpay.amount_msat": 2,
             "Xpay.invstring": 1,
             "Xpay.layers[]": 4,
+            "Xpay.maxdelay": 7,
             "Xpay.maxfee": 3,
             "Xpay.partial_msat": 6,
             "Xpay.retry_for": 5
@@ -7317,6 +7319,10 @@
             "added": "v24.08",
             "deprecated": null
         },
+        "GetRoutes.maxdelay": {
+            "added": "v25.02",
+            "deprecated": null
+        },
         "GetRoutes.maxfee_msat": {
             "added": "v24.08",
             "deprecated": null
@@ -12969,6 +12975,10 @@
             "added": "v24.11",
             "deprecated": null
         },
+        "Xpay.maxdelay": {
+            "added": "v25.02",
+            "deprecated": null
+        },
         "Xpay.maxfee": {
             "added": "v24.11",
             "deprecated": null
diff --git a/cln-grpc/proto/node.proto b/cln-grpc/proto/node.proto
diff --git a/cln-grpc/src/convert.rs b/cln-grpc/src/convert.rs
diff --git a/cln-rpc/src/model.rs b/cln-rpc/src/model.rs
diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json
@@ -15827,6 +15827,14 @@
             "description": [
               "Number of blocks for the final node.  We need to know this because no HTLC is allowed to have a CLTV delay more than 2016 blocks."
             ]
+          },
+          "maxdelay": {
+            "type": "u32",
+            "added": "v25.02",
+            "description": [
+              "Maximum number of blocks of delay for the route. Cannot be bigger than 2016."
+            ],
+            "default": "2016"
           }
         }
       },
@@ -36353,6 +36361,14 @@
             "description": [
               "Explicitly state that you are only paying some part of the invoice.  Presumably someone else is paying the rest (otherwise the payment will time out at the recipient)."
             ]
+          },
+          "maxdelay": {
+            "type": "u32",
+            "added": "v25.02",
+            "description": [
+              "A payment may be delayed for up to `maxdelay` blocks by another node; clients should be prepared for this worst case."
+            ],
+            "default": "2016"
           }
         }
       },
diff --git a/contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py b/contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py
diff --git a/doc/schemas/lightning-getroutes.json b/doc/schemas/lightning-getroutes.json
@@ -65,6 +65,14 @@
         "description": [
           "Number of blocks for the final node.  We need to know this because no HTLC is allowed to have a CLTV delay more than 2016 blocks."
         ]
+      },
+      "maxdelay": {
+        "type": "u32",
+        "added": "v25.02",
+        "description": [
+          "Maximum number of blocks of delay for the route. Cannot be bigger than 2016."
+        ],
+        "default": "2016"
       }
     }
   },
diff --git a/doc/schemas/lightning-xpay.json b/doc/schemas/lightning-xpay.json
@@ -57,6 +57,14 @@
         "description": [
           "Explicitly state that you are only paying some part of the invoice.  Presumably someone else is paying the rest (otherwise the payment will time out at the recipient)."
         ]
+      },
+      "maxdelay": {
+        "type": "u32",
+        "added": "v25.02",
+        "description": [
+          "A payment may be delayed for up to `maxdelay` blocks by another node; clients should be prepared for this worst case."
+        ],
+        "default": "2016"
       }
     }
   },
diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c
@@ -323,6 +323,7 @@ static const char *get_routes(const tal_t *ctx,
 			      struct amount_msat amount,
 			      struct amount_msat maxfee,
 			      u32 finalcltv,
+			      u32 maxdelay,
 			      const char **layers,
 			      struct gossmap_localmods *localmods,
 			      const struct layer *local_layer,
@@ -418,18 +419,11 @@ static const char *get_routes(const tal_t *ctx,
 	}
 
 	/* Too much delay? */
-	/* BOLT #4:
-	 * ## `max_htlc_cltv` Selection
-	 *
-	 * This ... value is defined as 2016 blocks, based on historical value
-	 * deployed by Lightning implementations.
-	 */
-	/* FIXME: Typo in spec for CLTV in descripton!  But it breaks our spelling check, so we omit it above */
-	while (finalcltv + flows_worst_delay(flows) > 2016) {
+	while (finalcltv + flows_worst_delay(flows) > maxdelay) {
 		delay_feefactor *= 2;
 		rq_log(tmpctx, rq, LOG_UNUSUAL,
 		       "The worst flow delay is %"PRIu64" (> %i), retrying with delay_feefactor %f...",
-		       flows_worst_delay(flows), 2016 - finalcltv, delay_feefactor);
+		       flows_worst_delay(flows), maxdelay - finalcltv, delay_feefactor);
 		flows = minflow(rq, rq, srcnode, dstnode, amount,
 				mu, delay_feefactor);
 		if (!flows || delay_feefactor > 10) {
@@ -487,7 +481,7 @@ static const char *get_routes(const tal_t *ctx,
 		flows = new_flows;
 	}
 
-	if (finalcltv + flows_worst_delay(flows) > 2016) {
+	if (finalcltv + flows_worst_delay(flows) > maxdelay) {
 		ret = rq_log(ctx, rq, LOG_UNUSUAL,
 			     "Could not find route without excessive cost or delays");
 		goto fail;
@@ -601,7 +595,7 @@ struct getroutes_info {
 	struct command *cmd;
 	struct node_id *source, *dest;
 	struct amount_msat *amount, *maxfee;
-	u32 *finalcltv;
+	u32 *finalcltv, *maxdelay;
 	const char **layers;
 	struct additional_cost_htable *additional_costs;
 	/* Non-NULL if we are told to use "auto.localchans" */
@@ -621,7 +615,7 @@ static struct command_result *do_getroutes(struct command *cmd,
 	err = get_routes(cmd, cmd,
 			 info->source, info->dest,
 			 *info->amount, *info->maxfee, *info->finalcltv,
-			 info->layers, localmods, info->local_layer,
+			 *info->maxdelay, info->layers, localmods, info->local_layer,
 			 &routes, &amounts, info->additional_costs, &probability);
 	if (err)
 		return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "%s", err);
@@ -751,6 +745,14 @@ static struct command_result *json_getroutes(struct command *cmd,
 					     const char *buffer,
 					     const jsmntok_t *params)
 {
+	/* BOLT #4:
+	 * ## `max_htlc_cltv` Selection
+	 *
+	 * This ... value is defined as 2016 blocks, based on
+	 * historical value deployed by Lightning implementations.
+	 */
+	/* FIXME: Typo in spec for CLTV in descripton! But it breaks our spelling check, so we omit it above */
+	const u32 maxdelay_allowed = 2016;
 	struct getroutes_info *info = tal(cmd, struct getroutes_info);
 
 	if (!param(cmd, buffer, params,
@@ -760,9 +762,17 @@ static struct command_result *json_getroutes(struct command *cmd,
 		   p_req("layers", param_layer_names, &info->layers),
 		   p_req("maxfee_msat", param_msat, &info->maxfee),
 		   p_req("final_cltv", param_u32, &info->finalcltv),
+		   p_opt_def("maxdelay", param_u32, &info->maxdelay,
+			     maxdelay_allowed),
 		   NULL))
 		return command_param_failed();
 
+	if (*info->maxdelay > maxdelay_allowed) {
+		return command_fail(cmd, PAY_USER_ERROR,
+				    "maximum delay allowed is %d",
+				    maxdelay_allowed);
+	}
+
 	info->cmd = cmd;
 	info->additional_costs = tal(info, struct additional_cost_htable);
 	additional_cost_htable_init(info->additional_costs);
diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c
@@ -73,6 +73,8 @@ struct payment {
 	struct amount_msat full_amount;
 	/* Maximum fee we're prepare to pay */
 	struct amount_msat maxfee;
+	/* Maximum delay on the route we're ok with */
+	u32 *maxdelay;
 	/* BOLT11 payment secret (NULL for BOLT12, it uses blinded paths) */
 	const struct secret *payment_secret;
 	/* BOLT11 payment metadata (NULL for BOLT12, it uses blinded paths) */
@@ -1186,6 +1188,7 @@ static struct command_result *getroutes_for(struct command *aux_cmd,
 	json_array_end(req->js);
 	json_add_amount_msat(req->js, "maxfee_msat", maxfee);
 	json_add_u32(req->js, "final_cltv", payment->final_cltv);
+	json_add_u32(req->js, "maxdelay", *payment->maxdelay);
 
 	return send_payment_req(aux_cmd, payment, req);
 }
@@ -1470,6 +1473,7 @@ static struct command_result *json_xpay_core(struct command *cmd,
 			 p_opt("layers", param_string_array, &payment->layers),
 			 p_opt_def("retry_for", param_number, &retryfor, 60),
 			 p_opt("partial_msat", param_msat, &partial),
+			 p_opt_def("maxdelay", param_u32, &payment->maxdelay, 2016),
 			 NULL))
 		return command_param_failed();
 
@@ -1884,7 +1888,7 @@ static struct command_result *handle_rpc_command(struct command *cmd,
 	struct xpay *xpay = xpay_of(cmd->plugin);
 	const jsmntok_t *rpc_tok, *method_tok, *params_tok, *id_tok,
 		*bolt11 = NULL, *amount_msat = NULL,
-		*partial_msat = NULL, *retry_for = NULL;
+		*partial_msat = NULL, *retry_for = NULL, *maxdelay = NULL;
 	const char *maxfee = NULL;
 	struct json_stream *response;
 
@@ -1933,6 +1937,8 @@ static struct command_result *handle_rpc_command(struct command *cmd,
 				maxfeepercent = t + 1;
 			else if (json_tok_streq(buf, t, "exemptfee"))
 				exemptfee = t + 1;
+			else if (json_tok_streq(buf, t, "maxdelay"))
+				maxdelay = t + 1;
 			else {
 				plugin_log(cmd->plugin, LOG_INFORM,
 					   "Not redirecting pay (unknown arg %.*s)",
@@ -1978,6 +1984,8 @@ static struct command_result *handle_rpc_command(struct command *cmd,
 		json_add_string(response, "maxfee", maxfee);
 	if (partial_msat)
 		json_add_tok(response, "partial_msat", partial_msat, buf);
+	if (maxdelay)
+		json_add_tok(response, "maxdelay", maxdelay, buf);
 	json_object_end(response);
 	json_object_end(response);
 	return command_finished(cmd, response);
diff --git a/tests/test_askrene.py b/tests/test_askrene.py
@@ -679,6 +679,55 @@ def test_getroutes_auto_sourcefree(node_factory):
                          final_cltv=99)
 
 
+def test_getroutes_maxdelay(node_factory):
+    gsfile, nodemap = generate_gossip_store([GenChannel(0, 1, forward=GenChannel.Half(propfee=10000, delay=80)),
+                                             GenChannel(0, 1, forward=GenChannel.Half(propfee=10001, delay=40))])
+
+    # Set up l1 with this as the gossip_store
+    l1 = node_factory.get_node(gossip_store_file=gsfile.name)
+
+    # Should prefer the cheaper channel
+    assert l1.rpc.getroutes(source=nodemap[0],
+                            destination=nodemap[1],
+                            amount_msat=1000,
+                            layers=[],
+                            maxfee_msat=1000,
+                            final_cltv=99) == {'probability_ppm': 999999,
+                                               'routes': [{'probability_ppm': 999999,
+                                                           'final_cltv': 99,
+                                                           'amount_msat': 1000,
+                                                           'path': [{'short_channel_id_dir': '0x1x0/1',
+                                                                     'next_node_id': nodemap[1],
+                                                                     'amount_msat': 1010,
+                                                                     'delay': 179}]}]}
+
+    # But use the channel with lower delay when needed
+    assert l1.rpc.getroutes(source=nodemap[0],
+                            destination=nodemap[1],
+                            amount_msat=1000,
+                            layers=[],
+                            maxfee_msat=2000,
+                            final_cltv=99,
+                            maxdelay=170) == {'probability_ppm': 999999,
+                                              'routes': [{'probability_ppm': 999999,
+                                                          'final_cltv': 99,
+                                                          'amount_msat': 1000,
+                                                          'path': [{'short_channel_id_dir': '0x1x1/1',
+                                                                    'next_node_id': nodemap[1],
+                                                                    'amount_msat': 1010,
+                                                                    'delay': 139}]}]}
+
+    # Excessive maxdelay parameter
+    with pytest.raises(RpcError, match="maximum delay allowed is 2016"):
+        l1.rpc.getroutes(source=nodemap[0],
+                         destination=nodemap[1],
+                         amount_msat=100000,
+                         layers=[],
+                         maxfee_msat=100,
+                         final_cltv=99,
+                         maxdelay=2017)
+
+
 def test_getroutes_auto_localchans(node_factory):
     """Test getroutes call with auto.localchans layer"""
     l1 = node_factory.get_node()
diff --git a/tests/test_xpay.py b/tests/test_xpay.py
@@ -433,10 +433,6 @@ def test_xpay_takeover(node_factory, executor):
     l1.rpc.pay(inv['bolt11'], amount_msat=10000, riskfactor=1)
     l1.daemon.wait_for_log(r'Not redirecting pay \(unknown arg \\"riskfactor\\"\)')
 
-    inv = l3.rpc.invoice('any', "test_xpay_takeover10", "test_xpay_takeover10")
-    l1.rpc.pay(inv['bolt11'], amount_msat=10000, maxdelay=200)
-    l1.daemon.wait_for_log(r'Not redirecting pay \(unknown arg \\"maxdelay\\"\)')
-
     # Test that it's really dynamic.
     l1.rpc.setconfig('xpay-handle-pay', False)
 
@@ -545,6 +541,15 @@ def test_xpay_maxfee(node_factory, bitcoind, chainparams):
     assert fee <= maxfee
 
 
+def test_xpay_maxdelay(node_factory):
+    l1, l2 = node_factory.line_graph(2, wait_for_announce=True)
+
+    inv = l2.rpc.invoice('10000msat', 'test_xpay_maxdelay', 'test_xpay_maxdelay')["bolt11"]
+
+    with pytest.raises(RpcError, match=r"Could not find route without excessive delays"):
+        l1.rpc.xpay(invstring=inv, maxdelay=1)
+
+
 def test_xpay_unannounced(node_factory):
     l1, l2 = node_factory.line_graph(2, announce_channels=False)
 

Original file line number	Diff line number	Diff line change
`@@ -15827,6 +15827,14 @@`
`15827`	`15827`	`"description": [`
`15828`	`15828`	`"Number of blocks for the final node. We need to know this because no HTLC is allowed to have a CLTV delay more than 2016 blocks."`
`15829`	`15829`	`]`
	`15830`	`+ },`
	`15831`	`+ "maxdelay": {`
	`15832`	`+ "type": "u32",`
	`15833`	`+ "added": "v25.02",`
	`15834`	`+ "description": [`
	`15835`	`+ "Maximum number of blocks of delay for the route. Cannot be bigger than 2016."`
	`15836`	`+ ],`
	`15837`	`+ "default": "2016"`
`15830`	`15838`	`}`
`15831`	`15839`	`}`
`15832`	`15840`	`},`
`@@ -36353,6 +36361,14 @@`
`36353`	`36361`	`"description": [`
`36354`	`36362`	`"Explicitly state that you are only paying some part of the invoice. Presumably someone else is paying the rest (otherwise the payment will time out at the recipient)."`
`36355`	`36363`	`]`
	`36364`	`+ },`
	`36365`	`+ "maxdelay": {`
	`36366`	`+ "type": "u32",`
	`36367`	`+ "added": "v25.02",`
	`36368`	`+ "description": [`
	`36369`	+ "A payment may be delayed for up to `maxdelay` blocks by another node; clients should be prepared for this worst case."
	`36370`	`+ ],`
	`36371`	`+ "default": "2016"`
`36356`	`36372`	`}`
`36357`	`36373`	`}`
`36358`	`36374`	`},`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,14 @@`
`65`	`65`	`"description": [`
`66`	`66`	`"Number of blocks for the final node. We need to know this because no HTLC is allowed to have a CLTV delay more than 2016 blocks."`
`67`	`67`	`]`
	`68`	`+ },`
	`69`	`+ "maxdelay": {`
	`70`	`+ "type": "u32",`
	`71`	`+ "added": "v25.02",`
	`72`	`+ "description": [`
	`73`	`+ "Maximum number of blocks of delay for the route. Cannot be bigger than 2016."`
	`74`	`+ ],`
	`75`	`+ "default": "2016"`
`68`	`76`	`}`
`69`	`77`	`}`
`70`	`78`	`},`
Original file line number	Diff line number	Diff line change
`@@ -57,6 +57,14 @@`
`57`	`57`	`"description": [`
`58`	`58`	`"Explicitly state that you are only paying some part of the invoice. Presumably someone else is paying the rest (otherwise the payment will time out at the recipient)."`
`59`	`59`	`]`
	`60`	`+ },`
	`61`	`+ "maxdelay": {`
	`62`	`+ "type": "u32",`
	`63`	`+ "added": "v25.02",`
	`64`	`+ "description": [`
	`65`	+ "A payment may be delayed for up to `maxdelay` blocks by another node; clients should be prepared for this worst case."
	`66`	`+ ],`
	`67`	`+ "default": "2016"`
`60`	`68`	`}`
`61`	`69`	`}`
`62`	`70`	`},`