clippy fixes and node id support for fhe/zkp compilers #269
Conversation
…roup id's into the stacktrie
Merge branch 'mliu-backend' of https://github.com/Sunscreen-tech/Sunscreen into mliu-backend
| }); | ||
| } | ||
|
|
||
| #[cfg(feature = "debugger")] |
There was a problem hiding this comment.
You don't need to gate these, as the entire module it lives in is gates (I think)
| @@ -5,12 +5,14 @@ | |||
| //! (i.e. an [`FheProgram`](sunscreen_fhe_program::FheProgram)). | |||
|
|
|||
| mod array; | |||
| mod debugger; | |||
| @@ -5,12 +5,14 @@ | |||
| //! (i.e. an [`FheProgram`](sunscreen_fhe_program::FheProgram)). | |||
|
|
|||
| mod array; | |||
| mod debugger; | |||
There was a problem hiding this comment.
We should gate this under the debugger feature.
sunscreen_runtime/src/lib.rs
Outdated
| mod error; | ||
| mod keys; | ||
| mod metadata; | ||
| mod run; | ||
| mod runtime; | ||
| mod serialization; | ||
| mod zkp; |
There was a problem hiding this comment.
I think we can delete this module and the file zkp.rs.
sunscreen_runtime/src/lib.rs
Outdated
|
|
||
| use seal_fhe::{Ciphertext as SealCiphertext, Plaintext as SealPlaintext}; | ||
| use serde::{Deserialize, Serialize}; | ||
| use sunscreen_zkp_backend::BigInt; | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Hash, Serialize, Deserialize, Eq)] | ||
| //#[serde(tag = "type")] |
There was a problem hiding this comment.
Just remove all these for now.
| /** | ||
| * The underlying backend implementation of a plaintext (e.g. SEAL's [`Plaintext`](seal_fhe::Plaintext)). | ||
| */ | ||
| pub enum InnerPlaintext { | ||
| /** | ||
| * This plaintext wraps a SEAL [`Plaintext`](seal_fhe::Plaintext). | ||
| */ | ||
| Seal(Vec<WithContext<SealPlaintext>>), | ||
| Seal { | ||
| /** |
There was a problem hiding this comment.
In the future, start using
/// this is a doc
/// commentstyle doc comments. JS doc tends to give subtle annoying issues and we've agreed to consolodate around ///.
| @@ -103,6 +128,8 @@ pub unsafe fn run_program_unchecked<E: Evaluator + Sync + Send>( | |||
| evaluator: &E, | |||
| relin_keys: &Option<&RelinearizationKeys>, | |||
| galois_keys: &Option<&GaloisKeys>, | |||
| debug_info: Option<DebugInfo>, | |||
| #[cfg(feature = "debugger")] source_code: &str, | |||
There was a problem hiding this comment.
This should not be an argument. The source code should exist on ir's metadata..
| @@ -157,25 +184,70 @@ pub unsafe fn run_program_unchecked<E: Evaluator + Sync + Send>( | |||
| data.push(AtomicCell::new(None)); | |||
| } | |||
|
|
|||
| #[cfg(feature = "debugger")] | |||
There was a problem hiding this comment.
Don't need to change for this code review, but check how Brian and I make a session for ZKP. It's way cleaner.
|
|
||
| #[cfg(feature = "debugger")] | ||
| #[test] | ||
| #[cfg_attr(feature = "debugger", serial)] |
There was a problem hiding this comment.
We should not have tests that require to be run serially.
|
|
||
| use sunscreen_zkp_backend::{BigInt, Operation as ZkpOperation}; | ||
|
|
||
| use std::time::Instant; |
| @@ -244,13 +281,20 @@ pub fn jit_prover<U>( | |||
| constant_inputs: &[U], | |||
| public_inputs: &[U], | |||
| private_inputs: &[U], | |||
| session_provider: Option<&dyn DebugSessionProvider<Operation, BigInt, String>>, | |||
| where | ||
| S: serde::Serializer, | ||
| { | ||
| serializer.serialize_str(self.debug_name()) |
There was a problem hiding this comment.
Hmm... I don't approve of this. The expectation when you serialize something is that you can deserialize it and get a the exact thing back. This cannot be the case with Gadgets because they contain code.
Unfortunately, there isn't a simple alternative; you need to implement serialization that isn't serialization.
| where | ||
| S: serde::Serializer, | ||
| { | ||
| serializer.serialize_str(self.to_string().as_str()) |
There was a problem hiding this comment.
I assume this prints hexadecimal?
We also need a matching Deserialize.
No description provided.