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

tarogarden: cache block header verifier results during batch confirmation #315

Closed
jharveyb opened this issue May 22, 2023 · 7 comments · Fixed by #660
Closed

tarogarden: cache block header verifier results during batch confirmation #315

jharveyb opened this issue May 22, 2023 · 7 comments · Fixed by #660
Assignees
Labels
Milestone

Comments

@jharveyb
Copy link
Contributor

Related to #314.

Generating proofs for a large batch is liable to time out due to delay from fetching blocks via RPC in the BatchStateConfirmed state.

Specifically, we create a GenHeaderVerifier that will be called at least twice, during proof generation and import. In this part of the state machine, we are repeatedly fetching the same block, which we already fetched and stored before generating these proofs.

We could replace the 'real' headerVerifier with a mock verifier that just checks for block header equality. If we wanted to use header verification for some purpose like reorg detection, it likely shouldn't be done for each proof.

@jharveyb
Copy link
Contributor Author

This is more tractable now that we have re-org handling. P sure what we want here is to use the caching verifier in BatchStateBroadcast, but NOT use it in planter.updateMintingProofs(), which is what actually handles re-orgs.

We also already have a template for caching verifier callbacks than use an LRU cache. To properly test this we should update the mint batch stress test to generate large blocks.

@jharveyb
Copy link
Contributor Author

Related, we can now use https://lightning.engineering/api-docs/api/lnd/neutrino-kit/get-block-header in the non-caching header verifier so speed that up as well.

@jharveyb jharveyb self-assigned this Oct 23, 2023
@guggero
Copy link
Member

guggero commented Oct 24, 2023

That API is only available to Neutrino backends. We should add a generic one to the chain kit RPC service.

@jharveyb
Copy link
Contributor Author

Can implement this once lnd 0.17.1 is taped out. We'll need a version check somewhere to create the correct header verifier to match the lnd version.

@jharveyb
Copy link
Contributor Author

jharveyb commented Nov 2, 2023

Specifically, we replace this GetBlock call IIUC?

_, err = l.GetBlock(ctx, header.BlockHash())

@jharveyb jharveyb added this to the v0.3.2 milestone Nov 6, 2023
@dstadulis dstadulis added the p2p label Nov 6, 2023
@dstadulis dstadulis moved this from 🆕 New to 🔖 Ready in Taproot-Assets Project Board Nov 6, 2023
@dstadulis
Copy link
Collaborator

@jharveyb has a draft branch of this which works.

Will post it after lightninglabs/lndclient#161 is merged

@dstadulis
Copy link
Collaborator

will need to Tag release for lnd-client after lnd release arrives

@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Taproot-Assets Project Board Nov 16, 2023
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 a pull request may close this issue.

3 participants