Skip to content

Derive shutdown scripts without blocking on wallet persistence - #1011

Draft
jkczyz wants to merge 1 commit into
lightningdevkit:mainfrom
jkczyz:2026-07-signer-blockon-deadlock
Draft

Derive shutdown scripts without blocking on wallet persistence#1011
jkczyz wants to merge 1 commit into
lightningdevkit:mainfrom
jkczyz:2026-07-signer-blockon-deadlock

Conversation

@jkczyz

@jkczyz jkczyz commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #1010

LDK invokes the sync SignerProvider::get_shutdown_scriptpubkey and get_destination_script callbacks on runtime worker threads while holding channel locks, e.g. from the event handler when accepting an inbound channel. These callbacks called Runtime::block_on to await wallet persistence, which can deadlock the runtime:

  1. The event-handler task parks in block_on while holding the per-peer channel Mutex (its worker core is handed off via block_in_place).
  2. The replacement worker picks up a lightning-net-tokio task whose PeerManager::process_events blocks synchronously on that same Mutex, capturing the core.
  3. The task holding the wallet persister lock can then never be polled, so the awaited future never completes and no party can make progress.

Observed as permanent hangs of integration_tests_rust runs under load (the tests run the node on a single-worker runtime, where one captured core is fatal). See #1010 for full thread samples and analysis.

Instead of waiting on persistence, the callbacks now reveal the address under the sync wallet lock and persist the staged change set from a background task, using the same persister-lock → take_staged ordering as every other persist path.

Two semantics changes worth calling out:

  • A persistence failure no longer rejects the channel open.
  • If the node crashes before the background flush lands, the revealed index is lost and the address may be handed out again after restart. BDK's keychain lookahead still detects any funds it receives, and this matches the durability LDK's own KeysManager provides for shutdown scripts.

The new test gates the acceptor's bdk_wallet-namespace store writes during a channel open: without the fix the acceptor wedges in get_shutdown_scriptpubkey and the open times out (reproducing the deadlock deterministically); with it the open completes and the deferred persist lands once the store recovers.

Drafted with the assistance of Claude Code.

LDK invokes the sync `SignerProvider::get_shutdown_scriptpubkey` and
`get_destination_script` callbacks on runtime worker threads while
holding channel locks, e.g. when accepting an inbound channel. Blocking
there on wallet persistence could deadlock the runtime: the parked
callback still held the channel locks, other tasks blocking
synchronously on those locks captured the remaining worker cores, and
the persistence future the callback waited on could then never be
polled. Observed as a permanent hang of integration test runs.

Instead, reveal the address without waiting and persist the staged
change set in the background. A persistence failure therefore no longer
rejects the channel; if the node crashes before the flush lands, the
revealed index may be handed out again after restart, which BDK's
keychain lookahead tolerates.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ldk-reviews-bot

Copy link
Copy Markdown

👋 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!

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.

Runtime deadlock in sync SignerProvider callbacks

2 participants