Make the API async where needed, avoid block_on #926
Draft
tnull wants to merge 35 commits into
Draft
Conversation
Use a temporary rust-lightning fork revision that exposes async migratable KV-store support. This lets ldk-node migrate filesystem stores without reimplementing LDK's migration logic locally. Co-Authored-By: HAL 9000
Read and write BDK wallet state through async KVStore helpers while keeping the current WalletPersister entry points bridged through the node runtime. This reduces the wallet persistence surface that still depends on KVStoreSync. Co-Authored-By: HAL 9000
Static invoice persistence already runs from async handlers, so use KVStore directly instead of routing those reads and writes through the blocking KVStoreSync trait. Co-Authored-By: HAL 9000
Persist peer store updates through async KVStore operations. The synchronous node APIs keep bridging at their runtime boundary while async event handling awaits peer persistence directly. Co-Authored-By: HAL 9000
Co-Authored-By: HAL 9000
Document that synchronous peer readers do not wait for the async mutation guard and can observe changes before persistence catches up. Co-Authored-By: HAL 9000
Persist DataStore mutations through async KVStore operations while keeping the existing synchronous APIs bridged through the node runtime. Async event handling now awaits payment store writes directly. Co-Authored-By: HAL 9000
Document that synchronous DataStore readers do not wait for the async mutation guard and can observe transient state while persistence catches up. Co-Authored-By: HAL 9000
Persist node metric updates through async KVStore writes and await them from the chain, gossip, and scoring tasks. This removes the remaining blocking metrics writer while keeping the helper name stable. Co-Authored-By: HAL 9000
Avoid introducing a temporary macro when moving node metrics persistence onto async KV storage. Co-Authored-By: HAL 9000
Co-Authored-By: HAL 9000
Document that synchronous node metrics readers do not wait for the async mutation guard and can observe metrics while persistence catches up. Co-Authored-By: HAL 9000
Persist the on-chain wallet through BDK's AsyncWalletPersister so wallet state writes use the async KVStore path. Existing synchronous wallet APIs keep bridging through the node runtime until their callers are made async. Co-Authored-By: HAL 9000
Open filesystem stores through the async LDK migration helper so v1-to-v2 store migration no longer depends on the blocking KVStoreSync migration path. Co-Authored-By: HAL 9000
Move the existing in-memory test store into a shared module without changing its behavior. This lets integration tests reuse it while keeping the later async TestSyncStore change separate. Co-Authored-By: HAL 9000
Keep the shared test store move as a pure code move by restoring the original comments and spacing. Co-Authored-By: HAL 9000
Exercise async KVStore operations in TestSyncStore and filesystem migration tests while keeping the temporary sync comparison path until the final KVStoreSync removal. Co-Authored-By: HAL 9000
Use a local async KVStore-backed monitor persister in store tests so the later blocking KVStore removal can drop the old synchronous MonitorUpdatingPersister path without mixing the test adapter into that change. Co-Authored-By: HAL 9000
Drop the remaining synchronous KV store trait bounds and implementations. After the preceding migrations, custom stores only need to provide async KVStore persistence, and pathfinding score export also reads from the async store. Co-Authored-By: HAL 9000
Compile the VSS persistence tests after the shared KV store helper moved to async persistence. Co-Authored-By: HAL 9000
Add a crate-local runtime wrapper for store backends that need to keep their I/O isolated while shutting down safely from async contexts. Co-Authored-By: HAL 9000
Document why store backends temporarily use isolated runtimes while synchronous APIs still bridge async persistence with block_on. Co-Authored-By: HAL 9000
Co-Authored-By: HAL 9000
Keep VSS persistence on the isolated runtime without hiding each spawned task behind an extra helper, making the temporary runtime bridge easier to follow at each call site. Co-Authored-By: HAL 9000
Co-Authored-By: HAL 9000
Keep PostgreSQL persistence on the isolated runtime without hiding each spawned task behind an extra helper, making the temporary runtime bridge easier to follow at each call site. Co-Authored-By: HAL 9000
Enable Tokio's eager driver handoff when building with tokio_unstable so node-owned runtimes can use the dedicated driver handoff path where available. Build binding artifacts and selected CI coverage with tokio_unstable so the cfg-gated runtime path remains exercised. Co-Authored-By: HAL 9000
Document why Tokio eager driver handoff only narrows the block_on deadlock risk and why StoreRuntime remains necessary until fully async APIs remove the blocking bridge. Co-Authored-By: HAL 9000
Give ldk-node-created runtime workers a stable ldk-node-prefixed thread name so runtime activity is easier to identify in debuggers and logs, matching the store runtime naming convention. Co-Authored-By: HAL 9000
Avoid nested runtime waits when peer, payment, and node metrics operations persist through async storage. This lets callers already in async contexts await those operations directly while keeping existing sync boundaries explicit. Co-Authored-By: HAL 9000
Resolve VSS schema setup through async initialization on the caller runtime so VSS stores no longer create or retain a Tokio runtime. Co-Authored-By: HAL 9000
Construct PostgreSQL storage on the caller runtime now that store-backed APIs are async, removing the store-owned runtime and shutdown path. Co-Authored-By: HAL 9000
Expose startup, shutdown, builder, wallet, liquidity, and bindings entrypoints as async so store-backed persistence no longer needs local synchronous future driving. Update tests and benches to await the async surface and keep store persistence helpers on the async storage path. Co-Authored-By: HAL 9000
Describe the Tokio runtime expectations for LDK Node callers so async API users know when runtime handles are reused and why runtime worker threads must remain available for node progress. Co-Authored-By: HAL 9000
|
👋 Hi! This PR is now in draft status. |
benthecarman
reviewed
Jun 8, 2026
| node_b.node_id(), | ||
| preimage, | ||
| None, | ||
| ); |
|
|
||
| let liquidity_source = Arc::clone(&liquidity_source); | ||
| let invoice = self.runtime.block_on(async move { | ||
| let invoice = async move { |
Contributor
There was a problem hiding this comment.
do we still need this async move?
| } | ||
| }; | ||
|
|
||
| if let Err(e) = self.runtime.block_on(self.update_payment_store(events)) { |
Contributor
There was a problem hiding this comment.
do we need an async version of these traits in LDK?
| let wallet = self.clone(); | ||
| let runtime = Arc::clone(&self.runtime); | ||
| let logger = Arc::clone(&self.logger); | ||
| runtime.spawn_background_task(async move { |
Contributor
There was a problem hiding this comment.
this seems dangerous to not return an error if persists fails. Idk if we can really get around it though
| let logger = Arc::clone(&listening_logger); | ||
| let listening_addrs = listening_addresses.clone(); | ||
| let listeners = self.runtime.block_on(async move { | ||
| let listeners = async move { |
Contributor
There was a problem hiding this comment.
dont really need this async move
| return Err(Error::NotRunning); | ||
| pub async fn stop(&self) -> Result<(), Error> { | ||
| { | ||
| let mut is_running_lock = self.is_running.write().expect("lock"); |
Contributor
There was a problem hiding this comment.
should we still be using a sync lock for is_running?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #919.
WIP