Renepay refactors and cleanups

plugins/renepay/pay.c

@@ -490,19 +490,9 @@ static struct command_result *try_paying(struct command *cmd,
 	struct payment * const p = renepay->payment;
 	plugin_log(pay_plugin->plugin,LOG_DBG,"calling %s",__PRETTY_FUNCTION__);
 
-	// TODO(eduardo): does it make sense to have this limit on attempts?
-	/* I am classifying the flows in attempt cycles. */
-	renepay_new_attempt(renepay);
-	/* We try only MAX_NUM_ATTEMPTS, then we give up. */
-	if ( renepay_attempt_count(renepay) > MAX_NUM_ATTEMPTS)
-	{
-		return renepay_fail(renepay, PAY_STOPPED_RETRYING,
-				    "Reached maximum number of attempts (%d)",
-				    MAX_NUM_ATTEMPTS);
-	}
-
 	struct amount_msat feebudget, fees_spent, remaining;
 
+	renepay->payment->status=PAYMENT_PENDING;
 	if (time_after(time_now(), p->stop_time))
 		return renepay_fail(renepay, PAY_STOPPED_RETRYING, "Timed out");
 
@@ -556,6 +546,7 @@ static struct command_result *try_paying(struct command *cmd,
 
 	/* We let this return an unlikely path, as it's better to try once
 	 * than simply refuse.  Plus, models are not truth! */
+	gossmap_apply_localmods(pay_plugin->gossmap, renepay->local_gossmods);
 	struct pay_flow **pay_flows = get_payflows(
 						renepay,
 						remaining, feebudget,
@@ -568,6 +559,7 @@ static struct command_result *try_paying(struct command *cmd,
 						amount_msat_eq(p->total_delivering, AMOUNT_MSAT(0)),
 
 						&err_msg);
+	gossmap_remove_localmods(pay_plugin->gossmap, renepay->local_gossmods);
 
 	// plugin_log(pay_plugin->plugin,LOG_DBG,"get_payflows produced %s",fmt_payflows(tmpctx,pay_flows));
 
@@ -600,13 +592,9 @@ static struct command_result *listpeerchannels_done(
 				    "listpeerchannels malformed: %.*s",
 				    json_tok_full_len(result),
 				    json_tok_full(buf, result));
-	// So we have all localmods data, now we apply it. Only once per
-	// payment.
 	// TODO(eduardo): check that there won't be a prob. cost associated with
 	// any gossmap local chan. The same way there aren't fees to pay for my
 	// local channels.
-	gossmap_apply_localmods(pay_plugin->gossmap,renepay->local_gossmods);
-	renepay->localmods_applied=true;
 	return try_paying(cmd, renepay, true);
 }
 

plugins/renepay/pay_flow.c

@@ -244,7 +244,6 @@ static struct pay_flow **flows_to_pay_flows(struct payment *payment,
 		pf->amounts = tal_steal(pf, f->amounts);
 		pf->path_dirs = tal_steal(pf, f->dirs);
 		pf->success_prob = f->success_prob;
-		pf->attempt = renepay_current_attempt(payment->renepay);
 	}
 	tal_free(flows);
 

plugins/renepay/pay_flow.h

@@ -14,9 +14,6 @@ struct pay_flow {
 	/* So we can be an independent object for callbacks. */
 	struct payment * payment;
 
-	// TODO(eduardo): remove this, unnecessary
-	int attempt;
-
 	/* Information to link this flow to a unique sendpay. */
 	struct payflow_key
 	{

plugins/renepay/payment.c

@@ -49,13 +49,10 @@ struct renepay * renepay_new(struct command *cmd)
 
 	renepay->cmd = cmd;
 	renepay->payment = payment_new(renepay);
-	renepay->localmods_applied=false;
  	renepay->local_gossmods = gossmap_localmods_new(renepay);
 	renepay->disabled = tal_arr(renepay,struct short_channel_id,0);
 	renepay->rexmit_timer = NULL;
-	renepay->next_attempt=1;
 	renepay->next_partid=1;
-	renepay->all_flows = tal(renepay,tal_t);
 
 	return renepay;
 }
@@ -132,20 +129,6 @@ void payment_assert_delivering_all(const struct payment *p)
 	}
 }
 
-
-int renepay_current_attempt(const struct renepay * renepay)
-{
-	return renepay->next_attempt-1;
-}
-int renepay_attempt_count(const struct renepay * renepay)
-{
-	return renepay->next_attempt-1;
-}
-void renepay_new_attempt(struct renepay * renepay)
-{
-	renepay->payment->status=PAYMENT_PENDING;
-	renepay->next_attempt++;
-}
 struct command_result *renepay_success(struct renepay * renepay)
 {
 	debug_info("calling %s",__PRETTY_FUNCTION__);
@@ -206,21 +189,10 @@ void renepay_cleanup(
 		struct gossmap * gossmap)
 {
 	debug_info("calling %s",__PRETTY_FUNCTION__);
-	/* Always remove our local mods (routehints) so others can use
-	 * gossmap. We do this only after the payment completes. */
 	// TODO(eduardo): it can happen that local_gossmods removed below
 	// contained a set of channels for which there is information in the
 	// uncertainty network (chan_extra_map) and that are part of some pending
 	// payflow (payflow_map). Handle this situation.
-	if(renepay->localmods_applied)
-		gossmap_remove_localmods(gossmap,
-					 renepay->local_gossmods);
-	// TODO(eduardo): I wonder if it is possible to have two instances of
-	// renepay at the same time.
-	// 1st problem: dijkstra datastructure is global, this can be fixed,
-	// 2nd problem: we don't know if gossmap_apply_localmods and gossmap_remove_localmods,
-	// 	can handle different local_gossmods applied to the same gossmap.
-	renepay->localmods_applied=false;
 	tal_free(renepay->local_gossmods);
 
 	renepay->rexmit_timer = tal_free(renepay->rexmit_timer);

plugins/renepay/payment.h

@@ -111,7 +111,6 @@ struct renepay
 	struct payment * payment;
 
 	/* Localmods to apply to gossip_map for our own use. */
-	bool localmods_applied;
 	struct gossmap_localmods *local_gossmods;
 
 	/* Channels we decided to disable for various reasons. */
@@ -120,13 +119,8 @@ struct renepay
 	/* Timers. */
 	struct plugin_timer *rexmit_timer;
 
-	/* Keep track of the number of attempts. */
-	int next_attempt;
 	/* Used in get_payflows to set ids to each pay_flow. */
 	u64 next_partid;
-
-	/* Root to destroy pending flows */
-	tal_t *all_flows;
 };
 
 struct payment * payment_new(struct renepay *renepay);
@@ -146,11 +140,6 @@ void payment_note(struct payment *p, const char *fmt, ...);
 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);
-int renepay_attempt_count(const struct renepay *renepay);
-void renepay_new_attempt(struct renepay *renepay);
-
 struct command_result *renepay_success(struct renepay *renepay);
 
 struct command_result *renepay_fail(