From f739c3112576469fe79f77c9777936608b1cfebc Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 2 Jul 2023 17:17:07 +0000 Subject: [PATCH] Build reminder updates with correct SCID field When the reminder updates were added, a dummy `ChannelUpdate` with a number of zero'd fields were created under the assumption that the zero'd fields would be ignored downstream when building serialized updates. However, the SCID field was `assert`'ed on (and serialized in the update), causing any reminder updates to cause an assertion panic. Instead, we do it the Right Way (tm) here and move the only-sometimes-available fields into the update type enum, ensuring we can't access "poison" fields downstream. --- src/lib.rs | 10 +++--- src/serialization.rs | 84 ++++++++++++++++++-------------------------- 2 files changed, 40 insertions(+), 54 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c6f266d..1115568 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,7 @@ use tokio::sync::mpsc; use crate::lookup::DeltaSet; use crate::persistence::GossipPersister; -use crate::serialization::UpdateSerializationMechanism; +use crate::serialization::UpdateSerialization; use crate::snapshot::Snapshotter; use crate::types::TestLogger; @@ -209,11 +209,11 @@ async fn serialize_delta(network_graph: Arc>, last_sync let mut update_count_full = 0; let mut update_count_incremental = 0; for current_update in serialization_details.updates { - match ¤t_update.mechanism { - UpdateSerializationMechanism::Full => { + match ¤t_update { + UpdateSerialization::Full(_) => { update_count_full += 1; } - UpdateSerializationMechanism::Incremental(_) | UpdateSerializationMechanism::Reminder => { + UpdateSerialization::Incremental(_, _) | UpdateSerialization::Reminder(_, _) => { update_count_incremental += 1; } }; @@ -221,7 +221,7 @@ async fn serialize_delta(network_graph: Arc>, last_sync let mut stripped_update = serialization::serialize_stripped_channel_update(¤t_update, &default_update_values, previous_update_scid); output.append(&mut stripped_update); - previous_update_scid = current_update.update.short_channel_id; + previous_update_scid = current_update.scid(); } // some stats diff --git a/src/serialization.rs b/src/serialization.rs index 1ab410a..4c1d828 100644 --- a/src/serialization.rs +++ b/src/serialization.rs @@ -58,11 +58,6 @@ impl Default for MutatedProperties { } } -pub(super) struct UpdateSerialization { - pub(super) update: UnsignedChannelUpdate, - pub(super) mechanism: UpdateSerializationMechanism, -} - impl MutatedProperties { /// Does not include flags because the flag byte is always sent in full fn len(&self) -> u8 { @@ -76,10 +71,27 @@ impl MutatedProperties { } } -pub(super) enum UpdateSerializationMechanism { - Full, - Incremental(MutatedProperties), - Reminder, +pub(super) enum UpdateSerialization { + Full(UnsignedChannelUpdate), + Incremental(UnsignedChannelUpdate, MutatedProperties), + Reminder(u64, u8), +} +impl UpdateSerialization { + pub(super) fn scid(&self) -> u64 { + match self { + UpdateSerialization::Full(latest_update)| + UpdateSerialization::Incremental(latest_update, _) => latest_update.short_channel_id, + UpdateSerialization::Reminder(scid, _) => *scid, + } + } + + fn flags(&self) -> u8 { + match self { + UpdateSerialization::Full(latest_update)| + UpdateSerialization::Incremental(latest_update, _) => latest_update.flags, + UpdateSerialization::Reminder(_, flags) => *flags, + } + } } struct FullUpdateValueHistograms { @@ -117,7 +129,7 @@ pub(super) fn serialize_delta_set(delta_set: DeltaSet, last_sync_timestamp: u32) *full_update_histograms.htlc_maximum_msat.entry(full_update.htlc_maximum_msat).or_insert(0) += 1; }; - for (_scid, channel_delta) in delta_set.into_iter() { + for (scid, channel_delta) in delta_set.into_iter() { // any announcement chain hash is gonna be the same value. Just set it from the first one. let channel_announcement_delta = channel_delta.announcement.as_ref().unwrap(); @@ -146,6 +158,7 @@ pub(super) fn serialize_delta_set(delta_set: DeltaSet, last_sync_timestamp: u32) if let Some(updates) = directed_updates { if let Some(latest_update_delta) = updates.latest_update_after_seen { let latest_update = latest_update_delta.update; + assert_eq!(latest_update.short_channel_id, scid, "Update in DB had wrong SCID column"); // the returned seen timestamp should be the latest of all the returned // announcements and latest updates @@ -158,43 +171,19 @@ pub(super) fn serialize_delta_set(delta_set: DeltaSet, last_sync_timestamp: u32) // serialize the update as a full update instead of as a change // this way, the default values can be computed more efficiently record_full_update_in_histograms(&latest_update); - serialization_set.updates.push(UpdateSerialization { - update: latest_update, - mechanism: UpdateSerializationMechanism::Full, - }); + serialization_set.updates.push(UpdateSerialization::Full(latest_update)); } else if mutated_properties.len() > 0 || mutated_properties.flags { // we don't count flags as mutated properties - serialization_set.updates.push(UpdateSerialization { - update: latest_update, - mechanism: UpdateSerializationMechanism::Incremental(mutated_properties), - }); + serialization_set.updates.push( + UpdateSerialization::Incremental(latest_update, mutated_properties)); } } else { // serialize the full update record_full_update_in_histograms(&latest_update); - serialization_set.updates.push(UpdateSerialization { - update: latest_update, - mechanism: UpdateSerializationMechanism::Full, - }); + serialization_set.updates.push(UpdateSerialization::Full(latest_update)); } } else if let Some(flags) = updates.serialization_update_flags { - // we need to insert a fake channel update where the only information - let fake_update = UnsignedChannelUpdate { - flags, - chain_hash: BlockHash::all_zeros(), - short_channel_id: 0, - cltv_expiry_delta: 0, - fee_base_msat: 0, - fee_proportional_millionths: 0, - htlc_maximum_msat: 0, - htlc_minimum_msat: 0, - timestamp: 0, - excess_data: Vec::with_capacity(0), - }; - serialization_set.updates.push(UpdateSerialization { - update: fake_update, - mechanism: UpdateSerializationMechanism::Reminder - }) + serialization_set.updates.push(UpdateSerialization::Reminder(scid, flags)); } } }; @@ -235,18 +224,17 @@ pub fn serialize_stripped_channel_announcement(announcement: &UnsignedChannelAnn } pub(super) fn serialize_stripped_channel_update(update: &UpdateSerialization, default_values: &DefaultUpdateValues, previous_scid: u64) -> Vec { - let latest_update = &update.update; - let mut serialized_flags = latest_update.flags; + let mut serialized_flags = update.flags(); - if previous_scid > latest_update.short_channel_id { + if previous_scid > update.scid() { panic!("unsorted scids!"); } let mut delta_serialization = Vec::new(); let mut prefixed_serialization = Vec::new(); - match &update.mechanism { - UpdateSerializationMechanism::Full => { + match update { + UpdateSerialization::Full(latest_update) => { if latest_update.cltv_expiry_delta != default_values.cltv_expiry_delta { serialized_flags |= 0b_0100_0000; latest_update.cltv_expiry_delta.write(&mut delta_serialization).unwrap(); @@ -272,8 +260,7 @@ pub(super) fn serialize_stripped_channel_update(update: &UpdateSerialization, de latest_update.htlc_maximum_msat.write(&mut delta_serialization).unwrap(); } } - - UpdateSerializationMechanism::Incremental(mutated_properties) => { + UpdateSerialization::Incremental(latest_update, mutated_properties) => { // indicate that this update is incremental serialized_flags |= 0b_1000_0000; @@ -302,13 +289,12 @@ pub(super) fn serialize_stripped_channel_update(update: &UpdateSerialization, de latest_update.htlc_maximum_msat.write(&mut delta_serialization).unwrap(); } }, - - UpdateSerializationMechanism::Reminder => { + UpdateSerialization::Reminder(_, _) => { // indicate that this update is incremental serialized_flags |= 0b_1000_0000; } } - let scid_delta = BigSize(latest_update.short_channel_id - previous_scid); + let scid_delta = BigSize(update.scid() - previous_scid); scid_delta.write(&mut prefixed_serialization).unwrap(); serialized_flags.write(&mut prefixed_serialization).unwrap(); -- 2.39.5