X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannelmanager.rs;h=69093d7f6a35ce22cc67fc2167ebc6e0d2974d4d;hb=82b532c54db938c64f9bc9fb96c8f572337052be;hp=392f3e1cb052cf196b50b2b7e3734b62ab167e4c;hpb=4ab6c551a00ce167abfad2ea97310e8043e08375;p=rust-lightning diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 392f3e1c..69093d7f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -803,7 +803,7 @@ pub type SimpleArcChannelManager = ChannelManager< Arc>>, Arc, - Arc>>, Arc>>>, + Arc>>, Arc>>>, ProbabilisticScoringFeeParameters, ProbabilisticScorer>>, Arc>, >>, @@ -832,7 +832,7 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = &'e DefaultRouter< &'f NetworkGraph<&'g L>, &'g L, - &'h Mutex, &'g L>>, + &'h RwLock, &'g L>>, ProbabilisticScoringFeeParameters, ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L> >, @@ -840,6 +840,9 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = >; /// A trivial trait which describes any [`ChannelManager`]. +/// +/// This is not exported to bindings users as general cover traits aren't useful in other +/// languages. pub trait AChannelManager { /// A type implementing [`chain::Watch`]. type Watch: chain::Watch + ?Sized; @@ -1448,12 +1451,6 @@ pub struct ChannelCounterparty { } /// Details of a channel, as returned by [`ChannelManager::list_channels`] and [`ChannelManager::list_usable_channels`] -/// -/// Balances of a channel are available through [`ChainMonitor::get_claimable_balances`] and -/// [`ChannelMonitor::get_claimable_balances`], calculated with respect to the corresponding on-chain -/// transactions. -/// -/// [`ChainMonitor::get_claimable_balances`]: crate::chain::chainmonitor::ChainMonitor::get_claimable_balances #[derive(Clone, Debug, PartialEq)] pub struct ChannelDetails { /// The channel's ID (prior to funding transaction generation, this is a random 32 bytes, @@ -1535,11 +1532,24 @@ pub struct ChannelDetails { /// /// This value will be `None` for objects serialized with LDK versions prior to 0.0.115. pub feerate_sat_per_1000_weight: Option, + /// Our total balance. This is the amount we would get if we close the channel. + /// This value is not exact. Due to various in-flight changes and feerate changes, exactly this + /// amount is not likely to be recoverable on close. + /// + /// This does not include any pending HTLCs which are not yet fully resolved (and, thus, whose + /// balance is not available for inclusion in new outbound HTLCs). This further does not include + /// any pending outgoing HTLCs which are awaiting some other resolution to be sent. + /// This does not consider any on-chain fees. + /// + /// See also [`ChannelDetails::outbound_capacity_msat`] + pub balance_msat: u64, /// The available outbound capacity for sending HTLCs to the remote peer. This does not include /// any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not /// available for inclusion in new outbound HTLCs). This further does not include any pending /// outgoing HTLCs which are awaiting some other resolution to be sent. /// + /// See also [`ChannelDetails::balance_msat`] + /// /// This value is not exact. Due to various in-flight changes, feerate changes, and our /// conflict-avoidance policy, exactly this amount is not likely to be spendable. However, we /// should be able to spend nearly this amount. @@ -1549,8 +1559,8 @@ pub struct ChannelDetails { /// the current state and per-HTLC limit(s). This is intended for use when routing, allowing us /// to use a limit as close as possible to the HTLC limit we can currently send. /// - /// See also [`ChannelDetails::next_outbound_htlc_minimum_msat`] and - /// [`ChannelDetails::outbound_capacity_msat`]. + /// See also [`ChannelDetails::next_outbound_htlc_minimum_msat`], + /// [`ChannelDetails::balance_msat`], and [`ChannelDetails::outbound_capacity_msat`]. pub next_outbound_htlc_limit_msat: u64, /// The minimum value for sending a single HTLC to the remote peer. This is the equivalent of /// [`ChannelDetails::next_outbound_htlc_limit_msat`] but represents a lower-bound, rather than @@ -1680,6 +1690,7 @@ impl ChannelDetails { channel_value_satoshis: context.get_value_satoshis(), feerate_sat_per_1000_weight: Some(context.get_feerate_sat_per_1000_weight()), unspendable_punishment_reserve: to_self_reserve_satoshis, + balance_msat: balance.balance_msat, inbound_capacity_msat: balance.inbound_capacity_msat, outbound_capacity_msat: balance.outbound_capacity_msat, next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat, @@ -2605,6 +2616,8 @@ where // it does not exist for this peer. Either way, we can attempt to force-close it. // // An appropriate error will be returned for non-existence of the channel if that's the case. + mem::drop(peer_state_lock); + mem::drop(per_peer_state); return self.force_close_channel_with_peer(&channel_id, counterparty_node_id, None, false).map(|_| ()) }, } @@ -3471,9 +3484,8 @@ where /// In general, a path may raise: /// * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee, /// node public key) is specified. - /// * [`APIError::ChannelUnavailable`] if the next-hop channel is not available for updates - /// (including due to previous monitor update failure or new permanent monitor update - /// failure). + /// * [`APIError::ChannelUnavailable`] if the next-hop channel is not available as it has been + /// closed, doesn't exist, or the peer is currently disconnected. /// * [`APIError::MonitorUpdateInProgress`] if a new monitor update failure prevented sending the /// relevant updates. /// @@ -3542,19 +3554,10 @@ where /// wait until you receive either a [`Event::PaymentFailed`] or [`Event::PaymentSent`] event to /// determine the ultimate status of a payment. /// - /// # Requested Invoices - /// - /// In the case of paying a [`Bolt12Invoice`], abandoning the payment prior to receiving the - /// invoice will result in an [`Event::InvoiceRequestFailed`] and prevent any attempts at paying - /// it once received. The other events may only be generated once the invoice has been received. - /// /// # Restart Behavior /// /// If an [`Event::PaymentFailed`] is generated and we restart without first persisting the - /// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated; likewise for - /// [`Event::InvoiceRequestFailed`]. - /// - /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + /// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated. pub fn abandon_payment(&self, payment_id: PaymentId) { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); self.pending_outbound_payments.abandon_payment(payment_id, PaymentFailureReason::UserAbandoned, &self.pending_events); @@ -3895,7 +3898,7 @@ where btree_map::Entry::Vacant(vacant) => Some(vacant.insert(Vec::new())), } }); - for (channel_idx, &(temporary_channel_id, counterparty_node_id)) in temporary_channels.iter().enumerate() { + for &(temporary_channel_id, counterparty_node_id) in temporary_channels.iter() { result = result.and_then(|_| self.funding_transaction_generated_intern( temporary_channel_id, counterparty_node_id, @@ -4000,7 +4003,7 @@ where for channel_id in channel_ids { if !peer_state.has_channel(channel_id) { return Err(APIError::ChannelUnavailable { - err: format!("Channel with ID {} was not found for the passed counterparty_node_id {}", channel_id, counterparty_node_id), + err: format!("Channel with id {} not found for the passed counterparty node_id {}", channel_id, counterparty_node_id), }); }; } @@ -4111,7 +4114,7 @@ where next_hop_channel_id, next_node_id) }), None => return Err(APIError::ChannelUnavailable { - err: format!("Channel with id {} not found for the passed counterparty node_id {}.", + err: format!("Channel with id {} not found for the passed counterparty node_id {}", next_hop_channel_id, next_node_id) }) } @@ -4724,8 +4727,10 @@ where if !chan.context.is_outbound() { return NotifyOption::SkipPersistNoEvents; } // If the feerate has decreased by less than half, don't bother if new_feerate <= chan.context.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.context.get_feerate_sat_per_1000_weight() { - log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {}.", + if new_feerate != chan.context.get_feerate_sat_per_1000_weight() { + log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {}.", chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate); + } return NotifyOption::SkipPersistNoEvents; } if !chan.context.is_live() { @@ -6382,6 +6387,9 @@ where if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() { let res = try_chan_phase_entry!(self, chan.update_fulfill_htlc(&msg), chan_phase_entry); if let HTLCSource::PreviousHopData(prev_hop) = &res.0 { + log_trace!(self.logger, + "Holding the next revoke_and_ack from {} until the preimage is durably persisted in the inbound edge's ChannelMonitor", + msg.channel_id); peer_state.actions_blocking_raa_monitor_updates.entry(msg.channel_id) .or_insert_with(Vec::new) .push(RAAMonitorUpdateBlockingAction::from_prev_hop_data(&prev_hop)); @@ -6757,7 +6765,12 @@ where return Ok(NotifyOption::SkipPersistNoEvents); } else { log_debug!(self.logger, "Received channel_update {:?} for channel {}.", msg, chan_id); - try_chan_phase_entry!(self, chan.channel_update(&msg), chan_phase_entry); + let did_change = try_chan_phase_entry!(self, chan.channel_update(&msg), chan_phase_entry); + // If nothing changed after applying their update, we don't need to bother + // persisting. + if !did_change { + return Ok(NotifyOption::SkipPersistNoEvents); + } } } else { return try_chan_phase_entry!(self, Err(ChannelError::Close( @@ -6972,8 +6985,7 @@ where fn maybe_generate_initial_closing_signed(&self) -> bool { let mut handle_errors: Vec<(PublicKey, Result<(), _>)> = Vec::new(); let mut has_update = false; - let mut shutdown_result = None; - let mut unbroadcasted_batch_funding_txid = None; + let mut shutdown_results = Vec::new(); { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -6984,7 +6996,7 @@ where peer_state.channel_by_id.retain(|channel_id, phase| { match phase { ChannelPhase::Funded(chan) => { - unbroadcasted_batch_funding_txid = chan.context.unbroadcasted_batch_funding_txid(); + let unbroadcasted_batch_funding_txid = chan.context.unbroadcasted_batch_funding_txid(); match chan.maybe_propose_closing_signed(&self.fee_estimator, &self.logger) { Ok((msg_opt, tx_opt)) => { if let Some(msg) = msg_opt { @@ -7007,7 +7019,7 @@ where log_info!(self.logger, "Broadcasting {}", log_tx!(tx)); self.tx_broadcaster.broadcast_transactions(&[&tx]); update_maps_on_chan_removal!(self, &chan.context); - shutdown_result = Some((None, Vec::new(), unbroadcasted_batch_funding_txid)); + shutdown_results.push((None, Vec::new(), unbroadcasted_batch_funding_txid)); false } else { true } }, @@ -7029,7 +7041,7 @@ where let _ = handle_error!(self, err, counterparty_node_id); } - if let Some(shutdown_result) = shutdown_result { + for shutdown_result in shutdown_results.drain(..) { self.finish_close_channel(shutdown_result); } @@ -8415,7 +8427,7 @@ impl Writeable for ChannelDetails { (10, self.channel_value_satoshis, required), (12, self.unspendable_punishment_reserve, option), (14, user_channel_id_low, required), - (16, self.next_outbound_htlc_limit_msat, required), // Forwards compatibility for removed balance_msat field. + (16, self.balance_msat, required), (18, self.outbound_capacity_msat, required), (19, self.next_outbound_htlc_limit_msat, required), (20, self.inbound_capacity_msat, required), @@ -8451,7 +8463,7 @@ impl Readable for ChannelDetails { (10, channel_value_satoshis, required), (12, unspendable_punishment_reserve, option), (14, user_channel_id_low, required), - (16, _balance_msat, option), // Backwards compatibility for removed balance_msat field. + (16, balance_msat, required), (18, outbound_capacity_msat, required), // Note that by the time we get past the required read above, outbound_capacity_msat will be // filled in, so we can safely unwrap it here. @@ -8477,8 +8489,6 @@ impl Readable for ChannelDetails { let user_channel_id = user_channel_id_low as u128 + ((user_channel_id_high_opt.unwrap_or(0 as u64) as u128) << 64); - let _balance_msat: Option = _balance_msat; - Ok(Self { inbound_scid_alias, channel_id: channel_id.0.unwrap(), @@ -8491,6 +8501,7 @@ impl Readable for ChannelDetails { channel_value_satoshis: channel_value_satoshis.0.unwrap(), unspendable_punishment_reserve, user_channel_id, + balance_msat: balance_msat.0.unwrap(), outbound_capacity_msat: outbound_capacity_msat.0.unwrap(), next_outbound_htlc_limit_msat: next_outbound_htlc_limit_msat.0.unwrap(), next_outbound_htlc_minimum_msat: next_outbound_htlc_minimum_msat.0.unwrap(), @@ -9964,6 +9975,9 @@ where Some((blocked_node_id, blocked_channel_outpoint, blocking_action)), .. } = action { if let Some(blocked_peer_state) = per_peer_state.get(&blocked_node_id) { + log_trace!(args.logger, + "Holding the next revoke_and_ack from {} until the preimage is durably persisted in the inbound edge's ChannelMonitor", + blocked_channel_outpoint.to_channel_id()); blocked_peer_state.lock().unwrap().actions_blocking_raa_monitor_updates .entry(blocked_channel_outpoint.to_channel_id()) .or_insert_with(Vec::new).push(blocking_action.clone()); @@ -10744,6 +10758,16 @@ mod tests { check_api_error_message(expected_message, res_err) } + fn check_channel_unavailable_error(res_err: Result, expected_channel_id: ChannelId, peer_node_id: PublicKey) { + let expected_message = format!("Channel with id {} not found for the passed counterparty node_id {}", expected_channel_id, peer_node_id); + check_api_error_message(expected_message, res_err) + } + + fn check_api_misuse_error(res_err: Result) { + let expected_message = "No such channel awaiting to be accepted.".to_string(); + check_api_error_message(expected_message, res_err) + } + fn check_api_error_message(expected_err_message: String, res_err: Result) { match res_err { Err(APIError::APIMisuseError { err }) => { @@ -10788,6 +10812,36 @@ mod tests { check_unkown_peer_error(nodes[0].node.update_channel_config(&unkown_public_key, &[channel_id], &ChannelConfig::default()), unkown_public_key); } + #[test] + fn test_api_calls_with_unavailable_channel() { + // Tests that our API functions that expects a `counterparty_node_id` and a `channel_id` + // as input, behaves as expected if the `counterparty_node_id` is a known peer in the + // `ChannelManager::per_peer_state` map, but the peer state doesn't contain a channel with + // the given `channel_id`. + let chanmon_cfg = create_chanmon_cfgs(2); + let node_cfg = create_node_cfgs(2, &chanmon_cfg); + let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[None, None]); + let nodes = create_network(2, &node_cfg, &node_chanmgr); + + let counterparty_node_id = nodes[1].node.get_our_node_id(); + + // Dummy values + let channel_id = ChannelId::from_bytes([4; 32]); + + // Test the API functions. + check_api_misuse_error(nodes[0].node.accept_inbound_channel(&channel_id, &counterparty_node_id, 42)); + + check_channel_unavailable_error(nodes[0].node.close_channel(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id); + + check_channel_unavailable_error(nodes[0].node.force_close_broadcasting_latest_txn(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id); + + check_channel_unavailable_error(nodes[0].node.force_close_without_broadcasting_txn(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id); + + check_channel_unavailable_error(nodes[0].node.forward_intercepted_htlc(InterceptId([0; 32]), &channel_id, counterparty_node_id, 1_000_000), channel_id, counterparty_node_id); + + check_channel_unavailable_error(nodes[0].node.update_channel_config(&counterparty_node_id, &[channel_id], &ChannelConfig::default()), channel_id, counterparty_node_id); + } + #[test] fn test_connection_limiting() { // Test that we limit un-channel'd peers and un-funded channels properly.