From 3a61e0e18182b3e5ba704f4386ecceb333eb9df6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 10 Jul 2022 16:16:37 +0930 Subject: [PATCH] invoice: turn assert failure into more informative log_broken message. We had this assertion fail, and I can't see a clear reason why. Remove it (i.e. don't crash because we're having trouble with creating invoice routehints) and add logging. ``` Assertion failed: a->c->rr_number < b->c->rr_number (lightningd/invoice.c: cmp_rr_number: 623) lightningd: FATAL SIGNAL 6 (version v0.10.2-modded) 0x5654f1c40061 send_backtrace common/daemon.c:33 0x5654f1c400e9 crashdump common/daemon.c:46 0x7efd87da6c8a ??? ???:0 ``` There are several possible causes for this: 1. We have two channels with the same rr_number. A quick audit shows we always set that rr_number to a unique value (and 64 bits, so wrap is not possible between the release and now!). 2. It's theoretically possible that sort() could compare a value with itself, but that would be really dumb: it doesn't that I've ever seen, but then, we've never seen this assert() hit, either. 3. listincoming has given us the same channel twice. I don't see how that is possible: we had a race where channels could momentarily vanish, but never be duplicated (emailed crash.log shows no duplicates!). 4. General corruption/freed memory access. If a channel we've just looked up is gone but still in the hash table, this is possible but would cause lots of random behavior and crashes. Signed-off-by: Rusty Russell --- lightningd/invoice.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 08dbde612..affd55edf 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -617,13 +617,17 @@ static struct route_info **select_inchan(const tal_t *ctx, static int cmp_rr_number(const struct routehint_candidate *a, const struct routehint_candidate *b, - void *unused) + struct lightningd *ld) { - /* They're unique, so can't be equal */ if (a->c->rr_number > b->c->rr_number) return 1; - assert(a->c->rr_number < b->c->rr_number); - return -1; + if (a->c->rr_number < b->c->rr_number) + return -1; + + /* They're unique, so can't be equal */ + log_broken(ld->log, "Two equal candidates %p and %p, channels %p and %p, rr_number %"PRIu64" and %"PRIu64, + a, b, a->c, b->c, a->c->rr_number, b->c->rr_number); + return 0; } /** select_inchan_mpp @@ -650,7 +654,7 @@ static struct route_info **select_inchan_mpp(const tal_t *ctx, routehints = tal_arr(ctx, struct route_info *, 0); /* Sort by rr_number, so we get fresh channels. */ - asort(candidates, tal_count(candidates), cmp_rr_number, NULL); + asort(candidates, tal_count(candidates), cmp_rr_number, ld); for (size_t i = 0; i < tal_count(candidates); i++) { if (amount_msat_greater_eq(gathered, amount_needed)) break;