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: remove partial precomputation #129

Merged

Conversation

AaronFeickert
Copy link
Contributor

@AaronFeickert AaronFeickert commented Mar 22, 2024

Partial precomputation is the sole reason for maintaining a custom curve library fork, which has proven to be a headache and limits compatibility with other libraries.

This PR removes partial precomputation altogether. If you supply parameters with more inner-product generators than you need, padding is used. This incurs an efficiency hit, but only in this particular case. For the use cases in the Tari ecosystem, this is not an issue.

As a result of this change, we also switch back to the latest version of the upstream curve library and simplify some scalar exponentiation operations. The audit report is also updated to note the curve library dependency change.

Closes #128. Closes #93. Closes #96.

@AaronFeickert
Copy link
Contributor Author

Note that this PR was written such that if partial precomputation is added upstream, it will be easy to support by effectively reverting the changes here.

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

Looks good. Maybe we can add a unit test for the case where If you supply parameters with more inner-product generators than you need, padding is used.?

src/range_proof.rs Outdated Show resolved Hide resolved
@AaronFeickert
Copy link
Contributor Author

AaronFeickert commented Mar 26, 2024

Updated to ensure that at least version 4.1.0 of curve25519-dalek is used. This is required to support the group feature that lets us use built-in efficient scalar exponentiation.

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

ACK

@AaronFeickert
Copy link
Contributor Author

Rebased, with some commit cleanup.

@hansieodendaal hansieodendaal merged commit 58409fa into tari-project:main Mar 29, 2024
7 checks passed
@AaronFeickert AaronFeickert deleted the no-more-partial-precomp branch March 29, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants