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

Replace 1.4 by pure secp256k1 implementation #79

Merged
merged 6 commits into from
Mar 8, 2023

Conversation

goncalo-frade-iohk
Copy link
Contributor

This is a great step since 1.4 added a few issues that are problematic to development.
Mainly it didn't allow for targets like: macOS, watchOS, tvOS.
Besides that it had issues running on arm64 Macs making any developer that intended to use the SDK to disregard arm64 iOS sim.

Now the target macOS was added and the SDK runs well in Xcode arm64 mode without the need to use Rosetta.

Tests were added to make sure the cryptographic logic is correct and checks were made before this change to acknowledge that the outputs from the new implementation were the same as the 1.4 outputs.

This logic was based on BitcoinKit framework and was changed to our needs.
This logic was based on BitcoinKit framework and was changed to our needs.
…st use secp256k1.swift lib

secp256k1 lib uses libsecp256k1 internally that is the same one used by Prism SDK1.4 internally
@goncalo-frade-iohk goncalo-frade-iohk requested review from a team March 8, 2023 12:05
@goncalo-frade-iohk goncalo-frade-iohk merged commit 8fd3e2f into main Mar 8, 2023
@goncalo-frade-iohk goncalo-frade-iohk deleted the feature/replacePrism1_4 branch March 8, 2023 12:17
Copy link
Contributor

@hamada147 hamada147 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,62 @@
//
// File.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// File.swift
// ECVerify.swift

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhh cannot forget actually to delete all this comment nice catch :)

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.

3 participants