lightningd: cancel watching original funding when we switch to the new one via splice.
What the developer wrote
Authored by Rusty Russell
lightningd: cancel watching original funding when we switch to the new one via splice.
This happens if the channel is *not* announcable yet. Then we hit the assertion
in funding_depth_cb that the txid is the same as the current funding.txid.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: fixed crash when we splice a channel which hasn't been announced yet.
What changed, and why it matters
This patch fixes a crash in Core Lightning that could happen when using the experimental splicing feature on a channel that hasn't been publicly announced yet. The crash was caused by the node still watching the old funding transaction while switching to a new one after a splice. The fix stops watching the old funding transaction before updating to the new one. It is a bug fix for an experimental feature and does not appear to be an exploitable security vulnerability.
Treat as a stability bug fix for an experimental feature. No urgent security action is required, but operators using experimental splicing should upgrade to avoid crashes when splicing unannounced channels.
Security signals we found
Fixes assertion/crash in funding_depth_cb
Adds channel_unwatch_funding() to remove stale txwatch before funding update
Changelog labels fix as EXPERIMENTAL
Regression test enabled for unannounced splice path
Evidence from the diff
In handle_peer_splice_locked(), update_channel_from_inflight() was called before the old funding transaction watch was removed. For unannounced channels, this caused funding_depth_cb to later assert that the watched txid matched channel->funding.txid, which it no longer did after the splice. The fix introduces channel_unwatch_funding() to remove the txwatch on the old funding outpoint before calling update_channel_from_inflight(). A regression test (test_splice_unannounced) is enabled and marked for both v1 and v2 openchannel protocols.
Changed components
lightningd/channel_control.clightningd/peer_control.clightningd/peer_control.htests/test_splicing.pyInspect captured patch +35 / −2
diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c
index a5fb7d46..fd41ac47 100644
--- a/lightningd/channel_control.c
+++ b/lightningd/channel_control.c
@@ -1125,7 +1125,8 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg)
wallet_htlcsigs_confirm_inflight(channel->peer->ld->wallet, channel,
&inflight->funding->outpoint);
- update_channel_from_inflight(channel->peer->ld, channel, inflight, true);
+ /* Stop watching previous funding tx (could be, for announcement) */
+ channel_unwatch_funding(channel->peer->ld, channel);
/* Stash prev funding data so we can log it after scid is updated
* (to get the blockheight) */
@@ -1133,6 +1134,8 @@ static void handle_peer_splice_locked(struct channel *channel, const u8 *msg)
prev_funding_sats = channel->funding_sats;
prev_funding_out = channel->funding;
+ update_channel_from_inflight(channel->peer->ld, channel, inflight, true);
+
channel->our_msat.millisatoshis += splice_amnt * 1000; /* Raw: splicing */
channel->msat_to_us_min.millisatoshis += splice_amnt * 1000; /* Raw: splicing */
channel->msat_to_us_max.millisatoshis += splice_amnt * 1000; /* Raw: splicing */
diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
index 7ecd01a6..9a5494b0 100644
--- a/lightningd/peer_control.c
+++ b/lightningd/peer_control.c
@@ -2347,6 +2347,14 @@ void channel_watch_wrong_funding(struct lightningd *ld, struct channel *channel)
}
}
+/* We need to do this before we change channel funding (for splice), otherwise
+ * funding_depth_cb will fail the assertion that it's the current funding tx */
+void channel_unwatch_funding(struct lightningd *ld, struct channel *channel)
+{
+ tal_free(find_txwatch(ld->topology,
+ &channel->funding.txid, funding_depth_cb, channel));
+}
+
void channel_watch_funding(struct lightningd *ld, struct channel *channel)
{
log_debug(channel->log, "Watching for funding txid: %s",
diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h
index 382d0826..63826f5b 100644
--- a/lightningd/peer_control.h
+++ b/lightningd/peer_control.h
@@ -131,6 +131,7 @@ void update_channel_from_inflight(struct lightningd *ld,
const struct channel_inflight *inflight,
bool is_splice);
+void channel_unwatch_funding(struct lightningd *ld, struct channel *channel);
void channel_watch_funding(struct lightningd *ld, struct channel *channel);
/* If this channel has a "wrong funding" shutdown, watch that too. */
diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c
index 91b07448..fe895a47 100644
--- a/lightningd/test/run-invoice-select-inchan.c
+++ b/lightningd/test/run-invoice-select-inchan.c
@@ -244,6 +244,16 @@ struct channel *find_channel_by_id(const struct peer *peer UNNEEDED,
struct plugin *find_plugin_for_command(struct lightningd *ld UNNEEDED,
const char *cmd_name UNNEEDED)
{ fprintf(stderr, "find_plugin_for_command called!\n"); abort(); }
+/* Generated stub for find_txwatch_ */
+struct txwatch *find_txwatch_(struct chain_topology *topo UNNEEDED,
+ const struct bitcoin_txid *txid UNNEEDED,
+ enum watch_result (*cb)(struct lightningd *ld UNNEEDED,
+ const struct bitcoin_txid * UNNEEDED,
+ const struct bitcoin_tx * UNNEEDED,
+ unsigned int depth UNNEEDED,
+ void *arg) UNNEEDED,
+ void *arg UNNEEDED)
+{ fprintf(stderr, "find_txwatch_ called!\n"); abort(); }
/* Generated stub for fixup_htlcs_out */
void fixup_htlcs_out(struct lightningd *ld UNNEEDED)
{ fprintf(stderr, "fixup_htlcs_out called!\n"); abort(); }
diff --git a/tests/test_splicing.py b/tests/test_splicing.py
index ec758006..966c75f6 100644
--- a/tests/test_splicing.py
+++ b/tests/test_splicing.py
@@ -558,8 +558,9 @@ def test_route_by_old_scid(node_factory, bitcoind):
l1.rpc.waitsendpay(inv2['payment_hash'])
-@pytest.mark.xfail(strict=True)
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
+@pytest.mark.openchannel('v1')
+@pytest.mark.openchannel('v2')
def test_splice_unannounced(node_factory, bitcoind):
l1, l2 = node_factory.line_graph(2, fundamount=1000000, wait_for_announce=False, opts={'experimental-splicing': None})
diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
index 6371dfb2..c24382c1 100644
--- a/wallet/test/run-wallet.c
+++ b/wallet/test/run-wallet.c
@@ -243,6 +243,16 @@ void fatal(const char *fmt UNNEEDED, ...)
/* Generated stub for fatal_vfmt */
void fatal_vfmt(const char *fmt UNNEEDED, va_list ap UNNEEDED)
{ fprintf(stderr, "fatal_vfmt called!\n"); abort(); }
+/* Generated stub for find_txwatch_ */
+struct txwatch *find_txwatch_(struct chain_topology *topo UNNEEDED,
+ const struct bitcoin_txid *txid UNNEEDED,
+ enum watch_result (*cb)(struct lightningd *ld UNNEEDED,
+ const struct bitcoin_txid * UNNEEDED,
+ const struct bitcoin_tx * UNNEEDED,
+ unsigned int depth UNNEEDED,
+ void *arg) UNNEEDED,
+ void *arg UNNEEDED)
+{ fprintf(stderr, "find_txwatch_ called!\n"); abort(); }
/* Generated stub for force_peer_disconnect */
void force_peer_disconnect(struct lightningd *ld UNNEEDED,
const struct peer *peer UNNEEDED,
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.