liquidity: Allow setting process_events callback in c_bindings

[tnull]

To trigger message processing, we previously had the user set a callback to PeerManager::process_events via an Fn() callback. This is however not supported by c_bindings.

Here, we therefore introduce as ProcessMesssagesCallback trait that can be used via LiquidityManager::set_process_msgs_callback_fn, which is exposed in c_bindings.

(Also confirmed to work with LDK Node)

[TheBlueMatt]

What's the difference here compared to just leaving only this for Rust users? If we require ownership anyway, the caller probably doesn't have the callback in a Box to begin with, so us doing it for them even in Rust seems fine?

[tnull]

Mhh, good point. I basically followed the recipe in Future/FutureCallback. I guess there we could also consider dropping the trait version in favor of the _fn version?

[tnull]

Took the liberty to force-push with these additional changes (to avoid another review roundtrip for the squashing):

> git
diff-tree -U2 c6abf0902 e05b76af7
diff --git a/lightning-liquidity/src/manager.rs b/lightning-liquidity/src/manager.rs
index 6b6b146af..a4c130333 100644
--- a/lightning-liquidity/src/manager.rs+++ b/lightning-liquidity/src/manager.rs@@ -311,25 +311,11 @@ where {
/// let process_msgs_callback = move || process_msgs_pm.process_events();
///
- /// my_liquidity_manager.set_process_msgs_callback(Box::new(process_msgs_callback));+ /// my_liquidity_manager.set_process_msgs_callback(process_msgs_callback);
/// # }
/// ```
///
/// [`PeerManager::process_events`]: lightning::ln::peer_handler::PeerManager::process_events
- pub fn set_process_msgs_callback(&self, callback: Box<dyn ProcessMessagesCallback>) {- self.pending_messages.set_process_msgs_callback(callback);- }-- /// Allows to set a callback that will be called after new messages are pushed to the message- /// queue.- ///- /// C bindings don't (currently) know how to map `Box<dyn Trait>`, and while it could add the- /// following wrapper, doing it in the bindings is currently much more work than simply doing it- /// here.- ///- /// Hence we simply allow setting a callback function that will be set via- /// [`Self::set_process_msgs_callback`] internally.- #[cfg(c_bindings)]- pub fn set_process_msgs_callback_fn<F: 'static + ProcessMessagesCallback>(&self, callback: F) {- self.set_process_msgs_callback(Box::new(callback));+ pub fn set_process_msgs_callback<F: 'static + ProcessMessagesCallback>(&self, callback: F) {+ self.pending_messages.set_process_msgs_callback(Box::new(callback));
}
diff --git a/lightning-liquidity/tests/common/mod.rs b/lightning-liquidity/tests/common/mod.rs
index faf4cc5a6..8b8507a9f 100644
--- a/lightning-liquidity/tests/common/mod.rs+++ b/lightning-liquidity/tests/common/mod.rs@@ -479,8 +479,5 @@ pub(crate) fn create_liquidity_node(
let process_msgs_flag = Arc::clone(&check_msgs_processed);
let process_msgs_callback = move || process_msgs_flag.store(true, Ordering::Release);
- #[cfg(not(c_bindings))]- liquidity_manager.set_process_msgs_callback(Box::new(process_msgs_callback));- #[cfg(c_bindings)]- liquidity_manager.set_process_msgs_callback_fn(process_msgs_callback);+ liquidity_manager.set_process_msgs_callback(process_msgs_callback);
Node {

[TheBlueMatt]

Super straightforward, just gonna land this.

[TheBlueMatt]

Backported in #3536.