renepay: more cleanups

- adopt "const <type> *"convention
- remove use_shadow option for some pyln tests
- show prob. information of flows into paynotes
- show prob. of success of entire payment flow in paynotes
- minflow: We were not releasing the memory of flow arrays when replacing
  them with a new canditate.
- use memleak_scan_obj in memleak_check
- replace u64 with size_t

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

Author: Lagrang3

plugins/renepay/flow.c

@@ -18,7 +18,7 @@
 #define MAX(x, y) (((x) > (y)) ? (x) : (y))
 #define MIN(x, y) (((x) < (y)) ? (x) : (y))
 
-bool chan_extra_is_busy(struct chan_extra const * const ce)
+bool chan_extra_is_busy(const struct chan_extra *const ce)
 {
 	if(ce==NULL)return false;
 	return ce->half[0].num_htlcs || ce->half[1].num_htlcs;
@@ -624,7 +624,7 @@ struct chan_inflight_flow
 // TODO(eduardo): here flows should be const
 double flow_set_probability(
 		struct flow ** flows,
-		struct gossmap const*const gossmap,
+		const struct gossmap *const gossmap,
 		struct chan_extra_map * chan_extra_map)
 {
 	tal_t *this_ctx = tal(tmpctx,tal_t);

plugins/renepay/flow.h

@@ -40,7 +40,7 @@ struct chan_extra {
 	} half[2];
 };
 
-bool chan_extra_is_busy(struct chan_extra const * const ce);
+bool chan_extra_is_busy(const struct chan_extra *const ce);
 
 static inline const struct short_channel_id
 chan_extra_scid(const struct chan_extra *cd)
@@ -205,7 +205,7 @@ struct chan_extra_half *get_chan_extra_half_by_chan(const struct gossmap *gossma
 
 /* An actual partial flow. */
 struct flow {
-	struct gossmap_chan const **path;
+	const struct gossmap_chan **path;
 	/* The directions to traverse. */
 	int *dirs;
 	/* Amounts for this flow (fees mean this shrinks across path). */
@@ -241,7 +241,7 @@ void flow_complete(struct flow *flow,
 /* Compute the prob. of success of a set of concurrent set of flows. */
 double flow_set_probability(
 		struct flow ** flows,
-		struct gossmap const*const gossmap,
+		const struct gossmap *const gossmap,
 		struct chan_extra_map * chan_extra_map);
 
 // TODO(eduardo): we probably don't need this. Instead we should have payflow

plugins/renepay/mcf.c

@@ -273,8 +273,8 @@ typedef union
 struct pay_parameters {
 	/* The gossmap we are using */
 	struct gossmap *gossmap;
-	struct gossmap_node const*source;
-	struct gossmap_node const*target;
+	const struct gossmap_node *source;
+	const struct gossmap_node *target;
 
 	/* Extra information we intuited about the channels */
 	struct chan_extra_map *chan_extra_map;
@@ -865,7 +865,7 @@ static int  find_optimal_path(
 	for(size_t i=0;i<tal_count(prev);++i)
 		prev[i].idx=INVALID_INDEX;
 
-	s64 const * const distance=dijkstra_distance_data(dijkstra);
+	const s64 *const distance=dijkstra_distance_data(dijkstra);
 
 	dijkstra_init(dijkstra);
 	dijkstra_update(dijkstra,source,0);
@@ -961,7 +961,7 @@ static int optimize_mcf(
 	zero_flow(linear_network,residual_network);
 	arc_t *prev = tal_arr(this_ctx,arc_t,linear_network->max_num_nodes);
 
-	s64 const*const distance = dijkstra_distance_data(dijkstra);
+	const s64 *const distance = dijkstra_distance_data(dijkstra);
 
 	s64 remaining_amount = amount;
 
@@ -1021,7 +1021,7 @@ static u32 find_positive_balance(
 		const u32 start_idx,
 		const s64 *balance,
 
-		struct gossmap_chan const** prev_chan,
+		const struct gossmap_chan **prev_chan,
 		int *prev_dir,
 		u32 *prev_idx)
 {
@@ -1045,7 +1045,7 @@ static u32 find_positive_balance(
 		for(size_t i=0;i<cur->num_chans;++i)
 		{
 			int dir;
-			struct gossmap_chan const *c
+			const struct gossmap_chan *c
 				= gossmap_nth_chan(gossmap,
 				                   cur,i,&dir);
 
@@ -1112,8 +1112,8 @@ static struct flow **
 	const size_t max_num_nodes = gossmap_max_node_idx(gossmap);
 	s64 *balance = tal_arrz(this_ctx,s64,max_num_nodes);
 
-	struct gossmap_chan const **prev_chan
-		= tal_arr(this_ctx,struct gossmap_chan const*,max_num_nodes);
+	const struct gossmap_chan **prev_chan
+		= tal_arr(this_ctx,const struct gossmap_chan *,max_num_nodes);
 
 	int *prev_dir = tal_arr(this_ctx,int,max_num_nodes);
 	u32 *prev_idx = tal_arr(this_ctx,u32,max_num_nodes);
@@ -1171,7 +1171,7 @@ static struct flow **
 				assert(cur_idx!=INVALID_INDEX);
 
 				const int dir = prev_dir[cur_idx];
-				struct gossmap_chan const * const c = prev_chan[cur_idx];
+				const struct gossmap_chan *const c = prev_chan[cur_idx];
 				const u32 c_idx = gossmap_chan_idx(gossmap,c);
 
 				delta=MIN(delta,chan_flow[c_idx].half[dir]);
@@ -1186,7 +1186,7 @@ static struct flow **
 
 
 			struct flow *fp = tal(this_ctx,struct flow);
-			fp->path = tal_arr(fp,struct gossmap_chan const*,length);
+			fp->path = tal_arr(fp,const struct gossmap_chan *,length);
 			fp->dirs = tal_arr(fp,int,length);
 
 			balance[node_idx] += delta;
@@ -1200,7 +1200,7 @@ static struct flow **
 				assert(cur_idx!=INVALID_INDEX);
 
 				const int dir = prev_dir[cur_idx];
-				struct gossmap_chan const * const c = prev_chan[cur_idx];
+				const struct gossmap_chan *const c = prev_chan[cur_idx];
 				const u32 c_idx = gossmap_chan_idx(gossmap,c);
 
 				length--;
@@ -1475,17 +1475,23 @@ struct flow** minflow(
 						params->chan_extra_map);
 		struct amount_msat fee = flow_set_fee(flow_paths);
 
-		// is this better than the previous one?
+		/* Is this better than the previous one? */
 		if(!best_flow_paths ||
 			is_better(params->max_fee,params->min_probability,
 				  fee,prob_success,
 			          best_fee, best_prob_success))
 		{
+			struct flow **tmp = best_flow_paths;
 			best_flow_paths = tal_steal(ctx,flow_paths);
+			tal_free(tmp);
+
 			best_fee = fee;
 			best_prob_success=prob_success;
 			flow_paths = NULL;
 		}
+		/* I don't like this candidate. */
+		else
+			tal_free(flow_paths);
 
 		if(amount_msat_greater(fee,params->max_fee))
 		{
@@ -1502,9 +1508,6 @@ struct flow** minflow(
 			// the fees
 			mu_left = mu+1;
 		}
-
-		if(flow_paths)
-			tal_free(flow_paths);
 	}
 
 

plugins/renepay/pay.c

@@ -65,19 +65,10 @@ void amount_msat_reduce_(struct amount_msat *dst,
 #if DEVELOPER
 static void memleak_mark(struct plugin *p, struct htable *memtable)
 {
-	/* TODO(eduardo): understand the purpose of memleak_scan_obj, why use it
-	 * instead of tal_free?
-	 * 1st problem: this is executed before the plugin can process the
-	 * shutdown notification,
-	 * 2nd problem: memleak_scan_obj does not propagate to children.
-	 * For the moment let's just (incorrectly) do tal_free here
-	 * */
-	pay_plugin->ctx = tal_free(pay_plugin->ctx);
-
-	// memleak_scan_obj(memtable, pay_plugin->ctx);
-	// memleak_scan_obj(memtable, pay_plugin->gossmap);
-	// memleak_scan_obj(memtable, pay_plugin->chan_extra_map);
-	// memleak_scan_htable(memtable, &pay_plugin->chan_extra_map->raw);
+	memleak_scan_obj(memtable, pay_plugin->ctx);
+	memleak_scan_obj(memtable, pay_plugin->gossmap);
+	memleak_scan_obj(memtable, pay_plugin->chan_extra_map);
+	memleak_scan_htable(memtable, &pay_plugin->chan_extra_map->raw);
 }
 #endif
 
@@ -411,12 +402,13 @@ sendpay_flows(struct command *cmd,
 	debug_paynote(p, "Sending out batch of %zu payments", tal_count(flows));
 
 	for (size_t i = 0; i < tal_count(flows); i++) {
-		const u64 path_lengh = tal_count(flows[i]->amounts);
-		debug_paynote(p, "sendpay flow groupid=%ld, partid=%ld, delivering=%s",
+		const size_t path_lengh = tal_count(flows[i]->amounts);
+		debug_paynote(p, "sendpay flow groupid=%ld, partid=%ld, delivering=%s, probability=%.3lf",
 			      flows[i]->key.groupid,
 			      flows[i]->key.partid,
 			      type_to_string(tmpctx,struct amount_msat,
-				&flows[i]->amounts[path_lengh-1]));
+				&flows[i]->amounts[path_lengh-1]),
+			      flows[i]->success_prob);
 		struct out_req *req;
 		req = jsonrpc_request_start(cmd->plugin, cmd, "sendpay",
 					    flow_sent, flow_sendpay_failed,
@@ -560,7 +552,7 @@ static struct command_result *try_paying(struct command *cmd,
 
 	// plugin_log(pay_plugin->plugin,LOG_DBG,fmt_chan_extra_map(tmpctx,pay_plugin->chan_extra_map));
 
-	char const * err_msg;
+	const char *err_msg;
 
 	/* We let this return an unlikely path, as it's better to try once
 	 * than simply refuse.  Plus, models are not truth! */

plugins/renepay/pay_flow.c

@@ -341,7 +341,7 @@ struct pay_flow **get_payflows(struct renepay * renepay,
 			       struct amount_msat feebudget,
 			       bool unlikely_ok,
 			       bool is_entire_payment,
-			       char const ** err_msg)
+			       const char **err_msg)
 {
 	*err_msg = tal_fmt(tmpctx,"[no error]");
 
@@ -396,6 +396,13 @@ struct pay_flow **get_payflows(struct renepay * renepay,
 		fee = flow_set_fee(flows);
 		delay = flows_worst_delay(flows) + p->final_cltv;
 
+		debug_paynote(p,
+			      "we have computed a set of %ld flows with probability %.3lf, fees %s and delay %ld",
+			      tal_count(flows),
+			      prob,
+			      type_to_string(tmpctx,struct amount_msat,&fee),
+			      delay);
+
 		too_unlikely = (prob < p->min_prob_success);
 		if (too_unlikely && !unlikely_ok)
 		{

plugins/renepay/pay_flow.h

@@ -69,7 +69,7 @@ static inline size_t payflow_key_hash(const struct payflow_key k)
 	return k.payment_hash->u.u32[0] ^ (k.groupid << 32) ^ k.partid;
 }
 
-static inline bool payflow_key_equal(struct pay_flow const *pf,
+static inline bool payflow_key_equal(const struct pay_flow *pf,
 				      const struct payflow_key k)
 {
 	return pf->key.partid==k.partid && pf->key.groupid==k.groupid
@@ -86,7 +86,7 @@ struct pay_flow **get_payflows(struct renepay * renepay,
 			       struct amount_msat feebudget,
 			       bool unlikely_ok,
 			       bool is_entire_payment,
-			       char const ** err_msg);
+			       const char **err_msg);
 
 void commit_htlc_payflow(
 		struct chan_extra_map *chan_extra_map,

plugins/renepay/payment.c

@@ -73,20 +73,20 @@ void payment_success(struct payment * p)
 	p->status=PAYMENT_SUCCESS;
 }
 
-struct amount_msat payment_sent(struct payment const * p)
+struct amount_msat payment_sent(const struct payment *p)
 {
 	return p->total_sent;
 }
-struct amount_msat payment_delivered(struct payment const * p)
+struct amount_msat payment_delivered(const struct payment *p)
 {
 	return p->total_delivering;
 }
-struct amount_msat payment_amount(struct payment const * p)
+struct amount_msat payment_amount(const struct payment *p)
 {
 	return p->amount;
 }
 
-struct amount_msat payment_fees(struct payment const*p)
+struct amount_msat payment_fees(const struct payment *p)
 {
 	struct amount_msat fees;
 	struct amount_msat sent = payment_sent(p),
@@ -111,7 +111,7 @@ void payment_note(struct payment *p, const char *fmt, ...)
 	debug_info("%s",str);
 }
 
-void payment_assert_delivering_incomplete(struct payment const * p)
+void payment_assert_delivering_incomplete(const struct payment *p)
 {
 	if(!amount_msat_less(p->total_delivering, p->amount))
 	{
@@ -121,7 +121,7 @@ void payment_assert_delivering_incomplete(struct payment const * p)
 			type_to_string(tmpctx,struct amount_msat,&p->amount));
 	}
 }
-void payment_assert_delivering_all(struct payment const * p)
+void payment_assert_delivering_all(const struct payment *p)
 {
 	if(amount_msat_less(p->total_delivering, p->amount))
 	{
@@ -194,7 +194,7 @@ struct command_result *renepay_fail(
 	return command_fail(renepay->cmd,code,"%s",message);
 }
 
-u64 renepay_parts(struct renepay const * renepay)
+u64 renepay_parts(const struct renepay *renepay)
 {
 	return renepay->next_partid-1;
 }

plugins/renepay/payment.h

@@ -137,14 +137,14 @@ void renepay_cleanup(
 
 void payment_fail(struct payment * p);
 void payment_success(struct payment * p);
-struct amount_msat payment_sent(struct payment const * p);
-struct amount_msat payment_delivered(struct payment const * p);
-struct amount_msat payment_amount(struct payment const * p);
-struct amount_msat payment_fees(struct payment const*p);
+struct amount_msat payment_sent(const struct payment *p);
+struct amount_msat payment_delivered(const struct payment *p);
+struct amount_msat payment_amount(const struct payment *p);
+struct amount_msat payment_fees(const struct payment *p);
 
 void payment_note(struct payment *p, const char *fmt, ...);
-void payment_assert_delivering_incomplete(struct payment const * p);
-void payment_assert_delivering_all(struct payment const * p);
+void payment_assert_delivering_incomplete(const struct payment *p);
+void payment_assert_delivering_all(const struct payment *p);
 
 
 int renepay_current_attempt(const struct renepay *renepay);
@@ -158,6 +158,6 @@ struct command_result *renepay_fail(
 	enum jsonrpc_errcode code,
 	const char *fmt, ...);
 
-u64 renepay_parts(struct renepay const * renepay);
+u64 renepay_parts(const struct renepay *renepay);
 
 #endif /* LIGHTNING_PLUGINS_RENEPAY_PAYMENT_H */

plugins/renepay/test/run-testflow.c

@@ -630,7 +630,7 @@ static void test_flow_complete(void)
 	struct amount_msat deliver;
 
 	// flow 1->2
-	F->path = tal_arr(F,struct gossmap_chan const *,1);
+	F->path = tal_arr(F,const struct gossmap_chan *,1);
 	F->dirs = tal_arr(F,int,1);
 	F->path[0]=gossmap_find_chan(gossmap,&scid12);
 	F->dirs[0]=0;
@@ -640,7 +640,7 @@ static void test_flow_complete(void)
 	assert(fabs(F->success_prob - 0.5)<eps);
 
 	// flow 3->4->5
-	F->path = tal_arr(F,struct gossmap_chan const *,2);
+	F->path = tal_arr(F,const struct gossmap_chan *,2);
 	F->dirs = tal_arr(F,int,2);
 	F->path[0]=gossmap_find_chan(gossmap,&scid34);
 	F->path[1]=gossmap_find_chan(gossmap,&scid45);
@@ -652,7 +652,7 @@ static void test_flow_complete(void)
 	assert(fabs(F->success_prob - 1.)<eps);
 
 	// flow 2->3->4->5
-	F->path = tal_arr(F,struct gossmap_chan const *,3);
+	F->path = tal_arr(F,const struct gossmap_chan *,3);
 	F->dirs = tal_arr(F,int,3);
 	F->path[0]=gossmap_find_chan(gossmap,&scid23);
 	F->path[1]=gossmap_find_chan(gossmap,&scid34);
@@ -666,7 +666,7 @@ static void test_flow_complete(void)
 	assert(fabs(F->success_prob - 1. + 250.087534/2000)<eps);
 
 	// flow 1->2->3->4->5
-	F->path = tal_arr(F,struct gossmap_chan const *,4);
+	F->path = tal_arr(F,const struct gossmap_chan *,4);
 	F->dirs = tal_arr(F,int,4);
 	F->path[0]=gossmap_find_chan(gossmap,&scid12);
 	F->path[1]=gossmap_find_chan(gossmap,&scid23);

plugins/renepay/uncertainty_network.c

@@ -86,7 +86,7 @@ void uncertainty_network_add_routehints(
 		struct chan_extra_map *chan_extra_map,
 		struct renepay *renepay)
 {
-	struct payment const * const p = renepay->payment;
+	const struct payment *const p = renepay->payment;
 	struct bolt11 *b11;
 	char *fail;
 

tests/test_renepay.py

@@ -75,7 +75,7 @@ def test_errors(node_factory, bitcoind):
                              .format(scid56),
                              r'update for channel {}/1 now ACTIVE'
                              .format(scid56)])
-    details = l1.rpc.call('renepay', {'invstring': inv, 'use_shadow': False})
+    details = l1.rpc.call('renepay', {'invstring': inv})
     assert details['status'] == 'complete'
     assert details['amount_msat'] == send_amount
     assert details['destination'] == l6.info['id']