```
Already in transaction from lightningd/plugin.c:727
```
There are two callers, and one didn't disable transactions, so do it in plugin_exclusive_loop.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we've asserted that channeld would tell lightningd the same thing it
would do anyway, we can simply have channeld say "enable=True|False" and
lightningd fill in the other fields.
This means there's a pile of things channeld doesn't need to know any more!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
channeld used to talk directly to gossipd, so it made sense for it to
tell gossipd directly when it wanted it to make a new channel_update.
When that changed with v0.11, we simply directed the message via
lightningd.
But much of the information is actually told to channeld by lightningd!
So I applied this assertion and ran the test suite, before the next patch makes it redundant.
We got one assertion: test_setchannel_zero deliberately drives the
advertized htlc_max over the real htlc max in test_setchannel_zero for
testing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have to work quite hard to do this, since we don't want to call
finish if the broadcast has been freed in the meantime.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously, every broadcast was attached to a channel, but we can
make it explicit, so when the context is freed, the re-broadcast stops
(if rebroadcast is set).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the context is freed, the callback isn't called. This doesn't matter
yet, since our callbacks tend to be such that the callback itself is
required to free things, but it's clearer this way and allows more
flexible usage in following patches.
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>
We should really unify the cases of a local request, vs a forwarded
request, but for now, don't steal the request onto the plugin, and
if we return from the plugin and the request is gone, don't get upset.
This uncovered a case where we weren't inside a transaction, in
test_hook_crash, so fix that.
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>
It was always a bit weird they weren't, and it seems a premature
optimization to make the callbacks to this themselves.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's per-plugin, so why is there a single map for all plugins? It
works because we always make unique ids, but it's weird.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When using DEBUG_SUBD with pytest:
```
lightningd: Unknown decode for --dev-debugger=<subprocess>
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
During the changeset calculation after the `openchannel2_sign`
hook.
So this commit patch the problem with the following change:
- Addressed an issue where `psbt_get_changeset` was modifying the original PSBT unnecessarily.
- This modification led to problems with a different hsmd, as referenced in [Issue #6672](https://github.com/ElementsProject/lightning/issues/6672).
- Noted a potential optimization where only a subpart of the PSBT
needs to be cloned, as the mutation is specific to inputs.
Link: https://github.com/ElementsProject/lightning/issues/6672
Reported-by: @devrandom
Suggested-by: Ken Sedgwick <ken@bonsai.com>
Co-Developed-by: Ken Sedgwick <ken@bonsai.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Added: Plugins: plugins can now specify (unknown) even messages we should accept from peers.
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>
Previously, we would forward the message to a subd, but now we have
the case where the subd is gone, but we're still connected. If the
peer anything but a reestablish in that state, we drop the connection.
Instead, an error should always make us fail the channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We generalize the current df-only "aborted" flag (and invert it) to a
"disconnected" flag in the peer status message.
We convert it back to the aborted flag for now inside subd.c, but that's
next.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
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>
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>
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>
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>
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>
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>
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>
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`
We usually hand times by copy, not by pointer (and if we did, they should
be const!). I noticed this particularly for the state changed code, but
it goes down to to json_add_timeiso, so I fixed that too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We should use capability tests for states (can you add htlcs?) rather than vague
descriptions (are you closing?).
And as much as possible, use switch () statements to force us to think
about all the cases, especially when we add new states!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
