Lightning block sync and sample block sources #763

jkczyz · 2020-12-01T06:18:32Z

Adds a lightning-block-sync crate consisting of the following components:

MicroSPVClient capable of syncing chain listeners from a variety of block sources, including fork detection
BlockSource trait for fetching blocks and headers
Sample REST and RPC HTTP clients implementing the BlockSource trait
Poll trait for polling one or more BlockSource implementations for new block data
Chain pollers implementing the Poll trait for both one and multiple blocks sources.

This PR largely builds off #614 by accounting for recent refactoring in #649. It refactors much of that code and adds unit testing.

Looking for feedback from reviewers on the overall API and advice on breaking the code into smaller PRs for ease of review. Some code from #614 either remains untouched and/or may be dropped if not desirable to achieve an MVP.

This adds a new subcrate `lightning-block-sync` which is designed to make it easier to get up-and-running by removing the effort of building an SPV client and fetching the chain. Instead of building a P2P client (and all the address management that entails), this focuses on building a trivial SPV client which can fetch from several instances of an abstract BlockSource. Then, we provide two example BlockSource implementations that can fetch from Bitcoin Core's RPC interface and Bitcoin Core's REST interface. The code here is taken with heavy modifications from rust-lightning-bitcoinrpc.

Use http::uri::Uri instead of custom parsing. This includes the following differences in behavior: - Reject URI with malformed authority - Keep brackets around IP literals - Use default port (80 or 443) when the port is invalid - Use "/" for an empty path

Encapsulate URI to HTTP endpoint logic into an abstraction, exposing host, port, and path for use in creating HTTP requests. Implements std::net::ToSocketAddrs for use with std::net::TcpStream. Adds an error type for handling invalid URIs.

TheBlueMatt

Left a few comments that may be useful for cleanup-direction.

TheBlueMatt · 2020-12-01T17:17:07Z

lightning-block-sync/src/http_endpoint.rs

@@ -0,0 +1,178 @@
+use http;


Hmmm...is it really worth taking the dependency just for URI parsing? Somehow I was figuring it would alleviate the need for all the custom parsing garbage in read_http_resp but I'm not sure that we can't parse a URI ourselves.

I suppose we should define when it would be appropriate to take on a dependency. I can see an argument where the lightning crate should take on very few. And as we move outward to crates like lightning-block-sync we may want to allow for some others, possibly only in sample implementations they provide like the HTTP clients.

My argument for using this particular crate is that it is fairly lightweight and lets us focus on domain-specific code. I'd go further to say that string parsing may be an ideal example of when to take on a dependency. We are already doing so for parsing JSON, and I'd agree we should also do so for parsing HTTP responses in read_http_resp.

And as we move outward to crates like lightning-block-sync we may want to allow for some others, possibly only in sample implementations they provide like the HTTP clients.

Totally agreed here.

My argument for using this particular crate is that it is fairly lightweight and lets us focus on domain-specific code.

Yea, if it were more lightweight I may agree, but I think that's the wrong metric - I think the right one would be something that captures the differing costs - how many LoC and of what complexity would it take to write our own, and how many extra dependencies are involved (here its, I believe, three crates just to take http, but it may be more). For JSON, the dependency tree is steep, but JSON is also rather difficult to parse correctly, and easy to screw up. For URI parsing, we're just looking for some separators and returning a few parts, it should be rather difficult to screw that up. As for read_http_resp, I'd love to replace that with a "real" HTTP crate, but last I heard all the options were downright terrible (https://medium.com/@shnatsel/smoke-testing-rust-http-clients-b8f2ee5db4e6 has been updated to indicate its rather out-of-date, but I haven't done any research on what the latest state is).

Yea, if it were more lightweight I may agree, but I think that's the wrong metric - I think the right one would be something that captures the differing costs - how many LoC and of what complexity would it take to write our own, and how many extra dependencies are involved (here its, I believe, three crates just to take http, but it may be more). For JSON, the dependency tree is steep, but JSON is also rather difficult to parse correctly, and easy to screw up. For URI parsing, we're just looking for some separators and returning a few parts, it should be rather difficult to screw that up. As for read_http_resp, I'd love to replace that with a "real" HTTP crate, but last I heard all the options were downright terrible (https://medium.com/@shnatsel/smoke-testing-rust-http-clients-b8f2ee5db4e6 has been updated to indicate its rather out-of-date, but I haven't done any research on what the latest state is).

Yeah, it takes on three dependencies:

├── http v0.2.1 │ ├── bytes v0.5.5 │ ├── fnv v1.0.7 │ └── itoa v0.4.6

For URI parsing, the implementation that I replaced was not compatible with Rust's std::net::ToSocketAddrs for some cases. So there's an argument in how much time we want to invest in making sure the implementation is robust and well tested vs grabbing something off the shelf.

As for read_http_resp, https://crates.io/crates/chunked_transfer ! Has zero dependencies, too.

Nice find! I'll take a stab at using it.

This one seems more suitable for async stream processing: https://docs.rs/uhttp_chunked_bytes/0.5.0/uhttp_chunked_bytes/ - it can defer when data is not yet available.

Turns out uhttp_chunked_bytes requires an Iterator over the body, which is problematic because iterators cannot be async. We'd have to read the entire stream into memory (assuming the server disconnects) then pass an iterator over the read buffer to the library. Let me know if you had something else in mind.

The same approach can be used with chunked_transfer, which has an easier interface to work with, IMHO. Need to add some DoS protection, but I think this should be doable using AsyncReadExt::take.

You are right about uhttp_chunked_bytes - I misread the code - it doesn't seem like it can defer in the middle of a read.

About reading the body - isn't the issue that with chunked encoding you don't know how long the body is, so you can't read it fully without doing the decoding as you go along?

Right, the approach only works if the server closes the stream, as then the client would get an EOF. Not sure if this is a safe assumption though. Otherwise, the only way of knowing if you hit the end is by doing the decoding, as you said.

I ended up writing a simple adapter using futures::executor::block_on.

struct ReadAdapter<'a, R: tokio::io::AsyncRead + std::marker::Unpin>(&'a mut R); impl<'a, R: tokio::io::AsyncRead + std::marker::Unpin> std::io::Read for ReadAdapter<'a, R> { fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> { #[cfg(test)] std::thread::yield_now(); futures::executor::block_on(self.0.read(buf)) } }

It does the trick, but in order to consistently avoid test deadlocks I had to have the HttpServer drop its side of the TcpStream after writing the response.

TheBlueMatt · 2020-12-01T17:33:52Z

lightning-block-sync/src/lib.rs

+
+	/// Gets the block for a given hash. BlockSources may be headers-only, in which case they
+	/// should always return Err(BlockSourceError::Transient) here.
+	fn get_block<'a>(&'a mut self, header_hash: &'a BlockHash) -> AsyncBlockSourceResult<'a, Block>;


To make things more generic, I do wonder if we should instead replace Block here with Vec<(usize, Transaction)> or similar.

I take it you're alluding to clients that wouldn't provide a full block? Would they be polled in the same manner?

Yes. I presume all clients can be polled in the same manner for headers. Block data I could imagine being a broad mix of potential options - I anticipate eventually you may even have headers-only sources that exist purely to give you a sanity check (eg "if the headers-only source proves I'm missing blocks, also go download blocks from http://nsa.gov cause I need the blocks more than I need privacy from the NSA").

TheBlueMatt · 2020-12-01T17:41:09Z

lightning-block-sync/src/http_clients.rs

+	}
+}
+
+async fn read_http_resp(mut socket: &TcpStream, max_resp: usize) -> std::io::Result<Vec<u8>> {


This may very well be the single most complicated bit to review. If you're looking to cut something out of the initial PR, honestly, this should be the focus. We could (a) drop either RPC or REST so at least there's only one, (b) drop tokio support, maybe reintroducing it later, (c) drop support for chunked encoding (sadly Bitcoin Core sends this, but we could just use nginx-proxied Bitcoin Core for testing), and/or (d) drop HTTP stuff entirely. Of course that would leave us without any clients, but there's a few simpler options - Bitcoin P2P protocol using rust-bitcoin is more trivial than you think (https://git.bitcoin.ninja/?p=dnsseed-rust;a=blob;f=src/peer.rs;h=ff0d502191dd5fa3b7eb00f7439b805c2631691a;hb=refs/heads/master is an old pre-async one that stands up a connection, then polling for new blocks/headers is as easy as sending a request and waiting for a response), but maybe its as much work as this sans-chunked.

Yeah, I tend to agree thought didn't get around into investigating this fully. Thanks for pointing out some of the alternatives. Will spend a little time looking into this.

in case this does stay with HTTP, why not use one of the HTTP client crates that are already out there?

This is definitely a consideration as per #763 (comment). From offline discussions, the URI parsing can be removed entirely by simply having the user of HttpEndpoint pass the components parts as parameters instead of the URI string. I took a stab at this yesterday with good results. But I still plan on investigating whether there is a suitable, off-the-shelf replacement for read_http_resp.

Sorry, I didn't read Matt's comment carefully enough.

Maybe could steal the hyper response reader - https://github.com/hyperium/hyper/blob/master/src/proto/h1/decode.rs

TheBlueMatt · 2020-12-01T18:32:31Z

lightning-block-sync/src/lib.rs

+			} }
+		}
+
+		match self.chain_poller.poll_chain_tip(self.chain_tip.1).await {


I'm a little confused by the distinction between chain_poller and the block sources - given many block sources almost certainly will be a part of what we expect to poll for the current best chain, it seems like they shouldn't be separated.

Still, I wonder if we can't move a bunch of the source logic into the Poll trait - instead of multiplexing between sources here, make the Poll trait (maybe renamed to Multiplexer or something) implement BlockSource given a list of primary and backup BlockSources. This may also be an opportunity to simplify the initial PR, since, assuming its doable, we could implement MicroSPVClient against a single known block source and not have to do a lot of the tracking we do here, just implementing the chain walking.

I'm a little confused by the distinction between chain_poller and the block sources - given many block sources almost certainly will be a part of what we expect to poll for the current best chain, it seems like they shouldn't be separated.

The chain_poller may poll more than one BlockSource for a new chain tip. Along with the block header, it returns a block source that yielded that header. This is important later when fetching the block. We don't want to query a block source that returned a worse tip for instance. Rather, we'd prefer to query one that is known to have the block and fallback to a backup source if needed.

The advantage that I see in this approach is that implementations of Poll and BlockSource can be essentially stateless, leaving any more complicated logic to MicroSPVClient instead of to potential implementors of those traits.

Still, I wonder if we can't move a bunch of the source logic into the Poll trait - instead of multiplexing between sources here, make the Poll trait (maybe renamed to Multiplexer or something) implement BlockSource given a list of primary and backup BlockSources. This may also be an opportunity to simplify the initial PR, since, assuming its doable, we could implement MicroSPVClient against a single known block source and not have to do a lot of the tracking we do here, just implementing the chain walking.

The primary issue that I see with pushing more logic into a BlockSource/Poll implementation is that it may complicate the interaction between it and MicroSPVClient (e.g., maintaining the header cache).

Other than the header cache, I believe all the tracking has been removed. I left in backup sources to keep the simple_block_connect test working. But there isn't much logic other than looping over them. Let me know if I misunderstood you about tracking. Or perhaps let me know what you think could be simplified.

Right, so we chatted about this a bit offline, but I'm still a bit unsure about the separation here. I went back and reminded myself of how it was done in 614, but I think maybe we want to expand, or at least have more flexibility, around disabling block sources. Right now, this separation (keeping state in poller to figure out when to stop polling a block source) feels very tied to the idea that we'd only ever decide to disable a poller based on its response to a get_best_block() call. In the future, we may decide that a block source providing us garbage blocks is also ground for disabling it entirely, at which point this abstraction doesn't make sense anymore.

I see that putting all of the multi-source logic behind a single BlockSource multiplexer may take a bit more complexity, but I still think it wouldn't be as bad as you're thinking. It probably even makes sense to replace the recent headers cache with a simple limited-to-100k-entries HashMap or so - its only 100KB and covers a week of headers, then you don't even have to consider the complexity of clearing it when things are synced.

The larger question is around where responsibility lies. I prefer to keep the responsibilities narrow as a means to separate concerns. So in this example, a BlockSource is only concerned with retrieving chain data. And implementations of Poll are only concerned with how to query block sources.

To support the use case that you mentioned, the Poll trait can be expanded with a disable_source function, which still falls under the concern of "how to query block sources". Then MicroSPVClient could signal to disable a block source if it provided garbage blocks. Such decisions may not be possible from within a BlockSource alone but rather may require the additional context provided by MicroSPVClient (e.g., disabling a source that is on a different chain).

Thought about this some more after our offline discussion. I do see value in having a struct like BlockSoureMultiplexer in so far as it would allow users to customize policies around retrying, using backup sources, and generally choosing a source as it sees fit. However, such a multiplexer could accomplish this by implementing both the BlockSource and Poll traits. Returning itself from Poll::poll_chain_tip is effectively how it would multiplex get_block. It could even decided to return a single source from poll_chain_tip in cases where that is best.

As mentioned above, there is still a need to signal back somehow to say some source returned bad data. This is because there are two checks that MicroSPVClient performs that either require its context or it should be relied upon performing:

found a different chain during fork detection

sanity checking block data received from a source

The former can only be determined in the context of fork detection. The latter presumably should be handled by MicroSPVClient rather than relying on a BlockSource to implement this.

Agreed that the caching logic can be simplified which would clean up the Poll interface a bit. Backup querying would be removed since the multiplexer would handle it. Otherwise, I'm not sure if there is much else that can be removed from MicroSPVClient. Let me know what you think and if I missed something.

As for your two checks, can't the former be detected in some other way? Basically just have the block source know about the chain's genesis or even pass to the block source in calls from the spv client. The second is obviously more nuanced - I'd generally think the block source could handle it, but you end up easily double-checking the data there, which isn't ideal. One design could be to make some container which holds the transaction list and does the checking there - the neat part there is it may line up with returning a list of transactions anyway, by allowing us to have a constructor that takes a full block (and does the checks) or one that takes a pre-filtered list of transactions.

Alright, I managed to massage this into a cleaner design now, IMHO.

Poll's interface has been expanded such that it is now a pure adapter of BlockSource, meaning neither it nor MicroSPVClient expose BlockSource as part of their interface any longer. Instead, functionality needed by MicroSPVClient is defined by Poll.

ChainPoller implements Poll for a single BlockSource, while Poll's interface is defined in terms of ValidatedBlockHeader and ValidatedBlock. These essentially encapsulate the checks that MicroSPVClient had been performing previously. Therefore, all checks now occur directly within ChainPoller.

More elaborate implementations of Poll can and must be defined in terms of ChainPoller. For example, ChainMultiplexer is defined in this manner and no longer needs to implement BlockSource.

Regarding the nuances around transaction lists, I think this can be part of a later refactor after we get a sample node working with a full-block source. Something like what you imagine should work well with the current design with minimal refactoring.

Thoughts?

Looks good to me.

Add a ValidatedBlockHeader wrapper around BlockHeaderData such that the former can only be produced after a successful stateless_check_header on the latter. Used throughout to ensure that any BlockHeaderData returned by a BlockSource is validated in terms of proof of work and block hash.

Add look_up_previous_header and fetch_block functions and remove BlockSource from Poll's interface. Update MicroSPVClient helpers to use Poll instead of BlockSource.

Moving the check to ChainPoller::look_up_previous_header allows for errors to be detected from within a Poll implementation written in terms of ChainPoller. This requires each ChainPoller to know what Network it is on in order to perform the difficulty check.

Removes the BlockSource implementation for ChainMultiplexer as it is no longer needed.

lightning-block-sync/src/lib.rs

TheBlueMatt

New API changes looks great, but one question about the poll function specifically.

TheBlueMatt · 2020-12-28T18:10:38Z

lightning-block-sync/src/lib.rs

+
+	/// Check each source for a new best tip and update the chain listener accordingly.
+	/// Returns true if some blocks were [dis]connected, false otherwise.
+	pub async fn poll_best_tip<CL: ChainListener + Sized>(&mut self, chain_listener: &mut CL) ->


Hmm, why move away from having the MicroSPVClient hold a reference to the ChainListener? It seems like it could make it easier to screw up tracking of best tip - I believe with the new API you'd need one MicroSPVClient per ChainListener (or we'd end up connecting blocks only on one listener), but the API doesn't make that obvious (naively, I'd implement the client-side by just calling poll on all the listeners in series). May also make sense to have a add_sync_listener which calls init_sync_listener as a part of adding a listener to the client.

Hmm, why move away from having the MicroSPVClient hold a reference to the ChainListener? It seems like it could make it easier to screw up tracking of best tip - I believe with the new API you'd need one MicroSPVClient per ChainListener (or we'd end up connecting blocks only on one listener), but the API doesn't make that obvious (naively, I'd implement the client-side by just calling poll on all the listeners in series).

Very good point. I was considering moving it back for that very reason and also because blocks would be needlessly re-fetched. Can't recall if I moved it to ease testing. Anyhow, I'll move it back.

May also make sense to have a add_sync_listener which calls init_sync_listener as a part of adding a listener to the client.

This would imply the client holding multiple listeners vs the user implementing ChainListener as needed. For example, having an implementation calling out to both ChannelManager and ChainMonitor. Haven't put much thought into having a more flexible interface as this, but just wanted to call that out.

This would imply the client holding multiple listeners vs the user implementing ChainListener as needed. For example, having an implementation calling out to both ChannelManager and ChainMonitor. Haven't put much thought into having a more flexible interface as this, but just wanted to call that out.

Right, yea, either could work fine. In either case it may make sense to pull init_sync_listener into the MicroSPVClient, I think, though.

Right, yea, either could work fine. In either case it may make sense to pull init_sync_listener into the MicroSPVClient, I think, though.

I tried adding a member function that accepts a block source in a similar manner as ChainPoller (i.e., <'b, B: DerefMut<Target=dyn BlockSource + 'b> + Sized + Sync + Send>. However, attempting to use this with a block source already referenced by ChainPoller would result in two mutable borrows. I thought there was a workaround for this. Any advice here?

Right, we'd need to (a) accept the old_block blockhash as an argument, (b) poll the poller for how to get to our current tip (but not the current tip, it cant change between sources) from old_block, and then (c) somehow track the sources that are done as a part of MicroSPVClient. I admit that may be a bigger change than I was envisioning.

Right, I figured that you wouldn't need to pass the block source to the new init_sync_listener - it would just make some request through the ChainPoller to get a block source/block data.

Yeah, that's easier, but it would mean possibly fetching through multiple sources if a different implementation of Poll is used.

I think we're maybe out of sync here, but no matter - in general using different sources shouldn't matter - they are all "trusted" to give us the right transaction data, presumably. As long as we can (a) iteratively add new listeners and sync them up to the same block in a safe way that makes accidentally syncing them to different blocks hard, and (b) then let the spv client run, keeping all the listeners up to date, we should be good.

Gotcha. I was referring to the comment above init_sync_listener:

https://github.com/rust-bitcoin/rust-lightning/blob/eaedc0a8a3cce989aa792daff3737b48201dec23/lightning-block-sync/src/lib.rs#L390-L394

Right, I think that comment was just a general note about how that block source could leave us un-sync-able if it drove us onto some fork and then refused to provide previous headers on the chain to the main chain. Yet another reason to move it into the rest of the spvclient logic - we could use the disconnected-headers caching to ensure that no single block source could do that to us.

TheBlueMatt and others added 14 commits October 19, 2020 15:11

f - Use rust-bitcoin 0.24

55af3da

f - Changes needed for rebase on lightningdevkit#649

71b49e3

Reorder imports in lightning-block-sync

f9ff8f9

f - Handle odd-length hex strings in hex_to_vec

265b8a9

Add unit tests for utils::hex_to_uint256

6d79c6f

Add unit tests for utils::hex_to_vec

a7ac3c3

Use bitcoin::hashes::hex::FromHex in utils

4d5ca87

Replace hex_to_vec with Vec::<u8>::from_hex

d1af694

Return Result from hex_to_uint256

9b88bb1

Refactor split_uri into HttpEndpoint

801a4a5

Encapsulate URI to HTTP endpoint logic into an abstraction, exposing host, port, and path for use in creating HTTP requests. Implements std::net::ToSocketAddrs for use with std::net::TcpStream. Adds an error type for handling invalid URIs.

f - Make http_endpoint pub

748d34e

Refactor HTTP client into separate abstraction

75ae003

TheBlueMatt mentioned this pull request Dec 1, 2020

Add subcrate which impls a simple SPV client from Bitcoin Core RPC #614

Closed

TheBlueMatt reviewed Dec 1, 2020

View reviewed changes

jkczyz added 14 commits December 4, 2020 17:02

f - Fix tokio support in HttpClient

359ead8

Use type conversion for HTTP response parsing

97e486a

f - Fix previousblockhash for JSON genesis block

808cec3

f - Check that JSON response contains an object

eb3ad66

f - Handle invalid JSON cases separately

110c07d

Add tests for HttpClient::connect

89b72f0

f - Fix tokio support in HttpClient tests

8881435

Return std::io::Result from read_http_resp

82a4dc7

f - Use ErrorKind::InvalidData instead of Other

927ee26

Add tests for REST client

3cceaf4

Add tests for RPC client

d3511b3

Add tests converting from Vec<u8> into responses

4ce724e

Add tests converting into Block from responses

eb45430

Add tests converting into BlockHash from response

2fdd593

jkczyz added 3 commits December 14, 2020 16:26

Proof of concept for BlockSource multiplexing

6ab4b24

Reuse ChainPoller in ChainMultiplexer

7af1dbe

Remove MultipleChainPoller

ade6ace

jkczyz force-pushed the 2020-09-block-sync branch from 7b1c96c to ade6ace Compare December 15, 2020 00:27

jkczyz added 11 commits December 15, 2020 12:39

Return ChainTip from MicroSPVClient::poll_best_tip

dc04ee2

Expand Poll trait to be an adapter for BlockSource

a6beac8

Add look_up_previous_header and fetch_block functions and remove BlockSource from Poll's interface. Update MicroSPVClient helpers to use Poll instead of BlockSource.

Move check for different chain to ChainPoller

ffbdaa5

Validate blocks directly in ChainPoller

6d54ffb

Implement ChainMultiplexer in terms of ChainPoller

4a0a9b1

Removes the BlockSource implementation for ChainMultiplexer as it is no longer needed.

Remove unnecessary match expressions

9d36b29

Refactor sync_chain_monitor macro into a method

b0d4b20

Add ChainNotifier abstraction for functions

2774200

Add ChainNotifier tests for more error cases

24976bc

arik-so reviewed Dec 22, 2020

View reviewed changes

lightning-block-sync/src/lib.rs Outdated Show resolved Hide resolved

lightning-block-sync/src/lib.rs Show resolved Hide resolved

lightning-block-sync/src/lib.rs Show resolved Hide resolved

devrandom mentioned this pull request Dec 27, 2020

Meeting Minutes #717

Closed

TheBlueMatt reviewed Dec 28, 2020

View reviewed changes

jkczyz added 6 commits December 28, 2020 14:52

Remove Sized requirement from ChainListener params

7546fd6

Parameterize MicroSPVClient by ChainListener

74e8d8d

Rename best_chain_tip to best_known_chain_tip

4b83f18

Add MicroSPVClient tests

b652f9e

Rename MicroSPVClient::init to new

87a9b99

Clean up MicroSPVClient documentation

eaedc0a

jkczyz force-pushed the 2020-09-block-sync branch from 758496d to eaedc0a Compare January 11, 2021 18:56

jkczyz mentioned this pull request Jan 12, 2021

HTTP-based block source clients #774

Merged

jkczyz mentioned this pull request Feb 5, 2021

SPV client utility for syncing a lightning node #791

Merged

Remove dyn from ChainPoller

c3fdc74

jkczyz closed this Mar 4, 2021

jkczyz mentioned this pull request Oct 31, 2022

Initial chain access implementation lightningdevkit/ldk-node#9

Closed

Lightning block sync and sample block sources #763

Lightning block sync and sample block sources #763

Conversation

jkczyz commented Dec 1, 2020

TheBlueMatt 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

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

jkczyz Dec 10, 2020 • 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

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz Dec 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt Jan 11, 2021 • 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

jkczyz Dec 10, 2020 •

edited

Loading

jkczyz Dec 28, 2020 •

edited

Loading

TheBlueMatt Jan 11, 2021 •

edited

Loading