Skip to content

Reject channels serialized with version <= 2 #3632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 3, 2025
Merged

Reject channels serialized with version <= 2 #3632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13b0598
Reject channels serialized with version <= 2
jkczyz Feb 28, 2025
43a57fa
Remove unnecessary channel_value_satoshis overwrite
jkczyz Feb 28, 2025
6f7e1c5
Mark a few TLVs as `required` when reading `Channel`s
TheBlueMatt Feb 28, 2025
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
70 changes: 27 additions & 43 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10296,28 +10296,25 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
}
}

impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)> for FundedChannel<SP>
impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c ChannelTypeFeatures)> for FundedChannel<SP>
where
ES::Target: EntropySource,
SP::Target: SignerProvider
{
fn read<R : io::Read>(reader: &mut R, args: (&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)) -> Result<Self, DecodeError> {
let (entropy_source, signer_provider, serialized_height, our_supported_features) = args;
fn read<R : io::Read>(reader: &mut R, args: (&'a ES, &'b SP, &'c ChannelTypeFeatures)) -> Result<Self, DecodeError> {
let (entropy_source, signer_provider, our_supported_features) = args;
let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
if ver <= 2 {
return Err(DecodeError::UnknownVersion);
}

// `user_id` used to be a single u64 value. In order to remain backwards compatible with
// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We read
// the low bytes now and the high bytes later.
let user_id_low: u64 = Readable::read(reader)?;

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().options.forwarding_fee_proportional_millionths = Readable::read(reader)?;
config.as_mut().unwrap().options.cltv_expiry_delta = Readable::read(reader)?;
config.as_mut().unwrap().announce_for_forwarding = Readable::read(reader)?;
config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?;
} else {
let mut config = LegacyChannelConfig::default();
{
// Read the 8 bytes of backwards-compatibility ChannelConfig data.
let mut _val: u64 = Readable::read(reader)?;
}
Expand Down Expand Up @@ -10481,23 +10478,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
let holder_dust_limit_satoshis = Readable::read(reader)?;
let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?;
let mut counterparty_selected_channel_reserve_satoshis = None;
if ver == 1 {
// Read the old serialization from version 0.0.98.
counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?);
} else {
// Read the 8 bytes of backwards-compatibility data.
{
// Read the 8 bytes of backwards-compatibility counterparty_selected_channel_reserve_satoshis data.
let _dummy: u64 = Readable::read(reader)?;
}
let counterparty_htlc_minimum_msat = Readable::read(reader)?;
let holder_htlc_minimum_msat = Readable::read(reader)?;
let counterparty_max_accepted_htlcs = Readable::read(reader)?;

let mut minimum_depth = None;
if ver == 1 {
// Read the old serialization from version 0.0.98.
minimum_depth = Some(Readable::read(reader)?);
} else {
// Read the 4 bytes of backwards-compatibility data.
{
// Read the 4 bytes of backwards-compatibility minimum_depth data.
let _dummy: u32 = Readable::read(reader)?;
}

Expand Down Expand Up @@ -10542,20 +10533,20 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
// Prior to supporting channel type negotiation, all of our channels were static_remotekey
// only, so we default to that if none was written.
let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key());
let mut channel_creation_height = Some(serialized_height);
let mut channel_creation_height = 0u32;
let mut preimages_opt: Option<Vec<Option<PaymentPreimage>>> = None;

// If we read an old Channel, for simplicity we just treat it as "we never sent an
// AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine.
let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent);
let mut announcement_sigs_state = AnnouncementSigsState::NotSent;
let mut latest_inbound_scid_alias = None;
let mut outbound_scid_alias = None;
let mut outbound_scid_alias = 0u64;
let mut channel_pending_event_emitted = None;
let mut channel_ready_event_emitted = None;
let mut funding_tx_broadcast_safe_event_emitted = None;

let mut user_id_high_opt: Option<u64> = None;
let mut channel_keys_id: Option<[u8; 32]> = None;
let mut channel_keys_id = [0u8; 32];
let mut temporary_channel_id: Option<ChannelId> = None;
let mut holder_max_accepted_htlcs: Option<u16> = None;

Expand Down Expand Up @@ -10584,21 +10575,21 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
(2, channel_type, option),
(3, counterparty_selected_channel_reserve_satoshis, option),
(4, holder_selected_channel_reserve_satoshis, option),
(5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
(5, config, required),
(6, holder_max_htlc_value_in_flight_msat, option),
(7, shutdown_scriptpubkey, option),
(8, blocked_monitor_updates, optional_vec),
(9, target_closing_feerate_sats_per_kw, option),
(10, monitor_pending_update_adds, option), // Added in 0.0.122
(11, monitor_pending_finalized_fulfills, optional_vec),
(13, channel_creation_height, option),
(13, channel_creation_height, required),
(15, preimages_opt, optional_vec),
(17, announcement_sigs_state, option),
(17, announcement_sigs_state, required),
(19, latest_inbound_scid_alias, option),
(21, outbound_scid_alias, option),
(21, outbound_scid_alias, required),
(23, channel_ready_event_emitted, option),
(25, user_id_high_opt, option),
(27, channel_keys_id, option),
(27, channel_keys_id, required),
(28, holder_max_accepted_htlcs, option),
(29, temporary_channel_id, option),
(31, channel_pending_event_emitted, option),
Expand All @@ -10615,12 +10606,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
(53, funding_tx_broadcast_safe_event_emitted, option),
});

let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id {
let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
(channel_keys_id, holder_signer)
} else {
return Err(DecodeError::InvalidValue);
};
let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);

if let Some(preimages) = preimages_opt {
let mut iter = preimages.into_iter();
Expand Down Expand Up @@ -10651,8 +10637,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
// ChannelTransactionParameters may have had an empty features set upon deserialization.
// To account for that, we're proactively setting/overriding the field here.
channel_parameters.channel_type_features = chan_features.clone();
// ChannelTransactionParameters::channel_value_satoshis defaults to 0 prior to version 0.2.
channel_parameters.channel_value_satoshis = channel_value_satoshis;

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());
Expand Down Expand Up @@ -10764,7 +10748,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
context: ChannelContext {
user_id,

config: config.unwrap(),
config,

prev_config: None,

Expand All @@ -10775,7 +10759,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
channel_id,
temporary_channel_id,
channel_state,
announcement_sigs_state: announcement_sigs_state.unwrap(),
announcement_sigs_state,
secp_ctx,

latest_monitor_update_id,
Expand Down Expand Up @@ -10825,7 +10809,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
funding_tx_confirmed_in,
funding_tx_confirmation_height,
short_channel_id,
channel_creation_height: channel_creation_height.unwrap(),
channel_creation_height,

counterparty_dust_limit_satoshis,
holder_dust_limit_satoshis,
Expand Down Expand Up @@ -10858,7 +10842,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch

latest_inbound_scid_alias,
// Later in the ChannelManager deserialization phase we scan for channels and assign scid aliases if its missing
outbound_scid_alias: outbound_scid_alias.unwrap_or(0),
outbound_scid_alias,

funding_tx_broadcast_safe_event_emitted: funding_tx_broadcast_safe_event_emitted.unwrap_or(false),
channel_pending_event_emitted: channel_pending_event_emitted.unwrap_or(true),
Expand Down Expand Up @@ -11567,7 +11551,7 @@ mod tests {
let mut s = crate::io::Cursor::new(&encoded_chan);
let mut reader = crate::util::ser::FixedLengthReader::new(&mut s, encoded_chan.len() as u64);
let features = channelmanager::provided_channel_type_features(&config);
let decoded_chan = FundedChannel::read(&mut reader, (&&keys_provider, &&keys_provider, 0, &features)).unwrap();
let decoded_chan = FundedChannel::read(&mut reader, (&&keys_provider, &&keys_provider, &features)).unwrap();
assert_eq!(decoded_chan.context.pending_outbound_htlcs, pending_outbound_htlcs);
assert_eq!(decoded_chan.context.holding_cell_htlc_updates, holding_cell_htlc_updates);
}
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13634,7 +13634,7 @@ where
let mut close_background_events = Vec::new();
for _ in 0..channel_count {
let mut channel: FundedChannel<SP> = FundedChannel::read(reader, (
&args.entropy_source, &args.signer_provider, best_block_height, &provided_channel_type_features(&args.default_config)
&args.entropy_source, &args.signer_provider, &provided_channel_type_features(&args.default_config)
))?;
let logger = WithChannelContext::from(&args.logger, &channel.context, None);
let channel_id = channel.context.channel_id();
Expand Down
Loading