feat: range proof sum optimization and cleanup #101
Conversation
There was a problem hiding this comment.
- Nice, the addition explains a really nice maths trick and makes the implementation more readily understandable. I have some comments below to make the explanation more palatable.
- I think we need to retain the previous
## Comparative performancesection but remove all the tables and discuss relative performance in general terms.
|
What kind of comparative performance would you be looking for here? And should updated performance data be a separate PR? I removed it here only because it was very out of date. It's not clear to me if an RFC is the right place for that. In my opinion, if the comparative performance is for functions within this repository, those should be included in benchmarks that the user can run themselves to get accurate absolute numbers for their system. Comparative performance between protocols using operation counts is fine, since those typically scale independently of the target system. Comparative performance between implementations of different protocols seems sketchier to me, since different libraries or algorithmic optimizations might not make it a fair fight. That being said, I think limited informal comparisons to give the user a broad overview of implementations can be fine if presented as such (this is done in the Bulletproofs+ documentation to a give a very rough comparison to the |
I think we could include what the maths predict, for example,
|
|
Should that go into the RFC, or into the repository documentation? The RFC's "goals" section states the current intention is to describe Tari-specific implementation details. |
|
I would change the goal to accommodate discussing |
Does this commit Bulletproofs+ repository contributors also to keep RFC benchmarks up to date? |
|
Merging to assist auditors |
Description --- Updates range proof [RFC-0181](https://rfc.tari.com/RFC-0181_BulletproofsPlus.html) to add comparative performance data and improve notation. Closes #102. Motivation and Context --- An [earlier PR](#101) removed outdated benchmark data, but it was [suggested](#101 (comment)) to include reasonable comparative performance data against Bulletproofs. This data has been added for both proof size and verification complexity. Some notation was incomplete or incorrect. In particular, sum and index bounds were left implicit in many places, and indexing relating to mask recovery was incorrect. Both of these have been addressed. How Has This Been Tested? --- The changes build and appear to render correctly.
Description
Adds details of a Bulletproofs+ optimization to RFC-0181. Removes old benchmark data.
Motivation and Context
The Bulletproofs+ implementation contains a particular optimization used when computing a vector sum. This optimization is not particularly obvious, as it uses some fun arithmetic and a fact about geometric series partial sums.
This PR adds details about the math behind the optimization to RFC-0181. It also removes outdated benchmark data, similarly to earlier removal in the Bulletproofs+ repository.
How Has This Been Tested?
The changes build and render properly. The underlying mathematics should be manually checked for correctness.