Make `Score : Writeable` in c_bindings and impl on `LockedScore`

Ultimately we likely need to wrap the locked `Score` in a struct
that exposes writeable somehow, but because all traits have to be
fully concretized for C bindings we'll still need `Writeable` on
all `Score` in order to expose `Writeable` on the locked score.
Otherwise, we'll only have a `LockedScore` with a `Score` visible
that only has the `Score` methods, never the original type.

diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs
index a480d40e95d296beecfa2bb73fdb7682de378099..a141eb2c3e1445b3e3f08ce2afbd411989b32389 100644 (file)
--- a/lightning-invoice/src/payment.rs
+++ b/lightning-invoice/src/payment.rs
@@ -36,6 +36,7 @@
 //! # use lightning::routing::router::{Route, RouteHop, RouteParameters};
 //! # use lightning::util::events::{Event, EventHandler, EventsProvider};
 //! # use lightning::util::logger::{Logger, Record};
+//! # use lightning::util::ser::{Writeable, Writer};
 //! # use lightning_invoice::Invoice;
 //! # use lightning_invoice::payment::{InvoicePayer, Payer, RetryAttempts, Router};
 //! # use secp256k1::key::PublicKey;
@@ -71,6 +72,9 @@
 //! # }
 //! #
 //! # struct FakeScorer {};
+//! # impl Writeable for FakeScorer {
+//! #     fn write<W: Writer>(&self, w: &mut W) -> Result<(), std::io::Error> { unimplemented!(); }
+//! # }
 //! # impl Score for FakeScorer {
 //! #     fn channel_penalty_msat(
 //! #         &self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId
@@ -1224,6 +1228,10 @@ mod tests {
                }
        }
 
+       #[cfg(c_bindings)]
+       impl lightning::util::ser::Writeable for TestScorer {
+               fn write<W: lightning::util::ser::Writer>(&self, _: &mut W) -> Result<(), std::io::Error> { unreachable!(); }
+       }
        impl Score for TestScorer {
                fn channel_penalty_msat(
                        &self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId

diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 8b2e4d137fd93f83fed0904fda12f6148a1e6834..00c3ee6bcebc947c5ba44b09ab1b5812f14a7406 100644 (file)
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -1488,6 +1488,8 @@ mod tests {
        use ln::channelmanager;
        use util::test_utils;
        use util::ser::Writeable;
+       #[cfg(c_bindings)]
+       use util::ser::Writer;
 
        use bitcoin::hashes::sha256d::Hash as Sha256dHash;
        use bitcoin::hashes::Hash;
@@ -4653,6 +4655,10 @@ mod tests {
                short_channel_id: u64,
        }
 
+       #[cfg(c_bindings)]
+       impl Writeable for BadChannelScorer {
+               fn write<W: Writer>(&self, _w: &mut W) -> Result<(), ::io::Error> { unimplemented!() }
+       }
        impl Score for BadChannelScorer {
                fn channel_penalty_msat(&self, short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId) -> u64 {
                        if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
@@ -4665,6 +4671,11 @@ mod tests {
                node_id: NodeId,
        }
 
+       #[cfg(c_bindings)]
+       impl Writeable for BadNodeScorer {
+               fn write<W: Writer>(&self, _w: &mut W) -> Result<(), ::io::Error> { unimplemented!() }
+       }
+
        impl Score for BadNodeScorer {
                fn channel_penalty_msat(&self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, target: &NodeId) -> u64 {
                        if *target == self.node_id { u64::max_value() } else { 0 }

diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs
index a2d314665d1c7d9273fe71529ad2c46e1fe5fc01..eb522a48ae59cf1713249e87c43841e0f3d14262 100644 (file)
--- a/lightning/src/routing/scoring.rs
+++ b/lightning/src/routing/scoring.rs
@@ -63,10 +63,22 @@ use core::ops::{DerefMut, Sub};
 use core::time::Duration;
 use io::{self, Read}; use sync::{Mutex, MutexGuard};
 
+/// We define Score ever-so-slightly differently based on whether we are being built for C bindings
+/// or not. For users, `LockableScore` must somehow be writeable to disk. For Rust users, this is
+/// no problem - you move a `Score` that implements `Writeable` into a `Mutex`, lock it, and now
+/// you have the original, concrete, `Score` type, which presumably implements `Writeable`.
+///
+/// For C users, once you've moved the `Score` into a `LockableScore` all you have after locking it
+/// is an opaque trait object with an opaque pointer with no type info. Users could take the unsafe
+/// approach of blindly casting that opaque pointer to a concrete type and calling `Writeable` from
+/// there, but other languages downstream of the C bindings (e.g. Java) can't even do that.
+/// Instead, we really want `Score` and `LockableScore` to implement `Writeable` directly, which we
+/// do here by defining `Score` differently for `cfg(c_bindings)`.
+macro_rules! define_score { ($($supertrait: path)*) => {
 /// An interface used to score payment channels for path finding.
 ///
 ///    Scoring is in terms of fees willing to be paid in order to avoid routing through a channel.
-pub trait Score {
+pub trait Score $(: $supertrait)* {
        /// Returns the fee in msats willing to be paid to avoid routing `send_amt_msat` through the
        /// given channel in the direction from `source` to `target`.
        ///
@@ -85,6 +97,22 @@ pub trait Score {
        fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64);
 }
 
+impl<S: Score, T: DerefMut<Target=S> $(+ $supertrait)*> Score for T {
+       fn channel_penalty_msat(&self, short_channel_id: u64, send_amt_msat: u64, channel_capacity_msat: Option<u64>, source: &NodeId, target: &NodeId) -> u64 {
+               self.deref().channel_penalty_msat(short_channel_id, send_amt_msat, channel_capacity_msat, source, target)
+       }
+
+       fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
+               self.deref_mut().payment_path_failed(path, short_channel_id)
+       }
+}
+} }
+
+#[cfg(c_bindings)]
+define_score!(Writeable);
+#[cfg(not(c_bindings))]
+define_score!();
+
 /// A scorer that is accessed under a lock.
 ///
 /// Needed so that calls to [`Score::channel_penalty_msat`] in [`find_route`] can be made while
@@ -101,6 +129,7 @@ pub trait LockableScore<'a> {
        fn lock(&'a self) -> Self::Locked;
 }
 
+/// (C-not exported)
 impl<'a, T: 'a + Score> LockableScore<'a> for Mutex<T> {
        type Locked = MutexGuard<'a, T>;
 
@@ -117,13 +146,34 @@ impl<'a, T: 'a + Score> LockableScore<'a> for RefCell<T> {
        }
 }
 
-impl<S: Score, T: DerefMut<Target=S>> Score for T {
-       fn channel_penalty_msat(&self, short_channel_id: u64, send_amt_msat: u64, channel_capacity_msat: Option<u64>, source: &NodeId, target: &NodeId) -> u64 {
-               self.deref().channel_penalty_msat(short_channel_id, send_amt_msat, channel_capacity_msat, source, target)
+#[cfg(c_bindings)]
+/// A concrete implementation of [`LockableScore`] which supports multi-threading.
+pub struct MultiThreadedLockableScore<S: Score> {
+       score: Mutex<S>,
+}
+#[cfg(c_bindings)]
+/// (C-not exported)
+impl<'a, T: Score + 'a> LockableScore<'a> for MultiThreadedLockableScore<T> {
+       type Locked = MutexGuard<'a, T>;
+
+       fn lock(&'a self) -> MutexGuard<'a, T> {
+               Mutex::lock(&self.score).unwrap()
        }
+}
 
-       fn payment_path_failed(&mut self, path: &[&RouteHop], short_channel_id: u64) {
-               self.deref_mut().payment_path_failed(path, short_channel_id)
+#[cfg(c_bindings)]
+/// (C-not exported)
+impl<'a, T: Writeable> Writeable for RefMut<'a, T> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               T::write(&**self, writer)
+       }
+}
+
+#[cfg(c_bindings)]
+/// (C-not exported)
+impl<'a, S: Writeable> Writeable for MutexGuard<'a, S> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               S::write(&**self, writer)
        }
 }