Skip to content

Trivial post-#3604 cleanups #3631

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

Closed
wants to merge 3 commits into from
Closed

Trivial post-#3604 cleanups #3631

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
bd11796
Simplify `Channel` deserialization in a few places
TheBlueMatt Feb 28, 2025
0af8ddb
Mark a few TLVs as `required` when reading `Channel`s
TheBlueMatt Feb 28, 2025
0b27457
Remove spurious set introduced in 21ed4778413af700c590b8aaa5e3a1485
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
62 changes: 26 additions & 36 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10296,31 +10296,26 @@ 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::InvalidValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, #3632 uses UnknownVersion instead. Not sure which you'd prefer.

}

// `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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a few more of these further down this method

Copy link
Contributor

Choose a reason for hiding this comment

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

@TheBlueMatt Yeah, see #3632. Didn't see this PR before opening that. Happy to close it if you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cherry-picked the extra commit on that branch in case you want to take it.

// 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 {
// Read the 8 bytes of backwards-compatibility ChannelConfig data.
let mut _val: u64 = Readable::read(reader)?;
}
// Read the 8 bytes of backwards-compatibility ChannelConfig data.
let mut _val: u64 = Readable::read(reader)?;

let channel_id: ChannelId = Readable::read(reader)?;
let channel_state = ChannelState::from_u32(Readable::read(reader)?).map_err(|_| DecodeError::InvalidValue)?;
Expand Down Expand Up @@ -10542,20 +10537,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 All @@ -10578,27 +10573,29 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
let mut next_holder_commitment_point_opt: Option<PublicKey> = None;
let mut is_manual_broadcast = None;

let mut config = LegacyChannelConfig::default();

read_tlv_fields!(reader, {
(0, announcement_sigs, option),
(1, minimum_depth, option),
(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 +10612,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 +10643,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 +10754,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 +10765,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 +10815,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 +10848,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 +11557,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