Fix critical bugs and harden error handling for production readiness

barq-common/src/algorithms/probabilistic/mod.rs

@@ -1,6 +1,7 @@
 use core::ops::Deref;
 use std::str::FromStr;
 use std::sync::Arc;
+use std::time::SystemTime;
 
 use anyhow::Result;
 use reqwest::blocking;
@@ -40,7 +41,7 @@ impl LDKRoutingStrategy {
         &self,
         graph: &dyn NetworkGraph,
     ) -> anyhow::Result<LdkNetworkGraph<Arc<LampoLogger>>> {
-        let ldkgraph = LdkNetworkGraph::new(self.network.clone(), self.logger.clone());
+        let ldkgraph = LdkNetworkGraph::new(self.network, self.logger.clone());
 
         for channel in graph.get_channels() {
             // FIXME: we need to set the annouce message insie the channel struct
@@ -56,40 +57,55 @@ impl LDKRoutingStrategy {
         Ok(ldkgraph)
     }
 
-    fn construct_route_params(input: &RouteInput) -> RouteParameters {
-        // SAFETY: safe to unwrap because it should be a valid pub key
-        let payment_params = PaymentParameters::from_node_id(
-            PublicKey::from_str(&input.dest_pubkey).unwrap(),
-            input.cltv as u32,
-        );
-        RouteParameters::from_payment_params_and_value(payment_params, input.amount_msat)
+    fn construct_route_params(input: &RouteInput) -> anyhow::Result<RouteParameters> {
+        let dest_pubkey = PublicKey::from_str(&input.dest_pubkey).map_err(|e| {
+            anyhow::anyhow!("Invalid destination pubkey '{}': {}", input.dest_pubkey, e)
+        })?;
+        let payment_params = PaymentParameters::from_node_id(dest_pubkey, input.cltv as u32);
+        Ok(RouteParameters::from_payment_params_and_value(
+            payment_params,
+            input.amount_msat,
+        ))
     }
 
-    fn convert_route_to_output(route: Route) -> RouteOutput {
-        let path = route.paths.first().expect("No LDK path available");
-        let mut amt_to_forward = 0;
-        let mut delay = 0;
-
-        let output_path: Vec<RouteHop> = path
-            .hops
-            .iter()
-            .rev()
-            .map(|hop| {
-                amt_to_forward += hop.fee_msat;
-                delay += hop.cltv_expiry_delta;
-
-                RouteHop::new(
-                    hop.pubkey.to_string(),
-                    hop.short_channel_id.to_string(),
-                    delay,
-                    amt_to_forward,
-                )
-            })
-            .collect();
-
-        RouteOutput {
-            path: output_path.into_iter().rev().collect(),
+    fn convert_route_to_output(route: Route) -> anyhow::Result<RouteOutput> {
+        let path = route
+            .paths
+            .first()
+            .ok_or_else(|| anyhow::anyhow!("No LDK path available in route result"))?;
+
+        let hops = &path.hops;
+        let num_hops = hops.len();
+        let mut output_path = Vec::with_capacity(num_hops);
+
+        // Build amounts and delays from destination back to source.
+        // The last hop forwards the final payment amount (its fee_msat IS the amount).
+        // Each preceding hop forwards: next_hop_amount + next_hop_fee.
+        // CLTV delay accumulates from the destination backwards.
+        let mut amounts: Vec<u64> = vec![0; num_hops];
+        let mut delays: Vec<u32> = vec![0; num_hops];
+
+        // Last hop: amount to forward is the fee_msat field (which for the
+        // final hop equals the payment amount), delay is the final CLTV.
+        amounts[num_hops - 1] = hops[num_hops - 1].fee_msat;
+        delays[num_hops - 1] = hops[num_hops - 1].cltv_expiry_delta;
+
+        // Walk backwards from second-to-last hop
+        for i in (0..num_hops - 1).rev() {
+            amounts[i] = amounts[i + 1] + hops[i].fee_msat;
+            delays[i] = delays[i + 1] + hops[i].cltv_expiry_delta;
+        }
+
+        for (i, hop) in hops.iter().enumerate() {
+            output_path.push(RouteHop::new(
+                hop.pubkey.to_string(),
+                hop.short_channel_id.to_string(),
+                delays[i],
+                amounts[i],
+            ));
         }
+
+        Ok(RouteOutput { path: output_path })
     }
 
     fn rapid_gossip_sync_network(
@@ -144,7 +160,7 @@ impl Strategy for LDKRoutingStrategy {
     fn route(&self, input: &RouteInput) -> Result<RouteOutput> {
         let our_node_pubkey = PublicKey::from_str(&input.src_pubkey)
             .map_err(|_| anyhow::anyhow!("Failed to parse source pubkey"))?;
-        let route_params = Self::construct_route_params(input);
+        let route_params = Self::construct_route_params(input)?;
 
         let ldk_graph = if input.use_rapid_gossip_sync {
             self.rapid_gossip_sync_network(input.network)?
@@ -157,8 +173,23 @@ impl Strategy for LDKRoutingStrategy {
         let feeparams = ProbabilisticScoringFeeParameters::default();
         let scorer = ProbabilisticScorer::new(parms, &ldk_graph, self.logger.clone());
 
-        // FIXME: Implement the logic to generate random seed bytes
-        let random_seed_bytes = [0; 32];
+        // Use time-based entropy for route randomization.
+        // This prevents deterministic routing which could be exploited by
+        // an adversary to predict payment paths.
+        let seed_time = SystemTime::now()
+            .duration_since(SystemTime::UNIX_EPOCH)
+            .unwrap_or_default()
+            .as_nanos();
+        let mut random_seed_bytes = [0u8; 32];
+        let time_bytes = seed_time.to_le_bytes();
+        random_seed_bytes[..16].copy_from_slice(&time_bytes);
+        // Mix in the payment amount and source pubkey for additional entropy
+        let amt_bytes = input.amount_msat.to_le_bytes();
+        random_seed_bytes[16..24].copy_from_slice(&amt_bytes);
+        let src_bytes = input.src_pubkey.as_bytes();
+        for (i, byte) in src_bytes.iter().take(8).enumerate() {
+            random_seed_bytes[24 + i] = *byte;
+        }
 
         let route = find_route(
             &our_node_pubkey,
@@ -173,20 +204,19 @@ impl Strategy for LDKRoutingStrategy {
         // FIXME: we are losing context, we should return an better error for the plugin
         .map_err(|e| anyhow::anyhow!("Failed to find route: {:?}", e))?;
 
-        Ok(Self::convert_route_to_output(route))
+        Self::convert_route_to_output(route)
     }
 }
 
 #[cfg(test)]
 mod tests {
 
     use super::*;
-    use lampo_common::ldk::util::logger::{Logger, Record};
 
     #[test]
     fn test_rapid_gossip_sync_network_sanity() {
         let network = Network::Bitcoin;
-        let strategy = LDKRoutingStrategy::new(network.clone(), "/tmp".to_string());
+        let strategy = LDKRoutingStrategy::new(network, "/tmp".to_string());
         let result = strategy.rapid_gossip_sync_network(network);
 
         assert!(
@@ -199,7 +229,7 @@ mod tests {
     #[test]
     fn test_rapid_gossip_sync_network_testnet() {
         let network = Network::Testnet;
-        let strategy = LDKRoutingStrategy::new(network.clone(), "/tmp".to_string());
+        let strategy = LDKRoutingStrategy::new(network, "/tmp".to_string());
         let result = strategy.rapid_gossip_sync_network(network);
 
         assert!(
@@ -212,7 +242,7 @@ mod tests {
     #[test]
     fn test_rapid_gossip_sync_network_not_empty() {
         let network = Network::Testnet;
-        let strategy = LDKRoutingStrategy::new(network.clone(), "/tmp".to_string());
+        let strategy = LDKRoutingStrategy::new(network, "/tmp".to_string());
         let graph = strategy.rapid_gossip_sync_network(network).unwrap();
 
         let read_only_graph = graph.read_only();

barq-common/src/graph.rs

@@ -77,19 +77,25 @@ impl Channel {
 
 impl From<GossipChannel> for Channel {
     fn from(value: GossipChannel) -> Self {
-        // FIXME: we should encode the channel id
+        let capacity = value.satoshi.unwrap_or(0);
         let mut val = Self::new(
             &hex::encode(value.inner.short_channel_id),
             &hex::encode(value.inner.node_id_1),
             &hex::encode(value.inner.node_id_2),
-            value.satoshi.unwrap(),
+            capacity,
             0,
             0,
             0,
         );
         let mut buffer = Vec::new();
-        value.inner.to_wire(&mut buffer).unwrap();
-        val.channel_announcement = Some(buffer);
+        if let Ok(()) = value.inner.to_wire(&mut buffer) {
+            val.channel_announcement = Some(buffer);
+        } else {
+            log::warn!(
+                "Failed to serialize channel announcement for channel {}",
+                hex::encode(value.inner.short_channel_id)
+            );
+        }
         val
     }
 }

barq-common/src/strategy.rs

@@ -7,8 +7,9 @@ use lampo_common::conf::Network;
 
 use crate::graph::NetworkGraph;
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, Default, PartialEq, Eq, Clone)]
 pub enum StrategyKind {
+    #[default]
     Direct,
     Probabilistic,
 }
@@ -25,12 +26,6 @@ impl FromStr for StrategyKind {
     }
 }
 
-impl Default for StrategyKind {
-    fn default() -> Self {
-        Self::Direct
-    }
-}
-
 /// The `Strategy` trait defines an interface for routing strategies used within
 /// Barq.
 ///

barq-plugin/src/methods/graph/p2p.rs

@@ -42,14 +42,21 @@ impl P2PNetworkGraph {
     pub fn add_channel(&mut self, channel: Channel) {
         self.channels
             .insert(channel.short_channel_id.clone(), channel.clone());
-        self.nodes
-            .get_mut(&channel.node1)
-            .unwrap_or(&mut Node::new(&channel.node1))
-            .add_channel(&channel);
-        self.nodes
-            .get_mut(&channel.node2)
-            .unwrap_or(&mut Node::new(&channel.node1))
-            .add_channel(&channel);
+        if let Some(node1) = self.nodes.get_mut(&channel.node1) {
+            node1.add_channel(&channel);
+        } else {
+            let mut new_node = Node::new(&channel.node1);
+            new_node.add_channel(&channel);
+            self.nodes.insert(channel.node1.clone(), new_node);
+        }
+
+        if let Some(node2) = self.nodes.get_mut(&channel.node2) {
+            node2.add_channel(&channel);
+        } else {
+            let mut new_node = Node::new(&channel.node2);
+            new_node.add_channel(&channel);
+            self.nodes.insert(channel.node2.clone(), new_node);
+        }
     }
 }
 

barq-plugin/src/methods/pay.rs

@@ -137,10 +137,16 @@ pub fn barq_pay(
 
     let amount = match (b11.amount_msat, request.amount_msat) {
         (Some(_), Some(_)) => {
-            return Err(error!("barqpay execution failed: amount_msat not required"))
+            return Err(error!(
+                "barqpay execution failed: amount_msat specified in both invoice and request, provide only one"
+            ))
         }
         (Some(amount), None) | (None, Some(amount)) => amount,
-        (None, None) => return Err(error!("barqpay execution failed: amount_msat not required")),
+        (None, None) => {
+            return Err(error!(
+                "barqpay execution failed: amount_msat is required but missing from both invoice and request"
+            ))
+        }
     };
 
     let node_network = node_info.network;
@@ -175,7 +181,11 @@ pub fn barq_pay(
         StrategyKind::Direct => Box::new(Direct::new()),
         StrategyKind::Probabilistic => Box::new(LDKRoutingStrategy::new(
             node_network,
-            plugin.state.cln_rpc_path.clone().unwrap(),
+            plugin
+                .state
+                .cln_rpc_path
+                .clone()
+                .ok_or_else(|| error!("CLN RPC path not initialized"))?,
         )),
     };
 

barq-plugin/src/plugin.rs

@@ -1,6 +1,6 @@
 //! Barq Plugin implementation
 
-use clightningrpc_common::errors::{Error, RpcError};
+use clightningrpc_common::errors::RpcError;
 use serde::de::DeserializeOwned;
 use serde::Serialize;
 use serde_json::Value;
@@ -84,7 +84,10 @@ pub fn build_plugin() -> anyhow::Result<Plugin<State>> {
 
 /// This method is called when the plugin is initialized
 fn on_init(plugin: &mut Plugin<State>) -> Value {
-    let config = plugin.configuration.clone().unwrap();
+    let Some(config) = plugin.configuration.clone() else {
+        log::error!("Plugin configuration is missing during init");
+        return serde_json::json!({"disable": "Plugin configuration missing"});
+    };
     let rpc_file = format!("{}/{}", config.lightning_dir, config.rpc_file);
     plugin.state.network = Some(config.network);
     plugin.state.cln_rpc_path = Some(rpc_file);