Differential tests #14
Conversation
Autoparallel
left a comment
Autoparallel
left a comment
There was a problem hiding this comment.
Seems straightforward to me. I'd happily review this further if you submit any more changes :)
Autoparallel
left a comment
Autoparallel
left a comment
There was a problem hiding this comment.
I'm okay with merging this, but I would make an issue to go back and do the share_x as a genuine type instead of a feature flag.
Let's go ahead and get this in now though, that's a later problem. Low prio.
| #[cfg(feature = "share_x")] | ||
| { | ||
| values.push(share.clone()); | ||
| } | ||
| #[cfg(not(feature = "share_x"))] | ||
| { | ||
| values.push(ShareWithX { | ||
| x: GF256(i as u8 + 1), | ||
| y: share.y.clone(), | ||
| }); |
There was a problem hiding this comment.
This isn't a good way to do this unfortunately.
The reason why is because features in Rust are not ever meant to be mutually exclusive. Someone could enable both features and this won't work.
Instead, I'd just use generics and impl differently for each.
Share<WithOrder> and Share<WithCoordinate> are reasonable names, for example. Then you just have an impl block for each and some impl'd functions can probably be on Share<T> so you don't have to implement everything twice (especially if you had a trait for CoordinateType or something.
| #[cfg(feature = "share_x")] | ||
| { | ||
| values.push((share.x.clone(), share.clone())); | ||
| } | ||
| #[cfg(not(feature = "share_x"))] | ||
| { | ||
| values.push((GF256(i as u8 + 1), share.clone())); | ||
| } |
|
merging this. will handle share variant properly with #15 |
Changes required for differential testing with MFKDF
xcoordinate fromShare