fix: ensure reasonable types for numeric ops (#733)

Please be sure to look over the pull request guidelines here:
https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr.

# Please go through the following checklist
- [x] The PR title and commit messages adhere to guidelines here:
https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md.
In particular `!` is used if and only if at least one breaking change
has been introduced.
- [x] I have run the ci check script with `source
scripts/run_ci_checks.sh`.
- [x] I have run the clean commit check script with `source
scripts/check_commits.sh`, and the commit history is certified to follow
clean commit guidelines as described here:
https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/COMMIT_GUIDELINES.md
- [x] The latest changes from `main` have been incorporated to this PR
by simple rebase if possible, if not, then conflicts are resolved
appropriately.

# Rationale for this change
Currently `ProofExpr` of numeric types require scale casting as well as
explicit passing of scales since arithmetic ops on `ProofExpr` always
return `Scalar` type columns. In order to port the code to Solidity we
need to simplify the logic.
<!--
Why are you proposing this change? If this is already explained clearly
in the linked issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.

 Example:
 Add `NestedLoopJoinExec`.
 Closes #345.

Since we added `HashJoinExec` in #323 it has been possible to do
provable inner joins. However performance is not satisfactory in some
cases. Hence we need to fix the problem by implement
`NestedLoopJoinExec` and speed up the code
 for `HashJoinExec`.
-->

# What changes are included in this PR?
<!--
There is no need to duplicate the description in the ticket here but it
is sometimes worth providing a summary of the individual changes in this
PR.

Example:
- Add `NestedLoopJoinExec`.
- Speed up `HashJoinExec`.
- Route joins to `NestedLoopJoinExec` if the outer input is sufficiently
small.
-->
1. Make sure all arithmetic `ProofExpr` have decimal types and return
decimal columns to ensure consistency && prevent overflows from being a
thing
2. Cap precision to 75 and wrap around as opposed to banning ops that
cause precision to be too large
# Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
3. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?

Example:
Yes.
-->
Yes.

Author: iajoiner

crates/proof-of-sql-planner/tests/e2e_tests.rs

@@ -183,7 +183,8 @@ fn test_simple_filter_queries() {
     let alloc = Bump::new();
     let sql = "select id, name from cats where age > 2;
     select * from cats;
-    select name == $1 as name_eq from cats;";
+    select name == $1 as name_eq from cats;
+    select 2 * age as double_age from cats";
     let tables: IndexMap<TableRef, Table<DoryScalar>> = indexmap! {
         TableRef::from_names(None, "cats") => table(
             vec![
@@ -204,6 +205,7 @@ fn test_simple_filter_queries() {
             tinyint("age", [13_i8, 2, 0, 4, 4]),
         ]),
         owned_table([boolean("name_eq", [false, false, true, false, false])]),
+        owned_table([decimal75("double_age", 39, 0, [26_i8, 4, 0, 8, 8])]),
     ];
 
     // Create public parameters for DynamicDoryEvaluationProof
@@ -449,14 +451,14 @@ fn test_coin() {
             vec![
                 borrowed_varchar("from_address", ["0x1", "0x2", "0x3", "0x2", "0x1"], &alloc),
                 borrowed_varchar("to_address", ["0x2", "0x3", "0x1", "0x3", "0x2"], &alloc),
-                borrowed_decimal75("value", 20, 0, [100, 200, 300, 400, 500], &alloc),
+                borrowed_decimal75("value", 75, 0, [100, 200, 300, 400, 500], &alloc),
                 borrowed_timestamptz("timestamp", PoSQLTimeUnit::Second, PoSQLTimeZone::utc(), [1, 2, 3, 4, 4], &alloc),
             ]
         )
     };
     let expected_results: Vec<OwnedTable<DoryScalar>> = vec![owned_table([
-        decimal75("weighted_value", 62, 0, [100]),
-        decimal75("total_balance", 41, 0, [0]),
+        decimal75("weighted_value", 75, 0, [100]),
+        decimal75("total_balance", 75, 0, [0]),
         bigint("num_transactions", [5_i64]),
     ])];
 

crates/proof-of-sql/src/base/database/column_type_operation.rs

@@ -315,6 +315,7 @@ mod tests {
         base::{
             database::{ColumnType, TableRef},
             map::indexmap,
+            math::decimal::Precision,
         },
         sql::parse::query_expr_tests::schema_accessor_from_table_ref_with_schema,
     };
@@ -349,6 +350,6 @@ mod tests {
                 &Expression::Column(Identifier::try_new("b").unwrap()),
             )
             .unwrap();
-        assert_eq!(res, ColumnType::BigInt);
+        assert_eq!(res, ColumnType::Decimal75(Precision::new(20).unwrap(), 0));
     }
 }

crates/proof-of-sql/src/sql/parse/query_context_builder.rs

@@ -1,4 +1,6 @@
-use super::{add_subtract_columns, scale_and_add_subtract_eval, DynProofExpr, ProofExpr};
+use super::{
+    add_subtract_columns, scale_and_add_subtract_eval, DecimalProofExpr, DynProofExpr, ProofExpr,
+};
 use crate::{
     base::{
         database::{
@@ -43,14 +45,8 @@ impl ProofExpr for AddExpr {
     ) -> PlaceholderResult<Column<'a, S>> {
         let lhs_column: Column<'a, S> = self.lhs.first_round_evaluate(alloc, table, params)?;
         let rhs_column: Column<'a, S> = self.rhs.first_round_evaluate(alloc, table, params)?;
-        Ok(Column::Scalar(add_subtract_columns(
-            lhs_column,
-            rhs_column,
-            self.lhs.data_type().scale().unwrap_or(0),
-            self.rhs.data_type().scale().unwrap_or(0),
-            alloc,
-            false,
-        )))
+        let res = add_subtract_columns(lhs_column, rhs_column, alloc, false);
+        Ok(Column::Decimal75(self.precision(), self.scale(), res))
     }
 
     #[tracing::instrument(
@@ -73,18 +69,10 @@ impl ProofExpr for AddExpr {
         let rhs_column: Column<'a, S> = self
             .rhs
             .final_round_evaluate(builder, alloc, table, params)?;
-        let res = Column::Scalar(add_subtract_columns(
-            lhs_column,
-            rhs_column,
-            self.lhs.data_type().scale().unwrap_or(0),
-            self.rhs.data_type().scale().unwrap_or(0),
-            alloc,
-            false,
-        ));
-
+        let res = add_subtract_columns(lhs_column, rhs_column, alloc, false);
         log::log_memory_usage("End");
 
-        Ok(res)
+        Ok(Column::Decimal75(self.precision(), self.scale(), res))
     }
 
     fn verifier_evaluate<S: Scalar>(
@@ -111,3 +99,5 @@ impl ProofExpr for AddExpr {
         self.rhs.get_column_references(columns);
     }
 }
+
+impl DecimalProofExpr for AddExpr {}

crates/proof-of-sql/src/sql/proof_exprs/add_expr.rs

@@ -2,17 +2,15 @@ use crate::{
     base::{
         commitment::InnerProductProof,
         database::{
-            owned_table_utility::*, table_utility::*, Column, OwnedTableTestAccessor, TableRef,
+            owned_table_utility::*, table_utility::*, OwnedTableTestAccessor, TableRef,
             TableTestAccessor,
         },
-        scalar::test_scalar::TestScalar,
     },
     proof_primitive::inner_product::curve_25519_scalar::Curve25519Scalar,
     sql::{
-        proof::{exercise_verification, QueryError, VerifiableQueryResult},
+        proof::{exercise_verification, VerifiableQueryResult},
         proof_exprs::{test_utility::*, DynProofExpr, ProofExpr},
-        proof_plans::{test_utility::*, DynProofPlan},
-        AnalyzeError,
+        proof_plans::test_utility::*,
     },
 };
 use bumpalo::Bump;
@@ -57,7 +55,7 @@ fn we_can_prove_a_typical_add_subtract_query() {
     let expected_res = owned_table([
         smallint("a", [3_i16, 4]),
         bigint("c", [2_i16, 0]),
-        bigint("res", [4_i64, 5]),
+        decimal75("res", 20, 0, [4_i64, 5]),
         varchar("d", ["efg", "g"]),
     ]);
     assert_eq!(res, expected_res);
@@ -110,133 +108,6 @@ fn we_can_prove_a_typical_add_subtract_query_with_decimals() {
     assert_eq!(res, expected_res);
 }
 
-// Column type issue tests
-#[test]
-fn decimal_column_type_issues_error_out_when_producing_provable_ast() {
-    let data = owned_table([decimal75("a", 75, 2, [1_i16, 2, 3, 4])]);
-    let t = TableRef::new("sxt", "t");
-    let accessor =
-        OwnedTableTestAccessor::<InnerProductProof>::new_from_table(t.clone(), data, 0, ());
-    assert!(matches!(
-        DynProofExpr::try_new_add(column(&t, "a", &accessor), const_bigint(1)),
-        Err(AnalyzeError::DataTypeMismatch { .. })
-    ));
-}
-
-// Overflow tests
-// select a + b as c from sxt.t where b = 1
-#[test]
-fn result_expr_can_overflow() {
-    let data = owned_table([
-        smallint("a", [i16::MAX, i16::MIN]),
-        smallint("b", [1_i16, 0]),
-    ]);
-    let t = TableRef::new("sxt", "t");
-    let accessor =
-        OwnedTableTestAccessor::<InnerProductProof>::new_from_table(t.clone(), data, 0, ());
-    let ast: DynProofPlan = filter(
-        vec![aliased_plan(
-            add(column(&t, "a", &accessor), column(&t, "b", &accessor)),
-            "c",
-        )],
-        tab(&t),
-        equal(column(&t, "b", &accessor), const_bigint(1)),
-    );
-    let verifiable_res: VerifiableQueryResult<InnerProductProof> =
-        VerifiableQueryResult::new(&ast, &accessor, &(), &[]).unwrap();
-    assert!(matches!(
-        verifiable_res.verify(&ast, &accessor, &(), &[]),
-        Err(QueryError::Overflow)
-    ));
-}
-
-// select a + b as c from sxt.t where b == 0
-#[test]
-fn overflow_in_nonselected_rows_doesnt_error_out() {
-    let data = owned_table([
-        smallint("a", [i16::MAX, i16::MIN + 1]),
-        smallint("b", [1_i16, 0]),
-    ]);
-    let t = TableRef::new("sxt", "t");
-    let accessor =
-        OwnedTableTestAccessor::<InnerProductProof>::new_from_table(t.clone(), data, 0, ());
-    let ast: DynProofPlan = filter(
-        vec![aliased_plan(
-            add(column(&t, "a", &accessor), column(&t, "b", &accessor)),
-            "c",
-        )],
-        tab(&t),
-        equal(column(&t, "b", &accessor), const_bigint(0)),
-    );
-    let verifiable_res: VerifiableQueryResult<InnerProductProof> =
-        VerifiableQueryResult::new(&ast, &accessor, &(), &[]).unwrap();
-    exercise_verification(&verifiable_res, &ast, &accessor, &t);
-    let res = verifiable_res
-        .verify(&ast, &accessor, &(), &[])
-        .unwrap()
-        .table;
-    let expected_res = owned_table([smallint("c", [i16::MIN + 1])]);
-    assert_eq!(res, expected_res);
-}
-
-// select a, b from sxt.t where a + b >= 0
-#[test]
-fn overflow_in_where_clause_doesnt_error_out() {
-    let data = owned_table([bigint("a", [i64::MAX, i64::MIN]), smallint("b", [1_i16, 0])]);
-    let t = TableRef::new("sxt", "t");
-    let accessor =
-        OwnedTableTestAccessor::<InnerProductProof>::new_from_table(t.clone(), data, 0, ());
-    let ast: DynProofPlan = filter(
-        cols_expr_plan(&t, &["a", "b"], &accessor),
-        tab(&t),
-        gte(
-            add(column(&t, "a", &accessor), column(&t, "b", &accessor)),
-            const_bigint(0),
-        ),
-    );
-    let verifiable_res: VerifiableQueryResult<InnerProductProof> =
-        VerifiableQueryResult::new(&ast, &accessor, &(), &[]).unwrap();
-    exercise_verification(&verifiable_res, &ast, &accessor, &t);
-    let res = verifiable_res
-        .verify(&ast, &accessor, &(), &[])
-        .unwrap()
-        .table;
-    let expected_res = owned_table([bigint("a", [i64::MAX]), smallint("b", [1_i16])]);
-    assert_eq!(res, expected_res);
-}
-
-// select a + b as c, a - b as d from sxt.t
-#[test]
-fn result_expr_can_overflow_more() {
-    let data = owned_table([
-        bigint("a", [i64::MAX, i64::MIN, i64::MAX, i64::MIN]),
-        bigint("b", [i64::MAX, i64::MAX, i64::MIN, i64::MIN]),
-    ]);
-    let t = TableRef::new("sxt", "t");
-    let accessor =
-        OwnedTableTestAccessor::<InnerProductProof>::new_from_table(t.clone(), data, 0, ());
-    let ast: DynProofPlan = filter(
-        vec![
-            aliased_plan(
-                add(column(&t, "a", &accessor), column(&t, "b", &accessor)),
-                "c",
-            ),
-            aliased_plan(
-                subtract(column(&t, "a", &accessor), column(&t, "b", &accessor)),
-                "d",
-            ),
-        ],
-        tab(&t),
-        const_bool(true),
-    );
-    let verifiable_res: VerifiableQueryResult<InnerProductProof> =
-        VerifiableQueryResult::new(&ast, &accessor, &(), &[]).unwrap();
-    assert!(matches!(
-        verifiable_res.verify(&ast, &accessor, &(), &[]),
-        Err(QueryError::Overflow)
-    ));
-}
-
 fn test_random_tables_with_given_offset(offset: usize) {
     let dist = Uniform::new(-3, 4);
     let mut rng = StdRng::from_seed([0u8; 32]);
@@ -283,11 +154,11 @@ fn test_random_tables_with_given_offset(offset: usize) {
             and(
                 equal(
                     column(&t, "b", &accessor),
-                    const_scalar::<TestScalar, _>(filter_val1.as_str()),
+                    const_scalar::<Curve25519Scalar, _>(filter_val1.as_str()),
                 ),
                 equal(
                     column(&t, "c", &accessor),
-                    const_scalar::<TestScalar, _>(filter_val2),
+                    const_scalar::<Curve25519Scalar, _>(filter_val2),
                 ),
             ),
         );
@@ -307,13 +178,14 @@ fn test_random_tables_with_given_offset(offset: usize) {
         ))
         .filter_map(|(a, b, c, d)| {
             if b == &filter_val1 && c == &filter_val2 {
-                Some((i128::from(*a + *c - 4), d.clone()))
+                Some((Curve25519Scalar::from(*a + *c - 4), d.clone()))
             } else {
                 None
             }
         })
         .multiunzip();
-        let expected_result = owned_table([varchar("d", expected_d), int128("f", expected_f)]);
+        let expected_result =
+            owned_table([varchar("d", expected_d), decimal75("f", 40, 0, expected_f)]);
 
         assert_eq!(expected_result, res);
     }
@@ -349,10 +221,6 @@ fn we_can_compute_the_correct_output_of_an_add_subtract_expr_using_first_round_e
     let res = add_subtract_expr
         .first_round_evaluate(&alloc, &data, &[])
         .unwrap();
-    let expected_res_scalar = [0, 2, 2, 4]
-        .iter()
-        .map(|v| Curve25519Scalar::from(*v))
-        .collect::<Vec<_>>();
-    let expected_res = Column::Scalar(&expected_res_scalar);
+    let expected_res = borrowed_decimal75("res", 21, 0, [0_i64, 2, 2, 4], &alloc).1;
     assert_eq!(res, expected_res);
 }

crates/proof-of-sql/src/sql/proof_exprs/add_subtract_expr_test.rs

@@ -23,8 +23,6 @@ pub(crate) fn scale_and_subtract<'a, S: Scalar>(
     alloc: &'a Bump,
     lhs: Column<'a, S>,
     rhs: Column<'a, S>,
-    lhs_scale: i8,
-    rhs_scale: i8,
     is_equal: bool,
 ) -> AnalyzeResult<&'a [S]> {
     let lhs_len = lhs.len();
@@ -48,6 +46,8 @@ pub(crate) fn scale_and_subtract<'a, S: Scalar>(
             right_type: rhs_type.to_string(),
         });
     }
+    let lhs_scale = lhs_type.scale().unwrap_or(0);
+    let rhs_scale = rhs_type.scale().unwrap_or(0);
     let max_scale = max(lhs_scale, rhs_scale);
     let lhs_upscale = max_scale - lhs_scale;
     let rhs_upscale = max_scale - rhs_scale;

crates/proof-of-sql/src/sql/proof_exprs/comparison_util.rs

@@ -44,9 +44,7 @@ impl ProofExpr for EqualsExpr {
 
         let lhs_column = self.lhs.first_round_evaluate(alloc, table, params)?;
         let rhs_column = self.rhs.first_round_evaluate(alloc, table, params)?;
-        let lhs_scale = self.lhs.data_type().scale().unwrap_or(0);
-        let rhs_scale = self.rhs.data_type().scale().unwrap_or(0);
-        let res = scale_and_subtract(alloc, lhs_column, rhs_column, lhs_scale, rhs_scale, true)
+        let res = scale_and_subtract(alloc, lhs_column, rhs_column, true)
             .expect("Failed to scale and subtract");
         let res = Column::Boolean(first_round_evaluate_equals_zero(
             table.num_rows(),
@@ -75,11 +73,8 @@ impl ProofExpr for EqualsExpr {
         let rhs_column = self
             .rhs
             .final_round_evaluate(builder, alloc, table, params)?;
-        let lhs_scale = self.lhs.data_type().scale().unwrap_or(0);
-        let rhs_scale = self.rhs.data_type().scale().unwrap_or(0);
-        let scale_and_subtract_res =
-            scale_and_subtract(alloc, lhs_column, rhs_column, lhs_scale, rhs_scale, true)
-                .expect("Failed to scale and subtract");
+        let scale_and_subtract_res = scale_and_subtract(alloc, lhs_column, rhs_column, true)
+            .expect("Failed to scale and subtract");
         let res = Column::Boolean(final_round_evaluate_equals_zero(
             table.num_rows(),
             builder,

crates/proof-of-sql/src/sql/proof_exprs/equals_expr.rs

@@ -53,14 +53,12 @@ impl ProofExpr for InequalityExpr {
 
         let lhs_column = self.lhs.first_round_evaluate(alloc, table, params)?;
         let rhs_column = self.rhs.first_round_evaluate(alloc, table, params)?;
-        let lhs_scale = self.lhs.data_type().scale().unwrap_or(0);
-        let rhs_scale = self.rhs.data_type().scale().unwrap_or(0);
         let table_length = table.num_rows();
         let diff = if self.is_lt {
-            scale_and_subtract(alloc, lhs_column, rhs_column, lhs_scale, rhs_scale, false)
+            scale_and_subtract(alloc, lhs_column, rhs_column, false)
                 .expect("Failed to scale and subtract")
         } else {
-            scale_and_subtract(alloc, rhs_column, lhs_column, rhs_scale, lhs_scale, false)
+            scale_and_subtract(alloc, rhs_column, lhs_column, false)
                 .expect("Failed to scale and subtract")
         };
 
@@ -92,13 +90,11 @@ impl ProofExpr for InequalityExpr {
         let rhs_column = self
             .rhs
             .final_round_evaluate(builder, alloc, table, params)?;
-        let lhs_scale = self.lhs.data_type().scale().unwrap_or(0);
-        let rhs_scale = self.rhs.data_type().scale().unwrap_or(0);
         let diff = if self.is_lt {
-            scale_and_subtract(alloc, lhs_column, rhs_column, lhs_scale, rhs_scale, false)
+            scale_and_subtract(alloc, lhs_column, rhs_column, false)
                 .expect("Failed to scale and subtract")
         } else {
-            scale_and_subtract(alloc, rhs_column, lhs_column, rhs_scale, lhs_scale, false)
+            scale_and_subtract(alloc, rhs_column, lhs_column, false)
                 .expect("Failed to scale and subtract")
         };
 

crates/proof-of-sql/src/sql/proof_exprs/inequality_expr.rs

@@ -1,5 +1,6 @@
 //! This module proves provable expressions.
 mod proof_expr;
+pub(crate) use proof_expr::DecimalProofExpr;
 pub use proof_expr::ProofExpr;
 #[cfg(all(test, feature = "blitzar"))]
 mod proof_expr_test;