From 38b78eccb5e579558369f6e152647588acd11b38 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 29 Mar 2024 17:48:26 -0400 Subject: [PATCH] Add test coverage for cc78b77c715d6ef62693d4c1bc7190da990ec0fa cc78b77c715d6ef62693d4c1bc7190da990ec0fa fixed an important downgrade bug, but neglected to add test coverage. Here we recitfy that by adding a few simple tests of common cases. Tests heavily tweaked by Matt Corallo . --- lightning/src/util/ser_macros.rs | 127 ++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index df030d0b0..740b7c125 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -1148,7 +1148,7 @@ mod tests { use crate::io::{self, Cursor}; use crate::ln::msgs::DecodeError; - use crate::util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter}; + use crate::util::ser::{MaybeReadable, Readable, Writeable, HighZeroBytesDroppedBigSize, VecWriter}; use bitcoin::hashes::hex::FromHex; use bitcoin::secp256k1::PublicKey; @@ -1258,6 +1258,131 @@ mod tests { } else { panic!(); } } + /// A "V1" enum with only one variant + enum InnerEnumV1 { + StructVariantA { + field: u32, + }, + } + + impl_writeable_tlv_based_enum_upgradable!(InnerEnumV1, + (0, StructVariantA) => { + (0, field, required), + }, + ); + + struct OuterStructOptionalEnumV1 { + inner_enum: Option, + other_field: u32, + } + + impl_writeable_tlv_based!(OuterStructOptionalEnumV1, { + (0, inner_enum, upgradable_option), + (2, other_field, required), + }); + + /// An upgraded version of [`InnerEnumV1`] that added a second variant + enum InnerEnumV2 { + StructVariantA { + field: u32, + }, + StructVariantB { + field2: u64, + } + } + + impl_writeable_tlv_based_enum_upgradable!(InnerEnumV2, + (0, StructVariantA) => { + (0, field, required), + }, + (1, StructVariantB) => { + (0, field2, required), + }, + ); + + struct OuterStructOptionalEnumV2 { + inner_enum: Option, + other_field: u32, + } + + impl_writeable_tlv_based!(OuterStructOptionalEnumV2, { + (0, inner_enum, upgradable_option), + (2, other_field, required), + }); + + #[test] + fn upgradable_enum_option() { + // Test downgrading from `OuterStructOptionalEnumV2` to `OuterStructOptionalEnumV1` and + // ensure we still read the `other_field` just fine. + let serialized_bytes = OuterStructOptionalEnumV2 { + inner_enum: Some(InnerEnumV2::StructVariantB { field2: 64 }), + other_field: 0x1bad1dea, + }.encode(); + let mut s = Cursor::new(serialized_bytes); + + let outer_struct: OuterStructOptionalEnumV1 = Readable::read(&mut s).unwrap(); + assert!(outer_struct.inner_enum.is_none()); + assert_eq!(outer_struct.other_field, 0x1bad1dea); + } + + /// A struct that is read with an [`InnerEnumV1`] but is written with an [`InnerEnumV2`]. + struct OuterStructRequiredEnum { + #[allow(unused)] + inner_enum: InnerEnumV1, + } + + impl MaybeReadable for OuterStructRequiredEnum { + fn read(reader: &mut R) -> Result, DecodeError> { + let mut inner_enum = crate::util::ser::UpgradableRequired(None); + read_tlv_fields!(reader, { + (0, inner_enum, upgradable_required), + }); + Ok(Some(Self { + inner_enum: inner_enum.0.unwrap(), + })) + } + } + + impl Writeable for OuterStructRequiredEnum { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { + write_tlv_fields!(writer, { + (0, InnerEnumV2::StructVariantB { field2: 0xdeadbeef }, required), + }); + Ok(()) + } + } + + struct OuterOuterStruct { + outer_struct: Option, + other_field: u32, + } + + impl_writeable_tlv_based!(OuterOuterStruct, { + (0, outer_struct, upgradable_option), + (2, other_field, required), + }); + + + #[test] + fn upgradable_enum_required() { + // Test downgrading from an `OuterOuterStruct` (i.e. test downgrading an + // `upgradable_required` `InnerEnumV2` to an `InnerEnumV1`). + // + // Note that `OuterStructRequiredEnum` has a split write/read implementation that writes an + // `InnerEnumV2::StructVariantB` irrespective of the value of `inner_enum`. + + let dummy_inner_enum = InnerEnumV1::StructVariantA { field: 42 }; + let serialized_bytes = OuterOuterStruct { + outer_struct: Some(OuterStructRequiredEnum { inner_enum: dummy_inner_enum }), + other_field: 0x1bad1dea, + }.encode(); + let mut s = Cursor::new(serialized_bytes); + + let outer_outer_struct: OuterOuterStruct = Readable::read(&mut s).unwrap(); + assert!(outer_outer_struct.outer_struct.is_none()); + assert_eq!(outer_outer_struct.other_field, 0x1bad1dea); + } + // BOLT TLV test cases fn tlv_reader_n1(s: &[u8]) -> Result<(Option>, Option, Option<(PublicKey, u64, u64)>, Option), DecodeError> { let mut s = Cursor::new(s); -- 2.39.5