light-client-verifier: optimise validator lookup in voting_power_in

[mina86]

Rather than performing a linear search for a validator by its address, construct an index for the validator set sorted by address. While building the index takes O(N log N) time, it cuts lookup to O(log n).

On Solana, with ~50 validators, this change causes reduction of the cost of a single lookup from 20k compute units to 700.

Furthermore, merge the index with the seen_validators map so that only a single lookup is now performed which allows us to find the validator and check whether it’s not a duplicate.

Closes: #1406

• Referenced an issue explaining the need for the change
• Updated all relevant documentation in docs
• Updated all code comments where relevant
• Wrote tests
• Added entry in .changelog/

[mina86]

Test / nightly-coverage (pull_request) failure is unrelated to this PR.

[romac]

@mina86 Thanks, this is very nice. That cost reduction looks great!

What do you think about extracting the validator index into its own struct, something like this?

diff --git a/light-client-verifier/src/operations/voting_power.rs b/light-client-verifier/src/operations/voting_power.rs
index cc6899f0..302e24d3 100644
--- a/light-client-verifier/src/operations/voting_power.rs
+++ b/light-client-verifier/src/operations/voting_power.rs
@@ -5,9 +5,11 @@ use core::{fmt, marker::PhantomData};
 
 use serde::{Deserialize, Serialize};
 use tendermint::{
+    account,
     block::CommitSig,
     crypto::signature,
     trust_threshold::TrustThreshold as _,
+    validator,
     vote::{SignedVote, ValidatorIndex, Vote},
 };
 
@@ -123,6 +125,33 @@ impl<V> Default for ProvidedVotingPowerCalculator<V> {
 pub type ProdVotingPowerCalculator =
     ProvidedVotingPowerCalculator<tendermint::crypto::default::signature::Verifier>;
 
+/// Create index of validators sorted by address. The index stores
+/// reference to the validator’s Info object held by the validator_set
+/// and boolean flag indicating whether we've already seen that
+/// validator.
+struct ValidatorMap<'a> {
+    validators: Vec<(&'a validator::Info, bool)>,
+}
+
+impl<'a> ValidatorMap<'a> {
+    fn new(validators: &'a [validator::Info]) -> Self {
+        let mut validators = validators.iter().map(|v| (v, false)).collect::<Vec<_>>();
+        validators.sort_unstable_by_key(|item| &item.0.address);
+        Self { validators }
+    }
+
+    fn find_mut(&mut self, address: &account::Id) -> Option<(&'a validator::Info, &mut bool)> {
+        let index = self
+            .validators
+            .binary_search_by_key(&address, |item| &item.0.address)
+            .ok()?;
+
+        let (validator, seen) = &mut self.validators[index];
+
+        Some((validator, seen))
+    }
+}
+
 impl<V: signature::Verifier> VotingPowerCalculator for ProvidedVotingPowerCalculator<V> {
     fn voting_power_in(
         &self,
@@ -145,28 +174,17 @@ impl<V: signature::Verifier> VotingPowerCalculator for ProvidedVotingPowerCalcul
             .map(|vote| (signature, vote))
         });
 
-        // Create index of validators sorted by address.  The index stores
-        // reference to the validaotr’s Info object held by the validator_set
+        // Create index of validators sorted by address. The index stores
+        // reference to the validator’s Info object held by the validator_set
         // and boolean flag indicating whether we've already seen that
         // validator.
-        let mut validators: Vec<(&_, bool)> = validator_set
-            .validators()
-            .iter()
-            .map(|v| (v, false))
-            .collect();
-        validators.sort_unstable_by_key(|item| &item.0.address);
+        let mut validators = ValidatorMap::new(validator_set.validators());
 
         for (signature, vote) in non_absent_votes {
             // Find the validator by address.
-            let index = validators
-                .binary_search_by_key(&&vote.validator_address, |item| &item.0.address)
-                .map(|index| {
-                    let item = &mut validators[index];
-                    (item.0, &mut item.1)
-                });
-            let (validator, seen) = match index {
-                Ok(it) => it,
-                Err(_) => continue, // Cannot find matching validator, so we skip the vote
+            let Some((validator, seen)) = validators.find_mut(&vote.validator_address) else {
+                // Cannot find matching validator, so we skip the vote
+                continue;
             };
 
             // Ensure we only count a validator's power once
@@ -175,6 +193,8 @@ impl<V: signature::Verifier> VotingPowerCalculator for ProvidedVotingPowerCalcul
                     vote.validator_address,
                 ));
             }
+
+            // Mark this validator as seen
             *seen = true;
 
             let signed_vote =

I chose the ValidatorMap name here because ValidatorIndex is already in scope and refers to something else, but feel free to pick a better name if you see one.

[mina86]

Done. I’ve changed find to return an error and mark seen if already seen. This way ValidatorMap interface enforces that each validator can be seen only once constraint which I think is a tad cleaner code.

[romac]

@mina86 Even better! 🚀