chaintopology: fix infinite RBF loop after replacement tx is confirmed

[enaples]

Important

26.04 FREEZE March 11th: Non-bugfix PRs not ready by this date will wait for 26.06.

RC1 is scheduled on March 23rd

The final release is scheduled for April 15th.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

• The changelog has been updated in the relevant commit(s) according to the guidelines.
• Tests have been added or modified to reflect the changes.
• Documentation has been reviewed and updated as needed.
• Related issues have been listed and linked, including any that this PR closes.
• Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade

Problem

This issue raised twice. Last time has been raised on #8921.
rebroadcast_txs() in lightningd/chaintopology.c iterates over all pending outgoing transactions once per block and, for each one, asks: "has this tx already been confirmed?" before deciding whether to rebroadcast or RBF it. The confirmation check is:

if (wallet_transaction_height(topo->ld->wallet, &otx->txid))
    continue;

otx->txid is initialised from the tx that was originally broadcast and is also the key of the outgoing_tx_map hash table. It is never updated after a replacement (RBF) transaction is created.

When otx->refresh (i.e. consider_onchain_rebroadcast()) is called, it may produce a higher-fee replacement and write it into otx->tx. The replacement has a different txid, but otx->txid still holds the original. The original was never mined (only the replacement was) so wallet_transaction_height(original_txid) returns 0 forever, the continue
is never taken, and consider_onchain_rebroadcast is invoked on every subsequent block even though the output is already resolved.

Test scenario rationale: Why penalty transactions trigger this and to-local sweeps do not

The bug requires actual fee escalation to be observable: consider_onchain_rebroadcast only emits an INFO log and produces a new tx when newfee > info->fee. Fee escalation happens when feerate_for_target(deadline) returns an increasing value.

• Penalty tx — deadline is close_blockheight + to_self_delay. With watchtime-blocks = 10 the deadline is only 10 blocks away at close, so feerate_for_target returns a higher value on each new block.
• To-local sweep — deadline is slow_sweep_deadline (~300 blocks, hardcoded). feerate_for_target returns FEERATE_FLOOR until the sweep is imminent, so fees never change, no new tx is produced, and the bug is
  invisible.

Why mutating otx->txid directly would crash

The first attempted fix (bitcoin_txid(otx->tx, &otx->txid) after the refresh call) caused a SIGABRT / SIGSEGV on shutdown. otx->txid is the key of the outgoing_tx_map htable (keyof_outgoing_tx_map returns &t->txid). Changing the key in-place while the entry lives in the table corrupts its internal bucket chain, producing a use-after-free when the topology is torn down in destroy_chain_topology.

Suggested Fix

Instead of storing a separate up-to-date txid (which would require touching the map key or adding a new field), we compute the txid of the current otx->tx on the fly at the top of the loop:

struct bitcoin_txid cur_txid;
bitcoin_txid(otx->tx, &cur_txid);
if (wallet_transaction_height(topo->ld->wallet, &cur_txid))
    continue;

otx->tx is always the latest version of the transaction: it is set to the original tx at broadcast time and updated by each successful refresh() call. So bitcoin_txid(otx->tx) naturally tracks whichever version was most recently
broadcast (including any RBF replacement) without touching the map key.

otx->txid is left completely unchanged.

The SHA-256d required by bitcoin_txid is negligible compared with the round-trip to bitcoind that follows immediately; the cost is not a concern.