diff --git a/lightning-types/src/features.rs b/lightning-types/src/features.rs index 62aca645486..37e011a7f4e 100644 --- a/lightning-types/src/features.rs +++ b/lightning-types/src/features.rs @@ -1024,6 +1024,15 @@ impl ChannelTypeFeatures { ::set_required_bit(&mut ret); ret } + + /// Constructs a ChannelTypeFeatures with zero fee commitment anchors support. + pub fn anchors_zero_fee_commitments() -> Self { + let mut ret = Self::empty(); + ::set_required_bit( + &mut ret, + ); + ret + } } impl Features { diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index be43a02b381..53c090388af 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -58,8 +58,13 @@ use crate::prelude::*; /// 483 for non-zero-fee-commitment channels and 114 for zero-fee-commitment channels. /// /// Actual maximums can be set equal to or below this value by each channel participant. -pub fn max_htlcs(_channel_type: &ChannelTypeFeatures) -> u16 { - 483 +pub fn max_htlcs(channel_type: &ChannelTypeFeatures) -> u16 { + if channel_type.supports_anchor_zero_fee_commitments() { + // TRUC restricts the size of our commitment transactions to 10K vB rather than 100K vB + 114 + } else { + 483 + } } /// The weight of a BIP141 witnessScript for a BOLT3's "offered HTLC output" on a commitment transaction, non-anchor variant. pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133; diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7f33937f2dd..5dd616de291 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1557,6 +1557,7 @@ impl Channel where pub fn maybe_handle_error_without_close( &mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + user_config: &UserConfig, their_features: &InitFeatures, ) -> Result, ()> where F::Target: FeeEstimator, @@ -1567,13 +1568,17 @@ impl Channel where ChannelPhase::Funded(_) => Ok(None), ChannelPhase::UnfundedOutboundV1(chan) => { let logger = WithChannelContext::from(logger, &chan.context, None); - chan.maybe_handle_error_without_close(chain_hash, fee_estimator, &&logger) + chan.maybe_handle_error_without_close( + chain_hash, fee_estimator, &&logger, user_config, their_features, + ) .map(|msg| Some(OpenChannelMessage::V1(msg))) }, ChannelPhase::UnfundedInboundV1(_) => Ok(None), ChannelPhase::UnfundedV2(chan) => { if chan.funding.is_outbound() { - chan.maybe_handle_error_without_close(chain_hash, fee_estimator) + chan.maybe_handle_error_without_close( + chain_hash, fee_estimator, user_config, their_features, + ) .map(|msg| Some(OpenChannelMessage::V2(msg))) } else { Ok(None) @@ -3072,12 +3077,18 @@ impl ChannelContext where SP::Target: SignerProvider { debug_assert!(!channel_type.supports_any_optional_bits()); debug_assert!(!channel_type.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config))); - let (commitment_conf_target, anchor_outputs_value_msat) = if channel_type.supports_anchors_zero_fee_htlc_tx() { - (ConfirmationTarget::AnchorChannelFee, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000) - } else { - (ConfirmationTarget::NonAnchorChannelFee, 0) - }; - let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target); + let (commitment_feerate, anchor_outputs_value_msat) = + if channel_type.supports_anchor_zero_fee_commitments() { + (0, 0) + } else if channel_type.supports_anchors_zero_fee_htlc_tx() { + let feerate = fee_estimator + .bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee); + (feerate, ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000) + } else { + let feerate = fee_estimator + .bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); + (feerate, 0) + }; let value_to_self_msat = channel_value_satoshis * 1000 - push_msat; let commitment_tx_fee = commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) * 1000; @@ -4868,7 +4879,8 @@ impl ChannelContext where SP::Target: SignerProvider { /// of the channel type we tried, not of our ability to open any channel at all. We can see if a /// downgrade of channel features would be possible so that we can still open the channel. pub(crate) fn maybe_downgrade_channel_features( - &mut self, funding: &mut FundingScope, fee_estimator: &LowerBoundedFeeEstimator + &mut self, funding: &mut FundingScope, fee_estimator: &LowerBoundedFeeEstimator, + user_config: &UserConfig, their_features: &InitFeatures, ) -> Result<(), ()> where F::Target: FeeEstimator @@ -4889,21 +4901,48 @@ impl ChannelContext where SP::Target: SignerProvider { // features one by one until we've either arrived at our default or the counterparty has // accepted one. // - // Due to the order below, we may not negotiate `option_anchors_zero_fee_htlc_tx` if the - // counterparty doesn't support `option_scid_privacy`. Since `get_initial_channel_type` - // checks whether the counterparty supports every feature, this would only happen if the - // counterparty is advertising the feature, but rejecting channels proposing the feature for - // whatever reason. - let channel_type = &mut funding.channel_transaction_parameters.channel_type_features; - if channel_type.supports_anchors_zero_fee_htlc_tx() { - channel_type.clear_anchors_zero_fee_htlc_tx(); - self.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - assert!(!channel_type.supports_anchors_nonzero_fee_htlc_tx()); + // To downgrade the current channel type, we start with the remote party's full set of + // feature bits and un-set any features that are set for the current channel type or any + // channel types that come before it in our order of preference. This will allow us to + // negotiate the "next best" channel type per our ranking in `get_initial_channel_type`. + let channel_type = &funding.channel_transaction_parameters.channel_type_features; + let mut eligible_features = their_features.clone(); + + if channel_type.supports_anchor_zero_fee_commitments() { + eligible_features.clear_anchor_zero_fee_commitments(); + assert!(!eligible_features.supports_anchor_zero_fee_commitments()); + } else if channel_type.supports_anchors_zero_fee_htlc_tx() { + eligible_features.clear_anchor_zero_fee_commitments(); + eligible_features.clear_anchors_zero_fee_htlc_tx(); + assert!(!eligible_features.supports_anchor_zero_fee_commitments()); + assert!(!eligible_features.supports_anchors_nonzero_fee_htlc_tx()); } else if channel_type.supports_scid_privacy() { - channel_type.clear_scid_privacy(); + eligible_features.clear_scid_privacy(); + eligible_features.clear_anchors_zero_fee_htlc_tx(); + eligible_features.clear_anchor_zero_fee_commitments(); + assert!(!eligible_features.supports_scid_privacy()); + assert!(!eligible_features.supports_anchors_nonzero_fee_htlc_tx()); + assert!(!eligible_features.supports_anchor_zero_fee_commitments()); + } + + let next_channel_type = get_initial_channel_type(user_config, &eligible_features); + + // Note that we can't get `anchor_zero_fee_commitments` type here, which requires zero + // fees, because we downgrade from this channel type first. If there were a superior + // channel type that downgrades to `anchor_zero_fee_commitments`, we'd need to handle + // fee setting differently here. If we proceeded to open a `anchor_zero_fee_commitments` + // channel with non-zero fees, we could produce a non-standard commitment transaction that + // puts us at risk of losing funds. We would expect our peer to reject such a channel + // open, but we don't want to rely on their validation. + assert!(!next_channel_type.supports_anchor_zero_fee_commitments()); + let conf_target = if next_channel_type.supports_anchors_zero_fee_htlc_tx() { + ConfirmationTarget::AnchorChannelFee } else { - *channel_type = ChannelTypeFeatures::only_static_remote_key(); - } + ConfirmationTarget::NonAnchorChannelFee + }; + self.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(conf_target); + funding.channel_transaction_parameters.channel_type_features = next_channel_type; + Ok(()) } @@ -5262,6 +5301,15 @@ impl FundedChannel where feerate_per_kw: u32, cur_feerate_per_kw: Option, logger: &L ) -> Result<(), ChannelError> where F::Target: FeeEstimator, L::Target: Logger, { + if channel_type.supports_anchor_zero_fee_commitments() { + if feerate_per_kw != 0 { + let err = "Zero Fee Channels must never attempt to use a fee".to_owned(); + return Err(ChannelError::close(err)); + } else { + return Ok(()); + } + } + let lower_limit_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() { ConfirmationTarget::MinAllowedAnchorChannelRemoteFee } else { @@ -9893,13 +9941,16 @@ impl OutboundV1Channel where SP::Target: SignerProvider { /// not of our ability to open any channel at all. Thus, on error, we should first call this /// and see if we get a new `OpenChannel` message, otherwise the channel is failed. pub(crate) fn maybe_handle_error_without_close( - &mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator, logger: &L + &mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + user_config: &UserConfig, their_features: &InitFeatures, ) -> Result where F::Target: FeeEstimator, L::Target: Logger, { - self.context.maybe_downgrade_channel_features(&mut self.funding, fee_estimator)?; + self.context.maybe_downgrade_channel_features( + &mut self.funding, fee_estimator, user_config, their_features, + )?; self.get_open_channel(chain_hash, logger).ok_or(()) } @@ -10078,8 +10129,9 @@ pub(super) fn channel_type_from_open_channel( // We only support the channel types defined by the `ChannelManager` in // `provided_channel_type_features`. The channel type must always support - // `static_remote_key`. - if !channel_type.requires_static_remote_key() { + // `static_remote_key`, either implicitly with `option_zero_fee_commitments` + // or explicitly. + if !channel_type.requires_static_remote_key() && !channel_type.requires_anchor_zero_fee_commitments() { return Err(ChannelError::close("Channel Type was not understood - we require static remote key".to_owned())); } // Make sure we support all of the features behind the channel type. @@ -10405,12 +10457,15 @@ impl PendingV2Channel where SP::Target: SignerProvider { /// not of our ability to open any channel at all. Thus, on error, we should first call this /// and see if we get a new `OpenChannelV2` message, otherwise the channel is failed. pub(crate) fn maybe_handle_error_without_close( - &mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator + &mut self, chain_hash: ChainHash, fee_estimator: &LowerBoundedFeeEstimator, + user_config: &UserConfig, their_features: &InitFeatures, ) -> Result where F::Target: FeeEstimator { - self.context.maybe_downgrade_channel_features(&mut self.funding, fee_estimator)?; + self.context.maybe_downgrade_channel_features( + &mut self.funding, fee_estimator, user_config, their_features, + )?; Ok(self.get_open_channel_v2(chain_hash)) } @@ -10663,10 +10718,21 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) ret.set_scid_privacy_required(); } - // Optionally, if the user would like to negotiate the `anchors_zero_fee_htlc_tx` option, we - // set it now. If they don't understand it, we'll fall back to our default of - // `only_static_remotekey`. - if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx && + // Optionally, if the user would like to negotiate `option_zero_fee_commitments` we set it now. + // If they don't understand it (or we don't want it), we check the same conditions for + // `option_anchors_zero_fee_htlc_tx`. The counterparty can still refuse the channel and we'll + // try to fall back (all the way to `only_static_remotekey`). + #[cfg(not(test))] + let negotiate_zero_fee_commitments = false; + + #[cfg(test)] + let negotiate_zero_fee_commitments = config.channel_handshake_config.negotiate_anchor_zero_fee_commitments; + + if negotiate_zero_fee_commitments && their_features.supports_anchor_zero_fee_commitments() { + ret.set_anchor_zero_fee_commitments_required(); + // `option_static_remote_key` is assumed by `option_zero_fee_commitments`. + ret.clear_static_remote_key(); + } else if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx && their_features.supports_anchors_zero_fee_htlc_tx() { ret.set_anchors_zero_fee_htlc_tx_required(); } @@ -13267,6 +13333,45 @@ mod tests { fn test_supports_anchors_zero_htlc_tx_fee() { // Tests that if both sides support and negotiate `anchors_zero_fee_htlc_tx`, it is the // resulting `channel_type`. + let mut config = UserConfig::default(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + + let mut expected_channel_type = ChannelTypeFeatures::empty(); + expected_channel_type.set_static_remote_key_required(); + expected_channel_type.set_anchors_zero_fee_htlc_tx_required(); + + do_test_supports_channel_type(config, expected_channel_type) + } + + #[test] + fn test_supports_zero_fee_commitments() { + // Tests that if both sides support and negotiate `anchors_zero_fee_commitments`, it is + // the resulting `channel_type`. + let mut config = UserConfig::default(); + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + + let mut expected_channel_type = ChannelTypeFeatures::empty(); + expected_channel_type.set_anchor_zero_fee_commitments_required(); + + do_test_supports_channel_type(config, expected_channel_type) + } + + #[test] + fn test_supports_zero_fee_commitments_and_htlc_tx_fee() { + // Tests that if both sides support and negotiate `anchors_zero_fee_commitments` and + // `anchors_zero_fee_htlc_tx`, the resulting `channel_type` is + // `anchors_zero_fee_commitments`. + let mut config = UserConfig::default(); + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + + let mut expected_channel_type = ChannelTypeFeatures::empty(); + expected_channel_type.set_anchor_zero_fee_commitments_required(); + + do_test_supports_channel_type(config, expected_channel_type) + } + + fn do_test_supports_channel_type(config: UserConfig, expected_channel_type: ChannelTypeFeatures) { let secp_ctx = Secp256k1::new(); let fee_estimator = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let network = Network::Testnet; @@ -13276,21 +13381,13 @@ mod tests { let node_id_a = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[1; 32]).unwrap()); let node_id_b = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[2; 32]).unwrap()); - let mut config = UserConfig::default(); - config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; - - // It is not enough for just the initiator to signal `option_anchors_zero_fee_htlc_tx`, both - // need to signal it. + // Assert that we get `static_remotekey` when no custom config is negotiated. let channel_a = OutboundV1Channel::<&TestKeysInterface>::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_b, &channelmanager::provided_init_features(&UserConfig::default()), 10000000, 100000, 42, &config, 0, 42, None, &logger ).unwrap(); - assert!(!channel_a.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx()); - - let mut expected_channel_type = ChannelTypeFeatures::empty(); - expected_channel_type.set_static_remote_key_required(); - expected_channel_type.set_anchors_zero_fee_htlc_tx_required(); + assert_eq!(channel_a.funding.get_channel_type(), &ChannelTypeFeatures::only_static_remote_key()); let mut channel_a = OutboundV1Channel::<&TestKeysInterface>::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_b, @@ -13307,6 +13404,14 @@ mod tests { assert_eq!(channel_a.funding.get_channel_type(), &expected_channel_type); assert_eq!(channel_b.funding.get_channel_type(), &expected_channel_type); + + if expected_channel_type.supports_anchor_zero_fee_commitments() { + assert_eq!(channel_a.context.feerate_per_kw, 0); + assert_eq!(channel_b.context.feerate_per_kw, 0); + } else { + assert_ne!(channel_a.context.feerate_per_kw, 0); + assert_ne!(channel_b.context.feerate_per_kw, 0); + } } #[test] diff --git a/lightning/src/ln/channel_acceptance_tests.rs b/lightning/src/ln/channel_acceptance_tests.rs new file mode 100644 index 00000000000..e7da6f1a776 --- /dev/null +++ b/lightning/src/ln/channel_acceptance_tests.rs @@ -0,0 +1,301 @@ +use crate::events::Event; +use crate::ln::functional_test_utils::*; +use crate::ln::msgs::{ + AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, MessageSendEvent, +}; +use crate::util::config::{ChannelConfigOverrides, ChannelHandshakeConfigUpdate, UserConfig}; +use lightning_types::features::ChannelTypeFeatures; + +#[test] +fn test_inbound_anchors_manual_acceptance() { + let mut anchors_cfg = test_default_channel_config(); + anchors_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + do_test_manual_inbound_accept_with_override(anchors_cfg, None); +} + +#[test] +fn test_inbound_anchors_manual_acceptance_overridden() { + let overrides = ChannelConfigOverrides { + handshake_overrides: Some(ChannelHandshakeConfigUpdate { + max_inbound_htlc_value_in_flight_percent_of_channel: Some(5), + htlc_minimum_msat: Some(1000), + minimum_depth: Some(2), + to_self_delay: Some(200), + max_accepted_htlcs: Some(5), + channel_reserve_proportional_millionths: Some(20000), + }), + update_overrides: None, + }; + + let mut anchors_cfg = test_default_channel_config(); + anchors_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + + let accept_message = do_test_manual_inbound_accept_with_override(anchors_cfg, Some(overrides)); + assert_eq!(accept_message.common_fields.max_htlc_value_in_flight_msat, 5_000_000); + assert_eq!(accept_message.common_fields.htlc_minimum_msat, 1_000); + assert_eq!(accept_message.common_fields.minimum_depth, 2); + assert_eq!(accept_message.common_fields.to_self_delay, 200); + assert_eq!(accept_message.common_fields.max_accepted_htlcs, 5); + assert_eq!(accept_message.channel_reserve_satoshis, 2_000); +} + +#[test] +fn test_inbound_zero_fee_commitments_manual_acceptance() { + let mut zero_fee_cfg = test_default_channel_config(); + zero_fee_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + do_test_manual_inbound_accept_with_override(zero_fee_cfg, None); +} + +fn do_test_manual_inbound_accept_with_override( + start_cfg: UserConfig, config_overrides: Option, +) -> AcceptChannel { + let mut mannual_accept_cfg = start_cfg.clone(); + mannual_accept_cfg.manually_accept_inbound_channels = true; + + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs( + 3, + &node_cfgs, + &[Some(start_cfg.clone()), Some(start_cfg.clone()), Some(mannual_accept_cfg.clone())], + ); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + nodes[0] + .node + .create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None) + .unwrap(); + let open_channel_msg = get_event_msg!( + nodes[0], + MessageSendEvent::SendOpenChannel, + nodes[1].node.get_our_node_id() + ); + + nodes[1].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + match &msg_events[0] { + MessageSendEvent::HandleError { node_id, action } => { + assert_eq!(*node_id, nodes[0].node.get_our_node_id()); + match action { + ErrorAction::SendErrorMessage { msg } => { + assert_eq!(msg.data, "No channels with anchor outputs accepted".to_owned()) + }, + _ => panic!("Unexpected error action"), + } + }, + _ => panic!("Unexpected event"), + } + + nodes[2].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); + let events = nodes[2].node.get_and_clear_pending_events(); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => nodes[2] + .node + .accept_inbound_channel( + &temporary_channel_id, + &nodes[0].node.get_our_node_id(), + 23, + config_overrides, + ) + .unwrap(), + _ => panic!("Unexpected event"), + } + get_event_msg!(nodes[2], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()) +} + +#[test] +fn test_anchors_zero_fee_htlc_tx_downgrade() { + // Tests that if both nodes support anchors, but the remote node does not want to accept + // anchor channels at the moment, an error it sent to the local node such that it can retry + // the channel without the anchors feature. + let mut initiator_cfg = test_default_channel_config(); + initiator_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + + let mut receiver_cfg = test_default_channel_config(); + receiver_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + receiver_cfg.manually_accept_inbound_channels = true; + + let start_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + let end_type = ChannelTypeFeatures::only_static_remote_key(); + do_test_channel_type_downgrade(initiator_cfg, receiver_cfg, start_type, vec![end_type]); +} + +#[test] +fn test_scid_privacy_downgrade() { + // Tests downgrade from `anchors_zero_fee_commitments` with `option_scid_alias` when the + // remote node advertises the features but does not accept the channel, asserting that + // `option_scid_alias` is the last feature to be downgraded. + let mut initiator_cfg = test_default_channel_config(); + initiator_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + initiator_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + initiator_cfg.channel_handshake_config.negotiate_scid_privacy = true; + initiator_cfg.channel_handshake_config.announce_for_forwarding = false; + + let mut receiver_cfg = test_default_channel_config(); + receiver_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + receiver_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + receiver_cfg.channel_handshake_config.negotiate_scid_privacy = true; + receiver_cfg.manually_accept_inbound_channels = true; + + let mut start_type = ChannelTypeFeatures::anchors_zero_fee_commitments(); + start_type.set_scid_privacy_required(); + let mut with_anchors = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + with_anchors.set_scid_privacy_required(); + let mut with_scid_privacy = ChannelTypeFeatures::only_static_remote_key(); + with_scid_privacy.set_scid_privacy_required(); + let static_remote = ChannelTypeFeatures::only_static_remote_key(); + let downgrade_types = vec![with_anchors, with_scid_privacy, static_remote]; + + do_test_channel_type_downgrade(initiator_cfg, receiver_cfg, start_type, downgrade_types); +} + +#[test] +fn test_zero_fee_commitments_downgrade() { + // Tests that the local node will retry without zero fee commitments in the case where the + // remote node supports the feature but does not accept it. + let mut initiator_cfg = test_default_channel_config(); + initiator_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + initiator_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + + let mut receiver_cfg = test_default_channel_config(); + receiver_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + receiver_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + receiver_cfg.manually_accept_inbound_channels = true; + + let start_type = ChannelTypeFeatures::anchors_zero_fee_commitments(); + let downgrade_types = vec![ + ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), + ChannelTypeFeatures::only_static_remote_key(), + ]; + do_test_channel_type_downgrade(initiator_cfg, receiver_cfg, start_type, downgrade_types); +} + +#[test] +fn test_zero_fee_commitments_downgrade_to_static_remote() { + // Tests that the local node will retry with static remote key when zero fee commitments + // are supported (but not accepted), but not legacy anchors. + let mut initiator_cfg = test_default_channel_config(); + initiator_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + initiator_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + + let mut receiver_cfg = test_default_channel_config(); + receiver_cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + receiver_cfg.manually_accept_inbound_channels = true; + + let start_type = ChannelTypeFeatures::anchors_zero_fee_commitments(); + let end_type = ChannelTypeFeatures::only_static_remote_key(); + do_test_channel_type_downgrade(initiator_cfg, receiver_cfg, start_type, vec![end_type]); +} + +fn do_test_channel_type_downgrade( + initiator_cfg: UserConfig, acceptor_cfg: UserConfig, start_type: ChannelTypeFeatures, + downgrade_types: Vec, +) { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(initiator_cfg), Some(acceptor_cfg)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let error_message = "Channel force-closed"; + + nodes[0] + .node + .create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 0, None, None) + .unwrap(); + let mut open_channel_msg = get_event_msg!( + nodes[0], + MessageSendEvent::SendOpenChannel, + nodes[1].node.get_our_node_id() + ); + assert_eq!(open_channel_msg.common_fields.channel_type.as_ref().unwrap(), &start_type); + + for downgrade_type in downgrade_types { + nodes[1].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); + let events = nodes[1].node.get_and_clear_pending_events(); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + nodes[1] + .node + .force_close_broadcasting_latest_txn( + &temporary_channel_id, + &nodes[0].node.get_our_node_id(), + error_message.to_string(), + ) + .unwrap(); + }, + _ => panic!("Unexpected event"), + } + + let error_msg = get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()); + nodes[0].node.handle_error(nodes[1].node.get_our_node_id(), &error_msg); + + open_channel_msg = get_event_msg!( + nodes[0], + MessageSendEvent::SendOpenChannel, + nodes[1].node.get_our_node_id() + ); + let channel_type = open_channel_msg.common_fields.channel_type.as_ref().unwrap(); + assert_eq!(channel_type, &downgrade_type); + + // Since nodes[1] should not have accepted the channel, it should + // not have generated any events. + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + } +} + +#[test] +fn test_no_channel_downgrade() { + // Tests that the local node will not retry when a `option_static_remote` channel is + // rejected by a peer that advertises support for the feature. + let initiator_cfg = test_default_channel_config(); + let mut receiver_cfg = test_default_channel_config(); + receiver_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + receiver_cfg.manually_accept_inbound_channels = true; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(initiator_cfg), Some(receiver_cfg)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let error_message = "Channel force-closed"; + + nodes[0] + .node + .create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 0, None, None) + .unwrap(); + let open_channel_msg = get_event_msg!( + nodes[0], + MessageSendEvent::SendOpenChannel, + nodes[1].node.get_our_node_id() + ); + let start_type = ChannelTypeFeatures::only_static_remote_key(); + assert_eq!(open_channel_msg.common_fields.channel_type.as_ref().unwrap(), &start_type); + + nodes[1].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); + let events = nodes[1].node.get_and_clear_pending_events(); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + nodes[1] + .node + .force_close_broadcasting_latest_txn( + &temporary_channel_id, + &nodes[0].node.get_our_node_id(), + error_message.to_string(), + ) + .unwrap(); + }, + _ => panic!("Unexpected event"), + } + + let error_msg = get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()); + nodes[0].node.handle_error(nodes[1].node.get_our_node_id(), &error_msg); + + // Since nodes[0] could not retry the channel with a different type, it should close it. + let chan_closed_events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(chan_closed_events.len(), 1); + if let Event::ChannelClosed { .. } = chan_closed_events[0] { + } else { + panic!(); + } +} diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b018c6c74cd..5cd7ff81c0f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8372,7 +8372,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if channel_type.requires_zero_conf() { return Err(MsgHandleErrInternal::send_err_msg_no_close("No zero confirmation channels accepted".to_owned(), common_fields.temporary_channel_id)); } - if channel_type.requires_anchors_zero_fee_htlc_tx() { + if channel_type.requires_anchors_zero_fee_htlc_tx() || channel_type.requires_anchor_zero_fee_commitments() { return Err(MsgHandleErrInternal::send_err_msg_no_close("No channels with anchor outputs accepted".to_owned(), common_fields.temporary_channel_id)); } @@ -12472,6 +12472,7 @@ where match peer_state.channel_by_id.get_mut(&msg.channel_id) { Some(chan) => match chan.maybe_handle_error_without_close( self.chain_hash, &self.fee_estimator, &self.logger, + &self.default_configuration, &peer_state.latest_features, ) { Ok(Some(OpenChannelMessage::V1(msg))) => { peer_state.pending_msg_events.push(MessageSendEvent::SendOpenChannel { @@ -13004,6 +13005,12 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures { // quiescent-dependent protocols (e.g., splicing). #[cfg(any(test, fuzzing))] features.set_quiescence_optional(); + + #[cfg(test)] + if config.channel_handshake_config.negotiate_anchor_zero_fee_commitments { + features.set_anchor_zero_fee_commitments_optional(); + } + features } @@ -15210,13 +15217,16 @@ mod tests { use crate::routing::router::{find_route, PaymentParameters, RouteParameters}; use crate::sign::EntropySource; use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; - use crate::util::config::{ChannelConfig, ChannelConfigUpdate, ChannelHandshakeConfigUpdate}; + use crate::util::config::{ + ChannelConfig, ChannelConfigUpdate, ChannelHandshakeConfigUpdate, UserConfig, + }; use crate::util::errors::APIError; use crate::util::ser::Writeable; use crate::util::test_utils; use bitcoin::secp256k1::ecdh::SharedSecret; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use core::sync::atomic::Ordering; + use lightning_types::features::ChannelTypeFeatures; #[test] #[rustfmt::skip] @@ -16276,117 +16286,6 @@ mod tests { assert!(result.is_ok()); } - #[test] - fn test_inbound_anchors_manual_acceptance() { - test_inbound_anchors_manual_acceptance_with_override(None); - } - - #[test] - fn test_inbound_anchors_manual_acceptance_overridden() { - let overrides = ChannelConfigOverrides { - handshake_overrides: Some(ChannelHandshakeConfigUpdate { - max_inbound_htlc_value_in_flight_percent_of_channel: Some(5), - htlc_minimum_msat: Some(1000), - minimum_depth: Some(2), - to_self_delay: Some(200), - max_accepted_htlcs: Some(5), - channel_reserve_proportional_millionths: Some(20000), - }), - update_overrides: None, - }; - - let accept_message = test_inbound_anchors_manual_acceptance_with_override(Some(overrides)); - assert_eq!(accept_message.common_fields.max_htlc_value_in_flight_msat, 5_000_000); - assert_eq!(accept_message.common_fields.htlc_minimum_msat, 1_000); - assert_eq!(accept_message.common_fields.minimum_depth, 2); - assert_eq!(accept_message.common_fields.to_self_delay, 200); - assert_eq!(accept_message.common_fields.max_accepted_htlcs, 5); - assert_eq!(accept_message.channel_reserve_satoshis, 2_000); - } - - #[rustfmt::skip] - fn test_inbound_anchors_manual_acceptance_with_override(config_overrides: Option) -> AcceptChannel { - // Tests that we properly limit inbound channels when we have the manual-channel-acceptance - // flag set and (sometimes) accept channels as 0conf. - let mut anchors_cfg = test_default_channel_config(); - anchors_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; - - let mut anchors_manual_accept_cfg = anchors_cfg.clone(); - anchors_manual_accept_cfg.manually_accept_inbound_channels = true; - - let chanmon_cfgs = create_chanmon_cfgs(3); - let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, - &[Some(anchors_cfg.clone()), Some(anchors_cfg.clone()), Some(anchors_manual_accept_cfg.clone())]); - let nodes = create_network(3, &node_cfgs, &node_chanmgrs); - - nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None).unwrap(); - let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - - nodes[1].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); - assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - match &msg_events[0] { - MessageSendEvent::HandleError { node_id, action } => { - assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - match action { - ErrorAction::SendErrorMessage { msg } => - assert_eq!(msg.data, "No channels with anchor outputs accepted".to_owned()), - _ => panic!("Unexpected error action"), - } - } - _ => panic!("Unexpected event"), - } - - nodes[2].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); - let events = nodes[2].node.get_and_clear_pending_events(); - match events[0] { - Event::OpenChannelRequest { temporary_channel_id, .. } => - nodes[2].node.accept_inbound_channel(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 23, config_overrides).unwrap(), - _ => panic!("Unexpected event"), - } - get_event_msg!(nodes[2], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()) - } - - #[test] - #[rustfmt::skip] - fn test_anchors_zero_fee_htlc_tx_fallback() { - // Tests that if both nodes support anchors, but the remote node does not want to accept - // anchor channels at the moment, an error it sent to the local node such that it can retry - // the channel without the anchors feature. - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let mut anchors_config = test_default_channel_config(); - anchors_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; - anchors_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config.clone()), Some(anchors_config.clone())]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let error_message = "Channel force-closed"; - - nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 0, None, None).unwrap(); - let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - assert!(open_channel_msg.common_fields.channel_type.as_ref().unwrap().supports_anchors_zero_fee_htlc_tx()); - - nodes[1].node.handle_open_channel(nodes[0].node.get_our_node_id(), &open_channel_msg); - let events = nodes[1].node.get_and_clear_pending_events(); - match events[0] { - Event::OpenChannelRequest { temporary_channel_id, .. } => { - nodes[1].node.force_close_broadcasting_latest_txn(&temporary_channel_id, &nodes[0].node.get_our_node_id(), error_message.to_string()).unwrap(); - } - _ => panic!("Unexpected event"), - } - - let error_msg = get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()); - nodes[0].node.handle_error(nodes[1].node.get_our_node_id(), &error_msg); - - let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - assert!(!open_channel_msg.common_fields.channel_type.unwrap().supports_anchors_zero_fee_htlc_tx()); - - // Since nodes[1] should not have accepted the channel, it should - // not have generated any events. - assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); - } - #[test] #[rustfmt::skip] fn test_update_channel_config() { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 0c0bb0713cb..038bd1da431 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -256,6 +256,66 @@ pub fn test_insane_channel_opens() { }); } +#[test] +fn test_insane_zero_fee_channel_open() { + let mut cfg = UserConfig::default(); + cfg.manually_accept_inbound_channels = true; + cfg.channel_handshake_config.negotiate_anchor_zero_fee_commitments = true; + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = + create_node_chanmgrs(2, &node_cfgs, &[Some(cfg.clone()), Some(cfg.clone())]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_a_id = nodes[0].node.get_our_node_id(); + let node_b_id = nodes[1].node.get_our_node_id(); + + nodes[0].node.create_channel(node_b_id, 100_000, 0, 42, None, None).unwrap(); + + let open_channel_message = + get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id); + + let insane_open_helper = + |expected_error_str: &str, message_mutator: fn(msgs::OpenChannel) -> msgs::OpenChannel| { + let open_channel_mutated = message_mutator(open_channel_message.clone()); + nodes[1].node.handle_open_channel(node_a_id, &open_channel_mutated); + + let events = nodes[1].node.get_and_clear_pending_events(); + match events[0] { + Event::OpenChannelRequest { temporary_channel_id, .. } => { + match nodes[1].node.accept_inbound_channel( + &temporary_channel_id, + &nodes[0].node.get_our_node_id(), + 23, + None, + ) { + Ok(_) => panic!("Unexpected successful channel accept"), + Err(e) => assert!(format!("{:?}", e).contains(expected_error_str)), + } + }, + _ => panic!("Unexpected event"), + } + + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + assert!(matches!(events[0], MessageSendEvent::HandleError { .. })); + }; + + insane_open_helper( + "max_accepted_htlcs was 115. It must not be larger than 114".into(), + |mut msg| { + msg.common_fields.max_accepted_htlcs = 115; + msg + }, + ); + + insane_open_helper("Zero Fee Channels must never attempt to use a fee".into(), |mut msg| { + msg.common_fields.commitment_feerate_sat_per_1000_weight = 123; + msg + }); +} + #[xtest(feature = "_externalize_tests")] pub fn test_funding_exceeds_no_wumbo_limit() { // Test that if a peer does not support wumbo channels, we'll refuse to open a wumbo channel to diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs index 8d741f2954d..46cf31ba31f 100644 --- a/lightning/src/ln/mod.rs +++ b/lightning/src/ln/mod.rs @@ -74,6 +74,9 @@ pub mod bolt11_payment_tests; mod chanmon_update_fail_tests; #[cfg(test)] #[allow(unused_mut)] +mod channel_acceptance_tests; +#[cfg(test)] +#[allow(unused_mut)] mod dual_funding_tests; #[cfg(any(test, feature = "_externalize_tests"))] #[allow(unused_mut)] diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 672783a127f..e98b237691c 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -183,6 +183,41 @@ pub struct ChannelHandshakeConfig { /// [`DecodeError::InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue pub negotiate_anchors_zero_fee_htlc_tx: bool, + /// If set, we attempt to negotiate the `zero_fee_commitments` option for all future channels. + /// + /// These channels operate very similarly to the `anchors_zero_fee_htlc` channels but rely on + /// [TRUC] to assign zero fee to the commitment transactions themselves, avoiding many protocol + /// edge-cases involving fee updates and greatly simplifying the concept of your "balance" in + /// lightning. + /// + /// Like `anchors_zero_fee_htlc` channels, this feature requires having a reserve of onchain + /// funds readily available to bump transactions in the event of a channel force close to avoid + /// the possibility of losing funds. + /// + /// Note that if you wish accept inbound channels with anchor outputs, you must enable + /// [`UserConfig::manually_accept_inbound_channels`] and manually accept them with + /// [`ChannelManager::accept_inbound_channel`]. This is done to give you the chance to check + /// whether your reserve of onchain funds is enough to cover the fees for all existing and new + /// channels featuring anchor outputs in the event of a force close. + /// + /// If this option is set, channels may be created that will not be readable by LDK versions + /// prior to 0.2, causing [`ChannelManager`]'s read method to return a + /// [`DecodeError::InvalidValue`]. + /// + /// Note that setting this to true does *not* prevent us from opening channels with + /// counterparties that do not support the `zero_fee_commitments` option; we will simply fall + /// back to a `anchors_zero_fee_htlc` (if [`Self::negotiate_anchors_zero_fee_htlc_tx`] + /// is set) or `static_remote_key` channel. + /// + /// Default value: `false` (This value is likely to change to `true` in the future.) + /// + /// [TRUC]: (https://bitcoinops.org/en/topics/version-3-transaction-relay/) + /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`DecodeError::InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue + #[cfg(test)] + pub negotiate_anchor_zero_fee_commitments: bool, + /// The maximum number of HTLCs in-flight from our counterparty towards us at the same time. /// /// Increasing the value can help improve liquidity and stability in @@ -212,6 +247,8 @@ impl Default for ChannelHandshakeConfig { commit_upfront_shutdown_pubkey: true, their_channel_reserve_proportional_millionths: 10_000, negotiate_anchors_zero_fee_htlc_tx: false, + #[cfg(test)] + negotiate_anchor_zero_fee_commitments: false, our_max_accepted_htlcs: 50, } } @@ -233,6 +270,8 @@ impl Readable for ChannelHandshakeConfig { commit_upfront_shutdown_pubkey: Readable::read(reader)?, their_channel_reserve_proportional_millionths: Readable::read(reader)?, negotiate_anchors_zero_fee_htlc_tx: Readable::read(reader)?, + #[cfg(test)] + negotiate_anchor_zero_fee_commitments: Readable::read(reader)?, our_max_accepted_htlcs: Readable::read(reader)?, }) }