Fix future unknown Event variant backwards compatibility

In 8ffc2d1, in 0.0.100, we added
a backwards compatibility feature to the reading of `Event`s - if
the type was unknown and odd, we'd simply ignore the event and
treat it as no event. However, we failed to read the
length-prefixed TLV stream when doing so, resulting in us reading
some of the skipped-event data as the next event or other data in
the ChannelManager.

We fix this by reading the varint length prefix written, then
skipping that many bytes when we come across an unknown odd event
type.

Author: TheBlueMatt

lightning/src/util/events.rs

@@ -18,7 +18,7 @@ use chain::keysinterface::SpendableOutputDescriptor;
 use ln::msgs;
 use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 use routing::network_graph::NetworkUpdate;
-use util::ser::{Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper};
+use util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper};
 
 use bitcoin::blockdata::script::Script;
 
@@ -265,13 +265,18 @@ impl Writeable for Event {
 					(2, claim_from_onchain_tx, required),
 				});
 			},
+			// Note that, going forward, all new events must only write data inside of
+			// `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write
+			// data via `write_tlv_fields`.
 		}
 		Ok(())
 	}
 }
 impl MaybeReadable for Event {
 	fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, msgs::DecodeError> {
 		match Readable::read(reader)? {
+			// Note that we do not write a length-prefixed TLV for FundingGenerationReady events,
+			// unlike all other events, thus we return immediately here.
 			0u8 => Ok(None),
 			1u8 => {
 				let f = || {
@@ -379,7 +384,19 @@ impl MaybeReadable for Event {
 				f()
 			},
 			// Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue.
-			x if x % 2 == 1 => Ok(None),
+			// Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt
+			// reads.
+			x if x % 2 == 1 => {
+				// If the event is of unknown type, assume it was written with `write_tlv_fields`,
+				// which prefixes the whole thing with a length BigSize. Because the event is
+				// odd-type unknown, we should treat it as `Ok(None)` even if it has some TLV
+				// fields which are event. Thus, we avoid using `read_tlv_fields` and simply read
+				// exactly the number of bytes specified, ignoring them entirely.
+				let tlv_len: BigSize = Readable::read(reader)?;
+				FixedLengthReader::new(reader, tlv_len.0)
+					.eat_remaining().map_err(|_| msgs::DecodeError::ShortRead)?;
+				Ok(None)
+			},
 			_ => Err(msgs::DecodeError::InvalidValue)
 		}
 	}