From ec9db029eabb18d5b676eaf1e4a2d2650b69fed0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Viktor=20Tigerstr=C3=B6m?= <11711198+ViktorTigerstrom@users.noreply.github.com> Date: Fri, 19 Aug 2022 00:54:47 +0200 Subject: [PATCH] Consider `channel_id`s in `short_to_chan_info` as unguaranteed As the `short_to_chan_info` map has been removed from the `channel_state`, there is no longer any consistency guarantees between the `by_id` and `short_to_chan_info` maps. This commit ensures that we don't force unwrap channels where the channel_id has been queried from the `short_to_chan_info` map. --- lightning/src/ln/channelmanager.rs | 34 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 98ac04406..1dfddf940 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2317,7 +2317,14 @@ impl ChannelManager Some(chan_id.clone()), }; let chan_update_opt = if let Some(forwarding_id) = forwarding_id_opt { - let chan = channel_state.by_id.get_mut(&forwarding_id).unwrap(); + let chan = match channel_state.by_id.get_mut(&forwarding_id) { + None => { + // Channel was removed. The short_to_chan_info and by_id maps have + // no consistency guarantees. + break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None)); + }, + Some(chan) => chan + }; if !chan.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels { // Note that the behavior here should be identical to the above block - we // should NOT reveal the existence or non-existence of a private channel if @@ -2544,7 +2551,12 @@ impl ChannelManager { }, } - } else { unreachable!(); } + } else { + // The channel was likely removed after we fetched the id from the + // `short_to_chan_info` map, but before we successfully locked the `by_id` map. + // This can occur as no consistency guarantees exists between the two maps. + return Err(APIError::ChannelUnavailable{err: "No channel available with first hop!".to_owned()}); + } return Ok(()); }; @@ -3133,9 +3145,8 @@ impl ChannelManager chan_id.clone(), - None => { + macro_rules! forwarding_channel_not_found { + () => { for forward_info in pending_forwards.drain(..) { match forward_info { HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo { @@ -3222,6 +3233,12 @@ impl ChannelManager chan_id.clone(), + None => { + forwarding_channel_not_found!(); continue; } }; @@ -3351,7 +3368,8 @@ impl ChannelManager ChannelManager) { @@ -5226,7 +5244,7 @@ impl ChannelManager unreachable!() + hash_map::Entry::Vacant(_) => return Ok(NotifyOption::SkipPersist) } Ok(NotifyOption::DoPersist) } -- 2.39.5