Skip to content

fix: prevent panic on empty vectors in AMD64 vector operations#727

Merged
gbotrel merged 12 commits intoConsensys:fix/emptyvectorfrom
Fibonacci747:fix/vector-empty-panic-prevention
Sep 2, 2025
Merged

fix: prevent panic on empty vectors in AMD64 vector operations#727
gbotrel merged 12 commits intoConsensys:fix/emptyvectorfrom
Fibonacci747:fix/vector-empty-panic-prevention

Conversation

@Fibonacci747
Copy link
Copy Markdown
Contributor

Fix panic when calling Add/Sub methods on empty vectors in AMD64-optimized
vector implementations across multiple elliptic curve packages.

  • Add length check for empty vectors before accessing &a[0] in Add/Sub methods
  • Fix InnerProduct validation order to check lengths before empty check
  • Ensure consistent behavior with generic implementations
  • Apply fixes to all affected curve packages: grumpkin, bn254, stark-curve,
    bls12-381, bls12-377, bls24-315, bls24-317 (both fp and fr packages)
  • Add comprehensive test coverage for empty vector operations

This prevents runtime panics that could occur when processing empty vectors
in high-performance cryptographic operations, improving robustness of the
vector math library.

@gbotrel
Copy link
Copy Markdown
Collaborator

gbotrel commented Aug 22, 2025

Good catch, thanks for the PR.
For your PR to pass the CI, do you mind doing your fix in: vector_ops_asm.go and running go generate ./...?
Also would be nice to have a unit test to accompany it, but can do later 👍

@gbotrel
Copy link
Copy Markdown
Collaborator

gbotrel commented Aug 22, 2025

I see you already had a commit with tests; similarly for the CI to pass, this needs to go in field/generator/internal/templates/element/tests_vector.go to be generated for all implementations

@Fibonacci747
Copy link
Copy Markdown
Contributor Author

Fibonacci747 commented Aug 22, 2025

I see you already had a commit with tests; similarly for the CI to pass, this needs to go in field/generator/internal/templates/element/tests_vector.go to be generated for all implementations

should i close then this PR and create new PR with changes in correct files? or i should just revert changes in these files to make changes in vector_ops_asm.go and tests_vector.go

@gbotrel
Copy link
Copy Markdown
Collaborator

gbotrel commented Aug 22, 2025

we do squash merge so you can work on the same PR (go generate ./... in the root will overwrite your changes anyway -- all these files are generated from templates)

@Fibonacci747
Copy link
Copy Markdown
Contributor Author

we do squash merge so you can work on the same PR (go generate ./... in the root will overwrite your changes anyway -- all these files are generated from templates)

Done

@gbotrel gbotrel changed the base branch from master to fix/emptyvector September 2, 2025 13:39
@gbotrel gbotrel merged commit 608496d into Consensys:fix/emptyvector Sep 2, 2025
3 of 4 checks passed
gbotrel added a commit that referenced this pull request Sep 2, 2025
gbotrel added a commit that referenced this pull request Sep 2, 2025
Co-authored-by: Fibonacci747 <albertofibonacci12@gmail.com>
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.

2 participants