Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

TheBlueMatt
Copy link
Collaborator

@TheBlueMatt TheBlueMatt commented May 2, 2020

This is still very much a WIP, and I want to reduce the dependencies to at least not rely on hyper as well as actually implement the REST-based client, though doing so should be trivial.

This adds a new subcrate lightning-http-blocks 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.

Addresses #627.

@TheBlueMatt TheBlueMatt force-pushed the 2020-05-rest-chainsync branch 2 times, most recently from aae0859 to 8fcb6e1 Compare May 2, 2020 04:42
These are essentially required to make rescan-at-reload doable as
individual ChannelMonitors may be synced to different chain states
and thus need to have blocks replayed separately.
@TheBlueMatt TheBlueMatt force-pushed the 2020-05-rest-chainsync branch 3 times, most recently from b0319cd to c872ef4 Compare May 2, 2020 05:27
@TheBlueMatt TheBlueMatt changed the title WIP: Add subcrate which impls a simple SPV client from Bitcoin Core RPC Add subcrate which impls a simple SPV client from Bitcoin Core RPC May 2, 2020
@TheBlueMatt TheBlueMatt marked this pull request as ready for review May 2, 2020 05:27
@TheBlueMatt
Copy link
Collaborator Author

Still a bit raw in terms of code quality and still need testing, but its a thing, and worth at least a glance-over.

@TheBlueMatt TheBlueMatt force-pushed the 2020-05-rest-chainsync branch 2 times, most recently from a5f78fc to a61c2cb Compare May 2, 2020 19:01
@codecov
Copy link

codecov bot commented May 2, 2020

Codecov Report

Merging #614 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #614      +/-   ##
==========================================
- Coverage   91.12%   91.11%   -0.01%     
==========================================
  Files          34       34              
  Lines       20544    20544              
==========================================
- Hits        18720    18719       -1     
- Misses       1824     1825       +1     
Impacted Files Coverage Δ
lightning/src/ln/channelmonitor.rs 95.50% <100.00%> (ø)
lightning/src/ln/functional_tests.rs 97.02% <0.00%> (-0.02%) ⬇️

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 9098240...197e244. Read the comment docs.

@TheBlueMatt TheBlueMatt mentioned this pull request May 3, 2020
@TheBlueMatt TheBlueMatt force-pushed the 2020-05-rest-chainsync branch 4 times, most recently from 0861e43 to 23368f5 Compare May 5, 2020 20:14
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.
@TheBlueMatt TheBlueMatt force-pushed the 2020-05-rest-chainsync branch from 23368f5 to 197e244 Compare May 5, 2020 21:26
@valentinewallace valentinewallace self-requested a review May 18, 2020 22:03
Copy link
Contributor

@valentinewallace valentinewallace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't dived into the details yet but overall the structure looks good and makes sense!

I think the commits could be split up a bit more though. If you're busy with language bindings I don't mind taking on this task. The following split makes sense to me, in order:

  1. common utilities used between REST client and RPC client
  2. the HTTP client
  3. the REST client (or swap 2 and 3, or possibly combine them)
  4. BlockSource + its implementors + the structs that only BlockSource/its implementors use
  5. The AChainListener + its implementors
  6. The MicroSPVClient and its static functions

Copy link
Contributor

@valentinewallace valentinewallace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also -- I think this could benefit from some example usage similar to what's in lightning-net-tokio: https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning-net-tokio/src/lib.rs#L17 through line 62.

@TheBlueMatt
Copy link
Collaborator Author

@valentinewallace if you feel up to it feel free to take this over! Or @jkczyz if so inclined.

@jkczyz
Copy link
Contributor

jkczyz commented Jun 17, 2020

@valentinewallace if you feel up to it feel free to take this over! Or @jkczyz if so inclined.

FYI, I've taken on this work and am currently working on some necessary refactoring to BlockNotifier and ChainListener.

Regarding eec4beb, the refactoring mentioned above may alter this in some way. I'm looking into that now and will ping you with any questions that I may have.

@jkczyz jkczyz mentioned this pull request Jun 23, 2020
@jkczyz
Copy link
Contributor

jkczyz commented Jun 25, 2020

FYI, I've taken on this work and am currently working on some necessary refactoring to BlockNotifier and ChainListener.

Here's the PR rebased on #649 using BlockNotifier instead of AChainListener: https://github.com/jkczyz/rust-lightning/tree/pr-614-rebased.

@TheBlueMatt
Copy link
Collaborator Author

Superseded by #763.

@TheBlueMatt TheBlueMatt closed this Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants