Uh oh!
There was an error while loading. Please reload this page.
Make the API async where needed, avoid block_on - #926
Conversation
👋 Hi! This PR is now in draft status. |
| 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 { |
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)) { |
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 { |
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 { |
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"); |
There was a problem hiding this comment.
should we still be using a sync lock for is_running?
joostjager
left a comment
There was a problem hiding this comment.
This looks great. But please don't finalize this as a single big PR, but do it incrementally instead.
| Ok(tx) | ||
| fn get_new_address_sync(&self) -> bitcoin::Address { | ||
| let address = self.get_new_address_inner(); | ||
| self.spawn_persist_wallet(); |
There was a problem hiding this comment.
Is it safe, return before persist?
| handle.spawn_blocking(func) | ||
| } | ||
| pub fn block_on<F: Future>(&self, future: F) -> F::Output { |
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
6a3db1c to
f58384dComparetnull
commented
Jun 9, 2026
Rebased after #919 landed, though we'll probably wait with moving this forward until after we branched-off for v0.8. |
Based on #919.
WIP