From 87ed9286ab237bb82dc27add1d502a1461404455 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 4 Sep 2023 09:57:01 +0100 Subject: [PATCH 1/2] fix: allow passing values larger than a `i128` as circuit argument --- Cargo.lock | 2 + crates/noirc_abi/Cargo.toml | 4 +- crates/noirc_abi/src/input_parser/mod.rs | 47 +++++++++++++++++++++--- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1d6b36af03..c6e746632df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2278,6 +2278,8 @@ dependencies = [ "acvm", "iter-extended", "noirc_frontend", + "num-bigint", + "num-traits", "serde", "serde_json", "strum", diff --git a/crates/noirc_abi/Cargo.toml b/crates/noirc_abi/Cargo.toml index 45466061fde..fda19e15b29 100644 --- a/crates/noirc_abi/Cargo.toml +++ b/crates/noirc_abi/Cargo.toml @@ -14,7 +14,9 @@ toml.workspace = true serde_json = "1.0" serde.workspace = true thiserror.workspace = true +num-bigint = "0.4" +num-traits = "0.2" [dev-dependencies] strum = "0.24" -strum_macros = "0.24" \ No newline at end of file +strum_macros = "0.24" diff --git a/crates/noirc_abi/src/input_parser/mod.rs b/crates/noirc_abi/src/input_parser/mod.rs index badfb1f5b08..4020cae2218 100644 --- a/crates/noirc_abi/src/input_parser/mod.rs +++ b/crates/noirc_abi/src/input_parser/mod.rs @@ -1,6 +1,5 @@ -mod json; -mod toml; - +use num_bigint::BigUint; +use num_traits::Num; use std::collections::BTreeMap; use acvm::FieldElement; @@ -8,6 +7,10 @@ use serde::Serialize; use crate::errors::InputParserError; use crate::{Abi, AbiType}; + +mod json; +mod toml; + /// This is what all formats eventually transform into /// For example, a toml file will parse into TomlTypes /// and those TomlTypes will be mapped to Value @@ -183,20 +186,52 @@ fn parse_str_to_field(value: &str) -> Result { if value.starts_with("0x") { FieldElement::from_hex(value).ok_or_else(|| InputParserError::ParseHexStr(value.to_owned())) } else { - value - .parse::() + BigUint::from_str_radix(value, 10) .map_err(|err_msg| InputParserError::ParseStr(err_msg.to_string())) - .map(FieldElement::from) + // Note that we're reducing here. Should we reject inputs that don't fit in the field? + .map(field_from_big_uint) } } +fn field_from_big_uint(bigint: BigUint) -> FieldElement { + FieldElement::from_be_bytes_reduce(&bigint.to_bytes_be()) +} + #[cfg(test)] mod test { + use acvm::FieldElement; + use num_bigint::BigUint; + use super::parse_str_to_field; + fn big_uint_from_field(field: FieldElement) -> BigUint { + BigUint::from_bytes_be(&field.to_be_bytes()) + } + #[test] fn parse_empty_str_fails() { // Check that this fails appropriately rather than being treated as 0, etc. assert!(parse_str_to_field("").is_err()); } + + #[test] + fn parse_fields_from_strings() { + let fields = vec![ + FieldElement::zero(), + FieldElement::one(), + FieldElement::from(u128::MAX) + FieldElement::one(), + // Equivalent to `FieldElement::modulus() - 1` + -FieldElement::one(), + ]; + + for field in fields { + let hex_field = format!("0x{}", field.to_hex()); + let field_from_hex = parse_str_to_field(&hex_field).unwrap(); + assert_eq!(field_from_hex, field); + + let dec_field = big_uint_from_field(field).to_string(); + let field_from_dec = parse_str_to_field(&dec_field).unwrap(); + assert_eq!(field_from_dec, field); + } + } } From c915b6140bcef19a98287853bb0e5a14fb685326 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 4 Sep 2023 11:17:02 +0100 Subject: [PATCH 2/2] fix: reject non-canonical fields --- crates/noirc_abi/src/input_parser/mod.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/noirc_abi/src/input_parser/mod.rs b/crates/noirc_abi/src/input_parser/mod.rs index 4020cae2218..139f3276179 100644 --- a/crates/noirc_abi/src/input_parser/mod.rs +++ b/crates/noirc_abi/src/input_parser/mod.rs @@ -188,8 +188,16 @@ fn parse_str_to_field(value: &str) -> Result { } else { BigUint::from_str_radix(value, 10) .map_err(|err_msg| InputParserError::ParseStr(err_msg.to_string())) - // Note that we're reducing here. Should we reject inputs that don't fit in the field? - .map(field_from_big_uint) + .and_then(|bigint| { + if bigint < FieldElement::modulus() { + Ok(field_from_big_uint(bigint)) + } else { + Err(InputParserError::ParseStr(format!( + "Input exceeds field modulus. Values must fall within [0, {})", + FieldElement::modulus(), + ))) + } + }) } } @@ -234,4 +242,11 @@ mod test { assert_eq!(field_from_dec, field); } } + + #[test] + fn rejects_noncanonical_fields() { + let noncanonical_field = FieldElement::modulus().to_string(); + let parsed_field = parse_str_to_field(&noncanonical_field); + println!("{parsed_field:?}"); + } }