-
Notifications
You must be signed in to change notification settings - Fork 787
[perf]: Remove duplicated signature checks in light.VerifyNonAdjacent #2365
Description
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:
- Very simple to reason about correctness
- Improve the speed quite effectively!