askrene: remove non child-friendly fields from struct route_query.

Notably no access to the struct command and struct plugin.

Note: we actually *do* mess with askrene->reserves, but the previous code
used cmd to get to it.  Now we need to include a non-const pointer in
struct route_query.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Author: rustyrussell

plugins/askrene/askrene.c

@@ -301,11 +301,12 @@ struct amount_msat get_additional_per_htlc_cost(const struct route_query *rq,
 		return AMOUNT_MSAT(0);
 }
 
-const char *rq_log(const tal_t *ctx,
-		   const struct route_query *rq,
-		   enum log_level level,
-		   const char *fmt,
-		   ...)
+PRINTF_FMT(4, 5)
+static const char *cmd_log(const tal_t *ctx,
+			   struct command *cmd,
+			   enum log_level level,
+			   const char *fmt,
+			   ...)
 {
 	va_list args;
 	const char *msg;
@@ -314,14 +315,14 @@ const char *rq_log(const tal_t *ctx,
 	msg = tal_vfmt(ctx, fmt, args);
 	va_end(args);
 
-	plugin_notify_message(rq->cmd, level, "%s", msg);
+	plugin_notify_message(cmd, level, "%s", msg);
 
 	/* Notifications already get logged at debug. Otherwise reduce
 	 * severity. */
 	if (level != LOG_DBG)
-		plugin_log(rq->plugin,
+		plugin_log(cmd->plugin,
 			   level == LOG_BROKEN ? level : level - 1,
-			   "%s: %s", rq->cmd->id, msg);
+			   "%s: %s", cmd->id, msg);
 	return msg;
 }
 
@@ -363,7 +364,9 @@ struct getroutes_info {
 	u32 maxparts;
 };
 
-static void apply_layers(struct askrene *askrene, struct route_query *rq,
+static void apply_layers(struct askrene *askrene,
+			 struct command *cmd,
+			 struct route_query *rq,
 			 const struct node_id *source,
 			 struct amount_msat amount,
 			 struct gossmap_localmods *localmods,
@@ -375,14 +378,17 @@ static void apply_layers(struct askrene *askrene, struct route_query *rq,
 		const struct layer *l = find_layer(askrene, layers[i]);
 		if (!l) {
 			if (streq(layers[i], "auto.localchans")) {
-				plugin_log(rq->plugin, LOG_DBG, "Adding auto.localchans");
+				cmd_log(tmpctx, cmd, LOG_DBG,
+				       "Adding auto.localchans");
 				l = local_layer;
 			} else if (streq(layers[i], "auto.no_mpp_support")) {
-				plugin_log(rq->plugin, LOG_DBG, "Adding auto.no_mpp_support, sorry");
+				cmd_log(tmpctx, cmd, LOG_DBG,
+				       "Adding auto.no_mpp_support, sorry");
 				l = remove_small_channel_layer(layers, askrene, amount, localmods);
 			} else {
 				assert(streq(layers[i], "auto.sourcefree"));
-				plugin_log(rq->plugin, LOG_DBG, "Adding auto.sourcefree");
+				cmd_log(tmpctx, cmd, LOG_DBG,
+				       "Adding auto.sourcefree");
 				l = source_free_layer(layers, askrene, source, localmods);
 			}
 		}
@@ -433,15 +439,16 @@ void get_constraints(const struct route_query *rq,
 	reserve_sub(rq->reserved, &scidd, rq->layers, max);
 }
 
-static void process_child_logs(struct route_query *rq, int log_fd)
+static void process_child_logs(struct command *cmd,
+			       int log_fd)
 {
 	u8 *msg;
 	while ((msg = wire_sync_read(tmpctx, log_fd)) != NULL) {
 		enum log_level level;
 		char *entry;
 		struct node_id *peer;
 		if (fromwire_status_log(tmpctx, msg, &level, &peer, &entry))
-			rq_log(tmpctx, rq, level, "%s", entry);
+			cmd_log(tmpctx, cmd, level, "%s", entry);
 	}
 }
 
@@ -454,6 +461,7 @@ static struct command_result *do_getroutes(struct command *cmd,
 	const char *err, *json;
 	struct timemono time_start, deadline;
 	int child_fd, log_fd, child_pid, child_status;
+	s8 *biases;
 
 	/* update the gossmap */
 	if (gossmap_refresh(askrene->gossmap)) {
@@ -464,18 +472,16 @@ static struct command_result *do_getroutes(struct command *cmd,
 	}
 
 	/* build this request structure */
-	rq->cmd = cmd;
-	rq->plugin = cmd->plugin;
+	rq->cmd_id = cmd->id;
 	rq->gossmap = askrene->gossmap;
 	rq->reserved = askrene->reserved;
 	rq->layers = tal_arr(rq, const struct layer *, 0);
 	rq->capacities = tal_dup_talarr(rq, fp16_t, askrene->capacities);
 	/* FIXME: we still need to do something useful with these */
 	rq->additional_costs = info->additional_costs;
-	rq->maxparts = info->maxparts;
 
 	/* apply selected layers to the localmods */
-	apply_layers(askrene, rq, &info->source, info->amount, localmods,
+	apply_layers(askrene, cmd, rq, &info->source, info->amount, localmods,
 		     info->layers, info->local_layer);
 
 	/* Clear scids with reservations, too, so we don't have to look up
@@ -495,18 +501,19 @@ static struct command_result *do_getroutes(struct command *cmd,
 
 	/* localmods can add channels, so we need to allocate biases array
 	 * *afterwards* */
-	rq->biases =
+	rq->biases = biases =
 	    tal_arrz(rq, s8, gossmap_max_chan_idx(askrene->gossmap) * 2);
 
 	/* Note any channel biases */
 	for (size_t i = 0; i < tal_count(rq->layers); i++)
-		layer_apply_biases(rq->layers[i], askrene->gossmap, rq->biases);
+		layer_apply_biases(rq->layers[i], askrene->gossmap, biases);
 
 	/* checkout the source */
 	const struct gossmap_node *srcnode =
 	    gossmap_find_node(askrene->gossmap, &info->source);
 	if (!srcnode) {
-		err = rq_log(tmpctx, rq, LOG_INFORM, "Unknown source node %s",
+		err = cmd_log(tmpctx, cmd, LOG_INFORM,
+			     "Unknown source node %s",
 			     fmt_node_id(tmpctx, &info->source));
 		goto fail;
 	}
@@ -515,7 +522,7 @@ static struct command_result *do_getroutes(struct command *cmd,
 	const struct gossmap_node *dstnode =
 	    gossmap_find_node(askrene->gossmap, &info->dest);
 	if (!dstnode) {
-		err = rq_log(tmpctx, rq, LOG_INFORM,
+		err = cmd_log(tmpctx, cmd, LOG_INFORM,
 			     "Unknown destination node %s",
 			     fmt_node_id(tmpctx, &info->dest));
 		goto fail;
@@ -525,14 +532,14 @@ static struct command_result *do_getroutes(struct command *cmd,
 	if (have_layer(info->layers, "auto.no_mpp_support") &&
 	    info->dev_algo != ALGO_SINGLE_PATH) {
 		info->dev_algo = ALGO_SINGLE_PATH;
-		rq_log(tmpctx, rq, LOG_DBG,
+		cmd_log(tmpctx, cmd, LOG_DBG,
 		       "Layer no_mpp_support is active we switch to a "
 		       "single path algorithm.");
 	}
-	if (rq->maxparts == 1 &&
+	if (info->maxparts == 1 &&
 	    info->dev_algo != ALGO_SINGLE_PATH) {
 		info->dev_algo = ALGO_SINGLE_PATH;
-		rq_log(tmpctx, rq, LOG_DBG,
+		cmd_log(tmpctx, cmd, LOG_DBG,
 		       "maxparts == 1: switching to a single path algorithm.");
 	}
 
@@ -542,14 +549,14 @@ static struct command_result *do_getroutes(struct command *cmd,
 	child_fd = fork_router_child(rq, info->dev_algo == ALGO_SINGLE_PATH,
 				     deadline, srcnode, dstnode, info->amount,
 				     info->maxfee, info->finalcltv, info->maxdelay,
-				     cmd->id, cmd->filter, &log_fd, &child_pid);
+				     info->maxparts, cmd->id, cmd->filter, &log_fd, &child_pid);
 	if (child_fd == -1) {
 		err = tal_fmt(tmpctx, "failed to fork: %s", strerror(errno));
 		goto fail_broken;
 	}
 
 	/* FIXME: Go async! */
-	process_child_logs(rq, log_fd);
+	process_child_logs(cmd, log_fd);
 	close(log_fd);
 	json = grab_fd_str(cmd, child_fd);
 	close(child_fd);
@@ -558,9 +565,9 @@ static struct command_result *do_getroutes(struct command *cmd,
 	struct timerel time_delta = timemono_between(time_mono(), time_start);
 
 	/* log the time of computation */
-	rq_log(tmpctx, rq, LOG_DBG, "get_routes %s %" PRIu64 " ms",
-	       WEXITSTATUS(child_status) != 0 ? "failed after" : "completed in",
-	       time_to_msec(time_delta));
+	cmd_log(tmpctx, cmd, LOG_DBG, "get_routes %s %" PRIu64 " ms",
+		WEXITSTATUS(child_status) != 0 ? "failed after" : "completed in",
+		time_to_msec(time_delta));
 
 	if (WIFSIGNALED(child_status)) {
 		err = tal_fmt(tmpctx, "child died with signal %u",

plugins/askrene/askrene.h

@@ -32,35 +32,30 @@ struct askrene {
 
 /* Information for a single route query. */
 struct route_query {
-	/* Command pointer, mainly for command id. */
-	struct command *cmd;
-
-	/* Plugin pointer, for logging mainly */
-	struct plugin *plugin;
-
 	/* This is *not* updated during a query!  Has all layers applied. */
 	const struct gossmap *gossmap;
 
-	/* We need to take in-flight payments into account */
-	const struct reserve_htable *reserved;
+	/* command id to use for reservations we create. */
+	const char *cmd_id;
 
 	/* Array of layers we're applying */
 	const struct layer **layers;
 
-	/* Cache of channel capacities for non-reserved, unknown channels. */
-	fp16_t *capacities;
-
 	/* Compact cache of biases */
-	s8 *biases;
+	const s8 *biases;
 
 	/* Additional per-htlc cost for local channels */
 	const struct additional_cost_htable *additional_costs;
 
+	/* We need to take in-flight payments into account (this is
+	 * askrene->reserved, so make sure to undo changes! */
+	struct reserve_htable *reserved;
+
+	/* Cache of channel capacities for non-reserved, unknown channels. */
+	fp16_t *capacities;
+
 	/* channels we disable during computation to meet constraints */
 	bitmap *disabled_chans;
-
-	/* limit the number of paths in the solution */
-	u32 maxparts;
 };
 
 /* Given a gossmap channel, get the current known min/max */
@@ -70,14 +65,6 @@ void get_constraints(const struct route_query *rq,
 		     struct amount_msat *min,
 		     struct amount_msat *max);
 
-/* Say something about this route_query */
-const char *rq_log(const tal_t *ctx,
-		   const struct route_query *rq,
-		   enum log_level level,
-		   const char *fmt,
-		   ...)
-	PRINTF_FMT(4, 5);
-
 /* Is there a known additional per-htlc cost for this channel? */
 struct amount_msat get_additional_per_htlc_cost(const struct route_query *rq,
 						const struct short_channel_id_dir *scidd);

plugins/askrene/child/entry.c

@@ -134,7 +134,7 @@ int fork_router_child(struct route_query *rq,
 		      const struct gossmap_node *srcnode,
 		      const struct gossmap_node *dstnode,
 		      struct amount_msat amount, struct amount_msat maxfee,
-		      u32 finalcltv, u32 maxdelay,
+		      u32 finalcltv, u32 maxdelay, size_t maxparts,
 		      const char *cmd_id,
 		      struct json_filter *cmd_filter,
 		      int *log_fd,
@@ -181,7 +181,7 @@ int fork_router_child(struct route_query *rq,
 	} else {
 		err = default_routes(rq, rq, deadline, srcnode, dstnode,
 				     amount, maxfee, finalcltv, maxdelay,
-				     &flows, &probability);
+				     maxparts, &flows, &probability);
 	}
 	if (err) {
 		write_all(replyfds[1], err, strlen(err));

plugins/askrene/child/entry.h

@@ -17,7 +17,7 @@ int fork_router_child(struct route_query *rq,
 		      const struct gossmap_node *srcnode,
 		      const struct gossmap_node *dstnode,
 		      struct amount_msat amount, struct amount_msat maxfee,
-		      u32 finalcltv, u32 maxdelay,
+		      u32 finalcltv, u32 maxdelay, size_t maxparts,
 		      const char *cmd_id,
 		      struct json_filter *cmd_filter,
 		      int *log_fd,

plugins/askrene/child/mcf.c

@@ -1344,6 +1344,7 @@ linear_routes(const tal_t *ctx, struct route_query *rq,
 	      const struct gossmap_node *srcnode,
 	      const struct gossmap_node *dstnode, struct amount_msat amount,
 	      struct amount_msat maxfee, u32 finalcltv, u32 maxdelay,
+	      size_t maxparts,
 	      struct flow ***flows, double *probability,
 	      struct flow **(*solver)(const tal_t *, const struct route_query *,
 				      const struct gossmap_node *,
@@ -1554,10 +1555,10 @@ linear_routes(const tal_t *ctx, struct route_query *rq,
 
 	/* If we're over the number of parts, try to cram excess into the
 	 * largest-capacity parts */
-	if (tal_count(*flows) > rq->maxparts) {
+	if (tal_count(*flows) > maxparts) {
 		struct amount_msat fee;
 
-		error_message = reduce_num_flows(rq, rq, flows, amount, rq->maxparts);
+		error_message = reduce_num_flows(rq, rq, flows, amount, maxparts);
 		if (error_message) {
 			*flows = tal_free(*flows);
 			return error_message;
@@ -1597,14 +1598,13 @@ linear_routes(const tal_t *ctx, struct route_query *rq,
 		return child_log(rq, LOG_BROKEN,
 				 "%s: check_htlc_max_limits failed", __func__);
 	}
-	if (tal_count(*flows) > rq->maxparts) {
+	if (tal_count(*flows) > maxparts) {
 		size_t num_flows = tal_count(*flows);
 		*flows = tal_free(*flows);
 		return child_log(rq, LOG_BROKEN,
 				 "%s: the number of flows (%zu) exceeds the limit set "
-				 "on payment parts (%" PRIu32
-				 "), please submit a bug report",
-				 __func__, num_flows, rq->maxparts);
+				 "on payment parts (%zu), please submit a bug report",
+				 __func__, num_flows, maxparts);
 	}
 
 	return NULL;
@@ -1621,11 +1621,12 @@ const char *default_routes(const tal_t *ctx, struct route_query *rq,
 			   const struct gossmap_node *srcnode,
 			   const struct gossmap_node *dstnode,
 			   struct amount_msat amount, struct amount_msat maxfee,
-			   u32 finalcltv, u32 maxdelay, struct flow ***flows,
+			   u32 finalcltv, u32 maxdelay, size_t maxparts,
+			   struct flow ***flows,
 			   double *probability)
 {
 	return linear_routes(ctx, rq, deadline, srcnode, dstnode, amount, maxfee,
-			     finalcltv, maxdelay, flows, probability, minflow);
+			     finalcltv, maxdelay, maxparts, flows, probability, minflow);
 }
 
 const char *single_path_routes(const tal_t *ctx, struct route_query *rq,
@@ -1638,6 +1639,6 @@ const char *single_path_routes(const tal_t *ctx, struct route_query *rq,
 			       double *probability)
 {
 	return linear_routes(ctx, rq, deadline, srcnode, dstnode, amount, maxfee,
-			     finalcltv, maxdelay, flows, probability,
+			     finalcltv, maxdelay, 1, flows, probability,
 			     single_path_flow);
 }

plugins/askrene/child/mcf.h

@@ -17,7 +17,7 @@ const char *default_routes(const tal_t *ctx, struct route_query *rq,
 			   const struct gossmap_node *dstnode,
 			   struct amount_msat amount,
 			   struct amount_msat maxfee, u32 finalcltv,
-			   u32 maxdelay, struct flow ***flows,
+			   u32 maxdelay, size_t maxparts, struct flow ***flows,
 			   double *probability);
 
 /* A wrapper to the single-path constrained solver. */