Rebase forked master #4
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rusty Russell <[email protected]>
We shadow local variables in several places: generally, these changes make the code clearer. Signed-off-by: Rusty Russell <[email protected]>
We annotate them with UNNEEDED, which is legal but weird, but it makes gcc (at least 11.2.0) complain about shadowing: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106424 I simply removed the names. Signed-off-by: Rusty Russell <[email protected]>
This will warn us if there are local variables which shadow the same-named variable. Signed-off-by: Rusty Russell <[email protected]>
The test fails because the closed channel is deleted by the time we check the state change history. It is unnecessary to mine any blocks after the close, since the state changes up to CLOSINGD_COMPLETE occur without onchain changes.
Merged a commit that broke everything with shadows, which turned out to be the bookkeeper code.
This happens with deprecated-apis and listconfigs, breaking some python plugins! Fixes: ElementsProject#5546 Fixes: ElementsProject#5563 Signed-off-by: Rusty Russell <[email protected]>
Should help diagnose ElementsProject#5572 which hit the invalid csum on a >64MB entry, if it happens again. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Here's the before-vs-after comparison (ignoring whitespace changes):
```diff
--- /tmp/before 2022-07-20 21:52:44.336641810 +0930
+++ /tmp/after 2022-07-20 21:55:54.355487769 +0930
@@ -1,7 +1,7 @@
-LIGHTNING-CLI(1) lightning-cli LIGHTNING-CLI(1)
+LIGHTNING-CLI(1) LIGHTNING-CLI(1)
NAME
- lightning-cli - Control lightning daemon
+ lightning-cli -- Control lightning daemon
SYNOPSIS
lightning-cli [OPTIONS] command
@@ -14,10 +14,7 @@
--conf=PATH Sets configuration file (default: lightning-dir/config ).
- --network=network
- --mainnet
- --testnet
- --signet Sets network explicitly.
+ --network=network --mainnet --testnet --signet Sets network explicitly.
--rpc-file=FILE Named pipe to use to talk to lightning daemon: default is lightning-rpc in the lightning directory.
@@ -43,8 +40,8 @@
--version/-V Print version number to standard output and exit.
- allow-deprecated-apis=BOOL Enable deprecated options. It defaults to true, but you should set it to false when testing
- to ensure that an upgrade won't break your configuration.
+ allow-deprecated-apis=BOOL Enable deprecated options. It defaults to true, but you should set it to false when testing to
+ ensure that an upgrade won't break your configuration.
COMMANDS
lightning-cli simply uses the JSON RPC interface to talk to lightningd, and prints the results. Thus the commands avail‐
@@ -67,7 +64,7 @@
lightning-cli help
- 1. Fund a 10k sat channel using uncomfirmed outputs
+ 2. Fund a 10k sat channel using uncomfirmed outputs
lightning-cli --keywords fundchannel id=028f...ae7d amount=10000sat minconf=0
@@ -84,4 +81,4 @@
Note: the modules in the ccan/ directory have their own licenses, but the rest of the code is covered by the BSD-style
MIT license.
- LIGHTNING-CLI(1)
+Core Lightning v0.11.0.1-350-gac2e137 LIGHTNING-CLI(1)
```
Signed-off-by: Rusty Russell <[email protected]>
Fixes: ElementsProject#5437
I guessed it's called "lowdown" for everyone? Signed-off-by: Rusty Russell <[email protected]>
There are no definition lists in Markdown, and lists get mangled if
they follow immediately: they need a line between them.
So use bullets for options, and use an indent so the text gets in the
line below.
Here's a before-and-after example:
```diff
--- /tmp/after 2022-07-20 21:55:54.355487769 +0930
+++ /tmp/after2 2022-07-20 21:58:17.305642576 +0930
@@ -10,38 +10,71 @@
lightning-cli sends commands to the lightning daemon.
OPTIONS
- --lightning-dir=DIR Set the directory for the lightning daemon we're talking to; defaults to $HOME/.lightning.
+ • --lightning-dir=DIR
- --conf=PATH Sets configuration file (default: lightning-dir/config ).
+ Set the directory for the lightning daemon we're talking to; defaults to $HOME/.lightning.
- --network=network --mainnet --testnet --signet Sets network explicitly.
+ • --conf=PATH
- --rpc-file=FILE Named pipe to use to talk to lightning daemon: default is lightning-rpc in the lightning directory.
+ Sets configuration file (default: lightning-dir/config ).
- --keywords/-k Use format key=value for parameters in any order
+ • --network=network
- --order/-o Follow strictly the order of parameters for the command
+ • --mainnet
- --json/-J Return result in JSON format (default unless help command, or result contains a format-hint field).
+ • --testnet
- --raw/-R Return raw JSON directly as lightningd replies; this can be faster for large requests.
+ • --signet
- --human-readable/-H Return result in human-readable output.
+ Sets network explicitly.
- --flat/-F Return JSON result in flattened one-per-line output, e.g. { "help": [ { "command": "check" } ] } would become
+ • --rpc-file=FILE
+
+ Named pipe to use to talk to lightning daemon: default is lightning-rpc in the lightning directory.
+
+ • --keywords/-k
+
+ Use format key=value for parameters in any order
+
+ • --order/-o
+
+ Follow strictly the order of parameters for the command
+
+ • --json/-J
+
+ Return result in JSON format (default unless help command, or result contains a format-hint field).
+
+ • --raw/-R
+
+ Return raw JSON directly as lightningd replies; this can be faster for large requests.
+
+ • --human-readable/-H
+
+ Return result in human-readable output.
+
+ • --flat/-F
+
+ Return JSON result in flattened one-per-line output, e.g. { "help": [ { "command": "check" } ] } would become
help[0].command=check. This is useful for simple scripts which want to find a specific output field without parsing
JSON.
- --notifications/-N=LEVEL If LEVEL is 'none', then never print out notifications. Otherwise, print out notifications of
- LEVEL or above (one of io, debug, info (the default), unusual or broken: they are prefixed with # .
+ • --notifications/-N=LEVEL
+
+ If LEVEL is 'none', then never print out notifications. Otherwise, print out notifications of LEVEL or above (one of
+ io, debug, info (the default), unusual or broken: they are prefixed with # .
+
+ • --help/-h
+
+ Pretty-print summary of options to standard output and exit. The format can be changed using -F, -R, -J, -H etc.
+
+ • --version/-V
- --help/-h Pretty-print summary of options to standard output and exit. The format can be changed using -F, -R, -J, -H
- etc.
+ Print version number to standard output and exit.
- --version/-V Print version number to standard output and exit.
+ • allow-deprecated-apis=BOOL
- allow-deprecated-apis=BOOL Enable deprecated options. It defaults to true, but you should set it to false when testing to
- ensure that an upgrade won't break your configuration.
+ Enable deprecated options. It defaults to true, but you should set it to false when testing to ensure that an upgrade
+ won't break your configuration.
COMMANDS
lightning-cli simply uses the JSON RPC interface to talk to lightningd, and prints the results. Thus the commands avail‐
@@ -60,13 +93,13 @@
this is not encouraged.
EXAMPLES
- 1. List commands
+ 1. List commands:
- lightning-cli help
+ • lightning-cli help
- 2. Fund a 10k sat channel using uncomfirmed outputs
+ 2. Fund a 10k sat channel using uncomfirmed outputs:
- lightning-cli --keywords fundchannel id=028f...ae7d amount=10000sat minconf=0
+ • lightning-cli --keywords fundchannel id=028f...ae7d amount=10000sat minconf=0
BUGS
This manpage documents how it should work, not how it does work. The pretty printing of results isn't pretty.
```
Signed-off-by: Rusty Russell <[email protected]>
You can't start a list without a paragraph separator.
```diff
--- /tmp/before 2022-07-20 22:02:23.485372596 +0930
+++ /tmp/after 2022-07-20 22:02:33.745528456 +0930
@@ -21,12 +21,16 @@
On startup of the daemon, no autoclean is set up.
RETURN VALUE
- On success, an object is returned, containing: - enabled (boolean):
- whether invoice autocleaning is active
+ On success, an object is returned, containing:
- If enabled is true: - expired_by (u64): how long an invoice must be ex‐
- pired (seconds) before we delete it - cycle_seconds (u64): how long an
- invoice must be expired (seconds) before we delete it
+ • enabled (boolean): whether invoice autocleaning is active
+
+ If enabled is true:
+
+ • expired_by (u64): how long an invoice must be expired (seconds) be‐
+ fore we delete it
+ • cycle_seconds (u64): how long an invoice must be expired (seconds)
+ before we delete it
AUTHOR
ZmnSCPxj <[email protected]> is mainly responsible.
```
Signed-off-by: Rusty Russell <[email protected]>
….py in SHASUMS) 1. If the tool changes, you need to regenerate since the output may change. 2. This didn't just filter that out, ignored all but the first dependency, which made bisecting the bookkeeper plugin a nightmare: it didn't regenerate the .po file, causing random crashes. If we want this, try $(filter-out tools/fromschema.py) instead. But I don't think we want that. Signed-off-by: Rusty Russell <[email protected]>
If there's only a single underscore, lowdown ignores it, but if there are multiple (see min_final_cltv_expiry) it decides we're trying to highlight part of the word. Reported-by: @wtogami Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
People who copy paste the commands might not realize this chunk is not copy paste-able.
Requires us to update to latest lnproto which is now using the most up to date python-bitcoinlib, as well as updating our python lock files (which pin the grpcio deps, because of locking problems h/t @cdecker)
Few extremely minor updates to the ubuntu dockerfile and ci builds
Currently discovered IPs are only announced when we don't have any usable addresses detected or configured already. However, the cli command `getinfo` still showed theses unannounced addr as if they were announced. Changelog-Fixed: peer_control: getinfo showing unannounced addresses.
This one got badly messed up over time. I know we usually don't fix these to have easier git-bisect. I can remove this commit if required.
This FIXME caught my eye, as it's wrong: TLVs are canonical, so they cannot differ in bits and be equal. The equality function needs to be written correctly, however, otherwise it will crash! Signed-off-by: Rusty Russell <[email protected]>
In particular, we didn't check the remote_addr in the init msg. Signed-off-by: Rusty Russell <[email protected]>
…r_internal. Signed-off-by: Rusty Russell <[email protected]>
Somehow we missed this deprecation, found by grep. Changelog-Removed: JSON API: Removed double wrapping of `rpc_command` payload in `rpc_command` JSON field (deprecated v0.8.2) Signed-off-by: Rusty Russell <[email protected]>
We still have an "enum forward_style" for the database, where old-style forwards can still exist. Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: Protocol: we no longer forward HTLCs with legacy onions.
In particular, remove special routines to pull length: it's there, take it and check it yourself. Signed-off-by: Rusty Russell <[email protected]>
"sphinx_add_hop" takes a literal hop to include, "sphinx_add_modern_hop" prepends the length. Now we always prepend a length, make it clear that the literal version is a shortcut: * sphinx_add_hop -> sphinx_add_hop_has_length * sphinx_add_modern_hop -> sphinx_add_hop In addition, we check that length is actually correct! This means `createonion` can no longer create legacy or otherwise-invalid onions: fix tests and update man page to remove legacy usage. Signed-off-by: Rusty Russell <[email protected]> Changelog-Changed: JSON-RPC: `createonion` no longer allows non-TLV-style payloads.
If a channel goes offline while the count of outstanding outgoing HTLCs exceeds the limit that we enforce against the peer, then the channel could never be brought online again because `add_htlc` called by `channel_force_htlcs` in `channeld/full_channel.c` would return `CHANNEL_ERR_TOO_MANY_HTLCS`. The protocol specification actually does allow us to exceed the limits that we are enforcing against the peer; we are only prohibited from exceeding the limits that the peer is enforcing against us. `add_htlc` takes an `enforce_aggregate_limits` parameter that appears to have been intended for `channel_force_htlcs` to exempt the local node from obeying the limits that it is enforcing against the peer, but this parameter was only being respected for the total HTLC value-in-flight check but not for the HTLC count check. This commit respects the parameter for the HTLC count check as well and resolves the problem of "Could not restore HTLCs". Fixes: ElementsProject#5636 Changelog-Fixed: channeld: Channel reinitialization no longer fails when the number of outstanding outgoing HTLCs exceeds `max_accepted_htlcs`.
This was missing in the last version
The goal here is to test the node validation, not whether we can trigger the schema validation with bogus values. So we bypass the verifying RPC wrapper.
…store`) Changelog-Added: JSON-RPC: `makesecret` can take a string argument instead of hex. Signed-off-by: Rusty Russell <[email protected]>
This reveals that common/test/run-bolt12_merkle-json.c was broken! Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Generate the payload in the callers. Signed-off-by: Rusty Russell <[email protected]>
This makes it easier to add/delete fields manually to a bolt12 encoding. Signed-off-by: Rusty Russell <[email protected]>
If we do, an upgrade would mean we can no longer get refunds on old invoices. Signed-off-by: Rusty Russell <[email protected]>
There's no secp256k1 routine to do this, but we're going to need it. Signed-off-by: Rusty Russell <[email protected]>
It's been obsoleted and needs replacing; less confusing if we remove it first. Also, these fields are now present even without an expermintal build (we'll control at runtime). Signed-off-by: Rusty Russell <[email protected]>
We don't actually process onion messages here any more (they moved to connectd), but the flag and object files were still linked. Signed-off-by: Rusty Russell <[email protected]>
Mac is updating to using zsh in general. The “for i in” reads strings with spaces as a single entry instead of multiple entries as sh did. Using “while read” … “<<< $var” makes it treat each space as a new entry. Changelog-None
…ing... Changelog-None: Increasing test scope
Signed-off-by: Rusty Russell <[email protected]> Changelog-Fixed: Plugins: `keysend` now removes unknown even (technically illegal!) fields, to try to accept more payments.
|
@NicolasDorier is this worth integrating or shall we close it? |
NicolasDorier
pushed a commit
that referenced
this pull request
Aug 24, 2023
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 \ElementsProject#5 0x00406fec in map_catchup (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:643 \ElementsProject#6 0x004073a4 in load_gossip_store (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:697 \ElementsProject#7 0x00408244 in gossmap_load (ctx=0x0, filename=0x4e16b8 "gossip_store", num_channel_updates_rejected=0xbe9e3a40) at common/gossmap.c:976 \ElementsProject#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 \ElementsProject#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 \ElementsProject#10 0x0041fc04 in ld_command_handle (plugin=0x93831c, toks=0x939bec) at plugins/libplugin.c:1572 \ElementsProject#11 0x00420050 in ld_read_json_one (plugin=0x93831c) at plugins/libplugin.c:1667 \ElementsProject#12 0x004201bc in ld_read_json (conn=0x9391c4, plugin=0x93831c) at plugins/libplugin.c:1687 \ElementsProject#13 0x004cb82c in next_plan (conn=0x9391c4, plan=0x9391d8) at ccan/ccan/io/io.c:59 \ElementsProject#14 0x004cc67c in do_plan (conn=0x9391c4, plan=0x9391d8, idle_on_epipe=false) at ccan/ccan/io/io.c:407 \ElementsProject#15 0x004cc6dc in io_ready (conn=0x9391c4, pollflags=1) at ccan/ccan/io/io.c:417 \ElementsProject#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 <[email protected]>
NicolasDorier
pushed a commit
that referenced
this pull request
Aug 24, 2023
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)
ElementsProject#5 0x563bae1f4c3f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long)
ElementsProject#6 0x563bae1fa6e6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))
ElementsProject#7 0x563bae223052 in main (tests/fuzz/fuzz-channel_id+0x181052) (BuildId: f7f56e14ffc06df54ab732d79ea922e773de1f25)
ElementsProject#8 0x7fa7fa113082 in __libc_start_main
ElementsProject#9 0x563bae1efbdd in _start
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior bitcoin/pubkey.c:22:33 in
NicolasDorier
pushed a commit
that referenced
this pull request
Aug 24, 2023
It is possible for db_column_bytes() to return 0 and for db_column_blob() to return NULL even when db_column_is_null() returns false. We need to short circuit in this case. Detected by UBSan: db/bindings.c:479:12: runtime error: null pointer passed as argument 2, which is declared to never be null /usr/include/string.h:44:28: note: nonnull attribute specified here #0 0x95f117 in db_col_arr_ db/bindings.c:479:2 #1 0x95ef85 in db_col_channel_type db/bindings.c:459:32 #2 0x852c03 in wallet_stmt2channel wallet/wallet.c:1483:9 #3 0x81f396 in wallet_channels_load_active wallet/wallet.c:1749:23 #4 0x81f03d in wallet_init_channels wallet/wallet.c:1765:9 ElementsProject#5 0x72f1f9 in load_channels_from_wallet lightningd/peer_control.c:2257:7 ElementsProject#6 0x672856 in main lightningd/lightningd.c:1121:25
NicolasDorier
pushed a commit
that referenced
this pull request
Aug 24, 2023
Fixes nullability errors detected by UBSan:
wire/fromwire.c:173:46: runtime error: null pointer passed as argument 1, which is declared to never be null
external/libwally-core/src/secp256k1/include/secp256k1.h:432:3: note: nonnull attribute specified here
#0 0x65214a in fromwire_secp256k1_ecdsa_signature wire/fromwire.c:173:6
#1 0x659500 in printwire_secp256k1_ecdsa_signature devtools/print_wire.c:331:1
#2 0x646ba2 in printwire_channel_update wire/peer_printgen.c:1900:7
#3 0x637182 in printpeer_wire_message wire/peer_printgen.c:128:11
#4 0x65a097 in main devtools/decodemsg.c:85:10
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds our two commits on top of the current Core Lightning master, so that we could publish a new version of our images.
Prerequisite for btcpayserver/btcpayserver-docker#664.
Applies cleanly to
master.