X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=dd9fcbea9a8fa2bf05c21c9cf34c04b5e8698660;hb=34205af801f321140b90e42d15486415e9cecd0f;hp=e76656e75ce38e9ee9b65c513ebc8c916e7b4ac7;hpb=b8e9e8b83467182a9d66ba10eea5b1ccfaa59410;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e76656e7..dd9fcbea 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -46,7 +46,7 @@ use io; use prelude::*; use core::{cmp,mem,fmt}; use core::ops::Deref; -#[cfg(any(test, feature = "fuzztarget", debug_assertions))] +#[cfg(any(test, fuzzing, debug_assertions))] use sync::Mutex; use bitcoin::hashes::hex::ToHex; @@ -584,6 +584,19 @@ pub(super) struct Channel { #[cfg(not(test))] closing_fee_limits: Option<(u64, u64)>, + /// Flag that ensures that `accept_inbound_channel` must be called before `funding_created` + /// is executed successfully. The reason for this flag is that when the + /// `UserConfig::manually_accept_inbound_channels` config flag is set to true, inbound channels + /// are required to be manually accepted by the node operator before the `msgs::AcceptChannel` + /// message is created and sent out. During the manual accept process, `accept_inbound_channel` + /// is called by `ChannelManager::accept_inbound_channel`. + /// + /// The flag counteracts that a counterparty node could theoretically send a + /// `msgs::FundingCreated` message before the node operator has manually accepted an inbound + /// channel request made by the counterparty node. That would execute `funding_created` before + /// `accept_inbound_channel`, and `funding_created` should therefore not execute successfully. + inbound_awaiting_accept: bool, + /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, @@ -657,9 +670,9 @@ pub(super) struct Channel { // `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will // be, by comparing the cached values to the fee of the tranaction generated by // `build_commitment_transaction`. - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex>, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_remote_commitment_tx_fee_info_cached: Mutex>, /// lnd has a long-standing bug where, upon reconnection, if the channel is not yet confirmed @@ -671,7 +684,7 @@ pub(super) struct Channel { /// See-also pub workaround_lnd_bug_4006: Option, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] // When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the // corresponding HTLC on the inbound path. If, then, the outbound path channel is // disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack @@ -684,7 +697,7 @@ pub(super) struct Channel { channel_type: ChannelTypeFeatures, } -#[cfg(any(test, feature = "fuzztarget"))] +#[cfg(any(test, fuzzing))] struct CommitmentTxInfoCached { fee: u64, total_pending_htlcs: usize, @@ -883,6 +896,8 @@ impl Channel { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, + inbound_awaiting_accept: false, + funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, @@ -925,14 +940,14 @@ impl Channel { announcement_sigs: None, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] historical_inbound_htlc_fulfills: HashSet::new(), // We currently only actually support one channel type, so don't retry with new types @@ -1182,6 +1197,8 @@ impl Channel { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, + inbound_awaiting_accept: true, + funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, @@ -1228,14 +1245,14 @@ impl Channel { announcement_sigs: None, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] historical_inbound_htlc_fulfills: HashSet::new(), channel_type, @@ -1633,7 +1650,7 @@ impl Channel { } } if pending_idx == core::usize::MAX { - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] // If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and // this is simply a duplicate claim, not previously failed and we lost funds. debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); @@ -1659,7 +1676,7 @@ impl Channel { if htlc_id_arg == htlc_id { // Make sure we don't leave latest_monitor_update_id incremented here: self.latest_monitor_update_id -= 1; - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); return UpdateFulfillFetch::DuplicateClaim {}; } @@ -1680,11 +1697,11 @@ impl Channel { self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC { payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg, }); - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] self.historical_inbound_htlc_fulfills.insert(htlc_id_arg); return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None }; } - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] self.historical_inbound_htlc_fulfills.insert(htlc_id_arg); { @@ -1765,7 +1782,7 @@ impl Channel { } } if pending_idx == core::usize::MAX { - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] // If we failed to find an HTLC to fail, make sure it was previously fulfilled and this // is simply a duplicate fail, not previously failed and we failed-back too early. debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); @@ -1778,7 +1795,7 @@ impl Channel { match pending_update { &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => { if htlc_id_arg == htlc_id { - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); return Ok(None); } @@ -1878,6 +1895,16 @@ impl Channel { return Err(ChannelError::Close("Minimum confirmation depth must be at least 1".to_owned())); } + if let Some(ty) = &msg.channel_type { + if *ty != self.channel_type { + return Err(ChannelError::Close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned())); + } + } else if their_features.supports_channel_type() { + // Assume they've accepted the channel type as they said they understand it. + } else { + self.channel_type = ChannelTypeFeatures::from_counterparty_init(&their_features) + } + let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() { match &msg.shutdown_scriptpubkey { &OptionalField::Present(ref script) => { @@ -1973,6 +2000,9 @@ impl Channel { // channel. return Err(ChannelError::Close("Received funding_created after we got the channel!".to_owned())); } + if self.inbound_awaiting_accept { + return Err(ChannelError::Close("FundingCreated message received before the channel was accepted".to_owned())); + } if self.commitment_secrets.get_min_seen_secret() != (1 << 48) || self.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER || self.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { @@ -2154,11 +2184,11 @@ impl Channel { /// Returns transaction if there is pending funding transaction that is yet to broadcast pub fn unbroadcasted_funding(&self) -> Option { - if self.channel_state & (ChannelState::FundingCreated as u32) != 0 { - self.funding_transaction.clone() - } else { - None - } + if self.channel_state & (ChannelState::FundingCreated as u32) != 0 { + self.funding_transaction.clone() + } else { + None + } } /// Returns a HTLCStats about inbound pending htlcs @@ -2347,7 +2377,7 @@ impl Channel { let num_htlcs = included_htlcs + addl_htlcs; let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs, self.opt_anchors()); - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] { let mut fee = res; if fee_spike_buffer_htlc.is_some() { @@ -2425,7 +2455,7 @@ impl Channel { let num_htlcs = included_htlcs + addl_htlcs; let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs, self.opt_anchors()); - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] { let mut fee = res; if fee_spike_buffer_htlc.is_some() { @@ -2708,7 +2738,7 @@ impl Channel { return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee".to_owned()))); } } - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] { if self.is_outbound() { let projected_commit_tx_info = self.next_local_commitment_tx_fee_info_cached.lock().unwrap().take(); @@ -3015,7 +3045,7 @@ impl Channel { return Err(ChannelError::Close("Received an unexpected revoke_and_ack".to_owned())); } - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] { *self.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; *self.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; @@ -4446,9 +4476,9 @@ impl Channel { // If we generated the funding transaction and it doesn't match what it // should, the client is really broken and we should just panic and // tell them off. That said, because hash collisions happen with high - // probability in fuzztarget mode, if we're fuzzing we just close the + // probability in fuzzing mode, if we're fuzzing we just close the // channel and move on. - #[cfg(not(feature = "fuzztarget"))] + #[cfg(not(fuzzing))] panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); } self.update_time_counter += 1; @@ -4460,7 +4490,7 @@ impl Channel { if input.witness.is_empty() { // We generated a malleable funding transaction, implying we've // just exposed ourselves to funds loss to our counterparty. - #[cfg(not(feature = "fuzztarget"))] + #[cfg(not(fuzzing))] panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); } } @@ -4645,7 +4675,15 @@ impl Channel { } } - pub fn get_accept_channel(&self) -> msgs::AcceptChannel { + pub fn inbound_is_awaiting_accept(&self) -> bool { + self.inbound_awaiting_accept + } + + /// Marks an inbound channel as accepted and generates a [`msgs::AcceptChannel`] message which + /// should be sent back to the counterparty node. + /// + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + pub fn accept_inbound_channel(&mut self) -> msgs::AcceptChannel { if self.is_outbound() { panic!("Tried to send accept_channel for an outbound channel?"); } @@ -4655,7 +4693,21 @@ impl Channel { if self.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { panic!("Tried to send an accept_channel for a channel that has already advanced"); } + if !self.inbound_awaiting_accept { + panic!("The inbound channel has already been accepted"); + } + self.inbound_awaiting_accept = false; + + self.generate_accept_channel_message() + } + + /// This function is used to explicitly generate a [`msgs::AcceptChannel`] message for an + /// inbound channel. If the intention is to accept an inbound channel, use + /// [`Channel::accept_inbound_channel`] instead. + /// + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + fn generate_accept_channel_message(&self) -> msgs::AcceptChannel { let first_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); let keys = self.get_holder_pubkeys(); @@ -4678,9 +4730,19 @@ impl Channel { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), + channel_type: Some(self.channel_type.clone()), } } + /// Enables the possibility for tests to extract a [`msgs::AcceptChannel`] message for an + /// inbound channel without accepting it. + /// + /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel + #[cfg(test)] + pub fn get_accept_channel_message(&self) -> msgs::AcceptChannel { + self.generate_accept_channel_message() + } + /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created) fn get_outbound_funding_created_signature(&mut self, logger: &L) -> Result where L::Target: Logger { let counterparty_keys = self.build_remote_transaction_keys()?; @@ -4886,9 +4948,9 @@ impl Channel { // Prior to static_remotekey, my_current_per_commitment_point was critical to claiming // current to_remote balances. However, it no longer has any use, and thus is now simply // set to a dummy (but valid, as required by the spec) public key. - // fuzztarget mode marks a subset of pubkeys as invalid so that we can hit "invalid pubkey" + // fuzzing mode marks a subset of pubkeys as invalid so that we can hit "invalid pubkey" // branches, but we unwrap it below, so we arbitrarily select a dummy pubkey which is both - // valid, and valid in fuzztarget mode's arbitrary validity criteria: + // valid, and valid in fuzzing mode's arbitrary validity criteria: let mut pk = [2; 33]; pk[1] = 0xff; let dummy_pubkey = PublicKey::from_slice(&pk).unwrap(); let data_loss_protect = if self.cur_counterparty_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER { @@ -5174,7 +5236,7 @@ impl Channel { let counterparty_commitment_txid = commitment_stats.tx.trust().txid(); let (signature, htlc_signatures); - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] { if !self.is_outbound() { let projected_commit_tx_info = self.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take(); @@ -5660,9 +5722,9 @@ impl Writeable for Channel { self.channel_update_status.write(writer)?; - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] (self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?; - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] for htlc in self.historical_inbound_htlc_fulfills.iter() { htlc.write(writer)?; } @@ -5924,9 +5986,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel let channel_update_status = Readable::read(reader)?; - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] let mut historical_inbound_htlc_fulfills = HashSet::new(); - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] { let htlc_fulfills_len: u64 = Readable::read(reader)?; for _ in 0..htlc_fulfills_len { @@ -6064,6 +6126,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel closing_fee_limits: None, target_closing_feerate_sats_per_kw, + inbound_awaiting_accept: false, + funding_tx_confirmed_in, funding_tx_confirmation_height, short_channel_id, @@ -6098,14 +6162,14 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel announcement_sigs, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, - #[cfg(any(test, feature = "fuzztarget"))] + #[cfg(any(test, fuzzing))] historical_inbound_htlc_fulfills, channel_type: channel_type.unwrap(), @@ -6135,7 +6199,7 @@ mod tests { use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, htlc_success_tx_weight, htlc_timeout_tx_weight}; use chain::BestBlock; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; - use chain::keysinterface::{InMemorySigner, KeyMaterial, KeysInterface, BaseSign}; + use chain::keysinterface::{InMemorySigner, Recipient, KeyMaterial, KeysInterface, BaseSign}; use chain::transaction::OutPoint; use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; @@ -6183,7 +6247,7 @@ mod tests { impl KeysInterface for Keys { type Signer = InMemorySigner; - fn get_node_secret(&self) -> SecretKey { panic!(); } + fn get_node_secret(&self, _recipient: Recipient) -> Result { panic!(); } fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); } fn get_destination_script(&self) -> Script { let secp_ctx = Secp256k1::signing_only(); @@ -6203,7 +6267,7 @@ mod tests { } fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] } fn read_chan_signer(&self, _data: &[u8]) -> Result { panic!(); } - fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result { panic!(); } + fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient) -> Result { panic!(); } } fn public_from_secret_hex(secp_ctx: &Secp256k1, hex: &str) -> PublicKey { @@ -6281,10 +6345,10 @@ mod tests { // Make sure A's dust limit is as we expect. let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap(); + let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap(); // Node B --> Node A: accept channel, explicitly setting B's dust limit. - let mut accept_channel_msg = node_b_chan.get_accept_channel(); + let mut accept_channel_msg = node_b_chan.accept_inbound_channel(); accept_channel_msg.dust_limit_satoshis = 546; node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap(); node_a_chan.holder_dust_limit_satoshis = 1560; @@ -6402,7 +6466,7 @@ mod tests { let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap(); // Node B --> Node A: accept channel - let accept_channel_msg = node_b_chan.get_accept_channel(); + let accept_channel_msg = node_b_chan.accept_inbound_channel(); node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap(); // Node A --> Node B: funding created