Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use precomputation for default commitments #136

Merged
merged 4 commits into from
Oct 25, 2022

Conversation

AaronFeickert
Copy link
Contributor

@AaronFeickert AaronFeickert commented Oct 11, 2022

This work adds precomputation support for Pedersen commitments that use a provided generator. Basepoint tables are created via lazy_static alongside the generators themselves. When computing a commitment using the default generators, the corresponding tables are used. Commitments using any other generators are unchanged.

Closes issue #135.

@AaronFeickert AaronFeickert changed the title Use precomputation for default commitments feat: use precomputation for default commitments Oct 11, 2022
@AaronFeickert
Copy link
Contributor Author

One downside to this implementation is that it generates tables for all provided points, but only checks for the default generators when computing Pedersen commitments. Checking for any combination of the standard basepoint (G) and one of the provided generators seems like unnecessary computation that won't target any practical use case.

Should we only generate a table for the default H point?

stringhandler
stringhandler previously approved these changes Oct 19, 2022
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - one minor comment

src/ristretto/constants.rs Outdated Show resolved Hide resolved
sdbondi
sdbondi previously approved these changes Oct 20, 2022
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Thanks for adding a bench :)

arr[i] = RISTRETTO_NUMS_POINTS_COMPRESSED[i].decompress().unwrap();
}
arr
};

/// Precomputation table for the first point, which is used as the default commitment generator
pub static ref RISTRETTO_NUMS_TABLE_0: RistrettoBasepointTable = RistrettoBasepointTable::create(&RISTRETTO_NUMS_POINTS[0]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, we only need the first entry of RISTRETTO_NUMS_POINTS to defined RISTRETTO_NUM_TABLE_0, why do we need then an array RISTRETTO_NUMS_POINTS_COMPRESSED of len 10 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The library already contained this set of points for implementers to use. The default Pedersen factory uses the standard base point G (from the upstream curve library) and the first point in the set as its generators. The extended Pedersen factories use additional points from the set.

An earlier version of this PR generated precomputation tables for all the points in the set, but it was decided this was wasted computation at this point, since only the default Pedersen factory is used in the codebase (and the extended Pedersen factories use multiscalar multiplication for commitment evaluation, which would require different precomputation table structures and architecture).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CjS77 had mentioned elsewhere that a future strategy could be to keep some expensive curve point generation operations (like hash-based generation and precomputation) within the commitment factories, and then ensure that these factories are generated only once within the implementing codebase. This is already how the much larger set of range proof generators is handled. However, default Pedersen factory copies are currently generated on the fly more often, presumably because it was cheap to do so.

@stringhandler stringhandler merged commit acdcee6 into tari-project:main Oct 25, 2022
@AaronFeickert AaronFeickert deleted the commitment-precomp branch October 25, 2022 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose and use precomputation tables for commitment generators
5 participants