Repository: microsoft/electionguard-python

Issue:
✨ Perf enhancement for Tally
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Suggestion

The guardians and the admin all do a verify_ballot_encryption to go over all of the encryptions in each of the ballots. There is no need for this to be done 3+ times when a single check is enough.  

The guardians do not need to verify the encryptions and this will save time.

### Possible Implementation

_No response_

### Anything else?

_No response_

Discussion:

Patch:
diff --git a/src/electionguard/ballot_box.py b/src/electionguard/ballot_box.py
index b24e62c..088099c 100644
--- a/src/electionguard/ballot_box.py
+++ b/src/electionguard/ballot_box.py
@@ -57,7 +57,7 @@ def submit_ballot_to_box(
     that the ballot has not already been cast or spoiled.
     :return: a `SubmittedBallot` or `None` if there was an error
     """
-    if not ballot_is_valid_for_election(ballot, internal_manifest, context):
+    if not ballot_is_valid_for_election(ballot, internal_manifest, context, True):
         log_warning(f"ballot: {ballot.object_id} failed validity check")
         return None
 
diff --git a/src/electionguard/ballot_validator.py b/src/electionguard/ballot_validator.py
index 93eb479..a928dab 100644
--- a/src/electionguard/ballot_validator.py
+++ b/src/electionguard/ballot_validator.py
@@ -12,6 +12,7 @@ def ballot_is_valid_for_election(
     ballot: CiphertextBallot,
     internal_manifest: InternalManifest,
     context: CiphertextElectionContext,
+    should_validate: bool,
 ) -> bool:
     """
     Determine if a ballot is valid for a given election
@@ -20,15 +21,16 @@ def ballot_is_valid_for_election(
     if not ballot_is_valid_for_style(ballot, internal_manifest):
         return False
 
-    if not ballot.is_valid_encryption(
-        internal_manifest.manifest_hash,
-        context.elgamal_public_key,
-        context.crypto_extended_base_hash,
-    ):
-        log_warning(
-            f"ballot_is_valid_for_election: mismatching ballot encryption {ballot.object_id}"
-        )
-        return False
+    if should_validate:
+        if not ballot.is_valid_encryption(
+            internal_manifest.manifest_hash,
+            context.elgamal_public_key,
+            context.crypto_extended_base_hash,
+        ):
+            log_warning(
+                f"ballot_is_valid_for_election: mismatching ballot encryption {ballot.object_id}"
+            )
+            return False
 
     return True
 
diff --git a/src/electionguard/tally.py b/src/electionguard/tally.py
index 2416bc6..db1f0f2 100644
--- a/src/electionguard/tally.py
+++ b/src/electionguard/tally.py
@@ -226,7 +226,10 @@ class CiphertextTally(ElectionObjectBase, Container, Sized):
         return False
 
     def append(
-        self, ballot: SubmittedBallot, scheduler: Optional[Scheduler] = None
+        self,
+        ballot: SubmittedBallot,
+        should_validate: bool,
+        scheduler: Optional[Scheduler] = None,
     ) -> bool:
         """
         Append a ballot to the tally and recalculate the tally.
@@ -240,7 +243,7 @@ class CiphertextTally(ElectionObjectBase, Container, Sized):
             return False
 
         if not ballot_is_valid_for_election(
-            ballot, self._internal_manifest, self._encryption
+            ballot, self._internal_manifest, self._encryption, should_validate
         ):
             return False
 
@@ -256,6 +259,7 @@ class CiphertextTally(ElectionObjectBase, Container, Sized):
     def batch_append(
         self,
         ballots: Iterable[Tuple[Any, SubmittedBallot]],
+        should_validate: bool,
         scheduler: Optional[Scheduler] = None,
     ) -> bool:
         """
@@ -268,7 +272,7 @@ class CiphertextTally(ElectionObjectBase, Container, Sized):
             # get the value of the dict
             ballot_value = ballot[1]
             if not self.__contains__(ballot) and ballot_is_valid_for_election(
-                ballot_value, self._internal_manifest, self._encryption
+                ballot_value, self._internal_manifest, self._encryption, should_validate
             ):
                 if ballot_value.state == BallotBoxState.CAST:
 
@@ -431,7 +435,7 @@ def tally_ballot(
         )
         return None
 
-    if tally.append(ballot):
+    if tally.append(ballot, True):
         return tally
 
     return None
@@ -450,6 +454,6 @@ def tally_ballots(
     tally: CiphertextTally = CiphertextTally(
         "election-results", internal_manifest, context
     )
-    if tally.batch_append(store):
+    if tally.batch_append(store, True):
         return tally
     return None
diff --git a/src/electionguard_cli/cli_steps/tally_step.py b/src/electionguard_cli/cli_steps/tally_step.py
index 34cda15..fc46228 100644
--- a/src/electionguard_cli/cli_steps/tally_step.py
+++ b/src/electionguard_cli/cli_steps/tally_step.py
@@ -44,7 +44,7 @@ class TallyStep(CliStepBase):
             build_election_results.context,
         )
         with Scheduler() as scheduler:
-            tally.batch_append(ballots, scheduler)
+            tally.batch_append(ballots, True, scheduler)
         self.print_value("Ballots in tally", tally.__len__())
         return tally
 
diff --git a/src/electionguard_gui/services/decryption_stages/decryption_s1_join_service.py b/src/electionguard_gui/services/decryption_stages/decryption_s1_join_service.py
index f3e54be..7c4ed56 100644
--- a/src/electionguard_gui/services/decryption_stages/decryption_s1_join_service.py
+++ b/src/electionguard_gui/services/decryption_stages/decryption_s1_join_service.py
@@ -25,7 +25,7 @@ class DecryptionS1JoinService(DecryptionStageBase):
         spoiled_ballots = [
             ballot for ballot in ballots if ballot.state == BallotBoxState.SPOILED
         ]
-        ciphertext_tally = get_tally(manifest, context, ballots)
+        ciphertext_tally = get_tally(manifest, context, ballots, False)
         decryption_share = guardian.compute_tally_share(ciphertext_tally, context)
         if decryption_share is None:
             raise Exception("No decryption_shares found")
diff --git a/src/electionguard_gui/services/decryption_stages/decryption_s2_announce_service.py b/src/electionguard_gui/services/decryption_stages/decryption_s2_announce_service.py
index 2f07ffc..d43b625 100644
--- a/src/electionguard_gui/services/decryption_stages/decryption_s2_announce_service.py
+++ b/src/electionguard_gui/services/decryption_stages/decryption_s2_announce_service.py
@@ -13,10 +13,10 @@ class DecryptionS2AnnounceService(DecryptionStageBase):
     """Responsible for the 2nd stage in decryptions where the admin announces guardian decryptions"""
 
     def should_run(self, db: Database, decryption: DecryptionDto) -> bool:
-        isAdmin = self._auth_service.is_admin()
-        allGuardiansJoined = len(decryption.guardians_joined) >= decryption.guardians
-        isCompleted = decryption.completed_at_utc is not None
-        return isAdmin and allGuardiansJoined and not isCompleted
+        is_admin = self._auth_service.is_admin()
+        all_guardians_joined = len(decryption.guardians_joined) >= decryption.guardians
+        is_completed = decryption.completed_at_utc is not None
+        return is_admin and all_guardians_joined and not is_completed
 
     def run(self, db: Database, decryption: DecryptionDto) -> None:
         self._log.info(f"S2: Announcing decryption {decryption.decryption_id}")
@@ -47,7 +47,7 @@ class DecryptionS2AnnounceService(DecryptionStageBase):
         spoiled_ballots = [
             ballot for ballot in ballots if ballot.state == BallotBoxState.SPOILED
         ]
-        ciphertext_tally = get_tally(manifest, context, ballots)
+        ciphertext_tally = get_tally(manifest, context, ballots, True)
         self._log.debug("getting plaintext tally")
         plaintext_tally = decryption_mediator.get_plaintext_tally(
             ciphertext_tally, manifest
diff --git a/src/electionguard_gui/services/decryption_stages/decryption_stage_base.py b/src/electionguard_gui/services/decryption_stages/decryption_stage_base.py
index e351fe6..13dd0f3 100644
--- a/src/electionguard_gui/services/decryption_stages/decryption_stage_base.py
+++ b/src/electionguard_gui/services/decryption_stages/decryption_stage_base.py
@@ -59,6 +59,7 @@ def get_tally(
     manifest: Manifest,
     context: CiphertextElectionContext,
     ballots: List[SubmittedBallot],
+    should_validate: bool,
 ) -> CiphertextTally:
     internal_manifest = InternalManifest(manifest)
 
@@ -69,5 +70,5 @@ def get_tally(
     )
     ballot_tuples = [(None, ballot) for ballot in ballots]
     with Scheduler() as scheduler:
-        tally.batch_append(ballot_tuples, scheduler)
+        tally.batch_append(ballot_tuples, should_validate, scheduler)
     return tally
diff --git a/src/electionguard_verify/verify.py b/src/electionguard_verify/verify.py
index c06dec9..a8ca357 100644
--- a/src/electionguard_verify/verify.py
+++ b/src/electionguard_verify/verify.py
@@ -77,7 +77,7 @@ def verify_aggregation(
     new_tally = CiphertextTally("verify", InternalManifest(manifest), context)
 
     for ballot in submitted_ballots:
-        new_tally.append(ballot)
+        new_tally.append(ballot, True)
 
     if (
         isinstance(tally, CiphertextTally)


Test patch:
diff --git a/tests/property/test_tally.py b/tests/property/test_tally.py
index 01ddbe8..5515b23 100644
--- a/tests/property/test_tally.py
+++ b/tests/property/test_tally.py
@@ -175,15 +175,15 @@ class TestTally(BaseTestCase):
 
         # verify an UNKNOWN state ballot fails
         self.assertIsNone(tally_ballot(first_ballot, tally))
-        self.assertFalse(tally.append(first_ballot))
+        self.assertFalse(tally.append(first_ballot, True))
 
         # cast a ballot
         first_ballot.state = BallotBoxState.CAST
-        self.assertTrue(tally.append(first_ballot))
+        self.assertTrue(tally.append(first_ballot, False))
 
         # try to append a spoiled ballot
         first_ballot.state = BallotBoxState.SPOILED
-        self.assertFalse(tally.append(first_ballot))
+        self.assertFalse(tally.append(first_ballot, True))
 
         # Verify accumulation fails if the selection collection is empty
         if first_ballot.state == BallotBoxState.CAST:
@@ -217,12 +217,12 @@ class TestTally(BaseTestCase):
 
         # verify a cast ballot cannot be added twice
         first_ballot.state = BallotBoxState.CAST
-        self.assertTrue(tally.append(first_ballot))
-        self.assertFalse(tally.append(first_ballot))
+        self.assertTrue(tally.append(first_ballot, True))
+        self.assertFalse(tally.append(first_ballot, False))
 
         # verify an already submitted ballot cannot be changed or readded
         first_ballot.state = BallotBoxState.SPOILED
-        self.assertFalse(tally.append(first_ballot))
+        self.assertFalse(tally.append(first_ballot, True))
 
     @staticmethod
     def _decrypt_with_secret(
@@ -255,7 +255,7 @@ class TestTally(BaseTestCase):
         ballot.contests[0].ballot_selections.remove(first_selection)
 
         self.assertIsNone(tally_ballot(ballot, tally))
-        self.assertFalse(tally.append(ballot))
+        self.assertFalse(tally.append(ballot, True))
 
         # Verify accumulation fails if the selection count does not match
         if ballot.state == BallotBoxState.CAST:
@@ -278,7 +278,7 @@ class TestTally(BaseTestCase):
         first_contest_hash = ballot.contests[0].description_hash
         ballot.contests[0].description_hash = ONE_MOD_Q
         self.assertIsNone(tally_ballot(ballot, tally))
-        self.assertFalse(tally.append(ballot))
+        self.assertFalse(tally.append(ballot, True))
 
         ballot.contests[0].description_hash = first_contest_hash
 
@@ -286,7 +286,7 @@ class TestTally(BaseTestCase):
         first_contest_object_id = ballot.contests[0].object_id
         ballot.contests[0].object_id = "a-bad-object-id"
         self.assertIsNone(tally_ballot(ballot, tally))
-        self.assertFalse(tally.append(ballot))
+        self.assertFalse(tally.append(ballot, True))
 
         ballot.contests[0].object_id = first_contest_object_id
 
@@ -297,7 +297,7 @@ class TestTally(BaseTestCase):
         ballot.contests[0].ballot_selections[0].object_id = "another-bad-object-id"
 
         self.assertIsNone(tally_ballot(ballot, tally))
-        self.assertFalse(tally.append(ballot))
+        self.assertFalse(tally.append(ballot, True))
 
         # Verify accumulation fails if the selection object id does not match
         if ballot.state == BallotBoxState.CAST:
@@ -315,7 +315,7 @@ class TestTally(BaseTestCase):
         first_ballot_hash = ballot.manifest_hash
         ballot.manifest_hash = ONE_MOD_Q
         self.assertIsNone(tally_ballot(ballot, tally))
-        self.assertFalse(tally.append(ballot))
+        self.assertFalse(tally.append(ballot, True))
 
         ballot.manifest_hash = first_ballot_hash
         ballot.state = input_state