From 850ca13fbca5418c08c52313213acb675a73ad00 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 9 Jun 2022 16:11:15 -0700 Subject: [PATCH 1/3] Move announced_channel to ChannelHandshakeConfig In the near future, we plan to allow users to update their `ChannelConfig` after the initial channel handshake. In order to reuse the same struct and expose it to users, we opt to move out all static fields that cannot be updated after the initial channel handshake. --- fuzz/src/chanmon_consistency.rs | 4 ++-- fuzz/src/full_stack.rs | 2 +- lightning-invoice/src/utils.rs | 4 ++-- lightning/src/ln/channel.rs | 6 +++--- lightning/src/ln/functional_test_utils.rs | 4 ++-- lightning/src/ln/functional_tests.rs | 12 ++++++------ lightning/src/ln/onion_route_tests.rs | 6 +++--- lightning/src/ln/priv_short_conf_tests.rs | 18 +++++++++--------- lightning/src/ln/shutdown_tests.rs | 10 +++++----- lightning/src/util/config.rs | 19 +++++++++++++++---- 10 files changed, 48 insertions(+), 37 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 9625677c2..8e70ba42e 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -357,7 +357,7 @@ pub fn do_test(data: &[u8], underlying_out: Out) { let mut config = UserConfig::default(); config.channel_options.forwarding_fee_proportional_millionths = 0; - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; let network = Network::Bitcoin; let params = ChainParameters { network, @@ -377,7 +377,7 @@ pub fn do_test(data: &[u8], underlying_out: Out) { let mut config = UserConfig::default(); config.channel_options.forwarding_fee_proportional_millionths = 0; - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; let mut monitors = HashMap::new(); let mut old_monitors = $old_monitors.latest_monitors.lock().unwrap(); diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index eb270f679..782457cfe 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -383,7 +383,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { let keys_manager = Arc::new(KeyProvider { node_secret: our_network_key.clone(), inbound_payment_key: KeyMaterial(inbound_payment_key.try_into().unwrap()), counter: AtomicU64::new(0) }); let mut config = UserConfig::default(); config.channel_options.forwarding_fee_proportional_millionths = slice_to_be32(get_slice!(4)); - config.channel_options.announced_channel = get_slice!(1)[0] != 0; + config.own_channel_config.announced_channel = get_slice!(1)[0] != 0; let network = Network::Bitcoin; let params = ChainParameters { network, diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index d42bc503f..c79e59045 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -659,7 +659,7 @@ mod test { // `msgs::ChannelUpdate` is never handled for the node(s). As the `msgs::ChannelUpdate` // is never handled, the `channel.counterparty.forwarding_info` is never assigned. let mut private_chan_cfg = UserConfig::default(); - private_chan_cfg.channel_options.announced_channel = false; + private_chan_cfg.own_channel_config.announced_channel = false; let temporary_channel_id = nodes[2].node.create_channel(nodes[0].node.get_our_node_id(), 1_000_000, 500_000_000, 42, Some(private_chan_cfg)).unwrap(); let open_channel = get_event_msg!(nodes[2], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id()); nodes[0].node.handle_open_channel(&nodes[2].node.get_our_node_id(), InitFeatures::known(), &open_channel); @@ -1046,7 +1046,7 @@ mod test { // `msgs::ChannelUpdate` is never handled for the node(s). As the `msgs::ChannelUpdate` // is never handled, the `channel.counterparty.forwarding_info` is never assigned. let mut private_chan_cfg = UserConfig::default(); - private_chan_cfg.channel_options.announced_channel = false; + private_chan_cfg.own_channel_config.announced_channel = false; let temporary_channel_id = nodes[1].node.create_channel(nodes[3].node.get_our_node_id(), 1_000_000, 500_000_000, 42, Some(private_chan_cfg)).unwrap(); let open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[3].node.get_our_node_id()); nodes[3].node.handle_open_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4daaf630a..c2b33759f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -855,7 +855,7 @@ impl Channel { // available. If it's private, we first try `scid_privacy` as it provides better privacy // with no other changes, and fall back to `only_static_remotekey` let mut ret = ChannelTypeFeatures::only_static_remote_key(); - if !config.channel_options.announced_channel && config.own_channel_config.negotiate_scid_privacy { + if !config.own_channel_config.announced_channel && config.own_channel_config.negotiate_scid_privacy { ret.set_scid_privacy_required(); } ret @@ -1182,7 +1182,7 @@ impl Channel { // Convert things into internal flags and prep our state: if config.peer_channel_config_limits.force_announced_channel_preference { - if local_config.announced_channel != announced_channel { + if config.own_channel_config.announced_channel != announced_channel { return Err(ChannelError::Close("Peer tried to open channel but their announcement preference is different from ours".to_owned())); } } @@ -6918,7 +6918,7 @@ mod tests { let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let mut config = UserConfig::default(); - config.channel_options.announced_channel = false; + config.own_channel_config.announced_channel = false; let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config, 0, 42).unwrap(); // Nothing uses their network key in this test chan.holder_dust_limit_satoshis = 546; chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 087312717..300efb12b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -772,7 +772,7 @@ pub fn create_announced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a Vec>, a: usize, b: usize, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::ChannelReady, Transaction) { let mut no_announce_cfg = test_default_channel_config(); - no_announce_cfg.channel_options.announced_channel = false; + no_announce_cfg.own_channel_config.announced_channel = false; nodes[a].node.create_channel(nodes[b].node.get_our_node_id(), channel_value, push_msat, 42, Some(no_announce_cfg)).unwrap(); let open_channel = get_event_msg!(nodes[a], MessageSendEvent::SendOpenChannel, nodes[b].node.get_our_node_id()); nodes[b].node.handle_open_channel(&nodes[a].node.get_our_node_id(), a_flags, &open_channel); @@ -1956,7 +1956,7 @@ pub fn test_default_channel_config() -> UserConfig { // Set cltv_expiry_delta slightly lower to keep the final CLTV values inside one byte in our // tests so that our script-length checks don't fail (see ACCEPTED_HTLC_SCRIPT_WEIGHT). default_config.channel_options.cltv_expiry_delta = MIN_CLTV_EXPIRY_DELTA; - default_config.channel_options.announced_channel = true; + default_config.own_channel_config.announced_channel = true; default_config.peer_channel_config_limits.force_announced_channel_preference = false; // When most of our tests were written, the default HTLC minimum was fixed at 1000. // It now defaults to 1, so we simply set it to the expected value here. diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 5a88d1b9e..161e6af2d 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2362,11 +2362,11 @@ fn channel_monitor_network_test() { fn test_justice_tx() { // Test justice txn built on revoked HTLC-Success tx, against both sides let mut alice_config = UserConfig::default(); - alice_config.channel_options.announced_channel = true; + alice_config.own_channel_config.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::default(); - bob_config.channel_options.announced_channel = true; + bob_config.own_channel_config.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; let user_cfgs = [Some(alice_config), Some(bob_config)]; @@ -8282,16 +8282,16 @@ fn test_channel_update_has_correct_htlc_maximum_msat() { // 2. MUST be set to less than or equal to the `max_htlc_value_in_flight_msat` received from the peer. let mut config_30_percent = UserConfig::default(); - config_30_percent.channel_options.announced_channel = true; + config_30_percent.own_channel_config.announced_channel = true; config_30_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 30; let mut config_50_percent = UserConfig::default(); - config_50_percent.channel_options.announced_channel = true; + config_50_percent.own_channel_config.announced_channel = true; config_50_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 50; let mut config_95_percent = UserConfig::default(); - config_95_percent.channel_options.announced_channel = true; + config_95_percent.own_channel_config.announced_channel = true; config_95_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 95; let mut config_100_percent = UserConfig::default(); - config_100_percent.channel_options.announced_channel = true; + config_100_percent.own_channel_config.announced_channel = true; config_100_percent.own_channel_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100; let chanmon_cfgs = create_chanmon_cfgs(4); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index eabd22220..afbf87e00 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -308,7 +308,7 @@ fn test_onion_failure() { // Channel::get_counterparty_htlc_minimum_msat(). let mut node_2_cfg: UserConfig = Default::default(); node_2_cfg.own_channel_config.our_htlc_minimum_msat = 2000; - node_2_cfg.channel_options.announced_channel = true; + node_2_cfg.own_channel_config.announced_channel = true; node_2_cfg.peer_channel_config_limits.force_announced_channel_preference = false; // When this test was written, the default base fee floated based on the HTLC count. @@ -600,7 +600,7 @@ fn test_default_to_onion_payload_tlv_format() { // `features` for a node in the `network_graph` exists, or when the node isn't in the // `network_graph`, and no other known `features` for the node exists. let mut priv_channels_conf = UserConfig::default(); - priv_channels_conf.channel_options.announced_channel = false; + priv_channels_conf.own_channel_config.announced_channel = false; let chanmon_cfgs = create_chanmon_cfgs(5); let node_cfgs = create_node_cfgs(5, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(5, &node_cfgs, &[None, None, None, None, Some(priv_channels_conf)]); @@ -1085,7 +1085,7 @@ fn test_phantom_dust_exposure_failure() { let max_dust_exposure = 546; let mut receiver_config = UserConfig::default(); receiver_config.channel_options.max_dust_htlc_exposure_msat = max_dust_exposure; - receiver_config.channel_options.announced_channel = true; + receiver_config.own_channel_config.announced_channel = true; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 051703a7f..f62275c9d 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -171,11 +171,11 @@ fn do_test_1_conf_open(connect_style: ConnectStyle) { // tests that we properly send one in that case. let mut alice_config = UserConfig::default(); alice_config.own_channel_config.minimum_depth = 1; - alice_config.channel_options.announced_channel = true; + alice_config.own_channel_config.announced_channel = true; alice_config.peer_channel_config_limits.force_announced_channel_preference = false; let mut bob_config = UserConfig::default(); bob_config.own_channel_config.minimum_depth = 1; - bob_config.channel_options.announced_channel = true; + bob_config.own_channel_config.announced_channel = true; bob_config.peer_channel_config_limits.force_announced_channel_preference = false; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -308,7 +308,7 @@ fn test_scid_privacy_on_pub_channel() { let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let mut scid_privacy_cfg = test_default_channel_config(); - scid_privacy_cfg.channel_options.announced_channel = true; + scid_privacy_cfg.own_channel_config.announced_channel = true; scid_privacy_cfg.own_channel_config.negotiate_scid_privacy = true; nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(scid_privacy_cfg)).unwrap(); let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); @@ -332,7 +332,7 @@ fn test_scid_privacy_negotiation() { let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let mut scid_privacy_cfg = test_default_channel_config(); - scid_privacy_cfg.channel_options.announced_channel = false; + scid_privacy_cfg.own_channel_config.announced_channel = false; scid_privacy_cfg.own_channel_config.negotiate_scid_privacy = true; nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(scid_privacy_cfg)).unwrap(); @@ -378,7 +378,7 @@ fn test_inbound_scid_privacy() { create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0, InitFeatures::known(), InitFeatures::known()); let mut no_announce_cfg = test_default_channel_config(); - no_announce_cfg.channel_options.announced_channel = false; + no_announce_cfg.own_channel_config.announced_channel = false; no_announce_cfg.own_channel_config.negotiate_scid_privacy = true; nodes[1].node.create_channel(nodes[2].node.get_our_node_id(), 100_000, 10_000, 42, Some(no_announce_cfg)).unwrap(); let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[2].node.get_our_node_id()); @@ -665,7 +665,7 @@ fn test_0conf_channel_with_async_monitor() { create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0, InitFeatures::known(), InitFeatures::known()); - chan_config.channel_options.announced_channel = false; + chan_config.own_channel_config.announced_channel = false; nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(chan_config)).unwrap(); let open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); @@ -811,7 +811,7 @@ fn test_0conf_close_no_early_chan_update() { let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // This is the default but we force it on anyway - chan_config.channel_options.announced_channel = true; + chan_config.own_channel_config.announced_channel = true; open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); // We can use the channel immediately, but won't generate a channel_update until we get confs @@ -835,7 +835,7 @@ fn test_public_0conf_channel() { let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // This is the default but we force it on anyway - chan_config.channel_options.announced_channel = true; + chan_config.own_channel_config.announced_channel = true; let tx = open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); // We can use the channel immediately, but we can't announce it until we get 6+ confirmations @@ -888,7 +888,7 @@ fn test_0conf_channel_reorg() { let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // This is the default but we force it on anyway - chan_config.channel_options.announced_channel = true; + chan_config.own_channel_config.announced_channel = true; let tx = open_zero_conf_channel(&nodes[0], &nodes[1], Some(chan_config)); // We can use the channel immediately, but we can't announce it until we get 6+ confirmations diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 3dc245877..3e45c2c76 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -409,7 +409,7 @@ fn test_upfront_shutdown_script() { // enforce it at shutdown message let mut config = UserConfig::default(); - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; config.channel_options.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; @@ -574,7 +574,7 @@ fn test_invalid_upfront_shutdown_script() { #[test] fn test_segwit_v0_shutdown_script() { let mut config = UserConfig::default(); - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; config.channel_options.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; @@ -609,7 +609,7 @@ fn test_segwit_v0_shutdown_script() { #[test] fn test_anysegwit_shutdown_script() { let mut config = UserConfig::default(); - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; config.channel_options.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; @@ -644,7 +644,7 @@ fn test_anysegwit_shutdown_script() { #[test] fn test_unsupported_anysegwit_shutdown_script() { let mut config = UserConfig::default(); - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; config.channel_options.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; @@ -686,7 +686,7 @@ fn test_unsupported_anysegwit_shutdown_script() { #[test] fn test_invalid_shutdown_script() { let mut config = UserConfig::default(); - config.channel_options.announced_channel = true; + config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; config.channel_options.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index bdd222e31..1410767f8 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -94,7 +94,7 @@ pub struct ChannelHandshakeConfig { /// private channel without that option. /// /// Ignored if the channel is negotiated to be announced, see - /// [`ChannelConfig::announced_channel`] and + /// [`ChannelHandshakeConfig::announced_channel`] and /// [`ChannelHandshakeLimits::force_announced_channel_preference`] for more. /// /// Default value: false. This value is likely to change to true in the future. @@ -102,6 +102,16 @@ pub struct ChannelHandshakeConfig { /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [`DecodeError:InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue pub negotiate_scid_privacy: bool, + /// Set to announce the channel publicly and notify all nodes that they can route via this + /// channel. + /// + /// This should only be set to true for nodes which expect to be online reliably. + /// + /// As the node which funds a channel picks this value this will only apply for new outbound + /// channels unless [`ChannelHandshakeLimits::force_announced_channel_preference`] is set. + /// + /// Default value: false. + pub announced_channel: bool, } impl Default for ChannelHandshakeConfig { @@ -112,6 +122,7 @@ impl Default for ChannelHandshakeConfig { our_htlc_minimum_msat: 1, max_inbound_htlc_value_in_flight_percent_of_channel: 10, negotiate_scid_privacy: false, + announced_channel: false, } } } @@ -186,10 +197,10 @@ pub struct ChannelHandshakeLimits { /// Default value: true pub trust_own_funding_0conf: bool, /// Set to force an incoming channel to match our announced channel preference in - /// [`ChannelConfig::announced_channel`]. + /// [`ChannelHandshakeConfig::announced_channel`]. /// /// For a node which is not online reliably, this should be set to true and - /// [`ChannelConfig::announced_channel`] set to false, ensuring that no announced (aka public) + /// [`ChannelHandshakeConfig::announced_channel`] set to false, ensuring that no announced (aka public) /// channels will ever be opened. /// /// Default value: true. @@ -372,7 +383,7 @@ pub struct UserConfig { /// node which is not online reliably. /// /// For nodes which are not online reliably, you should set all channels to *not* be announced - /// (using [`ChannelConfig::announced_channel`] and + /// (using [`ChannelHandshakeConfig::announced_channel`] and /// [`ChannelHandshakeLimits::force_announced_channel_preference`]) and set this to false to /// ensure you are not exposed to any forwarding risk. /// From 8027c2ff06c790016b1a9e1d67af065c44d2995d Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 8 Jun 2022 15:40:58 -0700 Subject: [PATCH 2/3] Move commit_upfront_shutdown_pubkey to ChannelHandshakeConfig As like the previous commit, `commit_upfront_shutdown_pubkey` is another static field that cannot change after the initial channel handshake. We therefore move it out from its existing place in `ChannelConfig`. --- lightning/src/ln/chanmon_update_fail_tests.rs | 6 +++--- lightning/src/ln/channel.rs | 4 ++-- lightning/src/ln/shutdown_tests.rs | 10 +++++----- lightning/src/util/config.rs | 19 +++++++++++++++++-- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 495b50742..710027706 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -2541,7 +2541,7 @@ fn test_temporary_error_during_shutdown() { // Test that temporary failures when updating the monitor's shutdown script delay cooperative // close. let mut config = test_default_channel_config(); - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -2596,7 +2596,7 @@ fn test_permanent_error_during_sending_shutdown() { // Test that permanent failures when updating the monitor's shutdown script result in a force // close when initiating a cooperative close. let mut config = test_default_channel_config(); - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -2617,7 +2617,7 @@ fn test_permanent_error_during_handling_shutdown() { // Test that permanent failures when updating the monitor's shutdown script result in a force // close when handling a cooperative close. let mut config = test_default_channel_config(); - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c2b33759f..c9e500007 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -918,7 +918,7 @@ impl Channel { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); - let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + let shutdown_scriptpubkey = if config.own_channel_config.commit_upfront_shutdown_pubkey { Some(keys_provider.get_shutdown_scriptpubkey()) } else { None }; @@ -1239,7 +1239,7 @@ impl Channel { } } else { None }; - let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + let shutdown_scriptpubkey = if config.own_channel_config.commit_upfront_shutdown_pubkey { Some(keys_provider.get_shutdown_scriptpubkey()) } else { None }; diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 3e45c2c76..edfa596f6 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -411,7 +411,7 @@ fn test_upfront_shutdown_script() { let mut config = UserConfig::default(); config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); @@ -576,7 +576,7 @@ fn test_segwit_v0_shutdown_script() { let mut config = UserConfig::default(); config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); @@ -611,7 +611,7 @@ fn test_anysegwit_shutdown_script() { let mut config = UserConfig::default(); config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); @@ -646,7 +646,7 @@ fn test_unsupported_anysegwit_shutdown_script() { let mut config = UserConfig::default(); config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); @@ -688,7 +688,7 @@ fn test_invalid_shutdown_script() { let mut config = UserConfig::default(); config.own_channel_config.announced_channel = true; config.peer_channel_config_limits.force_announced_channel_preference = false; - config.channel_options.commit_upfront_shutdown_pubkey = false; + config.own_channel_config.commit_upfront_shutdown_pubkey = false; let user_cfgs = [None, Some(config), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 1410767f8..c2c73d8cb 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -87,7 +87,7 @@ pub struct ChannelHandshakeConfig { /// /// If this option is set, channels may be created that will not be readable by LDK versions /// prior to 0.0.106, causing [`ChannelManager`]'s read method to return a - /// [`DecodeError:InvalidValue`]. + /// [`DecodeError::InvalidValue`]. /// /// Note that setting this to true does *not* prevent us from opening channels with /// counterparties that do not support the `scid_alias` option; we will simply fall back to a @@ -100,7 +100,7 @@ pub struct ChannelHandshakeConfig { /// Default value: false. This value is likely to change to true in the future. /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager - /// [`DecodeError:InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue + /// [`DecodeError::InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue pub negotiate_scid_privacy: bool, /// Set to announce the channel publicly and notify all nodes that they can route via this /// channel. @@ -112,6 +112,20 @@ pub struct ChannelHandshakeConfig { /// /// 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 + /// at intialization, preventing us from closing to an alternate pubkey. + /// + /// This is set to true by default to provide a slight increase in security, though ultimately + /// any attacker who is able to take control of a channel can just as easily send the funds via + /// lightning payments, so we never require that our counterparties support this option. + /// + /// The upfront key committed is provided from [`KeysInterface::get_shutdown_scriptpubkey`]. + /// + /// Default value: true. + /// + /// [`KeysInterface::get_shutdown_scriptpubkey`]: crate::chain::keysinterface::KeysInterface::get_shutdown_scriptpubkey + pub commit_upfront_shutdown_pubkey: bool, } impl Default for ChannelHandshakeConfig { @@ -123,6 +137,7 @@ impl Default for ChannelHandshakeConfig { max_inbound_htlc_value_in_flight_percent_of_channel: 10, negotiate_scid_privacy: false, announced_channel: false, + commit_upfront_shutdown_pubkey: true, } } } From ec7ccf0415d665441d74edbc479fb9ad357c2751 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 9 Jun 2022 14:01:56 -0700 Subject: [PATCH 3/3] Introduce LegacyChannelConfig to remain backwards compatible ChannelConfig now has its static fields removed. We introduce a new LegacyChannelConfig struct that maintains the serialization as previously defined by ChannelConfig to remain backwards compatible with clients running 0.0.107 and earlier. --- lightning/src/ln/channel.rs | 41 +++++---- lightning/src/ln/functional_test_utils.rs | 4 +- lightning/src/ln/payment_tests.rs | 3 +- lightning/src/util/config.rs | 106 +++++++++++++++------- 4 files changed, 101 insertions(+), 53 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c9e500007..a168177fd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -39,7 +39,7 @@ use util::events::ClosureReason; use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use util::logger::Logger; use util::errors::APIError; -use util::config::{UserConfig, ChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits}; +use util::config::{UserConfig, LegacyChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits}; use util::scid_utils::scid_from_parts; use io; @@ -491,9 +491,9 @@ pub(crate) const MIN_AFFORDABLE_HTLC_COUNT: usize = 4; // Counterparty designates channel data owned by the another channel participant entity. pub(super) struct Channel { #[cfg(any(test, feature = "_test_utils"))] - pub(crate) config: ChannelConfig, + pub(crate) config: LegacyChannelConfig, #[cfg(not(any(test, feature = "_test_utils")))] - config: ChannelConfig, + config: LegacyChannelConfig, inbound_handshake_limits_override: Option, @@ -930,7 +930,13 @@ impl Channel { Ok(Channel { user_id, - config: config.channel_options.clone(), + + config: LegacyChannelConfig { + mutable: config.channel_options.clone(), + announced_channel: config.own_channel_config.announced_channel, + commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey, + }, + inbound_handshake_limits_override: Some(config.peer_channel_config_limits.clone()), channel_id: keys_provider.get_secure_random_bytes(), @@ -1117,7 +1123,6 @@ impl Channel { delayed_payment_basepoint: msg.delayed_payment_basepoint, htlc_basepoint: msg.htlc_basepoint }; - let mut local_config = (*config).channel_options.clone(); if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT { return Err(ChannelError::Close(format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks. It must be greater than {}", config.own_channel_config.our_to_self_delay, BREAKDOWN_TIMEOUT))); @@ -1186,8 +1191,6 @@ impl Channel { return Err(ChannelError::Close("Peer tried to open channel but their announcement preference is different from ours".to_owned())); } } - // we either accept their preference or the preferences match - local_config.announced_channel = announced_channel; let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis); if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { @@ -1254,7 +1257,13 @@ impl Channel { let chan = Channel { user_id, - config: local_config, + + config: LegacyChannelConfig { + mutable: config.channel_options.clone(), + announced_channel, + commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey, + }, + inbound_handshake_limits_override: None, channel_id: msg.temporary_channel_id, @@ -4011,7 +4020,7 @@ impl Channel { // We always add force_close_avoidance_max_fee_satoshis to our normal // feerate-calculated fee, but allow the max to be overridden if we're using a // target feerate-calculated fee. - cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.config.force_close_avoidance_max_fee_satoshis, + cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.config.mutable.force_close_avoidance_max_fee_satoshis, proposed_max_feerate as u64 * tx_weight / 1000) } else { self.channel_value_satoshis - (self.value_to_self_msat + 999) / 1000 @@ -4471,15 +4480,15 @@ impl Channel { } pub fn get_fee_proportional_millionths(&self) -> u32 { - self.config.forwarding_fee_proportional_millionths + self.config.mutable.forwarding_fee_proportional_millionths } pub fn get_cltv_expiry_delta(&self) -> u16 { - cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) + cmp::max(self.config.mutable.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) } pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 { - self.config.max_dust_htlc_exposure_msat + self.config.mutable.max_dust_htlc_exposure_msat } pub fn get_feerate(&self) -> u32 { @@ -4566,7 +4575,7 @@ impl Channel { /// Gets the fee we'd want to charge for adding an HTLC output to this Channel /// Allowed in any state (including after shutdown) pub fn get_outbound_forwarding_fee_base_msat(&self) -> u32 { - self.config.forwarding_fee_base_msat + self.config.mutable.forwarding_fee_base_msat } /// Returns true if we've ever received a message from the remote end for this Channel @@ -6022,11 +6031,11 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel let user_id = Readable::read(reader)?; - let mut config = Some(ChannelConfig::default()); + let mut config = Some(LegacyChannelConfig::default()); if ver == 1 { // Read the old serialization of the ChannelConfig from version 0.0.98. - config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?; - config.as_mut().unwrap().cltv_expiry_delta = Readable::read(reader)?; + config.as_mut().unwrap().mutable.forwarding_fee_proportional_millionths = Readable::read(reader)?; + config.as_mut().unwrap().mutable.cltv_expiry_delta = Readable::read(reader)?; config.as_mut().unwrap().announced_channel = Readable::read(reader)?; config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?; } else { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 300efb12b..9456d43b4 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1679,7 +1679,9 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, ($node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => { { $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0); - let fee = $node.node.channel_state.lock().unwrap().by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap().config.forwarding_fee_base_msat; + let fee = $node.node.channel_state.lock().unwrap() + .by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap() + .config.mutable.forwarding_fee_base_msat; expect_payment_forwarded!($node, $next_node, $prev_node, Some(fee as u64), false, false); expected_total_fee_msat += fee as u64; check_added_monitors!($node, 1); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 07e531c5b..6fcb18cf0 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -531,7 +531,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { // Update the fee on the middle hop to ensure PaymentSent events have the correct (retried) fee // and not the original fee. We also update node[1]'s relevant config as // do_claim_payment_along_route expects us to never overpay. - nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&chan_id_2).unwrap().config.forwarding_fee_base_msat += 100_000; + nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&chan_id_2).unwrap() + .config.mutable.forwarding_fee_base_msat += 100_000; new_route.paths[0][0].fee_msat += 100_000; assert!(nodes[0].node.retry_payment(&new_route, payment_id_1).is_err()); // Shouldn't be allowed to retry a fulfilled payment diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index c2c73d8cb..570e570a8 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -291,30 +291,6 @@ pub struct ChannelConfig { /// /// [`MIN_CLTV_EXPIRY_DELTA`]: crate::ln::channelmanager::MIN_CLTV_EXPIRY_DELTA pub cltv_expiry_delta: u16, - /// Set to announce the channel publicly and notify all nodes that they can route via this - /// channel. - /// - /// This should only be set to true for nodes which expect to be online reliably. - /// - /// As the node which funds a channel picks this value this will only apply for new outbound - /// channels unless [`ChannelHandshakeLimits::force_announced_channel_preference`] 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 - /// at intialization, preventing us from closing to an alternate pubkey. - /// - /// This is set to true by default to provide a slight increase in security, though ultimately - /// any attacker who is able to take control of a channel can just as easily send the funds via - /// 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, /// Limit our total exposure to in-flight HTLCs which are burned to fees as they are too /// small to claim on-chain. /// @@ -363,23 +339,83 @@ impl Default for ChannelConfig { forwarding_fee_proportional_millionths: 0, forwarding_fee_base_msat: 1000, cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours - announced_channel: false, - commit_upfront_shutdown_pubkey: true, max_dust_htlc_exposure_msat: 5_000_000, force_close_avoidance_max_fee_satoshis: 1000, } } } -impl_writeable_tlv_based!(ChannelConfig, { - (0, forwarding_fee_proportional_millionths, required), - (1, max_dust_htlc_exposure_msat, (default_value, 5_000_000)), - (2, cltv_expiry_delta, required), - (3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)), - (4, announced_channel, required), - (6, commit_upfront_shutdown_pubkey, required), - (8, forwarding_fee_base_msat, required), -}); +/// Legacy version of [`ChannelConfig`] that stored the static +/// [`ChannelHandshakeConfig::announced_channel`] and +/// [`ChannelHandshakeConfig::commit_upfront_shutdown_pubkey`] fields. +#[derive(Copy, Clone, Debug)] +pub(crate) struct LegacyChannelConfig { + pub(crate) mutable: ChannelConfig, + /// Deprecated but may still be read from. See [`ChannelHandshakeConfig::announced_channel`] to + /// set this when opening/accepting a channel. + pub(crate) announced_channel: bool, + /// Deprecated but may still be read from. See + /// [`ChannelHandshakeConfig::commit_upfront_shutdown_pubkey`] to set this when + /// opening/accepting a channel. + pub(crate) commit_upfront_shutdown_pubkey: bool, +} + +impl Default for LegacyChannelConfig { + fn default() -> Self { + Self { + mutable: ChannelConfig::default(), + announced_channel: false, + commit_upfront_shutdown_pubkey: true, + } + } +} + +impl ::util::ser::Writeable for LegacyChannelConfig { + fn write(&self, writer: &mut W) -> Result<(), ::io::Error> { + write_tlv_fields!(writer, { + (0, self.mutable.forwarding_fee_proportional_millionths, required), + (1, self.mutable.max_dust_htlc_exposure_msat, (default_value, 5_000_000)), + (2, self.mutable.cltv_expiry_delta, required), + (3, self.mutable.force_close_avoidance_max_fee_satoshis, (default_value, 1000)), + (4, self.announced_channel, required), + (6, self.commit_upfront_shutdown_pubkey, required), + (8, self.mutable.forwarding_fee_base_msat, required), + }); + Ok(()) + } +} + +impl ::util::ser::Readable for LegacyChannelConfig { + fn read(reader: &mut R) -> Result { + let mut forwarding_fee_proportional_millionths = 0; + let mut max_dust_htlc_exposure_msat = 5_000_000; + let mut cltv_expiry_delta = 0; + let mut force_close_avoidance_max_fee_satoshis = 1000; + let mut announced_channel = false; + let mut commit_upfront_shutdown_pubkey = false; + let mut forwarding_fee_base_msat = 0; + read_tlv_fields!(reader, { + (0, forwarding_fee_proportional_millionths, required), + (1, max_dust_htlc_exposure_msat, (default_value, 5_000_000)), + (2, cltv_expiry_delta, required), + (3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)), + (4, announced_channel, required), + (6, commit_upfront_shutdown_pubkey, required), + (8, forwarding_fee_base_msat, required), + }); + Ok(Self { + mutable: ChannelConfig { + forwarding_fee_proportional_millionths, + max_dust_htlc_exposure_msat, + cltv_expiry_delta, + force_close_avoidance_max_fee_satoshis, + forwarding_fee_base_msat, + }, + announced_channel, + commit_upfront_shutdown_pubkey, + }) + } +} /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig. ///