Merge pull request #1436 from TheBlueMatt/2022-04-event-process-try-lock
authorvalentinewallace <valentinewallace@users.noreply.github.com>
Tue, 26 Apr 2022 17:02:29 +0000 (13:02 -0400)
committerGitHub <noreply@github.com>
Tue, 26 Apr 2022 17:02:29 +0000 (13:02 -0400)
Reorder the BP loop to make manager persistence more reliable

fuzz/src/router.rs
lightning-background-processor/src/lib.rs
lightning-invoice/src/utils.rs
lightning-persister/src/util.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/routing/router.rs

index bff177b19091b47b6b7f2184e1e826240d63a838..b0f052dbc646f613a1d8c038c88c96c3604ef88e 100644 (file)
@@ -213,6 +213,8 @@ pub fn do_test<Out: test_logger::Output>(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<Out: test_logger::Output>(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[..])
index 255fb245e0543a16ad0a38fbd0e3e267b25954ec..e23737c0adcf16fec641c0d48aac940645c54716 100644 (file)
@@ -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 {
index 35490688b44df2a8fa3c8c13db260383993ff54d..a2edc43afc7a17b02f1b7eb13906e9d7837f874e 100644 (file)
@@ -412,8 +412,8 @@ fn filter_channels(channels: Vec<ChannelDetails>, 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::<EnforcingSigner, &test_utils::TestKeysInterface>(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() {
index 25bd00f5e9539ee8b486b79cb7436278015a5698..4adbb33e5b26fd886fb64d05d3d315efd3f28b16 100644 (file)
@@ -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{}
index 9ccdf816ad398d764d599b62e45e0e1274391572..b0551bf8323f1617f7cba4c991c783221c778c46 100644 (file)
@@ -4321,11 +4321,15 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        /// 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<u64> {
+               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<Signer: Sign> Channel<Signer> {
                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<u64> {
+               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<u64> {
+               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
        }
index 7f66e604d68e40984972645fbdedcb26d8cde6fd..93746f0fde05cbc6b3ae3272a6851f36ef7d993c 100644 (file)
@@ -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<CounterpartyForwardingInfo>,
+       /// 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<u64>,
+       /// The largest value HTLC (in msat) the remote peer currently will accept, for this channel.
+       pub outbound_htlc_maximum_msat: Option<u64>,
 }
 
 /// 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<u64>,
+       /// The largest value HTLC (in msat) we currently will accept, for this channel.
+       pub inbound_htlc_maximum_msat: Option<u64>,
 }
 
 impl ChannelDetails {
@@ -1670,6 +1681,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> 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, {
index 655a53c2dfeab3007c4660f74f04a67022066db6..212ac2f10cae7d62695f032c6fd8ab0a4e523ffa 100644 (file)
@@ -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,
                }
        }