Merged
Conversation
6 tasks
Contributor
|
Thanks for the delivery. We'll review it as soon as we can! |
Contributor
|
@rubyprotocol @mmagician Hi, I'm Mike Carroll, the new kid on the block, helping Marcin and Robert review the ruby protocol milestone deliverables. Hope you don't mind a few newbie questions / comments. I've reviewed most of the high level artifacts and am now jumping into the code a little bit. BTW, I really like your architecture and your concept. FE and SMCP seem to be key technology enablers for data privacy and monetization. |
Contributor
|
@rubyprotocol @mmagician Also, I ran the cargo test test_sip as per the unit test guide and got 0 passed 0 failed 0 ignored. successes: test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.96s running 0 tests successes: successes: test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out; finished in 0.00s running 0 tests successes: successes: test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 7 filtered out; finished in 0.00s Still, it seems to be very light testing. Perhaps you could document your plan for more robust testing in the user test guide. Thoughts? |
Contributor
|
@rubyprotocol @mmagician Likewise I got 0 results for all the other three tests |
Contributor
|
@rubyprotocol @mmagician Since the FE components are basically a rewrite in Rust of the previous C implementation, wouldn't it make sense to re-use some of the same validation tests that were used on the C implementation to ensure that the same results are obtained? |
Contributor
|
@rubyprotocol @mmagician @Lederstrumpf @lornacrevelingwgo23 The Build and Run instructions in the user guide begin with these lines: Install Rustcurl --tlsv1.2 https://sh.rustup.rs -sSf | sh Build the projectcargo build However, since some of the same code and directory structure is duplicated in the Functional_encryption project, perhaps it would be helpful if you included here the specific github clone instructions before the build command. E.g., the above snippet could read: Install Rustcurl --tlsv1.2 https://sh.rustup.rs -sSf | sh Clone the project:git clone https://github.com/Ruby-Protocol/private_ml.git Build the projectcargo build My mistake above was to download the Functional_encryption project and run the cargo test commands on that, only to get 0 results. The results now being obtained are looking better (see previous posts of mine). |
Contributor
|
@rubyprotocol @mmagician @Lederstrumpf |
Hey @musicarroll , thanks for your comments and sorry for the late response. I haven't checked this pr for a while and only see your comments today. Will get back to you soon regarding your comments. |
Contributor
|
@lornacrevelingwgo23 @rubyprotocol I've merged @musicarroll 's evaluation as "In progress" for now, and submitted my own first round of evaluation here. Please address the issues raised in both evaluations in order to proceed further. |
Contributor
|
@lornacrevelingwgo23 Any updates regarding the feedback you received on your milestone? |
@mmagician Sorry for the late response. Several members of our team were out of the office the last few weeks, which caused the delay. We have updated the private ml repo and modified the code and relevant documents. I have also provided a reponse to your evaluation, in which we have addressed your raised issues one by one. Please let me know if there is any question. Thanks. |
Contributor
|
Thanks for the updates. There are still a few things:
|
Thanks for the prompt response. Will get back to you on this ASAP. |
Thanks for your comments. We have updated the code (https://github.com/Ruby-Protocol/private_ml) according to your advice. Please let me know if there are any other problems. |
Contributor
|
Hi @lornacrevelingwgo23, I'm checking your updates as Marcin is out of office. I see that you have addressed all evaluation notes either in your response document or here. I just noticed two things:
|
Contributor
Author
Hey, thanks for your comments. Will check with my team and see if we can further reduce the warnings.
Already update the delivery document, and here is the link to the updated document: https://github.com/Ruby-Protocol/Grant-Milestone-Delivery/blob/master/deliveries/ruby_milestone_1.md. Please let me know if there are any other problems. |
@semuelle Would you mind telling me exactly what type of operating system and rust version you are currently running? We test our code on MacOS Big Sur version 11.6 and our Rust cargo version is 1.51.0. We haven't received any warning when we run our code in this setting. Is it possible the warnings are caused by older-version Rust? |
Contributor
Ubuntu 20.04 Here's the output for completeness: ❯ cargo clippy
warning: name `GT` contains a capitalized acronym
--> src/define.rs:16:10
|
16 | pub type GT = FP12;
| ^^ help: consider making the acronym lowercase, except the initial letter: `Gt`
|
= note: `#[warn(clippy::upper_case_acronyms)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: name `RandUtilsRAND` contains a capitalized acronym
--> src/utils/rand_utils.rs:26:12
|
26 | pub struct RandUtilsRAND {
| ^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `RandUtilsRand`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: 7 bindings with single-character names in scope
--> src/utils/mod.rs:61:29
|
61 | pub fn baby_step_giant_step(h: >, g: >, bound: &BigNum) -> Option<BigInt> {
| ^ ^
...
69 | let b = BigInt::from_str_radix(&bound.tostring(), 16).unwrap();
| ^
...
72 | let m = BigNum::fromstring(temp.to_str_radix(16));
| ^
...
75 | let (mut x, mut z) = (GT::new(), GT::new_copy(&g));
| ^ ^
76 | let mut i = BigNum::new_int(0);
| ^
|
= note: `#[warn(clippy::many_single_char_names)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
warning: 7 bindings with single-character names in scope
--> src/utils/mod.rs:61:29
|
61 | pub fn baby_step_giant_step(h: >, g: >, bound: &BigNum) -> Option<BigInt> {
| ^ ^
...
69 | let b = BigInt::from_str_radix(&bound.tostring(), 16).unwrap();
| ^
...
72 | let m = BigNum::fromstring(temp.to_str_radix(16));
| ^
...
75 | let (mut x, mut z) = (GT::new(), GT::new_copy(&g));
| ^ ^
76 | let mut i = BigNum::new_int(0);
| ^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
warning: 7 bindings with single-character names in scope
--> src/utils/mod.rs:127:32
|
127 | pub fn baby_step_giant_step_g1(h: &G1, g: &G1, bound: &BigNum) -> Option<BigInt> {
| ^ ^
...
134 | let b = BigInt::from_str_radix(&bound.tostring(), 16).unwrap();
| ^
...
137 | let m = BigNum::fromstring(temp.to_str_radix(16));
| ^
...
140 | let (mut x, mut z) = (G1::new(), g.clone());
| ^ ^
141 | let mut i = BigNum::new_int(0);
| ^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
warning: 5 bindings with single-character names in scope
--> src/quadratic_sgp.rs:103:14
|
103 | let (x, y) = (&plain.x, &plain.y);
| ^ ^
...
110 | let w = BigNumMatrix2x2::new_random(&(CURVE_ORDER));
| ^
...
118 | let mut a: G1Vector = vec![G1::generator(); L * 2];
| ^
119 | let mut b: G2Vector = vec![G2::generator(); L * 2];
| ^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
warning: name `JJParams` contains a capitalized acronym
--> src/zk/dlog.rs:21:6
|
21 | type JJParams = JubJubBN256;
| ^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `JjParams`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: redundant field names in struct initialization
--> src/zk/dlog.rs:59:13
|
59 | inputs: inputs,
| ^^^^^^^^^^^^^^ help: replace it with: `inputs`
|
= note: `#[warn(clippy::redundant_field_names)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
warning: name `JJParams` contains a capitalized acronym
--> src/zk/qp.rs:29:6
|
29 | type JJParams = JubJubBN256;
| ^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `JjParams`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: name `QPProofPublic` contains a capitalized acronym
--> src/zk/qp.rs:33:12
|
33 | pub struct QPProofPublic<Fr: PrimeField, const L: usize> {
| ^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `QpProofPublic`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: name `CQPProofPublic` contains a capitalized acronym
--> src/zk/qp.rs:44:12
|
44 | pub struct CQPProofPublic<C: CS, const L: usize> {
| ^^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `CqpProofPublic`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: name `QPProofSecret` contains a capitalized acronym
--> src/zk/qp.rs:54:12
|
54 | pub struct QPProofSecret<Fr: PrimeField, const L: usize> {
| ^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `QpProofSecret`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: name `CQPProofSecret` contains a capitalized acronym
--> src/zk/qp.rs:63:12
|
63 | pub struct CQPProofSecret<C: CS, const L: usize> {
| ^^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `CqpProofSecret`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: redundant field names in struct initialization
--> src/zk/qp.rs:160:13
|
160 | inputs: inputs,
| ^^^^^^^^^^^^^^ help: replace it with: `inputs`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
warning: name `JJParams` contains a capitalized acronym
--> src/zk/sip.rs:24:6
|
24 | type JJParams = JubJubBN256;
| ^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `JjParams`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
warning: 5 bindings with single-character names in scope
--> src/zk/sip.rs:107:21
|
107 | pub fn generate(g: &EdwardsPoint<Fr>, h: &EdwardsPoint<Fr>, s: &SizedVec<Num<Fr>, L>, y: &SizedVec<Num<Fr>, L>) -> SnarkInfo<E> {
| ^ ^ ^ ^
...
111 | let r: Num<Fr> = rng.gen();
| ^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
warning: 6 bindings with single-character names in scope
--> src/zk/sip.rs:107:21
|
107 | pub fn generate(g: &EdwardsPoint<Fr>, h: &EdwardsPoint<Fr>, s: &SizedVec<Num<Fr>, L>, y: &SizedVec<Num<Fr>, L>) -> SnarkInfo<E> {
| ^ ^ ^ ^
...
111 | let r: Num<Fr> = rng.gen();
| ^
...
129 | let v = s.iter()
| ^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
warning: redundant field names in struct initialization
--> src/zk/sip.rs:149:13
|
149 | inputs: inputs,
| ^^^^^^^^^^^^^^ help: replace it with: `inputs`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
warning: using `clone` on type `miracl_core::bls12381::big::BIG` which implements the `Copy` trait
--> src/math/matrix.rs:40:22
|
40 | modulus: modulus.clone()
| ^^^^^^^^^^^^^^^ help: try dereferencing it: `*modulus`
|
= note: `#[warn(clippy::clone_on_copy)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: using `clone` on type `miracl_core::bls12381::big::BIG` which implements the `Copy` trait
--> src/math/matrix.rs:55:22
|
55 | modulus: modulus.clone()
| ^^^^^^^^^^^^^^^ help: try dereferencing it: `*modulus`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: using `clone` on type `miracl_core::bls12381::big::BIG` which implements the `Copy` trait
--> src/math/matrix.rs:69:22
|
69 | modulus: modulus.clone()
| ^^^^^^^^^^^^^^^ help: try dereferencing it: `*modulus`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: you should consider adding a `Default` implementation for `math::matrix::BigNumMatrix2x2`
--> src/math/matrix.rs:121:5
|
121 | / pub fn new() -> Self {
122 | | Self {
123 | | data: vec![BigNum::new(); 2 * 2],
124 | | }
125 | | }
| |_____^
|
= note: `#[warn(clippy::new_without_default)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
help: try this
|
120 | impl Default for math::matrix::BigNumMatrix2x2 {
121 | fn default() -> Self {
122 | Self::new()
123 | }
124 | }
|
warning: you should consider adding a `Default` implementation for `math::matrix::BigIntMatrix2x2`
--> src/math/matrix.rs:277:5
|
277 | / pub fn new() -> Self {
278 | | Self {
279 | | data: vec![BigInt::from(0); 2 * 2],
280 | | }
281 | | }
| |_____^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
help: try this
|
276 | impl Default for math::matrix::BigIntMatrix2x2 {
277 | fn default() -> Self {
278 | Self::new()
279 | }
280 | }
|
warning: you should consider adding a `Default` implementation for `utils::rand_utils::RandUtilsRAND`
--> src/utils/rand_utils.rs:32:5
|
32 | / pub fn new() -> Self {
33 | | Self {
34 | | rng: Self::get_rng()
35 | | }
36 | | }
| |_____^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
help: try this
|
31 | impl Default for utils::rand_utils::RandUtilsRAND {
32 | fn default() -> Self {
33 | Self::new()
34 | }
35 | }
|
warning: unneeded `return` statement
--> src/utils/rand_utils.rs:46:9
|
46 | return Self::get_seeded_rng(&seed);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `Self::get_seeded_rng(&seed)`
|
= note: `#[warn(clippy::needless_return)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
warning: you should consider adding a `Default` implementation for `utils::rand_utils::RandUtilsRng`
--> src/utils/rand_utils.rs:95:5
|
95 | / pub fn new() -> Self {
96 | | Self {
97 | | rng: Self::get_rng()
98 | | }
99 | | }
| |_____^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
help: try this
|
94 | impl Default for utils::rand_utils::RandUtilsRng {
95 | fn default() -> Self {
96 | Self::new()
97 | }
98 | }
|
warning: unneeded `return` statement
--> src/utils/rand_utils.rs:106:9
|
106 | return StdRng::from_entropy();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `StdRng::from_entropy()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
warning: returning the result of a `let` binding from a block
--> src/utils/rand_utils.rs:111:9
|
110 | let rng = StdRng::from_seed(*seed);
| ----------------------------------- unnecessary `let` binding
111 | rng
| ^^^
|
= note: `#[warn(clippy::let_and_return)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
|
110 |
111 | StdRng::from_seed(*seed)
|
warning: manual implementation of an assign operation
--> src/utils/mod.rs:54:9
|
54 | y = y + m;
| ^^^^^^^^^ help: replace it with: `y += m`
|
= note: `#[warn(clippy::assign_op_pattern)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
warning: manual implementation of an assign operation
--> src/utils/mod.rs:195:9
|
195 | res = res + tmp;
| ^^^^^^^^^^^^^^^ help: replace it with: `res += tmp`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
warning: the loop variable `i` is used to index `x`
--> src/utils/mod.rs:205:14
|
205 | for i in 0..x.len() {
| ^^^^^^^^^^
|
= note: `#[warn(clippy::needless_range_loop)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
205 | for (i, <item>) in x.iter().enumerate() {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `j` is used to index `y`
--> src/utils/mod.rs:206:18
|
206 | for j in 0..y.len() {
| ^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
206 | for (j, <item>) in y.iter().enumerate() {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
warning: manual implementation of an assign operation
--> src/utils/mod.rs:209:13
|
209 | res = res + tmp;
| ^^^^^^^^^^^^^^^ help: replace it with: `res += tmp`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
warning: the loop variable `i` is used to index `fe_key`
--> src/dmcfe_ip.rs:110:22
|
110 | for i in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
110 | for (i, <item>) in fe_key.iter_mut().enumerate().take(2) {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `i` is used to index `pub_keys`
--> src/dmcfe_ip.rs:199:18
|
199 | for i in 0..pub_keys.len() {
| ^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
199 | for (i, <item>) in pub_keys.iter().enumerate() {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `i` is only used to index `x`.
--> src/dmcfe_ip.rs:257:18
|
257 | for i in 0..x.len() {
| ^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
257 | for <item> in &x {
| ^^^^^^ ^^
warning: the loop variable `i` is used to index `hs`
--> src/dmcfe_ip.rs:279:18
|
279 | for i in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
279 | for (i, <item>) in hs.iter_mut().enumerate().take(2) {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `i` is used to index `fe_key_share`
--> src/dmcfe_ip.rs:288:18
|
288 | for i in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
288 | for (i, <item>) in fe_key_share.iter_mut().enumerate().take(2) {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `j` is used to index `hs`
--> src/dmcfe_ip.rs:290:22
|
290 | for j in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
290 | for (j, <item>) in hs.iter().enumerate().take(2) {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `i` is only used to index `keys_sum`.
--> src/dmcfe_ip.rs:317:18
|
317 | for i in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
317 | for <item> in keys_sum.iter_mut().take(2) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: the loop variable `j` is used to index `keys_sum`
--> src/dmcfe_ip.rs:322:22
|
322 | for j in 0..2 {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
322 | for (j, <item>) in keys_sum.iter_mut().enumerate().take(2) {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices.
--> src/dmcfe_ip.rs:347:18
|
347 | ciphers: &G1Vector,
| ^^^^^^^^^
|
= note: `#[warn(clippy::ptr_arg)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
warning: the loop variable `i` is used to index `ciphers`
--> src/dmcfe_ip.rs:359:18
|
359 | for i in 0..L {
| ^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
|
359 | for (i, <item>) in ciphers.iter().enumerate().take(L) {
| ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: using `clone` on type `miracl_core::bls12381::big::BIG` which implements the `Copy` trait
--> src/dmcfe_ip.rs:360:28
|
360 | let mut temp = dk.y[i].clone();
| ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `dk.y[i]`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: you should consider adding a `Default` implementation for `ml::disease_prediction::DiseasePrediction<'a>`
--> src/ml/disease_prediction.rs:29:5
|
29 | / pub fn new() -> Self {
30 | | let y1: [f32; 8] = [0.34362, 2.63588, 1.8803, 1.12673, -0.90941, 0.59397, 0.5232, 0.68602];
31 | | let y2: [f32; 8] = [0.48123, 3.39222, 1.39862, -0.00439, 0.16081, 0.99858, 0.19035, 0.49756];
32 | | let fe = Dmcfe::<8>::new();
... |
43 | | }
44 | | }
| |_____^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
help: try this
|
19 | impl Default for ml::disease_prediction::DiseasePrediction<'a> {
20 | fn default() -> Self {
21 | Self::new()
22 | }
23 | }
|
warning: returning the result of a `let` binding from a block
--> src/ml/disease_prediction.rs:62:9
|
61 | let ciphers = self.fe.encrypt(&int_x);
| -------------------------------------- unnecessary `let` binding
62 | ciphers
| ^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
|
61 |
62 | self.fe.encrypt(&int_x)
|
warning: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices.
--> src/ml/disease_prediction.rs:73:36
|
73 | pub fn compute(&self, ciphers: &G1Vector) -> Vec<f32> {
| ^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
warning: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices.
--> src/ml/neural_network.rs:40:37
|
40 | pub fn new(p: &BigIntMatrix, q: &Vec<BigIntMatrix>) -> Self {
| ^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
help: change this to
|
40 | pub fn new(p: &BigIntMatrix, q: &[BigIntMatrix]) -> Self {
| ^^^^^^^^^^^^^^^
help: change `q.clone()` to
|
45 | q: q.to_owned(),
| ^^^^^^^^^^^^
warning: returning the result of a `let` binding from a block
--> src/ml/neural_network.rs:64:9
|
63 | let cipher = self.sgp.encrypt(&plain);
| -------------------------------------- unnecessary `let` binding
64 | cipher
| ^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
|
63 |
64 | self.sgp.encrypt(&plain)
|
warning: returning the result of a `let` binding from a block
--> src/zk/dlog.rs:28:5
|
26 | let signal_y = g.mul(&signal_x_bits, params);
| --------------------------------------------- unnecessary `let` binding
27 |
28 | signal_y
| ^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
|
26 |
27 |
28 | g.mul(&signal_x_bits, params)
|
warning: using `clone` on type `fawkes_crypto::native::ecc::EdwardsPoint<fawkes_crypto::engines::bn256::Fr>` which implements the `Copy` trait
--> src/zk/qp.rs:143:17
|
143 | g1: g1.clone(),
| ^^^^^^^^^^ help: try dereferencing it: `*g1`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: using `clone` on type `fawkes_crypto::native::ecc::EdwardsPoint<fawkes_crypto::engines::bn256::Fr>` which implements the `Copy` trait
--> src/zk/qp.rs:144:17
|
144 | h1: h1.clone(),
| ^^^^^^^^^^ help: try dereferencing it: `*h1`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: manual implementation of an assign operation
--> src/zk/sip.rs:123:13
|
123 | ys = ys + (y[i] * s[i]);
| ^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `ys += (y[i] * s[i])`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
warning: using `clone` on type `fawkes_crypto::native::ecc::EdwardsPoint<fawkes_crypto::engines::bn256::Fr>` which implements the `Copy` trait
--> src/zk/sip.rs:134:16
|
134 | g: g.clone(),
| ^^^^^^^^^ help: try dereferencing it: `*g`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: using `clone` on type `fawkes_crypto::native::ecc::EdwardsPoint<fawkes_crypto::engines::bn256::Fr>` which implements the `Copy` trait
--> src/zk/sip.rs:135:16
|
135 | h: h.clone(),
| ^^^^^^^^^ help: try dereferencing it: `*h`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
warning: 55 warnings emitted
Finished dev [unoptimized + debuginfo] target(s) in 0.03s |
@semuelle Thanks for your prompt response. Please check the updated repo. We have fixed 30+ warnings, but the remaining clippy warnings are mainly about the issues that we would prefer to stay as they are, such as “loop variable i is used to index ***” and “writing &Vec<> instead of &[]” etc. It would make more sense to us to keep the relevant code (e.g., the loop variable is used to index multiple arrays, some of which are even multi-dimensional) unchanged, instead of fixing them according to the warnings one-by-one. Please let us know if you definitely want us to remove all the remaining warnings. We can definitely try to restructure the code although we think it might make the code a bit less readable. |
Contributor
@musicarroll Please check the updated readme for the concrete schemes of the benchmark results. We didn't show the results for the disease prediction here since they are entirely determined by the performance of the underlying algorithms.
Yes, it is correct that the computation time scales linearly with the vector size, and the vector size for real-world data monetization would very much depend on the concrete use cases. For instance, the vector size for CVD prediction is around 8, and that of MNIST dataset is close to 1000 according to our reference paper. |
We use randomly generated test data (as in the previous C implementation test), and basically have the same test results as the C-implementation, and validate the test results by comparing to the plaintext computation as in the C-implementation. |
Sorry for the omission. Already answer the two questions in here and here. |
Contributor
|
Thanks for the quick replies and fixes, @lornacrevelingwgo23. Your milestone is hereby accepted. Here are the evaluation notes. I will forward your invoice for processing. |
|
hi @lornacrevelingwgo23. The payment is transferred. |
Thanks a lot.
Many thanks for your prompt and very helpful comments. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Milestone Delivery Checklist
Link to the application pull request: w3f/Grants-Program#311.