fix: Remove inner product types from generic test. #264

Abiji-2020 · 2024-10-12T07:10:28Z

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

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.
I have run the ci check script with source scripts/run_ci_checks.sh.

Rationale for this change

The tests in the base directory was all modified with the updated from

Curve25519Scalar -> TestScalar
RistrettoPoint -> NaiveCommitment
InnerProductProof -> TestEvaluationProof

Notes

During the previous commit , I accidentally modified the function which needs to be done in Curve25519Scalar , which was modified in the latest commit

closes #230
/claim #230

What changes are included in this PR?

all the test files, (unit tests) are modified.

Are these changes tested?

yes

Abiji-2020 · 2024-10-12T07:41:26Z

ci failing on

@JayWhite2357 @stuarttimwhite @iajoiner What can we do in this matter, as the methods are not implemented for the TestEvaluationProof

Abiji-2020 · 2024-10-17T12:49:42Z

@JayWhite2357 Need help on this 😶

JayWhite2357 · 2024-10-19T01:56:09Z

@Abiji-2020 Sorry for the delay, I was out of office the first half of the week and I'm playing catchup 😬.
Those specific test_random_ipa are actually testing the InnerProductProof itself, so they don't need to be changed over.

JayWhite2357

Looks excellent. Breaking this down into these commits was super helpful.

Two changes that are easy to make because of your clean commit history:

You can just drop this commit: 3eb4eb8

For da37c77,
I would actually ask that you rename the tests and still use TestScalar instead of reverting back to Curve25519Scalar.

(Additionally, it would be nice, but not necessary, if you could use git rebase --interactive or similar to edit the commits to fix the typos wherever they originated rather than putting it in a separate commit.)

Finally, I would prefer, but not require a rebase rather than a merge: c28f27e. That way we can merge all of your commits rather than doing a squash merge.

Abiji-2020 · 2024-10-19T05:09:38Z

Looks excellent. Breaking this down into these commits was super helpful.

Two changes that are easy to make because of your clean commit history:

You can just drop this commit: 3eb4eb8

For da37c77, I would actually ask that you rename the tests and still use TestScalar instead of reverting back to Curve25519Scalar.

(Additionally, it would be nice, but not necessary, if you could use git rebase --interactive or similar to edit the commits to fix the typos wherever they originated rather than putting it in a separate commit.)

Finally, I would prefer, but not require a rebase rather than a merge: c28f27e. That way we can merge all of your commits rather than doing a squash merge.

Currently all the modifications are done in the files named as test. Will update the tests in other files within the directory and update. Untill marking the PR as draft.

Abiji-2020 · 2024-10-19T12:50:35Z

@JayWhite2357 sorry for this much commit, after rebasing with the main branch, this commit was messed up heavily. I tried to make it clean as possible .

JayWhite2357

I've done a thorough review. If you do these changes and resolve the conflicts with main, this should be ready to go.

JayWhite2357 · 2024-10-20T02:29:20Z

scripts/run_ci_checks.sh

This file does not appear to be changed. Please be sure it is not included in the git diff.

I had tried to not include it in the git diff but it is still showing in the git diff

Do something like git checkout main scripts/run_ci_checks.sh. Then commit that version of the file. My guess is that there is some issue with line endings.

@JayWhite2357 yes now it was removed from the git diff. I downloaded the file from the main branch and replaced in the current branch.

JayWhite2357 · 2024-10-20T03:27:21Z

crates/proof-of-sql/src/base/commitment/vec_commitment_ext.rs

+            CommittableColumn::BigInt(&front_emptied_column_a),
+            CommittableColumn::VarChar(
+                front_emptied_column_b
+                    .iter()
+                    .map(Into::<TestScalar>::into)
+                    .map(Into::<[u64; 4]>::into)
+                    .collect(),
+            ),
+        ];

+        let expected_commitments =
+            NaiveCommitment::compute_commitments(&committable_columns, 0, &());


Something like this is better.

Suggested change

CommittableColumn::BigInt(&front_emptied_column_a),

CommittableColumn::VarChar(

front_emptied_column_b

.iter()

.map(Into::<TestScalar>::into)

.map(Into::<[u64; 4]>::into)

.collect(),

),

];

let expected_commitments =

NaiveCommitment::compute_commitments(&committable_columns, 0, &());

CommittableColumn::BigInt(&column_a[3..]),

CommittableColumn::VarChar(

column_b[3..]

.iter()

.map(Into::<TestScalar>::into)

.map(Into::<[u64; 4]>::into)

.collect(),

),

];

let expected_commitments =

NaiveCommitment::compute_commitments(&committable_columns, 3, &());

Modified it

JayWhite2357 · 2024-10-20T03:29:53Z

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

@@ -91,9 +92,11 @@ pub trait ArrayRefExt {
 impl ArrayRefExt for ArrayRef {
    #[cfg(any(test, feature = "test"))]
    #[cfg(feature = "blitzar")]
-    fn to_curve25519_scalars(
+    #[cfg(test)]
+    fn to_test_scalars(


This is dead code. Let's just remove this and all the tests in this file that are testing it.

Removed the code and its tests

JayWhite2357 · 2024-10-20T03:31:12Z

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

@@ -41,7 +41,7 @@ pub enum Column<'a, S: Scalar> {
    /// i128 columns
    Int128(&'a [i128]),
    /// Decimal columns with a max width of 252 bits
-    ///  - the backing store maps to the type [`crate::base::scalar::Curve25519Scalar`]
+    ///  - the backing store maps to the type [`crate::base::scalar::test_scalar::TestScalar`]


Suggested change

/// - the backing store maps to the type [`crate::base::scalar::test_scalar::TestScalar`]

/// - the backing store maps to the type `S`

JayWhite2357 · 2024-10-20T03:31:32Z

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

@@ -263,7 +263,7 @@ pub enum ColumnType {
    /// Mapped to i64
    #[serde(alias = "TIMESTAMP", alias = "timestamp")]
    TimestampTZ(PoSQLTimeUnit, PoSQLTimeZone),
-    /// Mapped to [`Curve25519Scalar`](crate::base::scalar::Curve25519Scalar)
+    /// Mapped to [`TestScalar`](crate::base::scalar::test_scalar::TestScalar)


Suggested change

/// Mapped to [`TestScalar`](crate::base::scalar::test_scalar::TestScalar)

/// Mapped to `S`

Abiji-2020 · 2024-10-21T15:43:14Z

@JayWhite2357 any updates on this?

Abiji-2020 · 2024-10-23T00:08:43Z

@JayWhite2357 now you can check it

Abiji-2020 · 2024-10-29T01:07:13Z

@JayWhite2357 could you merge it, as the auto merge was cancelled, when the main repo was updated. Only done rebaseing with the main after the approval .

Abiji-2020 · 2024-10-31T14:22:19Z

@JayWhite2357 could you merge this PR

Signed-off-by: Abinand P <[email protected]>

…nt directory Signed-off-by: Abinand P <[email protected]>

Signed-off-by: Abinand P <[email protected]>

JayWhite2357 · 2024-11-06T15:45:29Z

@Abiji-2020 hmmm. I missed this one. I'll merge it.

Abiji-2020 · 2024-11-06T15:47:45Z

@Abiji-2020 hmmm. I missed this one. I'll merge it.

Thanks @JayWhite2357, I was rebasing it for the past week. Hoping it to merge daily.

github-actions · 2024-11-06T16:31:42Z

🎉 This PR is included in version 0.36.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

algora-pbc bot mentioned this pull request Oct 12, 2024

Remove InnerProduct types from generic tests within base directory #230

Closed

algora-pbc bot added the 🙋 Bounty claim label Oct 12, 2024

Abiji-2020 changed the title ~~Remove inner product types from generic test.~~ fix: Remove inner product types from generic test. Oct 12, 2024

JayWhite2357 requested changes Oct 19, 2024

View reviewed changes

Abiji-2020 force-pushed the remove-innerProduct-types-test branch from c28f27e to 71b680b Compare October 19, 2024 03:51

Abiji-2020 marked this pull request as draft October 19, 2024 05:10

Abiji-2020 force-pushed the remove-innerProduct-types-test branch from 8533fa9 to f0a357b Compare October 19, 2024 09:46

Abiji-2020 marked this pull request as ready for review October 19, 2024 10:16

Abiji-2020 requested a review from JayWhite2357 October 19, 2024 10:16

Abiji-2020 force-pushed the remove-innerProduct-types-test branch from c1cb096 to d8c75c7 Compare October 19, 2024 10:32

Abiji-2020 force-pushed the remove-innerProduct-types-test branch 2 times, most recently from e453d14 to 024eb55 Compare October 20, 2024 02:34

JayWhite2357 requested changes Oct 20, 2024

View reviewed changes

Abiji-2020 force-pushed the remove-innerProduct-types-test branch from 7b9a784 to 4791e85 Compare October 20, 2024 04:09

Abiji-2020 requested a review from JayWhite2357 October 20, 2024 04:18

Abiji-2020 force-pushed the remove-innerProduct-types-test branch 4 times, most recently from 6be62ce to c28ad44 Compare October 21, 2024 15:12

Abiji-2020 force-pushed the remove-innerProduct-types-test branch 4 times, most recently from a44f61b to d04e6f7 Compare October 23, 2024 00:06

Abiji-2020 force-pushed the remove-innerProduct-types-test branch 2 times, most recently from 5d8696b to ca6e336 Compare October 29, 2024 00:28

Abiji-2020 force-pushed the remove-innerProduct-types-test branch 6 times, most recently from 0d3f5ae to 239348d Compare October 31, 2024 14:19

Abiji-2020 force-pushed the remove-innerProduct-types-test branch 2 times, most recently from ccc1195 to 60b0b0e Compare November 6, 2024 03:05

Abiji-2020 added 11 commits November 6, 2024 21:12

test: modified the types in the test from the bit folder

0330c3e

Signed-off-by: Abinand P <[email protected]>

test: updated the old types to new in the database folder

6d5fc3c

Signed-off-by: Abinand P <[email protected]>

test: updated the tests in enocde folder with the new types

4c72bb6

Signed-off-by: Abinand P <[email protected]>

test: updated the testes in polynomial folder to use the updated types

32da944

Signed-off-by: Abinand P <[email protected]>

test: updated the test in proof to use the new types

00ca01a

test: updated the tests in slice_ops to work on the updated types

d8a269a

Signed-off-by: Abinand P <[email protected]>

test: Modified the tests that are not in files named test in commitme…

b91aa4f

…nt directory Signed-off-by: Abinand P <[email protected]>

test: modified tests in the non-testfiles in database directory

e17e4ea

Signed-off-by: Abinand P <[email protected]>

test: Modified tests in the files on the math folder

1121abb

Signed-off-by: Abinand P <[email protected]>

test: modifed the tests in other files in slice_ops directory

9acfdd2

Signed-off-by: Abinand P <[email protected]>

test: remove ArrayRefExt::to_test_scalars

cebd80c

Signed-off-by: Abinand P <[email protected]>

Abiji-2020 force-pushed the remove-innerProduct-types-test branch from 60b0b0e to cebd80c Compare November 6, 2024 15:42

JayWhite2357 enabled auto-merge November 6, 2024 15:47

JayWhite2357 merged commit efdbb21 into spaceandtimelabs:main Nov 6, 2024
11 checks passed

github-actions bot added the released label Nov 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Remove inner product types from generic test. #264

fix: Remove inner product types from generic test. #264

Abiji-2020 commented Oct 12, 2024 •

edited

Loading

Abiji-2020 commented Oct 12, 2024 •

edited

Loading

Abiji-2020 commented Oct 17, 2024

JayWhite2357 commented Oct 19, 2024

JayWhite2357 left a comment

Abiji-2020 commented Oct 19, 2024

Abiji-2020 commented Oct 19, 2024

JayWhite2357 left a comment

JayWhite2357 Oct 20, 2024

Abiji-2020 Oct 20, 2024

JayWhite2357 Oct 21, 2024

Abiji-2020 Oct 22, 2024

JayWhite2357 Oct 20, 2024 •

edited

Loading

Abiji-2020 Oct 25, 2024

JayWhite2357 Oct 20, 2024

Abiji-2020 Oct 25, 2024

JayWhite2357 Oct 20, 2024

Abiji-2020 Oct 25, 2024

JayWhite2357 Oct 20, 2024

Abiji-2020 Oct 25, 2024

Abiji-2020 commented Oct 21, 2024

Abiji-2020 commented Oct 23, 2024

Abiji-2020 commented Oct 29, 2024 •

edited

Loading

Abiji-2020 commented Oct 31, 2024 •

edited

Loading

JayWhite2357 commented Nov 6, 2024

Abiji-2020 commented Nov 6, 2024

github-actions bot commented Nov 6, 2024

	/// - the backing store maps to the type [`crate::base::scalar::test_scalar::TestScalar`]
	/// - the backing store maps to the type `S`

	/// Mapped to [`TestScalar`](crate::base::scalar::test_scalar::TestScalar)
	/// Mapped to `S`

fix: Remove inner product types from generic test. #264

fix: Remove inner product types from generic test. #264

Conversation

Abiji-2020 commented Oct 12, 2024 • edited Loading

Please go through the following checklist

Rationale for this change

Notes

What changes are included in this PR?

Are these changes tested?

Abiji-2020 commented Oct 12, 2024 • edited Loading

ci failing on

Abiji-2020 commented Oct 17, 2024

JayWhite2357 commented Oct 19, 2024

JayWhite2357 left a comment

Choose a reason for hiding this comment

Abiji-2020 commented Oct 19, 2024

Abiji-2020 commented Oct 19, 2024

JayWhite2357 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayWhite2357 Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abiji-2020 commented Oct 21, 2024

Abiji-2020 commented Oct 23, 2024

Abiji-2020 commented Oct 29, 2024 • edited Loading

Abiji-2020 commented Oct 31, 2024 • edited Loading

JayWhite2357 commented Nov 6, 2024

Abiji-2020 commented Nov 6, 2024

github-actions bot commented Nov 6, 2024

Abiji-2020 commented Oct 12, 2024 •

edited

Loading

Abiji-2020 commented Oct 12, 2024 •

edited

Loading

JayWhite2357 Oct 20, 2024 •

edited

Loading

Abiji-2020 commented Oct 29, 2024 •

edited

Loading

Abiji-2020 commented Oct 31, 2024 •

edited

Loading