Sunscreen example comments about sunscreen

[rickwebiii]

Thanks for the feedback @d-haxton. I'll file issues for some of these items.

I had written up a bunch of stuff, but I see you example is more illustrative than anything else.

fn main() -> Result<()> {
    let runtime = Compiler::with_circuit(simple_multiply)
        .plain_modulus_constraint(PlainModulusConstraint::Raw(600))
        .noise_margin_bits(5)
        .encoder(BFVScalarEncoder::new())
        .relin(true)
        .compile()?;

    let a: u32 = 15;
    let b: u32 = 5;

    let results = run_program!(simple_multiply, runtime, a, b)?;

    assert_eq!(1, results.len());

    let c: u32 = runtime.decrypt_and_decode(results[0]);

    assert_eq!(c, 75);

    Ok(())
}

I do really like the run_circuit! macro and I think we should incorporate that. I know we have a bunch of work to do around types and encoders, so this is far from finalized.

I think a failure here is that our example has the same person doing everything, which is entirely unrepresentative of what someone would actually do. I'll try to write an example with the things Alice and Bob would each do, which I think would better explain the separation. A more realistic use case is:

0. Bob compiles a circuit and serializes the params and an interface for his circuit (this does not exist today). Once Bob has done this, he can just write the circuit and params to disk and deserialize it later.
1. Alice asks Bob for his circuit interface and params. Alice creates a public/private key, possibly relin+galois keys depending on the circuit requirements. Alice encodes and encrypts her data, gives her data, public key, relin+galois keys to Bob.
2. Bob takes Alice's Ciphertexts and keys and runs the cached circuit. He then serializes the result ciphertexts and returns them to Alice. Bob has no guarantees Alice sent him the right types, as they're encrypted.
3. Alice decrypts the result and then decodes it.

There are other scenarios with multi-party computation and such.

A couple of other points:

• Circuit arguments and return values may be heterogenous, so having a single encoder is too limiting.
• We want the types for a circuit to be flexible. What we have right now isn't representative of the final product. Users will be able to add new types, e.g. Matrix, Rational, etc.
• Encoders are a concept in SEAL that I think are probably going to go away. The primitives users will have are FHE types that will probably impl some traits and get lifted into a GraphMarker<T: BfvType> during circuit construction. These types will feature methods for converting to and from Plaintext types.
• A non-FHE type (e.g. f64, u64, Vec<T> etc) may have more than one encoding even within a single encryption scheme.
• A single FHE type may have multiple encode/decode methods. For example, a Rational might take an f64 or integral numerator and denominator.
• Leveraging the type system to cause compile errors and hint to Bob what he needs to do is preferable to runtime errors. For example,

fn run_sum(a: &Ciphertext, b: &Ciphertext, public_key: &PublicKey) -> Ciphertext {
  // Bob has already compiled the circuit and stored the circuit and params somewhere
  let sum_runtime = RuntimeBuilder::new(&SUM_PARAMS)
    .with_public_key(&public_key)
    .build();

  run_circuit!(sum_runtime, a, b)
}

versus

fn run_sum(a: &Ciphertext, b: &Ciphertext, public_key: &PublicKey) -> Ciphertext {
  // Bob has already compiled the circuit and stored the circuit and params somewhere
  let sum_runtime = RuntimeBuilder::new(&SUM_PARAMS)
    .build();

  run_circuit!(sum_runtime, public_key, a, b)
}

In the former API, Bob doesn't know until runtime that he remembered to pass the public_key. In the latter case, the run_circuit macro requires a public_key as the second argument and will give a compilation error if you forget.

[d-haxton]

Oh yes, the Alice and Bob examples would probably really help. I'll probably follow up with some more thoughts later, I'm still not super clear on everything, but the realistic use case makes a lot of sense and seeing more code around that would definitely help.

One question though. Have you thought at all about how Alice and Bob are going to be sharing the circuit definitions? At a cursory glance it may make sense to use something like protobufs, but that would be going away from your nice circuit macro. Nothing immediate on your end, but I could see it being nice for Bob to publish a protobuf file publicly so that Alices knows how to interact with him and don't all need to re-write the circuit in rust every time (which might mean less buggy code).

[rickwebiii]

TODOs that came out of this and feedback from others:

• Remove encoders and just let user encrypt/decrypt FHETypes
• Annotate all FHETypes with TypeName
• Have circuit! macro generate interface annotating the type arguments and return values for a circuit and which keys are needed.
• Compiler should return object with circuit interfaces
• Create TryIntoPlaintexts/TryFromPlaintexts to turn FHETypes into Vec<Plaintext>.
• Create derive(TryIntoPlaintexts/TryFromPlaintexts) macro that FHETypes can use.
• Create CircuitBundle<T: TryIntoPlaintexts> struct, which contains Galois, Relin keys,
• Runtime should have a call method that accepts `T:
• Compiler should have method to codegen struct for circuit interface CircuitNameArgs and CircuitNameReturn.
• Runtime should feature bundle<T: TryIntoPlaintexts>(x: T) -> CircuitBundle<T> method to package arguments to send to Bob.

[rickwebiii]

@ravital if @d-haxton wants to have another look at what we've done since last time, we might be able to close this.

[d-haxton]

Sorry about the lack of communication! Haven't used github in a while and don't have notifications set up.

I took another look at the calculator you have and it definitely reads a lot better. There's a TON of good improvements here and overall I'd feel pretty comfortable using this. Some comments though, feel free to ignore throw out whatever doesn't make sense.

• [question] simply_multiply required seal as a dependency in your Cargo.toml but calculator does not?

• [question] forced type coherence somewhere?

  1 + 2.1234567890123456789
  3.1234567890123457

• [nit] you could have one channel where T type is something like:

  enum Message {
    AlicePublicKey(PublicKey),
    AliceCalc(ParseResult),
    BobParams(Params),
    BobResult(Ciphertext),
  }

  But that might complicate Alice and Bob's code a bit so /shrug

• compile_circuits is pretty harsh to read, there's a lot you're forcing users to repeat.

  • Realistically passing in &add_circuit.metadata.params to every with_params just feels like code smell and a risk to introduce bugs.

  •     // In order for ciphertexts to be compatible between circuits, they must all use the same
        // parameters.
    

    This is a BIG comment and we should be enforcing this more in the code/type system rather than in a comment like this.

    Would love to hear your thoughts on this though. Perhaps just have CompiledCircuit have a generic type which is derived from the parameters, but this is probably my biggest concern right now.

• [nit] destructure! let calc = recv_calc.recv().unwrap(); -> let ParseResult { left, right, op} = recv_calc.recv().unwrap();

  • Then your matches are just match op / match left

• There's a lot happening here so let's try to break it down

•             let mut c = match calc.op {
                  Operand::Add => runtime.run(&add, vec![left, right], &public_key).unwrap(),
                  Operand::Sub => runtime.run(&sub, vec![left, right], &public_key).unwrap(),
                  Operand::Mul => runtime.run(&mul, vec![left, right], &public_key).unwrap(),
                  Operand::Div => runtime.run(&div, vec![left, right], &public_key).unwrap(),
              };
  
              // Our circuit produces a single value, so move the value out of the vector.
              let c = c.drain(0..).next().unwrap();
              ans = c.clone();
  	
  			send_res.send(c).unwrap();

  • c is redefined and complicates reading
  • c as a variable name is not ideal in either context.
  • ans continues to complicate and does not make reading intuitive. I understand it's nice to have as a product, but if this is the example you want to give to people maybe not? The looping in conjunction with reassigning ans to be able to use that as a value. Yes it's nice to see that Alice can just say "use the last result" and that she doesn't need to send it, but can't she just send the Ciphertext and that has the same result?
    • Is Bob caching the result going to be a common use case?
  • run as a term doesn't convey that anything is being returned. consider eval or exec as both these have fairly strong connotations already.
  • Unless we expect c (the Vec) to be refilled why do we want to use the drain API? I could understand if c was a Boxed reference that we Alice passed earlier for results, but feels a bit overkill otherwise.

• [nit] having string parsing in alice() is a bit rough, can we refactor the rest of this out into parse_input or another method?

• result is defined 3 times here.

              let result: Ciphertext = recv_res.recv().unwrap();
              let result: Rational = match runtime.decrypt(&result, &secret) {
                  Ok(v) => v,
                  Err(RuntimeError::TooMuchNoise) => {
                      println!("Decryption failed: too much noise");
                      continue;
                  }
                  Err(e) => panic!("{:#?}", e),
              };
              let result: f64 = result.into();
  
              println!("{}", result);

  Also I'm not clear what to do if I get the too much noise error. Do we give up on that circuit? Are we done / should we panic? Is it recoverable or did that operation just fail?

[rickwebiii]

[question] simply_multiply required seal as a dependency in your Cargo.toml but calculator does not?

Good catch. It used to, but no longer does since we now have our own Ciphertext type.

[question] forced type coherence somewhere?

User input and results parse and roundtrip through an f64.

f64 which has 52 bits of mantissa (and generally looks like 1.m^(1023-exp)). The integer part of the answer is 3, which is 11 in binary. To normalize the result, we lose a bit of mantissa as the 1s place moves down into the mantissa. 2^-51 = 4.4408920985e-16. I count 16 places after 3, so the answer (at first glance at least) is within floating point roundoff.

[nit] you could have one channel where T type is something like...

I think you'd need a bunch of unwrap_variant1, unwrap_variant2, etc methods so the example doesn't get bogged down in if let or match statements. In a REST service, the communication would be to different endpoints with their own contracts, so I think using multiple channels kinda captures the spirit of that.

compile_circuits is pretty harsh to read, there's a lot you're forcing users to repeat.

No argument there. @ravital and I have talked about "how do I compile a collection of circuits to guarantee their schemes are compatible." Our tentative proposal is something like:

Compiler::new()
        .with_circuit(add)
        .with_circuit(sub)
        .with_circuit(mul)
        .with_circuit(div)
        .noise_margin_bits(32)
        .plain_modulus_constraint(PlainModulusConstraint::Raw(1_000_000))
        .compile()
        .unwrap();

and the compiler will run a parameter search that satisfies the noise margin for all circuits. What exists today highlights that you cannot currently do this.

[nit] destructure!

I'm currently forget to destructuring structs; I only remember to do it for tuples.

There's a lot happening here so let's try to break it down
See question below on shadowing.

The rationale behind Bob storing the ciphertext is to show that you can feed ciphertexts that come out of one circuit into another without Alice decrypting the result (so long as the params used in compilation are identical). This will be an important scenario in some applications.

The drain is so we can move the Ciphertext out of the Vec without a clone before we sent it back to Alice. This saves copying maybe 100kb of data, but I think just cloning again is probably cleaner. We do still need to clone it once for ans.

result is defined 3 times here.

Are there any guidelines on when and when not to use shadowing in production Rust? I personally like using shadowing for successive processing of a thing. As per the Rust book:

Shadowing thus spares us from having to come up with different names, such as spaces_str and spaces_num; instead, we can reuse the simpler spaces name.

Admittedly, this is a Rustism and might be confusing to people coming from languages where this is illegal (i.e. most of them).