From: valentinewallace Date: Tue, 26 Apr 2022 17:02:29 +0000 (-0400) Subject: Merge pull request #1436 from TheBlueMatt/2022-04-event-process-try-lock X-Git-Tag: v0.0.107~55 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=72069bfc9d082d3d142cfa09dca2bf6e1f085710;hp=050b19cd9bceb9e3620f9233279f848b1f264475;p=rust-lightning Merge pull request #1436 from TheBlueMatt/2022-04-event-process-try-lock Reorder the BP loop to make manager persistence more reliable --- diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index bff177b1..b0f052db 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -213,6 +213,8 @@ pub fn do_test(data: &[u8], out: Out) { features: InitFeatures::known(), unspendable_punishment_reserve: 0, forwarding_info: None, + outbound_htlc_minimum_msat: None, + outbound_htlc_maximum_msat: None, }, funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }), channel_type: None, @@ -227,6 +229,8 @@ pub fn do_test(data: &[u8], out: Out) { is_usable: true, is_public: true, balance_msat: 0, outbound_capacity_msat: 0, + inbound_htlc_minimum_msat: None, + inbound_htlc_maximum_msat: None, }); } Some(&first_hops_vec[..]) diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index 255fb245..e23737c0 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -347,10 +347,9 @@ mod tests { use bitcoin::blockdata::constants::genesis_block; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::network::constants::Network; - use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; - use lightning::chain::{BestBlock, Confirm, chainmonitor, self}; + use lightning::chain::{BestBlock, Confirm, chainmonitor}; use lightning::chain::channelmonitor::ANTI_REORG_DELAY; - use lightning::chain::keysinterface::{InMemorySigner, Recipient, KeysInterface, KeysManager, Sign}; + use lightning::chain::keysinterface::{InMemorySigner, Recipient, KeysInterface, KeysManager}; use lightning::chain::transaction::OutPoint; use lightning::get_event_msg; use lightning::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChainParameters, ChannelManager, SimpleArcChannelManager}; @@ -360,15 +359,13 @@ mod tests { use lightning::routing::network_graph::{NetworkGraph, NetGraphMsgHandler}; use lightning::util::config::UserConfig; use lightning::util::events::{Event, MessageSendEventsProvider, MessageSendEvent}; - use lightning::util::logger::Logger; use lightning::util::ser::Writeable; use lightning::util::test_utils; use lightning::util::persist::KVStorePersister; use lightning_invoice::payment::{InvoicePayer, RetryAttempts}; use lightning_invoice::utils::DefaultRouter; use lightning_persister::FilesystemPersister; - use std::fs::{self, File}; - use std::ops::Deref; + use std::fs; use std::path::PathBuf; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -411,7 +408,6 @@ mod tests { } struct Persister { - data_dir: String, graph_error: Option<(std::io::ErrorKind, &'static str)>, manager_error: Option<(std::io::ErrorKind, &'static str)>, filesystem_persister: FilesystemPersister, @@ -420,7 +416,7 @@ mod tests { impl Persister { fn new(data_dir: String) -> Self { let filesystem_persister = FilesystemPersister::new(data_dir.clone()); - Self { data_dir, graph_error: None, manager_error: None, filesystem_persister } + Self { graph_error: None, manager_error: None, filesystem_persister } } fn with_graph_error(self, error: std::io::ErrorKind, message: &'static str) -> Self { diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 35490688..a2edc43a 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -412,8 +412,8 @@ fn filter_channels(channels: Vec, min_inbound_capacity_msat: Opt proportional_millionths: forwarding_info.fee_proportional_millionths, }, cltv_expiry_delta: forwarding_info.cltv_expiry_delta, - htlc_minimum_msat: None, - htlc_maximum_msat: None,}]) + htlc_minimum_msat: channel.inbound_htlc_minimum_msat, + htlc_maximum_msat: channel.inbound_htlc_maximum_msat,}]) }; // If all channels are private, return the route hint for the highest inbound capacity channel // per counterparty node. If channels with an higher inbound capacity than the @@ -535,10 +535,13 @@ mod test { // Invoice SCIDs should always use inbound SCID aliases over the real channel ID, if one is // available. + let chan = &nodes[1].node.list_usable_channels()[0]; assert_eq!(invoice.route_hints().len(), 1); assert_eq!(invoice.route_hints()[0].0.len(), 1); - assert_eq!(invoice.route_hints()[0].0[0].short_channel_id, - nodes[1].node.list_usable_channels()[0].inbound_scid_alias.unwrap()); + assert_eq!(invoice.route_hints()[0].0[0].short_channel_id, chan.inbound_scid_alias.unwrap()); + + assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan.inbound_htlc_minimum_msat); + assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan.inbound_htlc_maximum_msat); let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key()) .with_features(invoice.features().unwrap().clone()) @@ -879,6 +882,40 @@ mod test { } } + #[test] + #[cfg(feature = "std")] + fn test_multi_node_hints_has_htlc_min_max_values() { + let mut chanmon_cfgs = create_chanmon_cfgs(3); + let seed_1 = [42 as u8; 32]; + let seed_2 = [43 as u8; 32]; + let cross_node_seed = [44 as u8; 32]; + chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed); + chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001, InitFeatures::known(), InitFeatures::known()); + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 2, 100000, 10001, InitFeatures::known(), InitFeatures::known()); + + let payment_amt = 20_000; + let (payment_hash, _payment_secret) = nodes[1].node.create_inbound_payment(Some(payment_amt), 3600).unwrap(); + let route_hints = vec![ + nodes[1].node.get_phantom_route_hints(), + nodes[2].node.get_phantom_route_hints(), + ]; + + let invoice = ::utils::create_phantom_invoice::(Some(payment_amt), Some(payment_hash), "test".to_string(), 3600, route_hints, &nodes[1].keys_manager, Currency::BitcoinTestnet).unwrap(); + + let chan_0_1 = &nodes[1].node.list_usable_channels()[0]; + assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan_0_1.inbound_htlc_minimum_msat); + assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan_0_1.inbound_htlc_maximum_msat); + + let chan_0_2 = &nodes[2].node.list_usable_channels()[0]; + assert_eq!(invoice.route_hints()[1].0[0].htlc_minimum_msat, chan_0_2.inbound_htlc_minimum_msat); + assert_eq!(invoice.route_hints()[1].0[0].htlc_maximum_msat, chan_0_2.inbound_htlc_maximum_msat); + } + #[test] #[cfg(feature = "std")] fn create_phantom_invoice_with_description_hash() { diff --git a/lightning-persister/src/util.rs b/lightning-persister/src/util.rs index 25bd00f5..4adbb33e 100644 --- a/lightning-persister/src/util.rs +++ b/lightning-persister/src/util.rs @@ -84,7 +84,6 @@ mod tests { use super::{write_to_file}; use std::fs; use std::io; - use std::io::Write; use std::path::PathBuf; struct TestWriteable{} diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9ccdf816..b0551bf8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4321,11 +4321,15 @@ impl Channel { } /// Allowed in any state (including after shutdown) - #[cfg(test)] pub fn get_holder_htlc_minimum_msat(&self) -> u64 { self.holder_htlc_minimum_msat } + /// Allowed in any state (including after shutdown), but will return none before TheirInitSent + pub fn get_holder_htlc_maximum_msat(&self) -> Option { + self.get_htlc_maximum_msat(self.holder_max_htlc_value_in_flight_msat) + } + /// Allowed in any state (including after shutdown) pub fn get_announced_htlc_max_msat(&self) -> u64 { return cmp::min( @@ -4343,6 +4347,21 @@ impl Channel { self.counterparty_htlc_minimum_msat } + /// Allowed in any state (including after shutdown), but will return none before TheirInitSent + pub fn get_counterparty_htlc_maximum_msat(&self) -> Option { + self.get_htlc_maximum_msat(self.counterparty_max_htlc_value_in_flight_msat) + } + + fn get_htlc_maximum_msat(&self, party_max_htlc_value_in_flight_msat: u64) -> Option { + self.counterparty_selected_channel_reserve_satoshis.map(|counterparty_reserve| { + let holder_reserve = self.holder_selected_channel_reserve_satoshis; + cmp::min( + (self.channel_value_satoshis - counterparty_reserve - holder_reserve) * 1000, + party_max_htlc_value_in_flight_msat + ) + }) + } + pub fn get_value_satoshis(&self) -> u64 { self.channel_value_satoshis } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7f66e604..93746f0f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -922,6 +922,12 @@ pub struct ChannelCounterparty { /// Information on the fees and requirements that the counterparty requires when forwarding /// payments to us through this channel. pub forwarding_info: Option, + /// The smallest value HTLC (in msat) the remote peer will accept, for this channel. This field + /// is only `None` before we have received either the `OpenChannel` or `AcceptChannel` message + /// from the remote peer, or for `ChannelCounterparty` objects serialized prior to LDK 0.0.107. + pub outbound_htlc_minimum_msat: Option, + /// The largest value HTLC (in msat) the remote peer currently will accept, for this channel. + pub outbound_htlc_maximum_msat: Option, } /// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels @@ -1046,6 +1052,11 @@ pub struct ChannelDetails { pub is_usable: bool, /// True if this channel is (or will be) publicly-announced. pub is_public: bool, + /// The smallest value HTLC (in msat) we will accept, for this channel. This field + /// is only `None` for `ChannelDetails` objects serialized prior to LDK 0.0.107 + pub inbound_htlc_minimum_msat: Option, + /// The largest value HTLC (in msat) we currently will accept, for this channel. + pub inbound_htlc_maximum_msat: Option, } impl ChannelDetails { @@ -1670,6 +1681,14 @@ impl ChannelMana features: InitFeatures::empty(), unspendable_punishment_reserve: to_remote_reserve_satoshis, forwarding_info: channel.counterparty_forwarding_info(), + // Ensures that we have actually received the `htlc_minimum_msat` value + // from the counterparty through the `OpenChannel` or `AcceptChannel` + // message (as they are always the first message from the counterparty). + // Else `Channel::get_counterparty_htlc_minimum_msat` could return the + // default `0` value set by `Channel::new_outbound`. + outbound_htlc_minimum_msat: if channel.have_received_message() { + Some(channel.get_counterparty_htlc_minimum_msat()) } else { None }, + outbound_htlc_maximum_msat: channel.get_counterparty_htlc_maximum_msat(), }, funding_txo: channel.get_funding_txo(), // Note that accept_channel (or open_channel) is always the first message, so @@ -1689,6 +1708,8 @@ impl ChannelMana is_funding_locked: channel.is_usable(), is_usable: channel.is_live(), is_public: channel.should_announce(), + inbound_htlc_minimum_msat: Some(channel.get_holder_htlc_minimum_msat()), + inbound_htlc_maximum_msat: channel.get_holder_htlc_maximum_msat() }); } } @@ -5900,6 +5921,8 @@ impl_writeable_tlv_based!(ChannelCounterparty, { (4, features, required), (6, unspendable_punishment_reserve, required), (8, forwarding_info, option), + (9, outbound_htlc_minimum_msat, option), + (11, outbound_htlc_maximum_msat, option), }); impl_writeable_tlv_based!(ChannelDetails, { @@ -5921,6 +5944,8 @@ impl_writeable_tlv_based!(ChannelDetails, { (28, is_funding_locked, required), (30, is_usable, required), (32, is_public, required), + (33, inbound_htlc_minimum_msat, option), + (35, inbound_htlc_maximum_msat, option), }); impl_writeable_tlv_based!(PhantomRouteHints, { diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 655a53c2..212ac2f1 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1735,6 +1735,8 @@ mod tests { node_id, unspendable_punishment_reserve: 0, forwarding_info: None, + outbound_htlc_minimum_msat: None, + outbound_htlc_maximum_msat: None, }, funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }), channel_type: None, @@ -1750,6 +1752,8 @@ mod tests { force_close_spend_delay: None, is_outbound: true, is_funding_locked: true, is_usable: true, is_public: true, + inbound_htlc_minimum_msat: None, + inbound_htlc_maximum_msat: None, } } @@ -5456,6 +5460,8 @@ mod benches { node_id, unspendable_punishment_reserve: 0, forwarding_info: None, + outbound_htlc_minimum_msat: None, + outbound_htlc_maximum_msat: None, }, funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 @@ -5475,6 +5481,8 @@ mod benches { is_funding_locked: true, is_usable: true, is_public: true, + inbound_htlc_minimum_msat: None, + inbound_htlc_maximum_msat: None, } }