renepay: minor fixes

plugins/renepay/chan_extra.c

@@ -490,8 +490,7 @@ enum renepay_errorcode chan_extra_relax_fraction(struct chan_extra *ce,
 	fraction = fabs(fraction);     // this number is always non-negative
 	fraction = MIN(1.0, fraction); // this number cannot be greater than 1.
 	struct amount_msat delta =
-	    amount_msat(ce->capacity.millisatoshis *
-			fraction); /* Raw: get a fraction of the capacity */
+	    amount_msat(ce->capacity.millisatoshis*fraction); /* Raw: get a fraction of the capacity */
 
 	/* The direction here is not important because the 'down' and the 'up'
 	 * limits are changed by the same amount.

plugins/renepay/errorcodes.c

@@ -1,3 +1,4 @@
+#include "config.h"
 #include <ccan/tal/str/str.h>
 #include <plugins/renepay/errorcodes.h>
 #include <stdio.h>

plugins/renepay/errorcodes.h

@@ -1,6 +1,8 @@
 #ifndef LIGHTNING_PLUGINS_RENEPAY_ERRORCODES_H
 #define LIGHTNING_PLUGINS_RENEPAY_ERRORCODES_H
 
+#include "config.h"
+
 /* Common types of failures for low level functions in renepay. */
 enum renepay_errorcode {
 	RENEPAY_NOERROR = 0,

plugins/renepay/json.c

@@ -1,3 +1,4 @@
+#include "config.h"
 #include <common/json_stream.h>
 #include <plugins/renepay/json.h>
 

plugins/renepay/json.h

@@ -1,6 +1,7 @@
 #ifndef LIGHTNING_PLUGINS_RENEPAY_JSON_H
 #define LIGHTNING_PLUGINS_RENEPAY_JSON_H
 
+#include "config.h"
 #include <plugins/renepay/payment.h>
 #include <plugins/renepay/route.h>
 

plugins/renepay/mods.c

@@ -271,7 +271,7 @@ static struct command_result *previous_sendpays_done(struct command *cmd,
 		}
 
 		for (size_t j = 0; j < tal_count(pending_routes); j++) {
-			route_pending(pending_routes[j]);
+			route_pending_register(pending_routes[j]);
 		}
 
 	} else {
@@ -453,14 +453,13 @@ static void gossmod_cb(struct gossmap_localmods *mods,
 		       u32 fee_proportional,
 		       u32 cltv_delta,
 		       bool enabled,
-		       bool is_local,
 		       const char *buf,
 		       const jsmntok_t *chantok,
 		       struct payment *payment)
 {
 	struct amount_msat min, max;
 
-	if (is_local) {
+	if (scidd->dir == node_id_idx(self, peer)) {
 		/* local channels can send up to what's spendable */
 		min = AMOUNT_MSAT(0);
 		max = spendable;

plugins/renepay/payment.c

@@ -347,6 +347,8 @@ static struct command_result *payment_finish(struct payment *p)
 	return my_command_finish(p, cmd);
 }
 
+/* FIXME: disabled_scids should be a set rather than an array, so that we don't
+ * have to worry about disabling the same channel multiple times. */
 void payment_disable_chan(struct payment *p, struct short_channel_id scid,
 			  enum log_level lvl, const char *fmt, ...)
 {

plugins/renepay/route.c

@@ -1,5 +1,4 @@
 #include "config.h"
-#include <assert.h>
 #include <plugins/renepay/route.h>
 
 struct route *new_route(const tal_t *ctx, struct payment *payment, u32 groupid,

plugins/renepay/routebuilder.c

@@ -1,3 +1,4 @@
+#include "config.h"
 #include <ccan/bitmap/bitmap.h>
 #include <plugins/renepay/mcf.h>
 #include <plugins/renepay/routebuilder.h>

plugins/renepay/routefail.c

@@ -75,7 +75,7 @@ static struct command_result *routefail_rpc_failure(struct command *cmd,
 		   "routefail state machine has stopped due to a failed RPC "
 		   "call: %.*s",
 		   json_tok_full_len(toks), json_tok_full(buffer, toks));
-	return command_still_pending(r->cmd);
+	return notification_handled(r->cmd);
 }
 
 /*****************************************************************************
@@ -88,7 +88,9 @@ static struct command_result *end_done(struct command *cmd,
 				       const jsmntok_t *result UNUSED,
 				       struct routefail *r)
 {
-	route_is_failure(r->route);
+	/* Notify the tracker that route has failed and routefail have completed
+	 * handling all possible errors cases. */
+	route_failure_register(r->route);
 	tal_free(r);
 	return notification_handled(cmd);
 }
@@ -176,8 +178,6 @@ static struct command_result *update_gossip_failure(struct command *cmd UNUSED,
 	 * waitsendpay says it is present in the case of error. */
 	assert(r->route->result->erring_channel);
 
-	/* TODO disable chan? what if we endup disabling a channel twice? maybe
-	 * use a map instead of an array. */
 	payment_disable_chan(
 	    r->route->payment, *r->route->result->erring_channel, LOG_INFORM,
 	    "addgossip failed (%.*s)", json_tok_full_len(result),
@@ -222,10 +222,10 @@ static struct command_result *update_knowledge_cb(struct routefail *r)
 
 	/* FIXME: If we don't know the hops there isn't much we can infer, but
 	 * a little bit we could. */
-	if (!route->hops)
+	if (!route->hops || !result->erring_index)
 		goto skip_update_network;
 
-	uncertainty_channel_can_send(pay_plugin->uncertainty, r->route,
+	uncertainty_channel_can_send(pay_plugin->uncertainty, route,
 				  *result->erring_index);
 
 	if (result->failcode == WIRE_TEMPORARY_CHANNEL_FAILURE &&
@@ -236,6 +236,8 @@ static struct command_result *update_knowledge_cb(struct routefail *r)
 		    route->hops[*result->erring_index].direction);
 	}
 
+	uncertainty_remove_htlcs(pay_plugin->uncertainty, route);
+
 skip_update_network:
 	return routefail_continue(r);
 }
@@ -258,11 +260,20 @@ static void route_final_error(struct route *route, enum jsonrpc_errcode error,
 	route->final_msg = tal_strdup(route, what);
 }
 
-static void handle_unhandleable_error(struct route *route, const char *what)
+static void handle_unhandleable_error(struct route *route, const char *fmt, ...)
 {
 	if (!route->hops)
 		return;
 	size_t n = tal_count(route->hops);
+	if (n == 0)
+		return;
+
+	va_list ap;
+	const char *what;
+
+	va_start(ap, fmt);
+	what = tal_vfmt(tmpctx, fmt, ap);
+	va_end(ap);
 
 	if (n == 1) {
 		/* This is a terminal error. */
@@ -332,9 +343,17 @@ static struct command_result *handle_cases_cb(struct routefail *r)
 	case WIRE_INVALID_ONION_PAYLOAD:
 	case WIRE_INVALID_ONION_BLINDING:
 	case WIRE_EXPIRY_TOO_FAR:
-		payment_disable_chan(route->payment, *result->erring_channel,
-				     LOG_UNUSUAL, "%s",
-				     onion_wire_name(result->failcode));
+		if (result->erring_channel)
+			payment_disable_chan(route->payment,
+					     *result->erring_channel,
+					     LOG_UNUSUAL, "%s",
+					     onion_wire_name(result->failcode));
+		else
+			handle_unhandleable_error(
+			    route,
+			    "received %s error, but don't have an "
+			    "erring_channel",
+			    onion_wire_name(result->failcode));
 		break;
 
 	/* These can be fixed (maybe) by applying the included channel_update */
@@ -353,11 +372,17 @@ static struct command_result *handle_cases_cb(struct routefail *r)
 		break;
 
 	default:
-		/* FIXME: remember you might be disabling the same channel
-		 * multiple times */
-		payment_disable_chan(route->payment, *result->erring_channel,
-				     LOG_UNUSUAL, "Unexpected error code %u",
-				     result->failcode);
+		if (result->erring_channel)
+			payment_disable_chan(
+			    route->payment, *result->erring_channel,
+			    LOG_UNUSUAL, "Unexpected error code %u",
+			    result->failcode);
+		else
+			handle_unhandleable_error(
+			    route,
+			    "received %s error, but don't have an "
+			    "erring_channel",
+			    onion_wire_name(result->failcode));
 	}
 
 finish:

plugins/renepay/routetracker.c

@@ -1,3 +1,4 @@
+#include "config.h"
 #include <common/json_stream.h>
 #include <plugins/renepay/json.h>
 #include <plugins/renepay/payment.h>
@@ -40,15 +41,15 @@ static void routetracker_add_to_final(struct routetracker *routetracker,
 	tal_arr_expand(&routetracker->finalized_routes, route);
 	tal_steal(routetracker, route);
 }
-static void route_is_success(struct route *route)
+static void route_success_register(struct route *route)
 {
 	routetracker_add_to_final(route->payment->routetracker, route);
 }
-void route_is_failure(struct route *route)
+void route_failure_register(struct route *route)
 {
 	routetracker_add_to_final(route->payment->routetracker, route);
 }
-static void route_sent(struct route *route)
+static void route_sent_register(struct route *route)
 {
 	struct routetracker *routetracker = route->payment->routetracker;
 	route_map_add(routetracker->sent_routes, route);
@@ -71,7 +72,7 @@ static void route_sendpay_fail(struct route *route TAKES)
  *	- after a sendpay is accepted,
  *	- or after listsendpays reveals some pending route that we didn't
  *	previously know about. */
-void route_pending(const struct route *route)
+void route_pending_register(const struct route *route)
 {
 	assert(route);
 	struct payment *payment = route->payment;
@@ -92,8 +93,11 @@ void route_pending(const struct route *route)
 			   fmt_routekey(tmpctx, &route->key));
 
 	uncertainty_commit_htlcs(pay_plugin->uncertainty, route);
-	route_map_add(routetracker->pending_routes, route);
-	tal_steal(routetracker, route);
+
+	if (!route_map_add(routetracker->pending_routes, route) ||
+	    !tal_steal(routetracker, route))
+		plugin_err(pay_plugin->plugin, "%s: failed to register route.",
+			   __PRETTY_FUNCTION__);
 
 	if (!amount_msat_add(&payment->total_sent, payment->total_sent,
 			     route_sends(route)) ||
@@ -110,8 +114,6 @@ static void route_result_collected(struct route *route TAKES)
 {
 	assert(route);
 	assert(route->result);
-	// TODO: also improve knowledge here?
-	uncertainty_remove_htlcs(pay_plugin->uncertainty, route);
 
 	assert(route->payment);
 	struct payment *payment = route->payment;
@@ -140,7 +142,7 @@ static struct command_result *sendpay_done(struct command *cmd,
 					   struct route *route)
 {
 	assert(route);
-	route_pending(route);
+	route_pending_register(route);
 	return command_still_pending(cmd);
 }
 
@@ -247,7 +249,7 @@ struct command_result *route_sendpay_request(struct command *cmd,
 
 	json_add_route(req->js, route);
 
-	route_sent(route);
+	route_sent_register(route);
 	return send_outreq(pay_plugin->plugin, req);
 }
 
@@ -352,6 +354,6 @@ struct command_result *notification_sendpay_success(struct command *cmd,
 
 	// FIXME: what happens when several success notification arrive for the
 	// same payment? Even after the payment has been resolved.
-	route_is_success(route);
+	route_success_register(route);
 	return notification_handled(cmd);
 }

plugins/renepay/routetracker.h

@@ -29,7 +29,7 @@ void payment_collect_results(struct payment *payment,
 
 /* Announce that this route is pending and needs to be kept in the waiting list
  * for notifications. */
-void route_pending(const struct route *route);
+void route_pending_register(const struct route *route);
 
 /* Sends a sendpay request for this route. */
 struct command_result *route_sendpay_request(struct command *cmd,
@@ -43,7 +43,8 @@ struct command_result *notification_sendpay_success(struct command *cmd,
 						    const char *buf,
 						    const jsmntok_t *params);
 
-void route_is_failure(struct route *route);
+/* Notify the tracker that this route has failed. */
+void route_failure_register(struct route *route);
 
 // FIXME: double-check that we actually get one notification for each sendpay,
 // ie. that after some time we don't have yet pending sendpays for old failed or

plugins/renepay/test/run-mcf.c

@@ -9,6 +9,7 @@
 
 #include <ccan/array_size/array_size.h>
 #include <ccan/read_write_all/read_write_all.h>
+#include <ccan/str/hex/hex.h>
 #include <common/bigsize.h>
 #include <common/channel_id.h>
 #include <common/node_id.h>
@@ -336,9 +337,13 @@ int main(int argc, char *argv[])
 	struct short_channel_id scid12, scid23;
 	struct sha256 payment_hash;
 	struct amount_msat *amounts;
-
 	char *errmsg;
 
+	if (!hex_decode("0001020304050607080900010203040506070809000102030405060708090102",
+			strlen("0001020304050607080900010203040506070809000102030405060708090102"),
+			&payment_hash, sizeof(payment_hash)))
+		abort();
+
 	common_setup(argv[0]);
 
 	fd = tmpdir_mkstemp(tmpctx, "run-not_mcf.XXXXXX", &gossfile);

plugins/renepay/test/run-testflow.c

@@ -1,13 +1,14 @@
 #include "config.h"
-#include <stdio.h>
 #include <assert.h>
-#include <common/wireaddr.h>
+#include <ccan/read_write_all/read_write_all.h>
+#include <ccan/str/hex/hex.h>
 #include <common/bigsize.h>
 #include <common/channel_id.h>
+#include <common/node_id.h>
 #include <common/setup.h>
 #include <common/utils.h>
-#include <common/node_id.h>
-#include <ccan/read_write_all/read_write_all.h>
+#include <common/wireaddr.h>
+#include <stdio.h>
 #include <wire/onion_wiregen.h>
 
 #define MYLOG "/tmp/debug.txt"
@@ -661,6 +662,11 @@ static void test_flow_to_route(void)
 	struct sha256 payment_hash;
 	struct amount_msat deliver;
 
+	if (!hex_decode("0001020304050607080900010203040506070809000102030405060708090102",
+			strlen("0001020304050607080900010203040506070809000102030405060708090102"),
+			&payment_hash, sizeof(payment_hash)))
+		abort();
+
 	// flow 1->2
 	F = tal(this_ctx, struct flow);
 	F->path = tal_arr(F,const struct gossmap_chan *,1);

plugins/renepay/uncertainty.c

@@ -55,7 +55,7 @@ void uncertainty_commit_htlcs(struct uncertainty *uncertainty,
 }
 
 void uncertainty_channel_can_send(struct uncertainty *uncertainty,
-				  struct route *route, u32 erridx)
+				  const struct route *route, u32 erridx)
 {
 	if (!route->hops)
 		return;

plugins/renepay/uncertainty.h

@@ -1,5 +1,5 @@
-#ifndef LIGHTNING_PLUGINS_RENEPAY_UNETWORK_H
-#define LIGHTNING_PLUGINS_RENEPAY_UNETWORK_H
+#ifndef LIGHTNING_PLUGINS_RENEPAY_UNCERTAINTY_H
+#define LIGHTNING_PLUGINS_RENEPAY_UNCERTAINTY_H
 #include "config.h"
 #include <ccan/tal/tal.h>
 #include <common/gossmap.h>
@@ -19,6 +19,7 @@ struct uncertainty {
 	struct chan_extra_map *chan_extra_map;
 };
 
+/* FIXME: add bool return value and WARN_UNUSED_RESULT */
 void uncertainty_route_success(struct uncertainty *uncertainty,
 			       const struct route *route);
 void uncertainty_remove_htlcs(struct uncertainty *uncertainty,
@@ -28,7 +29,7 @@ void uncertainty_commit_htlcs(struct uncertainty *uncertainty,
 			      const struct route *route);
 
 void uncertainty_channel_can_send(struct uncertainty *uncertainty,
-				  struct route *route, u32 erridx);
+				  const struct route *route, u32 erridx);
 
 void uncertainty_channel_cannot_send(struct uncertainty *uncertainty,
 				     struct short_channel_id scid,
@@ -54,4 +55,4 @@ bool uncertainty_set_liquidity(struct uncertainty *uncertainty,
 struct chan_extra *uncertainty_find_channel(struct uncertainty *uncertainty,
 					    const struct short_channel_id scid);
 
-#endif /* LIGHTNING_PLUGINS_RENEPAY_UNETWORK_H */
+#endif /* LIGHTNING_PLUGINS_RENEPAY_UNCERTAINTY_H */