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

[perf]: Remove duplicated signature checks in light.VerifyNonAdjacent #2365

Open
ValarDragon opened this issue Feb 17, 2024 · 0 comments
Open
Labels
enhancement New feature or request light-client Light client-related

Comments

@ValarDragon
Copy link
Collaborator

Feature Request

Summary

Currently light.VerifyNonAdjacent runs two signature checks:

  • VerifyCommitLightTrusting - Check that 1/3rd of the trusted header's weight has signed the untrusted header.
  • VerifyCommitLight - Check that 2/3rds of the untrusted header's weight has signed the untrusted header.

Notice however, that for the signatures in VerifyCommitLight that were also checked in VerifyCommitLightTrusting, we have checked them twice.

Problem Definition

We see light.VerifyNonAdjacent taking lots of CPU time on Osmosis, coming from IBC. (Arguably IBC should have a copy of this logic there though)

VerifyCommitLightTrusting from light.VerifyNonAdjacent is currently 1.3% of the sync speed, and VerifyCommitLight from light.VerifyNonAdjacent is 4.1% of the sync speed. Which leads me to believe that fixing this signature duplication will lower the net sync speed by 1.3%. (Note: While this seems small right now, the other parts of the system, in particular the DB, is getting notably sped up. I expect once the next major release rolls around, that these IBC components are 4x more expensive within sync speed)

When Osmosis switches to a TM release that supports batch verification, these impacts will lower by a factor of 2, but I think are still significant enough performance for us to improve.

Proposal

There seems to me to be two ways to alleviate this:

  • Restructure the logic to make these signatures act as counted, but not need to be included in what we verify
  • Make a small cache for signature checks here, make variants of the methods that allow passing in a cache, and using a cache as an argument.

My intuition is that a small cache would be:

  1. Very simple to reason about correctness
  2. Improve the speed quite effectively!
@ValarDragon ValarDragon added enhancement New feature or request needs-triage This issue/PR has not yet been triaged by the team. labels Feb 17, 2024
@melekes melekes added light-client Light client-related block-sync and removed needs-triage This issue/PR has not yet been triaged by the team. block-sync labels Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request light-client Light client-related
Projects
None yet
Development

No branches or pull requests

2 participants