Repository: breez/breez-sdk-greenlight (language: python)

Issue:
Route hints may contain local fees instead of remote
If greenlight doesn't include the lsp route hint in a normal invoice (no channel open), we currently add the _local_ routing fees in the route hint rather than the _remote_ routing fees.

Relevant code here:
https://github.com/breez/breez-sdk/blob/2ff9646d03eac9f737834fdaeedc871d640d9649/libs/sdk-core/src/greenlight/node_api.rs#L1693-L1696

The code there has to be updated to use the remote policy (another call to the cln api `listchannels` api probably) to include the remote policies.
Another, additional, option can be to replace routing fees with the fees coming from `lsp_information` if the hint was not in the invoice originally, but it is included in the `get_routing_hints` call.


Discussion / hints:

Fix patch:
diff --git a/libs/sdk-core/src/breez_services.rs b/libs/sdk-core/src/breez_services.rs
index b530345c5..35cdcd980 100644
--- a/libs/sdk-core/src/breez_services.rs
+++ b/libs/sdk-core/src/breez_services.rs
@@ -2579,7 +2579,7 @@ impl PaymentReceiver {
         lsp_info: &LspInformation,
     ) -> Result<String, ReceivePaymentError> {
         info!("Getting routing hints from node");
-        let (mut hints, has_public_channel) = self.node_api.get_routing_hints().await?;
+        let (mut hints, has_public_channel) = self.node_api.get_routing_hints(lsp_info).await?;
         if !has_public_channel && hints.is_empty() {
             return Err(ReceivePaymentError::InvoiceNoRoutingHints {
                 err: "Must have at least one active channel".into(),
diff --git a/libs/sdk-core/src/greenlight/node_api.rs b/libs/sdk-core/src/greenlight/node_api.rs
index 654e5d83c..98a9af369 100644
--- a/libs/sdk-core/src/greenlight/node_api.rs
+++ b/libs/sdk-core/src/greenlight/node_api.rs
@@ -47,9 +47,9 @@ use crate::bitcoin::{
 use crate::invoice::{parse_invoice, validate_network, InvoiceError, RouteHintHop};
 use crate::lightning::util::message_signing::verify;
 use crate::lightning_invoice::{RawBolt11Invoice, SignedRawBolt11Invoice};
-use crate::models::*;
 use crate::node_api::{CreateInvoiceRequest, FetchBolt11Result, NodeAPI, NodeError, NodeResult};
 use crate::persist::db::SqliteStorage;
+use crate::{models::*, LspInformation};
 use crate::{
     NodeConfig, PrepareRedeemOnchainFundsRequest, PrepareRedeemOnchainFundsResponse, RouteHint,
 };
@@ -1653,16 +1653,20 @@ impl NodeAPI for Greenlight {
     }
 
     // Gets the routing hints related to all private channels that the node has
-    async fn get_routing_hints(&self) -> NodeResult<(Vec<RouteHint>, bool)> {
+    async fn get_routing_hints(
+        &self,
+        lsp_info: &LspInformation,
+    ) -> NodeResult<(Vec<RouteHint>, bool)> {
         let mut hints: Vec<RouteHint> = vec![];
         let mut node_client = self.get_node_client().await?;
-        let channels = node_client
+        // Get the peer channels
+        let peer_channels = node_client
             .list_peer_channels(cln::ListpeerchannelsRequest::default())
             .await?
             .into_inner();
 
         let mut has_public_channel = false;
-        let mut open_channels: Vec<cln::ListpeerchannelsChannels> = channels
+        let mut open_peer_channels: Vec<cln::ListpeerchannelsChannels> = peer_channels
             .channels
             .into_iter()
             .filter(|c| {
@@ -1671,38 +1675,90 @@ impl NodeAPI for Greenlight {
                 is_private && c.state == Some(cln::ChannelState::ChanneldNormal as i32)
             })
             .collect();
+        // Get channels where our node is the destination
+        let pubkey = self
+            .persister
+            .get_node_state()?
+            .map(|n| n.id)
+            .ok_or(NodeError::generic("Node info not found"))?;
+        let mut channels = node_client
+            .list_channels(cln::ListchannelsRequest {
+                destination: Some(hex::decode(pubkey)?),
+                ..Default::default()
+            })
+            .await?
+            .into_inner()
+            .channels;
 
         // Ensure one private channel from each peer.
-        open_channels.dedup_by_key(|c| c.peer_id.clone());
-
-        // Ceate a routing hint from each channel.
-        for c in open_channels {
-            let (alias_remote, _) = match c.alias {
+        open_peer_channels.dedup_by_key(|c| c.peer_id.clone());
+        channels.dedup_by_key(|c| c.source.clone());
+
+        // Create a routing hint from each channel.
+        for peer_channel in open_peer_channels {
+            let peer_id = peer_channel.clone().peer_id.ok_or(anyhow!("No peer id"))?;
+            let peer_id_str = hex::encode(peer_id.clone());
+            let (alias_remote, _) = match peer_channel.alias {
                 Some(a) => (a.remote.clone(), a.local.clone()),
                 None => (None, None),
             };
 
-            let optional_channel_id = alias_remote.clone().or(c.short_channel_id.clone());
+            let optional_channel_id = alias_remote
+                .clone()
+                .or(peer_channel.short_channel_id.clone());
 
             if let Some(channel_id) = optional_channel_id {
-                let scid = parse_short_channel_id(&channel_id)?;
-                let hint = RouteHint {
-                    hops: vec![RouteHintHop {
-                        src_node_id: hex::encode(c.peer_id.ok_or(anyhow!("no peer id"))?),
-                        short_channel_id: scid,
-                        fees_base_msat: c.fee_base_msat.clone().unwrap_or_default().msat as u32,
-                        fees_proportional_millionths: c
-                            .fee_proportional_millionths
-                            .unwrap_or_default(),
-                        cltv_expiry_delta: 144,
-                        htlc_minimum_msat: Some(
-                            c.minimum_htlc_in_msat.clone().unwrap_or_default().msat,
-                        ),
-                        htlc_maximum_msat: None,
-                    }],
+                // The local fee policy
+                let peer_fee_base_msat = peer_channel.fee_base_msat.unwrap_or_default().msat as u32;
+                let peer_fees_proportional_millionths =
+                    peer_channel.fee_proportional_millionths.unwrap_or_default();
+                // The remote fee policy
+                let maybe_channel = channels.clone().into_iter().find(|c| c.source == peer_id);
+                let (maybe_fees_base_msat, maybe_fees_proportional_millionths) = match maybe_channel
+                {
+                    Some(channel) => (
+                        Some(channel.base_fee_millisatoshi),
+                        Some(channel.fee_per_millionth),
+                    ),
+                    None if peer_id_str == lsp_info.pubkey => (
+                        Some(lsp_info.base_fee_msat as u32),
+                        Some((lsp_info.fee_rate * 1000000.0) as u32),
+                    ),
+                    _ => (None, None),
+                };
+                match (maybe_fees_base_msat, maybe_fees_proportional_millionths) {
+                    (Some(fees_base_msat), Some(fees_proportional_millionths)) => {
+                        debug!(
+                            "For peer {:?}: local base {:?} proportional {:?}, remote base {:?} proportional {:?}",
+                            peer_id_str,
+                            peer_fee_base_msat,
+                            peer_fees_proportional_millionths,
+                            fees_base_msat,
+                            fees_proportional_millionths
+                        );
+                        let scid = parse_short_channel_id(&channel_id)?;
+                        let hint = RouteHint {
+                            hops: vec![RouteHintHop {
+                                src_node_id: peer_id_str,
+                                short_channel_id: scid,
+                                fees_base_msat,
+                                fees_proportional_millionths,
+                                cltv_expiry_delta: 144,
+                                htlc_minimum_msat: Some(
+                                    peer_channel
+                                        .minimum_htlc_in_msat
+                                        .clone()
+                                        .unwrap_or_default()
+                                        .msat,
+                                ),
+                                htlc_maximum_msat: None,
+                            }],
+                        };
+                        info!("Generating hint hop as routing hint: {:?}", hint);
+                        hints.push(hint);
+                    }
+                    _ => debug!("No source channel found for peer: {:?}", peer_id_str),
                 };
-                info!("Generating hint hop as routing hint: {:?}", hint);
-                hints.push(hint);
             }
         }
         Ok((hints, has_public_channel))
diff --git a/libs/sdk-core/src/node_api.rs b/libs/sdk-core/src/node_api.rs
index 03a790db5..a02c065c0 100644
--- a/libs/sdk-core/src/node_api.rs
+++ b/libs/sdk-core/src/node_api.rs
@@ -10,7 +10,7 @@ use crate::{
     invoice::InvoiceError,
     lightning_invoice::RawBolt11Invoice,
     persist::error::PersistError,
-    CustomMessage, MaxChannelAmount, NodeCredentials, Payment, PaymentResponse,
+    CustomMessage, LspInformation, MaxChannelAmount, NodeCredentials, Payment, PaymentResponse,
     PrepareRedeemOnchainFundsRequest, PrepareRedeemOnchainFundsResponse, RouteHint, RouteHintHop,
     SyncResponse, TlvEntry,
 };
@@ -155,5 +155,8 @@ pub trait NodeAPI: Send + Sync {
 
     // Gets the routing hints related to all private channels that the node has.
     // Also returns a boolean indicating if the node has a public channel or not.
-    async fn get_routing_hints(&self) -> NodeResult<(Vec<RouteHint>, bool)>;
+    async fn get_routing_hints(
+        &self,
+        lsp_info: &LspInformation,
+    ) -> NodeResult<(Vec<RouteHint>, bool)>;
 }


Test patch:
diff --git a/libs/sdk-core/src/test_utils.rs b/libs/sdk-core/src/test_utils.rs
index 191a86cad..605940201 100644
--- a/libs/sdk-core/src/test_utils.rs
+++ b/libs/sdk-core/src/test_utils.rs
@@ -29,7 +29,10 @@ use crate::breez_services::{OpenChannelParams, Receiver};
 use crate::chain::{ChainService, OnchainTx, Outspend, RecommendedFees, TxStatus};
 use crate::error::{ReceivePaymentError, SdkError, SdkResult};
 use crate::fiat::{FiatCurrency, Rate};
-use crate::grpc::{PaymentInformation, RegisterPaymentNotificationResponse, RegisterPaymentReply, RemovePaymentNotificationResponse};
+use crate::grpc::{
+    PaymentInformation, RegisterPaymentNotificationResponse, RegisterPaymentReply,
+    RemovePaymentNotificationResponse,
+};
 use crate::invoice::{InvoiceError, InvoiceResult};
 use crate::lightning::ln::PaymentSecret;
 use crate::lightning_invoice::{Currency, InvoiceBuilder, RawBolt11Invoice};
@@ -497,7 +500,10 @@ impl NodeAPI for MockNodeAPI {
         ))
     }
 
-    async fn get_routing_hints(&self) -> NodeResult<(Vec<RouteHint>, bool)> {
+    async fn get_routing_hints(
+        &self,
+        _lsp_info: &LspInformation,
+    ) -> NodeResult<(Vec<RouteHint>, bool)> {
         Ok((vec![], false))
     }