Failure evidence and correctness proofs #129
Conversation
| #[derive(Debug, Clone)] | ||
| pub struct Evidence<Res: ProtocolResult<Verifier>, Sig, Verifier> { | ||
| party: Verifier, | ||
| result: Res::ProvableError, |
There was a problem hiding this comment.
Why call this a "result"? It seems more like it's used to build up a trace and a context to uphold
the evidence. Would calling this "trace" work?
There was a problem hiding this comment.
Naming is kind of messy for now, must have been an oversight.
| Res: ProtocolResult<Verifier>, | ||
| Sig: Clone + for<'de> Deserialize<'de>, | ||
| Verifier: Clone + Ord, | ||
| > Evidence<Res, Sig, Verifier> |
There was a problem hiding this comment.
Total nitpick, but I would prefer having the bounds in a where-clause, especially when there are
lots of them like here.
There was a problem hiding this comment.
I wonder if there's some rustfmt setting where we can formalize it. In this case I agree, it should be in the outer clause.
|
|
||
| impl< | ||
| Res: ProtocolResult<Verifier>, | ||
| Sig: Clone + for<'de> Deserialize<'de>, |
There was a problem hiding this comment.
Remind me again, Sig is usually read "Signature" yes?
There was a problem hiding this comment.
Yes. I'm not particularly happy with this abbreviation, but I didn't come up with anything better. I would prefer to use single letters, but unfortunately Signer and Signature get mixed up.
|
|
||
| /// A trait specifying which messages the evidence needs to prove a party's fault, | ||
| /// and how to do it. | ||
| // TODO: rename this |
There was a problem hiding this comment.
A few ideas: EvidenceContext, EvidenceTrace, BehaviorTranscript, BehaviorTrace,
EvidenceData, InteractionTrace…
| /// A trait specifying which messages the evidence needs to prove a party's fault, | ||
| /// and how to do it. | ||
| // TODO: rename this | ||
| // TODO (#74): this trait should not be visible to the user, |
There was a problem hiding this comment.
Either not pub, or at least sealed (I think it'll have to be public).
| malicious_round!(MRound3, Round3); | ||
|
|
||
| malicious_to_next_round!(MRound1, MRound2); | ||
| malicious_to_next_round!(MRound2, MRound3); |
There was a problem hiding this comment.
There's a bug in the macro I think: it does not allow me to specify the type with a path, e.g. crate::some::path::to::MyType.
There was a problem hiding this comment.
Yeah I don't have a lot of experience with macros and macro hygiene. I'll look into it.
| #[test] | ||
| fn execute_keygen() { | ||
| let mut shared_randomness = [0u8; 32]; |
There was a problem hiding this comment.
I think this still allows users to do silly things like e.g.:
malicious_to_next_round!(MRound1, Round2); // <–– This shouldn't be allowed right?
malicious_to_next_round!(MRound2, MRound3);
malicious_to_result!(Round1); // <–– makes no senseThere was a problem hiding this comment.
Not sure if it's possible to prevent this.
There was a problem hiding this comment.
Not without a lot of types and work. And I don't think it's worth it. A proc-macro could look at what the allowed type combinations are and make sure that the rounds fit together but I suspect it would be brittle and awkward to work with.
| let mut shared_randomness = [0u8; 32]; | ||
| OsRng.fill_bytes(&mut shared_randomness); | ||
|
|
||
| let ids = BTreeSet::from([Id(0), Id(1), Id(2)]); |
There was a problem hiding this comment.
Should this be called execute_keyinit?
There was a problem hiding this comment.
Yes, you're right. Copy-paste artifact.
| return Err(KeyInitError::R3InvalidSchProof); | ||
| return Err(KeyInitError { | ||
| error: KeyInitErrorType::R3InvalidSchProof, | ||
| phantom: (PhantomData, PhantomData), |
There was a problem hiding this comment.
To make it easier to read perhaps we can spell it out like so:
| phantom: (PhantomData, PhantomData), | |
| phantom: (PhantomData::<P>, PhantomData::<I>), |
| let mut bitvecs = bitvecs; | ||
| let mut result = bitvecs.next().unwrap().clone(); | ||
| for bitvec in bitvecs { | ||
| result ^= bitvec; | ||
| } | ||
| result |
There was a problem hiding this comment.
Would this work here?
bitvecs.fold(Self::default(), |mut result, bitvec| {
result ^= bitvec;
result
})There was a problem hiding this comment.
Yes, that looks better. Maybe should even go into a BitVec method.
No description provided.