From ac1660ef6753cd1e36760686305b5c0b87dd376f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 26 Jun 2022 01:44:21 +0000 Subject: [PATCH] Concretize `WriteableScore` into `MultiThreadedLockableScore` 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 | 7 +++---- lightning/src/routing/scoring.rs | 22 +++++++++++++++++----- lightning/src/util/persist.rs | 20 +++++++++----------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 11dca44bb..a99fab6fe 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -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; @@ -238,11 +238,10 @@ impl BackgroundProcessor { RGS: 'static + Deref> + Send, UMH: 'static + Deref + Send + Sync, PM: 'static + Deref> + Send + Sync, - S: 'static + Deref + Send + Sync, - SC: WriteableScore<'a>, + SC: Score + Send, >( persister: PS, event_handler: EH, chain_monitor: M, channel_manager: CM, - gossip_sync: GossipSync, peer_manager: PM, logger: L, scorer: Option, + gossip_sync: GossipSync, peer_manager: PM, logger: L, scorer: Option<&'static MultiThreadedLockableScore>, ) -> Self where CA::Target: 'static + chain::Access, diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index 4af97bc25..95e14208d 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -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}; @@ -146,11 +148,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 { type Locked = MutexGuard<'a, T>; @@ -159,6 +163,7 @@ impl<'a, T: 'a + Score> LockableScore<'a> for Mutex { } } +#[cfg(not(c_bindings))] impl<'a, T: 'a + Score> LockableScore<'a> for RefCell { type Locked = RefMut<'a, T>; @@ -174,14 +179,21 @@ pub struct MultiThreadedLockableScore { } #[cfg(c_bindings)] /// (C-not exported) -impl<'a, T: Score + 'a> LockableScore<'a> for MultiThreadedLockableScore { - type Locked = MutexGuard<'a, T>; +impl<'a, S: Score + 'a> LockableScore<'a> for MultiThreadedLockableScore { + 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 Writeable for MultiThreadedLockableScore { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { + S::write(&*self.lock(), writer) + } +} + #[cfg(c_bindings)] impl MultiThreadedLockableScore { /// Creates a new [`MultiThreadedLockableScore`] given an underlying [`Score`]. diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 522c1c92a..c271ecc63 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -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(&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, T::Target: 'static + BroadcasterInterface, K::Target: 'static + KeysInterface, 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) -> 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) -> 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) -> 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, T::Target: 'static + BroadcasterInterface, K::Target: 'static + KeysInterface, 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) -> 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) -> Result<(), io::Error> { self.persist("scorer", &scorer) } } -- 2.39.5