Merge pull request #651 from naumenkogs/2020-06-routing-data-improvements

TheBlueMatt · web-flow · commit 779ff6721bfc · 2020-07-27T10:18:13.000-07:00
Routing improvements
diff --git a/fuzz/src/bin/gen_target.sh b/fuzz/src/bin/gen_target.sh
@@ -31,12 +31,12 @@ GEN_TEST msg_update_fee msg_targets::
 GEN_TEST msg_update_fulfill_htlc msg_targets::
 
 GEN_TEST msg_channel_announcement msg_targets::
-GEN_TEST msg_channel_update msg_targets::
 GEN_TEST msg_node_announcement msg_targets::
 
 GEN_TEST msg_update_add_htlc msg_targets::
 GEN_TEST msg_error_message msg_targets::
-GEN_TEST msg_onion_hop_data msg_targets::
+GEN_TEST msg_channel_update msg_targets::
 
+GEN_TEST msg_onion_hop_data msg_targets::
 GEN_TEST msg_ping msg_targets::
 GEN_TEST msg_pong msg_targets::
diff --git a/fuzz/src/msg_targets/gen_target.sh b/fuzz/src/msg_targets/gen_target.sh
@@ -29,11 +29,11 @@ GEN_TEST UpdateFee test_msg ""
 GEN_TEST UpdateFulfillHTLC test_msg ""
 
 GEN_TEST ChannelAnnouncement test_msg_exact ""
-GEN_TEST ChannelUpdate test_msg_exact ""
 GEN_TEST NodeAnnouncement test_msg_exact ""
 
 GEN_TEST UpdateAddHTLC test_msg_hole ", 85, 33"
 GEN_TEST ErrorMessage test_msg_hole ", 32, 2"
+GEN_TEST ChannelUpdate test_msg_hole ", 108, 1"
 
 GEN_TEST Init test_msg_simple ""
 GEN_TEST OnionHopData test_msg_simple ""
diff --git a/fuzz/src/msg_targets/mod.rs b/fuzz/src/msg_targets/mod.rs
@@ -16,10 +16,10 @@ pub mod msg_update_fail_malformed_htlc;
 pub mod msg_update_fee;
 pub mod msg_update_fulfill_htlc;
 pub mod msg_channel_announcement;
-pub mod msg_channel_update;
 pub mod msg_node_announcement;
 pub mod msg_update_add_htlc;
 pub mod msg_error_message;
+pub mod msg_channel_update;
 pub mod msg_init;
 pub mod msg_onion_hop_data;
 pub mod msg_ping;
diff --git a/fuzz/src/msg_targets/msg_channel_update.rs b/fuzz/src/msg_targets/msg_channel_update.rs
@@ -8,11 +8,11 @@ use utils::test_logger;
 
 #[inline]
 pub fn msg_channel_update_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	test_msg_exact!(msgs::ChannelUpdate, data);
+	test_msg_hole!(msgs::ChannelUpdate, data, 108, 1);
 }
 
 #[no_mangle]
 pub extern "C" fn msg_channel_update_run(data: *const u8, datalen: usize) {
 	let data = unsafe { std::slice::from_raw_parts(data, datalen) };
-	test_msg_exact!(msgs::ChannelUpdate, data);
+	test_msg_hole!(msgs::ChannelUpdate, data, 108, 1);
 }
diff --git a/fuzz/src/msg_targets/utils.rs b/fuzz/src/msg_targets/utils.rs
@@ -78,8 +78,8 @@ macro_rules! test_msg_exact {
 	}
 }
 
-// Tests a message that must survive roundtrip exactly, modulo one "hole" which may be set to 0s on
-// re-serialization.
+// Tests a message that must survive roundtrip exactly, modulo one "hole" which may be set to
+// any value on re-serialization.
 #[macro_export]
 macro_rules! test_msg_hole {
 	($MsgType: path, $data: ident, $hole: expr, $hole_len: expr) => {
diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
@@ -172,12 +172,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 				let _ = net_graph_msg_handler.handle_channel_announcement(&decode_msg_with_len16!(msgs::ChannelAnnouncement, 64*4, 32+8+33*4));
 			},
 			2 => {
-				let _ = net_graph_msg_handler.handle_channel_update(&decode_msg!(msgs::ChannelUpdate, 128));
+				let _ = net_graph_msg_handler.handle_channel_update(&decode_msg!(msgs::ChannelUpdate, 136));
 			},
 			3 => {
 				match get_slice!(1)[0] {
 					0 => {
-						net_graph_msg_handler.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelUpdateMessage {msg: decode_msg!(msgs::ChannelUpdate, 128)});
+						net_graph_msg_handler.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelUpdateMessage {msg: decode_msg!(msgs::ChannelUpdate, 136)});
 					},
 					1 => {
 						let short_channel_id = slice_to_be64(get_slice!(8));
diff --git a/fuzz/targets.h b/fuzz/targets.h
@@ -22,10 +22,10 @@ void msg_update_fail_malformed_htlc_run(const unsigned char* data, size_t data_l
 void msg_update_fee_run(const unsigned char* data, size_t data_len);
 void msg_update_fulfill_htlc_run(const unsigned char* data, size_t data_len);
 void msg_channel_announcement_run(const unsigned char* data, size_t data_len);
-void msg_channel_update_run(const unsigned char* data, size_t data_len);
 void msg_node_announcement_run(const unsigned char* data, size_t data_len);
 void msg_update_add_htlc_run(const unsigned char* data, size_t data_len);
 void msg_error_message_run(const unsigned char* data, size_t data_len);
+void msg_channel_update_run(const unsigned char* data, size_t data_len);
 void msg_onion_hop_data_run(const unsigned char* data, size_t data_len);
 void msg_ping_run(const unsigned char* data, size_t data_len);
 void msg_pong_run(const unsigned char* data, size_t data_len);
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -3128,6 +3128,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		self.our_htlc_minimum_msat
 	}
 
+	/// Allowed in any state (including after shutdown)
+	pub fn get_announced_htlc_max_msat(&self) -> u64 {
+		return cmp::min(
+			// Upper bound by capacity. We make it a bit less than full capacity to prevent attempts
+			// to use full capacity. This is an effort to reduce routing failures, because in many cases
+			// channel might have been used to route very small values (either by honest users or as DoS).
+			self.channel_value_satoshis * 9 / 10,
+
+			Channel::<ChanSigner>::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis)
+		);
+	}
+
 	/// Allowed in any state (including after shutdown)
 	pub fn get_their_htlc_minimum_msat(&self) -> u64 {
 		self.our_htlc_minimum_msat
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -34,7 +34,7 @@ use ln::features::{InitFeatures, NodeFeatures};
 use routing::router::{Route, RouteHop};
 use ln::msgs;
 use ln::onion_utils;
-use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
+use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, OptionalField};
 use chain::keysinterface::{ChannelKeys, KeysInterface, KeysManager, InMemoryChannelKeys};
 use util::config::UserConfig;
 use util::{byte_utils, events};
@@ -1186,7 +1186,8 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 							res.extend_from_slice(&byte_utils::be32_to_array(msg.cltv_expiry));
 						}
 						else if code == 0x1000 | 20 {
-							res.extend_from_slice(&byte_utils::be16_to_array(chan_update.contents.flags));
+							// TODO: underspecified, follow https://github.com/lightningnetwork/lightning-rfc/issues/791
+							res.extend_from_slice(&byte_utils::be16_to_array(0));
 						}
 						res.extend_from_slice(&chan_update.encode_with_len()[..]);
 					}
@@ -1212,9 +1213,10 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 			chain_hash: self.genesis_hash,
 			short_channel_id: short_channel_id,
 			timestamp: chan.get_update_time_counter(),
-			flags: (!were_node_one) as u16 | ((!chan.is_live() as u16) << 1),
+			flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
 			cltv_expiry_delta: CLTV_EXPIRY_DELTA,
 			htlc_minimum_msat: chan.get_our_htlc_minimum_msat(),
+			htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
 			fee_base_msat: chan.get_our_fee_base_msat(&self.fee_estimator),
 			fee_proportional_millionths: chan.get_fee_proportional_millionths(),
 			excess_data: Vec::new(),
@@ -2494,7 +2496,8 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 							let reason = if let Ok(upd) = self.get_channel_update(chan) {
 								onion_utils::build_first_hop_failure_packet(incoming_shared_secret, error_code, &{
 									let mut res = Vec::with_capacity(8 + 128);
-									res.extend_from_slice(&byte_utils::be16_to_array(upd.contents.flags));
+									// TODO: underspecified, follow https://github.com/lightningnetwork/lightning-rfc/issues/791
+									res.extend_from_slice(&byte_utils::be16_to_array(0));
 									res.extend_from_slice(&upd.encode_with_len()[..]);
 									res
 								}[..])
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
@@ -15,7 +15,7 @@ use ln::{chan_utils, onion_utils};
 use routing::router::{Route, RouteHop, get_route};
 use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction};
+use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction, OptionalField};
 use util::enforcing_trait_impls::EnforcingChannelKeys;
 use util::{byte_utils, test_utils};
 use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
@@ -6058,6 +6058,7 @@ impl msgs::ChannelUpdate {
 				flags: 0,
 				cltv_expiry_delta: 0,
 				htlc_minimum_msat: 0,
+				htlc_maximum_msat: OptionalField::Absent,
 				fee_base_msat: 0,
 				fee_proportional_millionths: 0,
 				excess_data: vec![],
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
@@ -427,9 +427,10 @@ pub(crate) struct UnsignedChannelUpdate {
 	pub(crate) chain_hash: BlockHash,
 	pub(crate) short_channel_id: u64,
 	pub(crate) timestamp: u32,
-	pub(crate) flags: u16,
+	pub(crate) flags: u8,
 	pub(crate) cltv_expiry_delta: u16,
 	pub(crate) htlc_minimum_msat: u64,
+	pub(crate) htlc_maximum_msat: OptionalField<u64>,
 	pub(crate) fee_base_msat: u32,
 	pub(crate) fee_proportional_millionths: u32,
 	pub(crate) excess_data: Vec<u8>,
@@ -517,7 +518,7 @@ pub enum HTLCFailChannelUpdate {
 /// As we wish to serialize these differently from Option<T>s (Options get a tag byte, but
 /// OptionalFeild simply gets Present if there are enough bytes to read into it), we have a
 /// separate enum type for them.
-#[derive(Clone, PartialEq)]
+#[derive(Clone, PartialEq, Debug)]
 pub enum OptionalField<T> {
 	/// Optional field is included in message
 	Present(T),
@@ -742,6 +743,26 @@ impl Readable for OptionalField<Script> {
 	}
 }
 
+impl Writeable for OptionalField<u64> {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+		match *self {
+			OptionalField::Present(ref value) => {
+				value.write(w)?;
+			},
+			OptionalField::Absent => {}
+		}
+		Ok(())
+	}
+}
+
+impl Readable for OptionalField<u64> {
+	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+		let value: u64 = Readable::read(r)?;
+		Ok(OptionalField::Present(value))
+	}
+}
+
+
 impl_writeable_len_match!(AcceptChannel, {
 		{AcceptChannel{ shutdown_scriptpubkey: OptionalField::Present(ref script), .. }, 270 + 2 + script.len()},
 		{_, 270}
@@ -1180,31 +1201,46 @@ impl_writeable_len_match!(ChannelAnnouncement, {
 
 impl Writeable for UnsignedChannelUpdate {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-		w.size_hint(64 + self.excess_data.len());
+		let mut size = 64 + self.excess_data.len();
+		let mut message_flags: u8 = 0;
+		if let OptionalField::Present(_) = self.htlc_maximum_msat {
+			size += 8;
+			message_flags = 1;
+		}
+		w.size_hint(size);
 		self.chain_hash.write(w)?;
 		self.short_channel_id.write(w)?;
 		self.timestamp.write(w)?;
-		self.flags.write(w)?;
+		let all_flags = self.flags as u16 | ((message_flags as u16) << 8);
+		all_flags.write(w)?;
 		self.cltv_expiry_delta.write(w)?;
 		self.htlc_minimum_msat.write(w)?;
 		self.fee_base_msat.write(w)?;
 		self.fee_proportional_millionths.write(w)?;
+		self.htlc_maximum_msat.write(w)?;
 		w.write_all(&self.excess_data[..])?;
 		Ok(())
 	}
 }
 
 impl Readable for UnsignedChannelUpdate {
 	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+		let has_htlc_maximum_msat;
 		Ok(Self {
 			chain_hash: Readable::read(r)?,
 			short_channel_id: Readable::read(r)?,
 			timestamp: Readable::read(r)?,
-			flags: Readable::read(r)?,
+			flags: {
+				let flags: u16 = Readable::read(r)?;
+				let message_flags = flags >> 8;
+				has_htlc_maximum_msat = (message_flags as i32 & 1) == 1;
+				flags as u8
+			},
 			cltv_expiry_delta: Readable::read(r)?,
 			htlc_minimum_msat: Readable::read(r)?,
 			fee_base_msat: Readable::read(r)?,
 			fee_proportional_millionths: Readable::read(r)?,
+			htlc_maximum_msat: if has_htlc_maximum_msat { Readable::read(r)? } else { OptionalField::Absent },
 			excess_data: {
 				let mut excess_data = vec![];
 				r.read_to_end(&mut excess_data)?;
@@ -1597,20 +1633,21 @@ mod tests {
 		do_encoding_node_announcement(false, false, true, false, true, false, false);
 	}
 
-	fn do_encoding_channel_update(direction: bool, disable: bool, htlc_maximum_msat: bool) {
+	fn do_encoding_channel_update(direction: bool, disable: bool, htlc_maximum_msat: bool, excess_data: bool) {
 		let secp_ctx = Secp256k1::new();
 		let (privkey_1, _) = get_keys_from!("0101010101010101010101010101010101010101010101010101010101010101", secp_ctx);
 		let sig_1 = get_sig_on!(privkey_1, secp_ctx, String::from("01010101010101010101010101010101"));
 		let unsigned_channel_update = msgs::UnsignedChannelUpdate {
 			chain_hash: BlockHash::from_hex("6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000").unwrap(),
 			short_channel_id: 2316138423780173,
 			timestamp: 20190119,
-			flags: if direction { 1 } else { 0 } | if disable { 1 << 1 } else { 0 } | if htlc_maximum_msat { 1 << 8 } else { 0 },
+			flags: if direction { 1 } else { 0 } | if disable { 1 << 1 } else { 0 },
 			cltv_expiry_delta: 144,
 			htlc_minimum_msat: 1000000,
+			htlc_maximum_msat: if htlc_maximum_msat { OptionalField::Present(131355275467161) } else { OptionalField::Absent },
 			fee_base_msat: 10000,
 			fee_proportional_millionths: 20,
-			excess_data: if htlc_maximum_msat { vec![0, 0, 0, 0, 59, 154, 202, 0] } else { Vec::new() }
+			excess_data: if excess_data { vec![0, 0, 0, 0, 59, 154, 202, 0] } else { Vec::new() }
 		};
 		let channel_update = msgs::ChannelUpdate {
 			signature: sig_1,
@@ -1636,18 +1673,26 @@ mod tests {
 		}
 		target_value.append(&mut hex::decode("009000000000000f42400000271000000014").unwrap());
 		if htlc_maximum_msat {
+			target_value.append(&mut hex::decode("0000777788889999").unwrap());
+		}
+		if excess_data {
 			target_value.append(&mut hex::decode("000000003b9aca00").unwrap());
 		}
 		assert_eq!(encoded_value, target_value);
 	}
 
 	#[test]
 	fn encoding_channel_update() {
-		do_encoding_channel_update(false, false, false);
-		do_encoding_channel_update(true, false, false);
-		do_encoding_channel_update(false, true, false);
-		do_encoding_channel_update(false, false, true);
-		do_encoding_channel_update(true, true, true);
+		do_encoding_channel_update(false, false, false, false);
+		do_encoding_channel_update(false, false, false, true);
+		do_encoding_channel_update(true, false, false, false);
+		do_encoding_channel_update(true, false, false, true);
+		do_encoding_channel_update(false, true, false, false);
+		do_encoding_channel_update(false, true, false, true);
+		do_encoding_channel_update(false, false, true, false);
+		do_encoding_channel_update(false, false, true, true);
+		do_encoding_channel_update(true, true, true, false);
+		do_encoding_channel_update(true, true, true, true);
 	}
 
 	fn do_encoding_open_channel(random_bit: bool, shutdown: bool) {
diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs

Original file line number	Diff line number	Diff line change
`@@ -8,11 +8,11 @@ use utils::test_logger;`
`8`	`8`
`9`	`9`	`#[inline]`
`10`	`10`	`pub fn msg_channel_update_test<Out: test_logger::Output>(data: &[u8], _out: Out) {`
`11`		`- test_msg_exact!(msgs::ChannelUpdate, data);`
	`11`	`+ test_msg_hole!(msgs::ChannelUpdate, data, 108, 1);`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`#[no_mangle]`
`15`	`15`	`pub extern "C" fn msg_channel_update_run(data: *const u8, datalen: usize) {`
`16`	`16`	`let data = unsafe { std::slice::from_raw_parts(data, datalen) };`
`17`		`- test_msg_exact!(msgs::ChannelUpdate, data);`
	`17`	`+ test_msg_hole!(msgs::ChannelUpdate, data, 108, 1);`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,8 +78,8 @@ macro_rules! test_msg_exact {`
`78`	`78`	`}`
`79`	`79`	`}`
`80`	`80`
`81`		`-// Tests a message that must survive roundtrip exactly, modulo one "hole" which may be set to 0s on`
`82`		`-// re-serialization.`
	`81`	`+// Tests a message that must survive roundtrip exactly, modulo one "hole" which may be set to`
	`82`	`+// any value on re-serialization.`
`83`	`83`	`#[macro_export]`
`84`	`84`	`macro_rules! test_msg_hole {`
`85`	`85`	`($MsgType: path, $data: ident, $hole: expr, $hole_len: expr) => {`