Skip to content

chore: Remove redundant bls12-381 constants and cleanup naming#2235

Merged
rakita merged 7 commits intobluealloy:mainfrom
kevaundray:kw/remove-extraneous-constants
Mar 18, 2025
Merged

chore: Remove redundant bls12-381 constants and cleanup naming#2235
rakita merged 7 commits intobluealloy:mainfrom
kevaundray:kw/remove-extraneous-constants

Conversation

@kevaundray
Copy link
Contributor

@kevaundray kevaundray commented Mar 17, 2025

Some constants were being used as aliases for other constants. This PR removes those aliases and directly references the underlying constant.

For example, instead of having a constant called G1_OUTPUT_LENGTH which will be used when we are outputting a G1 element, we instead just use G1_LENGTH and the fact that it is an output is inferred by it being a return type.

@kevaundray kevaundray marked this pull request as ready for review March 17, 2025 17:08
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 17, 2025

CodSpeed Performance Report

Merging #2235 will degrade performances by 2.7%

Comparing kevaundray:kw/remove-extraneous-constants (820bb54) with main (5a2ff70)

Summary

❌ 1 regressions
✅ 7 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
precompile bench | ecrecover precompile 194.8 µs 200.2 µs -2.7%

pub const PAIRING_OFFSET_BASE: u64 = 37700;
pub const MSM_MULTIPLIER: u64 = 1000;
// TODO: Why does this have PAIRING twice?
pub const PAIRING_PAIRING_MULTIPLIER_BASE: u64 = 32600;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't entirely sure why this was named PAIRING_PAIRING -- can revert if this was intentional

Copy link
Member

Choose a reason for hiding this comment

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

It was not intentional, it is good to have just one pairing in name

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm

@rakita rakita merged commit 49d4dca into bluealloy:main Mar 18, 2025
28 of 29 checks passed
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