Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update secp256k1 library to version 0.6.0 #5254

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

legleux
Copy link
Collaborator

@legleux legleux commented Jan 25, 2025

High Level Overview of Change

Updates secp256k1 to 0.6.0.

Type of Change

  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)

Test Plan

No discrepancies in unit tests were observed.

Future Tasks

This library should be integrated with CMake or Conan for easier maintenance.

@legleux legleux requested review from ximinez and bthomee January 25, 2025 00:50
Copy link

codecov bot commented Jan 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.0%. Comparing base (ed4870c) to head (5ff72ad).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5254     +/-   ##
=========================================
- Coverage     78.0%   78.0%   -0.0%     
=========================================
  Files          789     789             
  Lines        67007   67007             
  Branches      8110    8108      -2     
=========================================
- Hits         52274   52273      -1     
- Misses       14733   14734      +1     

see 5 files with indirect coverage changes

Impacted file tree graph

@bthomee
Copy link
Collaborator

bthomee commented Jan 25, 2025

@legleux was updating the library only a matter of removing all the old files and copying in the new ones, or did you have to massage one or more of them to get it to compile?

While this change may have been a small chore to create, I think as "type of change" it should be modified to something else, since the binary is definitely affected by this PR.

@Bronek Bronek changed the title chore: Update secp256k1 library Update secp256k1 library to version 0.6.0 Jan 26, 2025
@Bronek
Copy link
Collaborator

Bronek commented Jan 26, 2025

While this change may have been a small chore to create, I think as "type of change" it should be modified to something else, since the binary is definitely affected by this PR.

Agree. I renamed the PR.

@bthomee
Copy link
Collaborator

bthomee commented Jan 26, 2025

While this change may have been a small chore to create, I think as "type of change" it should be modified to something else, since the binary is definitely affected by this PR.

Agree. I renamed the PR.

If Michael changes it in the description tomorrow as well, and confirms that this is the latest code without any changes, I'm good to approve. We'll need to run it through an extended testing cycle before including into 2.4.0 though.

@Bronek Bronek added the Perf Attn Needed Attention needed from RippleX Performance Team label Jan 27, 2025
@bthomee
Copy link
Collaborator

bthomee commented Jan 27, 2025

While this change may have been a small chore to create, I think as "type of change" it should be modified to something else, since the binary is definitely affected by this PR.

Agree. I renamed the PR.

If Michael changes it in the description tomorrow as well, and confirms that this is the latest code without any changes, I'm good to approve. We'll need to run it through an extended testing cycle before including into 2.4.0 though.

I did the check myself and confirm that dropping in v0.6.0 of secp256k1 matches this PR.

@Bronek Bronek added Perf Test Desired (Optional) RippleX Perf Team should look at this PR. The PR will not necessarily wait for testing to finish and removed Perf Attn Needed Attention needed from RippleX Performance Team labels Jan 27, 2025
@legleux legleux added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Jan 27, 2025
@bthomee bthomee merged commit b6e3453 into XRPLF:develop Jan 27, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Perf Test Desired (Optional) RippleX Perf Team should look at this PR. The PR will not necessarily wait for testing to finish Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants