From: Matt Corallo Date: Thu, 26 Jul 2018 18:33:01 +0000 (-0400) Subject: Properly calculate Channel::announce_publicly X-Git-Tag: v0.0.12~362^2~1 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=da014797b0e9b36a160541cccad454c09200706a;p=rust-lightning Properly calculate Channel::announce_publicly For some reason we were only setting "announce_publicly" when Channel::new_from_req had announce_publicly set to true and the open_channel message had the relevant flag set. However, this resulted in us rejecting peers for sending unsolicited announcement_signatures messages, despite them having indicated, and us having accepted, their announce-bit-set open_channel. --- diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs index 1bcc3d708..6b24d700e 100644 --- a/fuzz/fuzz_targets/channel_target.rs +++ b/fuzz/fuzz_targets/channel_target.rs @@ -213,7 +213,7 @@ pub fn do_test(data: &[u8]) { } else { decode_msg!(msgs::OpenChannel, 2*32+6*8+4+2*2+6*33+1) }; - let mut chan = match Channel::new_from_req(&fee_est, chan_keys!(), their_pubkey, &open_chan, slice_to_be64(get_slice!(8)), get_slice!(1)[0] == 0) { + let mut chan = match Channel::new_from_req(&fee_est, chan_keys!(), their_pubkey, &open_chan, slice_to_be64(get_slice!(8)), false, get_slice!(1)[0] == 0) { Ok(chan) => chan, Err(_) => return, }; diff --git a/src/ln/channel.rs b/src/ln/channel.rs index eb5ae008d..69b4645c0 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -445,7 +445,7 @@ impl Channel { /// Assumes chain_hash has already been checked and corresponds with what we expect! /// Generally prefers to take the DisconnectPeer action on failure, as a notice to the sender /// that we're rejecting the new channel. - pub fn new_from_req(fee_estimator: &FeeEstimator, chan_keys: ChannelKeys, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, announce_publicly: bool) -> Result { + pub fn new_from_req(fee_estimator: &FeeEstimator, chan_keys: ChannelKeys, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, require_announce: bool, allow_announce: bool) -> Result { // Check sanity of message fields: if msg.funding_satoshis >= MAX_FUNDING_SATOSHIS { return Err(HandleError{err: "funding value > 2^24", action: Some(msgs::ErrorAction::DisconnectPeer{ msg: None })}); @@ -476,6 +476,12 @@ impl Channel { // Convert things into internal flags and prep our state: let their_announce = if (msg.channel_flags & 1) == 1 { true } else { false }; + if require_announce && !their_announce { + return Err(HandleError{err: "Peer tried to open unannounced channel, but we require public ones", action: Some(msgs::ErrorAction::IgnoreError) }); + } + if !allow_announce && their_announce { + return Err(HandleError{err: "Peer tried to open announced channel, but we require private ones", action: Some(msgs::ErrorAction::IgnoreError) }); + } let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); @@ -496,7 +502,7 @@ impl Channel { channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32), channel_outbound: false, secp_ctx: secp_ctx, - announce_publicly: their_announce && announce_publicly, + announce_publicly: their_announce, local_keys: chan_keys, cur_local_commitment_transaction_number: (1 << 48) - 1, diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index c674c2696..73e1052d8 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -1185,7 +1185,7 @@ impl ChannelMessageHandler for ChannelManager { } }; - let channel = Channel::new_from_req(&*self.fee_estimator, chan_keys, their_node_id.clone(), msg, 0, self.announce_channels_publicly)?; + let channel = Channel::new_from_req(&*self.fee_estimator, chan_keys, their_node_id.clone(), msg, 0, false, self.announce_channels_publicly)?; let accept_msg = channel.get_accept_channel()?; channel_state.by_id.insert(channel.channel_id(), channel); Ok(accept_msg)