Skip to content

fix: Replace ECAlgorithm with EllipticCurve#5

Merged
djones6 merged 4 commits intomasterfrom
curve
Mar 5, 2019
Merged

fix: Replace ECAlgorithm with EllipticCurve#5
djones6 merged 4 commits intomasterfrom
curve

Conversation

@Andrew-Lees11
Copy link
Copy Markdown
Contributor

This pull request replaces ECAlgorithm with Curve.

This is now a public struct that has a public representation of the curve for comparisons and checking on your key and all the internal variables that are used for encrypting and signing.

This was done to replace the "CurveID" String with an Struct version of enum limiting the comparable options.

@Andrew-Lees11 Andrew-Lees11 requested a review from djones6 March 5, 2019 13:28
Copy link
Copy Markdown
Contributor

@djones6 djones6 left a comment

Choose a reason for hiding this comment

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

Looks good, though this is a breaking change.

public class ECPrivateKey {
/// The Elliptic curve this key was generated from.
public let curveId: String
public let curve: Curve
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: change to public API

}

/// A prime256v1 curve.
public static let prime256v1 = Curve.p256
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These are a new public API

public class ECPublicKey {
/// The Elliptic curve this key was generated from.
public let curveId: String
public let curve: Curve
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: change to public API

@Andrew-Lees11
Copy link
Copy Markdown
Contributor Author

We have kept curveId so that this is no longer a breaking change.

/// A String description of the curve this key was generated from.
public var curveId: String {
return curve.description
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we leave the curveId as a constant and set as before during the init?

@djones6 djones6 changed the title fix: Replace ECAlgorithm with Curve fix: Replace ECAlgorithm with EllipticCurve Mar 5, 2019
@djones6 djones6 merged commit 6b7bc9e into master Mar 5, 2019
@Andrew-Lees11 Andrew-Lees11 deleted the curve branch March 5, 2019 14:45
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