HTTP-based block source clients #774

jkczyz · 2021-01-12T16:47:38Z

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

BlockSource trait for fetching blocks and headers
HTTP-based REST and RPC clients implementing the BlockSource trait

This PR is the first half of #763.

codecov · 2021-01-13T05:28:08Z

Codecov Report

Merging #774 (52ff159) into main (e4b516d) will decrease coverage by 0.09%.
The diff coverage is 88.53%.

@@            Coverage Diff             @@
##             main     #774      +/-   ##
==========================================
- Coverage   90.80%   90.71%   -0.10%     
==========================================
  Files          38       44       +6     
  Lines       23168    24066     +898     
==========================================
+ Hits        21038    21831     +793     
- Misses       2130     2235     +105

Impacted Files	Coverage Δ
lightning-block-sync/src/lib.rs	`0.00% <0.00%> (ø)`
lightning-block-sync/src/rest.rs	`65.45% <65.45%> (ø)`
lightning-block-sync/src/rpc.rs	`78.37% <78.37%> (ø)`
lightning-block-sync/src/convert.rs	`92.80% <92.80%> (ø)`
lightning-block-sync/src/http.rs	`95.03% <95.03%> (ø)`
lightning-block-sync/src/utils.rs	`100.00% <100.00%> (ø)`
lightning/src/ln/functional_tests.rs	`96.99% <0.00%> (-0.04%)`	⬇️
... and 3 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 e4b516d...85fdfaa. Read the comment docs.

jkczyz · 2021-01-13T17:44:59Z

FYI, the latest push split http_clients into http, rest, rpc, and convert. It also moved the contents of http_endpoint into http. Hope this separation makes the code easier to review.

Not sure what's up with Clippy:

error: error reading Clippy's configuration file `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/pin-project-lite-0.2.4/.clippy.toml`: unknown field `msrv`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `third-party` at line 1 column 1

lightning-block-sync/src/rpc.rs

TheBlueMatt · 2021-01-13T18:30:24Z

lightning-block-sync/src/http.rs

+		let request = format!(
+			"GET {} HTTP/1.1\r\n\
+			 Host: {}\r\n\
+			 Connection: keep-alive\r\n\


Connection: keep-alive usually will only hold the connection for N requests (order 100), so we need some kind of logic to reconnect if write_request() fails once.

I had trouble getting a test to return an appropriate error from write_request when closing the stream. I was able to get an unexpected EOF from read_response though. I made the error check around both instead, reconnecting and retrying when detected.

BTW, the REST and RPC interfaces were creating new connections for each request. I updated them to maintain the connection instead.

lightning-block-sync/src/http.rs

lightning-block-sync/src/convert.rs

lightning-block-sync/src/lib.rs

valentinewallace · 2021-01-12T20:53:30Z

lightning-block-sync/src/lib.rs

+	/// Returns the hash of the best block and, optionally, its height. The height is passed to
+	/// `get_header` to allow a more efficient lookup on some block sources.


Second sentence may be able to be clearer. Who's passing the height? RL?
Is get_header's docs saying that if get_header returns a Transient error, then RL will try to get the height from get_best_block and use it on get_header (even though get_header seemingly would take any height, not just the best one)?

The forthcoming MicroSPVClient would pass the height from either the result of calling get_best_block or by computing it from a BlockHeaderData.

I can write a better summary at the trait-level, but I think this raises an important point regarding the interface. Namely, in what case would a source not provide a height? If get_best_block returns a None height with the hash, how could a call to get_header on the same source return a BlockHeaderData for the hash given that it has a height field to populate?

@TheBlueMatt Should we drop Option from around the height in these two functions? Or at very least from get_best_block? I suppose we may want to call get_header without a hint if we deserialized a listener that only had the most recent block hash.

Hmm, good question! I agree its maybe dubious to claim there will be an implementation that polls the best chain tip and only gets a hash, but its still somewhat nice to have - it wouldn't be unheard of, and requiring less data for the best block polling may leave some other things (like headers over radio) more easy to implement. I'd need to dig into the MicroSPVClient, but is it not the case that height_hint will never be None unless a source returned None for the height in get_best_block (or is it the case that you can hit it if one source returns None but another source returns Some?)? That would simplify the docs somewhat and I'm not sure it generates a lot of code in the MicroSPVClient?

The multiple-source case is handled in ChainMultiplexer, which uses a set of ChainPollers (one for each block source). So the result of get_best_block is never fed to get_header of a different source. So, yeah, I think the docs can be simplified a bit with this invariant in mind.

But note that relatedly, as it stands, get_header must return BlockHeaderData containing a height. If it were changed to be Option, this would prevent the fork detection logic from functioning. And more importantly, a height is eventually needed to pass to block_connected and block_disconnected.

IIUC, then, a headers-only source must return the height along with the header. Were you suggesting that it may be easier to implement such a source if this were not the case? We could conceivably make the height field an Option, but it would complicate the code quite a bit. Trying different source would be predicated on this field being None rather than on whether there was an error. I suppose that could be abstracted away to ChainMultiplexer if needed. Just want to make sure I understand the use case.

valentinewallace · 2021-01-13T16:31:51Z

lightning-block-sync/Cargo.toml

 [dependencies]
 bitcoin = "0.24"
 lightning = { version = "0.0.12", path = "../lightning" }
+tokio = { version = ">=0.2.12", features = [ "tcp", "io-util", "dns" ], optional = true }


tokio doesn't seem like the most stable, any reason to not just pin to a specific version?

I kept it matching lightning-net-tokio but did run into some trouble with dependency version inconsistency when needing serde later on.

@TheBlueMatt Was 0.2.12 chosen for a reason? Presumably there is some bug fix in there. Should we pin on that? I think ">=0.2.12" won't bump us up to 0.3.0 at very least, IIUC.

This matches what we use in lightning-net-tokio which is important. That said, we should just upgrade both to 1.0 and move on.

Updated tokio to 1.0. However, I was unable to get it working with lightning-net-tokio. The test would hang during connection setup.

https://github.com/rust-bitcoin/rust-lightning/blob/d529a8827bd860ecfef73059cd5097e778f63ddc/lightning-net-tokio/src/lib.rs#L626-L631

Initial investigation seemed to show it stuck waiting on tokio::select!. I tried both 1.0 and 0.3 (the public beta for 1.0) and had the same results.

Any insight?

FYI, decided to revert tokio 1.0 support since it required using Rust 1.45 which led to other issues.

It looks like a tokio bug, I filed one upstream. tokio-rs/tokio#3474 For now I guess lets use 0.2.

Note that the failure in threaded mode is separate - it seems tokio also no longer closes connections when you call close so the other thread doesn't learn the connection is gone.

Nevermind, they set me straight quick.

lightning-block-sync/src/rest.rs

valentinewallace · 2021-01-13T20:47:41Z

lightning-block-sync/src/convert.rs

+	}
+}
+
+/// Response data from `getblockheader` RPC and `headers` REST requests.


It'd be cool to have CI to spin up a bitcoind node and test these. Maybe too much for this PR though.

lightning-block-sync/src/http.rs

.github/workflows/build.yml

lightning-block-sync/src/rest.rs

valentinewallace · 2021-01-18T23:24:40Z

lightning-block-sync/src/http.rs

+	///
+	/// The request body consists of the provided JSON `content`. Returns the response body in `F`
+	/// format.
+	pub async fn post<F>(&mut self, uri: &str, host: &str, auth: &str, content: serde_json::Value) -> std::io::Result<F>


If we wanted to save some LoC, doesn't look like this is used and I don't think Core has any API that takes post requests? Edit: there's also a warning in CI output because it's unused.

If we wanted to save some LoC, doesn't look like this is used and I don't think Core has any API that takes post requests?

This is used by RpcClient::call_method as the client uses the HTTP request body to specify the RPC using JSON.

Edit: there's also a warning in CI output because it's unused.

I did see some warnings in intermediary commits but not from HEAD. Looks like this is because the methods aren't used until the implementations of BlockSource are added in a later commit.

This is used by RpcClient::call_method as the client uses the HTTP request body to specify the RPC using JSON.

Missed this!
As discussed, there still seem to be some warnings.

Fixed. They were coming up when either the rpc-client or rest-client feature was enabled but not when both were.

lightning-block-sync/src/http.rs

valentinewallace · 2021-01-19T01:09:24Z

lightning-block-sync/src/http.rs

+	}
+
+	/// Creates an endpoint using the HTTPS scheme.
+	pub fn secure_host(host: String) -> Self {


Not 100% sure but I think supporting HTTPS/TLS may need extra work and could be put off.
For example:

#[tokio::test] async fn test_https_servers() { let endpoint = HttpEndpoint::secure_host("www.facebook.com".to_string()); let mut client = HttpClient::connect(&endpoint).unwrap(); let res = client.get::<BinaryResponse>("/", "facebook.com").await.unwrap(); }

Seems to result in read_line! reading something binary that isn't what curl returns, so I think the TLS isn't being handled? Also other HTTP client implementations seem to be doing extra work for TLS (e.g. https://docs.rs/native-tls/0.2.7/native_tls/#examples).

If I'm off here, I still think it could be worth adding a test for both HTTP and HTTPS clients using google.com (as opposed to just the test HttpServer).

Not 100% sure but I think supporting HTTPS/TLS may need extra work and could be put off.

Ah, no, you're right. Thanks for testing this case! I remember running across this awhile ago but it fell off my radar. I'll just remove support for it and add a TODO. It would require refactoring the HttpClient API a bit.

If I'm off here, I still think it could be worth adding a test for both HTTP and HTTPS clients using google.com (as opposed to just the test HttpServer).

Generally, I'd prefer to make unit tests hermetic whenever possible to avoid flakiness with the external world.

Having separate tests would be useful, though. I would add a placeholder now to demonstrate that it's currently not supported, but that isn't possible with the current API taking E: ToSocketAddrs.

Generally, I'd prefer to make unit tests hermetic whenever possible to avoid flakiness with the external world.

I've come to agree with this, like we wouldn't want to require an internet connection to run our tests.

Having separate tests would be useful, though. I would add a placeholder now to demonstrate that it's currently not supported, but that isn't possible with the current API taking E: ToSocketAddrs.

Hmm should've asked in the call but I wasn't sure what this meant. Separate tests for what? and what does "placeholder" mean?

Hmm should've asked in the call but I wasn't sure what this meant. Separate tests for what? and what does "placeholder" mean?

More precisely a "placeholder test". Essentially, a test for HttpClient:connect that demonstrate that it fails when given a secure endpoint. Once HTTPS support is added, the test could be updated to check for success instead.

However, this won't work (compile) for the reason that I gave.

.github/workflows/build.yml

valentinewallace · 2021-01-20T00:59:47Z

I think I partly figured out why coverage is failing --
So the failure happens with this CI output:

 ==> Searching for coverage reports in:
    + .
--> No coverage report found.

and the reason it can't find any coverage reports is because in .github/workflows/build.yml, there's this code that generates the coverage reports:

          for file in target/debug/lightning-*; do
            [ -x "${file}" ] || continue;
            mkdir -p "target/cov/$(basename $file)";
            ./kcov-build/usr/local/bin/kcov --exclude-pattern=/.cargo,/usr/lib --verify "target/cov/$(basename $file)" "$file";
          done

which only looks /target/debug/. But for some reason, the lightning-block-sync tests put their build artifacts in /target/debug/deps (resulting in no coverage reports being generated). So presumably the superficial fix would be to run that snippet^ in the additional directory. However I don't know why those files are in this different place. Our current setup may be a bit of a hack given that: https://doc.rust-lang.org/cargo/guide/build-cache.html says that the layout of these directories is subject to change.

jkczyz · 2021-01-20T02:13:34Z

I think I partly figured out why coverage is failing --
So the failure happens with this CI output:
 ==> Searching for coverage reports in:
    + .
--> No coverage report found.
and the reason it can't find any coverage reports is because in .github/workflows/build.yml, there's this code that generates the coverage reports:
          for file in target/debug/lightning-*; do
            [ -x "${file}" ] || continue;
            mkdir -p "target/cov/$(basename $file)";
            ./kcov-build/usr/local/bin/kcov --exclude-pattern=/.cargo,/usr/lib --verify "target/cov/$(basename $file)" "$file";
          done
which only looks /target/debug/. But for some reason, the lightning-block-sync tests put their build artifacts in /target/debug/deps (resulting in no coverage reports being generated). So presumably the superficial fix would be to run that snippet^ in the additional directory. However I don't know why those files are in this different place. Our current setup may be a bit of a hack given that: https://doc.rust-lang.org/cargo/guide/build-cache.html says that the layout of these directories is subject to change.

Yeah, I think you're on to something. It may be related to moving to Rust 1.45, which Tokio 1.0 requires. I removed that commit. Let's see if it helps any.

valentinewallace

This is shaping up from my POV!

One minor thing would be to note somewhere what versions of Bitcoin Core this would work for (not crucial for this PR though).

valentinewallace · 2021-01-20T22:50:40Z

lightning-block-sync/src/http.rs

+	}
+
+	// TODO: Refactor HttpClient to support TLS.
+	#[cfg(test)]


I'm a bit in favor of just ripping all of the TLS-related out and adding all of it later when we can actually support it (e.g. the presence of Scheme could be confusing for future readers who then assume we support HTTPS).

Removed. It would probably make more sense to separate this anyhow.

valentinewallace · 2021-01-20T22:53:11Z

lightning-block-sync/src/lib.rs

+//! Both features support either blocking I/O using `std::net::TcpStream` or, with feature `tokio`,
+//! non-blocking I/O using `tokio::net::TcpStream` from inside a Tokio runtime.


As discussed offline, these docs could use an update for the pros/cons + more details of enabling tokio feature.

I may have added to the confusion during our discussion. For the non-tokio case, there is never an await on stream I/O since the stream used (std::net::TcpStream) has a synchronous interface. Therefore, stream I/O will block the thread.

For the tokio case, tokio::net::TcpStream has an asynchronous interface (i.e., it requires calling await on the returned future). Whenever steam I/O is hit, the runtime is given the chance to schedule other work if blocking would have occurred. Therefore, stream I/O will not block the thread.

So while the overall BlockSource interface is asynchronous (i.e., written in terms of async functions), HttpClient is only non-blocking when using the tokio feature. But either way a runtime is needed to drive the execution.

Let me know if that cleared anything up and if you have any suggestions for making the docs more intuitive.

lightning-block-sync/src/rpc.rs

valentinewallace · 2021-01-20T22:56:05Z

lightning-block-sync/src/http.rs

+	///
+	/// The request body consists of the provided JSON `content`. Returns the response body in `F`
+	/// format.
+	pub async fn post<F>(&mut self, uri: &str, host: &str, auth: &str, content: serde_json::Value) -> std::io::Result<F>


This is used by RpcClient::call_method as the client uses the HTTP request body to specify the RPC using JSON.

Missed this!
As discussed, there still seem to be some warnings.

valentinewallace · 2021-01-20T23:03:50Z

lightning-block-sync/src/lib.rs

+	Persistent,
+
+	/// Indicates an error that may resolve when retrying a request (e.g., unresponsive).
+	Transient,


In my testing, frequently I wanted to know what the actual error was (like EOF) and added print statements to retrieve it. So, might be slightly nicer if Transient contained the actual error. Not a big deal though.

Good point. Made this a wrapper around an underlying error similar to std::io::Error.

lightning-block-sync/src/rpc.rs

valentinewallace · 2021-01-20T23:19:39Z

lightning-block-sync/src/http.rs

+	}
+
+	/// Creates an endpoint using the HTTPS scheme.
+	pub fn secure_host(host: String) -> Self {


Generally, I'd prefer to make unit tests hermetic whenever possible to avoid flakiness with the external world.

I've come to agree with this, like we wouldn't want to require an internet connection to run our tests.

Having separate tests would be useful, though. I would add a placeholder now to demonstrate that it's currently not supported, but that isn't possible with the current API taking E: ToSocketAddrs.

Hmm should've asked in the call but I wasn't sure what this meant. Separate tests for what? and what does "placeholder" mean?

Rustc 1.45 moved the paths to test binaries, so we need to update our CI scripts to run the correct ones under kcov. The solution to this was pointed out by Val at lightningdevkit#774 (comment)

TheBlueMatt · 2021-01-27T18:34:23Z

lightning-block-sync/src/convert.rs

+	}
+}
+
+#[cfg(test)]


Can you extract some test-cases from a real bitcoin node response to the various queries used?

That's essentially what I'm doing here, primarily with the genesis block and generated programmatically (e.g. line 170) to ensure the tests are concise. I could add missing fields that are irrelevant to the conversion, but I don't think that adds much value to the tests.

Right, I meant using the full set of queries used, eg including GetHeaderResponse.

Hmm... not sure I understand. I believe that I covered all the queries. Did I miss something?

get_header:

into_block_header_from_json_response_with_valid_header

into_block_header_from_json_response_with_valid_header_array

into_block_header_from_json_response_without_previous_block_hash

get_block

into_block_from_valid_binary_response

into_block_from_json_response_with_valid_block_data

get_best_block

into_block_hash_from_json_response_without_height

into_block_hash_from_json_response_with_height

Right, I was looking for a string copied from the wire off wireshark or the like, but I suppose creating them by filling in objects manually is fine enough.

For unit tests, I prefer to keep the inputs as concise as possible yet still representative. Using raw wire data would be reasonable for an integration test, IMHO.

lightning-block-sync/src/http.rs

valentinewallace

There seems to be a (spurious?) failure on stable: https://i.imgur.com/jASWBwD.png

There may be others, didn't check. Also linting failing for some reason.

I'm good to approve after those fixes!

jkczyz · 2021-01-28T06:57:07Z

There seems to be a (spurious?) failure on stable: https://i.imgur.com/jASWBwD.png

Ugh, thought I had resolved this but apparently not. Added a timeout in 9871d40 when using block_on in the read adaptor.

There may be others, didn't check. Also linting failing for some reason.

Looks like the same problem as earlier: #774 (comment)

Found others having the same issue but need to delve into it further: rust-lang/rust-clippy#3874

jkczyz · 2021-01-28T22:39:50Z

There seems to be a (spurious?) failure on stable: https://i.imgur.com/jASWBwD.png

Ugh, thought I had resolved this but apparently not. Added a timeout in 9871d40 when using block_on in the read adaptor.

Turns out this doesn't seem to help. :( Tried implementing HttpServer with tokio instead but still experienced this issue occasionally.

@TheBlueMatt I'm only ever seeing this with read_chunked_message_body test and debugging shows it stuck in the ReadAdapter implemented using block_on. Not sure if you have any insight here. I think it's only a problem with the test setup. Not sure how much more time I want to spend debugging this, but I hate to include a flaky test.

jkczyz · 2021-01-30T05:22:30Z

@TheBlueMatt I'm only ever seeing this with read_chunked_message_body test and debugging shows it stuck in the ReadAdapter implemented using block_on. Not sure if you have any insight here. I think it's only a problem with the test setup. Not sure how much more time I want to spend debugging this, but I hate to include a flaky test.

Ended up removing the ReadAdaptor which was calling block_on in favor of reading into a buffer: 115c936. This should prevent read_chunked_message_body from occasionally hanging.

Looks like the same problem as earlier: #774 (comment)

Found others having the same issue but need to delve into it further: rust-lang/rust-clippy#3874

Fixed the linter error by rebasing on #781 since it moves stable to 1.45.2. IIUC, clippy runs over all dependencies which may have their own configs. One of them was likely incompatible with the previous stable.

jkczyz · 2021-01-31T02:22:04Z

Ended up removing the ReadAdaptor which was calling block_on in favor of reading into a buffer: 115c936. This should prevent read_chunked_message_body from occasionally hanging.

Forgot that this only works if the server closes the connection. Managed to get an upstream change that enable a workaround. Now we can read asynchronously and use chunked_transfer::Decoder to decode the chunk size as needed.

Defines an interface and related types for fetching block headers and data from a block source (e.g., Bitcoin Core). Used to keep lightning in sync with chain activity.

jkczyz · 2021-02-04T01:47:35Z

Rebased and squashed fixup commits.

TheBlueMatt

All super trivial notes.

TheBlueMatt · 2021-02-04T01:11:52Z

lightning-block-sync/Cargo.toml

+serde = { version = "1.0", features = ["derive"], optional = true }
+serde_json = { version = "1.0", optional = true }
+chunked_transfer = { version = "1.4.0", optional = true }
+futures = { version = "0.3.8" }


minor nit: I think just write 0.3? Looks like its already using 0.3.12 with this text, though, so it doesn't matter.

I think there was some dependency mismatch earlier which seemed to be fixed by specifying it this way. But it may have been my confusion. Changed to 0.3 as that works now.

TheBlueMatt · 2021-02-04T03:47:53Z

lightning-block-sync/src/lib.rs

+/// persistent errors.
+pub struct BlockSourceError {
+	kind: BlockSourceErrorKind,
+	error: Box<dyn std::error::Error + Send + Sync>,


Not a blocker, but I'm not really sure the value of this - with a dyn Error the only thing you can really do with it is string-format it, and a fixed, templated version would make mixing different block sources have type errors. I need to take another look at the next PR, however, to see how its used, and we can discuss more there.

I largely followed the pattern used by std::io::Error, which this error happens to wrap in this PR. That error also implements std::error::Error which allows examiners to follow the chain of errors. I chose not to implement that here for sake of simplicity, but leaving it as is gives us the option of adding it later.

In the follow-up, this error wraps either a string or any errors emanating from rust-bitcoin when validating block data, IIRC. Happy to discuss it more in that PR if necessary.

TheBlueMatt · 2021-02-04T03:50:58Z

lightning-block-sync/src/http.rs

+/// Endpoint for interacting with an HTTP-based API.
+#[derive(Debug)]
+pub struct HttpEndpoint {
+	host: String,


There aren't consistency requirements, so why not just make all the fields pub?

Partly it's this way as a result of the evolution of the struct. That said, not making these pub has some advantages. Once constructed it is immutable from the user's perspective. We can also vary the implementation in the future without worrying about clients. It also keeps the interface consistent given port is optional and path has a default. They would require accessor methods or special initialization to maintain the same behavior.

TheBlueMatt · 2021-02-04T04:19:45Z

lightning-block-sync/src/http.rs

+
+		// Read message body
+		let read_limit = MAX_HTTP_MESSAGE_BODY_SIZE - reader.buffer().len();
+		reader.get_mut().set_limit(read_limit as u64);


In the chunked encoding case, we're limiting to 4MB including chunked encoding overhead, which I guess isn't ideal - maybe we should just bump MAX_HTTP_MESSAGE_BODY_SIZE to 2*4MB + 32KB or something larger so that its clearly fine?

Good point. Updated as recommended.

TheBlueMatt · 2021-02-04T04:28:40Z

lightning-block-sync/src/http.rs

+								None => break,
+								Some(chunk_size) => chunk_size,
+							};
+							let mut chunk_body = vec![0; chunk_size + "\r\n".len()];


You can avoid the extra allocations by writing into content.

- let mut chunk_body = vec![0; chunk_size + "\r\n".len()]; - reader.read_exact(&mut chunk_body[..]).await?; - content.extend_from_slice(&chunk_body[..chunk_size]); + let orig_len = content.len(); + content.resize(orig_len + chunk_size + "\r\n".len(), 0); + reader.read_exact(&mut content[orig_len..]).await?; + content.resize(orig_len + chunk_size, 0);

TheBlueMatt · 2021-02-04T04:35:38Z

@valentinewallace had mentioned she's happy once the test hangs are fixed, so up to you when you want it merged @jkczyz.

TheBlueMatt

Can you squash the fixups?

Implements a simple HTTP client that can issue GET and POST requests. Used to implement REST and RPC clients, respectively. Both clients support either blocking or non-blocking I/O.

Interprets HTTP responses as either binary or JSON format, which are then converted to the appropriate data types.

jkczyz requested a review from valentinewallace January 12, 2021 16:47

jkczyz force-pushed the 2021-01-http-client branch from 7d5d75e to 5994c92 Compare January 13, 2021 05:14

jkczyz force-pushed the 2021-01-http-client branch from 5994c92 to 619f25c Compare January 13, 2021 07:13

TheBlueMatt reviewed Jan 13, 2021

View reviewed changes

valentinewallace reviewed Jan 15, 2021

View reviewed changes

jkczyz force-pushed the 2021-01-http-client branch 4 times, most recently from 5046f08 to 80db689 Compare January 16, 2021 18:31

valentinewallace reviewed Jan 18, 2021

View reviewed changes

jkczyz force-pushed the 2021-01-http-client branch 2 times, most recently from acbe5af to 208d7d6 Compare January 19, 2021 21:50

valentinewallace reviewed Jan 19, 2021

View reviewed changes

jkczyz force-pushed the 2021-01-http-client branch from 208d7d6 to 66e7cbd Compare January 20, 2021 02:11

jkczyz force-pushed the 2021-01-http-client branch 3 times, most recently from 4e092b3 to 7c8c68a Compare January 20, 2021 18:48

valentinewallace reviewed Jan 20, 2021

View reviewed changes

jkczyz force-pushed the 2021-01-http-client branch 2 times, most recently from d771419 to 8eec576 Compare January 26, 2021 05:57

jkczyz mentioned this pull request Jan 27, 2021

Update tokio to 1.0 #781

Merged

TheBlueMatt approved these changes Jan 27, 2021

View reviewed changes

valentinewallace reviewed Jan 27, 2021

View reviewed changes

jkczyz force-pushed the 2021-01-http-client branch from 8eec576 to 329d6cc Compare January 28, 2021 06:56

jkczyz force-pushed the 2021-01-http-client branch 3 times, most recently from 795b047 to 2895102 Compare January 30, 2021 03:25

jkczyz force-pushed the 2021-01-http-client branch from 2895102 to 09b3f12 Compare January 31, 2021 01:50

Add lightning-block-sync package and library

9860646

Defines an interface and related types for fetching block headers and data from a block source (e.g., Bitcoin Core). Used to keep lightning in sync with chain activity.

jkczyz force-pushed the 2021-01-http-client branch from 09b3f12 to 0c31383 Compare February 4, 2021 01:46

TheBlueMatt reviewed Feb 4, 2021

View reviewed changes

jkczyz force-pushed the 2021-01-http-client branch from 0c31383 to 34fe4ae Compare February 4, 2021 07:20

TheBlueMatt approved these changes Feb 4, 2021

View reviewed changes

jkczyz added 2 commits February 4, 2021 07:38

Add REST and RPC clients to lightning-block-sync

6ca3b8e

Implements a simple HTTP client that can issue GET and POST requests. Used to implement REST and RPC clients, respectively. Both clients support either blocking or non-blocking I/O.

Implement BlockSource for REST and RPC clients

85fdfaa

Interprets HTTP responses as either binary or JSON format, which are then converted to the appropriate data types.

jkczyz force-pushed the 2021-01-http-client branch from 34fe4ae to 85fdfaa Compare February 4, 2021 15:39

TheBlueMatt merged commit b67ec5a into lightningdevkit:main Feb 4, 2021

		/// Returns the hash of the best block and, optionally, its height. The height is passed to
		/// `get_header` to allow a more efficient lookup on some block sources.

		//! Both features support either blocking I/O using `std::net::TcpStream` or, with feature `tokio`,
		//! non-blocking I/O using `tokio::net::TcpStream` from inside a Tokio runtime.

HTTP-based block source clients #774

HTTP-based block source clients #774

Conversation

jkczyz commented Jan 12, 2021

codecov bot commented Jan 13, 2021 • edited Loading

Codecov Report

jkczyz commented Jan 13, 2021

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

valentinewallace Jan 18, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace commented Jan 20, 2021

jkczyz commented Jan 20, 2021

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

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

jkczyz commented Jan 28, 2021

jkczyz commented Jan 28, 2021

jkczyz commented Jan 30, 2021

jkczyz commented Jan 31, 2021

jkczyz commented Feb 4, 2021

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz Feb 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz Feb 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz Feb 4, 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

TheBlueMatt commented Feb 4, 2021

TheBlueMatt left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 13, 2021 •

edited

Loading

valentinewallace Jan 18, 2021 •

edited

Loading

jkczyz Feb 4, 2021 •

edited

Loading

jkczyz Feb 4, 2021 •

edited

Loading

jkczyz Feb 4, 2021 •

edited

Loading