X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannelmanager.rs;h=74636c0aa95607b9a870434f47f2e04595cb498d;hb=6c36e011a80cd3d02db8591fb9a19111dceecb86;hp=cdde8bd85325f9a68f5ed1ed4510c891428421c7;hpb=61341df39e90de9d650851a624c0644f5c9dd055;p=rust-lightning diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cdde8bd8..74636c0a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -413,6 +413,9 @@ struct PeerState { /// /// For users who don't want to bother doing their own payment preimage storage, we also store that /// here. +/// +/// Note that this struct will be removed entirely soon, in favor of storing no inbound payment data +/// and instead encoding it in the payment secret. struct PendingInboundPayment { /// The payment secret that the sender must use for us to accept this payment payment_secret: PaymentSecret, @@ -1141,7 +1144,7 @@ macro_rules! handle_monitor_err { res } }; ($self: ident, $err: expr, $channel_state: expr, $entry: expr, $action_type: path, $resend_raa: expr, $resend_commitment: expr, $failed_forwards: expr, $failed_fails: expr) => { - handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new()); + handle_monitor_err!($self, $err, $channel_state, $entry, $action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails, Vec::new()) } } @@ -1398,7 +1401,8 @@ impl ChannelMana let peer_state = peer_state.lock().unwrap(); let their_features = &peer_state.latest_features; let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration }; - Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_channel_id, config)? + Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, + channel_value_satoshis, push_msat, user_channel_id, config, self.best_block.read().unwrap().height())? }, None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }), } @@ -1959,17 +1963,24 @@ impl ChannelMana break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } let cur_height = self.best_block.read().unwrap().height() + 1; - // Theoretically, channel counterparty shouldn't send us a HTLC expiring now, but we want to be robust wrt to counterparty - // packet sanitization (see HTLC_FAIL_BACK_BUFFER rational) + // Theoretically, channel counterparty shouldn't send us a HTLC expiring now, + // but we want to be robust wrt to counterparty packet sanitization (see + // HTLC_FAIL_BACK_BUFFER rationale). if msg.cltv_expiry <= cur_height + HTLC_FAIL_BACK_BUFFER as u32 { // expiry_too_soon break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far break Some(("CLTV expiry is too far in the future", 21, None)); } - // In theory, we would be safe against unintentional channel-closure, if we only required a margin of LATENCY_GRACE_PERIOD_BLOCKS. - // But, to be safe against policy reception, we use a longer delay. - if (*outgoing_cltv_value) as u64 <= (cur_height + HTLC_FAIL_BACK_BUFFER) as u64 { + // If the HTLC expires ~now, don't bother trying to forward it to our + // counterparty. They should fail it anyway, but we don't want to bother with + // the round-trips or risk them deciding they definitely want the HTLC and + // force-closing to ensure they get it if we're offline. + // We previously had a much more aggressive check here which tried to ensure + // our counterparty receives an HTLC which has *our* risk threshold met on it, + // but there is no need to do that, and since we're a bit conservative with our + // risk threshold it just results in failing to forward payments. + if (*outgoing_cltv_value) as u64 <= (cur_height + LATENCY_GRACE_PERIOD_BLOCKS) as u64 { break Some(("Outgoing CLTV value is too soon", 0x1000 | 14, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } @@ -2085,6 +2096,21 @@ impl ChannelMana Some(id) => id.clone(), }; + macro_rules! insert_outbound_payment { + () => { + let payment = payment_entry.or_insert_with(|| PendingOutboundPayment::Retryable { + session_privs: HashSet::new(), + pending_amt_msat: 0, + pending_fee_msat: Some(0), + payment_hash: *payment_hash, + payment_secret: *payment_secret, + starting_block_height: self.best_block.read().unwrap().height(), + total_msat: total_value, + }); + assert!(payment.insert(session_priv_bytes, path)); + } + } + let channel_state = &mut *channel_lock; if let hash_map::Entry::Occupied(mut chan) = channel_state.by_id.entry(id) { match { @@ -2094,7 +2120,7 @@ impl ChannelMana if !chan.get().is_live() { return Err(APIError::ChannelUnavailable{err: "Peer for first hop currently disconnected/pending monitor update!".to_owned()}); } - let send_res = break_chan_entry!(self, chan.get_mut().send_htlc_and_commit( + break_chan_entry!(self, chan.get_mut().send_htlc_and_commit( htlc_msat, payment_hash.clone(), htlc_cltv, HTLCSource::OutboundRoute { path: path.clone(), session_priv: session_priv.clone(), @@ -2103,20 +2129,7 @@ impl ChannelMana payment_secret: payment_secret.clone(), payee: payee.clone(), }, onion_packet, &self.logger), - channel_state, chan); - - let payment = payment_entry.or_insert_with(|| PendingOutboundPayment::Retryable { - session_privs: HashSet::new(), - pending_amt_msat: 0, - pending_fee_msat: Some(0), - payment_hash: *payment_hash, - payment_secret: *payment_secret, - starting_block_height: self.best_block.read().unwrap().height(), - total_msat: total_value, - }); - assert!(payment.insert(session_priv_bytes, path)); - - send_res + channel_state, chan) } { Some((update_add, commitment_signed, monitor_update)) => { if let Err(e) = self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), monitor_update) { @@ -2126,8 +2139,10 @@ impl ChannelMana // is restored. Therefore, we must return an error indicating that // it is unsafe to retry the payment wholesale, which we do in the // send_payment check for MonitorUpdateFailed, below. + insert_outbound_payment!(); // Only do this after possibly break'ing on Perm failure above. return Err(APIError::MonitorUpdateFailed); } + insert_outbound_payment!(); log_debug!(self.logger, "Sending payment along path resulted in a commitment_signed for channel {}", log_bytes!(chan.get().channel_id())); channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs { @@ -2142,7 +2157,7 @@ impl ChannelMana }, }); }, - None => {}, + None => { insert_outbound_payment!(); }, } } else { unreachable!(); } return Ok(()); @@ -2275,6 +2290,9 @@ impl ChannelMana } else { None }, }) } else if has_err { + // If we failed to send any paths, we shouldn't have inserted the new PaymentId into + // our `pending_outbound_payments` map at all. + debug_assert!(self.pending_outbound_payments.lock().unwrap().get(&payment_id).is_none()); Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect())) } else { Ok(payment_id) @@ -2633,7 +2651,7 @@ impl ChannelMana htlc_id: prev_htlc_id, incoming_packet_shared_secret: incoming_shared_secret, }); - match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet) { + match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet, &self.logger) { Err(e) => { if let ChannelError::Ignore(msg) = e { log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg); @@ -2872,7 +2890,6 @@ impl ChannelMana purpose: events::PaymentPurpose::InvoicePayment { payment_preimage: inbound_payment.get().payment_preimage, payment_secret: payment_data.payment_secret, - user_payment_id: inbound_payment.get().user_payment_id, }, amt: total_value, }); @@ -3631,7 +3648,12 @@ impl ChannelMana return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone())); } - let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), &their_features, msg, 0, &self.default_configuration) + if !self.default_configuration.accept_inbound_channels { + return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone())); + } + + let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), + &their_features, msg, 0, &self.default_configuration, self.best_block.read().unwrap().height()) .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?; let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_state_lock; @@ -4503,7 +4525,7 @@ impl ChannelMana } } - fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option, min_value_msat: Option, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result { + fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result { assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106 let payment_secret = PaymentSecret(self.keys_manager.get_secure_random_bytes()); @@ -4513,7 +4535,8 @@ impl ChannelMana match payment_secrets.entry(payment_hash) { hash_map::Entry::Vacant(e) => { e.insert(PendingInboundPayment { - payment_secret, min_value_msat, user_payment_id, payment_preimage, + payment_secret, min_value_msat, payment_preimage, + user_payment_id: 0, // For compatibility with version 0.0.103 and earlier // We assume that highest_seen_timestamp is pretty close to the current time - // its updated when we receive a new block with the maximum time we've seen in // a header. It should never be more than two hours in the future. @@ -4545,12 +4568,12 @@ impl ChannelMana /// [`PaymentReceived`]: events::Event::PaymentReceived /// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash - pub fn create_inbound_payment(&self, min_value_msat: Option, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> (PaymentHash, PaymentSecret) { + pub fn create_inbound_payment(&self, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) { let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes()); let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner()); (payment_hash, - self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs, user_payment_id) + self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs) .expect("RNG Generated Duplicate PaymentHash")) } @@ -4564,12 +4587,6 @@ impl ChannelMana /// The [`PaymentHash`] (and corresponding [`PaymentPreimage`]) must be globally unique. This /// method may return an Err if another payment with the same payment_hash is still pending. /// - /// `user_payment_id` will be provided back in [`PaymentPurpose::InvoicePayment::user_payment_id`] events to - /// allow tracking of which events correspond with which calls to this and - /// [`create_inbound_payment`]. `user_payment_id` has no meaning inside of LDK, it is simply - /// copied to events and otherwise ignored. It may be used to correlate PaymentReceived events - /// with invoice metadata stored elsewhere. - /// /// `min_value_msat` should be set if the invoice being generated contains a value. Any payment /// received for the returned [`PaymentHash`] will be required to be at least `min_value_msat` /// before a [`PaymentReceived`] event will be generated, ensuring that we do not provide the @@ -4598,9 +4615,8 @@ impl ChannelMana /// /// [`create_inbound_payment`]: Self::create_inbound_payment /// [`PaymentReceived`]: events::Event::PaymentReceived - /// [`PaymentPurpose::InvoicePayment::user_payment_id`]: events::PaymentPurpose::InvoicePayment::user_payment_id - pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option, invoice_expiry_delta_secs: u32, user_payment_id: u64) -> Result { - self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs, user_payment_id) + pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option, invoice_expiry_delta_secs: u32) -> Result { + self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs) } #[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))] @@ -4836,7 +4852,7 @@ where /// Calls a function which handles an on-chain event (blocks dis/connected, transactions /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by /// the function. - fn do_chain_event) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage>> + fn do_chain_event) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), ClosureReason>> (&self, height_opt: Option, f: FN) { // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called // during initialization prior to the chain_monitor being fully configured in some cases. @@ -4881,7 +4897,7 @@ where } short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id()); } - } else if let Err(e) = res { + } else if let Err(reason) = res { if let Some(short_id) = channel.get_short_channel_id() { short_to_id.remove(&short_id); } @@ -4893,10 +4909,14 @@ where msg: update }); } - self.issue_channel_close_events(channel, ClosureReason::CommitmentTxConfirmed); + let reason_message = format!("{}", reason); + self.issue_channel_close_events(channel, reason); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: channel.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage { msg: e }, + action: msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { + channel_id: channel.channel_id(), + data: reason_message, + } }, }); return false; } @@ -5815,7 +5835,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128)); let mut channel_closures = Vec::new(); for _ in 0..channel_count { - let mut channel: Channel = Channel::read(reader, &args.keys_manager)?; + let mut channel: Channel = Channel::read(reader, (&args.keys_manager, best_block_height))?; let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?; funding_txo_set.insert(funding_txo.clone()); if let Some(ref mut monitor) = args.channel_monitors.get_mut(&funding_txo) { @@ -5850,6 +5870,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> reason: ClosureReason::OutdatedChannelManager }); } else { + log_info!(args.logger, "Successfully loaded channel {}", log_bytes!(channel.channel_id())); if let Some(short_channel_id) = channel.get_short_channel_id() { short_to_id.insert(short_channel_id, channel.channel_id()); } @@ -5867,6 +5888,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> for (ref funding_txo, ref mut monitor) in args.channel_monitors.iter_mut() { if !funding_txo_set.contains(funding_txo) { + log_info!(args.logger, "Broadcasting latest holder commitment transaction for closed channel {}", log_bytes!(funding_txo.to_channel_id())); monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger); } } @@ -6087,9 +6109,9 @@ mod tests { use ln::msgs; use ln::msgs::ChannelMessageHandler; use routing::router::{Payee, RouteParameters, find_route}; - use routing::scorer::Scorer; use util::errors::APIError; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider}; + use util::test_utils; #[cfg(feature = "std")] #[test] @@ -6325,7 +6347,7 @@ mod tests { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); // To start (1), send a regular payment but don't claim it. let expected_route = [&nodes[1]]; @@ -6338,8 +6360,8 @@ mod tests { final_cltv_expiry_delta: TEST_FINAL_CLTV, }; let route = find_route( - &nodes[0].node.get_our_node_id(), ¶ms, - &nodes[0].net_graph_msg_handler.network_graph, None, nodes[0].logger, &scorer + &nodes[0].node.get_our_node_id(), ¶ms, nodes[0].network_graph, None, + nodes[0].logger, &scorer ).unwrap(); nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap(); check_added_monitors!(nodes[0], 1); @@ -6369,8 +6391,8 @@ mod tests { // To start (2), send a keysend payment but don't claim it. let payment_preimage = PaymentPreimage([42; 32]); let route = find_route( - &nodes[0].node.get_our_node_id(), ¶ms, - &nodes[0].net_graph_msg_handler.network_graph, None, nodes[0].logger, &scorer + &nodes[0].node.get_our_node_id(), ¶ms, nodes[0].network_graph, None, + nodes[0].logger, &scorer ).unwrap(); let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap(); check_added_monitors!(nodes[0], 1); @@ -6428,9 +6450,9 @@ mod tests { final_value_msat: 10000, final_cltv_expiry_delta: 40, }; - let network_graph = &nodes[0].net_graph_msg_handler.network_graph; + let network_graph = nodes[0].network_graph; let first_hops = nodes[0].node.list_usable_channels(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route( &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), nodes[0].logger, &scorer @@ -6471,9 +6493,9 @@ mod tests { final_value_msat: 10000, final_cltv_expiry_delta: 40, }; - let network_graph = &nodes[0].net_graph_msg_handler.network_graph; + let network_graph = nodes[0].network_graph; let first_hops = nodes[0].node.list_usable_channels(); - let scorer = Scorer::with_fixed_penalty(0); + let scorer = test_utils::TestScorer::with_fixed_penalty(0); let route = find_route( &payer_pubkey, ¶ms, network_graph, Some(&first_hops.iter().collect::>()), nodes[0].logger, &scorer @@ -6538,7 +6560,7 @@ pub mod bench { use ln::msgs::{ChannelMessageHandler, Init}; use routing::network_graph::NetworkGraph; use routing::router::{Payee, get_route}; - use routing::scorer::Scorer; + use routing::scoring::Scorer; use util::test_utils; use util::config::UserConfig; use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose}; @@ -6646,7 +6668,7 @@ pub mod bench { macro_rules! send_payment { ($node_a: expr, $node_b: expr) => { let usable_channels = $node_a.list_usable_channels(); - let payee = Payee::new($node_b.get_our_node_id()) + let payee = Payee::from_node_id($node_b.get_our_node_id()) .with_features(InvoiceFeatures::known()); let scorer = Scorer::with_fixed_penalty(0); let route = get_route(&$node_a.get_our_node_id(), &payee, &dummy_graph, @@ -6656,7 +6678,7 @@ pub mod bench { payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes()); payment_count += 1; let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()); - let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap(); + let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200).unwrap(); $node_a.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap(); let payment_event = SendEvent::from_event($node_a.get_and_clear_pending_msg_events().pop().unwrap());