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>
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>
Mainly removing the PSBT re-marshalling which hasn't had any issues in
recent libwally, and making dev_no_grind into the clearer
dev_no_signature_grind.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Also requires us to expose memleak when !DEVELOPER, however we only
ever used the memleak tracking when the LIGHTNINGD_DEV_MEMLEAK
environment variable was set, so keep that.
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.
```
In function ‘peer_reconnect’,
inlined from ‘init_channel’ at channeld/channeld.c:5890:3,
inlined from ‘main’ at channeld/channeld.c:5951:2:
channeld/channeld.c:5028:21: error: ‘next_matches_inflight’ may be used uninitialized [-Werror=maybe-uninitialized]
5027 | if (remote_next_funding && !next_matches_current
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5028 | && !next_matches_inflight) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~
channeld/channeld.c: In function ‘main’:
channeld/channeld.c:4595:36: note: ‘next_matches_inflight’ was declared here
4595 | bool next_matches_current, next_matches_inflight;
| ^~~~~~~~~~~~~~~~~~~~~
channeld/channeld.c:5042:57: error: ‘inflight’ may be used uninitialized [-Werror=maybe-uninitialized]
5042 | &inflight->outpoint.txid),
| ^
channeld/channeld.c:4594:26: note: ‘inflight’ was declared here
4594 | struct inflight *inflight;
| ^~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:300: channeld/channeld.o] Error 1
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was recommended by @t-bast: if the final spec commits to something
compatible, we can simply advertize and accept both features, but if it
does change in incompatible ways we won't cause problems for nodes
who implement the official spec.
(I split this, so first, we remove the OPT_SPLICE entirely, to make
sure we caught them all. --RR)
Suggested-by: @t-bast
Changelog-None
This make ACINQ seize up, and not send revoke_and_ack. Eventually,
this can cause a bad signature error, should payments go in both
directions, which is a separate bug, but this is the trigger.
See: #6500
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since it's only for transitory splicing info, the new name makes sense.
```
cc channeld/channeld.c
In file included from channeld/channeld.c:23:
./channeld/splice.h:37:8: error: redefinition of 'splice'
struct splice {
^
/usr/include/sys/socket.h:140:8: note: previous definition is here
struct splice {
^
```
Reported-by: @grubles
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #6486
Update the lightningd <-> channeld interface with lots of new commands to needed to facilitate spicing.
Implement the channeld splicing protocol leveraging the interactivetx protocol.
Implement lightningd’s channel_control to support channeld in its splicing efforts.
Changelog-Added: Added the features to enable splicing & resizing of active channels.
This commit addresses an issue to enhance the resilience of core
lightning when receiving node announcements.
According to BOLT 7 (The announcement_signatures Message),
if the node_signature OR the bitcoin_signature is NOT correct,
it is recommended to either send a warning and close the connection or send an error and fail the channel.
In this commit, we take a strict approach. If any error is detected, we
send an error and fail the open channel operation.
This is because the announcement_signatures operation is optional,
and we assume that it must be correct.
lnprototest at commit dea47c29b5541dbfe7fe53cc2598330e897fa4f4 report
the following error now.
```
2023-07-06T21:03:20.930Z DEBUG hsmd: Shutting down
ERROR root:helpers.py:170 Traceback (most recent call last):
File "/home/vincent/Github/lightning/external/lnprototest/tests/helpers.py", line 167, in run_runner
runner.run(test)
File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/runner.py", line 99, in run
all_done = sequence.action(self)
^^^^^^^^^^^^^^^^^^^^^
File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/structure.py", line 55, in action
all_done &= e.action(runner)
^^^^^^^^^^^^^^^^
File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/event.py", line 365, in action
raise EventError(self, "{}: message was {}".format(err, msg.to_str()))
lnprototest.errors.EventError: `Expected msgtype-warning, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "157"},]
============================================================================================================================================================== short test summary info ===============================================================================================================================================================
FAILED tests/test_bolt2-01-close_channel.py::test_close_channel_shutdown_msg_normal_case_receiver_side - AssertionError: `Expected msgtype-shutdown, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "75"},]
FAILED tests/test_bolt2-01-close_channel.py::test_close_channel_shutdown_msg_wrong_script_pubkey_receiver_side - AssertionError: `Expected msgtype-warning, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "157"},]
```
Changelog-Fixes: channeld: Verify the signature sent in announcement_signatures by the counterparty
Reported-by: lnprototest
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
```
channeld WARNING: Bad update_fail_malformed_htlc failure code 4103
```
Warren Togami reports this happening with Bitrefill on every reconnect,
so it's clearly something LND does.
(4103 is TEMPORARY_CHANNEL_FAILURE, which does not belong in update_fail_malformed_htlc).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: We allow update_fail_malformed_htlc with invalid error codes (LND?)
In most cases, it's the same as option_anchor_outputs, but for
fees it's different. This transformation is the simplest:
pass it as a pair, and test it explicitly.
In future we could rationalize some paths, but this was nice
and mechanical.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And no longer insist on opt_quiesce.
Changelog-EXPERIMENTAL: Config: `--experimental-upgrade-protocol` enables simple channel upgrades.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since this was merged, `make extract-peer-csv` was broken!
But the field names changed:
1. `tlv_update_add_tlvs` -> `tlv_update_add_htlc_tlvs`
2. `blinding` -> `blinding_point`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. anchor_to_remote_redeem => bitcoin_wscript_to_remote_anchored,
which matches other witness script producing functions and makes
it clear that it's a to_remote variant.
2. is_anchor_witness_script => is_to_remote_anchored_witness_script
makes it clear that it's about a to_remote output (as altered
when anchors are enabled) not an anchor output!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have the FEERATE_FLOOR constant if you don't care, but usually you want
to use the current bitcoind lower limit, so call get_feerate_floor()
(which is currently the same, but coming!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's defined to be nonull:
```
channeld/channeld.c:2381:2: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/stdlib.h:856:3: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior channeld/channeld.c:2381:2 in
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We would sleep for 10msec (default) and try again, spamming the logs
every second. But we're waiting for revoke_and_ack, and that handler
already sets off the timer, so there's no need to spin at all!
Fixes: #6077
Changelog-Fixed: `channeld`: no longer spin and spam logs when waiting for revoke_and_ack.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The channel->htlcs map was exhibiting unbounded growth, as elements were
never removed from it. This was causing lightning_channeld processes to
consume ever-increasing amounts of memory, and iterating over the map
was causing ever-increasing CPU utilization. There were FIXME comments
suggesting that the intention was to remove HTLCs from the map upon
their deaths. This commit implements that intention.
Changelog-Fixed: channeld no longer retains dead HTLCs in memory.
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 <rusty@rustcorp.com.au>
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: #5636
Changelog-Fixed: channeld: Channel reinitialization no longer fails when the number of outstanding outgoing HTLCs exceeds `max_accepted_htlcs`.
This is the minimal change to meet the desired outcome of https://github.com/lightning/bolts/issues/934
which wants to give obsolete-db nodes a chance to fix things up, before we
close the channel.
We need to dance around a bit here, since we *will* close the channel if
we receive an ERROR, so we suppress that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: We now set the `dont_forward` bit on private channel_update's message_flags (as per latest BOLTs).
Add memleak_ignore_children() so callers can do exclusions themselves.
Having two exclusions was always such a hack!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1) dualopen has fd to connectd
2) channeld needs to take over
3) dualopen passes fd that leads to a connectd over for channeld to use
4) lightningd must receive the fd transfer request and process
5) dualopen shuts down and closes everything it owns
4 & 5 end up in a race. If 5 happens before 4, channeld ends up with an invalid fd for connectd — leaving it in a position to not receive messages.
Lingering for a second makes 4 win the race. Since the daemon is closing anyway, waiting for a second should be alright.
Changelog-Fixed: Fixed a condition for newly created channels that could trigger a need for reconnect.
Rusty Russell
Dusty Daemon
niftynei
Vincenzo Palazzo
Greg Sanders
adi2011
Michael Schmoock
Matt Whitlock