Merge pull request #413 from TheBlueMatt/2019-12-381-nits
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 9 Dec 2019 21:41:53 +0000 (21:41 +0000)
committerGitHub <noreply@github.com>
Mon, 9 Dec 2019 21:41:53 +0000 (21:41 +0000)
381 with a few nits resolved.

fuzz/fuzz_targets/chanmon_fail_consistency.rs
fuzz/fuzz_targets/full_stack_target.rs
lightning/src/ln/channel.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/config.rs

index 0af9083b5a96dca89c7ffbf1d12cd01cf629768f..b34d2421537648d5989f846868261ac85b03480b 100644 (file)
@@ -190,7 +190,7 @@ pub fn do_test(data: &[u8]) {
                        let monitor = Arc::new(TestChannelMonitor::new(watch.clone(), broadcast.clone(), logger.clone(), fee_est.clone()));
 
                        let keys_manager = Arc::new(KeyProvider { node_id: $node_id, session_id: atomic::AtomicU8::new(0), channel_id: atomic::AtomicU8::new(0) });
-                       let mut config = UserConfig::new();
+                       let mut config = UserConfig::default();
                        config.channel_options.fee_proportional_millionths = 0;
                        config.channel_options.announced_channel = true;
                        config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
@@ -206,7 +206,7 @@ pub fn do_test(data: &[u8]) {
                        let monitor = Arc::new(TestChannelMonitor::new(watch.clone(), broadcast.clone(), logger.clone(), fee_est.clone()));
 
                        let keys_manager = Arc::new(KeyProvider { node_id: $node_id, session_id: atomic::AtomicU8::new(0), channel_id: atomic::AtomicU8::new(0) });
-                       let mut config = UserConfig::new();
+                       let mut config = UserConfig::default();
                        config.channel_options.fee_proportional_millionths = 0;
                        config.channel_options.announced_channel = true;
                        config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
index b267a4647c5b1a16cb7cbbc877848a11c758a2c1..c7821e27056f4106644a96d9b5a2d99d9a058ea8 100644 (file)
@@ -328,7 +328,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
        let monitor = channelmonitor::SimpleManyChannelMonitor::new(watch.clone(), broadcast.clone(), Arc::clone(&logger), fee_est.clone());
 
        let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), counter: AtomicU64::new(0) });
-       let mut config = UserConfig::new();
+       let mut config = UserConfig::default();
        config.channel_options.fee_proportional_millionths =  slice_to_be32(get_slice!(4));
        config.channel_options.announced_channel = get_slice!(1)[0] != 0;
        config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
index 269aef215f8b82e60a7180a10b18ef72d9085049..e3edc0e9a98d1b5dfcdcdf24592f01c48fdc0f88 100644 (file)
@@ -4225,7 +4225,7 @@ mod tests {
                let keys_provider: Arc<KeysInterface> = Arc::new(Keys { chan_keys });
 
                let their_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
-               let mut config = UserConfig::new();
+               let mut config = UserConfig::default();
                config.channel_options.announced_channel = false;
                let mut chan = Channel::new_outbound(&feeest, &keys_provider, their_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config).unwrap(); // Nothing uses their network key in this test
                chan.their_to_self_delay = 144;
index 4c8f162bcf53dc41c982346ce4335222ce4e451f..4a1b5e8a790cd7ce57cf93b8261a9fdca40a951a 100644 (file)
@@ -845,7 +845,7 @@ pub fn create_network(node_count: usize, node_config: &[Option<UserConfig>]) ->
                let chan_monitor = Arc::new(test_utils::TestChannelMonitor::new(chain_monitor.clone(), tx_broadcaster.clone(), logger.clone(), feeest.clone()));
                let weak_res = Arc::downgrade(&chan_monitor.simple_monitor);
                block_notifier.register_listener(weak_res);
-               let mut default_config = UserConfig::new();
+               let mut default_config = UserConfig::default();
                default_config.channel_options.announced_channel = true;
                default_config.peer_channel_config_limits.force_announced_channel_preference = false;
                let node = ChannelManager::new(Network::Testnet, feeest.clone(), chan_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger), keys_manager.clone(), if node_config[i].is_some() { node_config[i].clone().unwrap() } else { default_config }, 0).unwrap();
index 1648538fac81382f77a15310a907523560030e4e..e5d54dab0861dacd749e86e75c425daf475777f4 100644 (file)
@@ -1846,11 +1846,11 @@ fn channel_monitor_network_test() {
 #[test]
 fn test_justice_tx() {
        // Test justice txn built on revoked HTLC-Success tx, against both sides
-       let mut alice_config = UserConfig::new();
+       let mut alice_config = UserConfig::default();
        alice_config.channel_options.announced_channel = true;
        alice_config.peer_channel_config_limits.force_announced_channel_preference = false;
        alice_config.own_channel_config.our_to_self_delay = 6 * 24 * 5;
-       let mut bob_config = UserConfig::new();
+       let mut bob_config = UserConfig::default();
        bob_config.channel_options.announced_channel = true;
        bob_config.peer_channel_config_limits.force_announced_channel_preference = false;
        bob_config.own_channel_config.our_to_self_delay = 6 * 24 * 3;
@@ -3388,7 +3388,7 @@ fn test_no_txn_manager_serialize_deserialize() {
        assert!(chan_0_monitor_read.is_empty());
 
        let mut nodes_0_read = &nodes_0_serialized[..];
-       let config = UserConfig::new();
+       let config = UserConfig::default();
        let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
        let (_, nodes_0_deserialized) = {
                let mut channel_monitors = HashMap::new();
@@ -3458,7 +3458,7 @@ fn test_simple_manager_serialize_deserialize() {
                let mut channel_monitors = HashMap::new();
                channel_monitors.insert(chan_0_monitor.get_funding_txo().unwrap(), &chan_0_monitor);
                <(Sha256dHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
-                       default_config: UserConfig::new(),
+                       default_config: UserConfig::default(),
                        keys_manager,
                        fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }),
                        monitor: nodes[0].chan_monitor.clone(),
@@ -3518,7 +3518,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        let mut nodes_0_read = &nodes_0_serialized[..];
        let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
        let (_, nodes_0_deserialized) = <(Sha256dHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
-               default_config: UserConfig::new(),
+               default_config: UserConfig::default(),
                keys_manager,
                fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }),
                monitor: nodes[0].chan_monitor.clone(),
@@ -5948,7 +5948,7 @@ fn test_upfront_shutdown_script() {
        // BOLT 2 : Option upfront shutdown script, if peer commit its closing_script at channel opening
        // enforce it at shutdown message
 
-       let mut config = UserConfig::new();
+       let mut config = UserConfig::default();
        config.channel_options.announced_channel = true;
        config.peer_channel_config_limits.force_announced_channel_preference = false;
        config.channel_options.commit_upfront_shutdown_pubkey = false;
@@ -6046,9 +6046,9 @@ fn test_upfront_shutdown_script() {
 fn test_user_configurable_csv_delay() {
        // We test our channel constructors yield errors when we pass them absurd csv delay
 
-       let mut low_our_to_self_config = UserConfig::new();
+       let mut low_our_to_self_config = UserConfig::default();
        low_our_to_self_config.own_channel_config.our_to_self_delay = 6;
-       let mut high_their_to_self_config = UserConfig::new();
+       let mut high_their_to_self_config = UserConfig::default();
        high_their_to_self_config.peer_channel_config_limits.their_to_self_delay = 100;
        let cfgs = [Some(high_their_to_self_config.clone()), None];
        let nodes = create_network(2, &cfgs);
@@ -6135,7 +6135,7 @@ fn test_data_loss_protect() {
                monitor: monitor.clone(),
                logger: Arc::clone(&logger),
                tx_broadcaster,
-               default_config: UserConfig::new(),
+               default_config: UserConfig::default(),
                channel_monitors: &channel_monitors
        }).unwrap().1;
        nodes[0].node = Arc::new(node_state_0);
index 0b61a5699ea853489d1a59901584784edcba7c0f..3c4ab77c16bc1d921d6057c21bb620fe410dce68 100644 (file)
@@ -4,6 +4,9 @@
 use ln::channelmanager::{BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 
 /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig.
+///
+/// Default::default() provides sane defaults for most configurations
+/// (but currently with 0 relay fees!)
 #[derive(Clone, Debug)]
 pub struct UserConfig {
        /// Channel config that we propose to our counterparty.
@@ -14,41 +17,44 @@ pub struct UserConfig {
        pub channel_options: ChannelConfig,
 }
 
-impl UserConfig {
-       /// Provides sane defaults for most configurations (but with 0 relay fees!)
-       pub fn new() -> Self{
+impl Default for UserConfig {
+       fn default() -> Self {
                UserConfig {
-                       own_channel_config: ChannelHandshakeConfig::new(),
-                       peer_channel_config_limits: ChannelHandshakeLimits::new(),
-                       channel_options: ChannelConfig::new(),
+                       own_channel_config: ChannelHandshakeConfig::default(),
+                       peer_channel_config_limits: ChannelHandshakeLimits::default(),
+                       channel_options: ChannelConfig::default(),
                }
        }
 }
 
 /// Configuration we set when applicable.
+///
+/// Default::default() provides sane defaults.
 #[derive(Clone, Debug)]
 pub struct ChannelHandshakeConfig {
        /// Confirmations we will wait for before considering the channel locked in.
        /// Applied only for inbound channels (see ChannelHandshakeLimits::max_minimum_depth for the
        /// equivalent limit applied to outbound channels).
+       ///
+       /// Default value: 6.
        pub minimum_depth: u32,
        /// Set to the amount of time we require our counterparty to wait to claim their money.
        ///
        /// It's one of the main parameter of our security model. We (or one of our watchtowers) MUST
        /// be online to check for peer having broadcast a revoked transaction to steal our funds
        /// at least once every our_to_self_delay blocks.
-       /// Default is BREAKDOWN_TIMEOUT, we enforce it as a minimum at channel opening so you can
-       /// tweak config to ask for more security, not less.
        ///
        /// Meanwhile, asking for a too high delay, we bother peer to freeze funds for nothing in
        /// case of an honest unilateral channel close, which implicitly decrease the economic value of
        /// our channel.
+       ///
+       /// Default value: BREAKDOWN_TIMEOUT (currently 144), we enforce it as a minimum at channel
+       /// opening so you can tweak config to ask for more security, not less.
        pub our_to_self_delay: u16,
 }
 
-impl ChannelHandshakeConfig {
-       /// Provides sane defaults for `ChannelHandshakeConfig`
-       pub fn new() -> ChannelHandshakeConfig {
+impl Default for ChannelHandshakeConfig {
+       fn default() -> ChannelHandshakeConfig {
                ChannelHandshakeConfig {
                        minimum_depth: 6,
                        our_to_self_delay: BREAKDOWN_TIMEOUT,
@@ -61,23 +67,39 @@ impl ChannelHandshakeConfig {
 /// These limits are only applied to our counterparty's limits, not our own.
 ///
 /// Use 0/<type>::max_value() as appropriate to skip checking.
+///
+/// Provides sane defaults for most configurations.
+///
+/// Most additional limits are disabled except those with which specify a default in individual
+/// field documentation. Note that this may result in barely-usable channels, but since they
+/// are applied mostly only to incoming channels that's not much of a problem.
 #[derive(Copy, Clone, Debug)]
 pub struct ChannelHandshakeLimits {
        /// Minimum allowed satoshis when a channel is funded, this is supplied by the sender and so
        /// only applies to inbound channels.
+       ///
+       /// Default value: 0.
        pub min_funding_satoshis: u64,
        /// The remote node sets a limit on the minimum size of HTLCs we can send to them. This allows
        /// you to limit the maximum minimum-size they can require.
+       ///
+       /// Default value: u64::max_value.
        pub max_htlc_minimum_msat: u64,
        /// The remote node sets a limit on the maximum value of pending HTLCs to them at any given
        /// time to limit their funds exposure to HTLCs. This allows you to set a minimum such value.
+       ///
+       /// Default value: 0.
        pub min_max_htlc_value_in_flight_msat: u64,
        /// The remote node will require we keep a certain amount in direct payment to ourselves at all
        /// time, ensuring that we are able to be punished if we broadcast an old state. This allows to
        /// you limit the amount which we will have to keep to ourselves (and cannot use for HTLCs).
+       ///
+       /// Default value: u64::max_value.
        pub max_channel_reserve_satoshis: u64,
        /// The remote node sets a limit on the maximum number of pending HTLCs to them at any given
        /// time. This allows you to set a minimum such value.
+       ///
+       /// Default value: 0.
        pub min_max_accepted_htlcs: u16,
        /// Outputs below a certain value will not be added to on-chain transactions. The dust value is
        /// required to always be higher than this value so this only applies to HTLC outputs (and
@@ -86,39 +108,40 @@ pub struct ChannelHandshakeLimits {
        /// This setting allows you to set a minimum dust limit for their commitment transactions,
        /// reflecting the reality that tiny outputs are not considered standard transactions and will
        /// not propagate through the Bitcoin network.
-       /// Defaults to 546, or the current dust limit on the Bitcoin network.
+       ///
+       /// Default value: 546, the current dust limit on the Bitcoin network.
        pub min_dust_limit_satoshis: u64,
        /// Maximum allowed threshold above which outputs will not be generated in their commitment
        /// transactions.
        /// HTLCs below this amount plus HTLC transaction fees are not enforceable on-chain.
+       ///
+       /// Default value: u64::max_value.
        pub max_dust_limit_satoshis: u64,
        /// Before a channel is usable the funding transaction will need to be confirmed by at least a
        /// certain number of blocks, specified by the node which is not the funder (as the funder can
        /// assume they aren't going to double-spend themselves).
-       /// This config allows you to set a limit on the maximum amount of time to wait. Defaults to
-       /// 144 blocks or roughly one day and only applies to outbound channels.
+       /// This config allows you to set a limit on the maximum amount of time to wait.
+       ///
+       /// Default value: 144, or roughly one day and only applies to outbound channels.
        pub max_minimum_depth: u32,
        /// Set to force the incoming channel to match our announced channel preference in
        /// ChannelConfig.
-       /// Defaults to true to make the default that no announced channels are possible (which is
+       ///
+       /// Default value: true, to make the default that no announced channels are possible (which is
        /// appropriate for any nodes which are not online very reliably).
        pub force_announced_channel_preference: bool,
        /// Set to the amount of time we're willing to wait to claim money back to us.
        ///
        /// Not checking this value would be a security issue, as our peer would be able to set it to
        /// max relative lock-time (a year) and we would "lose" money as it would be locked for a long time.
-       /// Default is MAX_LOCAL_BREAKDOWN_TIMEOUT, which we also enforce as a maximum value
+       ///
+       /// Default value: MAX_LOCAL_BREAKDOWN_TIMEOUT (1008), which we also enforce as a maximum value
        /// so you can tweak config to reduce the loss of having useless locked funds (if your peer accepts)
        pub their_to_self_delay: u16
 }
 
-impl ChannelHandshakeLimits {
-       /// Provides sane defaults for most configurations.
-       ///
-       /// Most additional limits are disabled except those with which specify a default in individual
-       /// field documentation. Note that this may result in barely-usable channels, but since they
-       /// are applied mostly only to incoming channels that's not much of a problem.
-       pub fn new() -> Self {
+impl Default for ChannelHandshakeLimits {
+       fn default() -> Self {
                ChannelHandshakeLimits {
                        min_funding_satoshis: 0,
                        max_htlc_minimum_msat: <u64>::max_value(),
@@ -141,6 +164,8 @@ pub struct ChannelConfig {
        /// Amount (in millionths of a satoshi) the channel will charge per transferred satoshi.
        /// This may be allowed to change at runtime in a later update, however doing so must result in
        /// update messages sent to notify all nodes of our updated relay fee.
+       ///
+       /// Default value: 0.
        pub fee_proportional_millionths: u32,
        /// Set to announce the channel publicly and notify all nodes that they can route via this
        /// channel.
@@ -151,6 +176,8 @@ pub struct ChannelConfig {
        /// channels unless ChannelHandshakeLimits::force_announced_channel_preferences is set.
        ///
        /// This cannot be changed after the initial channel handshake.
+       ///
+       /// Default value: false.
        pub announced_channel: bool,
        /// When set, we commit to an upfront shutdown_pubkey at channel open. If our counterparty
        /// supports it, they will then enforce the mutual-close output to us matches what we provided
@@ -161,12 +188,14 @@ pub struct ChannelConfig {
        /// lightning payments, so we never require that our counterparties support this option.
        ///
        /// This cannot be changed after a channel has been initialized.
+       ///
+       /// Default value: true.
        pub commit_upfront_shutdown_pubkey: bool
 }
 
-impl ChannelConfig {
+impl Default for ChannelConfig {
        /// Provides sane defaults for most configurations (but with zero relay fees!).
-       pub fn new() -> Self {
+       fn default() -> Self {
                ChannelConfig {
                        fee_proportional_millionths: 0,
                        announced_channel: false,