Skip to content

Commit

Permalink
Use legacy ed25519 as the default key scheme (#163)
Browse files Browse the repository at this point in the history
* use legcy ed25519 as default key scheme

* comments and changelog
  • Loading branch information
0xmaayan authored Nov 2, 2023
1 parent 0cf0987 commit 0c86625
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 80 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to the Aptos TypeScript SDK will be captured in this file. T

- Support derive account from private key `Account.fromPrivateKey()`
- Derive account from derivation path secp256k1 support
- Default Account generation to Legacy Ed25519

## 0.0.3 (2023-10-31)

Expand Down
38 changes: 23 additions & 15 deletions src/core/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ export class Account {
*/
private constructor(args: { privateKey: PrivateKey; address: AccountAddress; legacy?: boolean }) {
const { privateKey, address, legacy } = args;
const useLegacy = legacy ?? true;

// Derive the public key from the private key
this.publicKey = privateKey.publicKey();

// Derive the signing scheme from the public key
if (this.publicKey instanceof Ed25519PublicKey) {
if (legacy) {
if (useLegacy) {
this.signingScheme = SigningScheme.Ed25519;
} else {
this.publicKey = new AnyPublicKey(this.publicKey);
Expand All @@ -96,7 +97,8 @@ export class Account {

/**
* Derives an account with random private key and address.
* Default generation is using the Unified flow with ED25519 key
*
* Default generation is using the Legacy ED25519 key
*
* @param args optional. Unify GenerateAccount type for Legacy and Unified keys
*
Expand All @@ -122,27 +124,31 @@ export class Account {
* @returns Account with the given signing scheme
*/
static generate(args?: GenerateAccount): Account {
const useLegacy = args?.legacy ?? true;

let privateKey: PrivateKey;
let publicKey: PublicKey;

switch (args?.scheme) {
case SigningSchemeInput.Secp256k1Ecdsa:
privateKey = Secp256k1PrivateKey.generate();
publicKey = new AnyPublicKey(privateKey.publicKey());
break;
default:
privateKey = Ed25519PrivateKey.generate();
}

let publicKey = privateKey.publicKey();
if (!args?.legacy) {
publicKey = new AnyPublicKey(privateKey.publicKey());
if (useLegacy === false) {
publicKey = new AnyPublicKey(privateKey.publicKey());
} else {
publicKey = privateKey.publicKey();
}
}

const address = new AccountAddress({
data: Account.authKey({
publicKey,
}).toUint8Array(),
});
return new Account({ privateKey, address, legacy: args?.legacy });
return new Account({ privateKey, address, legacy: useLegacy });
}

/**
Expand All @@ -152,21 +158,22 @@ export class Account {
* that has not had its authentication key rotated.
*
* @param privateKey PrivateKey - private key of the account
* @param args.legacy optional. If set to true, the keypair generated is a Legacy keypair. Defaults
* to generating a Unified keypair
* @param args.legacy optional. If set to false, the keypair generated is a Unified keypair. Defaults
* to generating a Legacy Ed25519 keypair
*
* @returns Account
*/
static fromPrivateKey(args: { privateKey: PrivateKey; legacy?: boolean }): Account {
const { privateKey, legacy } = args;
const useLegacy = legacy ?? true;

let publicKey;
if (privateKey instanceof Secp256k1PrivateKey) {
// Secp256k1 single sender
publicKey = new AnyPublicKey(privateKey.publicKey());
} else if (privateKey instanceof Ed25519PrivateKey) {
// legacy Ed25519
if (legacy) {
if (useLegacy) {
publicKey = privateKey.publicKey();
} else {
// Ed25519 single sender
Expand All @@ -178,7 +185,7 @@ export class Account {

const authKey = AuthenticationKey.fromPublicKey({ publicKey });
const address = new AccountAddress({ data: authKey.toUint8Array() });
return new Account({ privateKey, address, legacy });
return new Account({ privateKey, address, legacy: useLegacy });
}

/**
Expand All @@ -187,8 +194,8 @@ export class Account {
*
* @param args.privateKey PrivateKey - the underlying private key for the account
* @param args.address AccountAddress - The account address the `Account` will sign for
* @param args.legacy An optional flag to indicate that the authentication key derivation should
* use the legacy Ed25519 scheme. Defaults to false
* @param args.legacy optional. If set to false, the keypair generated is a Unified keypair. Defaults
* to generating a Legacy Ed25519 keypair
*
* @returns Account
*/
Expand All @@ -209,7 +216,8 @@ export class Account {
* or non-hardened path (e.g. m/44'/637'/0'/0/0) for secp256k1
* Detailed description: {@link https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki}
* @param args.mnemonic the mnemonic seed phrase of the account
* @param args.legacy optional. To indicate whether to use a legacy Ed25519
* @param args.legacy optional. If set to false, the keypair generated is a Unified keypair. Defaults
* to generating a Legacy Ed25519 keypair
*
* @returns Account
*/
Expand Down
5 changes: 3 additions & 2 deletions src/core/crypto/secp256k1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,9 @@ export class Secp256k1PrivateKey extends PrivateKey {
* @returns The generated key
*/
private static fromDerivationPathInner(path: string, seed: Uint8Array): Secp256k1PrivateKey {
const { privateKey } = HDKey.fromMasterSeed(seed).derive(path);

const { privateKey, publicKey } = HDKey.fromMasterSeed(seed).derive(path);
console.log(privateKey);

Check warning on line 205 in src/core/crypto/secp256k1.ts

View workflow job for this annotation

GitHub Actions / run-tests

Unexpected console statement
console.log(publicKey);

Check warning on line 206 in src/core/crypto/secp256k1.ts

View workflow job for this annotation

GitHub Actions / run-tests

Unexpected console statement
// library returns privateKey as Uint8Array | null
if (privateKey === null) {
throw new Error("Invalid key");
Expand Down
13 changes: 11 additions & 2 deletions src/internal/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,15 @@ export async function getAccountOwnedObjects(args: {
return data.current_objects;
}

/**
* NOTE: There is a potential issue once unified single signer scheme will be adopted
* by the community.
*
* Becuase on could create 2 accounts with the same private key with this new authenticator type,
* we’ll need to determine the order in which we lookup the accounts. First unified
* scheme and then legacy scheme vs first legacy scheme and then unified scheme.
*
*/
export async function deriveAccountFromPrivateKey(args: {
aptosConfig: AptosConfig;
privateKey: PrivateKey;
Expand All @@ -518,14 +527,14 @@ export async function deriveAccountFromPrivateKey(args: {
});
if (isSingleSenderTransactionAuthenticator) {
const address = new AccountAddress({ data: SingleSenderTransactionAuthenticatorAuthKey.toUint8Array() });
return Account.fromPrivateKeyAndAddress({ privateKey, address });
return Account.fromPrivateKeyAndAddress({ privateKey, address, legacy: false });
}
// lookup legacy ed25519
const legacyAuthKey = AuthenticationKey.fromPublicKeyAndScheme({ publicKey, scheme: SigningScheme.Ed25519 });
const isLegacyEd25519 = await isAccountExist({ authKey: legacyAuthKey, aptosConfig });
if (isLegacyEd25519) {
const address = new AccountAddress({ data: legacyAuthKey.toUint8Array() });
return Account.fromPrivateKeyAndAddress({ privateKey, address, legacy: true });
return Account.fromPrivateKeyAndAddress({ privateKey, address });
}
}
// if we are here, it means we couldn't find an address with an
Expand Down
27 changes: 11 additions & 16 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -997,28 +997,23 @@ export type WaitForTransactionOptions = {
checkSuccess?: boolean;
indexerVersionCheck?: boolean;
};

/**
* Account input type to generate an account using Legacy
* Ed25519 or MultiEd25519 keys or without a specified `scheme`.
* If `scheme` is not specified, we default to ED25519
* In this case `legacy` is always true
* Input type to generate an account using Single Signer
* Ed25519 or Legacy Ed25519
*/
export type GenerateAccountWithLegacyKey = {
scheme?: SigningSchemeInput.Ed25519;
legacy: true;
export type GenerateAccountWithEd25519 = {
scheme: SigningSchemeInput.Ed25519;
legacy: boolean;
};

/**
* Account input type to generate an account using Unified
* Secp256k1Ecdsa key
* In this case `legacy` is always false
* Input type to generate an account using Single Signer
* Secp256k1
*/
export type GenerateAccountWithUnifiedKey = {
scheme: SigningSchemeInput.Secp256k1Ecdsa | SigningSchemeInput.Ed25519;
export type GenerateAccountWithSingleSignerSecp256k1Key = {
scheme: SigningSchemeInput.Secp256k1Ecdsa;
legacy?: false;
};

/**
* Unify GenerateAccount type for Legacy and Unified keys
*/
export type GenerateAccount = GenerateAccountWithLegacyKey | GenerateAccountWithUnifiedKey;
export type GenerateAccount = GenerateAccountWithEd25519 | GenerateAccountWithSingleSignerSecp256k1Key;
6 changes: 3 additions & 3 deletions tests/e2e/api/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ describe("account api", () => {
test("single sender ed25519", async () => {
const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
const account = Account.generate();
const account = Account.generate({ scheme: SigningSchemeInput.Ed25519, legacy: false });
await aptos.fundAccount({ accountAddress: account.accountAddress.toString(), amount: 100 });

const derivedAccount = await aptos.deriveAccountFromPrivateKey({ privateKey: account.privateKey });
Expand All @@ -233,7 +233,7 @@ describe("account api", () => {
test("legacy ed25519", async () => {
const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
const account = Account.generate({ legacy: true });
const account = Account.generate();
await aptos.fundAccount({ accountAddress: account.accountAddress.toString(), amount: 100 });

const derivedAccount = await aptos.deriveAccountFromPrivateKey({ privateKey: account.privateKey });
Expand Down Expand Up @@ -309,7 +309,7 @@ describe("account api", () => {
const aptos = new Aptos(config);

// Current Account
const account = Account.generate({ scheme: SigningSchemeInput.Ed25519, legacy: true });
const account = Account.generate();
await aptos.fundAccount({ accountAddress: account.accountAddress.toString(), amount: 1_000_000_000 });

// account that holds the new key
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/transaction/signTransaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const aptos = new Aptos(config);

describe("sign transaction", () => {
const contractPublisherAccount = Account.generate();
const singleSignerED25519SenderAccount = Account.generate();
const legacyED25519SenderAccount = Account.generate({ legacy: true });
const singleSignerED25519SenderAccount = Account.generate({ scheme: SigningSchemeInput.Ed25519, legacy: false });
const legacyED25519SenderAccount = Account.generate();
const receiverAccounts = [Account.generate(), Account.generate()];
const singleSignerSecp256k1Account = Account.generate({ scheme: SigningSchemeInput.Secp256k1Ecdsa });
const secondarySignerAccount = Account.generate();
Expand Down
8 changes: 4 additions & 4 deletions tests/e2e/transaction/transactionBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
EntryFunctionABI,
parseTypeTag,
} from "../../../src";
import { AccountAuthenticator, AccountAuthenticatorSingleKey } from "../../../src/transactions/authenticator/account";
import { AccountAuthenticator, AccountAuthenticatorEd25519 } from "../../../src/transactions/authenticator/account";
import {
FeePayerRawTransaction,
MultiAgentRawTransaction,
Expand Down Expand Up @@ -364,7 +364,7 @@ describe("transaction builder", () => {
expect(accountAuthenticator instanceof AccountAuthenticator).toBeTruthy();
const deserializer = new Deserializer(accountAuthenticator.bcsToBytes());
const authenticator = AccountAuthenticator.deserialize(deserializer);
expect(authenticator instanceof AccountAuthenticatorSingleKey).toBeTruthy();
expect(authenticator instanceof AccountAuthenticatorEd25519).toBeTruthy();
});

test("it signs a fee payer transaction", async () => {
Expand All @@ -390,7 +390,7 @@ describe("transaction builder", () => {
expect(accountAuthenticator instanceof AccountAuthenticator).toBeTruthy();
const deserializer = new Deserializer(accountAuthenticator.bcsToBytes());
const authenticator = AccountAuthenticator.deserialize(deserializer);
expect(authenticator instanceof AccountAuthenticatorSingleKey).toBeTruthy();
expect(authenticator instanceof AccountAuthenticatorEd25519).toBeTruthy();
});

test("it signs a multi agent transaction", async () => {
Expand Down Expand Up @@ -420,7 +420,7 @@ describe("transaction builder", () => {
expect(accountAuthenticator instanceof AccountAuthenticator).toBeTruthy();
const deserializer = new Deserializer(accountAuthenticator.bcsToBytes());
const authenticator = AccountAuthenticator.deserialize(deserializer);
expect(authenticator instanceof AccountAuthenticatorSingleKey).toBeTruthy();
expect(authenticator instanceof AccountAuthenticatorEd25519).toBeTruthy();
});
});
describe("generateSignedTransaction", () => {
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/transaction/transactionSimulation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ describe("transaction simulation", () => {
const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
const contractPublisherAccount = Account.generate();
const singleSignerED25519SenderAccount = Account.generate();
const legacyED25519SenderAccount = Account.generate({ legacy: true });
const singleSignerED25519SenderAccount = Account.generate({ scheme: SigningSchemeInput.Ed25519, legacy: false });
const legacyED25519SenderAccount = Account.generate();
const singleSignerSecp256k1Account = Account.generate({ scheme: SigningSchemeInput.Secp256k1Ecdsa });
const recieverAccounts = [Account.generate(), Account.generate()];
const secondarySignerAccount = Account.generate();
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/transaction/transactionSubmission.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const config = new AptosConfig({ network: Network.LOCAL });
const aptos = new Aptos(config);
describe("transaction submission", () => {
const contractPublisherAccount = Account.generate();
const singleSignerED25519SenderAccount = Account.generate();
const legacyED25519SenderAccount = Account.generate({ legacy: true });
const singleSignerED25519SenderAccount = Account.generate({ scheme: SigningSchemeInput.Ed25519, legacy: false });
const legacyED25519SenderAccount = Account.generate();
const receiverAccounts = [Account.generate(), Account.generate()];
const singleSignerSecp256k1Account = Account.generate({ scheme: SigningSchemeInput.Secp256k1Ecdsa });
const secondarySignerAccount = Account.generate();
Expand Down
Loading

0 comments on commit 0c86625

Please sign in to comment.