renepay: add precondition check + error msg

- add more checks
- add more error messages
- compute probabilities without fees during MCF
- compute probabilities with fees during get_routes

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>

plugins/renepay/flow.c

@@ -14,7 +14,8 @@
 #define SUPERVERBOSE_ENABLED 1
 #endif
 
-struct amount_msat *tal_flow_amounts(const tal_t *ctx, const struct flow *flow)
+struct amount_msat *tal_flow_amounts(const tal_t *ctx, const struct flow *flow,
+				     bool compute_fees)
 {
 	const size_t pathlen = tal_count(flow->path);
 	struct amount_msat *amounts = tal_arr(ctx, struct amount_msat, pathlen);
@@ -23,7 +24,8 @@ struct amount_msat *tal_flow_amounts(const tal_t *ctx, const struct flow *flow)
 	for (int i = (int)pathlen - 2; i >= 0; i--) {
 		const struct half_chan *h = flow_edge(flow, i + 1);
 		amounts[i] = amounts[i + 1];
-		if (!amount_msat_add_fee(&amounts[i], h->base_fee,
+		if (compute_fees &&
+		    !amount_msat_add_fee(&amounts[i], h->base_fee,
 					 h->proportional_fee))
 			goto function_fail;
 	}
@@ -39,11 +41,7 @@ const char *fmt_flows(const tal_t *ctx, const struct gossmap *gossmap,
 		      struct flow **flows)
 {
 	tal_t *this_ctx = tal(ctx, tal_t);
-	double tot_prob =
-	    flowset_probability(tmpctx, flows, gossmap, chan_extra_map, NULL);
-	assert(tot_prob >= 0);
-	char *buff = tal_fmt(ctx, "%zu subflows, prob %2lf\n", tal_count(flows),
-			     tot_prob);
+	char *buff = tal_fmt(ctx, "%zu subflows\n", tal_count(flows));
 	for (size_t i = 0; i < tal_count(flows); i++) {
 		struct amount_msat fee, delivered;
 		tal_append_fmt(&buff, "   ");
@@ -233,11 +231,20 @@ struct chan_inflight_flow
 	struct amount_msat half[2];
 };
 
+/* @ctx: allocator
+ * @flows: flows for which the probability is computed
+ * @gossmap: gossip
+ * @chan_extra_map: knowledge
+ * @compute_fees: compute fees along the way or not
+ * @fail: if a failure occurs, returns a message to the caller
+ * */
 // TODO(eduardo): here chan_extra_map should be const
 // TODO(eduardo): here flows should be const
 double flowset_probability(const tal_t *ctx, struct flow **flows,
 			   const struct gossmap *const gossmap,
-			   struct chan_extra_map *chan_extra_map, char **fail)
+			   struct chan_extra_map *chan_extra_map,
+			   bool compute_fees,
+			   char **fail)
 {
 	assert(flows);
 	assert(gossmap);
@@ -251,6 +258,12 @@ double flowset_probability(const tal_t *ctx, struct flow **flows,
 	struct chan_inflight_flow *in_flight =
 	    tal_arr(this_ctx, struct chan_inflight_flow, max_num_chans);
 
+	if (!in_flight) {
+		if (fail)
+			*fail = tal_fmt(ctx, "failed to allocate memory");
+		goto function_fail;
+	}
+
 	for (size_t i = 0; i < max_num_chans; ++i) {
 		in_flight[i].half[0] = in_flight[i].half[1] = AMOUNT_MSAT(0);
 	}
@@ -258,7 +271,8 @@ double flowset_probability(const tal_t *ctx, struct flow **flows,
 	for (size_t i = 0; i < tal_count(flows); ++i) {
 		const struct flow *f = flows[i];
 		const size_t pathlen = tal_count(f->path);
-		struct amount_msat *amounts = tal_flow_amounts(this_ctx, f);
+		struct amount_msat *amounts =
+		    tal_flow_amounts(this_ctx, f, compute_fees);
 		if (!amounts)
 		{
 			if (fail)
@@ -284,18 +298,32 @@ double flowset_probability(const tal_t *ctx, struct flow **flows,
 
 			const struct amount_msat deliver = amounts[j];
 
-			struct amount_msat prev_flow;
+			struct amount_msat prev_flow, all_inflight;
 			if (!amount_msat_add(&prev_flow, h->htlc_total,
-					     in_flight[c_idx].half[c_dir])) {
+					     in_flight[c_idx].half[c_dir]) ||
+			    !amount_msat_add(&all_inflight, prev_flow,
+					     deliver)) {
 				if (fail)
-				*fail = tal_fmt(
-				    ctx, "in-flight amount_msat overflow");
+					*fail = tal_fmt(
+					    ctx,
+					    "in-flight amount_msat overflow");
 				goto function_fail;
 			}
 
-			double edge_prob =
-			    edge_probability(h->known_min, h->known_max,
-					     prev_flow, deliver);
+			if (!amount_msat_less_eq(all_inflight, h->known_max)) {
+				if (fail)
+					*fail = tal_fmt(
+					    ctx,
+					    "in-flight (%s) exceeds known_max "
+					    "(%s)",
+					    fmt_amount_msat(ctx, all_inflight),
+					    fmt_amount_msat(ctx, h->known_max));
+				goto function_fail;
+			}
+
+			double edge_prob = edge_probability(
+			    h->known_min, h->known_max, prev_flow, deliver);
+
 			if (edge_prob < 0) {
 				if (fail)
 				*fail = tal_fmt(ctx,
@@ -406,7 +434,8 @@ bool flow_assign_delivery(struct flow *flow, const struct gossmap *gossmap,
  * success provided that there are no other flows in the current MPP flow set.
  * */
 double flow_probability(struct flow *flow, const struct gossmap *gossmap,
-			struct chan_extra_map *chan_extra_map)
+			struct chan_extra_map *chan_extra_map,
+			bool compute_fees)
 {
 	assert(flow);
 	assert(gossmap);
@@ -425,8 +454,8 @@ double flow_probability(struct flow *flow, const struct gossmap *gossmap,
 
 		if (prob < 0)
 			goto function_fail;
-		if (!amount_msat_add_fee(&spend, h->base_fee,
-					 h->proportional_fee))
+		if (compute_fees && !amount_msat_add_fee(&spend, h->base_fee,
+							 h->proportional_fee))
 			goto function_fail;
 	}
 

plugins/renepay/flow.h

@@ -43,7 +43,8 @@ double flow_edge_cost(const struct gossmap *gossmap,
 /* Compute the prob. of success of a set of concurrent set of flows. */
 double flowset_probability(const tal_t *ctx, struct flow **flows,
 			   const struct gossmap *const gossmap,
-			   struct chan_extra_map *chan_extra_map, char **fail);
+			   struct chan_extra_map *chan_extra_map,
+			   bool compute_fees, char **fail);
 
 /* How much do we need to send to make this flow arrive. */
 bool flow_spend(struct amount_msat *ret, struct flow *flow);
@@ -63,7 +64,8 @@ static inline struct amount_msat flow_delivers(const struct flow *flow)
 	return flow->amount;
 }
 
-struct amount_msat *tal_flow_amounts(const tal_t *ctx, const struct flow *flow);
+struct amount_msat *tal_flow_amounts(const tal_t *ctx, const struct flow *flow,
+				     bool compute_fees);
 
 enum renepay_errorcode
 flow_maximum_deliverable(struct amount_msat *max_deliverable,
@@ -79,7 +81,8 @@ bool flow_assign_delivery(struct flow *flow, const struct gossmap *gossmap,
 			  struct amount_msat requested_amount);
 
 double flow_probability(struct flow *flow, const struct gossmap *gossmap,
-			struct chan_extra_map *chan_extra_map);
+			struct chan_extra_map *chan_extra_map,
+			bool compute_fees);
 
 u64 flow_delay(const struct flow *flow);
 u64 flows_worst_delay(struct flow **flows);

plugins/renepay/mcf.c

@@ -1449,9 +1449,8 @@ get_flow_paths(const tal_t *ctx, const struct gossmap *gossmap,
 			excess = amount_msat(0);
 			fp->amount = delivered;
 
-
 			fp->success_prob =
-			    flow_probability(fp, gossmap, chan_extra_map)
+			    flow_probability(fp, gossmap, chan_extra_map, false)
 			    * pow(base_probability, tal_count(fp->path));
 			if (fp->success_prob < 0) {
 				if (fail)
@@ -1737,12 +1736,14 @@ struct flow **minflow(const tal_t *ctx, struct gossmap *gossmap,
 
 	best_prob_success =
 	    flowset_probability(this_ctx, best_flow_paths, params->gossmap,
-				params->chan_extra_map, &errmsg)
+				params->chan_extra_map, false, &errmsg)
 	    * pow(params->base_probability, flowset_size(best_flow_paths));
 	if (best_prob_success < 0) {
 		if (fail)
-		*fail =
-		    tal_fmt(ctx, "flowset_probability failed: %s", errmsg);
+			*fail = tal_fmt(
+			    ctx,
+			    "flowset_probability failed on MaxFlow phase: %s",
+			    errmsg);
 		goto function_fail;
 	}
 	if (!flowset_fee(&best_fee, best_flow_paths)) {
@@ -1795,7 +1796,7 @@ struct flow **minflow(const tal_t *ctx, struct gossmap *gossmap,
 
 		double prob_success =
 		    flowset_probability(this_ctx, flow_paths, params->gossmap,
-					params->chan_extra_map, &errmsg)
+					params->chan_extra_map, false, &errmsg)
 		    * pow(params->base_probability, flowset_size(flow_paths));
 		if (prob_success < 0) {
 			// flowset_probability doesn't fail unless there is a bug.

plugins/renepay/mods.c

@@ -654,7 +654,8 @@ static struct command_result *compute_routes_cb(struct payment *payment)
 
 	/* How much are we still trying to send? */
 	if (!amount_msat_sub(&remaining, payment->payment_info.amount,
-			     payment->total_delivering)) {
+			     payment->total_delivering) ||
+	    amount_msat_zero(remaining)) {
 		plugin_log(pay_plugin->plugin, LOG_UNUSUAL,
 			   "%s: Payment is pending with full amount already "
 			   "committed. We skip the computation of new routes.",

plugins/renepay/routebuilder.c

@@ -166,6 +166,11 @@ struct route **get_routes(const tal_t *ctx,
 				 "Failed to build disabled_bitmap.");
 		goto function_fail;
 	}
+	if (amount_msat_zero(amount_to_deliver)) {
+		tal_report_error(ctx, ecode, fail, PLUGIN_ERROR,
+				 "amount to deliver is zero");
+		goto function_fail;
+	}
 
 	/* Also disable every channel that we don't have in the chan_extra_map.
 	 * We might have channels in the gossmap that are not usable for
@@ -280,7 +285,7 @@ struct route **get_routes(const tal_t *ctx,
 
 			const double prob = flow_probability(
 			    flows[i], gossmap,
-			    uncertainty_get_chan_extra_map(uncertainty));
+			    uncertainty_get_chan_extra_map(uncertainty), true);
 			if (prob < 0) {
 				// should not happen
 				tal_report_error(ctx, ecode, fail, PLUGIN_ERROR,

plugins/renepay/test/common.h

@@ -18,7 +18,8 @@ static const char *print_routes(const tal_t *ctx,
 		delivered = route_delivers(routes[i]);
 		fee = route_fees(routes[i]);
 		tal_append_fmt(&buff, "   %s", fmt_route_path(this_ctx, routes[i]));
-		tal_append_fmt(&buff, " %s delivered with fee %s\n",
+		tal_append_fmt(&buff, " prob %.2f, %s delivered with fee %s\n",
+			       routes[i]->success_prob,
 			       fmt_amount_msat(this_ctx, delivered),
 			       fmt_amount_msat(this_ctx, fee));
 	}

plugins/renepay/test/run-bottleneck.c

@@ -34,7 +34,7 @@ static const char *print_flows(const tal_t *ctx, const char *desc,
 {
 	tal_t *this_ctx = tal(ctx, tal_t);
 	double tot_prob =
-	    flowset_probability(tmpctx, flows, gossmap, chan_extra_map, NULL);
+	    flowset_probability(tmpctx, flows, gossmap, chan_extra_map, false, NULL);
 	assert(tot_prob >= 0);
 	char *buff = tal_fmt(ctx, "%s: %zu subflows, prob %2lf\n", desc,
 			     tal_count(flows), tot_prob);
@@ -190,7 +190,8 @@ int main(int argc, char *argv[])
 	assert(skipped_count==0);
 
 	bitmap *disabled = tal_arrz(
- 	    tmpctx, bitmap, BITMAP_NWORDS(gossmap_max_chan_idx(gossmap)));
+	    tmpctx, bitmap, 2 * BITMAP_NWORDS(gossmap_max_chan_idx(gossmap)));
+	assert(disabled);
 
 	char *errmsg;
  	struct flow **flows;
@@ -208,11 +209,11 @@ int main(int argc, char *argv[])
  		    /* prob cost factor = */ 10, &errmsg);
 
 	if (!flows) {
-  		printf("Minflow has failed with: %s", errmsg);
-  		// assert(0 && "minflow failed");
-  	}
+		printf("Minflow has failed with: %s\n", errmsg);
+		assert(flows);
+	}
 
- 	if(flows)
+	if(flows)
   	printf("%s\n", print_flows(tmpctx, "Simple minflow", gossmap,
   				   uncertainty->chan_extra_map, flows));
 
@@ -274,10 +275,9 @@ int main(int argc, char *argv[])
 		&errcode,
 		&err_msg);
 
-	assert(routes);
-
 	if (!routes) {
-		printf("get_route failed with error %d: %s", errcode, err_msg);
+		printf("get_route failed with error %d: %s\n", errcode, err_msg);
+		assert(routes);
 	}
  	if(routes)
   	printf("get_routes: %s\n", print_routes(tmpctx, routes));

plugins/renepay/test/run-mcf-diamond.c

@@ -69,7 +69,7 @@ static const char* print_flows(
 {
 	tal_t *this_ctx = tal(ctx,tal_t);
 	double tot_prob =
-	    flowset_probability(tmpctx, flows, gossmap, chan_extra_map, NULL);
+	    flowset_probability(tmpctx, flows, gossmap, chan_extra_map, false, NULL);
 	assert(tot_prob>=0);
 	char *buff = tal_fmt(ctx,"%s: %zu subflows, prob %2lf\n", desc, tal_count(flows),tot_prob);
 	for (size_t i = 0; i < tal_count(flows); i++) {

plugins/renepay/test/run-mcf.c

@@ -423,7 +423,7 @@ int main(int argc, char *argv[])
 
 	printf("Checking results.\n");
 	/* Should go 1->2->3 */
-	amounts = tal_flow_amounts(tmpctx, flows[0]);
+	amounts = tal_flow_amounts(tmpctx, flows[0], true);
 	assert(amounts);
 	assert(tal_count(flows) == 1);
 	assert(tal_count(flows[0]->path) == 2);
@@ -440,7 +440,7 @@ int main(int argc, char *argv[])
 
 	/* Each one has probability ~ 0.5 */
 	assert(flows[0]->success_prob > 0.249);
-	assert(flows[0]->success_prob <= 0.250);
+	assert(flows[0]->success_prob <= 0.251);
 
 
 	/* Should have filled in some extra data! */

plugins/renepay/test/run-testflow.c

@@ -695,7 +695,7 @@ static void test_flow_to_route(void)
 	assert(route);
 
 	assert(amount_msat_eq(route->hops[0].amount, deliver));
-	assert(fabs(flow_probability(F, gossmap, chan_extra_map) - 0.5)<eps);
+	assert(fabs(flow_probability(F, gossmap, chan_extra_map, true) - 0.5)<eps);
 
 	// flow 3->4->5
 	F = tal(this_ctx, struct flow);
@@ -711,7 +711,7 @@ static void test_flow_to_route(void)
 	assert(route);
 
 	assert(amount_msat_eq(route->hops[0].amount, amount_msat(250050016)));
-	assert(fabs(flow_probability(F, gossmap, chan_extra_map) - 1.)<eps);
+	assert(fabs(flow_probability(F, gossmap, chan_extra_map, true) - 1.)<eps);
 
 	// flow 2->3->4->5
 	F = tal(this_ctx, struct flow);
@@ -729,7 +729,7 @@ static void test_flow_to_route(void)
 	assert(route);
 
 	assert(amount_msat_eq(route->hops[0].amount, amount_msat(250087534)));
-	assert(fabs(flow_probability(F, gossmap, chan_extra_map) - 1. + 250.087534/2000)<eps);
+	assert(fabs(flow_probability(F, gossmap, chan_extra_map, true) - 1. + 250.087534/2000)<eps);
 
 	// flow 1->2->3->4->5
 	F = tal(this_ctx, struct flow);
@@ -749,7 +749,7 @@ static void test_flow_to_route(void)
 	assert(route);
 
 	assert(amount_msat_eq(route->hops[0].amount, amount_msat(250112544)));
-	assert(fabs(flow_probability(F, gossmap, chan_extra_map) - 0.43728117)<eps);
+	assert(fabs(flow_probability(F, gossmap, chan_extra_map, true) - 0.43728117)<eps);
 
 	tal_free(this_ctx);
 }