From 48bb2d831bf2bd52effcab947ab786625809457a Mon Sep 17 00:00:00 2001 From: niftynei Date: Mon, 30 Oct 2023 21:17:38 -0500 Subject: [PATCH] dual-fund: don't re-notify plugin on arrival of sigs (2nd time) When we got our peer's sigs, if we were the remote, we would re-notify the plugin, which in turn would re-send the tx-sigs to use. In the case of CLN, we'd then - break, because we'd re-forward the sigs to the `openchannel` plugin, which was then in the wrong state (MULTIFUNDCHANNEL_SIGNED) spenderp: plugins/spender/openchannel.c:598: json_peer_sigs: Assertion `dest->state == MULTIFUNDCHANNEL_SECURED' failed. spenderp: FATAL SIGNAL 6 (version 5880d59-modded) In the case of eclair, they'd just see our 2nd TX_SIGS message and @t-bast would complain: > This test works, with one minor issue: on reconnection, cln sends its tx_signatures twice (duplicate?). This commit does two things: - has the openchannel / spender plugin log a broken instead of crashing when the state is not what we're expecting - stops us from calling the `funder` plugin if this is a replay/second receipt of commit-sigs. --- lightningd/dual_open_control.c | 22 +++++++++++++++------- plugins/spender/openchannel.c | 9 ++++++++- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 6d18a3349..96754019f 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1297,7 +1297,7 @@ wallet_update_channel(struct lightningd *ld, return inflight; } -static void +static bool wallet_update_channel_commit(struct lightningd *ld, struct channel *channel, struct channel_inflight *inflight, @@ -1343,10 +1343,13 @@ wallet_update_channel_commit(struct lightningd *ld, type_to_string(tmpctx, struct bitcoin_txid, &inflight_txid), type_to_string(tmpctx, struct bitcoin_txid, &txid)); } - } else { - inflight_set_last_tx(inflight, remote_commit, *remote_commit_sig); - wallet_inflight_save(ld->wallet, inflight); + return false; } + + + inflight_set_last_tx(inflight, remote_commit, *remote_commit_sig); + wallet_inflight_save(ld->wallet, inflight); + return true; } @@ -3469,6 +3472,7 @@ static void handle_commit_received(struct subd *dualopend, struct openchannel2_psbt_payload *payload; struct channel_inflight *inflight; struct command *cmd; + bool updated; if (!fromwire_dualopend_commit_rcvd(tmpctx, msg, &remote_commit, @@ -3491,9 +3495,9 @@ static void handle_commit_received(struct subd *dualopend, return; } - wallet_update_channel_commit(ld, channel, inflight, - remote_commit, - &remote_commit_sig); + updated = wallet_update_channel_commit(ld, channel, inflight, + remote_commit, + &remote_commit_sig); /* FIXME: handle RBF pbases */ if (pbase && channel->state != DUALOPEND_AWAITING_LOCKIN) { @@ -3517,6 +3521,10 @@ static void handle_commit_received(struct subd *dualopend, was_pending(command_success(cmd, response)); return; case REMOTE: + if (!updated) { + log_info(channel->log, "Already had sigs, skipping notif"); + return; + } payload = tal(dualopend, struct openchannel2_psbt_payload); payload->ld = ld; payload->dualopend = dualopend; diff --git a/plugins/spender/openchannel.c b/plugins/spender/openchannel.c index 669c16be3..e3996888f 100644 --- a/plugins/spender/openchannel.c +++ b/plugins/spender/openchannel.c @@ -595,7 +595,14 @@ static struct command_result *json_peer_sigs(struct command *cmd, if (dest->state == MULTIFUNDCHANNEL_UPDATED) dest->state = MULTIFUNDCHANNEL_SIGNED_NOT_SECURED; else { - assert(dest->state == MULTIFUNDCHANNEL_SECURED); + if (dest->state != MULTIFUNDCHANNEL_SECURED) { + plugin_log(cmd->plugin, LOG_BROKEN, + "mfc %"PRIu64":`openchannel_peer_sigs` " + " expected state MULTIFUNDCHANNEL_SECURED (%d)," + " state is %d", dest->mfc->id, + MULTIFUNDCHANNEL_SECURED, + dest->state); + } dest->state = MULTIFUNDCHANNEL_SIGNED; }