Skip to content

Conversation

just-erray
Copy link

@just-erray just-erray commented Sep 30, 2025

Changes the representation and commitment of polynomials to point value form for better scaling, especially for evaluating polynomials with the domain of EC points. Assuming there are a revealed points and b hidden points, previous algorithm used around 2 * (a + b) * a * log_2(a) (log from exponentiation) EC additions, this version uses around a * b * 256 (256 from exponentiation). (Although it doesn't look like it, it might be slower with the current parameters, so further performance testing before merging would be preferable.) (Whole work is on top of #59)

@sander2
Copy link

sander2 commented Sep 30, 2025

On my machine, in a quick test, ShareCommits::verify takes (avg over 100 iterations):

  • baseline on main: 92.131429ms
  • this pr 91.101107ms

@just-erray
Copy link
Author

just-erray commented Sep 30, 2025

Just added manual tables for MSM since I don't know any MSM libraries or more optimized versions off the top of my head. Current numbers are like this on my machine: (again, average of 100 iterations):

  • Tabled: 49.943901ms
  • Without Tables: 67.744429ms

@just-erray
Copy link
Author

Added another optimization to MSM, now it's like this:

  • With optimized tables: 30.261179ms
  • Without tables: 66.781639ms

@just-erray
Copy link
Author

Last commit also should add another ~3% decrease to the time.

@cyphersnake
Copy link
Collaborator

Full Flow

state poly count time
before 81_472 21m28s
after 81_472 11m59s

@cyphersnake
Copy link
Collaborator

Part of #62

Copy link

@sander2 sander2 left a comment

Choose a reason for hiding this comment

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

LGTM, but I was just thinking - would it be feasible to merge eval_at_suffix_points and lagrange_interpolate_whole_polynomial? That'd save some duplication

let mut result = vec![T::zero(); n_points];

for i in 0..n_known {
let mut table = vec![];
Copy link

Choose a reason for hiding this comment

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

Nit: maybe worth initializing with Vec::with_capacity to avoid some reallocations?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The main thing is that the maths is essentially correct. Next, I will refactor here to achieve greater performance.

@just-erray
Copy link
Author

I think that it is possible but would be more trouble than it's worth, since there are many special optimizations in similarly functioning parts.

@cyphersnake cyphersnake merged commit b80a841 into BitVM:main Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants