Skip to content

Commit 81abd15

Browse files
committed
Code review
1 parent bf3b710 commit 81abd15

File tree

5 files changed

+158
-145
lines changed

5 files changed

+158
-145
lines changed

src/backends/plonky2/mock_main/mod.rs

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -227,32 +227,38 @@ impl MockMainPod {
227227

228228
fn find_op_arg(
229229
statements: &[Statement],
230-
merkle_proofs: &[MerkleProof],
231-
op_arg: &middleware::OperationArg,
230+
op_arg: &middleware::Statement,
232231
) -> Result<OperationArg> {
233232
match op_arg {
234-
middleware::OperationArg::Statement(s_arg) => match s_arg {
235-
middleware::Statement::None => Ok(OperationArg::None),
236-
_ => statements
237-
.iter()
238-
.enumerate()
239-
.find_map(|(i, s)| {
240-
(&middleware::Statement::try_from(s.clone()).ok()? == s_arg).then_some(i)
241-
})
242-
.map(OperationArg::Index)
243-
.ok_or(anyhow!(
244-
"Statement corresponding to op arg {} not found",
245-
op_arg
246-
)),
247-
},
248-
middleware::OperationArg::MerkleProof(pf_arg) => merkle_proofs
233+
middleware::Statement::None => Ok(OperationArg::None),
234+
_ => statements
235+
.iter()
236+
.enumerate()
237+
.find_map(|(i, s)| {
238+
(&middleware::Statement::try_from(s.clone()).ok()? == op_arg).then_some(i)
239+
})
240+
.map(OperationArg::Index)
241+
.ok_or(anyhow!(
242+
"Statement corresponding to op arg {} not found",
243+
op_arg
244+
)),
245+
}
246+
}
247+
248+
fn find_op_aux(
249+
merkle_proofs: &[MerkleProof],
250+
op_aux: &middleware::OperationAux,
251+
) -> Result<OperationAux> {
252+
match op_aux {
253+
middleware::OperationAux::None => Ok(OperationAux::None),
254+
middleware::OperationAux::MerkleProof(pf_arg) => merkle_proofs
249255
.iter()
250256
.enumerate()
251257
.find_map(|(i, pf)| (pf == pf_arg).then_some(i))
252-
.map(OperationArg::MerkleProofIndex)
258+
.map(OperationAux::MerkleProofIndex)
253259
.ok_or(anyhow!(
254260
"Merkle proof corresponding to op arg {} not found",
255-
op_arg
261+
op_aux
256262
)),
257263
}
258264
}
@@ -272,10 +278,14 @@ impl MockMainPod {
272278
let mid_args = op.args();
273279
let mut args = mid_args
274280
.iter()
275-
.map(|mid_arg| Self::find_op_arg(statements, merkle_proofs, mid_arg))
281+
.map(|mid_arg| Self::find_op_arg(statements, mid_arg))
276282
.collect::<Result<Vec<_>>>()?;
283+
284+
let mid_aux = op.aux();
285+
let aux = Self::find_op_aux(merkle_proofs, &mid_aux)?;
286+
277287
Self::pad_operation_args(params, &mut args);
278-
operations.push(Operation(op.code(), args));
288+
operations.push(Operation(op.code(), args, aux));
279289
}
280290
Ok(operations)
281291
}
@@ -294,21 +304,22 @@ impl MockMainPod {
294304
operations.push(Operation(
295305
OperationType::Native(NativeOperation::NewEntry),
296306
vec![],
307+
OperationAux::None,
297308
));
298309
for i in 0..(params.max_public_statements - 1) {
299310
let st = &statements[offset_public_statements + i + 1];
300311
let mut op = if st.is_none() {
301-
Operation(OperationType::Native(NativeOperation::None), vec![])
312+
Operation(
313+
OperationType::Native(NativeOperation::None),
314+
vec![],
315+
OperationAux::None,
316+
)
302317
} else {
303318
let mid_arg = st.clone();
304319
Operation(
305320
OperationType::Native(NativeOperation::CopyStatement),
306-
// TODO
307-
vec![Self::find_op_arg(
308-
statements,
309-
merkle_proofs,
310-
&middleware::OperationArg::Statement(mid_arg.try_into().unwrap()),
311-
)?],
321+
vec![Self::find_op_arg(statements, &mid_arg.try_into()?)?],
322+
OperationAux::None,
312323
)
313324
};
314325
fill_pad(&mut op.1, OperationArg::None, params.max_operation_args);
@@ -388,7 +399,11 @@ impl MockMainPod {
388399
}
389400

390401
fn operation_none(params: &Params) -> Operation {
391-
let mut op = Operation(OperationType::Native(NativeOperation::None), vec![]);
402+
let mut op = Operation(
403+
OperationType::Native(NativeOperation::None),
404+
vec![],
405+
OperationAux::None,
406+
);
392407
fill_pad(&mut op.1, OperationArg::None, params.max_operation_args);
393408
op
394409
}

src/backends/plonky2/mock_main/operation.rs

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use anyhow::Result;
1+
use anyhow::{anyhow, Result};
22
use std::fmt;
33

44
use super::Statement;
@@ -11,7 +11,6 @@ use crate::{
1111
pub enum OperationArg {
1212
None,
1313
Index(usize),
14-
MerkleProofIndex(usize),
1514
}
1615

1716
impl OperationArg {
@@ -21,7 +20,13 @@ impl OperationArg {
2120
}
2221

2322
#[derive(Clone, Debug, PartialEq, Eq)]
24-
pub struct Operation(pub OperationType, pub Vec<OperationArg>);
23+
pub enum OperationAux {
24+
None,
25+
MerkleProofIndex(usize),
26+
}
27+
28+
#[derive(Clone, Debug, PartialEq, Eq)]
29+
pub struct Operation(pub OperationType, pub Vec<OperationArg>, pub OperationAux);
2530

2631
impl Operation {
2732
pub fn deref(
@@ -34,18 +39,18 @@ impl Operation {
3439
.iter()
3540
.flat_map(|arg| match arg {
3641
OperationArg::None => None,
37-
OperationArg::Index(i) => Some(
38-
statements[*i]
39-
.clone()
40-
.try_into()
41-
.map(|s| crate::middleware::OperationArg::Statement(s)),
42-
),
43-
OperationArg::MerkleProofIndex(i) => Some(Ok(
44-
crate::middleware::OperationArg::MerkleProof(merkle_proofs[*i].clone()),
45-
)),
42+
OperationArg::Index(i) => Some(statements[*i].clone().try_into()),
4643
})
4744
.collect::<Result<Vec<_>>>()?;
48-
middleware::Operation::op(self.0.clone(), &deref_args)
45+
let deref_aux = match self.2 {
46+
OperationAux::None => Ok(crate::middleware::OperationAux::None),
47+
OperationAux::MerkleProofIndex(i) => merkle_proofs
48+
.get(i)
49+
.cloned()
50+
.ok_or(anyhow!("Missing Merkle proof index {}", i))
51+
.map(crate::middleware::OperationAux::MerkleProof),
52+
}?;
53+
middleware::Operation::op(self.0.clone(), &deref_args, &deref_aux)
4954
}
5055
}
5156

@@ -60,10 +65,13 @@ impl fmt::Display for Operation {
6065
match arg {
6166
OperationArg::None => write!(f, "none")?,
6267
OperationArg::Index(i) => write!(f, "{:02}", i)?,
63-
OperationArg::MerkleProofIndex(i) => write!(f, "merkle_proof_{:02}", i)?,
6468
}
6569
}
6670
}
71+
match self.2 {
72+
OperationAux::None => (),
73+
OperationAux::MerkleProofIndex(i) => write!(f, "merkle_proof_{:02}", i)?,
74+
}
6775
Ok(())
6876
}
6977
}

src/frontend/mod.rs

Lines changed: 35 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::middleware::{
1212
containers::{Array, Dictionary, Set},
1313
hash_str, Hash, MainPodInputs, Params, PodId, PodProver, PodSigner, SELF,
1414
};
15-
use crate::middleware::{hash_value, EMPTY_VALUE, KEY_SIGNER, KEY_TYPE};
15+
use crate::middleware::{hash_value, OperationAux, EMPTY_VALUE, KEY_SIGNER, KEY_TYPE};
1616

1717
mod custom;
1818
mod operation;
@@ -348,8 +348,6 @@ impl MainPodBuilder {
348348
)));
349349
st_args.push(StatementArg::Literal(v.clone()))
350350
}
351-
// Merkle proofs are never arguments to statements.
352-
OperationArg::MerkleProof(_) => (),
353351
};
354352
}
355353
Ok(st_args)
@@ -365,7 +363,7 @@ impl MainPodBuilder {
365363

366364
fn op(&mut self, public: bool, mut op: Operation) -> Result<Statement, anyhow::Error> {
367365
use NativeOperation::*;
368-
let Operation(op_type, ref mut args) = &mut op;
366+
let Operation(op_type, ref mut args, _) = &mut op;
369367
// TODO: argument type checking
370368
let pred = op_type
371369
.output_predicate()
@@ -673,6 +671,7 @@ impl MainPodBuilder {
673671
Operation(
674672
OperationType::Native(NativeOperation::NewEntry),
675673
vec![OperationArg::Entry(k.clone(), v)],
674+
OperationAux::None,
676675
),
677676
)
678677
}
@@ -820,12 +819,9 @@ impl MainPodCompiler {
820819
self.operations.push(op);
821820
}
822821

823-
fn compile_op_arg(&self, op_arg: &OperationArg) -> Option<middleware::OperationArg> {
822+
fn compile_op_arg(&self, op_arg: &OperationArg) -> Option<middleware::Statement> {
824823
match op_arg {
825-
OperationArg::Statement(s) => self
826-
.compile_st(s)
827-
.ok()
828-
.map(|s| middleware::OperationArg::Statement(s)),
824+
OperationArg::Statement(s) => self.compile_st(s).ok(),
829825
OperationArg::Literal(_v) => {
830826
// OperationArg::Literal is a syntax sugar for the frontend. This is translated to
831827
// a new ValueOf statement and it's key used instead.
@@ -837,9 +833,6 @@ impl MainPodCompiler {
837833
// statement doesn't have any requirement on the key and value.
838834
None
839835
}
840-
OperationArg::MerkleProof(pf) => {
841-
Some(middleware::OperationArg::MerkleProof(pf.clone()))
842-
}
843836
}
844837
}
845838

@@ -903,11 +896,11 @@ impl MainPodCompiler {
903896
_ => Err(anyhow!("Statement compile failed in manual compile"))?,
904897
},
905898
empty_st,
906-
match &op.1[2] {
907-
OperationArg::MerkleProof(mp) => mp.clone(),
899+
match &op.2 {
900+
OperationAux::MerkleProof(mp) => mp.clone(),
908901
_ => {
909902
return Err(anyhow!(
910-
"Third argument to DictContainsFromEntries must be Merkle proof"
903+
"Auxiliary argument to DictContainsFromEntries must be Merkle proof"
911904
));
912905
}
913906
},
@@ -968,7 +961,7 @@ impl MainPodCompiler {
968961
op.1.iter()
969962
.flat_map(|arg| self.compile_op_arg(arg).map(|op_arg| Ok(op_arg)))
970963
.collect::<Result<Vec<_>>>()?;
971-
middleware::Operation::op(mop_code, &mop_args)
964+
middleware::Operation::op(mop_code, &mop_args, &op.2)
972965
}
973966

974967
fn compile_st_op(&mut self, st: &Statement, op: &Operation, params: &Params) -> Result<()> {
@@ -1038,56 +1031,55 @@ pub mod build_utils {
10381031
macro_rules! op {
10391032
(new_entry, ($key:expr, $value:expr)) => { $crate::frontend::Operation(
10401033
$crate::frontend::OperationType::Native($crate::frontend::NativeOperation::NewEntry),
1041-
$crate::op_args!(($key, $value))) };
1034+
$crate::op_args!(($key, $value)), crate::middleware::OperationAux::None) };
10421035
(eq, $($arg:expr),+) => { $crate::frontend::Operation(
10431036
$crate::frontend::OperationType::Native($crate::frontend::NativeOperation::EqualFromEntries),
1044-
$crate::op_args!($($arg),*)) };
1037+
$crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10451038
(ne, $($arg:expr),+) => { $crate::frontend::Operation(
10461039
$crate::frontend::OperationType::Native($crate::frontend::NativeOperation::NotEqualFromEntries),
1047-
$crate::op_args!($($arg),*)) };
1040+
$crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10481041
(gt, $($arg:expr),+) => { crate::frontend::Operation(
10491042
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::GtFromEntries),
1050-
crate::op_args!($($arg),*)) };
1043+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10511044
(lt, $($arg:expr),+) => { crate::frontend::Operation(
10521045
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::LtFromEntries),
1053-
crate::op_args!($($arg),*)) };
1046+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10541047
(transitive_eq, $($arg:expr),+) => { crate::frontend::Operation(
10551048
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::TransitiveEqualFromStatements),
1056-
crate::op_args!($($arg),*)) };
1049+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10571050
(gt_to_ne, $($arg:expr),+) => { crate::frontend::Operation(
10581051
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::GtToNotEqual),
1059-
crate::op_args!($($arg),*)) };
1052+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10601053
(lt_to_ne, $($arg:expr),+) => { crate::frontend::Operation(
10611054
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::LtToNotEqual),
1062-
crate::op_args!($($arg),*)) };
1055+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10631056
(sum_of, $($arg:expr),+) => { crate::frontend::Operation(
10641057
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::SumOf),
1065-
crate::op_args!($($arg),*)) };
1058+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10661059
(product_of, $($arg:expr),+) => { crate::frontend::Operation(
10671060
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::ProductOf),
1068-
crate::op_args!($($arg),*)) };
1061+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10691062
(max_of, $($arg:expr),+) => { crate::frontend::Operation(
10701063
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::MaxOf),
1071-
crate::op_args!($($arg),*)) };
1064+
crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
10721065
(custom, $op:expr, $($arg:expr),+) => { $crate::frontend::Operation(
10731066
$crate::frontend::OperationType::Custom($op),
1074-
$crate::op_args!($($arg),*)) };
1075-
(dict_contains, $($arg:expr),+) => { crate::frontend::Operation(
1067+
$crate::op_args!($($arg),*), crate::middleware::OperationAux::None) };
1068+
(dict_contains, $dict:expr, $key:expr, $value:expr, $aux:expr) => { crate::frontend::Operation(
10761069
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::DictContainsFromEntries),
1077-
crate::op_args!($($arg),*)) };
1078-
(dict_not_contains, $($arg:expr),+) => { crate::frontend::Operation(
1070+
crate::op_args!($dict, $key, $value), crate::middleware::OperationAux::MerkleProof($aux)) };
1071+
(dict_not_contains, $dict:expr, $key:expr, $aux:expr) => { crate::frontend::Operation(
10791072
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::DictNotContainsFromEntries),
1080-
crate::op_args!($($arg),*)) };
1081-
(set_contains, $($arg:expr),+) => { crate::frontend::Operation(
1073+
crate::op_args!($dict, $key), crate::middleware::OperationAux::MerkleProof($aux)) };
1074+
(set_contains, $set:expr, $value:expr, $aux:expr) => { crate::frontend::Operation(
10821075
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::SetContainsFromEntries),
1083-
crate::op_args!($($arg),*)) };
1084-
(set_not_contains, $($arg:expr),+) => { crate::frontend::Operation(
1076+
crate::op_args!($set, $value), crate::middleware::OperationAux::MerkleProof($aux)) };
1077+
(set_not_contains, $set:expr, $value:expr, $aux:expr) => { crate::frontend::Operation(
10851078
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::SetNotContainsFromEntries),
1086-
crate::op_args!($($arg),*)) };
1087-
(array_contains, $($arg:expr),+) => { crate::frontend::Operation(
1079+
crate::op_args!($set, $value), crate::middleware::OperationAux::MerkleProof($aux)) };
1080+
(array_contains, $array:expr, $value:expr, $aux:expr) => { crate::frontend::Operation(
10881081
crate::frontend::OperationType::Native(crate::frontend::NativeOperation::ArrayContainsFromEntries),
1089-
crate::op_args!($($arg),*)) };
1090-
1082+
crate::op_args!($array, $value), crate::middleware::OperationAux::MerkleProof($aux)) };
10911083
}
10921084
}
10931085

@@ -1268,6 +1260,7 @@ pub mod tests {
12681260
OperationArg::from((&signed_pod, "a")),
12691261
OperationArg::from((&signed_pod, "b")),
12701262
],
1263+
OperationAux::None,
12711264
);
12721265
let st1 = builder.op(true, op_eq1).unwrap();
12731266
let op_eq2 = Operation(
@@ -1276,12 +1269,14 @@ pub mod tests {
12761269
OperationArg::from((&signed_pod, "b")),
12771270
OperationArg::from((&signed_pod, "a")),
12781271
],
1272+
OperationAux::None,
12791273
);
12801274
let st2 = builder.op(true, op_eq2).unwrap();
12811275

12821276
let op_eq3 = Operation(
12831277
OperationType::Native(NativeOperation::TransitiveEqualFromStatements),
12841278
vec![OperationArg::Statement(st1), OperationArg::Statement(st2)],
1279+
OperationAux::None,
12851280
);
12861281
let st3 = builder.op(true, op_eq3);
12871282

@@ -1352,8 +1347,8 @@ pub mod tests {
13521347
OperationArg::Statement(st0),
13531348
OperationArg::Statement(st1),
13541349
OperationArg::Statement(st2),
1355-
OperationArg::MerkleProof(dict.prove(&Hash::from("a").into()).unwrap().1),
13561350
],
1351+
OperationAux::MerkleProof(dict.prove(&Hash::from("a").into()).unwrap().1),
13571352
))
13581353
.unwrap();
13591354
let mut main_prover = MockProver {};

0 commit comments

Comments
 (0)