dualopend: fix next_funding reconnect: error if both set it, tx_abort if only peer
What changed, and why it matters
This commit fixes a bug in Core Lightning's channel reconnection handshake for v2 (dual-funded) channel opens. Before the fix, the code could not tell whether it had itself sent a 'next_funding' field in its own reconnection message, so it always responded to a peer's mismatched 'next_funding' with a gentler 'tx_abort'. The BOLT specification requires a stronger 'error' and channel failure when both sides sent 'next_funding' but disagree, and only a 'tx_abort' when only the peer sent it. The patch tracks that distinction correctly and adds a regression test that corrupts one side's stored funding transaction ID to force the mismatch.
Review and merge. The change is a targeted protocol-compliance fix with a regression test. Operators should upgrade to avoid incorrect handling of inconsistent next_funding during v2 dual-funded channel reconnects.
Security signals we found
Protocol-state mismatch on reconnect could previously be handled as a soft abort instead of a fatal error
BOLT #2 compliance fix: both-set mismatch now triggers error/channel failure
Only-peer-set case now correctly triggers tx_abort instead of error
Regression test corrupts stored funding txid to force disagreeing next_funding values
Evidence from the diff
In openingd/dualopend.c’s do_reconnect_dance(), the local TLV channel_reestablish structure is first populated with our next_funding field, then later overwritten by the peer’s incoming channel_reestablish TLVs. That overwrite erased the knowledge of whether we had set next_funding. The patch captures we_set_next_funding before the receive and uses it in the post-receive logic: if both sides set next_funding and the txids differ, call open_err_fatal() (sending error and failing the channel); if only the peer set it, call open_abort() (sending tx_abort). A Python regression test manipulates l2’s SQLite DB to corrupt the inflight funding_tx_id, reconnects, and verifies both nodes log the mismatch fatal error.
Changed components
openingd/dualopend.ctests/test_opening.pyInspect captured patch +67 / −4
diff --git a/openingd/dualopend.c b/openingd/dualopend.c
index 451a2b1c..e1940a44 100644
--- a/openingd/dualopend.c
+++ b/openingd/dualopend.c
@@ -3976,7 +3976,9 @@ static void do_reconnect_dance(struct state *state)
* - MUST set `next_funding_txid` to the txid of that interactive transaction.
*/
tlvs = tlv_channel_reestablish_tlvs_new(tmpctx);
- if (!tx_state->remote_funding_sigs_rcvd) {
+ /* Track whether we set next_funding before tlvs is overwritten by received msg */
+ bool we_set_next_funding = !tx_state->remote_funding_sigs_rcvd;
+ if (we_set_next_funding) {
tlvs->next_funding = talz(tlvs, struct tlv_channel_reestablish_tlvs_next_funding);
tlvs->next_funding->next_funding_txid = tx_state->funding.txid;
tlvs->next_funding->retransmit_flags = 1; /* COMMITMENT_SIGNED */
@@ -4082,15 +4084,21 @@ static void do_reconnect_dance(struct state *state)
status_debug("Unable to send our sigs, our psbt isn't signed");
} else
status_debug("No commitment, not sending our sigs (reconnected)");
+ } else if (we_set_next_funding) {
+ /* BOLT #2: if it also sets `next_funding` in its own
+ * `channel_reestablish`, but the values don't match:
+ * - MUST send an `error` and fail the channel. */
+ open_err_fatal(state, "next_funding_txid %s doesn't match ours %s",
+ fmt_bitcoin_txid(tmpctx,
+ &tlvs->next_funding->next_funding_txid),
+ fmt_bitcoin_txid(tmpctx,
+ &tx_state->funding.txid));
} else {
- peer_billboard(true, "Non-matching next_funding on reconnect. Aborting.");
open_abort(state, "Sent next_funding_txid %s doesn't match ours %s",
-
fmt_bitcoin_txid(tmpctx,
&tlvs->next_funding->next_funding_txid),
fmt_bitcoin_txid(tmpctx,
&tx_state->funding.txid));
- return;
}
}
diff --git a/tests/test_opening.py b/tests/test_opening.py
index 918e37bb..dcfbdfdc 100644
--- a/tests/test_opening.py
+++ b/tests/test_opening.py
@@ -7,8 +7,10 @@ from utils import (
from pyln.testing.utils import FUNDAMOUNT
from pathlib import Path
+import os
import pytest
import re
+import threading
import unittest
import time
@@ -172,6 +174,59 @@ def test_v2_open_sigs_reconnect_2(node_factory, bitcoind):
l2.daemon.wait_for_log(r'to CHANNELD_NORMAL')
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "sqlite3-specific DB manipulation")
+@pytest.mark.openchannel('v2')
+def test_v2_open_reconnect_next_funding_mismatch(node_factory, bitcoind):
+ """Both nodes set next_funding on reconnect but disagree on txid: error sent, channel fails."""
+ # l2 always sends tx_signatures first. Disconnecting l2 just before it
+ # sends tx_signatures means neither node ever receives remote tx_sigs, so
+ # both end up in DUALOPEND_OPEN_COMMITTED with remote_funding_sigs_rcvd=False
+ # and will set next_funding in channel_reestablish.
+ broken = r'dualopend daemon died before signed PSBT returned|Owning subdaemon dualopend died'
+ l1, l2 = node_factory.get_nodes(2, opts=[
+ {'may_reconnect': True, 'dev-no-reconnect': None, 'broken_log': broken},
+ {'disconnect': ['-WIRE_TX_SIGNATURES'], 'may_reconnect': True,
+ 'dev-no-reconnect': None, 'broken_log': broken}
+ ])
+
+ l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+ amount = 2**24
+ bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['p2tr'], amount / 10**8 + 0.01)
+ bitcoind.generate_block(1)
+ wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0)
+
+ # -WIRE_TX_SIGNATURES causes fundchannel to block (it waits for reconnect
+ # that will never come due to dev-no-reconnect); run it in a daemon thread
+ # so the test can proceed. l2.stop() below unblocks it via peer death.
+ def _fund():
+ try:
+ l1.rpc.fundchannel(l2.info['id'], 100000)
+ except Exception:
+ pass
+
+ threading.Thread(target=_fund, daemon=True).start()
+
+ # Both have exchanged commitment_signed (inflight in DB) but no tx_sigs yet.
+ # Use any() because the channel record may not exist yet when the thread starts.
+ wait_for(lambda: any(c['state'] == 'DUALOPEND_OPEN_COMMITTED'
+ for c in l1.rpc.listpeerchannels()['channels']))
+ wait_for(lambda: any(c['state'] == 'DUALOPEND_OPEN_COMMITTED'
+ for c in l2.rpc.listpeerchannels()['channels']))
+
+ # Corrupt l2's stored funding txid so it disagrees with l1's on reconnect.
+ l2.stop()
+ l2.db_manip("UPDATE channel_funding_inflights SET funding_tx_id = X'{}'".format('01' * 32))
+ l2.start()
+
+ # Reconnect: both will set next_funding in channel_reestablish but with
+ # different txids, triggering open_err_fatal on each side per BOLT #2.
+ l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+ l1.daemon.wait_for_log(r"next_funding_txid .* doesn't match ours")
+ l2.daemon.wait_for_log(r"next_funding_txid .* doesn't match ours")
+
+
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
@pytest.mark.openchannel('v2')
def test_v2_open_sigs_reconnect_1(node_factory, bitcoind):
Why this scored 57/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.