Looking through logs I was surprise to see:
```
lightningd-1 2023-10-26T03:42:36.824Z INFO lightningd: Sending 200000000msat over 1 hops to deliver 200000000msat
```
On a re-payment where we simply returned from sendpay immediately! Move that log to later.
We used to have "unsaved" payments: now we don't we can use
our normal "iterator" pattern rather than returning arrays.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The log messages were changed but the test fields weren’t updated, resulting in some test flakiness. Being more explicit with the log message we’re looking for should help.
Changelog-None
This means refactoring out some of the generic anchor info, from the
per-commitment-tx info (we can have at least two, perhaps more with
splicing!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We try to use anchors to CPFP our own commitment, but what if they
get there first? We also need to use anchors on the commitment
txs they broadcast.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `recover` command to force (unused) lightningd node to restart with `--recover` flag.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes `check` much more thorough, and useful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `check` now does much more checking on every command (not just basic parameter types).
We had a complaint that you can't CPFP a mutual close, which you
should be able to do.
Fixes: #6692
Changelog-Fixed: wallet: close change outputs show up immediately in `listfunds` so you can CPFP.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
FAILED tests/test_connection.py::test_remote_addr_port - TimeoutError: Unable to find "[re.compile('Update our node_announcement for discovered address')]" in logs.
```
Because it can happen before the "Already have funding locked in" message:
```
lightningd-2 2023-10-24T22:07:02.018Z DEBUG gossipd: Update our node_announcement for discovered address: 127.0.0.1:1234
lightningd-2 2023-10-24T22:07:02.019Z DEBUG lightningd: Plugin chanbackup returned from peer_connected hook call
lightningd-2 2023-10-24T22:07:02.019Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-chan#2: Peer has reconnected, state CHANNELD_NORMAL: connecting subd
lightningd-2 2023-10-24T22:07:02.036Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-channeld-chan#2: pid 63142, msgfd 67
lightningd-2 2023-10-24T22:07:02.037Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-chan#2: Already have funding locked in (and ready to announce)
```
Also, wait_for_log() asserts itself, no need to assert on result.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Wait to be sure l1->l2 is ready. And use the same pattern for l2->l3.
```
def test_excluded_adjacent_routehint(node_factory, bitcoind):
"""Test case where we try have a routehint which leads to an adjacent
node, but the result exceeds our maxfee; we crashed trying to find
what part of the path was most expensive in that case
"""
l1, l2, l3 = node_factory.line_graph(3)
# We'll be forced to use routehint, since we don't know about l3.
wait_for(lambda: len(l3.rpc.listchannels(source=l2.info['id'])['channels']) == 1)
inv = l3.rpc.invoice(10**3, "lbl", "desc", exposeprivatechannels=l2.get_channel_scid(l3))
# This will make it reject the routehint.
err = r'Fee exceeds our fee budget: 1msat > 0msat, discarding route'
with pytest.raises(RpcError, match=err):
> l1.rpc.pay(bolt11=inv['bolt11'], maxfeepercent=0, exemptfee=0)
tests/test_pay.py:3420:
...
> raise RpcError(method, payload, resp['error'])
E pyln.client.lightning.RpcError: RPC call failed: method: pay, payload: {'bolt11': 'lnbcrt10n1pjntczasp59x0weqkg4u9amd364yaeyw6rmgnmf9qtra6epylcntvt65yalpzspp5x8wgtmjhq33qruk6mmhutyr7w74xxjhct7v9tppel0t9p4rtautsdq8v3jhxccxqyjw5qcqp9rzjqgkjyd3q5dv6gllh77kygly9c3kfy0d9xwyjyxsq2nq3c83u5vw4jqqqvuqqqqgqqqqqqqqpqqqqqzsqqc9qxpqysgq4euy2qyzl2nufpxv6ahf0s5zry0h5dgrpa5adwu4swrdvjw7qe48cj8kp5fl7k20ex0x3dnk6e8xk5jp82snrdcr6he7eyqd0wrmvlgqwe5nma', 'maxfeepercent': 0, 'exemptfee': 0}, error: {'code': 210, 'message': 'Destination 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d is not reachable directly and all routehints were unusable.', 'attempts': [{'status': 'failed', 'failreason': 'Destination 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d is not reachable directly and all routehints were unusable.', 'partid': 0, 'amount_msat': 1000msat}]}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make sure plugin has got message to connectd before sending!
```
def test_even_sendcustommsg(node_factory):
l1, l2 = node_factory.get_nodes(2, opts={'log-level': 'io',
'allow_warning': True})
l1.connect(l2)
# Even-numbered message
msg = hex(43690)[2:] + ('ff' * 30) + 'bb'
# l2 will hang up when it gets this.
l1.rpc.sendcustommsg(l2.info['id'], msg)
l2.daemon.wait_for_log(r'\[IN\] {}'.format(msg))
l1.daemon.wait_for_log('Invalid unknown even msg')
wait_for(lambda: l1.rpc.listpeers(l2.info['id'])['peers'] == [])
# Now with a plugin which allows it
l1.connect(l2)
l2.rpc.plugin_start(os.path.join(os.getcwd(), "tests/plugins/allow_even_msgs.py"))
l1.rpc.sendcustommsg(l2.info['id'], msg)
l2.daemon.wait_for_log(r'\[IN\] {}'.format(msg))
> l2.daemon.wait_for_log(r'allow_even_msgs.*Got message 43690')
tests/test_misc.py:3623:
...
> raise TimeoutError('Unable to find "{}" in logs.'.format(exs))
E TimeoutError: Unable to find "[re.compile('allow_even_msgs.*Got message 43690')]" in logs.
contrib/pyln-testing/pyln/testing/utils.py:327: TimeoutError
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We remove it from the pending_requests strmap before calling it,
so it doesn't get called again by destroy_plugin.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now the internal code will generate a "PLUGIN_TERMINATED" response
when the plugin dies, we can handle it in plugin_hook.
But we can also simplify it by turning the snapshot of hooks into
a simple array: this means we are robust against any combination of plugins
exiting at any time.
Note: this reveals an issue with test_rpc_command_hook where we run
the request hook again (unexpectedly), so we disable that for the next
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had special code to fail a forwarded request, but not for an
internally-generated request. Instead, we should pretend the (dead)
plugin responded with a PLUGIN_TERMINATED error, and handle the
request through the normal paths.
This breaks the case where a plugin crashes (or stops itself) in a
hook, so we handle that next.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make sure l1 can see l2's channel and it's in the gossip_store.
```
def test_excluded_adjacent_routehint(node_factory, bitcoind):
"""Test case where we try have a routehint which leads to an adjacent
node, but the result exceeds our maxfee; we crashed trying to find
what part of the path was most expensive in that case
"""
l1, l2, l3 = node_factory.line_graph(3)
# We'll be forced to use routehint, since we don't know about l3.
wait_for(lambda: len(l3.rpc.listchannels(source=l2.info['id'])['channels']) == 1)
inv = l3.rpc.invoice(10**3, "lbl", "desc", exposeprivatechannels=l2.get_channel_scid(l3))
# This will make it reject the routehint.
err = r'Fee exceeds our fee budget: 1msat > 0msat, discarding route'
with pytest.raises(RpcError, match=err):
> l1.rpc.pay(bolt11=inv['bolt11'], maxfeepercent=0, exemptfee=0)
tests/test_pay.py:3420:
...
> raise RpcError(method, payload, resp['error'])
E pyln.client.lightning.RpcError: RPC call failed: method: pay, payload: {'bolt11': 'lnbcrt10n1pjjm8hesp5kp4nfgrj2ev6dz6xuqgxg29hz7263ltlafylhw7nglhtjxeqpn7spp5w92tjq8a354psfhdzmeuytfc6eye4f5egl7tj7s0f5ftz0k4pmcqdq8v3jhxccxqyjw5qcqp9rzjqgkjyd3q5dv6gllh77kygly9c3kfy0d9xwyjyxsq2nq3c83u5vw4jqqqvuqqqqgqqqqqqqqpqqqqqzsqqc9qxpqysgqetjwr6ql24jrz02qhj7pdw3kqynw6j3sgj2h32ufeyzasjyp2j6yc5durewjjpjy5yqtfgdxmdj52n7jk0ylzk5wudk4ffmjyyw6jmsqkvjex9', 'maxfeepercent': 0, 'exemptfee': 0}, error: {'code': 210, 'message': 'Destination 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d is not reachable directly and all routehints were unusable.', 'attempts': [{'status': 'failed', 'failreason': 'Destination 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d is not reachable directly and all routehints were unusable.', 'partid': 0, 'amount_msat': 1000msat}]}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do it here, but it's not necessary, and we also deprive them of the
chance to do so (since we kill them).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was intermittant before: I added a sleep(1) in the code before
sending the error (temporarily) to make it always triggers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We truncate the file on stop(), but don't re-created it on start().
We didn't notice it before, but the net
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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
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
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
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.
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.
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>
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.
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
```
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.
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.
The latter is used when we're put in the db, the former is the uncommitted state.
Currently dbid == 0 is used in addition to the state, which is unwieldy.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Experimental: JSON-RPC: added new dual-funding state `DUALOPEND_OPEN_COMMITTED`
Currently it's half done in funding_depth_cb, and half in
channeld_tell_depth. It's very confusing as a result,
with splicing, dual-funding and zeroconf.
This does introduce a behaviour change: if a channel is NORMAL and
it gets reorganized, we force close (unless we were the one who funded
it, or it's zeroconf anyway). This is safer than continuing to use
the channel in this case!
Some tests are changed to zeroconf to make them work, but v2 doesn't
support zeroconf, so that's removed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is actually a real issue (l1 doesn't see the warning before l2
drops the connection), but it's unrelated to this PR, and will require
another one to fix.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And require --developer to use them.
Also refuse redirection to deprecated APIs if deprecated APIs are disabled!
Changelog-Removed: `dev-sendcustommsg` (use `sendcustommsg`, which was added in v0.10.1)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Added a test for splicing out that exposed some behavior and code glitches that are addressed in this commit.
Added test for splice gossip.
Also added documentation for how to do a splice out.
ChangeLog-Fixed: Added docs, testing, and some fixes related to splicing out, insufficent balance handling, and restarting during a splice.
We were allowed to, but the spec removed that. So we handle warnings
differently from errors now.
This also means the LND "internal error" workaround is done in
lightningd (we still disconnect, but we don't want to close channel).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: we no longer disconnect every time we receive a warning message.
If a node has an onchain balance with at least one uneconomical UTXO, the fundchannel RPC call will lock up the node and will eventually crash it with OOM issues if the economical UTXO(s) do not add up to the fundchannel amount. This is because the while loop never exits because it keeps pulling in the same uneconomical UTXOs forever.
Changelog-Fixed: wallet: fundchannel no longer loops forever if the wallet contains insufficient funds, but an uneconomical UTXO.
Tony Giorgio <tonygiorgio@protonmail.com> says:
Reproduce:
1. Add 1 600 sat UTXO to a fresh node
2. Verify the fundchannel command fails with a low fee rate:
```
./lightning-cli fundchannel 0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a 100000 1000
{
"code": 301,
"message": "Could not afford 100000sat using all 1 available UTXOs: 99522sat short"
}
```
3. Now do the command again, but with a higher fee rate, making the 600 sat UTXO uneconomical:
```
./lightning-cli fundchannel 0366abc8eb4da61e31a8d2c4520d31cabdf58cc5250f855657397f3dd62493938a 100000 10000
```
4. Observe the RPC call and the logs. The RPC call will never return, and the logs will stop after this:
```
2023-04-16T10:58:45.839Z DEBUG plugin-spenderp: mfc 34: multiconnect done.
2023-04-16T10:58:45.839Z DEBUG plugin-spenderp: mfc 34: 'parsefeerate' done
2023-04-16T10:58:45.839Z DEBUG plugin-spenderp: mfc 34: fundpsbt.
```
5. Keep CLN running long enough and you'll eventually run OOM.
It always is for runes we create, but in theory you can take our secret key
and make our own runes with your own tools.
(We correctly refuse runes without uniqueids if they're *not* ours
anyway: uniqueid is only used for our own runes).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
nodeid is only useful when we know the peer we're talking to (e.g. commando).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
No-schema-diff-check: We're simply making optional, not deprecating!
1. When we add a shadow amount, we were using the wrong channel for
the fee calculation.
2. Similarly, when calculating the delay amount.
The result is that we can get WIRE_INCORRECT_CLTV_EXPIRY repeatedly
from nodes.
Reported-by: https://github.com/SjorsFixes: #6620
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changlog-Experimental: Fixed: `renepay` handles ctlv correctly when it varies along a path.
"id" is a magic name, so it was being populated by sqlite3
automatically, starting at 0. Fortunately, we only fetched by id in
one place: to indicate the `stored` flag when asked about an explicit
rune in `showrunes`.
Reported-by: @ShahanaFarooqui
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `showrunes` on a specific rune would always say `stored`: false.
The code to workaround the intermittant error didn't work,
and we finally hit it again:
```
# If reject happens fast enough, connect fails with "disconnected
# during connection"
try:
l3.connect(l1)
except RpcError as err:
> assert "disconnected during connection" in err.error
E assert 'disconnected during connection' in {'code': 402, 'message': 'disconnected during connection'}
E + where {'code': 402, 'message': 'disconnected during connection'} = RpcError("RPC call failed: method: connect, payload: {'id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'host': '127.0.0.1', 'port': 41865}, error: {'code': 402, 'message': 'disconnected during connection'}").error
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
l3.rpc.setconfig('autoclean-cycle', 10)
# First it expires.
> wait_for(lambda: only_one(l3.rpc.listinvoices('inv1')['invoices'])['status'] == 'expired')
```
If we're slow enough, the invoice is cleaned before we see it expire!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As side-effect, getroute(0) is special too.
Reported-by: MiddleW4y in Discord
Fixes: #6577
Changelog-Fixed: `pay` will still use an invoice routehint if path to it doesn't take 1-msat payments.
Reported-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: we clean up properly if a plugin fails to start, and we don't kill all processes if it's from `plugin startdir`.
Don’t send the funding spend to onchaind if we detect it in inflights (aka. a splice). While we already prevented onchaind_funding_spent from being called directly, the call to wallet_channeltxs_add meant onchaind_funding_spent would be called *anyway* on restart. This is now fixed.
Additionally there was a potential for a race problem depending on the firing order of the channel depth and and funding spent events.
Instead of requiring these events fire in a specific order, we make a special “memory only” inflight object to prevent the race regardless of firing order.
Changelog-Fixed: Splice: bugfix for restart related race condition interacting with adversarial close detection.
Rusty Russell
Aditya Sharma
Dusty Daemon
Christian Decker
Chris Guida
Peter Neuroth
Matt Morehouse
Vincenzo Palazzo
Tony Aldon
dergoegge
ShahanaFarooqui
Tony Giorgio
Alex Myers