Support for SCID Aliases #1311

TheBlueMatt · 2022-02-16T00:52:49Z

This is part 1/3 of the 0conf PR set, which only adds support for SCID aliaes using the new field in funding_locked. It does not yet support the relevant feature bit which is going to be a chunk more work as it will require fully implementing channel type negotiation, something we're currently missing (that will be PR 2). PR 3 will then add actual 0conf support, which isn't too bad after the first two land.

This and all the 0conf PRs will need a very careful review. I'd especially recommend reviewers scan through all our existing uses of short_channel_id and familiarize yourself with them either for this or later 0conf PRs as most uses of them will need to change.

Based on the 1.41.1 MSRV bump cause I'm lazy.

codecov-commenter · 2022-02-16T03:12:48Z

Codecov Report

Merging #1311 (bee9478) into main (0e0aabe) will increase coverage by 0.02%.
The diff coverage is 91.56%.

❗ Current head bee9478 differs from pull request most recent head 98c8ca5. Consider uploading reports for the commit 98c8ca5 to get more accurate results

@@            Coverage Diff             @@
##             main    #1311      +/-   ##
==========================================
+ Coverage   90.57%   90.60%   +0.02%     
==========================================
  Files          72       73       +1     
  Lines       40082    40219     +137     
==========================================
+ Hits        36306    36442     +136     
- Misses       3776     3777       +1

Impacted Files	Coverage Δ
lightning/src/ln/mod.rs	`95.00% <ø> (ø)`
lightning/src/ln/msgs.rs	`85.84% <66.66%> (-0.05%)`	⬇️
lightning/src/ln/channelmanager.rs	`84.76% <75.82%> (+0.16%)`	⬆️
lightning/src/ln/channel.rs	`89.20% <90.24%> (-0.04%)`	⬇️
lightning/src/ln/priv_short_conf_tests.rs	`96.84% <96.84%> (ø)`
lightning-invoice/src/utils.rs	`91.82% <100.00%> (+3.68%)`	⬆️
lightning/src/ln/functional_test_utils.rs	`95.50% <100.00%> (+0.22%)`	⬆️
lightning/src/ln/functional_tests.rs	`97.06% <100.00%> (-0.07%)`	⬇️
lightning/src/ln/onion_route_tests.rs	`97.62% <100.00%> (ø)`
lightning/src/ln/reorg_tests.rs	`100.00% <100.00%> (ø)`
... and 6 more

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 0e0aabe...98c8ca5. Read the comment docs.

jkczyz · 2022-02-16T16:39:21Z

This and all the 0conf PRs will need a very careful review. I'd especially recommend reviewers scan through all our existing uses of short_channel_id and familiarize yourself with them either for this or later 0conf PRs as most uses of them will need to change.

Given this concern, could we make a ShortChannelId type that encapsulates the alias? Seems that would help prevent missed occurrences and possible future misuses. The interface would make it explicit which representation is used.

TheBlueMatt · 2022-02-16T16:44:37Z

Given this concern, could we make a ShortChannelId type that encapsulates the alias? Seems that would help prevent missed occurrences and possible future misuses. The interface would make it explicit which representation is used.

Hmm, we could make a newtype for it, I don't really have a strong feeling either way there, but I don't think we should make an enum, because there's not a ton (or any?) of places where we both (a) care which type it is, but (b) want to mix the two. In public APIs we mostly expose both and let the user figure it out, in internal stuff we mostly mix them and don't care which type it is.

jkczyz · 2022-02-16T17:01:13Z

Hmm, we could make a newtype for it, I don't really have a strong feeling either way there, but I don't think we should make an enum, because there's not a ton (or any?) of places where we both (a) care which type it is, but (b) want to mix the two. In public APIs we mostly expose both and let the user figure it out, in internal stuff we mostly mix them and don't care which type it is.

Was thinking a struct that has an optional alias (aliases?) or such, with some sort of accessors. Haven't stared at the PR long enough to figure what interface would be best, though. Could just be simple ones just so it's explicit which is being used. Or something more based on the use case / strongly typed to let the compiler enforce the right behavior.

TheBlueMatt · 2022-02-16T18:48:52Z

Right, I think we need to be explicit about where we want it - external accessor(s) in ChannelDetails I think could make sense, dunno if we have to change the storage at all for that, though, just a util method could suffice. Internally I'm not sure there's a ton of cleanup we can do, but the public interface I think we could make simpler.

jkczyz · 2022-02-16T19:52:57Z

The reason for using it internally is to avoid programmatic mistakes (both now and during further changes) and the need for reviewers to meticulously examine each use of short_channel_id. Maybe you could elaborate on what you mean by a util method as I'm not sure I understand.

TheBlueMatt · 2022-02-16T20:02:46Z

Right, I think my point is that there isn't a good way to do it for internal stuff that isn't needless boilerplate that adds anything - there is, I believe, only exactly one place where both (a) we co-mingle the two types and (b) we care about knowing which type the thing is.

TheBlueMatt · 2022-02-16T20:03:24Z

For public use, there's a few places now in eg lightning-invoice where we try to fetch the SCID alias and then fall back to SCID if there isn't one, that could be a util method on ChannelDetails, but its not exactly a super huge change.

jkczyz · 2022-02-17T18:18:48Z

Right, I think my point is that there isn't a good way to do it for internal stuff that isn't needless boilerplate that adds anything - there is, I believe, only exactly one place where both (a) we co-mingle the two types and (b) we care about knowing which type the thing is.

Yeah, I think you're right. Took a more thorough look through the PR and spec change. This just seemed rather ominous on first read without fully grasping the spec change:

This and all the 0conf PRs will need a very careful review. I'd especially recommend reviewers scan through all our existing uses of short_channel_id and familiarize yourself with them either for this or later 0conf PRs as most uses of them will need to change.

jkczyz

Took an overall pass after better understanding the spec change. Wish there were a way to not publish nits so I can comment on them when I see them without publishing them. 😛

lightning-invoice/src/utils.rs

lightning/src/ln/channel.rs

jkczyz · 2022-02-17T17:31:13Z

lightning/src/ln/channel.rs

+					// If they have sent updated points, funding_locked is always supposed to match
+					// their "first" point, which we re-derive here.
+					self.commitment_secrets.get_secret(INITIAL_COMMITMENT_NUMBER - 1)
+						.map(|secret| SecretKey::from_slice(&secret).ok()).flatten()


Use and_then instead of map and flatten. Although, do we expect from_slice to ever fail here? If it does, the error message below isn't entirely accurate.

Actaully, we can unwrap every step here, we should have already validated the whole thing by the time we get here.

jkczyz · 2022-02-17T17:39:30Z

lightning/src/ln/channel.rs

+	/// Allowed in any state (including after shutdown)
+	pub fn get_latest_inbound_scid_alias(&self) -> Option<u64> {
+		self.latest_inbound_scid_alias
+	}
+
+	/// Allowed in any state (including after shutdown)
+	pub fn get_outbound_scid_alias(&self) -> u64 {
+		self.outbound_scid_alias
+	}


nit: I know we haven't been consistent but can we drop the get_ prefixes?

I mean I guess, what's the rationale, though? I figured this is one of those things that everyone is gonna have an opinion on and neither is really more or less readable, but maybe you have some specific criteria in mind that makes things clearer? At least personally I like the get_ prefix as its pretty clear what's going on when you see it.

Mostly to align with idiomatic rust. Maybe not so important for internal structs.

lightning/src/ln/channelmanager.rs

TheBlueMatt · 2022-03-02T18:53:28Z

Rebased.

lightning/src/ln/channelmanager.rs

valentinewallace · 2022-03-02T20:34:16Z

lightning/src/ln/channelmanager.rs

-											if let Some(short_id) = channel.get_short_channel_id() {
-												channel_state.short_to_id.remove(&short_id);
-											}
+											let mut channel = remove_channel!(self, channel_state, chan);


About 100 lines up and maybe elsewhere, I see a call to get_channel_update_for_unicast(..).unwrap() that seems like it may panic due to get_short_chan_id returning None

I'm not sure that this is an issue in this PR? Its an issue once we go to 0conf, but we don't change the semantics of SCIDs here.

That's fair. On a semi-related note, I'm not sure we're compliant with this part of the spec:

- if it returns a `channel_update`: - MUST set `short_channel_id` to the `short_channel_id` used by the incoming onion.

seems we'll always use chan.get_short_channel_id() in channel updates on onion error.

Good catch. This is really nasty to resolve, so I may end up suggesting we leave this for the next PR as well, but I'll work on it and open up that PR today or so.

valentinewallace · 2022-03-02T21:28:09Z

lightning/src/ln/channelmanager.rs

@@ -3454,7 +3530,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		let ret_err = match res {
 			Ok(Some((update_fee, commitment_signed, monitor_update))) => {
 				if let Err(e) = self.chain_monitor.update_channel(chan.get_funding_txo().unwrap(), monitor_update) {
-					let (res, drop) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, false, true, Vec::new(), Vec::new(), Vec::new(), chan_id);
+					let (res, drop) = handle_monitor_err!(self, e, short_to_id, chan, RAACommitmentOrder::CommitmentFirst, chan_id, commitment_update_only);


200 lines down, sorry -- I'm wondering if there's a possibility we'll generate Payment*Failed events with the inbound_scid_alias instead of the real short_channel_id

Hmm, can you give a specific line number in reference to the top commit of the PR? line ~3730 is just short_channel_id: None?

Oops, I meant to refer to the other two places we generate PaymentPathFailed. In either case, is it possible that our own channel failed and the alias will be used as the scid? If so, those docs could use updating as well

Hmmm, dunno how that'd happen, we get the SCID from onion_utils::process_onion_failure which pulls from the route. I suppose its possible someone used an alias that is also one of our channels, if that's what you're asking. Or are you just saying it'd be bad if it were an alias and we'd punish the wrong channel if someone picks an alias that is also a real channel (but not the one we used)? I beileve we check that the channel's source pubkey matches the one from the route, so hopefully that is also impossible.

lightning/src/ln/channelmanager.rs

lightning/src/ln/priv_short_conf_tests.rs

lightning/src/ln/msgs.rs

valentinewallace · 2022-03-03T01:32:51Z

lightning/src/ln/channel.rs

+	// on-chain transaction is ours.
+	// We only bother storing the most recent SCID alias at any time, though our counterparty has
+	// to store all of them.
+	latest_inbound_scid_alias: Option<u64>,


I think it'd be more readable/less error-prone if we renamed inbound_scid_alias to route_hint_scid_alias if that's the only place we use it. Then the other could just be scid_alias

Hmm, that isn't the only place we use them post-0conf - they're general aliases, and someone could totally use them to route over a channel prior to confirmation.

I thought about this a bit more and I still kinda like inbound as the terminology here - it matches all the ways I'd think about it (its a value we received from our counterparty, and its also the value used for payments routed inbound towards us). I still kinda want to avoid terming it "route hint" cause users may use it outside of explicit invoice contexts (eg by handing someone a last-hop explicitly via some other protocol) and "route hint" sounds very invoice-y, even though the alternative uses I can see immediately are all similar concepts.

I guess I'm not quite sure why the inbound/outbound split is confusing - it seems to line up with all the things I can think of.

Sg, I guess not that it's confusing per se but I thought the rename would make it more obvious that the correct alias is being used in a given context. Fine to let it rest

valentinewallace · 2022-03-03T01:43:43Z

lightning/src/ln/channel.rs

+				// i.e. can be used for inbound payments and provided in invoices, but is not used
+				// when routing outbound payments.
+				self.latest_inbound_scid_alias = Some(scid_alias);
+			}


Given this spec requirement: MUST set alias to a value not related to the real short_channel_id. should we return an error in the else case here?

Hmm, probably not worth closing the channel for?

lightning/src/ln/channelmanager.rs

lightning/src/ln/channel.rs

lightning/src/ln/channelmanager.rs

valentinewallace · 2022-03-03T02:36:49Z

lightning/src/ln/channel.rs

@@ -6202,6 +6226,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			workaround_lnd_bug_4006: None,

 			latest_inbound_scid_alias,
+			// Later in the ChannelManager deserialization phase we scan for channels and assign scid aliases if its missing


Would it be feasible to add coverage for this deserialization phase, maybe in an existing test?

Hmmm, we don't currently do anything related to checking serialization from old versions, its definitely something we should do, but seems like a substantial project. cc #1089.

lightning/src/ln/functional_tests.rs

lightning/src/ln/channelmanager.rs

This makes tests slightly more realistic by delivering `channel_update`s to `ChannelManager`s, ensuring we have forwarding data stored locally for all channels, including public ones.

TheBlueMatt · 2022-03-08T19:17:06Z

Addressed feedback, had to push a new cleanup commit at the start to deliver channel_updates to ChannelManagers in test, which allowed the invoice tests to include route hints.

valentinewallace · 2022-03-08T19:29:42Z

@TheBlueMatt one comment here #1311 (comment)

jkczyz

Took another pass though. Remaining comments are fairly minor. Happy to see this go through and take a look at the next step.

jkczyz · 2022-03-08T21:40:46Z

lightning/src/ln/functional_test_utils.rs

+pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::FundingLocked, Transaction) {
+	let mut no_announce_cfg = test_default_channel_config();
+	no_announce_cfg.channel_options.announced_channel = false;


Getting some deja vu from another PR. Is there a way to reuse some existing utility code such that the code that follows these lines isn't needed?

Hmm, all the existing utilities end up either calling create_channel itself or expecting to exchange AnnouncementSignatures at the end. I could rejigger the utilities above to separate those bits out, if you prefer? I don't really have a strong feeling between that vs having some code duplication in utilities right around each other.

Don't feel too strongly. I think I already asked another contributor to do the same in another PR, heh.

Hmm, on second thought I'm kinda struggling to see how to do this - the top 7 lines could be split out into three lines and four lines, but that doesn't seem so useful. Then there's the funding generation, which almost can be combined, except that we rely on exchanging the funding_locked messages in order so that we only get one channel_update out at a time - we could swap for the create_chan_between_nodes_with_value_confirm_first helper, but then we're stick peeling apart multiple pending messages which takes more LoC than we save :(.

No problem with passing on this. My concern was more with code repetition resulting in needing to modify two places rather than reducing lines of code.

Yea, I agree, its unclear how to resolve it in this case, though, without digging through a number of other tests and changing their util method calls, which is yet more diff in this PR :/

lightning/src/ln/channelmanager.rs

jkczyz · 2022-03-08T22:04:58Z

lightning/src/ln/channelmanager.rs

+	/// Gets the SCID which should be used to identify this channel for inbound payments. This
+	/// should be used for providing invoice hints or in any other context where our counterparty
+	/// will forward a payment to us.
+	pub fn get_inbound_payment_scid(&self) -> Option<u64> {


Consider dropping the get_ prefix here, too.

Personally I'm kinda a fan of the get_ prefix, they communicate the functions behavior (really, that it doesn't have side-effects) in a useful way. Its a bit less rust-idiomatic, but its a bit more java-idiomatic, and I dunno about swift/JS, so I'm not sure that's a strong reason.

I guess my counter would be naming something that doesn't start with a verb indicates that it is a property.

In some idioms yes, but in Java isn't get_ expected, even for a property-fetcher?

Yeah, agreed. That is idiomatic for Java as I understand. Suppose we could always tag these somehow for the bindings generator, but no need to worry about this now.

lightning/src/ln/channelmanager.rs

valentinewallace

This looks good to me, done several passes now. Happy to start looking onto to part 2!

`handle_monitor_err!()` has a number of different forms depending on which messages and actions were outstanding when the monitor updating first failed. Instead of matching by argument count, its much more readable to put an explicit string in the arguments to make it easy to scan for the called form.

New `funding_locked` messages can include SCID aliases which our counterparty will recognize as "ours" for the purposes of relaying transactions to us. This avoids telling the world about our on-chain transactions every time we want to receive a payment, and will allow for receiving payments before the funding transaction appears on-chain. Here we store the new SCID aliases and use them in invoices instead of he "standard" SCIDs.

This avoids needing to update channel closure code in many places as we add multiple SCIDs for each channel and have to track them.

This creates an SCID alias for all of our outbound channels, which we send to our counterparties as a part of the `funding_locked` message and then recognize in any HTLC forwarding instructions. Note that we generate an SCID alias for all channels, including already open ones, even though we currently have no way of communicating to our peers the SCID alias for already-open channels.

As a part of adding SCID aliases to channels, we now have to accept otherwise-redundant funding_locked messages which serve only to update the SCID alias. Previously, we'd failt he channel as such an update used to be bogus.

TheBlueMatt · 2022-03-09T19:15:03Z

Squashed the fixup commits down.

jkczyz · 2022-03-09T20:16:10Z

lightning/src/ln/functional_test_utils.rs

+pub fn create_unannounced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::FundingLocked, Transaction) {
+	let mut no_announce_cfg = test_default_channel_config();
+	no_announce_cfg.channel_options.announced_channel = false;


No problem with passing on this. My concern was more with code repetition resulting in needing to modify two places rather than reducing lines of code.

jkczyz · 2022-03-09T20:18:26Z

lightning/src/ln/channelmanager.rs

+	/// Gets the SCID which should be used to identify this channel for inbound payments. This
+	/// should be used for providing invoice hints or in any other context where our counterparty
+	/// will forward a payment to us.
+	pub fn get_inbound_payment_scid(&self) -> Option<u64> {


I guess my counter would be naming something that doesn't start with a verb indicates that it is a property.

TheBlueMatt · 2022-03-09T22:55:05Z

Fuzz CI failure is an upstream regression, will fix separately. Kicked CI to hopefully make it pass here.

ariard · 2022-03-09T23:00:31Z

Good reviewing again quickly rn and starting to look on PR 2.

ariard

ACK e4486fe

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch 3 times, most recently from 9778520 to a9125e3 Compare February 16, 2022 02:50

jkczyz self-requested a review February 16, 2022 04:13

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch 2 times, most recently from 4c5453a to b4fc67c Compare February 16, 2022 05:03

TheBlueMatt added this to the 0.0.106 milestone Feb 16, 2022

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch from 5bab80f to 3e4494d Compare February 17, 2022 17:37

jkczyz reviewed Feb 17, 2022

View reviewed changes

TheBlueMatt mentioned this pull request Feb 17, 2022

Fix what bolt2_open_channel_sending_node_checks_part1 tests #1317

Merged

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch 4 times, most recently from 0ffca39 to 9c028f9 Compare February 18, 2022 18:54

TheBlueMatt added the Seeking Code Review label Feb 18, 2022

valentinewallace self-requested a review March 1, 2022 20:54

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch from 9c028f9 to 7d7d477 Compare March 2, 2022 18:53

valentinewallace reviewed Mar 2, 2022

View reviewed changes

valentinewallace reviewed Mar 3, 2022

View reviewed changes

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch from 7d7d477 to 55cccb5 Compare March 4, 2022 21:25

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch from c64afdc to f00ea15 Compare March 8, 2022 19:12

TheBlueMatt added 3 commits March 8, 2022 19:16

Give ChannelManagers channel_udpates for pub chans in test

a9c4e70

This makes tests slightly more realistic by delivering `channel_update`s to `ChannelManager`s, ensuring we have forwarding data stored locally for all channels, including public ones.

Add a new functional test utility to open an unannounced channel

3fe76e6

Move private channel and short-conf tests to new module

09f8aba

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch from f00ea15 to 21562c0 Compare March 8, 2022 19:16

TheBlueMatt mentioned this pull request Mar 8, 2022

Implement the SCIDAlias Channel Type and provide SCID Privacy #1351

Merged

jkczyz reviewed Mar 8, 2022

View reviewed changes

TheBlueMatt removed the Seeking Code Review label Mar 8, 2022

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch 2 times, most recently from bee9478 to 98c8ca5 Compare March 9, 2022 06:28

valentinewallace previously approved these changes Mar 9, 2022

View reviewed changes

TheBlueMatt added 6 commits March 9, 2022 19:14

Add support for deserializing the new SCID alias in funding_locked

f54ebf7

Handle short_to_id state updates on channel closure via macros

10be993

This avoids needing to update channel closure code in many places as we add multiple SCIDs for each channel and have to track them.

Support receiving multiple funding_locked messages

e4486fe

As a part of adding SCID aliases to channels, we now have to accept otherwise-redundant funding_locked messages which serve only to update the SCID alias. Previously, we'd failt he channel as such an update used to be bogus.

TheBlueMatt dismissed valentinewallace’s stale review via e4486fe March 9, 2022 19:15

TheBlueMatt force-pushed the 2022-02-0conf-part-1 branch from 98c8ca5 to e4486fe Compare March 9, 2022 19:15

jkczyz approved these changes Mar 9, 2022

View reviewed changes

ariard reviewed Mar 9, 2022

View reviewed changes

valentinewallace approved these changes Mar 10, 2022

View reviewed changes

TheBlueMatt merged commit b1cd5a7 into lightningdevkit:main Mar 10, 2022

jkczyz mentioned this pull request Apr 1, 2022

Cut 0.0.106 #1397

Merged

benthecarman mentioned this pull request Jul 18, 2022

scid alias stuff BitcoinDevShop/pln#29

Closed

Support for SCID Aliases #1311

Support for SCID Aliases #1311

Conversation

TheBlueMatt commented Feb 16, 2022 • edited Loading

codecov-commenter commented Feb 16, 2022 • edited Loading

Codecov Report

jkczyz commented Feb 16, 2022

TheBlueMatt commented Feb 16, 2022

jkczyz commented Feb 16, 2022

TheBlueMatt commented Feb 16, 2022

jkczyz commented Feb 16, 2022

TheBlueMatt commented Feb 16, 2022

TheBlueMatt commented Feb 16, 2022

jkczyz commented Feb 17, 2022

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

TheBlueMatt Feb 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Mar 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace Mar 7, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Mar 8, 2022

valentinewallace commented Mar 8, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace left a comment

Choose a reason for hiding this comment

TheBlueMatt commented Mar 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Mar 9, 2022

ariard commented Mar 9, 2022

ariard left a comment

Choose a reason for hiding this comment

TheBlueMatt commented Feb 16, 2022 •

edited

Loading

codecov-commenter commented Feb 16, 2022 •

edited

Loading

TheBlueMatt Feb 17, 2022 •

edited

Loading

valentinewallace Mar 7, 2022 •

edited

Loading

valentinewallace commented Mar 8, 2022 •

edited

Loading