search

spaceandtimelabs / sxt-proof-of-sql

Space and Time | Proof of SQL
Other
1.59k stars 71 forks source link

Remove InnerProduct types from generic tests within base directory #230

Open stuarttimwhite opened 2 weeks ago

stuarttimwhite commented 2 weeks ago

Background and Motivation

Currently within the base directory (proof-of-sql/src/base), there is reference to 3 types:

  1. Curve25519Scalar
  2. RistrettoPoint
  3. InnerProductProof.

Any tests (within base) that use one of these 3 types to test a different type or function should be modified to remove these types. Note that some tests within base are actually meant to test one of these 3 types. These may be left alone for this issue. Non-test code should not be touched.

Changes Required

For any of the 3 types that will be removed, replace each instance as follows:

  1. Curve25519Scalar -> TestScalar
  2. RistrettoPoint -> NaiveCommitment
  3. InnerProductProof -> TestEvaluationProof

We recommend splitting the PR into 4 commits:

  1. Scalar replacement
  2. Commitment replacement (RistrettoPoint -> NaiveCommitment)
  3. Proof replacement.
  4. Everything else (to get build and tests passing).

NOTE: https://github.com/spaceandtimelabs/sxt-proof-of-sql/pull/197 is a partially completed version of this that is a bit out of date. It may or may not be a helpful reference point.

JayWhite2357 commented 2 weeks ago

/bounty $50

algora-pbc[bot] commented 2 weeks ago

💎 $50 bounty • Space and Time

Steps to solve:

  1. Start working: (Optional) Comment /attempt #230 with your implementation plan. Note: we will only assign an issue if you include an implementation plan with a time estimate. Additionally, to be assigned an issue, you must have previously contributed to the project. You can still work on an issue and submit a PR without being assigned.
  2. Submit work: Create a pull request including /claim #230 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to spaceandtimelabs/sxt-proof-of-sql!

Add a bounty • Share on socials

Attempt Started (GMT+0) Solution
🟢 @Abiji-2020 Oct 11, 2024, 2:39:46 PM #264
onyedikachi-david commented 2 weeks ago

/attempt #230

algora-pbc[bot] commented 2 weeks ago

@onyedikachi-david: We appreciate your enthusiasm but since you already have 3 active bounty attempts, we're going to keep this open for other contributors to attempt. 🫡

Abiji-2020 commented 1 week ago

I would like to work on this issue . In this we need to modify also the tests in each of the individual directories too in the modified form right ?

/attempt #230

JayWhite2357 commented 1 week ago

I would like to work on this issue . In this we need to modify also the tests in each of the individual directories too in the modified form right ?

/attempt #230 Options

Only the tests should be modified. See #197, which is essentially an almost completed version of this, but just needs to be finished.

Abiji-2020 commented 1 week ago

I would like to work on this issue . In this we need to modify also the tests in each of the individual directories too in the modified form right ? /attempt #230 Options

Only the tests should be modified. See #144, which is essentially an almost completed version of this, but just needs to be finished.

I got that the tests only should be modified in the base and we need not modify the tests that test these types. Am i correct?

JayWhite2357 commented 1 week ago

I would like to work on this issue . In this we need to modify also the tests in each of the individual directories too in the modified form right ? /attempt #230 Options

Only the tests should be modified. See #144, which is essentially an almost completed version of this, but just needs to be finished.

I got that the tests only should be modified in the base and we need not modify the tests that test these types. Am i correct?

Correct. Also, I realized that I had referenced the wrong PR. The partially completed on is #197.

Abiji-2020 commented 1 week ago

I would like to work on this issue . In this we need to modify also the tests in each of the individual directories too in the modified form right ? /attempt #230 Options

Only the tests should be modified. See #144, which is essentially an almost completed version of this, but just needs to be finished.

I got that the tests only should be modified in the base and we need not modify the tests that test these types. Am i correct?

Correct. Also, I realized that I had referenced the wrong PR. The partially completed on is #197.

Ok so in current modification is it okay that i creatr separate PR for each types so that it will be easy to review than checking all of them at once.

JayWhite2357 commented 1 week ago

Ok so in current modification is it okay that i creatr separate PR for each types so that it will be easy to review than checking all of them at once.

Thats a good idea, but I'm pretty sure that there are some tests that require changing all 3 types at once.

The other option would be to create a separate commit for each type. That way, the reviewer can look at individual commits.

Alternatively, you could do smaller PRs/commits that do only one module/folder at a time.

Abiji-2020 commented 1 week ago

Ok so in current modification is it okay that i creatr separate PR for each types so that it will be easy to review than checking all of them at once.

Thats a good idea, but I'm pretty sure that there are some tests that require changing all 3 types at once.

The other option would be to create a separate commit for each type. That way, the reviewer can look at individual commits.

Alternatively, you could do smaller PRs/commits that do only one module/folder at a time.

Ok than let me do smaller commits that do only module/ folder, then it will be easier for both of us...

Abiji-2020 commented 1 week ago

@JayWhite2357 https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/crates/proof-of-sql/src/base/slice_ops/slice_cast_test.rs in this test there are some test_cases where we are converting them to the
Curve25519Scalar type they should not be modified ? right

algora-pbc[bot] commented 1 week ago

💡 @Abiji-2020 submitted a pull request that claims the bounty. You can visit your bounty board to reward.

Abiji-2020 commented 1 week ago

I would like to work on this issue . In this we need to modify also the tests in each of the individual directories too in the modified form right ? /attempt #230 Options

Only the tests should be modified. See #197, which is essentially an almost completed version of this, but just needs to be finished.

@stuarttimwhite @JayWhite2357 @iajoiner in pr #197 we are not modifying the commitmentEvalutionProof to implement the TestEvaluationProof, and we are also modifing the tests from a non-test file. Should they be addressed in this issue ??

JayWhite2357 commented 5 days ago

@Abiji-2020

we are not modifying the commitmentEvalutionProof to implement the TestEvaluationProof

If I understand the question, correct. We should not modify the tests that are testing the InnerProductProof. See #232 for a bit more context.

we are also modifing the tests from a non-test file

I'm not sure what file you are referring to here. Can you be more specific?

JayWhite2357 commented 5 days ago

@JayWhite2357 https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/crates/proof-of-sql/src/base/slice_ops/slice_cast_test.rs in this test there are some test_cases where we are converting them to the Curve25519Scalar type they should not be modified ? right

I would modify only the ones that don't use the curve25519_dalek::scalar::Scalar.