diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index dafeffe98bf..565cfa051b0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5132,7 +5132,7 @@ where chain_hash: self.chain_hash, short_channel_id, timestamp: chan.context.get_update_time_counter(), - message_flags: 1, // Only must_be_one + message_flags: 1 | if !chan.context.should_announce() { 1 << 1 } else { 0 }, // must_be_one + dont_forward channel_flags: (!were_node_one) as u8 | ((!enabled as u8) << 1), cltv_expiry_delta: chan.context.get_cltv_expiry_delta(), htlc_minimum_msat: chan.context.get_counterparty_htlc_minimum_msat(), @@ -12290,6 +12290,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Some((cp_id, chan_id)) => (cp_id.clone(), chan_id.clone()), None => { // It's not a local channel + if msg.contents.message_flags & (1 << 1) != 0 { + log_debug!(self.logger, "Received channel_update for unknown channel {} with dont_forward set. You may wish to check if an incorrect tx_index was passed to chain::Confirm::transactions_confirmed.", msg.contents.short_channel_id); + } return Ok(NotifyOption::SkipPersistNoEvents) } }; diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 83aaca24203..14a343814e2 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -1520,3 +1520,133 @@ fn test_0conf_ann_sigs_racing_conf() { let as_announcement = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(as_announcement.len(), 1); } + +#[test] +fn test_channel_update_dont_forward_flag() { + // Test that the `dont_forward` bit (bit 1 of message_flags) is set correctly: + // - For private channels: message_flags should have bit 1 set (value 3 = must_be_one + dont_forward) + // - For public channels: message_flags should NOT have bit 1 set (value 1 = must_be_one only) + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + let node_b_id = nodes[1].node.get_our_node_id(); + let node_c_id = nodes[2].node.get_our_node_id(); + + // Create a public (announced) channel between nodes[0] and nodes[1] + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000); + + // Create a private (unannounced) channel between nodes[1] and nodes[2] + create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 500_000_000); + + // Get the channel details for both channels + let public_channel = nodes[0] + .node + .list_channels() + .into_iter() + .find(|c| c.counterparty.node_id == node_b_id) + .unwrap(); + let private_channel = nodes[1] + .node + .list_channels() + .into_iter() + .find(|c| c.counterparty.node_id == node_c_id) + .unwrap(); + + // Verify is_announced correctly reflects the channel type + assert!(public_channel.is_announced, "Public channel should have is_announced = true"); + assert!(!private_channel.is_announced, "Private channel should have is_announced = false"); + + // Trigger channel_update by changing config on the public channel + let mut new_config = public_channel.config.unwrap(); + new_config.forwarding_fee_base_msat += 10; + nodes[0] + .node + .update_channel_config(&node_b_id, &[public_channel.channel_id], &new_config) + .unwrap(); + + // Get the channel_update for the public channel and verify dont_forward is NOT set + let events = nodes[0].node.get_and_clear_pending_msg_events(); + let public_channel_update = events + .iter() + .find_map(|e| { + if let MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } = e { + Some(msg.clone()) + } else { + None + } + }) + .expect("Expected BroadcastChannelUpdate for public channel"); + // message_flags should be 1 (only must_be_one bit set, dont_forward NOT set) + assert_eq!( + public_channel_update.contents.message_flags & (1 << 1), + 0, + "Public channel update should NOT have dont_forward bit set" + ); + assert_eq!( + public_channel_update.contents.message_flags & 1, + 1, + "Public channel update should have must_be_one bit set" + ); + + // Trigger channel_update by changing config on the private channel + let mut new_config = private_channel.config.unwrap(); + new_config.forwarding_fee_base_msat += 10; + nodes[1] + .node + .update_channel_config(&node_c_id, &[private_channel.channel_id], &new_config) + .unwrap(); + + // Get the channel_update for the private channel and verify dont_forward IS set + let private_channel_update = + get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, node_c_id); + // message_flags should have dont_forward bit set + assert_ne!( + private_channel_update.contents.message_flags & (1 << 1), + 0, + "Private channel update should have dont_forward bit set" + ); + assert_eq!( + private_channel_update.contents.message_flags & 1, + 1, + "Private channel update should have must_be_one bit set" + ); +} + +#[test] +fn test_unknown_channel_update_with_dont_forward_logs_debug() { + use bitcoin::constants::ChainHash; + use bitcoin::secp256k1::ecdsa::Signature; + use bitcoin::secp256k1::ffi::Signature as FFISignature; + use bitcoin::Network; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let unknown_scid = 42; + let msg = msgs::ChannelUpdate { + signature: Signature::from(unsafe { FFISignature::new() }), + contents: msgs::UnsignedChannelUpdate { + chain_hash: ChainHash::using_genesis_block(Network::Testnet), + short_channel_id: unknown_scid, + timestamp: 0, + message_flags: 1 | (1 << 1), // must_be_one + dont_forward + channel_flags: 0, + cltv_expiry_delta: 0, + htlc_minimum_msat: 0, + htlc_maximum_msat: msgs::MAX_VALUE_MSAT, + fee_base_msat: 0, + fee_proportional_millionths: 0, + excess_data: Vec::new(), + }, + }; + + nodes[0].node.handle_channel_update(nodes[1].node.get_our_node_id(), &msg); + nodes[0].logger.assert_log_contains( + "lightning::ln::channelmanager", + "Received channel_update for unknown channel", + 1, + ); +} diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 534bebe7618..29053bb6984 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -568,6 +568,14 @@ where fn handle_channel_update( &self, _their_node_id: Option, msg: &msgs::ChannelUpdate, ) -> Result, LightningError> { + // Ignore channel updates with dont_forward bit set - these are for private channels + // and shouldn't be gossiped or stored in the network graph + if msg.contents.message_flags & (1 << 1) != 0 { + return Err(LightningError { + err: "Ignoring channel_update with dont_forward bit set".to_owned(), + action: ErrorAction::IgnoreAndLog(Level::Debug), + }); + } match self.network_graph.update_channel(msg) { Ok(nodes) if msg.contents.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY => Ok(nodes), Ok(_) => Ok(None), @@ -3341,6 +3349,65 @@ pub(crate) mod tests { }; } + #[test] + fn handling_channel_update_with_dont_forward_flag() { + // Test that channel updates with the dont_forward bit set are rejected + let secp_ctx = Secp256k1::new(); + let logger = test_utils::TestLogger::new(); + let chain_source = test_utils::TestChainSource::new(Network::Testnet); + let network_graph = NetworkGraph::new(Network::Testnet, &logger); + let gossip_sync = P2PGossipSync::new(&network_graph, Some(&chain_source), &logger); + + let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap(); + let node_1_pubkey = PublicKey::from_secret_key(&secp_ctx, node_1_privkey); + let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap(); + + // First announce a channel so we have something to update + let good_script = get_channel_script(&secp_ctx); + *chain_source.utxo_ret.lock().unwrap() = UtxoResult::Sync(Ok(TxOut { + value: Amount::from_sat(1000_000), + script_pubkey: good_script.clone(), + })); + + let valid_channel_announcement = + get_signed_channel_announcement(|_| {}, node_1_privkey, node_2_privkey, &secp_ctx); + gossip_sync + .handle_channel_announcement(Some(node_1_pubkey), &valid_channel_announcement) + .unwrap(); + + // Create a channel update with dont_forward bit set (bit 1 of message_flags) + let dont_forward_update = get_signed_channel_update( + |unsigned_channel_update| { + unsigned_channel_update.message_flags = 1 | (1 << 1); // must_be_one + dont_forward + }, + node_1_privkey, + &secp_ctx, + ); + + // The update should be rejected because dont_forward is set + match gossip_sync.handle_channel_update(Some(node_1_pubkey), &dont_forward_update) { + Ok(_) => panic!("Expected channel update with dont_forward to be rejected"), + Err(e) => { + assert_eq!(e.err, "Ignoring channel_update with dont_forward bit set"); + match e.action { + crate::ln::msgs::ErrorAction::IgnoreAndLog(level) => { + assert_eq!(level, crate::util::logger::Level::Debug) + }, + _ => panic!("Expected IgnoreAndLog action"), + } + }, + }; + + // Verify the update was not applied to the network graph + let channels = network_graph.read_only(); + let channel = + channels.channels().get(&valid_channel_announcement.contents.short_channel_id).unwrap(); + assert!( + channel.one_to_two.is_none(), + "Channel update with dont_forward should not be stored in network graph" + ); + } + #[test] fn handling_network_update() { let logger = test_utils::TestLogger::new();