Skip to content

Derive per-channel destination and shutdown scripts for improved on-chain privacy#4756

Open
Abeeujah wants to merge 4 commits into
lightningdevkit:mainfrom
Abeeujah:key-script
Open

Derive per-channel destination and shutdown scripts for improved on-chain privacy#4756
Abeeujah wants to merge 4 commits into
lightningdevkit:mainfrom
Abeeujah:key-script

Conversation

@Abeeujah

Copy link
Copy Markdown
Contributor

Generate unique per-channel destination and shutdown scripts when V2 remote key derivation is enabled. Previously,
KeysManager used a single global destination_script and shutdown_pubkey for all channels, causing address reuse
across channels.

Changes:

  1. 04386c1 Derive per-channel destination/shutdown scripts from channel_keys_id.
  2. d429646 get_shutdown_script_pubkey now takes channel_keys_id to produce unique cooperative-close scripts per channel.
  3. 82e54e3 Add comprehensive tests verifying scripts are per-channel, distinct, non-legacy, and recoverable.
  4. e458aad Add changelog entry.

Funds sent to per-channel scripts remain recoverable from seed alone via possible_v2_static_output_spks. A new
find_static_output_key method supports spending both legacy (global) and per-channel keys.

closes #1139

@ldk-reviews-bot

ldk-reviews-bot commented Jun 27, 2026

Copy link
Copy Markdown

I've assigned @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Comment thread lightning/src/sign/mod.rs Outdated
Comment thread lightning/src/sign/mod.rs Outdated
Comment thread lightning/src/sign/mod.rs Outdated
Comment thread pending_changelog/1139.txt Outdated
@ldk-claude-review-bot

ldk-claude-review-bot commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

No new issues found.

This is the same commit (0170743…) as my previous review pass, and I re-verified the new logic in depth:

  • Backwards compatibility is correct: destination_key/shutdown_key are derived at m/1' and m/2' (DESTINATION_SCRIPT_INDEX/SHUTDOWN_PUBKEY_INDEX), the same indices the old spend_spendable_outputs path used via a fresh Xpriv::new_master(Network::Testnet, seed), so previously-created StaticOutputs remain spendable after upgrade.
  • find_static_output_key checks legacy roots first, then per-channel children — covers v1 channels, v2 channels (with channel_keys_id), and seed-only recovery (full scan).
  • static_output_key_idx distinct-index guarantee holds: counter is channel_keys_id[0..4] (incrementing) and the offset hashes [4..16] (constant starting-time bytes within an instance).
  • create_chanmon_cfgs_with_legacy_keys passes predefined_keys_ids.is_some() into the v1_derivation parameter — correct (Some ⇒ legacy V1).
  • use bitcoin::hex::FromHex in functional_tests.rs is the only such import (no duplicate).

Correction to my prior review: my earlier "compile failure" claim about fuzz/src/onion_message.rs:312 was a false positive — that file is included in this diff and is correctly updated to the new (&self, _channel_keys_id: [u8; 32]) signature. All SignerProvider implementors are updated.

The previously-posted inline comments still stand (they are unchanged in this commit):

  • lightning/src/sign/mod.rs:2027 — broken intra-doc link (space instead of _).
  • lightning/src/sign/mod.rs:2607 / :2423 — comment typos for possible_v2_static_output_spks.
  • pending_changelog/1139.txt:2 — wrong method name get_shutdown_script_pubkey.

Non-blocking note (carried over): for externally-supplied channel_keys_ids the per-channel index can collide, so script reuse is possible — acceptable per the documented behavior.

@ldk-reviews-bot ldk-reviews-bot requested a review from wpaulino June 27, 2026 18:37
Abeeujah added 4 commits June 27, 2026 19:39
Previously, `KeysManager` used a single global `destination_script`
and `shutdown_pubkey` for all channels. With V2 remote key derivation
enabled, each channel now gets a unique destination and
cooperative-close script derived from its `channel_keys_id`.

This improves on-chain privacy by avoiding script reuse across channels.
Funds sent to per-channel scripts remain recoverable from the seed alone
by scanning for scripts returned by `possible_v2_static_output_spks`.

A new `find_static_output_key` method added on `KeysManager` locates the
correct spending key for `StaticOutput` descriptors, supporting both
legacy (global) and per-channel keys.

The watchtower justice transaction test is updated to use legacy V1 keys
since per-channel destination scripts cannot be predicted before channel
creation.
`get_shutdown_scriptpubkey` now takes a `channel_keys_id` parameter,
allowing signers to derive a unique shutdown script per channel.

When `v2_remote_key_derivation` is enabled, `KeysManager` uses
`channel_keys_id` to derive a fresh per-channel cooperative-close
script from the shutdown key, avoiding address reuse across channels
and improving on-chain privacy.
Verify that v2 (static_remote_key) destination and shutdown scripts are
per-channel, distinct from each other, non-legacy, re-derivable across
restarts, and recoverable via chain scan. Also confirm that v1 scripts
remain static and that `find_static_output_key` correctly resolves
scripts under various scenarios.
@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.23656% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.97%. Comparing base (43ea836) to head (0170743).

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 70.00% 3 Missing ⚠️
lightning/src/sign/mod.rs 98.24% 2 Missing and 1 partial ⚠️
lightning/src/util/test_utils.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #4756    +/-   ##
========================================
  Coverage   86.96%   86.97%            
========================================
  Files         161      161            
  Lines      111648   111784   +136     
  Branches   111648   111784   +136     
========================================
+ Hits        97099    97224   +125     
- Misses      12045    12053     +8     
- Partials     2504     2507     +3     
Flag Coverage Δ
fuzzing-fake-hashes 8.43% <14.03%> (-0.01%) ⬇️
fuzzing-real-hashes 32.38% <3.50%> (-0.04%) ⬇️
tests 86.30% <96.23%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create fresh destination/shutdown scripts for each channel in KeysManager

3 participants