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

Add method to derive private keys outside the expected domain range #1499

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

MartinMinkov
Copy link
Contributor

@MartinMinkov MartinMinkov commented Mar 12, 2024

Description

Adds a method to derive public keys from private keys generated from the normal private key range. This sort of behaviour was allowed by the old client_sdk, but new checks added by mina-signer disallowed this action. To offer a backwards-compatible way to derive old private keys, we add a new function which takes the mod of the private key to ensure it's within the domain

Testing Methods

Wrote a little script which used a test keypair which worked with the old client_sdk

import { Client } from "mina-signer";
import * as CodaSDK from "@o1labs/client-sdk";

const client = new Client({
  network: "mainnet",
});

const sk = "..."; // old private key
const pk = "B62qqraqohhfoo8zAbv8UGkZ758mrBRQLw7aKcQuYEd8GfwaqfSXUym"; // corresponding public key used as a test

const oldPk = CodaSDK.derivePublicKey(sk);
const newPk = client.derivePublicKeyUnsafe(sk);

console.log(pk);
console.log(oldPk);
console.log(newPk);
console.log(oldPk === newPk);

Adds a `derivePublicKeyUnsafe` method which allows backwards
compatability with the old client_sdk library, which allowed private
keys to be out of the domain of the Pallas curve. We expose this
function which takes the mod of the domain to ensure it's within the
expected private key range.
* By performing a modulus operation on the key, it ensures that keys
* from the older client_sdk can be made compatible.
*/
function convertPrivateKeyToBase58WithMod(keyBase58: string): string {
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 we should also expose this function on Client, because people need to use it when creating signatures with their out of domain private keys

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, done!

@@ -99,6 +99,30 @@ class Client {
return PublicKey.toBase58(publicKey);
}

/**
* Derives the public key corresponding to a given private key, ensuring compatibility with keys from the older [client_sdk](https://www.npmjs.com/package/@o1labs/client-sdk) libraries by converting the private key into the domain of the Pallas curve if necessary.
Copy link
Contributor

Choose a reason for hiding this comment

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

hey @MartinMinkov I think it was other tools, not our own client sdk, that created out of domain keys, so would reword this comment to definitely not imply that

Copy link
Contributor

Choose a reason for hiding this comment

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

the old client sdk just accepted them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, fixed!

… backwards compatibility with older keys

This method allows older keys that do not fit the domain of the Pallas curve to be used by performing a modulus operation on the key.
@MartinMinkov MartinMinkov force-pushed the fix/mina-signer-privatekey branch from 816300c to 2bbf880 Compare March 12, 2024 18:30
@MartinMinkov MartinMinkov force-pushed the fix/mina-signer-privatekey branch from 2bbf880 to 1247ad1 Compare March 12, 2024 18:47
@MartinMinkov MartinMinkov merged commit ee3bd4a into main Mar 12, 2024
13 checks passed
@MartinMinkov MartinMinkov deleted the fix/mina-signer-privatekey branch March 12, 2024 21:01
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