Skip to content

Commit 703d737

Browse files
rustyrussellmadelinevibes
authored andcommitted
xpay: don't crash on circular routehints.
We earlier fixed the cases of gossipd inserting a same-node channel, but didn't prevent it for local modifications: ``` cln-askrene: common/gossmap.c:52: nodeidx_htable_add: Assertion `!nodeidx_htable_getmatch_(ht, k, h, v, &i)' failed. cln-askrene: FATAL SIGNAL 6 (version v26.06rc2-5-gd389c3f-modded) 0x5c50e80dd5cb send_backtrace common/daemon.c:38 0x5c50e80dd685 crashdump common/daemon.c:83 0x70a5b1e4532f ??? ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0 0x70a5b1e9eb2c __pthread_kill_implementation ./nptl/pthread_kill.c:44 0x70a5b1e9eb2c __pthread_kill_internal ./nptl/pthread_kill.c:78 0x70a5b1e9eb2c __GI___pthread_kill ./nptl/pthread_kill.c:89 0x70a5b1e4527d __GI_raise ../sysdeps/posix/raise.c:26 0x70a5b1e288fe __GI_abort ./stdlib/abort.c:79 0x70a5b1e2881a __assert_fail_base ./assert/assert.c:96 0x70a5b1e3b516 __assert_fail ./assert/assert.c:105 0x5c50e80dfb44 nodeidx_htable_add common/gossmap.c:52 0x5c50e80e1066 add_channel common/gossmap.c:515 0x5c50e80e327c gossmap_apply_localmods common/gossmap.c:1239 0x5c50e80bed55 do_getroutes plugins/askrene/askrene.c:620 0x5c50e80bf919 listpeerchannels_done ``` Reported-by: Won Hoi Kim and Ahmad Elmoursi Changelog-Fixed: Plugins: xpay no longer crashes on circular bolt11 routehints. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1 parent 19808e4 commit 703d737

9 files changed

Lines changed: 21 additions & 4 deletions

File tree

‎common/gossmap.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,8 @@ bool gossmap_local_addchan(struct gossmap_localmods *localmods,
10561056
u64 off;
10571057
struct localmod mod;
10581058

1059+
assert(!node_id_eq(n1, n2));
1060+
10591061
/* Don't create duplicate channels. */
10601062
if (find_localmod(localmods, scid))
10611063
return false;

‎common/gossmap.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ struct gossmap_localmods *gossmap_localmods_new(const tal_t *ctx);
8989
/* Create a local-only channel; if this conflicts with a real channel when added,
9090
* that will be used instead.
9191
* Returns false (and does nothing) if scid was already in localmods.
92+
* n1 must be different from n2.
9293
*/
9394
bool gossmap_local_addchan(struct gossmap_localmods *localmods,
9495
const struct node_id *n1,

‎contrib/msggen/msggen/schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@
652652
"destination": {
653653
"type": "pubkey",
654654
"description": [
655-
"The destination node id for the channel."
655+
"The destination node id for the channel (must be different from source)."
656656
]
657657
},
658658
"short_channel_id": {

‎doc/schemas/askrene-create-channel.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"destination": {
3333
"type": "pubkey",
3434
"description": [
35-
"The destination node id for the channel."
35+
"The destination node id for the channel (must be different from source)."
3636
]
3737
},
3838
"short_channel_id": {

‎plugins/askrene/askrene.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,10 @@ static struct command_result *json_askrene_create_channel(struct command *cmd,
10661066
plugin_log(cmd->plugin, LOG_TRACE, "%s called: %.*s", __func__,
10671067
json_tok_full_len(params), json_tok_full(buffer, params));
10681068

1069+
if (node_id_cmp(src, dst) == 0)
1070+
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
1071+
"source and destination must be different");
1072+
10691073
if (layer_find_local_channel(layer, *scid)) {
10701074
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
10711075
"channel already exists");

‎plugins/askrene/layer.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ static struct local_channel *add_local_channel(struct layer *layer,
234234
lc->n1 = *n1;
235235
lc->n2 = *n2;
236236
} else {
237+
assert(!node_id_eq(n1, n2));
237238
lc->n1 = *n2;
238239
lc->n2 = *n1;
239240
}

‎plugins/askrene/layer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ bool layer_check_local_channel(const struct local_channel *lc,
5252
const struct node_id *n2,
5353
struct amount_msat capacity);
5454

55-
/* Add a local channel to a layer! */
55+
/* Add a local channel to a layer: src must not be equal to dst!*/
5656
void layer_add_local_channel(struct layer *layer,
5757
const struct node_id *src,
5858
const struct node_id *dst,

‎plugins/xpay/xpay.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,6 +1922,16 @@ static void add_fake_channel(struct command *aux_cmd,
19221922
struct out_req *req;
19231923
struct short_channel_id_dir scidd;
19241924

1925+
/* We're not allowed to send these to askrene-create-channel,
1926+
* so catch them now */
1927+
if (node_id_eq(src, dst)) {
1928+
payment_log(payment, LOG_UNUSUAL,
1929+
"Invoice gave bad self-node route %s->%s",
1930+
fmt_node_id(tmpctx, src),
1931+
fmt_node_id(tmpctx, dst));
1932+
return;
1933+
}
1934+
19251935
scidd.scid = scid;
19261936
scidd.dir = node_id_idx(src, dst);
19271937
payment_log(payment, LOG_DBG,

‎tests/test_xpay.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,6 @@ def test_xpay_offer(node_factory):
975975
l1.rpc.xpay(offer2, 5000)
976976

977977

978-
@pytest.mark.xfail(strict=True)
979978
def test_xpay_circular_routehint(node_factory):
980979
"""Test that xpay gracefully skips a circular bolt11 routehint (src == dst)."""
981980
l1, l2 = node_factory.line_graph(2)

0 commit comments

Comments
 (0)