Concretize `WriteableScore` into `MultiThreadedLockableScore`
authorMatt Corallo <git@bluematt.me>
Sun, 26 Jun 2022 01:44:21 +0000 (01:44 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 26 Jul 2022 23:09:04 +0000 (23:09 +0000)
In general the bindings don't handle blanket implementations well -
they generate concrete implementations for everything and don't
bother building up enough context to be aware of the blanket
implementation to avoid duplicating it while still allowing users
to access struct(s) as all implemented traits.

Thus, implementing `WriteableScore` for all `LockableScore`s that
also implement `Writeable` is particularly impractical to map in
bindings.

Further, because `Score` already requires `Writeable`, having a
separate `WriteableScore` doesn't really make any sense.

Here we simply remove `WriteableScore` (in `c_bindings` mode)
entirely and push users through `MultiThreadedLockableScore` in the
higher-level traits that require `Score`.

lightning-background-processor/src/lib.rs
lightning/src/routing/scoring.rs
lightning/src/util/persist.rs

index 484439b3907b364dddc3dc2c13bb6c078be2ad0c..c4e8ea5c2b35ade6024df4821bb0ca17a45b2362 100644 (file)
@@ -19,7 +19,7 @@ use lightning::ln::channelmanager::ChannelManager;
 use lightning::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler};
 use lightning::ln::peer_handler::{CustomMessageHandler, PeerManager, SocketDescriptor};
 use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
-use lightning::routing::scoring::WriteableScore;
+use lightning::routing::scoring::{Score, MultiThreadedLockableScore};
 use lightning::util::events::{Event, EventHandler, EventsProvider};
 use lightning::util::logger::Logger;
 use lightning::util::persist::Persister;
@@ -287,11 +287,10 @@ impl BackgroundProcessor {
                RGS: 'static + Deref<Target = RapidGossipSync<G, L>> + Send,
                UMH: 'static + Deref + Send + Sync,
                PM: 'static + Deref<Target = PeerManager<Descriptor, CMH, RMH, L, UMH>> + Send + Sync,
-               S: 'static + Deref<Target = SC> + Send + Sync,
-               SC: WriteableScore<'a>,
+               SC: Score + Send,
        >(
                persister: PS, event_handler: EH, chain_monitor: M, channel_manager: CM,
-               gossip_sync: GossipSync<PGS, RGS, G, CA, L>, peer_manager: PM, logger: L, scorer: Option<S>,
+               gossip_sync: GossipSync<PGS, RGS, G, CA, L>, peer_manager: PM, logger: L, scorer: Option<&'static MultiThreadedLockableScore<SC>>,
        ) -> Self
        where
                CA::Target: 'static + chain::Access,
index 29848abf24dc21b24ab39f1927663c612deb1884..9fb7bee9a75c5a23796971b603d4f226cd07d19a 100644 (file)
@@ -63,7 +63,9 @@ use util::time::Time;
 
 use prelude::*;
 use core::fmt;
-use core::cell::{RefCell, RefMut};
+#[cfg(not(c_bindings))]
+use core::cell::RefCell;
+use core::cell::RefMut;
 use core::ops::{Deref, DerefMut};
 use core::time::Duration;
 use io::{self, Read};
@@ -160,11 +162,13 @@ pub trait LockableScore<'a> {
 ///
 /// We need this trait to be able to pass in a scorer to `lightning-background-processor` that will enable us to
 /// use the Persister to persist it.
+#[cfg(not(c_bindings))] // This doesn't make sense in bindings as all `Score`s are `Writeable`.
 pub trait WriteableScore<'a>: LockableScore<'a> + Writeable {}
 
+#[cfg(not(c_bindings))] // This doesn't make sense in bindings as all `Score`s are `Writeable`.
 impl<'a, T> WriteableScore<'a> for T where T: LockableScore<'a> + Writeable {}
 
-/// (C-not exported)
+#[cfg(not(c_bindings))]
 impl<'a, T: 'a + Score> LockableScore<'a> for Mutex<T> {
        type Locked = MutexGuard<'a, T>;
 
@@ -173,6 +177,7 @@ impl<'a, T: 'a + Score> LockableScore<'a> for Mutex<T> {
        }
 }
 
+#[cfg(not(c_bindings))]
 impl<'a, T: 'a + Score> LockableScore<'a> for RefCell<T> {
        type Locked = RefMut<'a, T>;
 
@@ -188,14 +193,21 @@ pub struct MultiThreadedLockableScore<S: Score> {
 }
 #[cfg(c_bindings)]
 /// (C-not exported)
-impl<'a, T: Score + 'a> LockableScore<'a> for MultiThreadedLockableScore<T> {
-       type Locked = MutexGuard<'a, T>;
+impl<'a, S: Score + 'a> LockableScore<'a> for MultiThreadedLockableScore<S> {
+       type Locked = MutexGuard<'a, S>;
 
-       fn lock(&'a self) -> MutexGuard<'a, T> {
+       fn lock(&'a self) -> MutexGuard<'a, S> {
                Mutex::lock(&self.score).unwrap()
        }
 }
 
+#[cfg(c_bindings)]
+impl<S: Score> Writeable for MultiThreadedLockableScore<S> {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               S::write(&*self.lock(), writer)
+       }
+}
+
 #[cfg(c_bindings)]
 impl<T: Score> MultiThreadedLockableScore<T> {
        /// Creates a new [`MultiThreadedLockableScore`] given an underlying [`Score`].
index 522c1c92ac3460773436f0dd037eb24ee8018f03..c271ecc63c040b4e6af1a5fd40f7f304a682219d 100644 (file)
@@ -11,7 +11,7 @@
 use core::ops::Deref;
 use bitcoin::hashes::hex::ToHex;
 use io::{self};
-use routing::scoring::WriteableScore;
+use routing::scoring::{Score, MultiThreadedLockableScore};
 
 use crate::{chain::{keysinterface::{Sign, KeysInterface}, self, transaction::{OutPoint}, chaininterface::{BroadcasterInterface, FeeEstimator}, chainmonitor::{Persist, MonitorUpdateId}, channelmonitor::{ChannelMonitor, ChannelMonitorUpdate}}, ln::channelmanager::ChannelManager, routing::gossip::NetworkGraph};
 use super::{logger::Logger, ser::Writeable};
@@ -25,14 +25,13 @@ pub trait KVStorePersister {
        fn persist<W: Writeable>(&self, key: &str, object: &W) -> io::Result<()>;
 }
 
-/// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`WriteableScore`] to disk.
-pub trait Persister<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref, S>
+/// Trait that handles persisting a [`ChannelManager`], [`NetworkGraph`], and [`MultiThreadedLockableScore`] to disk.
+pub trait Persister<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref, S: Score>
        where M::Target: 'static + chain::Watch<Signer>,
                T::Target: 'static + BroadcasterInterface,
                K::Target: 'static + KeysInterface<Signer = Signer>,
                F::Target: 'static + FeeEstimator,
                L::Target: 'static + Logger,
-               S: WriteableScore<'a>,
 {
        /// Persist the given ['ChannelManager'] to disk, returning an error if persistence failed.
        fn persist_manager(&self, channel_manager: &ChannelManager<Signer, M, T, K, F, L>) -> Result<(), io::Error>;
@@ -40,17 +39,16 @@ pub trait Persister<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L:
        /// Persist the given [`NetworkGraph`] to disk, returning an error if persistence failed.
        fn persist_graph(&self, network_graph: &NetworkGraph<L>) -> Result<(), io::Error>;
 
-       /// Persist the given [`WriteableScore`] to disk, returning an error if persistence failed.
-       fn persist_scorer(&self, scorer: &S) -> Result<(), io::Error>;
+       /// Persist the given [`MultiThreadedLockableScore`] to disk, returning an error if persistence failed.
+       fn persist_scorer(&self, scorer: &MultiThreadedLockableScore<S>) -> Result<(), io::Error>;
 }
 
-impl<'a, A: KVStorePersister, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref, S> Persister<'a, Signer, M, T, K, F, L, S> for A
+impl<'a, A: KVStorePersister, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref, S: Score> Persister<'a, Signer, M, T, K, F, L, S> for A
        where M::Target: 'static + chain::Watch<Signer>,
                T::Target: 'static + BroadcasterInterface,
                K::Target: 'static + KeysInterface<Signer = Signer>,
                F::Target: 'static + FeeEstimator,
-               L::Target: 'static + Logger,
-               S: WriteableScore<'a>,
+               L::Target: 'static + Logger
 {
        /// Persist the given ['ChannelManager'] to disk with the name "manager", returning an error if persistence failed.
        fn persist_manager(&self, channel_manager: &ChannelManager<Signer, M, T, K, F, L>) -> Result<(), io::Error> {
@@ -62,8 +60,8 @@ impl<'a, A: KVStorePersister, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Der
                self.persist("network_graph", network_graph)
        }
 
-       /// Persist the given [`WriteableScore`] to disk with name "scorer", returning an error if persistence failed.
-       fn persist_scorer(&self, scorer: &S) -> Result<(), io::Error> {
+       /// Persist the given [`MultiThreadedLockableScore`] to disk with name "scorer", returning an error if persistence failed.
+       fn persist_scorer(&self, scorer: &MultiThreadedLockableScore<S>) -> Result<(), io::Error> {
                self.persist("scorer", &scorer)
        }
 }