Skip to content

[0.1] Initial round of backports for 0.1.10#4680

Open
TheBlueMatt wants to merge 17 commits into
lightningdevkit:0.1from
TheBlueMatt:2026-05-0.1-backports
Open

[0.1] Initial round of backports for 0.1.10#4680
TheBlueMatt wants to merge 17 commits into
lightningdevkit:0.1from
TheBlueMatt:2026-05-0.1-backports

Conversation

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

There will obviously be a chunk more to go, but this is the first chunk of things already merged. Includes #3947 (as a prereq for #4377), #4377, #4520, #4537, #4544, #4558, #4590, #4591, #4593, #4605, #4595, #4596, #4618, #4647, and #4651.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 11, 2026

Copy link
Copy Markdown

I've assigned @joostjager 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.

@ldk-claude-review-bot

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

Copy link
Copy Markdown
Collaborator

I've now examined the portions of the diff that were truncated in my prior pass — the channelmanager.rs/outbound_payment.rs fee_paid_msat abandoned-case fix and the chainmonitor.rs get_partition_key change. Both are correct:

  • The Abandoned variant now carries pending_fee_msat (TLV key 5, option), preserved from Retryable on mark_abandoned, and read back via get_pending_fee_msat() when a late HTLC succeeds. Serialization is backward-compatible (older versions read None).
  • get_partition_key now returns Option<u32>; the single call site correctly uses is_some_and, intentionally skipping partition-based syncs when best_height is None.

No issues found.

All substantive changes in this backport set are correct. Verified during this pass:

  • outbound_payment.rsfee_paid_msat preserved across the abandoned→fulfilled transition; backward-compatible TLV serialization.
  • chainmonitor.rsget_partition_key Option refactor is sound (single closure, single call site).
  • composite_custom_message_handler, DNS resolver counter, fs_store artifact skipping, electrum signed-height check, and esplora Merkle-root validation all confirmed correct in the prior pass.

Cross-cutting (operational, not a code bug, unchanged from prior note): .github/workflows/check_unicode.yml calls gh issue create on every daily run where the table is stale, with no de-duplication, which can spawn duplicate issues until the table is refreshed.

@joostjager joostjager left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've assumed all conflicts were trivial aside from the first commit "Remove direct calls to handle_monitor_update_completion!". That commit, even though it is small, seems to have quite a few subtleties. Might want to ask one of the original reviewers of #3947 for a final look.

Comment thread lightning-dns-resolver/src/lib.rs
Comment thread lightning/src/ln/channelmanager.rs Outdated
Comment thread lightning/src/ln/channelmanager.rs Outdated
@ldk-reviews-bot

Copy link
Copy Markdown

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

TheBlueMatt and others added 10 commits June 11, 2026 11:27
`handle_monitor_update_completion!` (and
`handle_monitor_update_completion_actions`) are a pretty annoying
API as they have several preconditions. Luckily, we already have
`channel_monitor_update` which does the right work, handling both
the open- and closed- channel cases and correctly checking the
preconditions to `handle_monitor_update_completion!`, so here we
convert calls to `handle_monitor_update_completion!` (aside from
`handle_new_monitor_update!`) to just calling
`channelMonitor_update`.

Backport of 8b9b078

Nontrivial conflicts resolved in:
 * lightning/src/ln/channelmanager.rs due to the tracking of
   in-flight monitor updates having moved from `OutPoint`s to
   `ChannelId`s in 0.2. Also note that
   8f4a4d2 updated some code which
   was introduced in this commit and had already previously been
   backported to 0.1.
We previously assumed background events would eventually be processed
prior to another `ChannelManager` write, so we would immediately remove
all in-flight monitor updates that completed since the last
`ChannelManager` serialization. This isn't always the case, so we now
keep them all around until we're ready to handle them, i.e., when
`process_background_events` is called.

This was discovered while fuzzing `chanmon_consistency_target` on the
main branch with some changes that allow it to connect blocks. It was
triggered by reloading the `ChannelManager` after a monitor update
completion for an outgoing HTLC, calling
`ChannelManager::best_block_updated`, and reloading the `ChannelManager`
once again. A test is included that provides a minimal reproduction of
this case.

Backport of 7e84268

Conflicts resolved in:
 * lightning/src/ln/channelmanager.rs
 * lightning/src/ln/reload_tests.rs
Backport of f128b85

Conflicts resolved in:
 * lightning/src/ln/channelmanager.rs
When we make an MPP claim we push RAA blockers for each chanel to
ensure we don't allow any single channel to make too much progress
until all channels have the preimage durably on disk. We don't have
to store those RAA blockers on disk in the ChannelManager as
there's no point - if the ChannelManager gets to disk with the RAA
blockers it also brought with it the pending ChannelMonitorUpdates
that contain the preimages and will now be replayed, ensuring the
preimage makes it to all ChannelMonitors.

However, just because those RAA blockers dissapear on reload
doesn't mean the implications of them does too - if a later
ChannelMonitorUpdate was blocked in the channel we don't have logic
to unblock it on startup.

Here we add such logic, simply attempting to unblock all blocked
`ChannelMonitorUpdate`s that existed on startup.

Code written by Claude.

Fixes lightningdevkit#4518

Backport of b0c312d

Conflicts resolved in:
 * lightning/src/ln/channelmanager.rs
Add a characterization test for a claimed MPP payment whose
preimage monitor updates are only partially persisted before restart.
The test drives both channels through a held fee-update commitment
dance, claims with async monitor persistence, reloads one fresh and
one stale monitor, and verifies that we don't leave a sender-side
HTLC stuck after reconnect.

Backport of 01d55dc

Conflicts due to different channel sorting and holding cell free
timing resolved in:
 * lightning/src/ln/reload_tests.rs
In a very specific case, forgetting to do so can lead to a debug
assertion failure when we see a double-claim of an HTLC (see the
included test).

Found by @joostjager's work on growing the chanmon_consistency
fuzzer.

Backport of f14b4b2

Silent conflicts resolved in:
 * lightning/src/ln/functional_tests.rs
Backport of 6bf2352

Conflicts resolved in:
 * lightning/src/chain/chainmonitor.rs
Incomplete keysend MPPs skipped the receive timeout path,
allowing partial payments to hold HTLC slots until CLTV
expiry instead of failing after `MPP_TIMEOUT_TICKS`.
Apply the existing `total_mpp_amount_msat` completeness
check to all MPP receives and add a regression test
covering the keysend case.

The timeout logic was originally added only for
invoice-backed MPPs in 2022, and that invoice-only
guard remained when receive-side MPP keysend support
landed in 2023, leaving this gap latent until now.

Co-Authored-By: HAL 9000

Backport of fd8846b

Conflicts resolved in:
 * lightning/src/ln/channelmanager.rs
 * lightning/src/ln/payment_tests.rs
`GetHistoryRes::height` from electrum-client is a *signed* integer. Here
we first check for `<= 0` *before* casting to `u32`.

Signed-off-by: Elias Rohrer <dev@tnull.de>

Backport of 8b383bb
`OMDomainResolver` rate-limits in-flight DNSSEC proof builds via a
`pending_query_count` counter capped at `MAX_PENDING_RESPONSES` (1024).
The counter was only released when the proof build succeeded, so any
failure mode -- NXDOMAIN, insecure zones, unreachable resolvers, I/O
timeouts, malformed names -- permanently consumed a slot.

Because the queried name is attacker-controlled (it travels in over a
`DNSSECQuery` onion message from any LN peer, given DNS resolution is
an opt-in network-advertised feature), an adversary could exhaust the
counter with ~1025 failing queries and persistently DoS the resolver
for any subsequent legitimate BIP-353 lookups, until the process is
restarted.

Always release the slot once the proof build completes, regardless of
outcome, and add a regression test which points the resolver at a
TCP-refusing local port and asserts the counter returns to zero.

Co-Authored-By: HAL 9000

Backport of fb4103d

Conflicts resolved in:
 * lightning-dns-resolver/src/lib.rs
tnull and others added 7 commits June 11, 2026 11:27
`PrintableString` is the sanitiser LDK uses to render untrusted strings
(node aliases, BOLT-12 invoice / offer text, `UntrustedString`, LSPS
messages, `lightning-invoice` descriptions) to logs and UI. It only
replaced `char::is_control` matches (Unicode general category `Cc`)
with U+FFFD, leaving the entire `Cf` (Format) category untouched.

That is the exact category covering the bidirectional override /
isolate codepoints (U+202A..U+202E, U+2066..U+2069) and zero-width
characters (U+200B..U+200D, U+FEFF) behind the "Trojan Source" attack
family (CVE-2021-42574): a peer can set its alias / invoice description
/ offer fields to e.g. `safe\u{202E}cipsxe.exe`, which previously
passed through verbatim while a human reader sees `safeexe.cips` —
defeating the threat model `PrintableString` exists to defend against.

Replace `Cf` codepoints alongside `Cc` ones. The `Cf` ranges are
inlined as a `matches!` table sourced from Unicode 16.0 to keep the
change `no_std`-friendly with no new dependencies.

Co-Authored-By: HAL 9000
Signed-off-by: Elias Rohrer <dev@tnull.de>

Backport of 1a01b5a
1a01b5a added detection of unicode
format characters in `PrintableString`, but used a hard-coded table
which may eventually become out of date.

Here we switch to an auto-generated table, include all
`General_Category` `Other` characters, and also ban unallocated
code points.

Finally, CI validates that the file is kept up to date.

Written by Claude

Backport of 65e8cc8
`composite_custom_message_handler!` expanded `peer_connected` to call
every sub-handler and remember the last error, but never undo the
already-succeeded ones. The `CustomMessageHandler::peer_connected`
contract is that `PeerManager` will *not* invoke `peer_disconnected`
when `peer_connected` returns `Err` — so any per-peer state allocated
by an earlier sub-handler that returned `Ok` was leaked permanently
once a later sub-handler returned `Err`.

A peer who can elicit `Err` from any sub-handler in the composite
(feature-bit gate, banlist, etc.) could repeatedly reconnect to grow
that leaked state without bound (slow resource DoS), and "currently
connected" predicates in the leaking sub-handler would lie about
peers that were actually rejected.

Mirror the rollback pattern `PeerManager` already uses for the four
built-in handlers (`peer_handler.rs:2149-2188`): record each
sub-handler's `peer_connected` result, and if any returned `Err`,
call `peer_disconnected` on the ones that succeeded before
propagating the failure.

Co-Authored-By: HAL 9000
Signed-off-by: Elias Rohrer <dev@tnull.de>

Backport of 5455058

Silent conflicts resolved in:
 * lightning-custom-message/src/lib.rs
`EsploraSyncClient::get_confirmed_tx` parsed the SPV proof returned by
the Esplora server but threw away the security check: the merkle root
computed by `PartialMerkleTree::extract_matches` was discarded
(`let _ = …`), and only the leaf-equality check (`matches[0] == txid`)
remained. Anyone can construct a single-leaf partial tree advertising
an arbitrary txid via `PartialMerkleTree::from_txids(&[txid], &[true])`,
so this gate was vacuous.

A malicious or compromised Esplora server could therefore convince
`EsploraSyncClient` that any transaction was confirmed in any block by
returning `MerkleBlock { header: real_header, txn: forged_partial_tree }`,
causing LDK to feed a synthesized `ConfirmedTx` into `Confirm`
implementations such as `ChannelManager` / `ChainMonitor`. From there,
the channel-funding / closing / HTLC flows would treat the transaction
as confirmed at an attacker-chosen height, with consequences ranging
from premature state transitions to force-close races.

Capture the merkle root returned by `extract_matches` and require it
to equal `block_header.merkle_root`, matching the validation the
Electrum sibling already performs via `validate_merkle_proof`.

Co-Authored-By: HAL 9000

Backport of b64efcd
The exhaustive filesystem store listing treated leftover temp and trash
files as namespace directories after identifying them as non-keys.
Skip those artifacts before recursing so migrations can ignore crash
leftovers.

Backport of 9246d86

Conflicts resolved due to moved/split file in:
 * lightning-persister/src/fs_store.rs
lnd is preparing to ship a release with opt-in onion messages
without support for forwarding onion messages from non-channel
peers. This breaks the common BOLT 12 OM flow today where we
direct-connect to the blinded path introduction point and send the
`invoice_request` without a channel. For CLN it turns out this is
fine as they never select a peer for their introduction point at
all. However, for LDK this would break existing nodes as nodes
might now pick an lnd peer as an introduction node but it won't
forward the onion message.

For now, we just drop the separate introduction point selection and
just always use ourselves as an introduction point (assuming we're
an announced node).

This should also have the side-effect of making offers marginally
more robust, which may be worth it, even if it sucks to drop any
pretense of privacy.

Backport of 8c08a30

Conflicts resolved in:
 * lightning/src/ln/offers_tests.rs
 * lightning/src/onion_message/messenger.rs
If an outbound payment was abandoned with htlcs in-flight and later claimed, we
would previously have the PaymentSent::fee_paid_msat be set to None. This
contradicted some docs on the event that stated the field would always be Some
after 0.0.103.

Backport of 3e9e6e9

Conflicts resolved in:
 * lightning/src/ln/functional_tests.rs
 * lightning/src/ln/outbound_payment.rs
 * lightning/src/ln/payment_tests.rs
@TheBlueMatt TheBlueMatt force-pushed the 2026-05-0.1-backports branch from 9366866 to e60acde Compare June 11, 2026 11:27
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.

9 participants