Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow usage of decimal string encoding for fields larger than a i128 #2547

Merged
merged 2 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion crates/noirc_abi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
strum_macros = "0.24"
47 changes: 41 additions & 6 deletions crates/noirc_abi/src/input_parser/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
mod json;
mod toml;

use num_bigint::BigUint;
use num_traits::Num;
use std::collections::BTreeMap;

use acvm::FieldElement;
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
Expand Down Expand Up @@ -183,20 +186,52 @@ fn parse_str_to_field(value: &str) -> Result<FieldElement, InputParserError> {
if value.starts_with("0x") {
FieldElement::from_hex(value).ok_or_else(|| InputParserError::ParseHexStr(value.to_owned()))
} else {
value
.parse::<i128>()
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?
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
.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);
}
}
}