dual-fund/splice: mark superseded RBF/splice inflight candidates, stop recomputing their commitments

.msggen.json

@@ -3391,6 +3391,7 @@
             "ListPeerChannels.channels[].inflight[].our_funding_msat": 5,
             "ListPeerChannels.channels[].inflight[].scratch_txid": 6,
             "ListPeerChannels.channels[].inflight[].splice_amount": 7,
+            "ListPeerChannels.channels[].inflight[].superseded": 8,
             "ListPeerChannels.channels[].inflight[].total_funding_msat": 4
         },
         "ListpeerchannelsChannelsStateChanges": {
@@ -12024,6 +12025,10 @@
             "added": "v23.08",
             "deprecated": null
         },
+        "ListPeerChannels.channels[].inflight[].superseded": {
+            "added": "v26.09",
+            "deprecated": null
+        },
         "ListPeerChannels.channels[].inflight[].total_funding_msat": {
             "added": "v23.02",
             "deprecated": null

contrib/msggen/msggen/schema.json

@@ -27009,7 +27009,8 @@
                       "feerate",
                       "total_funding_msat",
                       "splice_amount",
-                      "our_funding_msat"
+                      "our_funding_msat",
+                      "superseded"
                     ],
                     "properties": {
                       "funding_txid": {
@@ -27054,6 +27055,13 @@
                         "description": [
                           "The commitment transaction txid we would use if we went onchain now."
                         ]
+                      },
+                      "superseded": {
+                        "type": "boolean",
+                        "added": "v26.09",
+                        "description": [
+                          "Whether this candidate has provably lost: a sibling RBF/splice candidate for this channel has already been confirmed, so this one will never be used and its commitment is no longer being updated."
+                        ]
                       }
                     }
                   }

contrib/pyln-testing/pyln/testing/grpc2py.py

@@ -956,6 +956,7 @@ def listpeerchannels_channels_inflight2py(m):
         "our_funding_msat": amount2msat(m.our_funding_msat),  # PrimitiveField in generate_composite
         "scratch_txid": hexlify(m.scratch_txid),  # PrimitiveField in generate_composite
         "splice_amount": m.splice_amount,  # PrimitiveField in generate_composite
+        "superseded": m.superseded,  # PrimitiveField in generate_composite
         "total_funding_msat": amount2msat(m.total_funding_msat),  # PrimitiveField in generate_composite
     })
 

doc/schemas/listpeerchannels.json

@@ -382,7 +382,8 @@
                   "feerate",
                   "total_funding_msat",
                   "splice_amount",
-                  "our_funding_msat"
+                  "our_funding_msat",
+                  "superseded"
                 ],
                 "properties": {
                   "funding_txid": {
@@ -427,6 +428,13 @@
                     "description": [
                       "The commitment transaction txid we would use if we went onchain now."
                     ]
+                  },
+                  "superseded": {
+                    "type": "boolean",
+                    "added": "v26.09",
+                    "description": [
+                      "Whether this candidate has provably lost - a sibling RBF/splice candidate for this channel has already been confirmed, so this one will never be used and its commitment is no longer being updated"
+                    ]
                   }
                 }
               }

lightningd/channel.c

@@ -223,13 +223,28 @@ new_inflight(struct channel *channel,
 	inflight->locked_scid = NULL;
 	inflight->i_sent_sigs = i_sent_sigs;
 	inflight->splice_locked_memonly = false;
+	inflight->superseded = false;
 
 	list_add_tail(&channel->inflights, &inflight->list);
 	tal_add_destructor(inflight, destroy_inflight);
 
 	return inflight;
 }
 
+void channel_mark_inflights_superseded(struct lightningd *ld,
+				       struct channel *channel,
+				       const struct channel_inflight *winner)
+{
+	struct channel_inflight *inflight;
+
+	list_for_each(&channel->inflights, inflight, list) {
+		if (inflight == winner || inflight->superseded)
+			continue;
+		inflight->superseded = true;
+		wallet_inflight_save(ld->wallet, inflight);
+	}
+}
+
 void inflight_set_last_tx(struct channel_inflight *inflight,
 		          struct bitcoin_tx *last_tx,
 		          const struct bitcoin_signature last_sig)

lightningd/channel.h

@@ -103,6 +103,13 @@ struct channel_inflight {
 	 * moves the responsiblity of cleaning up the inflight to the watcher,
 	 * avoiding any potential race conditions. */
 	bool splice_locked_memonly;
+
+	/* Once a sibling RBF/splice candidate for this channel has been
+	 * confirmed as the winner, every other inflight is provably never
+	 * going to be used - so we mark them so we can stop recomputing and
+	 * persisting a fresh commitment tx + HTLC sigs for them on every
+	 * commitment update */
+	bool superseded;
 };
 
 struct open_attempt {
@@ -474,6 +481,10 @@ struct channel_inflight *new_inflight(struct channel *channel,
 	     bool force_sign_first,
 	     bool i_sent_sigs);
 
+void channel_mark_inflights_superseded(struct lightningd *ld,
+				       struct channel *channel,
+				       const struct channel_inflight *winner);
+
 struct channel_state_change *new_channel_state_change(const tal_t *ctx,
 						      struct timeabs timestamp,
 						      enum channel_state old_state,

lightningd/dual_open_control.c

@@ -1042,10 +1042,14 @@ static void dual_funding_found(struct lightningd *ld,
 	 * anyone consulting the channel before we finish catching up
 	 * with the chain (e.g. a reconnecting peer) sees the mined
 	 * funding tx, not the latest RBF attempt. */
-	if (inflight->channel->state == DUALOPEND_AWAITING_LOCKIN)
+	if (inflight->channel->state == DUALOPEND_AWAITING_LOCKIN) {
 		update_channel_from_inflight(ld, inflight->channel,
 					     inflight, false);
 
+		channel_mark_inflights_superseded(ld, inflight->channel,
+						  inflight);
+	}
+
 	/* Otherwise, watch for block depth increases (we'll immediately expect one) */
 	watch_blockdepth(inflight, ld->topology, loc->blkheight,
 			 opening_depth_cb,

lightningd/peer_control.c

@@ -1099,6 +1099,7 @@ static void NON_NULL_ARGS(1, 2, 4, 5) json_add_channel(struct command *cmd,
 			json_add_s64(response,
 				     "splice_amount",
 				     inflight->funding->splice_amnt);
+			json_add_bool(response, "superseded", inflight->superseded);
 			/* Add the expected commitment tx id also */
 			if (inflight->last_tx) {
 				bitcoin_txid(inflight->last_tx, &txid);

lightningd/peer_htlcs.c

@@ -2529,6 +2529,10 @@ void peer_got_commitsig(struct channel *channel, const u8 *msg)
 			continue;
 
 		commit = inflight_commit_sigs[i];
+		i++;
+
+		if (inflight->superseded)
+			continue;
 
 		tal_free(inflight->last_tx);
 		inflight->last_tx = clone_bitcoin_tx(inflight, commit->tx);
@@ -2539,7 +2543,6 @@ void peer_got_commitsig(struct channel *channel, const u8 *msg)
 		wallet_htlc_sigs_add(ld->wallet, channel->dbid,
 				     inflight->funding->outpoint,
 				     commit->htlc_signatures);
-		i++;
 	}
 
 	/* Tell it we've committed, and to go ahead with revoke. */

tests/test_opening.py

@@ -1572,6 +1572,13 @@ def stalling_getblockhash(r):
     wait_for(lambda: 'short_channel_id'
              in only_one(l1.rpc.listpeerchannels()['channels']))
 
+    post_race_inflights = only_one(l1.rpc.listpeerchannels()['channels'])['inflight']
+    for inflight in post_race_inflights:
+        if inflight['funding_txid'] == inflights[1]['funding_txid']:
+            assert inflight['superseded'] is False
+        else:
+            assert inflight['superseded'] is True
+
     # l2 comes back fully synced, and reconnects to wedged l1.
     l2.start()
     sync_blockheight(bitcoind, [l2])

tests/test_splicing.py

@@ -1,6 +1,7 @@
 from fixtures import *  # noqa: F401,F403
 from pyln.client import RpcError
 import pytest
+import threading
 import unittest
 import time
 from utils import (
@@ -180,6 +181,102 @@ def test_splice_rbf(node_factory, bitcoind):
     assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0
 
 
+@pytest.mark.openchannel('v1')
+@pytest.mark.openchannel('v2')
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
+def test_splice_reconnect_non_last_mined(node_factory, bitcoind, chainparams):
+    l1, l2 = node_factory.line_graph(2, fundamount=1000000, wait_for_announce=True,
+                                     opts={'allow_warning': True, 'may_reconnect': True})
+
+    chan_id = l1.get_channel_id(l2)
+
+    # First splice candidate - goes to mempool, becomes the tip
+    funds_result = l1.rpc.addpsbtoutput(100000)
+    result = l1.rpc.splice_init(chan_id, -105801, funds_result['psbt'])
+    result = l1.rpc.splice_update(chan_id, result['psbt'])
+    assert result['commitments_secured'] is False
+    result = l1.rpc.splice_update(chan_id, result['psbt'])
+    assert result['commitments_secured'] is True
+    result = l1.rpc.splice_signed(chan_id, result['psbt'])
+
+    l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
+    l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
+
+    wait_for(lambda: len(list(bitcoind.rpc.getrawmempool(True).keys())) == 1)
+    first_txid = result['txid']
+    assert first_txid in bitcoind.rpc.getrawmempool()
+
+    # Censor broadcast so the RBF bump below never reaches the mempool - it
+    # exists only as an inflight, and never replaces the first tx there
+    def censoring_sendrawtx(r):
+        return {'id': r['id'], 'result': {}}
+
+    l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx)
+    l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx)
+
+    # RBF-bump the pending splice - a 2nd (latest) inflight that will never
+    # make it into the mempool
+    funds_result = l1.rpc.addpsbtoutput(100000)
+    result = l1.rpc.splice_init(chan_id, -111722, funds_result['psbt'])
+    result = l1.rpc.splice_update(chan_id, result['psbt'])
+    assert result['commitments_secured'] is False
+    result = l1.rpc.splice_update(chan_id, result['psbt'])
+    assert result['commitments_secured'] is True
+    last = len(l1.daemon.logs)
+    result = l1.rpc.splice_signed(chan_id, result['psbt'])
+
+    wait_for(lambda: l1.daemon.is_in_log("plugin-bcli: sendrawtx exit 0", start=last))
+    time.sleep(.05)
+
+    l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', None)
+    l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', None)
+
+    # The 1st tx is still the only one in the mempool - the RBF bump never made it there
+    assert list(bitcoind.rpc.getrawmempool(True).keys()) == [first_txid]
+
+    # l2 goes offline
+    l2.stop()
+
+    # Stall l1 fetch of the block *after* the funding block, so it sees
+    # the funding confirm but never finishes catching up
+    height = bitcoind.rpc.getblockcount()
+    release_block = threading.Event()
+
+    def stalling_getblockhash(r):
+        if r['params'][0] == height + 2:
+            release_block.wait(timeout=180)
+        return None
+
+    l1.daemon.rpcproxy.mock_rpc('getblockhash', stalling_getblockhash)
+
+    # The 1st (non-latest) inflight gets mined
+    bitcoind.generate_block(2, wait_for_mempool=1)
+
+    # l2 comes back fully synced, and reconnects to wedged l1
+    l2.start()
+    sync_blockheight(bitcoind, [l2])
+    l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
+
+    l2.daemon.wait_for_log(r'Peer has reconnected, state CHANNELD_AWAITING_SPLICE')
+    bitcoind.generate_block(1)
+    sync_blockheight(bitcoind, [l2])
+
+    l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL', timeout=120)
+    l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL', timeout=120)
+
+    # Let l1 finish catching up before we look at the result
+    release_block.set()
+    l1.daemon.rpcproxy.mock_rpc('getblockhash', None)
+    sync_blockheight(bitcoind, [l1])
+
+    # The mined inflight (the 1st) must be the one locked in - on both
+    # sides, and with matching funding
+    channel = only_one(l1.rpc.listpeerchannels()['channels'])
+    assert channel['funding_txid'] == first_txid
+    l2_channel = only_one(l2.rpc.listpeerchannels()['channels'])
+    assert l2_channel['funding_txid'] == first_txid
+
+
 @pytest.mark.openchannel('v1')
 @pytest.mark.openchannel('v2')
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')

wallet/migrations.c

@@ -1186,6 +1186,11 @@ static const struct db_migration dbmigrations[] = {
      * after the failure was recorded (issue #9341). */
     {SQL("ALTER TABLE payments ADD failmsg BLOB;"), NULL,
      SQL("ALTER TABLE payments DROP COLUMN failmsg"), NULL},
+    /* Marks an RBF/splice candidate that's provably lost (a sibling
+     * inflight was confirmed instead), so we stop recomputing its
+     * commitment tx + HTLC sigs on every update */
+    {SQL("ALTER TABLE channel_funding_inflights ADD superseded INTEGER DEFAULT 0;"), NULL,
+     SQL("ALTER TABLE channel_funding_inflights DROP COLUMN superseded"), NULL},
     /* ^v26.09 */
 };
 

wallet/test/run-wallet.c

@@ -1337,6 +1337,7 @@ static bool channel_inflightseq(struct channel_inflight *i1,
 		CHECK(memeq(i1->locked_scid, sizeof(*i1->locked_scid),
 			    i2->locked_scid, sizeof(*i2->locked_scid)));
 	CHECK(i1->splice_locked_memonly == i2->splice_locked_memonly);
+	CHECK(i1->superseded == i2->superseded);
 
 	return true;
 }
@@ -1767,6 +1768,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx,
 				false,
 				false);
 	inflight->splice_locked_memonly = true;
+	inflight->superseded = true;
 	inflight->locked_scid = tal(inflight, struct short_channel_id);
 	memset(inflight->locked_scid, 7, sizeof(struct short_channel_id));
 
@@ -1803,6 +1805,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx,
 				false,
 				false);
 	inflight->splice_locked_memonly = false;
+	inflight->superseded = false;
 	inflight->locked_scid = NULL;
 	inflight_set_last_tx(inflight, last_tx, sig);
 	wallet_inflight_add(w, inflight);