From a173ded03f84193f8da14301e81fdec419c5e441 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 22 Nov 2021 03:27:17 +0000 Subject: [PATCH] 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. --- lightning-invoice/src/payment.rs | 8 +++++ lightning/src/routing/router.rs | 11 ++++++ lightning/src/routing/scoring.rs | 62 ++++++++++++++++++++++++++++---- 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index a480d40e9..a141eb2c3 100644 --- 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(&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, _source: &NodeId, _target: &NodeId @@ -1224,6 +1228,10 @@ mod tests { } } + #[cfg(c_bindings)] + impl lightning::util::ser::Writeable for TestScorer { + fn write(&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, _source: &NodeId, _target: &NodeId diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 8b2e4d137..00c3ee6bc 100644 --- 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(&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, _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(&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, _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 a2d314665..eb522a48a 100644 --- 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 $(+ $supertrait)*> Score for T { + fn channel_penalty_msat(&self, short_channel_id: u64, send_amt_msat: u64, channel_capacity_msat: Option, 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 { type Locked = MutexGuard<'a, T>; @@ -117,13 +146,34 @@ impl<'a, T: 'a + Score> LockableScore<'a> for RefCell { } } -impl> Score for T { - fn channel_penalty_msat(&self, short_channel_id: u64, send_amt_msat: u64, channel_capacity_msat: Option, 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 { + score: Mutex, +} +#[cfg(c_bindings)] +/// (C-not exported) +impl<'a, T: Score + 'a> LockableScore<'a> for MultiThreadedLockableScore { + 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(&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(&self, writer: &mut W) -> Result<(), io::Error> { + S::write(&**self, writer) } } -- 2.39.5