diff --git a/common/dijkstra.c b/common/dijkstra.c index 1920017a8..53a17a9ef 100644 --- a/common/dijkstra.c +++ b/common/dijkstra.c @@ -4,21 +4,21 @@ #include #include #include +#include #include /* Each node has this side-info. */ struct dijkstra { + /* Number of hops: destination == 0, unreachable == UINTMAX */ u32 distance; - /* Total CLTV delay */ - u32 total_delay; - /* Total cost from here to destination */ - struct amount_msat cost; + /* Total amount from here to destination */ + struct amount_msat amount; /* I want to use an index here, except that gheap moves things onto * a temporary on the stack and that makes things complex. */ /* NULL means it's been visited already. */ const struct gossmap_node **heapptr; - /* How we decide "best", lower is better */ + /* How we decide "best", lower is better (this is the cost function output) */ u64 score; /* We could re-evaluate to determine this, but keeps it simple */ @@ -86,16 +86,14 @@ static const struct gossmap_node **mkheap(const tal_t *ctx, heap[0] = start; d->heapptr = &heap[0]; d->distance = 0; - d->total_delay = 0; - d->cost = sent; + d->amount = sent; d->score = 0; i--; } else { heap[i] = n; d->heapptr = &heap[i]; d->distance = UINT_MAX; - d->cost = AMOUNT_MSAT(-1ULL); - d->total_delay = 0; + d->amount = AMOUNT_MSAT(-1ULL); d->score = -1ULL; } } @@ -131,11 +129,11 @@ dijkstra_(const tal_t *ctx, int dir, struct amount_msat amount, void *arg), - u64 (*path_score)(u32 distance, - struct amount_msat cost, - struct amount_msat risk, - int dir, - const struct gossmap_chan *c), + u64 (*channel_score)(struct amount_msat fee, + struct amount_msat risk, + struct amount_msat total, + int dir, + const struct gossmap_chan *c), void *arg) { struct dijkstra *dij; @@ -216,7 +214,7 @@ dijkstra_(const tal_t *ctx, int which_half; struct gossmap_chan *c; struct dijkstra *d; - struct amount_msat cost, risk; + struct amount_msat fee, risk; u64 score; c = gossmap_nth_chan(map, cur, i, &which_half); @@ -228,28 +226,33 @@ dijkstra_(const tal_t *ctx, continue; /* We're going from neighbor to c, hence !which_half */ - if (!channel_ok(map, c, !which_half, cur_d->cost, arg)) + if (!channel_ok(map, c, !which_half, cur_d->amount, arg)) continue; - cost = cur_d->cost; - if (!amount_msat_add_fee(&cost, - c->half[!which_half].base_fee, - c->half[!which_half].proportional_fee)) + if (!amount_msat_fee(&fee, cur_d->amount, + c->half[!which_half].base_fee, + c->half[!which_half].proportional_fee)) { /* Shouldn't happen! */ continue; + } /* cltv_delay can't overflow: only 20 bits per hop. */ - risk = risk_price(cost, riskfactor, - cur_d->total_delay - + c->half[!which_half].delay); - score = path_score(cur_d->distance + 1, cost, risk, !which_half, c); + risk = risk_price(cur_d->amount, riskfactor, c->half[!which_half].delay); + score = channel_score(fee, risk, cur_d->amount, !which_half, c); + + /* That score is on top of current score */ + if (add_overflows_u64(score, cur_d->score)) + continue; + score += cur_d->score; + if (score >= d->score) continue; + /* Shouldn't happen! */ + if (!amount_msat_add(&d->amount, cur_d->amount, fee)) + continue; + d->distance = cur_d->distance + 1; - d->total_delay = cur_d->total_delay - + c->half[!which_half].delay; - d->cost = cost; d->best_chan = c; d->score = score; gheap_restore_heap_after_item_increase(&gheap_ctx, diff --git a/common/dijkstra.h b/common/dijkstra.h index 1a7d0a209..5dac1bec8 100644 --- a/common/dijkstra.h +++ b/common/dijkstra.h @@ -19,21 +19,22 @@ dijkstra_(const tal_t *ctx, int dir, struct amount_msat amount, void *arg), - u64 (*path_score)(u32 distance, - struct amount_msat cost, - struct amount_msat risk, - int dir, - const struct gossmap_chan *c), + u64 (*channel_score)(struct amount_msat fee, + struct amount_msat risk, + /* We want to know this to compare with channel capacity */ + struct amount_msat total, + int dir, + const struct gossmap_chan *c), void *arg); #define dijkstra(ctx, map, start, amount, riskfactor, channel_ok, \ - path_score, arg) \ + channel_score, arg) \ dijkstra_((ctx), (map), (start), (amount), (riskfactor), \ typesafe_cb_preargs(bool, void *, (channel_ok), (arg), \ const struct gossmap *, \ const struct gossmap_chan *, \ int, struct amount_msat), \ - (path_score), \ + (channel_score), \ (arg)) /* Returns UINT_MAX if unreachable. */ diff --git a/common/route.c b/common/route.c index 1fc5ffd3b..7ec8f7ba7 100644 --- a/common/route.c +++ b/common/route.c @@ -33,33 +33,33 @@ bool route_can_carry(const struct gossmap *map, } /* Squeeze total costs into a u32 */ -static u32 costs_to_score(struct amount_msat cost, +static u32 costs_to_score(struct amount_msat fee, struct amount_msat risk) { - u64 costs = cost.millisatoshis + risk.millisatoshis; /* Raw: score */ + u64 costs = fee.millisatoshis + risk.millisatoshis; /* Raw: score */ if (costs > 0xFFFFFFFF) costs = 0xFFFFFFFF; return costs; } /* Prioritize distance over costs */ -u64 route_score_shorter(u32 distance, - struct amount_msat cost, +u64 route_score_shorter(struct amount_msat fee, struct amount_msat risk, + struct amount_msat total UNUSED, int dir UNUSED, const struct gossmap_chan *c UNUSED) { - return costs_to_score(cost, risk) + ((u64)distance << 32); + return costs_to_score(fee, risk) + ((u64)1 << 32); } /* Prioritize costs over distance */ -u64 route_score_cheaper(u32 distance, - struct amount_msat cost, +u64 route_score_cheaper(struct amount_msat fee, struct amount_msat risk, + struct amount_msat total UNUSED, int dir UNUSED, const struct gossmap_chan *c UNUSED) { - return ((u64)costs_to_score(cost, risk) << 32) + distance; + return ((u64)costs_to_score(fee, risk) << 32) + 1; } /* Recursive version: return false if we can't get there. diff --git a/common/route.h b/common/route.h index 1bdb7f0f9..d6ea2b066 100644 --- a/common/route.h +++ b/common/route.h @@ -43,16 +43,16 @@ bool route_can_carry_even_disabled(const struct gossmap *map, void *unused); /* Shortest path, with lower amount tiebreak */ -u64 route_score_shorter(u32 distance, - struct amount_msat cost, +u64 route_score_shorter(struct amount_msat fee, struct amount_msat risk, + struct amount_msat total, int dir UNUSED, const struct gossmap_chan *c UNUSED); /* Cheapest path, with shorter path tiebreak */ -u64 route_score_cheaper(u32 distance, - struct amount_msat cost, +u64 route_score_cheaper(struct amount_msat fee, struct amount_msat risk, + struct amount_msat total, int dir UNUSED, const struct gossmap_chan *c UNUSED); diff --git a/common/test/run-route-infloop.c b/common/test/run-route-infloop.c index e317f6755..4fdd63de4 100644 --- a/common/test/run-route-infloop.c +++ b/common/test/run-route-infloop.c @@ -47,10 +47,10 @@ void towire_tlv(u8 **pptr UNNEEDED, /* Node id 03942f5fe67645fdce4584e7f159c1f0a396b05fbc15f0fb7d6e83c553037b1c73 */ static struct gossmap *gossmap; -static u64 capacity_bias(const struct gossmap *map, - const struct gossmap_chan *c, - int dir, - struct amount_msat amount) +static double capacity_bias(const struct gossmap *map, + const struct gossmap_chan *c, + int dir, + struct amount_msat amount) { struct amount_sat capacity; u64 amtmsat = amount.millisatoshis; /* Raw: lengthy math */ @@ -64,22 +64,41 @@ static u64 capacity_bias(const struct gossmap *map, return -log((capmsat + 1 - amtmsat) / (capmsat + 1)); } -static u64 route_score(u32 distance, - struct amount_msat cost, +/* Prioritize costs over distance, but bias to larger channels. */ +static u64 route_score(struct amount_msat fee, struct amount_msat risk, + struct amount_msat total, int dir, const struct gossmap_chan *c) { - u64 cmsat = cost.millisatoshis; /* Raw: lengthy math */ - u64 rmsat = risk.millisatoshis; /* Raw: lengthy math */ - u64 bias = capacity_bias(gossmap, c, dir, cost); + double score; + struct amount_msat msat; - /* Smoothed harmonic mean to avoid division by 0 */ - u64 costs = (cmsat * rmsat * bias) / (cmsat + rmsat + bias + 1); + /* These two are comparable, so simply sum them. */ + if (!amount_msat_add(&msat, fee, risk)) + msat = AMOUNT_MSAT(-1ULL); - if (costs > 0xFFFFFFFF) - costs = 0xFFFFFFFF; - return costs; + /* Slight tiebreaker bias: 1 msat per distance */ + if (!amount_msat_add(&msat, msat, AMOUNT_MSAT(1))) + msat = AMOUNT_MSAT(-1ULL); + + /* Percent penalty at different channel capacities: + * 1%: 1% + * 10%: 11% + * 25%: 29% + * 50%: 69% + * 75%: 138% + * 90%: 230% + * 95%: 300% + * 99%: 461% + */ + score = (capacity_bias(gossmap, c, dir, total) + 1) + * msat.millisatoshis; /* Raw: Weird math */ + if (score > 0xFFFFFFFF) + return 0xFFFFFFFF; + + /* Cast unnecessary, but be explicit! */ + return (u64)score; } int main(int argc, char *argv[]) @@ -92,9 +111,9 @@ int main(int argc, char *argv[]) common_setup(argv[0]); - /* 8388607 crashes, use half that so we don't break CI! */ + /* This used to crash! */ if (!argv[1]) - amt = 8388607 / 2; + amt = 8388607; else amt = atol(argv[1]); gossmap = gossmap_load(tmpctx, "tests/data/routing_gossip_store", NULL); diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 9a3204006..0a6d8ef71 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -765,21 +765,21 @@ static double capacity_bias(const struct gossmap *map, } /* Prioritize costs over distance, but bias to larger channels. */ -static u64 route_score(u32 distance, - struct amount_msat cost, +static u64 route_score(struct amount_msat fee, struct amount_msat risk, + struct amount_msat total, int dir, const struct gossmap_chan *c) { - double costs; + double score; struct amount_msat msat; /* These two are comparable, so simply sum them. */ - if (!amount_msat_add(&msat, cost, risk)) + if (!amount_msat_add(&msat, fee, risk)) msat = AMOUNT_MSAT(-1ULL); /* Slight tiebreaker bias: 1 msat per distance */ - if (!amount_msat_add(&msat, msat, amount_msat(distance))) + if (!amount_msat_add(&msat, msat, AMOUNT_MSAT(1))) msat = AMOUNT_MSAT(-1ULL); /* Percent penalty at different channel capacities: @@ -792,11 +792,13 @@ static u64 route_score(u32 distance, * 95%: 300% * 99%: 461% */ - costs = (capacity_bias(global_gossmap, c, dir, cost) + 1) + score = (capacity_bias(global_gossmap, c, dir, total) + 1) * msat.millisatoshis; /* Raw: Weird math */ - if (costs > 0xFFFFFFFF) + if (score > 0xFFFFFFFF) return 0xFFFFFFFF; - return (u64)costs; + + /* Cast unnecessary, but be explicit! */ + return (u64)score; } static struct route_hop *route(const tal_t *ctx,