Feat(syscall): add altbn128 g1 & g2 compression #32870
Conversation
02912d7 to
c496f7a
Compare
Codecov Report
@@ Coverage Diff @@
## master #32870 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 797 798 +1
Lines 216347 216572 +225
=========================================
+ Hits 177369 177499 +130
- Misses 38978 39073 +95 |
6670a6e to
bf4fc37
Compare
58dec71 to
2d73be6
Compare
There was a problem hiding this comment.
Everything is all straightforward and looks fine to me. Some small nits below. I'd also prefer if alt_bn128 and alt_bn128_compression modules in the sdk are refactored into their own separate module.
alt_bn128.rs --> alt_bn128/mod.rs
alt_bn128.rs --> alt_bn128/compression.rs
I haven't tested the CU units yet, but I will do so tomorrow and approve it if there are no issues found. Please rebase when you get a chance. Thanks!
| type G1 = ark_bn254::g1::G1Affine; | ||
| type G2 = ark_bn254::g2::G2Affine; | ||
|
|
||
| pub fn alt_bn128_compression_g1_decompress( |
There was a problem hiding this comment.
Nit: Let's keep this simple and use alt_bn128_g1_decompress. I think the compression part will be evident from the module this function is imported from. Same with all the functions below.
| #[error("Unexpected error")] | ||
| UnexpectedError, | ||
| #[error("Failed to decompress g1")] | ||
| DecompressingG1Failed, |
There was a problem hiding this comment.
Nit: This is really minor, but can we name it G1DecompressionFailed instead to make it slightly more natural? Same with the error variants below.
2d73be6 to
790e136
Compare
|
Thanks for the review!
|
|
Looks much cleaner now. Thanks! Unfortunately, there seems to be conflict in |
35b1409 to
790e136
Compare
|
Hey! It seems like you are removing the feature |
still fixing tests for point of infinity feat: proof compression syscall working add rust test to ci remove prints added c test added sycall pricing
790e136 to
37d4cdd
Compare
|
Hey, @samkim-crypto it's rebased. |
|
Thanks for all the changes! I want to avoid merging during the weekend if possible when other engineers are resting. I will merge the PR early on Monday. Thank you so much for the patience! |
* solana-program - altbn128: add g1 & g2 compression still fixing tests for point of infinity feat: proof compression syscall working add rust test to ci remove prints added c test added sycall pricing * fixed ci checks * refactored altbn128 and compression
Problem
Summary of Changes
Implementation Notes
Performance
CU calculations are based on 1 cu per 33ns compute time.
g1 compress: 1us * 1000 / 33 = 30 CU
g1 decompress: 13.154 us * 1000 / 33 = 398 CU
g2 compress: 2.8543 us * 1000 / 33 = 86 CU
g2 decompress: 449.13 us * 1000 / 33 = 13610 CU
See this branch for the bench implementation.
g1_decompress time: [13.089 us 13.154 us 13.237 us]
change: [-10.850% -10.279% -9.6768%] (p = 0.00 < 0.05)
Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
1 (1.00%) low mild
3 (3.00%) high mild
7 (7.00%) high severe
g1_compress time: [999.77 ns 1.0049 us 1.0110 us]
change: [-3.6976% -3.0299% -2.3047%] (p = 0.00 < 0.05)
Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
1 (1.00%) low mild
2 (2.00%) high mild
9 (9.00%) high severe
g2_decompress time: [447.25 us 449.13 us 451.51 us]
change: [-14.401% -13.723% -13.047%] (p = 0.00 < 0.05)
Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
8 (8.00%) high mild
7 (7.00%) high severe
g2_compress time: [2.8443 us 2.8543 us 2.8695 us]
change: [-6.2482% -5.4817% -4.7391%] (p = 0.00 < 0.05)
Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
5 (5.00%) high mild
10 (10.00%) high severe