Improve on pyln-client gossmap by m-schmoock · Pull Request #7 · rustyrussell/lightning

m-schmoock · 2021-08-22T15:48:51Z

Is it feasible to work on this PR in your repo so I can add ontop?
I don't have permissions to add to your branch directly.

Make it more usable for plugin developers to get i.e. fees of channels.
Adds half-channels objects containing the information about latest channel_update messages.
Adds satoshi capacity of channels by reading gossip_store_channel_amount messages.
Extends testcase

This fails once it has dependencies (next patch): instead extract version manually. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

It doesn't do much work, but it does parse the gossmap file and extract nodes and channels. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

We have to parse them anyway, so why not make them accessible. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

They're generally useful. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

This is more efficient than converting them all to Pubkeys: about 3.8 seconds vs 5.4 seconds. Usually treating them as raw bytes is what we want anyway. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

The variable `block` (instace of `struct block`) is allocated on the stack without being initialized, i.e. its member `prev` points to nowhere. This causes a segmentation fault on my machine on the binding of "prev_hash" on running `wallet_block_add`, as the following core-dump analysis shows: $ egdb ./wallet/test/run-wallet ./run-wallet.core [...] Core was generated by `run-wallet'. Program terminated with signal SIGSEGV, Segmentation fault. ---Type <return> to continue, or q <return> to quit--- #0 0x000008f67a04b660 in memcpy (dst0=<optimized out>, src0=0x100007f8c, length=32) at /usr/src/lib/libc/string/memcpy.c:97 97 TLOOP1(*dst++ = *src++); (gdb) bt #0 0x000008f67a04b660 in memcpy (dst0=<optimized out>, src0=0x100007f8c, length=32) at /usr/src/lib/libc/string/memcpy.c:97 #1 0x000008f73e838f60 in sqlite3VdbeMemSetStr () from /usr/local/lib/libsqlite3.so.37.12 #2 0x000008f73e83cb11 in bindText () from /usr/local/lib/libsqlite3.so.37.12 #3 0x000008f44bc91345 in db_sqlite3_query (stmt=0x8f6845bf028) at wallet/db_sqlite3.c:77 #4 0x000008f44bc91122 in db_sqlite3_exec (stmt=0x8f6845bf028) at wallet/db_sqlite3.c:110 #5 0x000008f44bcbb3b2 in db_exec_prepared_v2 (stmt=0x8f6845bf028) at ./wallet/db.c:2055 #6 0x000008f44bcc6890 in wallet_block_add (w=0x8f688b5bba8, b=0x7f7ffffca788) at ./wallet/wallet.c:3556 #7 0x000008f44bce2607 in test_wallet_outputs (ld=0x8f6a35a7828, ctx=0x8f6a35c0268) at wallet/test/run-wallet.c:1104 #8 0x000008f44bcddec0 in main (argc=1, argv=0x7f7ffffcaaf8) at wallet/test/run-wallet.c:1930 Fix by explicitely setting the whole structure to zero. [ Rebuilt generated files, too --RR ]

This uses the channel_flags to decode the direction of a channel and not the wrongly used message_flags

This reads the `gossip_store_channel_amount` that always follows the `channel_announcement` messages. Therefore it uses an internal variable _last_scid to know what channel has been added last time.

Do not mix bytes and GossmapNodeId when accessing Gossmap.nodes dicts. Therefore the definion got GossmapNodeId also needed to be pulled to the beginning of the file.

rustyrussell · 2021-08-26T00:30:52Z

I tried rebasing this, it was an unmitigated disaster. Fixup patches which move functions, changing them, etc.

This also makes it near impossible to review :(

I've applied the whole thing as a single commit. I hate doing that, but Ive spent 30 minutes on this so far and it's still unfinished.

rustyrussell · 2021-08-26T00:42:02Z

Actually, managed to salvage it...

m-schmoock · 2021-08-26T06:13:06Z

I tried to keep the fixups relating to your commits as minimal as possible, but fixup 4b7b65e needed to change the order of Classes in the file 😐. Do you know a better way?

m-schmoock · 2021-08-26T06:28:56Z

Ah, it was because I didn't put the last fixup upfront like the two others...
Sorry, I would have done this today, didn't managed to do it yesterday evening.
Next time first the fixups then my stuff :)

@rustyrussell

This will fix a crash that I caused on armv7 and by looking inside the coredump with gdb (by adding an assert on n that must be different from null) I get the following stacktrace ``` (gdb) bt \#0 0x00000000 in ?? () \#1 0x0043a038 in send_backtrace (why=0xbe9e3600 "FATAL SIGNAL 11") at common/daemon.c:36 \#2 0x0043a0ec in crashdump (sig=11) at common/daemon.c:46 \#3 <signal handler called> \#4 0x00406d04 in node_announcement (map=0x938ecc, nann_off=495146) at common/gossmap.c:586 \#5 0x00406fec in map_catchup (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:643 \#6 0x004073a4 in load_gossip_store (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:697 \#7 0x00408244 in gossmap_load (ctx=0x0, filename=0x4e16b8 "gossip_store", num_channel_updates_rejected=0xbe9e3a40) at common/gossmap.c:976 \#8 0x0041a548 in init (p=0x93831c, buf=0x9399d4 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"cln:init#25\",\"method\":\"init\",\"params\":{\"options\":{},\"configuration\":{\"lightning-dir\":\"/home/vincent/.lightning/testnet\",\"rpc-file\":\"lightning-rpc\",\"startup\":true,\"network\":\"te"..., config=0x939cdc) at plugins/topology.c:622 \#9 0x0041e5d0 in handle_init (cmd=0x938934, buf=0x9399d4 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"cln:init#25\",\"method\":\"init\",\"params\":{\"options\":{},\"configuration\":{\"lightning-dir\":\"/home/vincent/.lightning/testnet\",\"rpc-file\":\"lightning-rpc\",\"startup\":true,\"network\":\"te"..., params=0x939c8c) at plugins/libplugin.c:1208 \#10 0x0041fc04 in ld_command_handle (plugin=0x93831c, toks=0x939bec) at plugins/libplugin.c:1572 \#11 0x00420050 in ld_read_json_one (plugin=0x93831c) at plugins/libplugin.c:1667 \#12 0x004201bc in ld_read_json (conn=0x9391c4, plugin=0x93831c) at plugins/libplugin.c:1687 \#13 0x004cb82c in next_plan (conn=0x9391c4, plan=0x9391d8) at ccan/ccan/io/io.c:59 \#14 0x004cc67c in do_plan (conn=0x9391c4, plan=0x9391d8, idle_on_epipe=false) at ccan/ccan/io/io.c:407 \#15 0x004cc6dc in io_ready (conn=0x9391c4, pollflags=1) at ccan/ccan/io/io.c:417 \#16 0x004cf8cc in io_loop (timers=0x9383c4, expired=0xbe9e3ce4) at ccan/ccan/io/poll.c:453 \ElementsProject#17 0x00420af4 in plugin_main (argv=0xbe9e3eb4, init=0x41a46c <init>, restartability=PLUGIN_STATIC, init_rpc=true, features=0x0, commands=0x6167e8 <commands>, num_commands=4, notif_subs=0x0, num_notif_subs=0, hook_subs=0x0, num_hook_subs=0, notif_topics=0x0, num_notif_topics=0) at plugins/libplugin.c:1891 \ElementsProject#18 0x0041a6f8 in main (argc=1, argv=0xbe9e3eb4) at plugins/topology.c:679 ``` I do not know if this is a solution because I do not know when I can parse a node announcement for a node that it is not longer in the gossip map. So, I hope this is just usefult for @rustyrussell Changelog-Fixed: fixes `FATAL SIGNAL 11` on gossmap node announcement parsing. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

The issue is that common_setup() wasn't called by the fuzz target, leaving secp256k1_ctx as NULL. UBSan error: $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" \ ./fuzz-channel_id crash-1575b41ef09e62e4c09c165e6dc037a110b113f2 INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 1153355603 INFO: Loaded 1 modules (25915 inline 8-bit counters): 25915 [0x563bae7ac3a8, 0x563bae7b28e3), INFO: Loaded 1 PC tables (25915 PCs): 25915 [0x563bae7b28e8,0x563bae817c98), ./fuzz-channel_id: Running 1 inputs 1 time(s) each. Running: crash-1575b41ef09e62e4c09c165e6dc037a110b113f2 bitcoin/pubkey.c:22:33: runtime error: null pointer passed as argument 1, which is declared to never be null external/libwally-core/src/secp256k1/include/secp256k1.h:373:3: note: nonnull attribute specified here #0 0x563bae41e3db in pubkey_from_der bitcoin/pubkey.c:19:7 #1 0x563bae4205e0 in fromwire_pubkey bitcoin/pubkey.c:111:7 #2 0x563bae46437c in run tests/fuzz/fuzz-channel_id.c:42:3 #3 0x563bae2f6016 in LLVMFuzzerTestOneInput tests/fuzz/libfuzz.c:23:2 #4 0x563bae20a450 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) #5 0x563bae1f4c3f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) #6 0x563bae1fa6e6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) #7 0x563bae223052 in main (tests/fuzz/fuzz-channel_id+0x181052) (BuildId: f7f56e14ffc06df54ab732d79ea922e773de1f25) #8 0x7fa7fa113082 in __libc_start_main #9 0x563bae1efbdd in _start SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior bitcoin/pubkey.c:22:33 in

pubkey_from_hexstr() was failing, which we didn't notice because we weren't checking the return value. The problem was that we were passing it a strlen that was half the actual length. Relevant error: [libsecp256k1] illegal argument: !secp256k1_fe_is_zero(&ge->x) ==417723== ERROR: libFuzzer: deadly signal #7 0x7f5deaacc7fb in abort #8 0x51b0b0 in secp256k1_default_illegal_callback_fn secp256k1.c #9 0x51bd8e in secp256k1_ec_pubkey_serialize #10 0x4e235b in pubkey_to_der bitcoin/pubkey.c:29:7 #11 0x4e2941 in pubkey_cmp bitcoin/pubkey.c:89:2 #12 0x4e333d in bitcoin_redeem_2of2 bitcoin/script.c:144:6 #13 0x4f1396 in run tests/fuzz/fuzz-close_tx.c:78:19

Rather than crashing the entire node on invalid pubkey, we should return an error. Detected by libFuzzer: ==250024== ERROR: libFuzzer: deadly signal [ Changed so that `n` really does check that it's valid --RR ] #7 abort #8 bolt11_decode common/bolt11.c:1002:4

Rather than crashing the entire node on invalid pubkey, check the validity of the pubkey in decode_n, and return an error if invalid. Detected by libFuzzer: ==265599== ERROR: libFuzzer: deadly signal #7 abort #8 bolt11_decode common/bolt11.c:999:4

And add a check for new uses creeping in, since it got cut & paste everywhere. This means "this is a valid string, but truncate it to this many characters" vs "%.*s" which means "only read this many characters of string": ``` ['lightningd-3 2025-10-23T02:31:40.890Z **BROKEN** plugin-funder: Plugin marked as important, shutting down lightningd!'] --------------------------- Captured stderr teardown --------------------------- #0 0x557da58ad1dc in printf_common(void*, char const*, __va_list_tag*) asan_interceptors.cpp.o #1 0x557da5aff814 in json_out_addv /home/runner/work/lightning/lightning/ccan/ccan/json_out/json_out.c:239:11 #2 0x557da59740ce in plugin_logv /home/runner/work/lightning/lightning/plugins/libplugin.c:1777:2 #3 0x557da5969b6f in plugin_log /home/runner/work/lightning/lightning/plugins/libplugin.c:1934:2 #4 0x557da595c4f6 in datastore_del_success /home/runner/work/lightning/lightning/plugins/funder.c:161:2 #5 0x557da598b837 in handle_rpc_reply /home/runner/work/lightning/lightning/plugins/libplugin.c:1072:10 #6 0x557da598a4b0 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1361:3 #7 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9 #8 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8 #9 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10 #10 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5 #11 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3 #12 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2 #13 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #14 0x7f6572229e3f in __libc_start_main csu/../csu/libc-start.c:392:3 #15 0x557da588b584 in _start (/home/runner/work/lightning/lightning/plugins/funder+0x10d584) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d) 0x624000009d28 is located 0 bytes to the right of 7208-byte region [0x624000008100,0x624000009d28) allocated by thread T0 here: #0 0x557da590e7f6 in __interceptor_realloc (/home/runner/work/lightning/lightning/plugins/funder+0x1907f6) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d) #1 0x557da5b2149b in tal_resize_ /home/runner/work/lightning/lightning/ccan/ccan/tal/tal.c:755:13 #2 0x557da59f2032 in membuf_tal_resize /home/runner/work/lightning/lightning/common/utils.c:203:2 #3 0x557da5b03934 in membuf_prepare_space_ /home/runner/work/lightning/lightning/ccan/ccan/membuf/membuf.c:45:12 #4 0x557da59d4289 in jsonrpc_io_read_ /home/runner/work/lightning/lightning/common/jsonrpc_io.c:127:2 #5 0x557da598a635 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1366:9 #6 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9 #7 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8 #8 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10 #9 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5 #10 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3 #11 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2 #12 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: heap-buffer-overflow asan_interceptors.cpp.o in printf_common(void*, char const*, __va_list_tag*) Shadow bytes around the buggy address: 0x0c487fff9350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c487fff93a0: 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa 0x0c487fff93b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==26122==ABORTING ``` Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Changelog-None: `towire_wireaddr()` and `fromwire_wireaddr()` in `common/wireaddr.h` are responsible for marshalling/unmarshalling BOLT #7 address descriptors. Since these aren't tested by the existing wire fuzz tests, add a roundtrip test for them. This has the added benefit of testing `parse_wireaddr()` as well.

Update BOLT quotes in test and library Python files to match current BOLT text: punctuation (periods to semicolons), capitalization (Lightning on Bitcoin mainnet), fix field ordering in invoice breakdowns, add ... to skip features fields and SHA256 hex details, correct backtick-quoting of field names, and remove incorrect BOLT #7 label from a plain comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

…_final_cltv_expiry_delta. The BOLT #7 was updated to use the new default 18 as min_final_cltv_expiry_delta, so update our quote and test, and fix other textual spec changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Some of these are malformed (thus were unchecked!), some are from the stricter interpretation of `...` which won't cross section boundaries. Several BOLT quotes had drifted from the current spec text: - connectd/queries.c: BOLT #7 uses 'full_information' not 'complete'; remove a second quote that referenced query_channel_range but was actually wrong (that requirement is for query_short_channel_ids). - lightningd/dual_open_control.c: channel reserve is 1% rounded down (not just 1%); witness weight check now says SHOULD broadcast rather than MUST fail; RBF quotes simplified to match actual BOLT wording. - openingd/openingd.c: 'The sending node' became 'The sender'; invalid signature response now says MUST send warning/error not just MUST fail the channel. - openingd/dualopend.c: same invalid-signature quote as openingd.c. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>