-
Notifications
You must be signed in to change notification settings - Fork 871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wallet: do not set the null value as sci alias #7049
wallet: do not set the null value as sci alias #7049
Conversation
Looks like that we store in the database the local_alias, and for older channels there are some value NULL This patch will fix the following stacktrace ORMAL: connecting subd 2024-02-06T20:55:25.243Z DEBUG 02312627fdf07fbdd7e5ddb136611bdde9b00d26821d14d94891395452f67af248-channeld-chan#77: pid 148407, msgfd 83 2024-02-06T20:55:25.245Z DEBUG 02312627fdf07fbdd7e5ddb136611bdde9b00d26821d14d94891395452f67af248-chan#77: Already have funding locked in 2024-02-06T20:55:25.246Z UNUSUAL 02312627fdf07fbdd7e5ddb136611bdde9b00d26821d14d94891395452f67af248-chan#77: Ignoring fee limits! 2024-02-06T20:55:27.153Z **BROKEN** lightningd: FATAL SIGNAL 11 (version v23.11-312-g4fc9e71) 2024-02-06T20:55:27.155Z **BROKEN** lightningd: backtrace: common/daemon.c:38 (send_backtrace) 0x19f379 2024-02-06T20:55:27.171Z **BROKEN** lightningd: backtrace: common/daemon.c:75 (crashdump) 0x19f3b9 2024-02-06T20:55:27.172Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x4b4776f 2024-02-06T20:55:27.173Z **BROKEN** lightningd: backtrace: bitcoin/short_channel_id.c:96 (towire_short_channel_id) 0x1bac19 2024-02-06T20:55:27.174Z **BROKEN** lightningd: backtrace: channeld/channeld_wiregen.c:287 (towire_channeld_init) 0x1d9b22 2024-02-06T20:55:27.176Z **BROKEN** lightningd: backtrace: lightningd/channel_control.c:1588 (peer_start_channeld) 0x12de3c 2024-02-06T20:55:27.177Z **BROKEN** lightningd: backtrace: lightningd/peer_control.c:1309 (connect_activate_subd) 0x161d76 2024-02-06T20:55:27.178Z **BROKEN** lightningd: backtrace: lightningd/peer_control.c:1408 (peer_connected_hook_final) 0x164a2d 2024-02-06T20:55:27.179Z **BROKEN** lightningd: backtrace: lightningd/plugin_hook.c:194 (plugin_hook_call_next) 0x1746ea 2024-02-06T20:55:27.180Z **BROKEN** lightningd: backtrace: lightningd/plugin_hook.c:169 (plugin_hook_callback) 0x1748aa 2024-02-06T20:55:27.181Z **BROKEN** lightningd: backtrace: lightningd/plugin.c:662 (plugin_response_handle) 0x16ef6f 2024-02-06T20:55:27.182Z **BROKEN** lightningd: backtrace: lightningd/plugin.c:774 (plugin_read_json_one) 0x17274d 2024-02-06T20:55:27.183Z **BROKEN** lightningd: backtrace: lightningd/plugin.c:825 (plugin_read_json) 0x1729e6 2024-02-06T20:55:27.184Z **BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:59 (next_plan) 0x30929a 2024-02-06T20:55:27.185Z **BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:407 (do_plan) 0x309721 2024-02-06T20:55:27.186Z **BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:417 (io_ready) 0x3097ba 2024-02-06T20:55:27.187Z **BROKEN** lightningd: backtrace: ccan/ccan/io/poll.c:453 (io_loop) 0x30b01a 2024-02-06T20:55:27.188Z **BROKEN** lightningd: backtrace: lightningd/io_loop_with_timers.c:22 (io_loop_with_timers) 0x146d27 2024-02-06T20:55:27.189Z **BROKEN** lightningd: backtrace: lightningd/lightningd.c:1420 (main) 0x14c270 2024-02-06T20:55:27.190Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x4b30ccf 2024-02-06T20:55:27.191Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x4b30d89 2024-02-06T20:55:27.192Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0x121ce4 2024-02-06T20:55:27.193Z **BROKEN** lightningd: backtrace: (null):0 ((null)) 0xffffffffffffffff Link: ElementsProject#7039 Changelog-Fixes: wallet: avoid to load null local_alias Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
I wonder if we should rather make the alias in the message optional, rather than ensuring we have a dummy value 🤔 |
I have also this patch on my repository, if you want I make a PR for an alternative solution, I chose this because @rustyrussell suggested it inside the issue and due to the gossip code being new I did not want to write a patch that relays on assumption that I am making |
Ok this was the real patch, but @rustyrussell provide a better one #7053 |
Looks like we store in the database the local_alias, and for older channels there are some value NULL
This patch will fix the following stacktrace
Changelog-Fixes: wallet: avoid to load null local_alias
I think a cleaner solution would be to have run a migration query and set the alias to a default one, but idk if this looks like a good idea
For now, running the CI
Fixes #7039