Address review comments

eel/src/lib.rs

@@ -54,10 +54,10 @@ use crate::rapid_sync_client::RapidSyncClient;
 use crate::storage_persister::StoragePersister;
 use crate::task_manager::{PeriodConfig, RestartIfFailedPeriod, TaskManager, TaskPeriods};
 use crate::tx_broadcaster::TxBroadcaster;
-use crate::types::{ChainMonitor, ChannelManager, PeerManager, RapidGossipSync, TxSync};
+use crate::types::{ChainMonitor, ChannelManager, PeerManager, RapidGossipSync, Router, TxSync};
 
-pub use crate::router::MaxFeeStrategy;
-use crate::router::{FeeCappedRouter, SimpleMaxRoutingFeeProvider};
+pub use crate::router::MaxRoutingFeeMode;
+use crate::router::{FeeLimitingRouter, SimpleMaxRoutingFeeStrategy};
 use bitcoin::hashes::hex::ToHex;
 pub use bitcoin::Network;
 use cipher::consts::U32;
@@ -119,7 +119,7 @@ pub struct LightningNode {
     peer_manager: Arc<PeerManager>,
     task_manager: Arc<Mutex<TaskManager>>,
     data_store: Arc<Mutex<DataStore>>,
-    max_routing_fee_provider: Arc<SimpleMaxRoutingFeeProvider>,
+    max_routing_fee_strategy: Arc<SimpleMaxRoutingFeeStrategy>,
 }
 
 impl LightningNode {
@@ -200,13 +200,15 @@ impl LightningNode {
         ));
 
         // Step 13: Initialize the Router
-        let max_routing_fee_provider = Arc::new(SimpleMaxRoutingFeeProvider::new(21_000, 50));
-        let router = Arc::new(FeeCappedRouter::new(
-            Arc::clone(&graph),
-            Arc::clone(&logger),
-            keys_manager.get_secure_random_bytes(),
-            Arc::clone(&scorer),
-            Arc::clone(&max_routing_fee_provider),
+        let max_routing_fee_strategy = Arc::new(SimpleMaxRoutingFeeStrategy::new(21_000, 50));
+        let router = Arc::new(FeeLimitingRouter::new(
+            Router::new(
+                Arc::clone(&graph),
+                Arc::clone(&logger),
+                keys_manager.get_secure_random_bytes(),
+                Arc::clone(&scorer),
+            ),
+            Arc::clone(&max_routing_fee_strategy),
         ));
 
         // (needed when using Electrum or BIP 157/158)
@@ -345,7 +347,7 @@ impl LightningNode {
             peer_manager,
             task_manager,
             data_store,
-            max_routing_fee_provider,
+            max_routing_fee_strategy,
         })
     }
 
@@ -425,9 +427,9 @@ impl LightningNode {
         invoice::decode_invoice(&invoice, network)
     }
 
-    pub fn get_payment_max_fee_strategy(&self, amount_msat: u64) -> MaxFeeStrategy {
-        self.max_routing_fee_provider
-            .get_max_fee_strategy(amount_msat)
+    pub fn get_payment_max_routing_fee_mode(&self, amount_msat: u64) -> MaxRoutingFeeMode {
+        self.max_routing_fee_strategy
+            .get_payment_max_fee_mode(amount_msat)
     }
 
     pub fn pay_invoice(&self, invoice: String, metadata: String) -> PayResult<()> {

eel/src/router.rs

@@ -1,97 +1,68 @@
-use crate::logger::LightningLogger;
-use crate::types::{NetworkGraph, Router, Scorer};
+use crate::types::Router;
 use lightning::ln::channelmanager::{ChannelDetails, PaymentId};
 use lightning::ln::msgs::{ErrorAction, LightningError};
 use lightning::ln::PaymentHash;
 use lightning::routing::router::{InFlightHtlcs, Route, RouteParameters};
 use secp256k1::PublicKey;
-use std::ops::Deref;
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 
-pub(crate) trait MaxRoutingFeeProvider {
-    fn compute_max_fee_msat(&self, payment_amount_msat: u64) -> u64;
-}
-
-pub enum MaxFeeStrategy {
+pub enum MaxRoutingFeeMode {
     Relative { max_fee_permyriad: u16 },
     Absolute { max_fee_msat: u64 },
 }
 
-pub(crate) struct SimpleMaxRoutingFeeProvider {
+pub(crate) struct SimpleMaxRoutingFeeStrategy {
     min_max_fee_msat: u64,
     max_relative_fee_permyriad: u16,
 }
 
-impl SimpleMaxRoutingFeeProvider {
+impl SimpleMaxRoutingFeeStrategy {
     pub fn new(min_max_fee_msat: u64, max_relative_fee_permyriad: u16) -> Self {
-        SimpleMaxRoutingFeeProvider {
+        SimpleMaxRoutingFeeStrategy {
             min_max_fee_msat,
             max_relative_fee_permyriad,
         }
     }
 
-    pub fn get_max_fee_strategy(&self, payment_amount_msat: u64) -> MaxFeeStrategy {
+    pub fn get_payment_max_fee_mode(&self, payment_amount_msat: u64) -> MaxRoutingFeeMode {
         let threshold = self.min_max_fee_msat * 10_000 / self.max_relative_fee_permyriad as u64;
 
         if payment_amount_msat > threshold {
-            MaxFeeStrategy::Relative {
+            MaxRoutingFeeMode::Relative {
                 max_fee_permyriad: self.max_relative_fee_permyriad,
             }
         } else {
-            MaxFeeStrategy::Absolute {
+            MaxRoutingFeeMode::Absolute {
                 max_fee_msat: self.min_max_fee_msat,
             }
         }
     }
-}
 
-impl MaxRoutingFeeProvider for SimpleMaxRoutingFeeProvider {
-    fn compute_max_fee_msat(&self, payment_amount_msat: u64) -> u64 {
-        match self.get_max_fee_strategy(payment_amount_msat) {
-            MaxFeeStrategy::Relative { max_fee_permyriad } => {
+    pub fn compute_max_fee_msat(&self, payment_amount_msat: u64) -> u64 {
+        match self.get_payment_max_fee_mode(payment_amount_msat) {
+            MaxRoutingFeeMode::Relative { max_fee_permyriad } => {
                 payment_amount_msat * max_fee_permyriad as u64 / 10000
             }
-            MaxFeeStrategy::Absolute { max_fee_msat } => max_fee_msat,
+            MaxRoutingFeeMode::Absolute { max_fee_msat } => max_fee_msat,
         }
     }
 }
 
-pub(crate) struct FeeCappedRouter<MFP: Deref>
-where
-    MFP::Target: MaxRoutingFeeProvider,
-{
+pub(crate) struct FeeLimitingRouter {
     inner: Router,
-    max_fee_provider: MFP,
+    max_fee_strategy: Arc<SimpleMaxRoutingFeeStrategy>,
 }
 
-impl<MFP: Deref> FeeCappedRouter<MFP>
-where
-    MFP::Target: MaxRoutingFeeProvider,
-{
-    pub fn new(
-        network_graph: Arc<NetworkGraph>,
-        logger: Arc<LightningLogger>,
-        random_seed_bytes: [u8; 32],
-        scorer: Arc<Mutex<Scorer>>,
-        max_fee_provider: MFP,
-    ) -> Self {
-        let inner = Router::new(
-            Arc::clone(&network_graph),
-            Arc::clone(&logger),
-            random_seed_bytes,
-            Arc::clone(&scorer),
-        );
-        FeeCappedRouter {
-            inner,
-            max_fee_provider,
+impl FeeLimitingRouter {
+    pub fn new(router: Router, max_fee_strategy: Arc<SimpleMaxRoutingFeeStrategy>) -> Self {
+        FeeLimitingRouter {
+            inner: router,
+            max_fee_strategy,
         }
     }
 }
 
-impl<MFP: Deref> lightning::routing::router::Router for FeeCappedRouter<MFP>
-where
-    MFP::Target: MaxRoutingFeeProvider,
-{
+impl lightning::routing::router::Router for FeeLimitingRouter {
     fn find_route(
         &self,
         payer: &PublicKey,
@@ -100,7 +71,7 @@ where
         inflight_htlcs: &InFlightHtlcs,
     ) -> Result<Route, LightningError> {
         let max_fee_msat = self
-            .max_fee_provider
+            .max_fee_strategy
             .compute_max_fee_msat(route_params.final_value_msat);
 
         let route = self
@@ -127,7 +98,7 @@ where
         payment_id: PaymentId,
     ) -> Result<Route, LightningError> {
         let max_fee_msat = self
-            .max_fee_provider
+            .max_fee_strategy
             .compute_max_fee_msat(route_params.final_value_msat);
 
         let route = self.inner.find_route_with_id(
@@ -152,18 +123,18 @@ where
 
 #[cfg(test)]
 mod tests {
-    use crate::router::{MaxRoutingFeeProvider, SimpleMaxRoutingFeeProvider};
+    use crate::router::SimpleMaxRoutingFeeStrategy;
 
     #[test]
-    fn test_simple_max_routing_fee_provider() {
-        let max_fee_provider = SimpleMaxRoutingFeeProvider::new(21000, 50);
-
-        assert_eq!(max_fee_provider.compute_max_fee_msat(0), 21_000);
-        assert_eq!(max_fee_provider.compute_max_fee_msat(21_000), 21_000);
-        assert_eq!(max_fee_provider.compute_max_fee_msat(4199_000), 21_000);
-        assert_eq!(max_fee_provider.compute_max_fee_msat(4200_000), 21_000);
-        assert_eq!(max_fee_provider.compute_max_fee_msat(4201_000), 21_005);
-        assert_eq!(max_fee_provider.compute_max_fee_msat(4399_000), 21_995);
-        assert_eq!(max_fee_provider.compute_max_fee_msat(4400_000), 22_000);
+    fn test_simple_max_routing_fee_strategy() {
+        let max_fee_strategy = SimpleMaxRoutingFeeStrategy::new(21000, 50);
+
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(0), 21_000);
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(21_000), 21_000);
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(4199_000), 21_000);
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(4200_000), 21_000);
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(4201_000), 21_005);
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(4399_000), 21_995);
+        assert_eq!(max_fee_strategy.compute_max_fee_msat(4400_000), 22_000);
     }
 }

eel/src/storage_persister.rs

@@ -5,7 +5,7 @@ use crate::types::{ChainMonitor, ChannelManager, ChannelManagerReadArgs, Network
 use crate::LightningLogger;
 use std::cmp::Ordering;
 
-use crate::router::{FeeCappedRouter, SimpleMaxRoutingFeeProvider};
+use crate::router::FeeLimitingRouter;
 use crate::tx_broadcaster::TxBroadcaster;
 use bitcoin::hash_types::BlockHash;
 use bitcoin::hashes::hex::ToHex;
@@ -252,7 +252,7 @@ impl StoragePersister {
         keys_manager: Arc<KeysManager>,
         fee_estimator: Arc<crate::FeeEstimator>,
         logger: Arc<LightningLogger>,
-        router: Arc<FeeCappedRouter<Arc<SimpleMaxRoutingFeeProvider>>>,
+        router: Arc<FeeLimitingRouter>,
         channel_monitors: Vec<&mut ChannelMonitor<InMemorySigner>>,
         user_config: UserConfig,
         chain_params: ChainParameters,

eel/src/types.rs

@@ -3,7 +3,7 @@ use crate::logger::LightningLogger;
 use crate::storage_persister::StoragePersister;
 use crate::tx_broadcaster::TxBroadcaster;
 
-use crate::router::{FeeCappedRouter, SimpleMaxRoutingFeeProvider};
+use crate::router::FeeLimitingRouter;
 use lightning::chain::chainmonitor::ChainMonitor as LdkChainMonitor;
 use lightning::chain::keysinterface::{InMemorySigner, KeysManager};
 use lightning::ln::peer_handler::IgnoringMessageHandler;
@@ -31,7 +31,7 @@ pub(crate) type ChannelManager = lightning::ln::channelmanager::ChannelManager<
     Arc<KeysManager>,
     Arc<KeysManager>,
     Arc<FeeEstimator>,
-    Arc<FeeCappedRouter<Arc<SimpleMaxRoutingFeeProvider>>>,
+    Arc<FeeLimitingRouter>,
     Arc<LightningLogger>,
 >;
 
@@ -43,7 +43,7 @@ pub(crate) type ChannelManagerReadArgs<'a> = lightning::ln::channelmanager::Chan
     Arc<KeysManager>,
     Arc<KeysManager>,
     Arc<FeeEstimator>,
-    Arc<FeeCappedRouter<Arc<SimpleMaxRoutingFeeProvider>>>,
+    Arc<FeeLimitingRouter>,
     Arc<LightningLogger>,
 >;
 

examples/3l-node/cli.rs

@@ -1,6 +1,6 @@
 use crate::hinter::{CommandHint, CommandHinter};
 
-use uniffi_lipalightninglib::{Amount, MaxFeeStrategy, TzConfig};
+use uniffi_lipalightninglib::{Amount, MaxRoutingFeeMode, TzConfig};
 
 use bitcoin::secp256k1::PublicKey;
 use chrono::offset::FixedOffset;
@@ -92,8 +92,8 @@ pub(crate) fn poll_for_user_input(node: &LightningNode, log_file_path: &str) {
                         println!("{}", message.red());
                     }
                 }
-                "getmaxfeestrategy" => {
-                    if let Err(message) = get_max_fee_strategy(node, &mut words) {
+                "getmaxroutingfeemode" => {
+                    if let Err(message) = get_max_routing_fee_mode(node, &mut words) {
                         println!("{}", message.red());
                     }
                 }
@@ -164,8 +164,8 @@ fn setup_editor(history_path: &Path) -> Editor<CommandHinter, DefaultHistory> {
         "decodeinvoice ",
     ));
     hints.insert(CommandHint::new(
-        "getmaxfeestrategy <payment amount in SAT>",
-        "getmaxfeestrategy ",
+        "getmaxroutingfeemode <payment amount in SAT>",
+        "getmaxroutingfeemode ",
     ));
     hints.insert(CommandHint::new("payinvoice <invoice>", "payinvoice "));
     hints.insert(CommandHint::new(
@@ -198,7 +198,7 @@ fn help() {
     println!();
     println!("  invoice <amount in SAT> [description]");
     println!("  decodeinvoice <invoice>");
-    println!("  getmaxfeestrategy <payment amount in SAT>");
+    println!("  getmaxroutingfeemode <payment amount in SAT>");
     println!("  payinvoice <invoice>");
     println!("  payopeninvoice <invoice> <amount in SAT>");
     println!();
@@ -398,7 +398,7 @@ fn decode_invoice(
     Ok(())
 }
 
-fn get_max_fee_strategy(
+fn get_max_routing_fee_mode(
     node: &LightningNode,
     words: &mut dyn Iterator<Item = &str>,
 ) -> Result<(), String> {
@@ -410,16 +410,16 @@ fn get_max_fee_strategy(
         None => Err("The payment amount in SAT is required".to_string()),
     }?;
 
-    let max_fee_strategy = node.get_payment_max_fee_strategy(amount_argument);
+    let max_fee_strategy = node.get_payment_max_routing_fee_mode(amount_argument);
 
     match max_fee_strategy {
-        MaxFeeStrategy::Relative { max_fee_permyriad } => {
+        MaxRoutingFeeMode::Relative { max_fee_permyriad } => {
             println!(
                 "Max fee strategy: Relative (<= {} %)",
                 max_fee_permyriad as f64 / 100.0
             );
         }
-        MaxFeeStrategy::Absolute { max_fee_amount } => {
+        MaxRoutingFeeMode::Absolute { max_fee_amount } => {
             println!(
                 "Max fee strategy: Absolute (<= {})",
                 amount_to_string(max_fee_amount)

src/lib.rs

@@ -94,7 +94,7 @@ pub struct LightningNode {
     core_node: eel::LightningNode,
 }
 
-pub enum MaxFeeStrategy {
+pub enum MaxRoutingFeeMode {
     Relative { max_fee_permyriad: u16 },
     Absolute { max_fee_amount: Amount },
 }
@@ -212,15 +212,15 @@ impl LightningNode {
         Ok(InvoiceDetails::from_remote_invoice(invoice, &rate))
     }
 
-    pub fn get_payment_max_fee_strategy(&self, amount_sat: u64) -> MaxFeeStrategy {
+    pub fn get_payment_max_routing_fee_mode(&self, amount_sat: u64) -> MaxRoutingFeeMode {
         match self
             .core_node
-            .get_payment_max_fee_strategy(amount_sat * 1000)
+            .get_payment_max_routing_fee_mode(amount_sat * 1000)
         {
-            eel::MaxFeeStrategy::Relative { max_fee_permyriad } => {
-                MaxFeeStrategy::Relative { max_fee_permyriad }
+            eel::MaxRoutingFeeMode::Relative { max_fee_permyriad } => {
+                MaxRoutingFeeMode::Relative { max_fee_permyriad }
             }
-            eel::MaxFeeStrategy::Absolute { max_fee_msat } => MaxFeeStrategy::Absolute {
+            eel::MaxRoutingFeeMode::Absolute { max_fee_msat } => MaxRoutingFeeMode::Absolute {
                 max_fee_amount: max_fee_msat.to_amount_up(&self.get_exchange_rate()),
             },
         }