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

upgrade secp256kfun #1466

Merged
merged 20 commits into from
May 27, 2024
Merged

upgrade secp256kfun #1466

merged 20 commits into from
May 27, 2024

Conversation

delta1
Copy link
Collaborator

@delta1 delta1 commented Dec 11, 2023

re: #1459

upgrades the secp256kfun dependency, the previously pinned version fails to compile on rust 1.74

fixes a few resulting issues, and some minor cleanup refactoring

also adds a new CI job to run cargo check on rust stable

also closes:

@binarybaron
Copy link
Collaborator

I will need some time to review this as this is quite critical code and I'm not too familiar with the library.

@ikmckenz
Copy link
Contributor

Out of curiosity, why resolve on 0.24.1 instead of the most recent release (0.28)?

@delta1
Copy link
Collaborator Author

delta1 commented Dec 21, 2023

Out of curiosity, why resolve on 0.24.1 instead of the most recent release (0.28)?

looks like you’re referring to secp256k1, this PR is upgrading secp256kfun. as for the versions, from the lockfile it looks like bitset still has a dependency on 0.24 of secp256k1, and secp256kfun is still depending on 0.27 of secp256k1

@binarybaron
Copy link
Collaborator

LGTM but have you completed a swap using the new CLI version and an old ASB version yet?

@delta1
Copy link
Collaborator Author

delta1 commented Dec 29, 2023

LGTM but have you completed a swap using the new CLI version and an old ASB version yet?

no i have not

@delta1
Copy link
Collaborator Author

delta1 commented Feb 5, 2024

updated to include #1546 #1547

@delta1
Copy link
Collaborator Author

delta1 commented Feb 15, 2024

updated to include #1549 #1550

@delta1
Copy link
Collaborator Author

delta1 commented Feb 23, 2024

update to include #1556 and #1557

also sets the revision in the Cargo.toml manifest

@binarybaron
Copy link
Collaborator

Is this 100% backwards compatible in terms of the database and p2p protocols?

@delta1
Copy link
Collaborator Author

delta1 commented Mar 1, 2024

@ikmckenz if you have a moment, please test this by building this branch asb/swap, and use those binaries on testnet performing a swap against the previous version

@delta1
Copy link
Collaborator Author

delta1 commented Mar 4, 2024

Sigh I don't have any testnet bitcoin to test with 😩 @binarybaron any donations?

tb1qttry3wfcwcf7gyv26a5elt9hcran7nsnfj9zgj

@delta1
Copy link
Collaborator Author

delta1 commented Mar 6, 2024

Is this 100% backwards compatible in terms of the database and p2p protocols?

tested a number of testnet swaps between this version of asb and latest swap release, as well as this version of swap with latest asb release

happy for this change to be a version change to 0.13

@binarybaron
Copy link
Collaborator

No need. I'll trust you on that.

@binarybaron
Copy link
Collaborator

bors r+

@linsui
Copy link

linsui commented Mar 16, 2024

Why do you use the git repo instead of 0.10.0 on crates.io?

@delta1
Copy link
Collaborator Author

delta1 commented Mar 16, 2024

Why do you use the git repo instead of 0.10.0 on crates.io?

good question, i think that’s a hangover from where we were ahead of their releases to crates.io

i’m going to rework this PR to use the crates version, thanks for the suggestion

Copy link
Collaborator

@binarybaron binarybaron left a comment

Choose a reason for hiding this comment

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

LGTM!

@delta1
Copy link
Collaborator Author

delta1 commented May 27, 2024

includes a fix for #1645

@delta1 delta1 merged commit 7968633 into comit-network:master May 27, 2024
@delta1 delta1 deleted the issues-1459 branch May 27, 2024 09:03
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.

4 participants