axelarnetwork / tofn

A threshold cryptography library in Rust
Apache License 2.0
112 stars 23 forks source link

feat(build):malicious #27

Closed sdaveas closed 3 years ago

sdaveas commented 3 years ago

Put malicious mta functionality and tests inside a new module

Todo: Do the same with zkp::{pedersen,range}

ggutoski commented 3 years ago

I'm not a fan of putting pub(crate) in front of some fields (but not others) just to enable tests. [example]

The pattern I've been following so far is that things in zkp are self-contained, general-purpose utilities. One could argue that they should be in their own separate crate but I don't think it's worth the hassle right now. The lesson is that code to, for example, corrupt an individual proof should be contained in the module that defines that proof. Some options:

  1. Just put #[cfg(feature = "malicious")] like I had in prior comments. Yes, this violates the principle that malicious code should be quarantined inside a separate malicious module. But one could argue that these utilities are small enough that the benefit to code readability outweighs the loss of strict structure.
  2. Make a malicious submodule within each module. eg. range::malicious, pedersen::malicious, etc. This respects the above "quarantine" whilst obeying rules of structure. For readability, the malicious module need not be in a separate source file. The more I think about this option the more I like it.
  3. Don't bother at all with #[cfg(feature = "malicious")] and simply expose corrupt_proof methods as part of the default API for these modules.

I'm currently leaning toward option 2. Thoughts?