From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon, 9 Dec 2019 21:41:53 +0000 (+0000) Subject: Merge pull request #413 from TheBlueMatt/2019-12-381-nits X-Git-Tag: v0.0.12~170 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=821357ea61c4f636720ebe2b75a8c027b7009521;hp=9f30b305e6ee9c411f1f14aaded665b5001d994f;p=rust-lightning Merge pull request #413 from TheBlueMatt/2019-12-381-nits 381 with a few nits resolved. --- diff --git a/fuzz/fuzz_targets/chanmon_fail_consistency.rs b/fuzz/fuzz_targets/chanmon_fail_consistency.rs index 0af9083b..b34d2421 100644 --- a/fuzz/fuzz_targets/chanmon_fail_consistency.rs +++ b/fuzz/fuzz_targets/chanmon_fail_consistency.rs @@ -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; diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs index b267a464..c7821e27 100644 --- a/fuzz/fuzz_targets/full_stack_target.rs +++ b/fuzz/fuzz_targets/full_stack_target.rs @@ -328,7 +328,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { 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; diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 269aef21..e3edc0e9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4225,7 +4225,7 @@ mod tests { let keys_provider: Arc = 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; diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 4c8f162b..4a1b5e8a 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -845,7 +845,7 @@ pub fn create_network(node_count: usize, node_config: &[Option]) -> 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(); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 1648538f..e5d54dab 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 0b61a569..3c4ab77c 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -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/::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: ::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,