Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ fn assert_action_timeout_awaiting_response(action: &msgs::ErrorAction) {
);
}

#[derive(Copy, Clone)]
#[derive(Clone, Copy, PartialEq)]
enum ChanType {
Legacy,
KeyedAnchors,
Expand Down Expand Up @@ -2082,19 +2082,20 @@ impl<'a, Out: Output + MaybeSend + MaybeSync> Harness<'a, Out> {
connect_peers(&nodes[0], &nodes[1]);
connect_peers(&nodes[1], &nodes[2]);

let set_0reserve = if chan_type == ChanType::Legacy { false } else { true };
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This can be simplified to chan_type != ChanType::Legacy (which also explains why PartialEq was derived above).

// Create 3 channels between A-B and 3 channels between B-C (6 total).
//
// Use distinct version numbers for each funding transaction so each test
// channel gets its own txid and funding outpoint.
// A-B: channel 2 A and B have 0-reserve (trusted open + trusted accept),
// channel 3 A has 0-reserve (trusted accept).
Comment on lines 2090 to 2091
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments on lines 2090-2091 and 2095-2096 still describe channel 2/3/4/5 as having 0-reserve unconditionally, but that's no longer the case for ChanType::Legacy. Consider qualifying these with "if non-legacy" or restructuring the comments to be accurate for both code paths.

make_channel(&nodes[0], &nodes[1], 1, false, false, &mut chain_state);
make_channel(&nodes[0], &nodes[1], 2, true, true, &mut chain_state);
make_channel(&nodes[0], &nodes[1], 3, false, true, &mut chain_state);
make_channel(&nodes[0], &nodes[1], 2, set_0reserve, set_0reserve, &mut chain_state);
make_channel(&nodes[0], &nodes[1], 3, false, set_0reserve, &mut chain_state);
// B-C: channel 4 B has 0-reserve (via trusted accept),
// channel 5 C has 0-reserve (via trusted open).
make_channel(&nodes[1], &nodes[2], 4, false, true, &mut chain_state);
make_channel(&nodes[1], &nodes[2], 5, true, false, &mut chain_state);
make_channel(&nodes[1], &nodes[2], 4, false, set_0reserve, &mut chain_state);
make_channel(&nodes[1], &nodes[2], 5, set_0reserve, false, &mut chain_state);
make_channel(&nodes[1], &nodes[2], 6, false, false, &mut chain_state);

// Wipe the transactions-broadcasted set to make sure we don't broadcast
Expand Down
46 changes: 37 additions & 9 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3832,6 +3832,14 @@ impl<SP: SignerProvider> ChannelContext<SP> {
"Funding must be smaller than the total bitcoin supply. It was {channel_value_satoshis}"
)));
}
if !channel_type.supports_anchors_zero_fee_htlc_tx()
&& !channel_type.supports_anchor_zero_fee_commitments()
&& holder_selected_channel_reserve_satoshis == 0
{
return Err(ChannelError::close(
"0-reserve is not allowed on legacy channels".to_owned(),
));
}
if msg_channel_reserve_satoshis > channel_value_satoshis {
return Err(ChannelError::close(format!(
"Bogus channel_reserve_satoshis ({msg_channel_reserve_satoshis}). Must be no greater than channel_value_satoshis: {channel_value_satoshis}"
Expand Down Expand Up @@ -4323,6 +4331,14 @@ impl<SP: SignerProvider> ChannelContext<SP> {
}

let channel_type = get_initial_channel_type(&config, their_features);
if !channel_type.supports_anchors_zero_fee_htlc_tx()
&& !channel_type.supports_anchor_zero_fee_commitments()
&& holder_selected_channel_reserve_satoshis == 0
{
return Err(APIError::APIMisuseError {
err: "0-reserve is not allowed on legacy channels".to_owned(),
});
}
debug_assert!(!channel_type.supports_any_optional_bits());
debug_assert!(!channel_type
.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config)));
Expand Down Expand Up @@ -4869,6 +4885,14 @@ impl<SP: SignerProvider> ChannelContext<SP> {
}

let channel_type = funding.get_channel_type();
if !channel_type.supports_anchors_zero_fee_htlc_tx()
&& !channel_type.supports_anchor_zero_fee_commitments()
&& funding.holder_selected_channel_reserve_satoshis == 0
{
return Err(ChannelError::close(
"0-reserve is not allowed on legacy channels".to_owned(),
));
}
if common_fields.max_accepted_htlcs > max_htlcs(channel_type) {
return Err(ChannelError::close(format!(
"max_accepted_htlcs was {}. It must not be larger than {}",
Expand Down Expand Up @@ -6537,17 +6561,15 @@ impl<SP: SignerProvider> ChannelContext<SP> {
/// If we receive an error message when attempting to open a channel, it may only be a rejection
/// 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.
#[rustfmt::skip]
pub(crate) fn maybe_downgrade_channel_features<F: FeeEstimator>(
&mut self, funding: &mut FundingScope, fee_estimator: &LowerBoundedFeeEstimator<F>,
user_config: &UserConfig, their_features: &InitFeatures,
) -> Result<(), ()> {
if !funding.is_outbound() ||
!matches!(
if !funding.is_outbound()
|| !matches!(
self.channel_state, ChannelState::NegotiatingFunding(flags)
if flags == NegotiatingFundingFlags::OUR_INIT_SENT
)
{
) {
return Err(());
}
if funding.get_channel_type() == &ChannelTypeFeatures::only_static_remote_key() {
Expand Down Expand Up @@ -6578,11 +6600,17 @@ impl<SP: SignerProvider> ChannelContext<SP> {
}

let next_channel_type = get_initial_channel_type(user_config, &eligible_features);
if !next_channel_type.supports_anchors_zero_fee_htlc_tx()
&& !next_channel_type.supports_anchor_zero_fee_commitments()
&& funding.holder_selected_channel_reserve_satoshis == 0
{
// 0-reserve is not allowed on legacy channels
return Err(());
}

self.feerate_per_kw = selected_commitment_sat_per_1000_weight(
&fee_estimator, &next_channel_type,
);
funding.channel_transaction_parameters.channel_type_features = next_channel_type;
self.feerate_per_kw =
selected_commitment_sat_per_1000_weight(&fee_estimator, &next_channel_type);
funding.channel_transaction_parameters.channel_type_features = next_channel_type;

Ok(())
}
Expand Down
190 changes: 189 additions & 1 deletion lightning/src/ln/channel_open_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ use crate::ln::channelmanager::{
MAX_UNFUNDED_CHANS_PER_PEER,
};
use crate::ln::msgs::{
AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, MessageSendEvent,
AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, ErrorMessage,
MessageSendEvent,
};
use crate::ln::types::ChannelId;
use crate::ln::{functional_test_utils::*, msgs};
Expand All @@ -48,6 +49,7 @@ use bitcoin::{Amount, Sequence, Transaction, TxIn, TxOut, Witness};
use lightning_macros::xtest;

use lightning_types::features::ChannelTypeFeatures;
use types::string::UntrustedString;

#[test]
fn test_outbound_chans_unlimited() {
Expand Down Expand Up @@ -2496,3 +2498,189 @@ fn test_fund_pending_channel() {
};
check_closed_event(&nodes[0], 1, reason, &[node_b_id], 100_000);
}

#[xtest(feature = "_externalize_tests")]
fn test_holder_selected_0reserve_on_legacy_channel_is_not_allowed() {
{
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let mut channel_config = test_default_channel_config();
assert!(channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx);
let node_chanmgrs = create_node_chanmgrs(
2,
&node_cfgs,
&[Some(channel_config.clone()), Some(channel_config.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();

let mut legacy_channel_config = test_default_channel_config();
legacy_channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
legacy_channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments =
false;

// User tries to open a legacy 0-reserve channel with a config override, we fail
assert_eq!(
nodes[0]
.node
.create_channel_to_trusted_peer_0reserve(
node_b_id,
100_000,
0,
42,
None,
Some(legacy_channel_config)
)
.unwrap_err(),
APIError::APIMisuseError {
err: "0-reserve is not allowed on legacy channels".to_owned()
}
);
}
{
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let mut channel_config = test_default_channel_config();
channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
let node_chanmgrs = create_node_chanmgrs(
2,
&node_cfgs,
&[Some(channel_config.clone()), Some(channel_config.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();

// User tries to open a legacy 0-reserve channel from the default config, we fail
assert_eq!(
nodes[0]
.node
.create_channel_to_trusted_peer_0reserve(node_b_id, 100_000, 0, 42, None, None)
.unwrap_err(),
APIError::APIMisuseError {
err: "0-reserve is not allowed on legacy channels".to_owned()
}
);
}

let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let mut channel_config = test_default_channel_config();
channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
let node_chanmgrs = create_node_chanmgrs(
2,
&node_cfgs,
&[Some(channel_config.clone()), Some(channel_config.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 mut open_channel_msg_0reserve =
get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id);
open_channel_msg_0reserve.channel_reserve_satoshis = 0;
assert_eq!(
open_channel_msg_0reserve.common_fields.channel_type,
Some(ChannelTypeFeatures::only_static_remote_key())
);

// User accepts a legacy channel, and sets 0-reserve for the counterparty, we fail
nodes[1].node.handle_open_channel(node_a_id, &open_channel_msg_0reserve);
let events = nodes[1].node.get_and_clear_pending_events();
match events[0] {
Event::OpenChannelRequest { temporary_channel_id, .. } => {
let error = nodes[1]
.node
.accept_inbound_channel_from_trusted_peer(
&temporary_channel_id,
&node_a_id,
42,
TrustedChannelFeatures::ZeroReserve,
None,
)
.unwrap_err();
assert_eq!(
error,
APIError::ChannelUnavailable {
err: "0-reserve is not allowed on legacy channels".to_owned()
}
);
},
_ => panic!("Unexpected event"),
}
let err_msg = get_err_msg(&nodes[1], &node_a_id);
assert_eq!(
err_msg,
ErrorMessage {
channel_id: open_channel_msg_0reserve.common_fields.temporary_channel_id,
data: "0-reserve is not allowed on legacy channels".to_string()
}
);

// But legacy channels where only the counterparty sets 0-reserve are ok!
// Here node 1 accepts 0-reserve from node 0, and node 1 sets some non-zero reserve...
handle_and_accept_open_channel(&nodes[1], node_a_id, &open_channel_msg_0reserve);
let mut accept_channel_msg =
get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, node_a_id);
// Override the reserve selected by node 1, make sure node 0 accepts too
accept_channel_msg.channel_reserve_satoshis = 0;

nodes[0].node.handle_accept_channel(node_b_id, &accept_channel_msg);

let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
assert!(
matches!(events[0], Event::FundingGenerationReady { channel_value_satoshis: 100_000, user_channel_id: 42, counterparty_node_id, .. } if counterparty_node_id == node_b_id)
);
}

#[xtest(feature = "_externalize_tests")]
fn test_error_if_0reserve_negotiates_down_to_legacy() {
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let channel_config = test_default_channel_config();
assert!(channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx);
let node_chanmgrs = create_node_chanmgrs(
2,
&node_cfgs,
&[Some(channel_config.clone()), Some(channel_config.clone())],
);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let node_b_id = nodes[1].node.get_our_node_id();

nodes[0]
.node
.create_channel_to_trusted_peer_0reserve(node_b_id, 100_000, 0, 42, None, None)
.unwrap();
let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id);
assert_eq!(
open_channel_msg.common_fields.channel_type,
Some(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies())
);
assert_eq!(open_channel_msg.channel_reserve_satoshis, 0);

let reason = "Don't like your channel".to_owned();
nodes[0].node.handle_error(
node_b_id,
&ErrorMessage {
channel_id: open_channel_msg.common_fields.temporary_channel_id,
data: reason.clone(),
},
);

let reason = ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(reason) };
let expected_closing = ExpectedCloseEvent::from_id_reason(
open_channel_msg.common_fields.temporary_channel_id,
false,
reason,
);
check_closed_events(&nodes[0], &[expected_closing]);
}
4 changes: 4 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3617,6 +3617,8 @@ pub enum TrustedChannelFeatures {
/// with a revoked commitment transaction *for free*.
///
/// Note that there is no guarantee that the counterparty accepts such a channel themselves.
///
/// The zero-reserve feature is not allowed on legacy / anchorless channels.
ZeroReserve,
/// Sets the combination of [`TrustedChannelFeatures::ZeroConf`] and [`TrustedChannelFeatures::ZeroReserve`]
ZeroConfZeroReserve,
Expand Down Expand Up @@ -3873,6 +3875,8 @@ impl<
/// transaction *for free*.
///
/// Note that there is no guarantee that the counterparty accepts such a channel.
///
/// The zero-reserve feature is not allowed on legacy / anchorless channels.
pub fn create_channel_to_trusted_peer_0reserve(
&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64,
user_channel_id: u128, temporary_channel_id: Option<ChannelId>,
Expand Down
Loading
Loading