Skip to content
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

chain_bridge: fetch block headers over blocks when verifying proofs #660

Merged
merged 2 commits into from
Nov 16, 2023

Conversation

jharveyb
Copy link
Contributor

@jharveyb jharveyb commented Nov 8, 2023

Fixes #315 , #613 . Depends on lightninglabs/lndclient#161 .

This makes use of the GetBlockHeader RPC now available in LND v0.17.1 to avoid fetching a full block during proof verification.

VerifyBlock is used in the HeaderVerifier, which never made use of the full block. This should be safe to change for all places where a HeaderVerifier is used, so this is added without adding a separate implementation of HeaderVerifier.

Confirmed that the GetBlock calls are no longer made by running tapd with this PR backed by LND v0.17.1-beta-rc1 and manually syncing.

@dstadulis dstadulis added this to the v0.3.2 milestone Nov 13, 2023
@jharveyb jharveyb force-pushed the getblockheader_verifier branch from 3c4375c to 6013359 Compare November 14, 2023 17:30
@dstadulis
Copy link
Collaborator

dstadulis commented Nov 14, 2023

  • test with old version to test fall back (tested with testnet lnd 0.17.0)

@jharveyb
Copy link
Contributor Author

Have tested that this uses GetBlockHeader() with lnd 0.17.1, still need to manually test that this will fall back to GetBlock() with lnd 0.17.0.

@jharveyb jharveyb marked this pull request as ready for review November 14, 2023 19:44
@jharveyb jharveyb force-pushed the getblockheader_verifier branch from 6013359 to 422a004 Compare November 14, 2023 21:27
@jharveyb
Copy link
Contributor Author

Should not merge until we get a tagged version for lndclient that removes the dependency on an RC LND release.

In this commit, we update the header verifier to use the new
GetBlockHeader RPC when supported, and avoid fetching a full block.
This improves the reliability of proof validation and universe sync for
light clients.
@jharveyb jharveyb force-pushed the getblockheader_verifier branch from 422a004 to 8a52e70 Compare November 16, 2023 06:13
@jharveyb
Copy link
Contributor Author

Bumped to appropriate lndclient and lnd versions, ready for merge.

Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

Looks good 💯

Copy link
Contributor

@ffranr ffranr left a comment

Choose a reason for hiding this comment

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

Looks good to me! I appreciate the small diff. 👍

@ffranr ffranr added this pull request to the merge queue Nov 16, 2023
Merged via the queue into main with commit b8a007d Nov 16, 2023
14 checks passed
@jharveyb jharveyb deleted the getblockheader_verifier branch November 16, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

tarogarden: cache block header verifier results during batch confirmation
4 participants