pytest: channels reusing a funding outpoint must not crash us
What the developer wrote
Authored by cdecker
pytest: channels reusing a funding outpoint must not crash us
(cherry picked from commit bc01ae602ced2a5c46312900e6cea02c617ecd69)
What changed, and why it matters
This commit adds a test showing that Core Lightning can crash if a node's database ends up with two channels sharing the same internal channel ID—one already closed and another later closed with the same ID. The test is currently marked as expected to fail, meaning the underlying crash bug has not been fixed yet. It appears to be a denial-of-service issue: a malformed or manipulated local database state causes lightningd to crash on channel close and then again on every restart.
Treat this as a known unfixed bug. A developer should add a database-level constraint or application-level deduplication/renaming logic so that closed channels with duplicate channel_ids do not crash the daemon on close or startup. Users should avoid manually manipulating the database; operators who suspect duplicate channel_ids should seek a patched release before restarting.
Security signals we found
Denial-of-service via local database state corruption
Daemon crash on startup after duplicate channel_id enters closed channels
Database uniqueness/lookup assumption violation on channel_id
Test-only commit; no production code fix included
Evidence from the diff
The new pytest test_duplicate_channel_id_in_db simulates a scenario where two channels share the same full_channel_id in l2’s SQLite database. The first channel is canceled/discarded and ends up in closed channels. A second channel, awaiting lockin, has its full_channel_id manually overwritten via db_manip to match the first. When dev_forget_channel closes the second, l2’s database contains two closed channels with identical channel_ids, which currently crashes lightningd both at close time and on subsequent startup. The test is decorated with @pytest.mark.xfail(strict=True), indicating the bug is reproduced but not yet fixed by this commit.
Changed components
tests/test_opening.pylightningd channel database handlingclosed-channel startup/lookup logicInspect captured patch +44 / −0
### tests/test_opening.py
@@ -4183,3 +4183,47 @@ def test_v2_lockin_single_spend_watch(node_factory, bitcoind):
state_change_re = re.compile(r'State changed from \S+ to FUNDING_SPEND_SEEN')
for node in (l1, l2):
assert len([l for l in node.daemon.logs if state_change_re.search(l)]) == 1
+
+
+@pytest.mark.xfail(strict=True, reason="duplicate channel_ids in the db crash lightningd on close and on startup")
+@pytest.mark.openchannel('v1')
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Uses db_manip on sqlite3")
+def test_duplicate_channel_id_in_db(node_factory, bitcoind):
+ """Nodes which already committed two channels with the same channel_id
+ crashed when the second one was closed, and then on every restart."""
+ l1, l2 = node_factory.get_nodes(2, opts=[{}, {'may_fail': True, 'broken_log': 'lightningd: (FATAL SIGNAL|backtrace)'}])
+ l1.fundwallet(10**7)
+ l1.connect(l2)
+ amount = 10**6
+
+ # A channel l2 forgets, so it's in the closed channels.
+ funding_addr = l1.rpc.fundchannel_start(l2.info['id'], amount)['funding_address']
+ prep = l1.rpc.txprepare([{funding_addr: amount}])
+ l1.rpc.fundchannel_complete(l2.info['id'], prep['psbt'])
+ cid = only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['channel_id']
+ l1.rpc.fundchannel_cancel(l2.info['id'])
+ wait_for(lambda: l2.rpc.listpeerchannels(l1.info['id'])['channels'] == [])
+ l1.rpc.txdiscard(prep['txid'])
+
+ # Another one, still awaiting lockin.
+ funding_addr = l1.rpc.fundchannel_start(l2.info['id'], amount)['funding_address']
+ prep = l1.rpc.txprepare([{funding_addr: amount}])
+ l1.rpc.fundchannel_complete(l2.info['id'], prep['psbt'])
+ cid2 = only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['channel_id']
+
+ # Give the live channel the closed channel's channel_id, as old nodes
+ # may have done.
+ l1.stop()
+ l2.stop()
+ l2.db_manip(f"UPDATE channels SET full_channel_id = X'{cid}' WHERE full_channel_id = X'{cid2}';")
+ l2.start()
+ assert only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['channel_id'] == cid
+
+ # Closing it puts a second channel with that channel_id into closed channels.
+ l2.rpc.dev_forget_channel(l1.info['id'], True)
+ wait_for(lambda: l2.rpc.listpeerchannels(l1.info['id'])['channels'] == [])
+ assert [c['channel_id'] for c in l2.rpc.listclosedchannels()['closedchannels']] == [cid, cid]
+
+ # And we can still start with both in the db.
+ l2.restart()
+ assert [c['channel_id'] for c in l2.rpc.listclosedchannels()['closedchannels']] == [cid, cid]Why this scored 50/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.