[RFC] Add ChannelMonitor::get_justice_txs for simplified watchtower integration

[FreeOnlineUser]

Draft PR for design feedback. Implements the approach @TheBlueMatt suggested in lightningdevkit/ldk-node#813 and in review of #2552: move justice tx state tracking inside ChannelMonitor so Persist implementors don't need external queues.

What this does

Adds a single method that replaces the current 3-step dance of counterparty_commitment_txs_from_update() + manual queue + sign_to_local_justice_tx():

pub struct JusticeTransaction {
    pub tx: Transaction,
    pub revoked_commitment_txid: Txid,
    pub commitment_number: u64,
}

impl ChannelMonitor {
    pub fn get_justice_txs(
        &self,
        feerate_per_kw: u64,
        destination_script: ScriptBuf,
    ) -> Vec<JusticeTransaction>;
}

Internally, ChannelMonitorImpl stores recent counterparty CommitmentTransactions (populated during update application, pruned to entries within one revocation of current). get_justice_txs checks which have revocation secrets available, builds and signs the justice tx, and returns the result.

Changes

• channelmonitor.rs: new JusticeTransaction struct, latest_counterparty_commitment_txs field, TLV 39 serialization (optional, backwards-compatible), get_justice_txs() method
• test_utils.rs: simplified WatchtowerPersister (removed JusticeTxData, unsigned_justice_tx_data queue, form_justice_data_from_commitment)
• 127 additions, 83 deletions. All existing tests pass.

Splice handling

Pruning uses commitment numbers, not entry count. During a splice, multiple entries share the same commitment number (one per funding scope) and all are retained.

Backwards compatibility

• TLV 39 is optional. Old monitors load with empty vec, populated on next update.
• Old nodes reading monitors written by new code skip the unknown TLV field.
• Existing counterparty_commitment_txs_from_update() and sign_to_local_justice_tx() APIs unchanged.

Design questions

Looking for input on these before moving out of draft:

1. Signed vs unsigned return? Currently returns fully signed transactions. Returning unsigned gives callers more flexibility on fee choice at broadcast time. Preference?

2. HTLC outputs? This only covers to_local justice. HTLC-timeout and HTLC-success outputs on revoked commitments are not included. Worth adding here, or separate follow-up?

3. Feerate source? Caller provides feerate_per_kw. An alternative would be accepting a fee estimator. Any preference?

4. Dust filtering? If the revokeable output is dust, revokeable_output_index() returns None and the entry is skipped silently. Right behavior, or surface this to the caller?

[ldk-reviews-bot]

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

[codecov]

Codecov Report

❌ Patch coverage is 92.18750% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.91%. Comparing base (ec03159) to head (d6c7903).

Files with missing lines	Patch %	Lines
lightning/src/util/test_utils.rs	80.00%	2 Missing and 1 partial ⚠️
lightning/src/chain/channelmonitor.rs	95.91%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4453      +/-   ##
==========================================
- Coverage   85.93%   85.91%   -0.03%     
==========================================
  Files         159      159              
  Lines      104693   104694       +1     
  Branches   104693   104694       +1     
==========================================
- Hits        89972    89948      -24     
- Misses      12213    12240      +27     
+ Partials     2508     2506       -2     

Flag	Coverage Δ
tests	85.91% <92.18%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

This should be in the funding scope to support splicing. Also should probably just be cur_... and prev_... to match existing API pattern rather than a vec that is max-size 2.

[FreeOnlineUser]

Moved to FundingScope as cur/prev fields. TLV 13/15 on FundingScope, 39/41 on the outer scope for backwards compat.

[TheBlueMatt]

Claude loves to leave comments about how it changed the code that are totally useless - reading the code I don't care what changes claude made in the past, I care if there's something useful to know now. Probably can just drop this.

[FreeOnlineUser]

Removed, and noted for future.

[TheBlueMatt]

This API doesn't really make sense. There's no information on when I should call this or when the monitor "knows about" a revoked tx. We probably want to do soemthing like the old API where you can fetch revoked transactions in relation to a specific monitor update.

[FreeOnlineUser]

Replaced with sign_initial_justice_tx() for persist_new_channel and sign_justice_txs_from_update(update) for update_persisted_channel. Each is tied to a specific point in the persistence pipeline.