Skip to content

Commit 7fb9a29

Browse files
nGolinemadelinevibes
authored andcommitted
dualopend: fix next_funding reconnect: error if both set it, tx_abort if only peer
Per BOLT #2: when both nodes set next_funding in channel_reestablish but txids differ, MUST send error and fail the channel; when only the peer sets next_funding, MUST send tx_abort instead. Track we_set_next_funding before tlvs is overwritten by the received channel_reestablish so the distinction survives the receive. Includes a regression test: reconnect two nodes with a corrupted inflight txid so both set next_funding with mismatched values, and verify both sides call open_err_fatal. Changelog-None
1 parent b52d545 commit 7fb9a29

2 files changed

Lines changed: 67 additions & 4 deletions

File tree

openingd/dualopend.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3976,7 +3976,9 @@ static void do_reconnect_dance(struct state *state)
39763976
* - MUST set `next_funding_txid` to the txid of that interactive transaction.
39773977
*/
39783978
tlvs = tlv_channel_reestablish_tlvs_new(tmpctx);
3979-
if (!tx_state->remote_funding_sigs_rcvd) {
3979+
/* Track whether we set next_funding before tlvs is overwritten by received msg */
3980+
bool we_set_next_funding = !tx_state->remote_funding_sigs_rcvd;
3981+
if (we_set_next_funding) {
39803982
tlvs->next_funding = talz(tlvs, struct tlv_channel_reestablish_tlvs_next_funding);
39813983
tlvs->next_funding->next_funding_txid = tx_state->funding.txid;
39823984
tlvs->next_funding->retransmit_flags = 1; /* COMMITMENT_SIGNED */
@@ -4082,15 +4084,21 @@ static void do_reconnect_dance(struct state *state)
40824084
status_debug("Unable to send our sigs, our psbt isn't signed");
40834085
} else
40844086
status_debug("No commitment, not sending our sigs (reconnected)");
4087+
} else if (we_set_next_funding) {
4088+
/* BOLT #2: if it also sets `next_funding` in its own
4089+
* `channel_reestablish`, but the values don't match:
4090+
* - MUST send an `error` and fail the channel. */
4091+
open_err_fatal(state, "next_funding_txid %s doesn't match ours %s",
4092+
fmt_bitcoin_txid(tmpctx,
4093+
&tlvs->next_funding->next_funding_txid),
4094+
fmt_bitcoin_txid(tmpctx,
4095+
&tx_state->funding.txid));
40854096
} else {
4086-
peer_billboard(true, "Non-matching next_funding on reconnect. Aborting.");
40874097
open_abort(state, "Sent next_funding_txid %s doesn't match ours %s",
4088-
40894098
fmt_bitcoin_txid(tmpctx,
40904099
&tlvs->next_funding->next_funding_txid),
40914100
fmt_bitcoin_txid(tmpctx,
40924101
&tx_state->funding.txid));
4093-
return;
40944102
}
40954103
}
40964104

tests/test_opening.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@
77
from pyln.testing.utils import FUNDAMOUNT
88

99
from pathlib import Path
10+
import os
1011
import pytest
1112
import re
13+
import threading
1214
import unittest
1315
import time
1416

@@ -172,6 +174,59 @@ def test_v2_open_sigs_reconnect_2(node_factory, bitcoind):
172174
l2.daemon.wait_for_log(r'to CHANNELD_NORMAL')
173175

174176

177+
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
178+
@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "sqlite3-specific DB manipulation")
179+
@pytest.mark.openchannel('v2')
180+
def test_v2_open_reconnect_next_funding_mismatch(node_factory, bitcoind):
181+
"""Both nodes set next_funding on reconnect but disagree on txid: error sent, channel fails."""
182+
# l2 always sends tx_signatures first. Disconnecting l2 just before it
183+
# sends tx_signatures means neither node ever receives remote tx_sigs, so
184+
# both end up in DUALOPEND_OPEN_COMMITTED with remote_funding_sigs_rcvd=False
185+
# and will set next_funding in channel_reestablish.
186+
broken = r'dualopend daemon died before signed PSBT returned|Owning subdaemon dualopend died'
187+
l1, l2 = node_factory.get_nodes(2, opts=[
188+
{'may_reconnect': True, 'dev-no-reconnect': None, 'broken_log': broken},
189+
{'disconnect': ['-WIRE_TX_SIGNATURES'], 'may_reconnect': True,
190+
'dev-no-reconnect': None, 'broken_log': broken}
191+
])
192+
193+
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
194+
amount = 2**24
195+
bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['p2tr'], amount / 10**8 + 0.01)
196+
bitcoind.generate_block(1)
197+
wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0)
198+
199+
# -WIRE_TX_SIGNATURES causes fundchannel to block (it waits for reconnect
200+
# that will never come due to dev-no-reconnect); run it in a daemon thread
201+
# so the test can proceed. l2.stop() below unblocks it via peer death.
202+
def _fund():
203+
try:
204+
l1.rpc.fundchannel(l2.info['id'], 100000)
205+
except Exception:
206+
pass
207+
208+
threading.Thread(target=_fund, daemon=True).start()
209+
210+
# Both have exchanged commitment_signed (inflight in DB) but no tx_sigs yet.
211+
# Use any() because the channel record may not exist yet when the thread starts.
212+
wait_for(lambda: any(c['state'] == 'DUALOPEND_OPEN_COMMITTED'
213+
for c in l1.rpc.listpeerchannels()['channels']))
214+
wait_for(lambda: any(c['state'] == 'DUALOPEND_OPEN_COMMITTED'
215+
for c in l2.rpc.listpeerchannels()['channels']))
216+
217+
# Corrupt l2's stored funding txid so it disagrees with l1's on reconnect.
218+
l2.stop()
219+
l2.db_manip("UPDATE channel_funding_inflights SET funding_tx_id = X'{}'".format('01' * 32))
220+
l2.start()
221+
222+
# Reconnect: both will set next_funding in channel_reestablish but with
223+
# different txids, triggering open_err_fatal on each side per BOLT #2.
224+
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
225+
226+
l1.daemon.wait_for_log(r"next_funding_txid .* doesn't match ours")
227+
l2.daemon.wait_for_log(r"next_funding_txid .* doesn't match ours")
228+
229+
175230
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
176231
@pytest.mark.openchannel('v2')
177232
def test_v2_open_sigs_reconnect_1(node_factory, bitcoind):

0 commit comments

Comments
 (0)