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 <eduardo.quintana@pm.me>
This commit is contained in:
Lagrang3
2023-07-31 11:21:25 +09:30
committed by Rusty Russell
parent b5da85e85d
commit 3024afe524
11 changed files with 62 additions and 60 deletions

View File

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

View File

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

View File

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

View File

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

View File

@@ -341,7 +341,7 @@ struct pay_flow **get_payflows(struct renepay * renepay,
struct amount_msat feebudget, struct amount_msat feebudget,
bool unlikely_ok, bool unlikely_ok,
bool is_entire_payment, bool is_entire_payment,
char const ** err_msg) const char **err_msg)
{ {
*err_msg = tal_fmt(tmpctx,"[no error]"); *err_msg = tal_fmt(tmpctx,"[no error]");
@@ -396,6 +396,13 @@ struct pay_flow **get_payflows(struct renepay * renepay,
fee = flow_set_fee(flows); fee = flow_set_fee(flows);
delay = flows_worst_delay(flows) + p->final_cltv; 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); too_unlikely = (prob < p->min_prob_success);
if (too_unlikely && !unlikely_ok) if (too_unlikely && !unlikely_ok)
{ {

View File

@@ -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; 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) const struct payflow_key k)
{ {
return pf->key.partid==k.partid && pf->key.groupid==k.groupid 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, struct amount_msat feebudget,
bool unlikely_ok, bool unlikely_ok,
bool is_entire_payment, bool is_entire_payment,
char const ** err_msg); const char **err_msg);
void commit_htlc_payflow( void commit_htlc_payflow(
struct chan_extra_map *chan_extra_map, struct chan_extra_map *chan_extra_map,

View File

@@ -73,20 +73,20 @@ void payment_success(struct payment * p)
p->status=PAYMENT_SUCCESS; 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; 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; 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; 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 fees;
struct amount_msat sent = payment_sent(p), struct amount_msat sent = payment_sent(p),
@@ -111,7 +111,7 @@ void payment_note(struct payment *p, const char *fmt, ...)
debug_info("%s",str); 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)) 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)); 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)) 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); 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; return renepay->next_partid-1;
} }

View File

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

View File

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

View File

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

View File

@@ -75,7 +75,7 @@ def test_errors(node_factory, bitcoind):
.format(scid56), .format(scid56),
r'update for channel {}/1 now ACTIVE' r'update for channel {}/1 now ACTIVE'
.format(scid56)]) .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['status'] == 'complete'
assert details['amount_msat'] == send_amount assert details['amount_msat'] == send_amount
assert details['destination'] == l6.info['id'] assert details['destination'] == l6.info['id']