Add ability to broadcast our own node_announcement #435

TheBlueMatt · 2020-01-03T19:42:01Z

This is a somewhat-obvious oversight in the capabilities of
rust-lightning, though not a particularly interesting one until we
start relying on node_features (eg for variable-length-onions and
Base AMP).

Sadly its not fully automated as we don't really want to store the
list of available addresses from the user. However, with a simple
call to ChannelManager::broadcast_node_announcement and a sensible
peer_handler, the announcement is made.

TheBlueMatt · 2020-01-03T20:01:09Z

~~Based on #434 cause Im too lazy to properly rebase it.~~

ariard

Just to fix few comments otherwise looks good.

lightning/src/ln/msgs.rs

lightning/src/ln/channelmanager.rs

arik-so · 2020-02-11T23:31:38Z

lightning/src/ln/channelmanager.rs

+		let mut channel_state = self.channel_state.lock().unwrap();
+		channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastNodeAnnouncement {
+			msg: msgs::NodeAnnouncement {
+				signature: self.secp_ctx.sign(&msghash, &self.our_network_key),


we may wanna provide an interface for external signers later

Right. For now the external signing support hasn't even attempted to think about moving the node_id private key out, given its used in a ton of places. Eventually it'll need to be (well, probably after splitting it up more), but for now it is what it is.

arik-so · 2020-02-11T23:39:08Z

lightning/src/ln/msgs.rs

@@ -305,6 +305,58 @@ impl NetAddress {
 	}
 }

+/// A "set" of addresses which enforces that there can be only up to one of each net address type.


how does this actually enforce that the underlying NetAddress objects behave are what's expected?

Because the public API only allows you to add one address per type (replacing the previous one if there is one already).

see the set_address methods now. Also, hadn't realized NetAddress wasn't a standard library.

@arik-so FWIW, there is SocketAddrV4 and SocketAddrV6.

https://doc.rust-lang.org/std/net/index.html

lightning/src/ln/peer_handler.rs

arik-so · 2020-02-11T23:45:11Z

lightning/src/ln/peer_handler.rs

@@ -980,6 +988,21 @@ impl<Descriptor: SocketDescriptor, CM: Deref> PeerManager<Descriptor, CM> where
 							}
 						}
 					},
+					MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {


if Rust had a way of distributing match arms across multiple files, this would be the place

I'm not sure what you mean here?

I was referring to the fact that we're adding yet another match arm to a match that's hundreds of lines long.

arik-so · 2020-02-11T23:48:01Z

lightning/src/ln/router.rs

@@ -156,7 +156,7 @@ struct NodeInfo {
 	lowest_inbound_channel_fee_proportional_millionths: u32,

 	features: NodeFeatures,
-	last_update: u32,
+	last_update: Option<u32>,


what's the rationale for moving last_update into an Option as part of this diff? Also, there should be a comment somewhere saying it's a timestamp.

And also, this will break 18 years from now 😛

As otherwise we'll reject a 0 value (as we only accept anything newer than the previous value). Also note that it is not a timestamp, the spec only recommends that you use a timestamp. See-also #493. I added a comment noting that None implies we've not heard any updates yet.

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt · 2020-02-27T01:37:29Z

Addressed Jeff's feedback and added a commit which resolves #493.

As requested by Arik at lightningdevkit#435 (comment)

valentinewallace

Nice, very key feature!

lightning/src/ln/peer_handler.rs

valentinewallace · 2020-03-04T20:59:24Z

lightning/src/ln/peer_handler.rs

+							let encoded_msg = encode_msg!(msg);
+
+							for (ref descriptor, ref mut peer) in peers.peers.iter_mut() {
+								if !peer.channel_encryptor.is_ready_for_encryption() || peer.their_features.is_none() ||


Why would a Peer not have their_features set?

Before we've received their Init message (which will be the first message we receive, or we'll disconnect them), it should be None.

lightning/src/util/events.rs

lightning/src/ln/channelmanager.rs

valentinewallace · 2020-03-04T21:26:17Z

lightning/src/ln/channelmanager.rs

+
+		let announcement = msgs::UnsignedNodeAnnouncement {
+			features: NodeFeatures::supported(),
+			timestamp: self.last_node_announcement_serial.fetch_add(1, Ordering::AcqRel) as u32,


reasoning for AcqRel?

AcqRel is a good default - it ensures we always have the latest value here, without acting as a lock (as SeqCst does). If we're not relying on any other data to be consistent, but want consistency ourselves, AcqRel it is. Note that, on x86, AcqRel compiles down to nothing, whereas SeqCst is relatively expensive.

valentinewallace · 2020-03-04T21:42:04Z

lightning/src/ln/channelmanager.rs

+			node_id: self.get_our_node_id(),
+			rgb, alias,
+			addresses: addresses.into_vec(),
+			excess_address_data: Vec::new(),


Since this is our node announcement, we'll never have any excess address data, right? this is just for remote peer NodeAnnouncements that may randomly have excess address data? is this a common problem...?

That is correct. And, indeed, it is not a common thing. We have to have it as otherwise the signatures of things we relay will fail, but, in general, we anticipate almost never having anything in there, or if we do, a very small thing.

valentinewallace · 2020-03-04T21:46:38Z

lightning/src/ln/channelmanager.rs

+		loop {
+			// Just in case we end up in a race, we loop until we either successfully update
+			// last_node_announcement_serial or decide we don't need to.
+			let old_serial = self.last_node_announcement_serial.load(Ordering::Acquire);
+			if old_serial < header.time as usize {
+				if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() {
+					break;
+				}
+			} else { break; }
+		}


Confused about this loop -- we don't broadcast a NodeAnnouncement in this function, so what's the point of updating last_node_announcement_serial?

The compare_exchange updates it - the goal here is to update the value iff the block timestamp is >= the current latest value. I updated the comment to note that. Rust does have a method for this, but sadly its nightly-only (and it should compile down to something similar, just maybe with more optimal/effecient Orderings.

Ah, so the purpose is to have a "fresh" timestamp for the next time we do broadcast a node announcement

As requested by Arik at lightningdevkit#435 (comment)

codecov · 2020-03-05T04:06:35Z

Codecov Report

Merging #435 into master will increase coverage by 0.35%.
The diff coverage is 69.23%.

@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   89.75%   90.11%   +0.35%     
==========================================
  Files          34       34              
  Lines       18991    19054      +63     
==========================================
+ Hits        17046    17170     +124     
+ Misses       1945     1884      -61

Impacted Files	Coverage Δ
lightning/src/ln/peer_handler.rs	`48.70% <0.00%> (-1.39%)`	⬇️
lightning/src/ln/channelmanager.rs	`85.78% <0.00%> (+0.11%)`	⬆️
lightning/src/ln/functional_tests.rs	`96.42% <0.00%> (+0.12%)`	⬆️
lightning/src/util/ser_macros.rs	`97.27% <0.00%> (+0.68%)`	⬆️
lightning/src/ln/onion_utils.rs	`94.97% <0.00%> (+1.09%)`	⬆️
lightning/src/ln/router.rs	`91.08% <0.00%> (+2.28%)`	⬆️
lightning/src/ln/msgs.rs	`88.10% <0.00%> (+4.53%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d850e12...78c48f7. Read the comment docs.

As requested by Arik at lightningdevkit#435 (comment)

lightning/src/ln/channelmanager.rs

valentinewallace · 2020-03-05T20:34:33Z

lightning/src/ln/channelmanager.rs

+			excess_address_data: Vec::new(),
+			excess_data: Vec::new(),
+		};
+		let msghash = hash_to_message!(&Sha256dHash::hash(&announcement.encode()[..])[..]);


just to be sure -- spec says the signature should be over the double hash, and this seems to be a single hash?

Right, bitcoin_hashes types are confusing. That tiny little d that easy to miss means double :).

Ah, it is easy to miss.. 😅

valentinewallace · 2020-03-05T20:42:15Z

lightning/src/ln/channelmanager.rs

@@ -3459,6 +3509,8 @@ impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: De
 			per_peer_state.insert(peer_pubkey, Mutex::new(peer_state));
 		}

+		let last_node_announcement_serial: u32 = Readable::read(reader)?;


Hm, in theory could this cause a user migrating error?

Yea, we currently do no version checking for serialized types and break things all the time. We'll need to fix this come 0.1, but for now, no reason to slow down to build compatibility with 0.0.X.

valentinewallace · 2020-03-05T20:43:34Z

lightning/src/ln/functional_test_utils.rs

+	let a_events = nodes[a].node.get_and_clear_pending_msg_events();
+	assert_eq!(a_events.len(), 1);
+	let a_node_announcement = match a_events[0] {
+		MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {
+			(*msg).clone()
+		},
+		_ => panic!("Unexpected event"),
+	};
+
+	nodes[b].node.broadcast_node_announcement([1, 1, 1], [1; 32], Vec::new());
+	let b_events = nodes[b].node.get_and_clear_pending_msg_events();
+	assert_eq!(b_events.len(), 1);
+	let b_node_announcement = match b_events[0] {
+		MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {
+			(*msg).clone()
+		},
+		_ => panic!("Unexpected event"),
+	};


lightning/src/ln/channel.rs

As requested by Arik at lightningdevkit#435 (comment)

jkczyz

Looks good modulo some minor comments mostly around magic numbers. I had to dig through the BOLTs to discern their meaning. Using constants would prevent posterity from needing to do the same thing.

lightning/src/ln/channelmanager.rs

jkczyz · 2020-03-05T22:29:31Z

lightning/src/ln/channelmanager.rs

@@ -1334,6 +1340,50 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 		})
 	}

+	#[allow(dead_code)]
+	const HALF_MESSAGE_IS_ADDRS: u32 = 64*1024 / (msgs::NetAddress::MAX_LEN as u32 + 1) / 2;


It wasn't readily apparent to me that that the + 1 was for the address type. Could you make a constant for this using ::std::mem::size_of::<u8>?

Its explicit in the docs for MAX_LEN - there is a 1-byte type. I don't think mem::size_of makes that any clearer.

I only suggested to use mem::size_of when defining a constant. Fine to use a literal as well.

The point of defining a constant for 1 is to make its meaning explicit. Same goes for ::std::u16::MAX.

Right, but the 1 is explicit in the documentation for the len() method as well as the MAX_LEN docs, if we want to change the definition of that stuff we can, but any future users will see pretty clearly whats up. Using u16::MAX is fine, and I agreed it wasn't clear, so a comment was added indicating that its a message length.

lightning/src/ln/msgs.rs

jkczyz · 2020-03-05T22:49:59Z

lightning/src/ln/channelmanager.rs

+	const HALF_MESSAGE_IS_ADDRS: u32 = 64*1024 / (msgs::NetAddress::MAX_LEN as u32 + 1) / 2;
+	#[deny(const_err)]
+	#[allow(dead_code)]
+	const STATIC_ASSERT: u32 = Self::HALF_MESSAGE_IS_ADDRS - 500; // This will fail to compile if we use half of the message with 500 addresses


Use a constant for 500 since it is used in two places.

Its also written in the docs, so a comment confusingly indicates you may be able to change it freely (which you are not). The new comment I added to describe this const check a bit more explicitly calls it out as our public contract, is that fine?

I'm assuming you meant "a constant confusingly indicates you may be able to change it freely".

Isn't the point of a constant that it can't change? Saying that if a number is used in documentation we can't make it a constant seems rather silly. Just reference the constant in the documentation then. The actual value 500 seems rather unimportant in this case.

I tend to presume that if I see a constant I can change it, and the code will change appropriately. Same goes for a user who sees a constant referenced in documentation - it may change in a future version and code should handle it. That isn't the case here, so having the number appear three times across ten lines of code where one is in documentation seems like a better approach to me.

Unlike channel_update messages, node_announcement messages have no requirement that the timestamp is greater than 0.

lnd has been blatantly ignoring this line in the spec forever, so its somewhat of a lost cause trying to enforce it.

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt · 2020-03-05T23:45:25Z

Go travis fuzz checks, which caught a bug that would have been introduced here where we deduped addresses we read ending up writing only a subset of what we read which fails the strictness checks we need to apply on announcement messages.

lightning/src/ln/channelmanager.rs

jkczyz · 2020-03-06T01:18:47Z

lightning/src/ln/channelmanager.rs

@@ -1334,6 +1340,50 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 		})
 	}

+	#[allow(dead_code)]
+	const HALF_MESSAGE_IS_ADDRS: u32 = 64*1024 / (msgs::NetAddress::MAX_LEN as u32 + 1) / 2;


I only suggested to use mem::size_of when defining a constant. Fine to use a literal as well.

The point of defining a constant for 1 is to make its meaning explicit. Same goes for ::std::u16::MAX.

jkczyz

I'm not gonna hold this up with the last few comments, but I just want to state there's a good rationale for them. :) They may seem trivial, but I make sure to fully understand code that I review. And if there is something that is not obvious, I assume it may not be obvious to others reading the code. Thus, I try to make suggestions that will save other readers time.

This is a somewhat-obvious oversight in the capabilities of rust-lightning, though not a particularly interesting one until we start relying on node_features (eg for variable-length-onions and Base AMP). Sadly its not fully automated as we don't really want to store the list of available addresses from the user. However, with a simple call to ChannelManager::broadcast_node_announcement and a sensible peer_handler, the announcement is made.

As requested by Arik at lightningdevkit#435 (comment)

Fixes issue lightningdevkit#493 and should resolve some issues where other nodes (incorrectly) reject channel_update/node_announcement messages which have a serial number that is not a relatively recent timestamp.

valentinewallace

Great, nice progress on 0.0.11! 🏄‍♀️

valentinewallace · 2020-03-06T20:50:38Z

lightning/src/ln/channelmanager.rs

+	// Messages of up to 64KB should never end up more than half full with addresses, as that would
+	// be absurd. We ensure this by checking that at least 500 (our stated public contract on when


Hm, this isn't an actual spec rule, though, is it (mod the issue that this PR addresses)? >500 does seem extreme and I don't have an issue with enforcing it, just not sure the exact purpose of enforcing it...

IIRC (and if it doesn't it should) if we went to serialize it we'd generate something >64KB, panicing trying to send a message that overflows the max message size.

valentinewallace · 2020-03-06T20:57:58Z

lightning/src/ln/channelmanager.rs

+		if addresses.len() > 500 {
+			panic!("More than half the message size was taken up by public addresses!");
+		}


Good target for whomever addresses #529

TheBlueMatt force-pushed the 2020-01-node_announce branch from 1261639 to 198b089 Compare January 3, 2020 20:00

TheBlueMatt force-pushed the 2020-01-node_announce branch 5 times, most recently from c7f02e3 to c372439 Compare January 8, 2020 01:08

TheBlueMatt force-pushed the 2020-01-node_announce branch from c372439 to 91f4e5d Compare January 26, 2020 20:12

ariard reviewed Feb 11, 2020

View reviewed changes

lightning/src/ln/msgs.rs Outdated Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Show resolved Hide resolved

TheBlueMatt force-pushed the 2020-01-node_announce branch from 91f4e5d to 9c8165d Compare February 11, 2020 18:38

TheBlueMatt requested a review from arik-so February 11, 2020 23:17

arik-so reviewed Feb 11, 2020

View reviewed changes

TheBlueMatt force-pushed the 2020-01-node_announce branch from 9c8165d to 646acb8 Compare February 11, 2020 23:59

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 12, 2020

Further document the initial routing table send.

cba0e24

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt force-pushed the 2020-01-node_announce branch from 646acb8 to cba0e24 Compare February 12, 2020 18:41

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 12, 2020

Further document the initial routing table send.

bdaa10e

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 13, 2020

Further document the initial routing table send.

cd93bd0

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 15, 2020

Further document the initial routing table send.

4d99d00

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added this to the 0.0.10 milestone Feb 17, 2020

TheBlueMatt requested a review from jkczyz February 19, 2020 00:58

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 19, 2020

Further document the initial routing table send.

67870f1

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 20, 2020

Further document the initial routing table send.

b7ae627

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 21, 2020

Further document the initial routing table send.

c791d93

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 21, 2020

Further document the initial routing table send.

dac2475

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 21, 2020

Further document the initial routing table send.

82ff95a

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 23, 2020

Further document the initial routing table send.

96a7c48

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 24, 2020

Further document the initial routing table send.

3f87471

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 24, 2020

Further document the initial routing table send.

b14078f

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 24, 2020

Further document the initial routing table send.

8d46323

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 25, 2020

Further document the initial routing table send.

418dfe5

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt linked an issue Feb 27, 2020 that may be closed by this pull request

Swap announcement/update serials for block timestamp. #493

Closed

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 28, 2020

Further document the initial routing table send.

846c123

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt force-pushed the 2020-01-node_announce branch from 2340839 to b727baf Compare February 28, 2020 19:58

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Feb 29, 2020

Further document the initial routing table send.

fdf9886

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Mar 1, 2020

Further document the initial routing table send.

551c6f4

As requested by Arik at lightningdevkit#435 (comment)

valentinewallace reviewed Mar 4, 2020

View reviewed changes

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Mar 5, 2020

Further document the initial routing table send.

01a5d33

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt force-pushed the 2020-01-node_announce branch from b727baf to 4334755 Compare March 5, 2020 03:32

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Mar 5, 2020

Further document the initial routing table send.

abe15c0

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt force-pushed the 2020-01-node_announce branch from 4334755 to b8a4c45 Compare March 5, 2020 19:07

valentinewallace reviewed Mar 5, 2020

View reviewed changes

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Mar 5, 2020

Further document the initial routing table send.

914c486

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt force-pushed the 2020-01-node_announce branch from b8a4c45 to 295bde3 Compare March 5, 2020 21:17

jkczyz reviewed Mar 5, 2020

View reviewed changes

TheBlueMatt force-pushed the 2020-01-node_announce branch from 295bde3 to bd69f0d Compare March 5, 2020 23:01

TheBlueMatt added 2 commits March 5, 2020 18:30

Allow node_announcement timestamps of 0 in accordance with BOLT 7

fd08529

Unlike channel_update messages, node_announcement messages have no requirement that the timestamp is greater than 0.

Allow more than one address per type in node_announcement messages

107da97

lnd has been blatantly ignoring this line in the spec forever, so its somewhat of a lost cause trying to enforce it.

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Mar 5, 2020

Further document the initial routing table send.

5697f6d

As requested by Arik at lightningdevkit#435 (comment)

TheBlueMatt force-pushed the 2020-01-node_announce branch from bd69f0d to 48ee31f Compare March 5, 2020 23:43

jkczyz reviewed Mar 6, 2020

View reviewed changes

jkczyz approved these changes Mar 6, 2020

View reviewed changes

TheBlueMatt added 3 commits March 5, 2020 20:59

Further document the initial routing table send.

c2ca6d3

As requested by Arik at lightningdevkit#435 (comment)

Use block timestamps as the min for generated update messages.

78c48f7

Fixes issue lightningdevkit#493 and should resolve some issues where other nodes (incorrectly) reject channel_update/node_announcement messages which have a serial number that is not a relatively recent timestamp.

TheBlueMatt force-pushed the 2020-01-node_announce branch from 48ee31f to 78c48f7 Compare March 6, 2020 02:00

valentinewallace approved these changes Mar 6, 2020

View reviewed changes

TheBlueMatt merged commit 83c9eb4 into lightningdevkit:master Mar 9, 2020

		// Messages of up to 64KB should never end up more than half full with addresses, as that would
		// be absurd. We ensure this by checking that at least 500 (our stated public contract on when

Add ability to broadcast our own node_announcement #435

Add ability to broadcast our own node_announcement #435

Conversation

TheBlueMatt commented Jan 3, 2020

TheBlueMatt commented Jan 3, 2020 • edited

ariard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Feb 27, 2020

valentinewallace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace Mar 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 5, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Mar 5, 2020

Choose a reason for hiding this comment

jkczyz left a comment

Choose a reason for hiding this comment

valentinewallace left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace Mar 6, 2020 • edited

Choose a reason for hiding this comment

TheBlueMatt commented Jan 3, 2020 •

edited

valentinewallace Mar 4, 2020 •

edited

codecov bot commented Mar 5, 2020 •

edited

valentinewallace left a comment •

edited

valentinewallace Mar 6, 2020 •

edited