Commit Graph

13832 Commits

Author SHA1 Message Date
Dusty Daemon
e1ac2410b0 connectd: Ignore sodium fd for Mac tests
On Mac most tests report BROKEN because sodium creating an untracked fd pointing to /dev/random. dev_report_fd’s finds it at tear down and reports a BROKEN message.

We allow a single “char special” fd without reporting it as broken improving QOL for Mac developers.

While we’re here we added the fd mode to the log to help with future rogue fd issues.

ChangeLog-None
2023-10-19 14:31:25 +10:30
Matt Morehouse
87f4907bb4 bolt11: don't abort on invalid pubkey
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
2023-10-18 09:53:22 +10:30
Matt Morehouse
c1f20687a6 bolt11: validate recovery ID
Invalid recovery IDs cause
secp256k1_ecdsa_recoverable_signature_parse_compact to abort, which
crashes the entire node. We should return an error instead.

Detected by libFuzzer:
[libsecp256k1] illegal argument: recid >= 0 && recid <= 3
2023-10-18 09:53:22 +10:30
Matt Morehouse
4b29502098 fuzz: check invoice signatures
There may be bugs in signature validation, so we should fuzz that too.
2023-10-18 09:53:22 +10:30
Matt Morehouse
ee8cf69f28 bolt11: return error on unexpected lightning prefix
Remove the assertion so that an error is returned for invalid bech32.
An error is preferable to crashing the entire node if there's an extra
"lightning:" prefix:

  $ lightning-cli pay "lightning:lightning:"

Node log:
  pay: common/bolt11.c:718: bolt11_decode_nosig: Assertion `!has_lightning_prefix(str)' failed.
  pay: FATAL SIGNAL 6
  ...
  INFO    plugin-pay: Killing plugin: exited during normal operation
  **BROKEN** plugin-pay: Plugin marked as important, shutting down lightningd
2023-10-18 09:53:22 +10:30
Matt Morehouse
74b8fff3d4 github: re-enable ASan and UBSan for check-fuzz
ASan and UBSan were disabled by
364de00947, which has already prevented
multiple bugs from being discovered prior to merging.
2023-10-18 09:53:22 +10:30
Matt Morehouse
ee501b035b bolt11: avoid reading uninitialized memory
If both databits and *data_len are 0, pull_uint return uninitialized
stack memory in *val.

Detected by valgrind and UBSan.

valgrind:
==173904== Use of uninitialised value of size 8
==173904==    __sanitizer_cov_trace_cmp8
==173904==    decode_c (bolt11.c:292)
==173904==    bolt11_decode_nosig (bolt11.c:877)

UBSan:
common/bolt11.c:79:29: runtime error: shift exponent 64 is too large for 64-bit type 'uint64_t' (aka 'unsigned long')

Corpus input e6f7b9744a7d79b2aa4f7c477707bdd3483f40fa triggers the UBSan
report, but we didn't previously realize this because UBSan has been
disabled in the CI run. We rename the input to indicate its usefulness
as a permanent regression test.
2023-10-18 09:53:22 +10:30
Matt Morehouse
eeec529031 bolt11: check return value of pull_all
Otherwise, if pull_all fails, we attempt to create a script from NULL,
causing a UBSan report:

bitcoin/script.c:29:28: runtime error: null pointer passed as argument 2, which is declared to never be null

Corpus input bf703c2c20c0818af70a8c4caad6e6fd8cfd1ac6 triggers the UBSan
report, but we didn't previously realize this because UBSan has been
disabled in the CI run. We rename the input to indicate its usefulness
as a permanent regression test.
2023-10-18 09:53:22 +10:30
Rusty Russell
19c581c33f lightningd: simplify lease update blockheight code.
Move the "no lease, return" to the top, to avoid testing twice.  Also,
we won't spam now for most channels.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-17 14:59:54 +10:30
Rusty Russell
91699d8483 lightningd: fix crash caused by sending channeld message to closingd
```
Sending closingd an invalid message 03f40000006d
...
0x564b55bb7378 fatal
	lightningd/log.c:1035
0x564b55bf7841 subd_send_msg
	lightningd/subd.c:841
0x564b55b87d08 try_update_blockheight
	lightningd/channel_control.c:143
0x564b55b8c497 channel_notify_new_block
	lightningd/channel_control.c:1687
0x564b55bb26bb notify_new_block
	lightningd/lightningd.c:743
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-17 14:59:54 +10:30
Rusty Russell
03e51ae879 Revert "runes: fix checkrune when method parameter is the empty string."
This reverts commit ae94be4ce2.
2023-10-17 14:58:40 +10:30
Rusty Russell
c0782ee841 clnrest: correctly omit method to checkrune call if none is supplied.
Specifying the empty string is very different from not specifying it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-17 14:58:40 +10:30
Rusty Russell
cdd58e2fe3 pytest: correctly test missing method for runes.
This makes it clear exactly what the expectation is/

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-17 14:58:40 +10:30
Michael Schmoock
8ac9c73ea8 pytests: make pay helper able to route (optionaly)
This makes the pay helper function being able to route
a payment using the optional `route` paramter that defaults to `False`.

I added this, as some plugins maintained their own version of `pay` that
needed routed payment helper.

Changelog-None
2023-10-17 14:53:53 +10:30
Dusty Daemon
e938999027 channeld: comment typo fix
ChangeLog-None
2023-10-16 00:55:47 +02:00
Vincenzo Palazzo
e936eea9cb build: fix compilation issue on OpenBSD
During our development, we modified the way
we report backtraces.

On a minimal configuration in OpenBSD, it seems that we
no longer compile from commit a9f26b7d07 because
our conditional code is buggy.

With the following compiler

vultr# cc -v
OpenBSD clang version 13.0.0
Target: amd64-unknown-openbsd7.3
Thread model: posix
InstalledDir: /usr/bin

We have the following error

cc common/channel_id.c
cc common/daemon.c
common/daemon.c:218:2: error: implicit declaration of function 'add_steal_notifiers' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        add_steal_notifiers(NULL);
        ^
1 error generated.
gmake: *** [Makefile:298: common/daemon.o] Error 1

Reported-by: @grubles
Fixes a9f26b7d07
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
2023-10-15 14:01:45 +02:00
Tony Aldon
8f6a02c182 doc: fix display of lists and code blocks in manpages.
The conversion from markdown to manpages does not display lists and
code blocks correctly if they are not preceded by an empty line.

Changelog-None
2023-10-14 17:55:51 +02:00
Vincenzo Palazzo
9b69c56cfb lightningd: print the version of cln early
As a node matures and is no longer new, it can take some time
to determine which version of `cln` it's running.

To address this, we now display the version earlier. This ensures
that even in the event of a crash, we're aware of the running version.

(cln-meta-project-py3.11) ➜  lightning git:(macros/log-version) ✗ ./lightningd/lightningd
2023-10-12T19:21:00.899Z INFO    lightningd: v23.08.1-209-gae94be4-modded
2023-10-12T19:21:00.994Z INFO    lightningd: Creating configuration directory /home/vincent/.lightning/bitcoin
2023-10-12T19:21:01.235Z INFO    lightningd: Creating database
2023-10-12T19:21:01.279Z UNUSUAL hsmd: HSM: created new hsm_secret file

Could not connect to bitcoind using bitcoin-cli. Is bitcoind running?

Make sure you have bitcoind running and that bitcoin-cli is able to connect to bitcoind.

You can verify that your Bitcoin Core installation is ready for use by running:

    $ bitcoin-cli echo 'hello world'
2023-10-12T19:21:01.349Z **BROKEN** plugin-bcli: \nCould not connect to bitcoind using bitcoin-cli. Is bitcoind running?\n\nMake sure you have bitcoind running and that bitcoin-cli is able to connect to bitcoind.\n\nYou can verify that your Bitcoin Core installation is ready for use by running:\n\n    $ bitcoin-cli echo 'hello world'\n
2023-10-12T19:21:01.349Z INFO    plugin-bcli: Killing plugin: exited before we sent init
The Bitcoin backend died.

Link: https://github.com/ElementsProject/lightning/issues/6374
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
2023-10-14 17:52:15 +02:00
Tony Aldon
7bea339f2f cln-grpc: do not generate cerficates when no autostart.
Changelog-None
2023-10-13 10:25:36 +02:00
Tony Aldon
340a7316d6 pytest: test that cln-grpc certificates are not generated when cln-grpc is not started.
We update `test_grpc_no_auto_start` test to check that we do not
generate certificates when the cln-grpc plugin is not started.

This fails currently, so next commit fix it up.
2023-10-13 10:25:36 +02:00
Tony Aldon
ae94be4ce2 runes: fix checkrune when method parameter is the empty string.
This fix #6725.

Changelog-Fixed: fix `checkrune` when `method` parameter is the empty string.
2023-10-12 20:42:43 +02:00
Tony Aldon
ebee48ca8c pytest: test checkrune when method parameter is the empty string.
Add test `test_rune_method_not_equal_and_method_empty` that reproduces
issue #6725.

This fails currently, so next commit fix it up.

Error:

```
>       with pytest.raises(RpcError, match='Not permitted: method not present'):
E       Failed: DID NOT RAISE <class 'pyln.client.lightning.RpcError'>

tests/test_runes.py:605: Failed
```
2023-10-12 20:42:43 +02:00
dergoegge
02717c73dc fuzz: bolt11 inputs 2023-10-12 19:28:28 +02:00
dergoegge
0fc9fe1a81 fuzz: test bolt11 decoding 2023-10-12 19:28:28 +02:00
Tony Aldon
1a46b37609 contrib: add developer option to config file in startup_regtest.sh.
Without `developer` option, after compiling CLN and sourcing
`contrib/startup_regtest.sh`, if we try to start 2 nodes on regtest
running the command `start_ln` we get the following error:

```
lightningd: Config file /tmp/l1-regtest/config line 6: dev-fast-gossip: requires --developer
...
lightningd: Config file /tmp/l2-regtest/config line 6: dev-fast-gossip: requires --developer
```

Changelog-None
2023-10-10 23:04:31 +10:30
Rusty Russell
176a58f9e0 pytest: wean many tests off the assumption that listchannels shows private channels.
We will be changing this, or at least deprecating it, so get our
tests ready.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 08:02:33 +10:30
Rusty Russell
7bb6609f9a pytest: remove old test_tlv_or_legacy
We no longer support legacy at all, so this test doesn't really
test anything.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 08:02:33 +10:30
Rusty Russell
7d3f13ec0d renepay: work around change in fundchannel in tests.
The tests will wait until it's locally enabled, but it might not have
the update in the gossip store.  So have renepay enhance its local
view even if it already knows about the channel (this is correct
anyway, it just isn't very important usually).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 08:02:33 +10:30
Rusty Russell
bb60c77859 plugins/topology: convert listpeerchannels into local overlay.
This prepares us for when the gossmap doesn't contain private channel info.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 08:02:33 +10:30
Rusty Russell
8c0932e46a plugins/topology: split getroute logic.
Mechanical change to bundle into struct getroute_info for next
patch which uses callback.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 08:02:33 +10:30
Rusty Russell
ddb16991ee plugins/topology: remove unused fuzz factor.
This breaks Dijkstra, which is presumably why it was actually
disabled.  Remove the code altoghether, instead.

Changelog-Fixed: JSON-RPC: `getroute` now documents that it ignores `fuzzpercent`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 08:02:33 +10:30
Rusty Russell
11df13e779 channeld: fix memleak where tx gets leaked.
We steal it onto "peer" where we should steal it onto the inflight:

```
label=struct bitcoin_tx
backtrace:
  ccan/ccan/tal/tal.c:477 (tal_alloc_)
  bitcoin/tx.c:612 (clone_bitcoin_tx)
  channeld/channeld.c:2163 (handle_peer_commit_sig)
  channeld/channeld.c:2191 (handle_peer_commit_sig)
  channeld/channeld.c:2831 (interactive_send_commitments)
  channeld/channeld.c:3814 (splice_initiator_user_finalized)
  channeld/channeld.c:3882 (splice_initiator_user_update)
  channeld/channeld.c:5651 (req_in)
  channeld/channeld.c:6044 (main)
  ../sysdeps/nptl/libc_start_call_main.h:58 (__libc_start_call_main)
  ../csu/libc-start.c:360 (__libc_start_main_impl)
parents:
  struct peer
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 07:45:57 +10:30
Rusty Russell
e21bcbfbb9 channeld: fix memleak when inflights is NULL.
In this case, we were allocating off NULL, which meant a leak:

```
 MEMLEAK: 0x565086722e98
   label=channeld/channeld.c:3433:struct inflight
   backtrace:
     ccan/ccan/tal/tal.c:477 (tal_alloc_)
     channeld/channeld.c:3433 (inflights_new)
     channeld/channeld.c:3573 (splice_accepter)
     channeld/channeld.c:4145 (peer_in)
     channeld/channeld.c:6051 (main)
   parents:
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-04 07:45:57 +10:30
Dusty Daemon
181d6cc908 splice: minor code cleanups
A few little stylistic things were bugging me when reading through the splice code so I cleaned them up.

ChangeLog-None
2023-10-04 07:45:57 +10:30
Dusty Daemon
bc9333ac1e addpsbtoutput: New onchain command for PSBT output
Also added splice_out tests that use the new PSBT command.

ChangeLog-Added: New `addpsbtoutput` command for creating a PSBT that can receive funds to the on-chain wallet.
2023-10-03 10:12:39 +02:00
Rusty Russell
c5d36c4ba9 common/memleak: show tal_steal operations on memleak.
We show where the pointer was allocated, but it can be confusing if it
was later stolen onto another context.  Save and report those too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-03 10:05:55 +02:00
Rusty Russell
e11b35cb3a common/memleak: implement callback arg for dump_memleak.
This makes it easier to use outside simple subds, and now lightningd can
simply dump to log rather than returning JSON.

JSON formatting was a lot of work, and we only did it for lightningd, not for
subdaemons.  Easier to use the logs in all cases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-03 10:05:55 +02:00
Rusty Russell
0b680c5f40 lightningd: fix assertion when funding depth changes fast.
We didn't apply the inflight to the channel struct before asserting, so
we can break test_rbf_non_last_mined:

```
lightningd: lightningd/dual_open_control.c:981: dualopend_tell_depth: Assertion `bitcoin_txid_eq(&channel->funding.txid, txid)' failed.
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-03 11:40:43 +10:30
ShahanaFarooqui
9561094433 db: removed deprecated runes_uniqueid from vars table
Fixes: #6696

Changelog-Fixed: rune: use runes table `id` instead `runes_uniqueid` from `vars` because it returns incorrect unique id if rune/s migrated from datastore.
2023-10-03 08:56:53 +10:30
ShahanaFarooqui
c15fc54586 runes: Runes will use unique id returned from runes table not runes_uniqueid from vars 2023-10-03 08:56:53 +10:30
ShahanaFarooqui
90824a5d32 runes: Get next unique id from runes table 2023-10-03 08:56:53 +10:30
ShahanaFarooqui
9bcabbc912 runes: Reimplement rate in terms of per
Changelog-Changed: JSON-RPC: `checkrune` `rate` restriction is slightly stricter (exact division of time like `per`)
2023-10-03 06:57:54 +10:30
Rusty Russell
d61dec8925 pytest: trim mostly-reundant commando-rune test.
We test this in test_runes, so make this test more cursory (otherwise
next patch breaks it).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-03 06:57:54 +10:30
ShahanaFarooqui
b2fe1bc1e5 tools: Fixed release verify typo
Removed `bin-` while copying Fedora sums from release captain's file. Sums are saved as `clightning-$VERSION-Fedora-28-amd64.tar.gz` not as `clightning-$VERSION-bin-Fedora-28-amd64.tar.gz`

Changelog-None.
2023-10-02 15:18:00 +10:30
Rusty Russell
72f914a295 lightningd: simplify funding_depth_cb now it only handles main funding tx.
We make dualopend_tell_depth static, which means we move it
higher in the file.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-02 11:41:19 +10:30
Rusty Russell
79d04090fb lightningd: fix dual-funding case where we coop close and an RBF confirms.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-02 11:41:19 +10:30
Rusty Russell
2f394653a8 lightningd: ensure we *always* watch channel spend.
Now we're not always using the same functions to watch during
dual-funding opening, we need to make sure we're watching the close
(in particular, df close before the opening is confirmed).

So, keep a pointer, and if it's not set in drop_to_chain, set it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-02 11:41:19 +10:30
Rusty Russell
3ca753e0b4 lightningd: fix bug where we didn't correctly change outpoint of splice scid.
We used the original channel funding output number.  I'm not sure if this
was true in the previous code, or a regression I introduced, but it
caused occasonal failures in test_splice_gossip!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-02 11:41:19 +10:30
Rusty Russell
854bda81ac lightningd: don't share funding_depth_cb for non-funding txs.
We use the *same* callback for the funding tx, as well as for inflight dual-funding txs, as well as inflight splice txs.  This is deeply confusing!

Instead, use explicit cbs for splicing and df.  Once they're locked in, use the normal callback.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-02 11:41:19 +10:30
Rusty Russell
1cd53ae53e lightningd: fix watch on existing tx.
We never do this, but we're about to (we always watch before
we broadcast a tx).

We use a `depth` member to avoid calling the callback multiple times
for the same event, but we initialize it to 0.  This means if we
register a watch, and the first thing that happens is that it
reorganizes out, we *don't* make the callback.

Use an impossible value at initialization, instead.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2023-10-02 11:41:19 +10:30