dual_open_control: check dualopend liveness before validating PSBT signatures

lightningd/dual_open_control.c

@@ -1070,6 +1070,19 @@ openchannel2_sign_hook_cb(struct openchannel2_psbt_payload *payload STEALS)
 	/* Whatever happens, we free the payload */
 	tal_steal(tmpctx, payload);
 
+	/* Peer's gone away, let's try reconnecting.
+	 * Check this first: if dualopend died (e.g. peer disconnected),
+	 * there's no point validating signatures since we can't send
+	 * them anyway.  The disconnect notification can also race with
+	 * this hook in plugins, causing them to clean up state and
+	 * return the PSBT unsigned. */
+	if (!payload->dualopend) {
+		channel_saved_err_broken_reconn(channel,
+						"dualopend daemon died"
+						" before signed PSBT returned");
+		return;
+	}
+
 	/* Finalize it, if not already. It shouldn't work entirely */
 	psbt_finalize(payload->psbt);
 
@@ -1115,14 +1128,6 @@ openchannel2_sign_hook_cb(struct openchannel2_psbt_payload *payload STEALS)
 	msg = towire_dualopend_send_tx_sigs(NULL, inflight->funding_psbt);
 
 send_msg:
-	/* Peer's gone away, let's try reconnecting */
-	if (!payload->dualopend) {
-		channel_saved_err_broken_reconn(channel,
-						"dualopend daemon died"
-						" before signed PSBT returned");
-		tal_free(msg);
-		return;
-	}
 	tal_del_destructor2(payload->dualopend,
 			    openchannel2_psbt_remove_dualopend,
 			    payload);

tests/test_opening.py

@@ -2993,3 +2993,45 @@ def test_zeroconf_withhold_htlc_failback(node_factory, bitcoind):
 
     # l1's channel to l2 is still normal — no force-close
     assert only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['state'] == 'CHANNELD_NORMAL'
+
+
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
+@pytest.mark.openchannel('v2')
+def test_inflight_dbload(node_factory, bitcoind):
+    """Disconnect during dual-fund sign should not trigger spurious BROKEN.
+
+    Regression test for #8902: when the peer disconnects while the
+    openchannel2_sign hook is being processed, the funder plugin's
+    disconnect notification races with the hook, causing it to return
+    the PSBT unsigned.  Before the fix, this produced a spurious
+    'Plugin must return a psbt with signatures' BROKEN message.
+    """
+    disconnects = ["+WIRE_COMMITMENT_SIGNED"]
+
+    opts = [{'experimental-dual-fund': None, 'dev-no-reconnect': None,
+             'may_reconnect': True, 'disconnect': disconnects},
+            {'experimental-dual-fund': None, 'dev-no-reconnect': None,
+             'may_reconnect': True, 'funder-policy': 'match',
+             'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat',
+             'lease-fee-basis': 100,
+             # The daemon-death BROKEN is expected on disconnect
+             'broken_log': 'dualopend daemon died before signed PSBT returned'}]
+
+    l1, l2 = node_factory.get_nodes(2, opts=opts)
+
+    feerate = 2000
+    amount = 500000
+    l1.fundwallet(20000000)
+    l2.fundwallet(20000000)
+
+    # l1 leases a channel from l2
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    rates = l1.rpc.dev_queryrates(l2.info['id'], amount, amount)
+    l1.rpc.fundchannel(l2.info['id'], amount, request_amt=amount,
+                       feerate='{}perkw'.format(feerate),
+                       compact_lease=rates['compact_lease'])
+    l1.daemon.wait_for_log(r'dev_disconnect: \+WIRE_COMMITMENT_SIGNED')
+
+    # Restart l1; before the fix this would leave a spurious BROKEN
+    # 'Plugin must return a psbt with signatures' in l2's log.
+    l1.restart()