From 02412e5c4dc24e3697a4a17d77c6a2de5f5865c7 Mon Sep 17 00:00:00 2001 From: enaples Date: Tue, 10 Mar 2026 14:05:18 +0100 Subject: [PATCH 1/2] tests: test RBF stops after closed tx is confirmed --- tests/test_closing.py | 71 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/test_closing.py b/tests/test_closing.py index 5c4a17acc5f4..e12084b97d00 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1727,6 +1727,77 @@ def censoring_sendrawtx(r): check_utxos_channel(l2, [channel_id], expected_2) +def test_onchain_rbf_stops_after_confirmation(node_factory, bitcoind): + """Penalty tx RBF stops once the replacement tx is confirmed.""" + + to_self_delay = 10 + opts = {'watchtime-blocks': to_self_delay, 'dev-force-features': "-23"} + + # l1 is the thief; allow it to fail. + l1 = node_factory.get_node(options=opts, may_fail=True) + l2 = node_factory.get_node(options=opts) + + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1.fundchannel(l2, 10**7) + + # Save l1's current commitment before it is revoked. + theft_tx = l1.rpc.dev_sign_last_tx(l2.info['id'])['tx'] + + # Advance the commitment state — the saved commitment is now revoked. + l1.pay(l2, 1000000) + l1.rpc.stop() + + # Censor l2 so the penalty tx never reaches miners. + def censoring_sendrawtx(r): + return {'id': r['id'], 'result': {}} + l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', censoring_sendrawtx) + + # l1 broadcasts the revoked commitment. + bitcoind.rpc.sendrawtransaction(theft_tx) + bitcoind.generate_block(1) + l2.daemon.wait_for_log(' to ONCHAIN') + + _, txid, blocks = l2.wait_for_onchaind_tx('OUR_PENALTY_TX', + 'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM') + assert blocks == 0 # penalty tx is immediately broadcastable + + # Each block brings the deadline (close_blockheight + to_self_delay) closer + # so feerate_for_target() returns a higher fee and onchaind emits INFO-level "RBF onchain txid". + for _ in range(3): + bitcoind.generate_block(1) + l2.daemon.wait_for_log('RBF onchain txid') + + # Stop censoring. Generate a block to trigger rebroadcast (the penalty tx + # enters bitcoind's mempool) but filter it out so the next block mines it. + l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', None) + bitcoind.generate_block(1, needfeerate=10000000) + l2.daemon.wait_for_log('RBF onchain txid') + + # Mine the penalty tx (single output, no in-flight HTLCs). + bitcoind.generate_block(1, wait_for_mempool=1) + sync_blockheight(bitcoind, [l2]) + + l2.daemon.wait_for_log('Resolved THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM' + ' by our proposal OUR_PENALTY_TX') + + # Record log position right after confirmation. + log_pos = l2.daemon.logsearch_start + + # Bump feerate: any spurious rebroadcast would compute a higher fee + # (newfee > info->fee) and emit "RBF onchain txid" at INFO level. + # Without the fix: wallet_transaction_height(original_txid) returns 0 + # because the stale original txid was never mined (only the replacement + # was), so consider_onchain_rebroadcast keeps firing indefinitely. + l2.set_feerates([10000] * 4, False) + + # rebroadcast_txs() fires on every new block (chaintopology.c ~line 854). + bitcoind.generate_block(2) + sync_blockheight(bitcoind, [l2]) + + assert not l2.daemon.is_in_log('RBF onchain txid', start=log_pos), \ + "node kept RBF-ing penalty tx after the replacement was confirmed" + + def test_onchain_first_commit(node_factory, bitcoind): """Onchain handling where opener immediately drops to chain""" From d0ae1b9fa6161cda4ce94267dcc83808fbdbadc8 Mon Sep 17 00:00:00 2001 From: enaples Date: Tue, 10 Mar 2026 14:24:43 +0100 Subject: [PATCH 2/2] chaintopology: fix RBF loop that never stops after replacement tx confirms After consider_onchain_rebroadcast() creates a higher-fee replacement, rebroadcast_txs() calls refresh() which updates otx->tx in-place, but the confirmation guard still queries the original otx->txid (the map key): if (wallet_transaction_height(topo->ld->wallet, &otx->txid)) continue; Because the original tx was never mined (only the replacement was), wallet_transaction_height always returns 0 and the RBF loop fires on every subsequent block forever, even after the channel is fully resolved. Fix: compute cur_txid from the current otx->tx before the guard. This naturally reflects any replacement made by a prior refresh() call, so wallet_transaction_height finds the confirmed txid and skips the entry. otx->txid (the hash-map key) is intentionally left unchanged: mutating the key in-place while the entry lives in the map would corrupt the table. Co-Authored-By: Claude Sonnet 4.6 --- lightningd/chaintopology.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 6ab4c659706b..3152cb3224b6 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -166,8 +166,13 @@ static void rebroadcast_txs(struct chain_topology *topo) for (otx = outgoing_tx_map_first(topo->outgoing_txs, &it); otx; otx = outgoing_tx_map_next(topo->outgoing_txs, &it)) { struct tx_rebroadcast *txrb; - /* Already sent? */ - if (wallet_transaction_height(topo->ld->wallet, &otx->txid)) + struct bitcoin_txid cur_txid; + + /* Already confirmed? Use the txid of the current tx, not the + * original otx->txid: refresh() may have replaced otx->tx with + * a higher-fee version whose txid differs from the map key. */ + bitcoin_txid(otx->tx, &cur_txid); + if (wallet_transaction_height(topo->ld->wallet, &cur_txid)) continue; /* Don't send ones which aren't ready yet. Note that if the