Skip to content
This repository has been archived by the owner on Feb 19, 2024. It is now read-only.

Use uncompressed representation for ECPublicKey equality #178

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

LukasGasior1
Copy link
Contributor

Pull Request Template

Title

Use uncompressed representation for ECPublicKey equality

Description

Previously, if we initiated two instances of a public key of the same EC point with compressed and uncompressed byte representation (using ECPublicKey.fromBytes) we would end up with two unequal objects. This PR changes it so that they're equal. Precisely: two public keys are equal if their uncompressed byte representation is equal. It also adds a validation that the bytes passed actually represent a valid point. This change also revealed that some classes were implicitly relying and being given a key with compressed bytes representation (RadixAddress for example).

@@ -52,7 +52,7 @@ public void when_an_address_is_created_with_same_string__they_should_be_equal_an

@Test
public void address_from_key_and_magical() throws PublicKeyException {
String publicKeyHexString = "03000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed because these bytes didn't represent a valid ec point (which previously wasn't validated)

Copy link
Contributor

@siy siy left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -70,7 +70,7 @@ public static ECKeyPair generateNew() {
ECPrivateKeyParameters privParams = (ECPrivateKeyParameters) keypair.getPrivate();
ECPublicKeyParameters pubParams = (ECPublicKeyParameters) keypair.getPublic();

final ECPublicKey publicKey = ECPublicKey.fromBytes(pubParams.getQ().getEncoded(true));
final ECPublicKey publicKey = new ECPublicKey(pubParams.getQ());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to have factory method for this rather than call constructor directly.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.2% 90.2% Coverage
0.0% 0.0% Duplication

@LukasGasior1 LukasGasior1 merged commit ccf4f3a into rc/1.0-beta.29 Apr 15, 2021
@talekhinezh talekhinezh deleted the feature/change-pub-key branch August 13, 2021 16:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants