fix!: separate element decoding error from verification error (#16)

crates/proof-of-sql/src/sql/proof/provable_query_result.rs

@@ -83,11 +83,11 @@ impl ProvableQueryResult {
         evaluation_point: &[S],
         table_length: usize,
         column_result_fields: &[ColumnField],
-    ) -> Option<Vec<S>> {
+    ) -> Result<Vec<S>, QueryError> {
         assert_eq!(self.num_columns as usize, column_result_fields.len());
 
         if !self.indexes.valid(table_length) {
-            return None;
+            return Err(QueryError::InvalidIndexes);
         }
 
         let evaluation_vec_len = self
@@ -119,18 +119,17 @@ impl ProvableQueryResult {
                         decode_and_convert::<i64, S>(&self.data[offset..])
                     }
                 }?;
-
                 val += evaluation_vec[index as usize] * x;
                 offset += sz;
             }
             res.push(val);
         }
 
         if offset != self.data.len() {
-            return None;
+            return Err(QueryError::MiscellaneousEvaluationError);
         }
 
-        Some(res)
+        Ok(res)
     }
 
     /// Convert the intermediate query result into a final query result
@@ -150,56 +149,47 @@ impl ProvableQueryResult {
                 .iter()
                 .map(|field| match field.data_type() {
                     ColumnType::Boolean => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::Boolean(col)))
                     }
                     ColumnType::SmallInt => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::SmallInt(col)))
                     }
                     ColumnType::Int => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::Int(col)))
                     }
                     ColumnType::BigInt => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::BigInt(col)))
                     }
                     ColumnType::Int128 => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::Int128(col)))
                     }
                     ColumnType::VarChar => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::InvalidString)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::VarChar(col)))
                     }
                     ColumnType::Scalar => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::Scalar(col)))
                     }
                     ColumnType::Decimal75(precision, scale) => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::Decimal75(precision, scale, col)))
                     }
                     ColumnType::TimestampTZ(tu, tz) => {
-                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)
-                            .ok_or(QueryError::Overflow)?;
+                        let (col, num_read) = decode_multiple_elements(&self.data[offset..], n)?;
                         offset += num_read;
                         Ok((field.name(), OwnedColumn::TimestampTZ(tu, tz, col)))
                     }

crates/proof-of-sql/src/sql/proof/provable_query_result_test.rs

@@ -1,4 +1,4 @@
-use super::{ProvableQueryResult, ProvableResultColumn};
+use super::{ProvableQueryResult, ProvableResultColumn, QueryError};
 use crate::{
     base::{
         database::{ColumnField, ColumnType},
@@ -206,9 +206,10 @@ fn evaluation_fails_if_indexes_are_out_of_range() {
     compute_evaluation_vector(&mut evaluation_vec, &evaluation_point);
     let column_fields =
         vec![ColumnField::new("a".parse().unwrap(), ColumnType::BigInt); cols.len()];
-    assert!(res
-        .evaluate(&evaluation_point, 4, &column_fields[..])
-        .is_none());
+    assert!(matches!(
+        res.evaluate(&evaluation_point, 4, &column_fields[..]),
+        Err(QueryError::InvalidIndexes)
+    ));
 }
 
 #[test]
@@ -225,9 +226,10 @@ fn evaluation_fails_if_indexes_are_not_sorted() {
     compute_evaluation_vector(&mut evaluation_vec, &evaluation_point);
     let column_fields =
         vec![ColumnField::new("a".parse().unwrap(), ColumnType::BigInt); cols.len()];
-    assert!(res
-        .evaluate(&evaluation_point, 4, &column_fields[..])
-        .is_none());
+    assert!(matches!(
+        res.evaluate(&evaluation_point, 4, &column_fields[..]),
+        Err(QueryError::InvalidIndexes)
+    ));
 }
 
 #[test]
@@ -245,9 +247,10 @@ fn evaluation_fails_if_extra_data_is_included() {
     compute_evaluation_vector(&mut evaluation_vec, &evaluation_point);
     let column_fields =
         vec![ColumnField::new("a".parse().unwrap(), ColumnType::BigInt); cols.len()];
-    assert!(res
-        .evaluate(&evaluation_point, 4, &column_fields[..])
-        .is_none());
+    assert!(matches!(
+        res.evaluate(&evaluation_point, 4, &column_fields[..]),
+        Err(QueryError::MiscellaneousEvaluationError)
+    ));
 }
 
 #[test]
@@ -266,9 +269,30 @@ fn evaluation_fails_if_the_result_cant_be_decoded() {
     compute_evaluation_vector(&mut evaluation_vec, &evaluation_point);
     let column_fields =
         vec![ColumnField::new("a".parse().unwrap(), ColumnType::BigInt); res.num_columns()];
-    assert!(res
-        .evaluate(&evaluation_point, 4, &column_fields[..])
-        .is_none());
+    assert!(matches!(
+        res.evaluate(&evaluation_point, 4, &column_fields[..]),
+        Err(QueryError::Overflow)
+    ));
+}
+
+#[test]
+fn evaluation_fails_if_integer_overflow_happens() {
+    let indexes = Indexes::Sparse(vec![0, 2]);
+    let values: [i64; 3] = [i32::MAX as i64 + 1_i64, 11, 12];
+    let cols: [Box<dyn ProvableResultColumn>; 1] = [Box::new(values)];
+    let res = ProvableQueryResult::new(&indexes, &cols);
+    let evaluation_point = [
+        Curve25519Scalar::from(10u64),
+        Curve25519Scalar::from(100u64),
+    ];
+    let mut evaluation_vec = [Curve25519Scalar::ZERO; 4];
+    compute_evaluation_vector(&mut evaluation_vec, &evaluation_point);
+    let column_fields =
+        vec![ColumnField::new("a".parse().unwrap(), ColumnType::Int); res.num_columns()];
+    assert!(matches!(
+        res.evaluate(&evaluation_point, 4, &column_fields[..]),
+        Err(QueryError::Overflow)
+    ));
 }
 
 #[test]
@@ -286,9 +310,10 @@ fn evaluation_fails_if_data_is_missing() {
     compute_evaluation_vector(&mut evaluation_vec, &evaluation_point);
     let column_fields =
         vec![ColumnField::new("a".parse().unwrap(), ColumnType::BigInt); res.num_columns()];
-    assert!(res
-        .evaluate(&evaluation_point, 4, &column_fields[..])
-        .is_none());
+    assert!(matches!(
+        res.evaluate(&evaluation_point, 4, &column_fields[..]),
+        Err(QueryError::Overflow)
+    ));
 }
 
 #[test]
@@ -406,7 +431,6 @@ fn we_can_convert_a_provable_result_to_a_final_result_with_mixed_data_types() {
     .unwrap();
     let column_fields: Vec<Field> = column_fields.iter().map(|v| v.into()).collect();
     let schema = Arc::new(Schema::new(column_fields));
-    println!("{:?}", res);
     let expected_res = RecordBatch::try_new(
         schema,
         vec![
@@ -446,20 +470,3 @@ fn we_cannot_convert_a_provable_result_with_invalid_string_data() {
         .to_owned_table::<Curve25519Scalar>(&column_fields)
         .is_err());
 }
-
-// TODO: we don't correctly detect overflow yet
-// #[test]
-// #[should_panic]
-// fn we_can_detect_overflow() {
-//     let indexes = [0];
-//     let values = [i64::MAX];
-//     let cols : [Box<dyn ProvableResultColumn>; 1] = [
-//             Box::new(values),
-//     ];
-//     let res = ProvableQueryResult::new(&
-//         &indexes,
-//         &cols,
-//     );
-//     let column_fields = vec![ColumnField::new("a1".parse().unwrap(), ColumnType::BigInt)];
-//     let res = res.into_query_result(&column_fields).unwrap();
-// }

crates/proof-of-sql/src/sql/proof/query_proof.rs

@@ -242,16 +242,11 @@ impl<CP: CommitmentEvaluationProof> QueryProof<CP> {
         let column_result_fields = expr.get_column_result_fields();
 
         // compute the evaluation of the result MLEs
-        let result_evaluations = match result.evaluate(
+        let result_evaluations = result.evaluate(
             &subclaim.evaluation_point,
             table_length,
             &column_result_fields[..],
-        ) {
-            Some(evaluations) => evaluations,
-            _ => Err(ProofError::VerificationError(
-                "failed to evaluate intermediate result MLEs",
-            ))?,
-        };
+        )?;
 
         // pass over the provable AST to fill in the verification builder
         let sumcheck_evaluations = SumcheckMleEvaluations::new(

crates/proof-of-sql/src/sql/proof/query_result.rs

@@ -17,6 +17,15 @@ pub enum QueryError {
     /// This just means that the database was supposed to respond with a string that was not valid UTF-8.
     #[error("String decode error")]
     InvalidString,
+    /// Decoding errors other than overflow and invalid string.
+    #[error("Miscellaneous decoding error")]
+    MiscellaneousDecodingError,
+    /// Indexes are invalid.
+    #[error("Invalid indexes")]
+    InvalidIndexes,
+    /// Miscellaneous evaluation error.
+    #[error("Miscellaneous evaluation error")]
+    MiscellaneousEvaluationError,
     /// The proof failed to verify.
     #[error(transparent)]
     ProofError(#[from] ProofError),