Merge pull request #328 from Concordium/feature/COR-2315-precision-number-handling-verifier

fix: bug fix for precision handling

Author: squirer

Cargo.lock

@@ -1,5 +1,9 @@
 ## Unreleased
 
+## 0.3.1
+
+- fix for precision integer handling in the correct range for public info. Range supported for: `-9223372036854775808` -> `18446744073709551615`
+
 ## 0.3.0
 
 - Create verification API and verify presentation API are now changed to accept aribitrary json that is then encoded to CBOR for

credential-verification-service/CHANGELOG.md

@@ -1,6 +1,6 @@
 [package]
 name = "credential-verification-service"
-version = "0.3.0"
+version = "0.3.1"
 description = "Credential verification service"
 edition = "2024"
 

credential-verification-service/Cargo.toml

@@ -4,17 +4,18 @@ See data model here for request payload strucuture: [Link text](./data_model.md)
 
 Notes:
 - `connectionId`, `resourceId`, and `requestedClaims` are required for creating a verification request
+
 - `publicInfo` is optional and may be omitted. If provided, it must be a JSON object. The value is treated 
 as arbitrary public metadata and will be re-encoded as CBOR for submission on-chain as part of a registered data transaction.
 Supported JSON types inside `publicInfo` are: null, boolean, string, number, array, and object. JSON objects become CBOR maps, 
 arrays become CBOR arrays, strings become CBOR text, booleans/null become CBOR simple values.
 
-Number handling: integer JSON tokens are encoded as CBOR integers (Positive/Negative), and non-integer JSON tokens (fractional/exponent)
- are encoded as CBOR float64.
-
-Precision note: to preserve exact integer values across common clients (especially JavaScript), send integers outside the 
-safe range `-9007199254740991` to `9007199254740991` as JSON strings.
-
+- Json Number handling: 
+For Integer literals the range accepted is between Negative(i64 MIN) and Positve(u64 Max): `-9223372036854775808` -> `18446744073709551615`.
+Note that CBOR for negative is: -(`negative` + 1). Numbers outside this range will be treated as an f64 and may result in precision loss
+if the provided number contains 15-16 significant digits. It is advisable to submit the number as a JSON string if using very large numbers
+outside the number literal range above or float containing too many significant digits.
+ 
 
 a publicInfo sample may be as simple as:
 

credential-verification-service/docs/api/create_verification_request/examples.md

@@ -9,11 +9,11 @@ as arbitrary public metadata and will be re-encoded as CBOR for submission on-ch
 Supported JSON types inside `publicInfo` are: null, boolean, string, number, array, and object. JSON objects become CBOR maps, 
 arrays become CBOR arrays, strings become CBOR text, booleans/null become CBOR simple values.
 
-Number handling: integer JSON tokens are encoded as CBOR integers (Positive/Negative), and non-integer JSON tokens (fractional/exponent)
- are encoded as CBOR float64.
-
-Precision note: to preserve exact integer values across common clients (especially JavaScript), send integers outside the 
-safe range `-9007199254740991` to `9007199254740991` as JSON strings.
+- Json Number handling: 
+For Integer literals the range accepted is between Negative(i64 MIN) and Positve(u64 Max): `-9223372036854775808` -> `18446744073709551615`.
+Note that CBOR for negative is: -(`negative` + 1). Numbers outside this range will be treated as an f64 and may result in precision loss
+if the provided number contains 15-16 significant digits. It is advisable to submit the number as a JSON string if using very large numbers
+outside the number literal range above or float containing too many significant digits.
 
 a publicInfo sample may be as simple as:
 

credential-verification-service/docs/api/verify_presentation/examples.md

@@ -58,35 +58,26 @@ fn json_to_cbor_value(value: &serde_json::Value) -> Result<cbor::value::Value, V
         serde_json::Value::String(s) => cbor::value::Value::Text(s.clone()),
 
         serde_json::Value::Number(n) => {
-            if let Some(i) = n.as_i64() {
-                if i >= 0 {
-                    cbor::value::Value::Positive(i as u64)
-                } else {
-                    let mag: u64 =
-                        (-1i128 - i as i128)
-                            .try_into()
-                            .map_err(|_| ValidationError {
-                                details: vec![ErrorDetail {
-                                    code: "PUBLIC_INFO_NEGATIVE_INTEGER_OUT_OF_RANGE".to_string(),
-                                    path: "publicInfo".to_string(),
-                                    message: format!("negative integer out of range: {:?}", i),
-                                }],
-                            })?;
-                    cbor::value::Value::Negative(mag)
-                }
-            } else if let Some(u) = n.as_u64() {
-                cbor::value::Value::Positive(u)
-            } else if let Some(f) = n.as_f64() {
-                cbor::value::Value::Float(f)
+            if let Some(posint) = n.as_u64() {
+                // postive number that fits in u64
+                cbor::value::Value::Positive(posint)
+            } else if let Some(negint) = n.as_i64() {
+                // this number is definitely negative
+                let negintmag: u64 = (-1i64 - negint) as u64;
+                cbor::value::Value::Negative(negintmag)
+            } else if let Some(float) = n.as_f64() {
+                // either a float, or an integer that is too big
+                // for serialization without precision loss
+                cbor::value::Value::Float(float)
             } else {
+                // this should never happen, as serde_json::Number should always be one of the above 3 cases
+                // it does happen with way too big Float numbers such as `1e400`. In this case, we produce a
+                // validation error as it did not fit in u64, i64 or f64.
                 return Err(ValidationError {
                     details: vec![ErrorDetail {
                         code: "PUBLIC_INFO_NUMBER_NOT_REPRESENTABLE".to_string(),
+                        message: format!("Unable to parse this json number. {:?}", n),
                         path: "publicInfo".to_string(),
-                        message: format!(
-                            "serde_json::Number not representable as i64/u64/f64: {:?}",
-                            n
-                        ),
                     }],
                 });
             }
@@ -114,6 +105,7 @@ fn json_to_cbor_value(value: &serde_json::Value) -> Result<cbor::value::Value, V
 mod tests {
     use super::*;
 
+    /// Positive Test Scenarios
     #[test]
     fn test_public_info_none_returns_ok_none() {
         let public_info: Option<serde_json::Value> = None;
@@ -124,32 +116,6 @@ mod tests {
         assert!(result.unwrap().is_none());
     }
 
-    #[test]
-    fn test_public_info_not_json_object_at_root_validation_error() {
-        // The expected validation error
-        let expected_error = ValidationError {
-            details: vec![ErrorDetail {
-                code: "PUBLIC_INFO_EXPECTED_JSON".to_string(),
-                path: "publicInfo".to_string(),
-                message: "expected json at top level, got: Number(1)".to_string(),
-            }],
-        };
-
-        // creating a public info that produces the validation error (just a json number rather than an object at root)
-        let public_info: Option<serde_json::Value> =
-            Some(serde_json::Value::Number(serde_json::Number::from(1u64)));
-
-        // call our convert function
-        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
-
-        // ensure we have a result in error
-        assert!(result.is_err());
-
-        let result_error = result.unwrap_err();
-
-        assert_eq!(result_error, expected_error);
-    }
-
     #[test]
     fn test_parse_public_info_success_object() {
         let session_id_val = "alpha-numeric-1234-5678-sessionid-example";
@@ -179,15 +145,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_parse_public_info_fails_when_not_object() {
-        let public_info = Some(serde_json::json!("hello"));
-
-        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
-
-        assert!(result.is_err(), "top-level non-object should error");
-    }
-
     #[test]
     fn test_nested_conversion() {
         let public_info = Some(serde_json::json!({
@@ -236,4 +193,209 @@ mod tests {
             Some(&cbor::value::Value::Negative(211))
         );
     }
+
+    /// Error scenarios and validations.
+    #[test]
+    fn test_parse_public_info_fails_when_not_object() {
+        let public_info = Some(serde_json::json!("hello"));
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+
+        assert!(result.is_err(), "top-level non-object should error");
+    }
+
+    /// Public Info number parsing tests and constraints
+    #[test]
+    fn test_public_info_positive_integer_parsed_to_cbor_positive() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"simplePositiveInt": 42}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(
+            map.get("simplePositiveInt"),
+            Some(&cbor::value::Value::Positive(42))
+        );
+    }
+
+    #[test]
+    fn test_public_info_negative_one_parsed_to_cbor_negative_zero() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"simpleNegative": -1}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(
+            map.get("simpleNegative"),
+            Some(&cbor::value::Value::Negative(0))
+        );
+    }
+
+    /// Minimum negative that can be stored in CBOR
+    #[test]
+    fn test_public_info_cbor_i64_min_negative() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"i64min": -9223372036854775808}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+
+        assert_eq!(
+            map.get("i64min"),
+            Some(&cbor::value::Value::Negative(i64::MAX as u64))
+        );
+    }
+
+    /// Max positive that can be stored in CBOR
+    #[test]
+    fn test_public_info_cbor_max_u64_positive() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"u64max": 18446744073709551615}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(
+            map.get("u64max"),
+            Some(&cbor::value::Value::Positive(u64::MAX))
+        );
+    }
+
+    /// Test standard float works as expected.
+    #[test]
+    fn test_public_info_simple_float_parsed_to_cbor_float() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"simpleFloat": 0.1}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(
+            map.get("simpleFloat"),
+            Some(&cbor::value::Value::Float(0.1))
+        );
+    }
+
+    /// Ensure float followed by .0 is stored as a Float.
+    #[test]
+    fn test_public_info_float_with_dot_zero_stays_float() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"onePointZero": 1.0}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(
+            map.get("onePointZero"),
+            Some(&cbor::value::Value::Float(1.0))
+        );
+    }
+
+    /// Ensure scientific numbers with exponential are stored
+    /// as their true Float value
+    #[test]
+    fn test_public_info_scientific_notation_parsed_to_cbor_float() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"sci": 1e3}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(map.get("sci"), Some(&cbor::value::Value::Float(1000.0)));
+    }
+
+    /// Test a super large Float, returns with Validation
+    /// Error as not representable.
+    #[test]
+    fn test_public_info_float_out_of_f64_range_errors() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"tooBigFloat": 1e400}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_err());
+
+        let result = result.unwrap_err();
+
+        assert_eq!(result.details.len(), 1);
+        assert_eq!(
+            result.details[0].code,
+            "PUBLIC_INFO_NUMBER_NOT_REPRESENTABLE"
+        );
+        assert_eq!(result.details[0].path, "publicInfo");
+        assert!(
+            result.details[0]
+                .message
+                .contains("Unable to parse this json number")
+        );
+    }
+
+    // Float with 14 digits after decimal is precise
+    #[test]
+    fn test_public_info_float_14_digits_after_decimal_is_precise() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"preciseFloat": 99.99999999999999}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+        assert_eq!(
+            map.get("preciseFloat"),
+            Some(&cbor::value::Value::Float(99.99999999999999))
+        );
+    }
+
+    // Float with 15 digits after decimal loses precision
+    #[test]
+    fn test_public_info_float_15_digits_after_decimal_loses_precision() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"preciseFloat": 99.999999999999999}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+
+        // proof that precision gets lost for this
+        assert_eq!(
+            map.get("preciseFloat"),
+            Some(&cbor::value::Value::Float(100.0))
+        );
+    }
+
+    #[test]
+    fn test_public_info_zero_successfully_recorded_as_cbor_positive() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"val": 0}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+
+        assert_eq!(map.get("val"), Some(&cbor::value::Value::Positive(0)));
+    }
+
+    #[test]
+    fn test_public_info_neg_zero_successfully_recorded_as_cbor_positive() {
+        let public_info: Option<serde_json::Value> =
+            Some(serde_json::from_str(r#"{"val": -0}"#).unwrap());
+
+        let result = convert_public_info_to_hashmap_of_string_to_cbor(&public_info);
+        assert!(result.is_ok());
+
+        let map = result.unwrap().expect("should have public info");
+
+        assert_eq!(map.get("val"), Some(&cbor::value::Value::Positive(0)));
+    }
 }