1
0
Fork 0
mirror of synced 2025-09-23 12:18:44 +00:00

Merge pull request #1224 from Zokrates/private-parameter-check

Disallow the use of the private keyword on non-entrypoint functions
This commit is contained in:
Thibaut Schaeffer 2022-09-22 12:20:14 +02:00 committed by GitHub
commit 9e7f01ca2c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 134 additions and 72 deletions

View file

@ -0,0 +1 @@
Disallow the use of the `private` and `public` keywords on non-entrypoint functions

View file

@ -223,13 +223,10 @@ impl<'ast> From<pest::Parameter<'ast>> for untyped::ParameterNode<'ast> {
fn from(param: pest::Parameter<'ast>) -> untyped::ParameterNode<'ast> {
use crate::untyped::NodeValue;
let is_private = param
.visibility
.map(|v| match v {
pest::Visibility::Private(_) => true,
pest::Visibility::Public(_) => false,
})
.unwrap_or(false);
let is_private = param.visibility.map(|v| match v {
pest::Visibility::Private(_) => true,
pest::Visibility::Public(_) => false,
});
let is_mutable = param.mutable.is_some();
@ -949,9 +946,10 @@ mod tests {
.into(),
)
.into(),
untyped::Parameter::public(
untyped::Parameter::new(
untyped::Variable::mutable("b", UnresolvedType::Boolean.mock())
.into(),
.mock(),
None,
)
.into(),
],

View file

@ -4,20 +4,20 @@ use std::fmt;
#[derive(Clone, PartialEq)]
pub struct Parameter<'ast> {
pub id: VariableNode<'ast>,
pub is_private: bool,
pub is_private: Option<bool>,
}
impl<'ast> Parameter<'ast> {
pub fn new(v: VariableNode<'ast>, is_private: bool) -> Self {
pub fn new(v: VariableNode<'ast>, is_private: Option<bool>) -> Self {
Parameter { id: v, is_private }
}
pub fn private(v: VariableNode<'ast>) -> Self {
Self::new(v, true)
Self::new(v, Some(true))
}
pub fn public(v: VariableNode<'ast>) -> Self {
Self::new(v, false)
Self::new(v, Some(false))
}
}
@ -25,7 +25,12 @@ pub type ParameterNode<'ast> = Node<Parameter<'ast>>;
impl<'ast> fmt::Display for Parameter<'ast> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let visibility = if self.is_private { "private " } else { "" };
let visibility = if let Some(true) = self.is_private {
"private "
} else {
""
};
write!(
f,
"{}{} {}",

View file

@ -0,0 +1,8 @@
def mul(private field a) -> field { // `private` should not be allowed here
return a * a;
}
def main(private field a, field b) {
assert(mul(a) == b);
return;
}

View file

@ -0,0 +1,8 @@
def mul(public field a) -> field { // `public` should not be allowed here
return a * a;
}
def main(field a, field b) {
assert(mul(a) == b);
return;
}

View file

@ -89,6 +89,7 @@ type ConstantMap<'ast, T> =
/// The global state of the program during semantic checks
#[derive(Debug)]
struct State<'ast, T> {
main_id: OwnedModuleId,
/// The modules yet to be checked, which we consume as we explore the dependency tree
modules: Modules<'ast>,
/// The already checked modules, which we're returning at the end
@ -166,8 +167,9 @@ impl<'ast, T: std::cmp::Ord> SymbolUnifier<'ast, T> {
}
impl<'ast, T: Field> State<'ast, T> {
fn new(modules: Modules<'ast>) -> Self {
fn new(modules: Modules<'ast>, main_id: OwnedModuleId) -> Self {
State {
main_id,
modules,
typed_modules: BTreeMap::new(),
types: BTreeMap::new(),
@ -340,12 +342,13 @@ impl<'ast, T: Field> Checker<'ast, T> {
&mut self,
program: Program<'ast>,
) -> Result<TypedProgram<'ast, T>, Vec<Error>> {
let mut state = State::new(program.modules);
let main_id = program.main.clone();
let mut state = State::new(program.modules, main_id.clone());
let mut errors = vec![];
// recursively type-check modules starting with `main`
match self.check_module(&program.main, &mut state) {
match self.check_module(&main_id, &mut state) {
Ok(()) => {}
Err(e) => errors.extend(e),
};
@ -354,9 +357,7 @@ impl<'ast, T: Field> Checker<'ast, T> {
return Err(errors);
}
let main_id = program.main.clone();
Checker::check_single_main(state.typed_modules.get(&program.main).unwrap()).map_err(
Checker::check_single_main(state.typed_modules.get(&main_id).unwrap()).map_err(
|inner| {
vec![Error {
inner,
@ -744,7 +745,7 @@ impl<'ast, T: Field> Checker<'ast, T> {
}
}
Symbol::Here(SymbolDefinition::Function(f)) => {
match self.check_function(f, module_id, state) {
match self.check_function(declaration.id, f, module_id, state) {
Ok(funct) => {
match symbol_unifier
.insert_function(declaration.id, funct.signature.clone())
@ -1095,6 +1096,7 @@ impl<'ast, T: Field> Checker<'ast, T> {
fn check_function(
&mut self,
id: Identifier<'ast>,
funct_node: FunctionNode<'ast>,
module_id: &ModuleId,
state: &State<'ast, T>,
@ -1143,6 +1145,16 @@ impl<'ast, T: Field> Checker<'ast, T> {
let arg = arg.value;
// parameters defined on a non-entrypoint function should not have visibility modifiers
if (state.main_id != module_id || id != "main") && arg.is_private.is_some() {
errors.push(ErrorInner {
pos: Some(pos),
message:
"Visibility modifiers on arguments are only allowed on the entrypoint function"
.into(),
});
}
let decl_v = DeclarationVariable::new(
self.id_in_this_scope(arg.id.value.id),
decl_ty.clone(),
@ -1173,7 +1185,7 @@ impl<'ast, T: Field> Checker<'ast, T> {
arguments_checked.push(DeclarationParameter {
id: decl_v,
private: arg.is_private,
private: arg.is_private.unwrap_or(false),
});
}
@ -3751,14 +3763,14 @@ mod tests {
.mock()
}
/// Helper function to create: (private field a) { return; }
/// Helper function to create: (field a) { return; }
fn function1() -> FunctionNode<'static> {
let statements = vec![Statement::Return(None).mock()];
let arguments = vec![untyped::Parameter {
id: untyped::Variable::immutable("a", UnresolvedType::FieldElement.mock()).mock(),
is_private: true,
}
let arguments = vec![untyped::Parameter::new(
untyped::Variable::immutable("a", UnresolvedType::FieldElement.mock()).mock(),
None,
)
.mock()];
let signature =
@ -3885,6 +3897,7 @@ mod tests {
vec![("foo".into(), foo), ("bar".into(), bar)]
.into_iter()
.collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -3939,6 +3952,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -3952,10 +3966,10 @@ mod tests {
#[test]
fn duplicate_function_declaration_generic() {
// def foo<P>(private field[P] a) {
// def foo<P>(field[P] a) {
// return;
// }
// def foo(private field[3] a) {
// def foo(field[3] a) {
// return;
// }
//
@ -3963,7 +3977,7 @@ mod tests {
let mut f0 = function0();
f0.value.arguments = vec![untyped::Parameter::private(
f0.value.arguments = vec![untyped::Parameter::new(
untyped::Variable::immutable(
"a",
UnresolvedType::array(
@ -3973,6 +3987,7 @@ mod tests {
.mock(),
)
.mock(),
None,
)
.mock()];
f0.value.signature = UnresolvedSignature::new()
@ -3985,7 +4000,7 @@ mod tests {
let mut f1 = function0();
f1.value.arguments = vec![untyped::Parameter::private(
f1.value.arguments = vec![untyped::Parameter::new(
untyped::Variable::immutable(
"a",
UnresolvedType::array(
@ -3995,6 +4010,7 @@ mod tests {
.mock(),
)
.mock(),
None,
)
.mock()];
f1.value.signature = UnresolvedSignature::new().inputs(vec![UnresolvedType::array(
@ -4018,7 +4034,10 @@ mod tests {
],
};
let mut state = State::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
assert!(checker.check_module(&*MODULE_ID, &mut state).is_ok());
@ -4057,8 +4076,10 @@ mod tests {
],
};
let mut state =
State::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
assert!(checker.check_module(&*MODULE_ID, &mut state).is_ok());
@ -4111,8 +4132,10 @@ mod tests {
],
};
let mut state =
State::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
assert_eq!(
@ -4129,7 +4152,7 @@ mod tests {
// def foo() {
// return;
// }
// def foo(a) {
// def foo(field a) {
// return;
// }
//
@ -4152,6 +4175,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -4201,6 +4225,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -4244,6 +4269,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -4295,6 +4321,7 @@ mod tests {
vec![((*MODULE_ID).clone(), main), ("bar".into(), bar)]
.into_iter()
.collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -4343,6 +4370,7 @@ mod tests {
vec![((*MODULE_ID).clone(), main), ("bar".into(), bar)]
.into_iter()
.collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -4373,7 +4401,7 @@ mod tests {
#[test]
fn undeclared_generic() {
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let signature = UnresolvedSignature::new().inputs(vec![UnresolvedType::Array(
box UnresolvedType::FieldElement.mock(),
@ -4393,7 +4421,7 @@ mod tests {
fn success() {
// <K, L, M>(field[L][K]) -> field[L][K]
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let signature = UnresolvedSignature::new()
.generics(vec!["K".mock(), "L".mock(), "M".mock()])
@ -4546,8 +4574,10 @@ mod tests {
];
let module = Module { symbols };
let mut state =
State::<Bn128Field>::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
assert_eq!(
@ -4641,8 +4671,10 @@ mod tests {
];
let module = Module { symbols };
let mut state =
State::<Bn128Field>::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
assert!(checker.check_module(&*MODULE_ID, &mut state).is_ok());
@ -4675,11 +4707,11 @@ mod tests {
.mock();
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = Checker::default();
assert_eq!(
checker.check_function(foo, &*MODULE_ID, &state),
checker.check_function("foo", foo, &*MODULE_ID, &state),
Err(vec![ErrorInner {
pos: Some((Position::mock(), Position::mock())),
message: "Identifier \"i\" is undefined".into()
@ -4754,11 +4786,11 @@ mod tests {
};
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = Checker::default();
assert_eq!(
checker.check_function(foo, &*MODULE_ID, &state),
checker.check_function("foo", foo, &*MODULE_ID, &state),
Ok(foo_checked)
);
}
@ -4801,11 +4833,11 @@ mod tests {
.mock();
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), functions);
assert_eq!(
checker.check_function(bar, &*MODULE_ID, &state),
checker.check_function("bar", bar, &*MODULE_ID, &state),
Err(vec![ErrorInner {
pos: Some((Position::mock(), Position::mock())),
message:
@ -4840,11 +4872,11 @@ mod tests {
.mock();
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), HashSet::new());
assert_eq!(
checker.check_function(bar, &*MODULE_ID, &state),
checker.check_function("bar", bar, &*MODULE_ID, &state),
Err(vec![ErrorInner {
pos: Some((Position::mock(), Position::mock())),
@ -4912,8 +4944,10 @@ mod tests {
],
};
let mut state =
State::<Bn128Field>::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), HashSet::new());
assert_eq!(
@ -5008,8 +5042,10 @@ mod tests {
],
};
let mut state =
State::<Bn128Field>::new(vec![((*MODULE_ID).clone(), module)].into_iter().collect());
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), HashSet::new());
assert!(checker.check_module(&*MODULE_ID, &mut state).is_ok());
@ -5049,11 +5085,11 @@ mod tests {
.mock();
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), HashSet::new());
assert_eq!(
checker.check_function(bar, &*MODULE_ID, &state),
checker.check_function("bar", bar, &*MODULE_ID, &state),
Err(vec![ErrorInner {
pos: Some((Position::mock(), Position::mock())),
@ -5082,11 +5118,11 @@ mod tests {
.mock();
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), HashSet::new());
assert_eq!(
checker.check_function(bar, &*MODULE_ID, &state),
checker.check_function("bar", bar, &*MODULE_ID, &state),
Err(vec![ErrorInner {
pos: Some((Position::mock(), Position::mock())),
message: "Identifier \"a\" is undefined".into()
@ -5119,12 +5155,12 @@ mod tests {
]);
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let mut checker: Checker<Bn128Field> = new_with_args(Scope::default(), HashSet::new());
assert_eq!(
checker
.check_function(f, &*MODULE_ID, &state)
.check_function("main", f, &*MODULE_ID, &state)
.unwrap_err()[0]
.message,
"Duplicate name in function definition: `a` was previously declared as an argument, a generic parameter or a constant"
@ -5133,7 +5169,7 @@ mod tests {
#[test]
fn duplicate_main_function() {
// def main(a) -> field {
// def main(field a) -> field {
// return 1;
// }
// def main() -> field {
@ -5144,10 +5180,9 @@ mod tests {
let main1_statements: Vec<StatementNode> =
vec![Statement::Return(Some(Expression::IntConstant(1usize.into()).mock())).mock()];
let main1_arguments = vec![zokrates_ast::untyped::Parameter {
id: untyped::Variable::immutable("a", UnresolvedType::FieldElement.mock()).mock(),
is_private: false,
}
let main1_arguments = vec![zokrates_ast::untyped::Parameter::public(
untyped::Variable::immutable("a", UnresolvedType::FieldElement.mock()).mock(),
)
.mock()];
let main2_statements: Vec<StatementNode> =
@ -5413,6 +5448,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
let mut checker: Checker<Bn128Field> = Checker::default();
@ -5430,7 +5466,7 @@ mod tests {
fn empty_def() {
// an empty struct should be allowed to be defined
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let declaration: StructDefinitionNode = StructDefinition {
generics: vec![],
@ -5456,7 +5492,7 @@ mod tests {
fn valid_def() {
// a valid struct should be allowed to be defined
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let declaration: StructDefinitionNode = StructDefinition {
generics: vec![],
@ -5500,7 +5536,7 @@ mod tests {
fn duplicate_member_def() {
// definition of a struct with a duplicate member should be rejected
let modules = Modules::new();
let state = State::new(modules);
let state = State::new(modules, (*MODULE_ID).clone());
let declaration: StructDefinitionNode = StructDefinition {
generics: vec![],
@ -5577,6 +5613,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
assert!(Checker::default()
@ -5636,6 +5673,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
assert!(Checker::default()
@ -5669,6 +5707,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
assert!(Checker::default()
@ -5720,6 +5759,7 @@ mod tests {
let mut state = State::<Bn128Field>::new(
vec![((*MODULE_ID).clone(), module)].into_iter().collect(),
(*MODULE_ID).clone(),
);
assert!(Checker::default()
@ -6136,8 +6176,9 @@ mod tests {
let mut foo_field = function0();
foo_field.value.arguments = vec![untyped::Parameter::private(
foo_field.value.arguments = vec![untyped::Parameter::new(
untyped::Variable::immutable("a", UnresolvedType::FieldElement.mock()).mock(),
None,
)
.mock()];
foo_field.value.statements =
@ -6148,8 +6189,9 @@ mod tests {
let mut foo_u32 = function0();
foo_u32.value.arguments = vec![untyped::Parameter::private(
foo_u32.value.arguments = vec![untyped::Parameter::new(
untyped::Variable::immutable("a", UnresolvedType::Uint(32).mock()).mock(),
None,
)
.mock()];
foo_u32.value.statements =

View file

@ -29,7 +29,7 @@ import "utils/casts/u32_8_to_bool_256";
///
/// Returns:
/// Return true for S being a valid EdDSA Signature, false otherwise.
def main(private field[2] R, private field S, field[2] A, u32[8] M0, u32[8] M1, BabyJubJubParams context) -> bool {
def main(field[2] R, field S, field[2] A, u32[8] M0, u32[8] M1, BabyJubJubParams context) -> bool {
field[2] G = [context.Gu, context.Gv];
// Check if R is on curve and if it is not in a small subgroup. A is public input and can be checked offline