askrene: refactor get_routes

Prefer the following programming pattern:

do_getroutes(){
        do_something1();
        do_something2();
        do_something3();
}

rather than

get_routes(){
        do_something1();
        do_something2();
}

do_getroutes(){
        get_routes();
        do_something3();
}

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>

Author: Lagrang3

plugins/askrene/askrene.c

@@ -347,60 +347,35 @@ static struct amount_msat linear_flows_cost(struct flow **flows,
 	return total;
 }
 
-/* Returns an error message, or sets *routes */
-static const char *get_routes(const tal_t *ctx,
-			      struct command *cmd,
-			      const struct node_id *source,
-			      const struct node_id *dest,
-			      struct amount_msat amount,
-			      struct amount_msat maxfee,
-			      u32 finalcltv,
-			      u32 maxdelay,
-			      const char **layers,
-			      struct gossmap_localmods *localmods,
-			      const struct layer *local_layer,
-			      bool single_path,
-			      struct route ***routes,
-			      struct amount_msat **amounts,
-			      const struct additional_cost_htable *additional_costs,
-			      double *probability)
-{
-	struct askrene *askrene = get_askrene(cmd->plugin);
-	struct route_query *rq = tal(ctx, struct route_query);
-	struct flow **flows;
-	const struct gossmap_node *srcnode, *dstnode;
-	const char *ret;
-	struct timerel time_delta;
-	struct timemono time_start = time_mono();
-
-	if (gossmap_refresh(askrene->gossmap)) {
-		/* FIXME: gossmap_refresh callbacks to we can update in place */
-		tal_free(askrene->capacities);
-		askrene->capacities = get_capacities(askrene, askrene->plugin, askrene->gossmap);
-	}
-
-	rq->cmd = cmd;
-	rq->plugin = cmd->plugin;
-	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);
-	rq->additional_costs = additional_costs;
+struct getroutes_info {
+	struct command *cmd;
+	struct node_id *source, *dest;
+	struct amount_msat *amount, *maxfee;
+	u32 *finalcltv, *maxdelay;
+	const char **layers;
+	struct additional_cost_htable *additional_costs;
+	/* Non-NULL if we are told to use "auto.localchans" */
+	struct layer *local_layer;
+};
 
+static void apply_layers(struct askrene *askrene, struct route_query *rq,
+			 struct gossmap_localmods *localmods,
+			 const struct getroutes_info *info)
+{
 	/* Layers must exist, but might be special ones! */
-	for (size_t i = 0; i < tal_count(layers); i++) {
-		const struct layer *l = find_layer(askrene, layers[i]);
+	for (size_t i = 0; i < tal_count(info->layers); i++) {
+		const struct layer *l = find_layer(askrene, info->layers[i]);
 		if (!l) {
-			if (streq(layers[i], "auto.localchans")) {
+			if (streq(info->layers[i], "auto.localchans")) {
 				plugin_log(rq->plugin, LOG_DBG, "Adding auto.localchans");
-				l = local_layer;
-			} else if (streq(layers[i], "auto.no_mpp_support")) {
+				l = info->local_layer;
+			} else if (streq(info->layers[i], "auto.no_mpp_support")) {
 				plugin_log(rq->plugin, LOG_DBG, "Adding auto.no_mpp_support, sorry");
-				l = remove_small_channel_layer(layers, askrene, amount, localmods);
+				l = remove_small_channel_layer(info->layers, askrene, *info->amount, localmods);
 			} else {
-				assert(streq(layers[i], "auto.sourcefree"));
+				assert(streq(info->layers[i], "auto.sourcefree"));
 				plugin_log(rq->plugin, LOG_DBG, "Adding auto.sourcefree");
-				l = source_free_layer(layers, askrene, source, localmods);
+				l = source_free_layer(info->layers, askrene, info->source, localmods);
 			}
 		}
 
@@ -412,46 +387,14 @@ static const char *get_routes(const tal_t *ctx,
 		 * override them (incl local channels) */
 		layer_clear_overridden_capacities(l, askrene->gossmap, rq->capacities);
 	}
+}
 
-	/* Clear scids with reservations, too, so we don't have to look up
-	 * all the time! */
-	reserves_clear_capacities(askrene->reserved, askrene->gossmap, rq->capacities);
-
-	gossmap_apply_localmods(askrene->gossmap, localmods);
-
-	/* localmods can add channels, so we need to allocate biases array *afterwards* */
-	rq->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);
-
-	srcnode = gossmap_find_node(askrene->gossmap, source);
-	if (!srcnode) {
-		ret = rq_log(ctx, rq, LOG_INFORM,
-			     "Unknown source node %s",
-			     fmt_node_id(tmpctx, source));
-		goto fail;
-	}
-
-	dstnode = gossmap_find_node(askrene->gossmap, dest);
-	if (!dstnode) {
-		ret = rq_log(ctx, rq, LOG_INFORM,
-			     "Unknown destination node %s",
-			     fmt_node_id(tmpctx, dest));
-		goto fail;
-	}
-
-	/* FIXME: single_path should signal a change in algorithm. */
-	ret = default_routes(rq, rq, srcnode, dstnode, amount, single_path,
-			     maxfee, finalcltv, maxdelay, &flows, probability);
-	if (ret) {
-		goto fail;
-	}
-	assert(tal_count(flows) > 0);
-	rq_log(tmpctx, rq, LOG_DBG, "Final answer has %zu flows",
-	       tal_count(flows));
-
+static void convert_flows_to_routes(const tal_t *ctx, struct route_query *rq,
+				    struct route ***routes,
+				    struct amount_msat **amounts,
+				    u32 finalcltv,
+				    struct flow **flows)
+{
 	/* Convert back into routes, with delay and other information fixed */
 	*routes = tal_arr(ctx, struct route *, tal_count(flows));
 	*amounts = tal_arr(ctx, struct amount_msat, tal_count(flows));
@@ -485,26 +428,9 @@ static const char *get_routes(const tal_t *ctx,
 			rh->delay = delay;
 		}
 		(*amounts)[i] = flows[i]->delivers;
-		rq_log(tmpctx, rq, LOG_INFORM, "Flow %zu/%zu: %s",
-		       i, tal_count(flows),
+		rq_log(tmpctx, rq, LOG_INFORM, "Flow %zu/%zu: %s", i, tal_count(flows),
 		       fmt_route(tmpctx, r, (*amounts)[i], finalcltv));
 	}
-
-	gossmap_remove_localmods(askrene->gossmap, localmods);
-	time_delta = timemono_between(time_mono(), time_start);
-	rq_log(tmpctx, rq, LOG_DBG, "get_routes completed in %" PRIu64 " ms",
-	       time_to_msec(time_delta));
-	return NULL;
-
-	/* Explicit failure path keeps the compiler (gcc version 12.3.0 -O3) from
-	 * warning about uninitialized variables in the caller */
-fail:
-	assert(ret != NULL);
-	gossmap_remove_localmods(askrene->gossmap, localmods);
-	time_delta = timemono_between(time_mono(), time_start);
-	rq_log(tmpctx, rq, LOG_DBG, "get_routes failed after %" PRIu64 " ms",
-	       time_to_msec(time_delta));
-	return ret;
 }
 
 void get_constraints(const struct route_query *rq,
@@ -543,61 +469,164 @@ void get_constraints(const struct route_query *rq,
 	reserve_sub(rq->reserved, &scidd, max);
 }
 
-struct getroutes_info {
-	struct command *cmd;
-	struct node_id *source, *dest;
-	struct amount_msat *amount, *maxfee;
-	u32 *finalcltv, *maxdelay;
-	const char **layers;
-	struct additional_cost_htable *additional_costs;
-	/* Non-NULL if we are told to use "auto.localchans" */
-	struct layer *local_layer;
-};
+static void json_add_getroutes(
+    struct json_stream *js,
+    struct route **routes,
+    /* FIXME: can't we deduce these amounts from the routes themselves */
+    struct amount_msat *amounts,
+    double probability,
+    /* FIXME: can't we have different final_cltv for every route? */
+    u32 final_cltv)
+{
+	json_add_u64(js, "probability_ppm", (u64)(probability * 1000000));
+	json_array_start(js, "routes");
+	for (size_t i = 0; i < tal_count(routes); i++) {
+		json_object_start(js, NULL);
+		json_add_u64(js, "probability_ppm",
+			     (u64)(routes[i]->success_prob * 1000000));
+		json_add_amount_msat(js, "amount_msat", amounts[i]);
+		json_add_u32(js, "final_cltv", final_cltv);
+		json_array_start(js, "path");
+		for (size_t j = 0; j < tal_count(routes[i]->hops); j++) {
+			struct short_channel_id_dir scidd;
+			const struct route_hop *r = &routes[i]->hops[j];
+			json_object_start(js, NULL);
+			scidd.scid = r->scid;
+			scidd.dir = r->direction;
+			json_add_short_channel_id_dir(
+			    js, "short_channel_id_dir", scidd);
+			json_add_node_id(js, "next_node_id", &r->node_id);
+			json_add_amount_msat(js, "amount_msat", r->amount);
+			json_add_u32(js, "delay", r->delay);
+			json_object_end(js);
+		}
+		json_array_end(js);
+		json_object_end(js);
+	}
+	json_array_end(js);
+}
 
 static struct command_result *do_getroutes(struct command *cmd,
 					   struct gossmap_localmods *localmods,
 					   const struct getroutes_info *info)
 {
+	bool localmods_applied = false;
 	const char *err;
 	double probability;
 	struct amount_msat *amounts;
 	struct route **routes;
+	struct flow **flows;
 	struct json_stream *response;
 
-	err = get_routes(cmd, cmd,
-			 info->source, info->dest,
-			 *info->amount, *info->maxfee, *info->finalcltv,
-			 *info->maxdelay, info->layers, localmods, info->local_layer,
-			 have_layer(info->layers, "auto.no_mpp_support"),
-			 &routes, &amounts, info->additional_costs, &probability);
-	if (err)
-		return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "%s", err);
+	/* get me the global state structure */
+	struct askrene *askrene = get_askrene(cmd->plugin);
 
-	response = jsonrpc_stream_success(cmd);
-	json_add_u64(response, "probability_ppm", (u64)(probability * 1000000));
-	json_array_start(response, "routes");
-	for (size_t i = 0; i < tal_count(routes); i++) {
-		json_object_start(response, NULL);
-		json_add_u64(response, "probability_ppm", (u64)(routes[i]->success_prob * 1000000));
-		json_add_amount_msat(response, "amount_msat", amounts[i]);
-		json_add_u32(response, "final_cltv", *info->finalcltv);
-		json_array_start(response, "path");
-		for (size_t j = 0; j < tal_count(routes[i]->hops); j++) {
-			struct short_channel_id_dir scidd;
-			const struct route_hop *r = &routes[i]->hops[j];
-			json_object_start(response, NULL);
-			scidd.scid = r->scid;
-			scidd.dir = r->direction;
-			json_add_short_channel_id_dir(response, "short_channel_id_dir", scidd);
-			json_add_node_id(response, "next_node_id", &r->node_id);
-			json_add_amount_msat(response, "amount_msat", r->amount);
-			json_add_u32(response, "delay", r->delay);
-			json_object_end(response);
+	/* update the gossmap */
+	if (gossmap_refresh(askrene->gossmap)) {
+		/* FIXME: gossmap_refresh callbacks to we can update in place */
+		tal_free(askrene->capacities);
+		askrene->capacities =
+		    get_capacities(askrene, askrene->plugin, askrene->gossmap);
+	}
+
+	/* build this request structure */
+	struct route_query *rq = tal(cmd, struct route_query);
+	rq->cmd = cmd;
+	rq->plugin = cmd->plugin;
+	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;
+
+	/* apply selected layers to the localmods */
+	apply_layers(askrene, rq, localmods, info);
+
+	/* Clear scids with reservations, too, so we don't have to look up
+	 * all the time! */
+	reserves_clear_capacities(askrene->reserved, askrene->gossmap,
+				  rq->capacities);
+
+	/* we temporarily apply localmods */
+	gossmap_apply_localmods(askrene->gossmap, localmods);
+	localmods_applied = true;
+
+	/* localmods can add channels, so we need to allocate biases array
+	 * *afterwards* */
+	rq->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);
+
+	/* 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",
+			     fmt_node_id(tmpctx, info->source));
+		goto fail;
+	}
+
+	/* checkout the destination */
+	const struct gossmap_node *dstnode =
+	    gossmap_find_node(askrene->gossmap, info->dest);
+	if (!dstnode) {
+		err = rq_log(tmpctx, rq, LOG_INFORM,
+			     "Unknown destination node %s",
+			     fmt_node_id(tmpctx, info->dest));
+		goto fail;
+	}
+
+	/* Compute the routes. At this point we might select between multiple
+	 * algorithms. */
+	struct timemono time_start = time_mono();
+	err = default_routes(rq, rq, srcnode, dstnode, *info->amount,
+			     /* only one path? = */
+			     have_layer(info->layers, "auto.no_mpp_support"),
+			     *info->maxfee, *info->finalcltv, *info->maxdelay,
+			     &flows, &probability);
+	struct timerel time_delta = timemono_between(time_mono(), time_start);
+
+	/* log the time of computation */
+	rq_log(tmpctx, rq, LOG_INFORM, "get_routes %s %" PRIu64 " ms",
+	       err ? "failed after" : "completed in",
+	       time_delta_ms(time_delta));
+
+fail:
+	/* if we have an error we end the execution here */
+	if (err) {
+		/* Safety check, remove localmods only if we applied it. */
+		if (localmods_applied){
+			gossmap_remove_localmods(askrene->gossmap, localmods);
+			localmods_applied = false;
 		}
-		json_array_end(response);
-		json_object_end(response);
+		return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "%s", err);
 	}
-	json_array_end(response);
+
+	/* otherwise we continue */
+	assert(tal_count(flows) > 0);
+	rq_log(tmpctx, rq, LOG_DBG, "Final answer has %zu flows",
+	       tal_count(flows));
+
+	/* convert flows to routes */
+	convert_flows_to_routes(rq, rq, &routes, &amounts, *info->finalcltv,
+				flows);
+	assert(tal_count(routes) == tal_count(flows));
+	assert(tal_count(amounts) == tal_count(flows));
+
+	/* At last we remove the localmods from the gossmap. */
+	if (localmods_applied) {
+		gossmap_remove_localmods(askrene->gossmap, localmods);
+		localmods_applied = false;
+	}
+
+	/* output the results */
+	response = jsonrpc_stream_success(cmd);
+	json_add_getroutes(response, routes, amounts, probability,
+			   *info->finalcltv);
 	return command_finished(cmd, response);
 }