Hard-code scorer parameters to `ProbabilisticScoringFeeParameters`
authorMatt Corallo <git@bluematt.me>
Thu, 21 Sep 2023 22:15:09 +0000 (22:15 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 3 Oct 2023 23:23:15 +0000 (23:23 +0000)
The scorer currently relies on an associated type for the fee
parameters. This isn't supportable at all in bindings, and for
lack of a better option we simply hard-code the parameter for all
scorers  to `ProbabilisticScoringFeeParameters`.

lightning-background-processor/src/lib.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/peer_handler.rs
lightning/src/routing/router.rs
lightning/src/routing/scoring.rs
lightning/src/util/test_utils.rs

index 2682e3824b6576d19aaafc8f8d930999ac678a29..bcee3b59c94f69590cacab15a9058703285e801f 100644 (file)
@@ -894,6 +894,7 @@ mod tests {
                fn disconnect_socket(&mut self) {}
        }
 
+       #[cfg(not(c_bindings))]
        type ChannelManager =
                channelmanager::ChannelManager<
                        Arc<ChainMonitor>,
@@ -905,9 +906,22 @@ mod tests {
                        Arc<DefaultRouter<
                                Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
                                Arc<test_utils::TestLogger>,
-                               Arc<Mutex<TestScorer>>,
-                               (),
-                               TestScorer>
+                               Arc<Mutex<TestScorer>>>
+                       >,
+                       Arc<test_utils::TestLogger>>;
+       #[cfg(c_bindings)]
+       type ChannelManager =
+               channelmanager::ChannelManager<
+                       Arc<ChainMonitor>,
+                       Arc<test_utils::TestBroadcaster>,
+                       Arc<KeysManager>,
+                       Arc<KeysManager>,
+                       Arc<KeysManager>,
+                       Arc<test_utils::TestFeeEstimator>,
+                       Arc<DefaultRouter<
+                               Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+                               Arc<test_utils::TestLogger>,
+                               Arc<Mutex<TestScorer>>>
                        >,
                        Arc<test_utils::TestLogger>>;
 
@@ -1064,9 +1078,10 @@ mod tests {
        }
 
        impl ScoreLookUp for TestScorer {
+               #[cfg(not(c_bindings))]
                type ScoreParams = ();
                fn channel_penalty_msat(
-                       &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &Self::ScoreParams
+                       &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &lightning::routing::scoring::ProbabilisticScoringFeeParameters
                ) -> u64 { unimplemented!(); }
        }
 
index a926b889d649495f67cce7d3fc8d9d55a3a35d5b..defc5b76d74036ed84ebba897fd00b4fb93b5923 100644 (file)
@@ -792,7 +792,8 @@ struct PendingInboundPayment {
 /// or, respectively, [`Router`] for its router, but this type alias chooses the concrete types
 /// of [`KeysManager`] and [`DefaultRouter`].
 ///
-/// This is not exported to bindings users as Arcs don't make sense in bindings
+/// This is not exported to bindings users as type aliases aren't supported in most languages
+#[cfg(not(c_bindings))]
 pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
        Arc<M>,
        Arc<T>,
@@ -804,8 +805,6 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
                Arc<NetworkGraph<Arc<L>>>,
                Arc<L>,
                Arc<RwLock<ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>>>,
-               ProbabilisticScoringFeeParameters,
-               ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>,
        >>,
        Arc<L>
 >;
@@ -820,7 +819,8 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
 /// or, respectively, [`Router`]  for its router, but this type alias chooses the concrete types
 /// of [`KeysManager`] and [`DefaultRouter`].
 ///
-/// This is not exported to bindings users as Arcs don't make sense in bindings
+/// This is not exported to bindings users as type aliases aren't supported in most languages
+#[cfg(not(c_bindings))]
 pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> =
        ChannelManager<
                &'a M,
@@ -833,8 +833,6 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> =
                        &'f NetworkGraph<&'g L>,
                        &'g L,
                        &'h RwLock<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>,
-                       ProbabilisticScoringFeeParameters,
-                       ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>
                >,
                &'g L
        >;
index f231b097faaa9ed39bfc7538629f094dca1ded1e..ab6177f041bff4f42e217d8e9b08cdd7c7da2f1b 100644 (file)
@@ -24,6 +24,7 @@ use crate::ln::ChannelId;
 use crate::ln::features::{InitFeatures, NodeFeatures};
 use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler, LightningError, SocketAddress, OnionMessageHandler, RoutingMessageHandler};
+#[cfg(not(c_bindings))]
 use crate::ln::channelmanager::{SimpleArcChannelManager, SimpleRefChannelManager};
 use crate::util::ser::{VecWriter, Writeable, Writer};
 use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor,NextNoiseStep};
@@ -608,7 +609,8 @@ impl Peer {
 /// SimpleRefPeerManager is the more appropriate type. Defining these type aliases prevents
 /// issues such as overly long function definitions.
 ///
-/// This is not exported to bindings users as `Arc`s don't make sense in bindings.
+/// This is not exported to bindings users as type aliases aren't supported in most languages.
+#[cfg(not(c_bindings))]
 pub type SimpleArcPeerManager<SD, M, T, F, C, L> = PeerManager<
        SD,
        Arc<SimpleArcChannelManager<M, T, F, L>>,
@@ -626,7 +628,8 @@ pub type SimpleArcPeerManager<SD, M, T, F, C, L> = PeerManager<
 /// But if this is not necessary, using a reference is more efficient. Defining these type aliases
 /// helps with issues such as long function definitions.
 ///
-/// This is not exported to bindings users as general type aliases don't make sense in bindings.
+/// This is not exported to bindings users as type aliases aren't supported in most languages.
+#[cfg(not(c_bindings))]
 pub type SimpleRefPeerManager<
        'a, 'b, 'c, 'd, 'e, 'f, 'logger, 'h, 'i, 'j, 'graph, SD, M, T, F, C, L
 > = PeerManager<
index 28d452c56b616afde8e1aea886e5a0bfe65f8eb0..232d906cb3adfa8dbac00ed8a0e5e0818e601918 100644 (file)
@@ -33,31 +33,31 @@ use core::{cmp, fmt};
 use core::ops::Deref;
 
 /// A [`Router`] implemented using [`find_route`].
-pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> where
+pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> where
        L::Target: Logger,
-       S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
+       S::Target: for <'a> LockableScore<'a>,
 {
        network_graph: G,
        logger: L,
        random_seed_bytes: Mutex<[u8; 32]>,
        scorer: S,
-       score_params: SP
+       score_params: crate::routing::scoring::ProbabilisticScoringFeeParameters,
 }
 
-impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> DefaultRouter<G, L, S, SP, Sc> where
+impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> DefaultRouter<G, L, S> where
        L::Target: Logger,
-       S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
+       S::Target: for <'a> LockableScore<'a>,
 {
        /// Creates a new router.
-       pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32], scorer: S, score_params: SP) -> Self {
+       pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32], scorer: S, score_params: crate::routing::scoring::ProbabilisticScoringFeeParameters) -> Self {
                let random_seed_bytes = Mutex::new(random_seed_bytes);
                Self { network_graph, logger, random_seed_bytes, scorer, score_params }
        }
 }
 
-impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: ScoreLookUp<ScoreParams = SP>> Router for DefaultRouter<G, L, S, SP, Sc> where
+impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultRouter<G, L, S> where
        L::Target: Logger,
-       S::Target: for <'a> LockableScore<'a, ScoreLookUp = Sc>,
+       S::Target: for <'a> LockableScore<'a>,
 {
        fn find_route(
                &self,
@@ -128,8 +128,9 @@ impl<'a, S: Deref> ScorerAccountingForInFlightHtlcs<'a, S> where S::Target: Scor
 }
 
 impl<'a, S: Deref> ScoreLookUp for ScorerAccountingForInFlightHtlcs<'a, S> where S::Target: ScoreLookUp {
+       #[cfg(not(c_bindings))]
        type ScoreParams = <S::Target as ScoreLookUp>::ScoreParams;
-       fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
+       fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 {
                if let Some(used_liquidity) = self.inflight_htlcs.used_liquidity_msat(
                        source, target, short_channel_id
                ) {
@@ -1466,7 +1467,7 @@ fn sort_first_hop_channels(
 pub fn find_route<L: Deref, GL: Deref, S: ScoreLookUp>(
        our_node_pubkey: &PublicKey, route_params: &RouteParameters,
        network_graph: &NetworkGraph<GL>, first_hops: Option<&[&ChannelDetails]>, logger: L,
-       scorer: &S, score_params: &S::ScoreParams, random_seed_bytes: &[u8; 32]
+       scorer: &S, score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
 where L::Target: Logger, GL::Target: Logger {
        let graph_lock = network_graph.read_only();
@@ -1478,7 +1479,7 @@ where L::Target: Logger, GL::Target: Logger {
 
 pub(crate) fn get_route<L: Deref, S: ScoreLookUp>(
        our_node_pubkey: &PublicKey, route_params: &RouteParameters, network_graph: &ReadOnlyNetworkGraph,
-       first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S, score_params: &S::ScoreParams,
+       first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S, score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters,
        _random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
 where L::Target: Logger {
@@ -2763,9 +2764,10 @@ fn build_route_from_hops_internal<L: Deref>(
        }
 
        impl ScoreLookUp for HopScorer {
+               #[cfg(not(c_bindings))]
                type ScoreParams = ();
                fn channel_penalty_msat(&self, _short_channel_id: u64, source: &NodeId, target: &NodeId,
-                       _usage: ChannelUsage, _score_params: &Self::ScoreParams) -> u64
+                       _usage: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64
                {
                        let mut cur_id = self.our_node_id;
                        for i in 0..self.hop_ids.len() {
@@ -6098,8 +6100,9 @@ mod tests {
                fn write<W: Writer>(&self, _w: &mut W) -> Result<(), crate::io::Error> { unimplemented!() }
        }
        impl ScoreLookUp for BadChannelScorer {
+               #[cfg(not(c_bindings))]
                type ScoreParams = ();
-               fn channel_penalty_msat(&self, short_channel_id: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
+               fn channel_penalty_msat(&self, short_channel_id: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 {
                        if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
                }
        }
@@ -6114,8 +6117,9 @@ mod tests {
        }
 
        impl ScoreLookUp for BadNodeScorer {
+               #[cfg(not(c_bindings))]
                type ScoreParams = ();
-               fn channel_penalty_msat(&self, _: u64, _: &NodeId, target: &NodeId, _: ChannelUsage, _score_params:&Self::ScoreParams) -> u64 {
+               fn channel_penalty_msat(&self, _: u64, _: &NodeId, target: &NodeId, _: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters) -> u64 {
                        if *target == self.node_id { u64::max_value() } else { 0 }
                }
        }
@@ -7798,7 +7802,7 @@ pub(crate) mod bench_utils {
        }
 
        pub(crate) fn generate_test_routes<S: ScoreLookUp + ScoreUpdate>(graph: &NetworkGraph<&TestLogger>, scorer: &mut S,
-               score_params: &S::ScoreParams, features: Bolt11InvoiceFeatures, mut seed: u64,
+               score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, features: Bolt11InvoiceFeatures, mut seed: u64,
                starting_amount: u64, route_count: usize,
        ) -> Vec<(ChannelDetails, PaymentParameters, u64)> {
                let payer = payer_pubkey();
@@ -7983,7 +7987,7 @@ pub mod benches {
 
        fn generate_routes<S: ScoreLookUp + ScoreUpdate>(
                bench: &mut Criterion, graph: &NetworkGraph<&TestLogger>, mut scorer: S,
-               score_params: &S::ScoreParams, features: Bolt11InvoiceFeatures, starting_amount: u64,
+               score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters, features: Bolt11InvoiceFeatures, starting_amount: u64,
                bench_name: &'static str,
        ) {
                let payer = bench_utils::payer_pubkey();
index 67daf8578f43835a04d0baeb763a481043a88846..906616c47ee436ebbc3e00bf85a8e9f493ee448c 100644 (file)
@@ -90,9 +90,13 @@ macro_rules! define_score { ($($supertrait: path)*) => {
 ///
 /// Scoring is in terms of fees willing to be paid in order to avoid routing through a channel.
 pub trait ScoreLookUp {
+       #[cfg(not(c_bindings))]
        /// A configurable type which should contain various passed-in parameters for configuring the scorer,
        /// on a per-routefinding-call basis through to the scorer methods,
        /// which are used to determine the parameters for the suitability of channels for use.
+       ///
+       /// Note that due to limitations in many other languages' generics features, language bindings
+       /// use [`ProbabilisticScoringFeeParameters`] for the parameters on all scorers.
        type ScoreParams;
        /// 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`.
@@ -103,7 +107,7 @@ pub trait ScoreLookUp {
        /// [`u64::max_value`] is given to indicate sufficient capacity for the invoice's full amount.
        /// Thus, implementations should be overflow-safe.
        fn channel_penalty_msat(
-               &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams
+               &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters
        ) -> u64;
 }
 
@@ -135,9 +139,10 @@ impl<T: ScoreLookUp + ScoreUpdate $(+ $supertrait)*> Score for T {}
 
 #[cfg(not(c_bindings))]
 impl<S: ScoreLookUp, T: Deref<Target=S>> ScoreLookUp for T {
+       #[cfg(not(c_bindings))]
        type ScoreParams = S::ScoreParams;
        fn channel_penalty_msat(
-               &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams
+               &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters
        ) -> u64 {
                self.deref().channel_penalty_msat(short_channel_id, source, target, usage, score_params)
        }
@@ -314,9 +319,10 @@ impl<'a, T: 'a + Score> Deref for MultiThreadedScoreLockRead<'a, T> {
 
 #[cfg(c_bindings)]
 impl<'a, T: Score> ScoreLookUp for MultiThreadedScoreLockRead<'a, T> {
+       #[cfg(not(c_bindings))]
        type ScoreParams = T::ScoreParams;
        fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId,
-               target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams
+               target: &NodeId, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters
        ) -> u64 {
                self.0.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
        }
@@ -393,8 +399,9 @@ impl FixedPenaltyScorer {
 }
 
 impl ScoreLookUp for FixedPenaltyScorer {
+       #[cfg(not(c_bindings))]
        type ScoreParams = ();
-       fn channel_penalty_msat(&self, _: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params: &Self::ScoreParams) -> u64 {
+       fn channel_penalty_msat(&self, _: u64, _: &NodeId, _: &NodeId, _: ChannelUsage, _score_params: &ProbabilisticScoringFeeParameters) -> u64 {
                self.penalty_msat
        }
 }
@@ -1374,6 +1381,7 @@ impl<L: DerefMut<Target = u64>, BRT: DerefMut<Target = HistoricalBucketRangeTrac
 }
 
 impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ScoreLookUp for ProbabilisticScorerUsingTime<G, L, T> where L::Target: Logger {
+       #[cfg(not(c_bindings))]
        type ScoreParams = ProbabilisticScoringFeeParameters;
        fn channel_penalty_msat(
                &self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters
index bd66e4cae3e56fa70195a4627bf38451bac37d9c..2e16563b73ed38cd74907bf18f6598b30dc028d6 100644 (file)
@@ -1267,9 +1267,10 @@ impl crate::util::ser::Writeable for TestScorer {
 }
 
 impl ScoreLookUp for TestScorer {
+       #[cfg(not(c_bindings))]
        type ScoreParams = ();
        fn channel_penalty_msat(
-               &self, short_channel_id: u64, _source: &NodeId, _target: &NodeId, usage: ChannelUsage, _score_params: &Self::ScoreParams
+               &self, short_channel_id: u64, _source: &NodeId, _target: &NodeId, usage: ChannelUsage, _score_params: &crate::routing::scoring::ProbabilisticScoringFeeParameters
        ) -> u64 {
                if let Some(scorer_expectations) = self.scorer_expectations.borrow_mut().as_mut() {
                        match scorer_expectations.pop_front() {