Skip to content

Modify discrete log to support sync and async - #405

Merged
AddressXception merged 3 commits into
mainfrom
feature/dlog-cache
Aug 3, 2021
Merged

AddressXception merged 3 commits into
mainfrom
feature/dlog-cache

Conversation

@AddressXception

Copy link
Copy Markdown
Contributor

🚨 Please review the guidelines for contributing to this repository.

Issue

Link your PR to an issue

Fixes #393

Description

replaces PR #396. Make the discrete log sync or async. the methods use the synchronous version. Further work to load the dlog from a cache is tracked in #397

If necessary, async versions of the decryption logic may need to be added in the future.

Testing

  1. run the tests

Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved

@danwallach danwallach left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like overkill to go from global state to state within a class instance, but for most common usages of discrete_log, there won't be a practical difference.

The decision to raise an exception is a bit more interesting, because now you need to audit all the places that ask for a discrete log and make sure that they're prepared to catch that exception or it's okay for them to fail.

In my version, I just had it return Optional[int], which caused more pain initially, but the payoff was that mypy helped me find each and every place that was depending on that value succeeding, and then add more error handling where needed.

BTW, as to having the maximum value be 1 million or 100 million, you want this value to be big enough to encompass the homomorphic tally of the biggest election that ElectionGuard will ever handle. In L.A. County, California (which is probably the biggest single election jurisdiction in the U.S.), there were somewhere north of 4 million votes cast in November 2020. So, I picked 100 million to be really, really bigger than that, but still small enough that if you computed the whole thing by accident, you wouldn't run for a huge amount of time only to fail.

@danwallach danwallach left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if you really want to go the o-o route, I'd kill off the top-level functions and inline them into the DiscreteLog class, since anybody outside of this file shouldn't have to know about which mutex or which cache to use.

Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved
Comment thread src/electionguard/discrete_log.py Resolved
Comment thread tests/property/test_discrete_log.py Resolved
Comment thread src/electionguard/elgamal.py Resolved
@AddressXception

Copy link
Copy Markdown
Contributor Author

Also, if you really want to go the o-o route, I'd kill off the top-level functions and inline them into the DiscreteLog class, since anybody outside of this file shouldn't have to know about which mutex or which cache to use.

the idea is to have flexibility, allowing a consumer to use the functional low level process or the higher order "stateful" process. that's consistent with most of the other api's we have (e.g. encryptSelection, encryptContest, encryptBallot)

keithrfung
keithrfung previously approved these changes Aug 3, 2021
Comment thread pyproject.toml
[tool.poetry]
name = "electionguard"
version = "1.2.2"
version = "1.2.3"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good 👍

@AddressXception
AddressXception merged commit 2c29f48 into main Aug 3, 2021
@AddressXception
AddressXception deleted the feature/dlog-cache branch August 3, 2021 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove asyncio.run call in dlog.py

3 participants