added check to suppress unnecessary log warning

[danwallach]

Issue

Fixes #122

Description

Makes the warning conditional on the instance type of the CiphertextBallot. If it's a CiphertextAcceptedBallot, then no warning is generated. Otherwise, behavior is identical.

(This patch is only relevant to the fix/deserialization-issues branch, where it's -3+4 lines. If you compare it to main, it's much bigger.)

Testing

In regular use, should have no effect on testing. But logs will be cleaner, easier to read.

Checklist

🚨Please review the guidelines for contributing to this repository.

• 🤔 CONSIDER adding a unit test if your PR resolves an issue.
• ✅ DO check open PR's to avoid duplicates.
• ✅ DO keep pull requests small so they can be easily reviewed.
• ✅ DO build locally before pushing.
• ✅ DO make sure tests pass.
• ✅ DO make sure any new changes are documented.
• ✅ DO make sure not to introduce any compiler warnings.
• ❌AVOID breaking the continuous integration build.
• ❌AVOID making significant changes to the overall architecture.

💚Thank you!

[AddressXception]

why dont we just get rid of the error message entirely? or change it to log_debug? i think that's preferable over checking the subclassed type, right?

[danwallach]

I'm fine with nuking this entirely. If you're going to keep any sort of logging here, it should only happen with dealing with CiphertextBallots in other states. A CiphertextAcceptedBallot will never have a nonce in it.

[danwallach]

So I had an a-ha moment. This particular method doesn't actually care if the nonce is absent. We're computing a hash value over various things. If the nonce isn't there, we can still compute a hash value. hash_elems doesn't care. The only issue is whether we want this particular hash value to be the same for a CiphertextBallot (with nonce) and an otherwise equal CiphertextAcceptedBallot (without nonce).

Anyway, I pushed a change that gets rid of the warning check altogether. I think this is fine.

[danwallach]

Food for thought: should the nonce actually be included in this particular computation? I don't know exactly where we're calling this method, but if we want the same result for a CiphertextBallot and its corresopnding CiphertextAcceptedBallot, then the nonce should not be included in what's being passed to hash_elems.