Skip to content

Commit 821357e

Browse files
authored
Merge pull request #413 from TheBlueMatt/2019-12-381-nits
381 with a few nits resolved.
2 parents 9f30b30 + cd31cdb commit 821357e

File tree

6 files changed

+68
-39
lines changed

6 files changed

+68
-39
lines changed

fuzz/fuzz_targets/chanmon_fail_consistency.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ pub fn do_test(data: &[u8]) {
190190
let monitor = Arc::new(TestChannelMonitor::new(watch.clone(), broadcast.clone(), logger.clone(), fee_est.clone()));
191191

192192
let keys_manager = Arc::new(KeyProvider { node_id: $node_id, session_id: atomic::AtomicU8::new(0), channel_id: atomic::AtomicU8::new(0) });
193-
let mut config = UserConfig::new();
193+
let mut config = UserConfig::default();
194194
config.channel_options.fee_proportional_millionths = 0;
195195
config.channel_options.announced_channel = true;
196196
config.peer_channel_config_limits.min_dust_limit_satoshis = 0;
@@ -206,7 +206,7 @@ pub fn do_test(data: &[u8]) {
206206
let monitor = Arc::new(TestChannelMonitor::new(watch.clone(), broadcast.clone(), logger.clone(), fee_est.clone()));
207207

208208
let keys_manager = Arc::new(KeyProvider { node_id: $node_id, session_id: atomic::AtomicU8::new(0), channel_id: atomic::AtomicU8::new(0) });
209-
let mut config = UserConfig::new();
209+
let mut config = UserConfig::default();
210210
config.channel_options.fee_proportional_millionths = 0;
211211
config.channel_options.announced_channel = true;
212212
config.peer_channel_config_limits.min_dust_limit_satoshis = 0;

fuzz/fuzz_targets/full_stack_target.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
328328
let monitor = channelmonitor::SimpleManyChannelMonitor::new(watch.clone(), broadcast.clone(), Arc::clone(&logger), fee_est.clone());
329329

330330
let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), counter: AtomicU64::new(0) });
331-
let mut config = UserConfig::new();
331+
let mut config = UserConfig::default();
332332
config.channel_options.fee_proportional_millionths = slice_to_be32(get_slice!(4));
333333
config.channel_options.announced_channel = get_slice!(1)[0] != 0;
334334
config.peer_channel_config_limits.min_dust_limit_satoshis = 0;

lightning/src/ln/channel.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4225,7 +4225,7 @@ mod tests {
42254225
let keys_provider: Arc<KeysInterface> = Arc::new(Keys { chan_keys });
42264226

42274227
let their_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
4228-
let mut config = UserConfig::new();
4228+
let mut config = UserConfig::default();
42294229
config.channel_options.announced_channel = false;
42304230
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
42314231
chan.their_to_self_delay = 144;

lightning/src/ln/functional_test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,7 @@ pub fn create_network(node_count: usize, node_config: &[Option<UserConfig>]) ->
845845
let chan_monitor = Arc::new(test_utils::TestChannelMonitor::new(chain_monitor.clone(), tx_broadcaster.clone(), logger.clone(), feeest.clone()));
846846
let weak_res = Arc::downgrade(&chan_monitor.simple_monitor);
847847
block_notifier.register_listener(weak_res);
848-
let mut default_config = UserConfig::new();
848+
let mut default_config = UserConfig::default();
849849
default_config.channel_options.announced_channel = true;
850850
default_config.peer_channel_config_limits.force_announced_channel_preference = false;
851851
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();

lightning/src/ln/functional_tests.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,11 +1846,11 @@ fn channel_monitor_network_test() {
18461846
#[test]
18471847
fn test_justice_tx() {
18481848
// Test justice txn built on revoked HTLC-Success tx, against both sides
1849-
let mut alice_config = UserConfig::new();
1849+
let mut alice_config = UserConfig::default();
18501850
alice_config.channel_options.announced_channel = true;
18511851
alice_config.peer_channel_config_limits.force_announced_channel_preference = false;
18521852
alice_config.own_channel_config.our_to_self_delay = 6 * 24 * 5;
1853-
let mut bob_config = UserConfig::new();
1853+
let mut bob_config = UserConfig::default();
18541854
bob_config.channel_options.announced_channel = true;
18551855
bob_config.peer_channel_config_limits.force_announced_channel_preference = false;
18561856
bob_config.own_channel_config.our_to_self_delay = 6 * 24 * 3;
@@ -3388,7 +3388,7 @@ fn test_no_txn_manager_serialize_deserialize() {
33883388
assert!(chan_0_monitor_read.is_empty());
33893389

33903390
let mut nodes_0_read = &nodes_0_serialized[..];
3391-
let config = UserConfig::new();
3391+
let config = UserConfig::default();
33923392
let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
33933393
let (_, nodes_0_deserialized) = {
33943394
let mut channel_monitors = HashMap::new();
@@ -3458,7 +3458,7 @@ fn test_simple_manager_serialize_deserialize() {
34583458
let mut channel_monitors = HashMap::new();
34593459
channel_monitors.insert(chan_0_monitor.get_funding_txo().unwrap(), &chan_0_monitor);
34603460
<(Sha256dHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
3461-
default_config: UserConfig::new(),
3461+
default_config: UserConfig::default(),
34623462
keys_manager,
34633463
fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }),
34643464
monitor: nodes[0].chan_monitor.clone(),
@@ -3518,7 +3518,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
35183518
let mut nodes_0_read = &nodes_0_serialized[..];
35193519
let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::new(test_utils::TestLogger::new())));
35203520
let (_, nodes_0_deserialized) = <(Sha256dHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
3521-
default_config: UserConfig::new(),
3521+
default_config: UserConfig::default(),
35223522
keys_manager,
35233523
fee_estimator: Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }),
35243524
monitor: nodes[0].chan_monitor.clone(),
@@ -5948,7 +5948,7 @@ fn test_upfront_shutdown_script() {
59485948
// BOLT 2 : Option upfront shutdown script, if peer commit its closing_script at channel opening
59495949
// enforce it at shutdown message
59505950

5951-
let mut config = UserConfig::new();
5951+
let mut config = UserConfig::default();
59525952
config.channel_options.announced_channel = true;
59535953
config.peer_channel_config_limits.force_announced_channel_preference = false;
59545954
config.channel_options.commit_upfront_shutdown_pubkey = false;
@@ -6046,9 +6046,9 @@ fn test_upfront_shutdown_script() {
60466046
fn test_user_configurable_csv_delay() {
60476047
// We test our channel constructors yield errors when we pass them absurd csv delay
60486048

6049-
let mut low_our_to_self_config = UserConfig::new();
6049+
let mut low_our_to_self_config = UserConfig::default();
60506050
low_our_to_self_config.own_channel_config.our_to_self_delay = 6;
6051-
let mut high_their_to_self_config = UserConfig::new();
6051+
let mut high_their_to_self_config = UserConfig::default();
60526052
high_their_to_self_config.peer_channel_config_limits.their_to_self_delay = 100;
60536053
let cfgs = [Some(high_their_to_self_config.clone()), None];
60546054
let nodes = create_network(2, &cfgs);
@@ -6135,7 +6135,7 @@ fn test_data_loss_protect() {
61356135
monitor: monitor.clone(),
61366136
logger: Arc::clone(&logger),
61376137
tx_broadcaster,
6138-
default_config: UserConfig::new(),
6138+
default_config: UserConfig::default(),
61396139
channel_monitors: &channel_monitors
61406140
}).unwrap().1;
61416141
nodes[0].node = Arc::new(node_state_0);

lightning/src/util/config.rs

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
use ln::channelmanager::{BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
55

66
/// Top-level config which holds ChannelHandshakeLimits and ChannelConfig.
7+
///
8+
/// Default::default() provides sane defaults for most configurations
9+
/// (but currently with 0 relay fees!)
710
#[derive(Clone, Debug)]
811
pub struct UserConfig {
912
/// Channel config that we propose to our counterparty.
@@ -14,41 +17,44 @@ pub struct UserConfig {
1417
pub channel_options: ChannelConfig,
1518
}
1619

17-
impl UserConfig {
18-
/// Provides sane defaults for most configurations (but with 0 relay fees!)
19-
pub fn new() -> Self{
20+
impl Default for UserConfig {
21+
fn default() -> Self {
2022
UserConfig {
21-
own_channel_config: ChannelHandshakeConfig::new(),
22-
peer_channel_config_limits: ChannelHandshakeLimits::new(),
23-
channel_options: ChannelConfig::new(),
23+
own_channel_config: ChannelHandshakeConfig::default(),
24+
peer_channel_config_limits: ChannelHandshakeLimits::default(),
25+
channel_options: ChannelConfig::default(),
2426
}
2527
}
2628
}
2729

2830
/// Configuration we set when applicable.
31+
///
32+
/// Default::default() provides sane defaults.
2933
#[derive(Clone, Debug)]
3034
pub struct ChannelHandshakeConfig {
3135
/// Confirmations we will wait for before considering the channel locked in.
3236
/// Applied only for inbound channels (see ChannelHandshakeLimits::max_minimum_depth for the
3337
/// equivalent limit applied to outbound channels).
38+
///
39+
/// Default value: 6.
3440
pub minimum_depth: u32,
3541
/// Set to the amount of time we require our counterparty to wait to claim their money.
3642
///
3743
/// It's one of the main parameter of our security model. We (or one of our watchtowers) MUST
3844
/// be online to check for peer having broadcast a revoked transaction to steal our funds
3945
/// at least once every our_to_self_delay blocks.
40-
/// Default is BREAKDOWN_TIMEOUT, we enforce it as a minimum at channel opening so you can
41-
/// tweak config to ask for more security, not less.
4246
///
4347
/// Meanwhile, asking for a too high delay, we bother peer to freeze funds for nothing in
4448
/// case of an honest unilateral channel close, which implicitly decrease the economic value of
4549
/// our channel.
50+
///
51+
/// Default value: BREAKDOWN_TIMEOUT (currently 144), we enforce it as a minimum at channel
52+
/// opening so you can tweak config to ask for more security, not less.
4653
pub our_to_self_delay: u16,
4754
}
4855

49-
impl ChannelHandshakeConfig {
50-
/// Provides sane defaults for `ChannelHandshakeConfig`
51-
pub fn new() -> ChannelHandshakeConfig {
56+
impl Default for ChannelHandshakeConfig {
57+
fn default() -> ChannelHandshakeConfig {
5258
ChannelHandshakeConfig {
5359
minimum_depth: 6,
5460
our_to_self_delay: BREAKDOWN_TIMEOUT,
@@ -61,23 +67,39 @@ impl ChannelHandshakeConfig {
6167
/// These limits are only applied to our counterparty's limits, not our own.
6268
///
6369
/// Use 0/<type>::max_value() as appropriate to skip checking.
70+
///
71+
/// Provides sane defaults for most configurations.
72+
///
73+
/// Most additional limits are disabled except those with which specify a default in individual
74+
/// field documentation. Note that this may result in barely-usable channels, but since they
75+
/// are applied mostly only to incoming channels that's not much of a problem.
6476
#[derive(Copy, Clone, Debug)]
6577
pub struct ChannelHandshakeLimits {
6678
/// Minimum allowed satoshis when a channel is funded, this is supplied by the sender and so
6779
/// only applies to inbound channels.
80+
///
81+
/// Default value: 0.
6882
pub min_funding_satoshis: u64,
6983
/// The remote node sets a limit on the minimum size of HTLCs we can send to them. This allows
7084
/// you to limit the maximum minimum-size they can require.
85+
///
86+
/// Default value: u64::max_value.
7187
pub max_htlc_minimum_msat: u64,
7288
/// The remote node sets a limit on the maximum value of pending HTLCs to them at any given
7389
/// time to limit their funds exposure to HTLCs. This allows you to set a minimum such value.
90+
///
91+
/// Default value: 0.
7492
pub min_max_htlc_value_in_flight_msat: u64,
7593
/// The remote node will require we keep a certain amount in direct payment to ourselves at all
7694
/// time, ensuring that we are able to be punished if we broadcast an old state. This allows to
7795
/// you limit the amount which we will have to keep to ourselves (and cannot use for HTLCs).
96+
///
97+
/// Default value: u64::max_value.
7898
pub max_channel_reserve_satoshis: u64,
7999
/// The remote node sets a limit on the maximum number of pending HTLCs to them at any given
80100
/// time. This allows you to set a minimum such value.
101+
///
102+
/// Default value: 0.
81103
pub min_max_accepted_htlcs: u16,
82104
/// Outputs below a certain value will not be added to on-chain transactions. The dust value is
83105
/// required to always be higher than this value so this only applies to HTLC outputs (and
@@ -86,39 +108,40 @@ pub struct ChannelHandshakeLimits {
86108
/// This setting allows you to set a minimum dust limit for their commitment transactions,
87109
/// reflecting the reality that tiny outputs are not considered standard transactions and will
88110
/// not propagate through the Bitcoin network.
89-
/// Defaults to 546, or the current dust limit on the Bitcoin network.
111+
///
112+
/// Default value: 546, the current dust limit on the Bitcoin network.
90113
pub min_dust_limit_satoshis: u64,
91114
/// Maximum allowed threshold above which outputs will not be generated in their commitment
92115
/// transactions.
93116
/// HTLCs below this amount plus HTLC transaction fees are not enforceable on-chain.
117+
///
118+
/// Default value: u64::max_value.
94119
pub max_dust_limit_satoshis: u64,
95120
/// Before a channel is usable the funding transaction will need to be confirmed by at least a
96121
/// certain number of blocks, specified by the node which is not the funder (as the funder can
97122
/// assume they aren't going to double-spend themselves).
98-
/// This config allows you to set a limit on the maximum amount of time to wait. Defaults to
99-
/// 144 blocks or roughly one day and only applies to outbound channels.
123+
/// This config allows you to set a limit on the maximum amount of time to wait.
124+
///
125+
/// Default value: 144, or roughly one day and only applies to outbound channels.
100126
pub max_minimum_depth: u32,
101127
/// Set to force the incoming channel to match our announced channel preference in
102128
/// ChannelConfig.
103-
/// Defaults to true to make the default that no announced channels are possible (which is
129+
///
130+
/// Default value: true, to make the default that no announced channels are possible (which is
104131
/// appropriate for any nodes which are not online very reliably).
105132
pub force_announced_channel_preference: bool,
106133
/// Set to the amount of time we're willing to wait to claim money back to us.
107134
///
108135
/// Not checking this value would be a security issue, as our peer would be able to set it to
109136
/// max relative lock-time (a year) and we would "lose" money as it would be locked for a long time.
110-
/// Default is MAX_LOCAL_BREAKDOWN_TIMEOUT, which we also enforce as a maximum value
137+
///
138+
/// Default value: MAX_LOCAL_BREAKDOWN_TIMEOUT (1008), which we also enforce as a maximum value
111139
/// so you can tweak config to reduce the loss of having useless locked funds (if your peer accepts)
112140
pub their_to_self_delay: u16
113141
}
114142

115-
impl ChannelHandshakeLimits {
116-
/// Provides sane defaults for most configurations.
117-
///
118-
/// Most additional limits are disabled except those with which specify a default in individual
119-
/// field documentation. Note that this may result in barely-usable channels, but since they
120-
/// are applied mostly only to incoming channels that's not much of a problem.
121-
pub fn new() -> Self {
143+
impl Default for ChannelHandshakeLimits {
144+
fn default() -> Self {
122145
ChannelHandshakeLimits {
123146
min_funding_satoshis: 0,
124147
max_htlc_minimum_msat: <u64>::max_value(),
@@ -141,6 +164,8 @@ pub struct ChannelConfig {
141164
/// Amount (in millionths of a satoshi) the channel will charge per transferred satoshi.
142165
/// This may be allowed to change at runtime in a later update, however doing so must result in
143166
/// update messages sent to notify all nodes of our updated relay fee.
167+
///
168+
/// Default value: 0.
144169
pub fee_proportional_millionths: u32,
145170
/// Set to announce the channel publicly and notify all nodes that they can route via this
146171
/// channel.
@@ -151,6 +176,8 @@ pub struct ChannelConfig {
151176
/// channels unless ChannelHandshakeLimits::force_announced_channel_preferences is set.
152177
///
153178
/// This cannot be changed after the initial channel handshake.
179+
///
180+
/// Default value: false.
154181
pub announced_channel: bool,
155182
/// When set, we commit to an upfront shutdown_pubkey at channel open. If our counterparty
156183
/// supports it, they will then enforce the mutual-close output to us matches what we provided
@@ -161,12 +188,14 @@ pub struct ChannelConfig {
161188
/// lightning payments, so we never require that our counterparties support this option.
162189
///
163190
/// This cannot be changed after a channel has been initialized.
191+
///
192+
/// Default value: true.
164193
pub commit_upfront_shutdown_pubkey: bool
165194
}
166195

167-
impl ChannelConfig {
196+
impl Default for ChannelConfig {
168197
/// Provides sane defaults for most configurations (but with zero relay fees!).
169-
pub fn new() -> Self {
198+
fn default() -> Self {
170199
ChannelConfig {
171200
fee_proportional_millionths: 0,
172201
announced_channel: false,

0 commit comments

Comments
 (0)