sim-node: validate channel policy and enforce unique scid

sim-lib/src/sim_node.rs

@@ -122,6 +122,25 @@ pub struct ChannelPolicy {
     pub fee_rate_prop: u64,
 }
 
+impl ChannelPolicy {
+    /// Validates that the channel policy is acceptable for the size of the channel.
+    fn validate(&self, capacity_msat: u64) -> Result<(), SimulationError> {
+        if self.max_in_flight_msat > capacity_msat {
+            return Err(SimulationError::SimulatedNetworkError(format!(
+                "max_in_flight_msat {} > capacity {}",
+                self.max_in_flight_msat, capacity_msat
+            )));
+        }
+        if self.max_htlc_size_msat > capacity_msat {
+            return Err(SimulationError::SimulatedNetworkError(format!(
+                "max_htlc_size_msat {} > capacity {}",
+                self.max_htlc_size_msat, capacity_msat
+            )));
+        }
+        Ok(())
+    }
+}
+
 /// Fails with the forwarding error provided if the value provided fails its inequality check.
 macro_rules! fail_forwarding_inequality {
     ($value_1:expr, $op:tt, $value_2:expr, $error_variant:ident $(, $opt:expr)*) => {
@@ -291,6 +310,43 @@ impl SimulatedChannel {
         }
     }
 
+    /// Validates that a simulated channel has distinct node pairs and valid routing policies.
+    fn validate(&self) -> Result<(), SimulationError> {
+        if self.node_1.policy.pubkey == self.node_2.policy.pubkey {
+            return Err(SimulationError::SimulatedNetworkError(format!(
+                "Channel should have distinct node pubkeys, got: {} for both nodes.",
+                self.node_1.policy.pubkey
+            )));
+        }
+
+        if self.node_1.in_flight_total() != 0 {
+            return Err(SimulationError::SimulatedNetworkError(format!(
+                "Channel for node: {} should have a zero in-flight starting balance",
+                self.node_1.policy.pubkey,
+            )));
+        }
+
+        if self.node_2.in_flight_total() != 0 {
+            return Err(SimulationError::SimulatedNetworkError(format!(
+                "Channel for node: {} should have a zero in-flight starting balance",
+                self.node_2.policy.pubkey,
+            )));
+        }
+
+        if self.node_1.local_balance_msat + self.node_2.local_balance_msat != self.capacity_msat {
+            return Err(SimulationError::SimulatedNetworkError(format!(
+				"Channel does not have consistent balance state: {} (node_1 {}) + {} (node_2 {}) != {}",
+				self.node_1.local_balance_msat, self.node_1.policy.pubkey, self. node_2.local_balance_msat,
+				self.node_2.policy.pubkey,self.capacity_msat,
+			)));
+        }
+
+        self.node_1.policy.validate(self.capacity_msat)?;
+        self.node_2.policy.validate(self.capacity_msat)?;
+
+        Ok(())
+    }
+
     fn get_node_mut(&mut self, pubkey: &PublicKey) -> Result<&mut ChannelState, ForwardingError> {
         if pubkey == &self.node_1.policy.pubkey {
             Ok(&mut self.node_1)
@@ -617,13 +673,25 @@ impl SimGraph {
     pub fn new(
         graph_channels: Vec<SimulatedChannel>,
         shutdown_trigger: Trigger,
-    ) -> Result<Self, LdkError> {
+    ) -> Result<Self, SimulationError> {
         let mut nodes: HashMap<PublicKey, Vec<u64>> = HashMap::new();
         let mut channels = HashMap::new();
 
         for channel in graph_channels.iter() {
-            channels.insert(channel.short_channel_id, channel.clone());
+            // Assert that the channel is valid and that its short channel ID is unique within the simulation, required
+            // because we use scid to identify the channel.
+            channel.validate()?;
+            match channels.entry(channel.short_channel_id) {
+                Entry::Occupied(_) => {
+                    return Err(SimulationError::SimulatedNetworkError(format!(
+                        "Simulated short channel ID should be unique: {} duplicated",
+                        channel.short_channel_id
+                    )))
+                },
+                Entry::Vacant(v) => v.insert(channel.clone()),
+            };
 
+            // It's okay to have duplicate pubkeys because one node can have many channels.
             for pubkey in [channel.node_1.policy.pubkey, channel.node_2.policy.pubkey] {
                 match nodes.entry(pubkey) {
                     Entry::Occupied(o) => o.into_mut().push(channel.capacity_msat),
@@ -1769,4 +1837,50 @@ mod tests {
         test_kit.shutdown.trigger();
         test_kit.graph.wait_for_shutdown().await;
     }
+
+    #[tokio::test]
+    async fn test_validate_simulated_channel() {
+        // Create a test channel and mutate various parts of it to test validation. Since we just have error strings,
+        // we assert state is okay, mutate to check an error and repeat rather than bothering with string matching.
+        let capacity_mast = 100_000;
+        let mut channel_vec = create_simulated_channels(1, capacity_mast);
+        let channel = &mut channel_vec[0];
+
+        assert_eq!(channel.validate().is_ok(), true);
+
+        // In flight balance is not allowed.
+        let payment_hash = PaymentHash([0; 32]);
+        let htlc = Htlc {
+            amount_msat: 1,
+            cltv_expiry: 10,
+        };
+
+        channel.node_1.in_flight.insert(payment_hash, htlc);
+        assert_eq!(channel.validate().is_err(), true);
+
+        channel.node_1.in_flight.remove(&payment_hash);
+        assert_eq!(channel.validate().is_ok(), true);
+
+        channel.node_2.in_flight.insert(payment_hash, htlc);
+        assert_eq!(channel.validate().is_err(), true);
+
+        channel.node_2.in_flight.remove(&payment_hash);
+        assert_eq!(channel.validate().is_ok(), true);
+
+        // Sane capacity.
+        let original_balance = channel.node_1.local_balance_msat;
+        channel.node_1.local_balance_msat = original_balance * 2;
+        assert_eq!(channel.validate().is_err(), true);
+
+        channel.node_1.local_balance_msat = original_balance - 1;
+        assert_eq!(channel.validate().is_err(), true);
+
+        channel.node_1.local_balance_msat = original_balance;
+        assert_eq!(channel.validate().is_ok(), true);
+
+        // Pubkeys don't match.
+        let node_1 = channel.node_1.policy.pubkey;
+        channel.node_2.policy.pubkey = node_1;
+        assert_eq!(channel.validate().is_err(), true);
+    }
 }