Skip to content

Conversation

interc0der
Copy link
Contributor

Continuation of previous pull request: #3059

High Level Overview of Change

As there are more wallets natively supporting the XRP Ledger, private keys are being passed around in different context and there should be a method in this codebase to derive a wallet from a private key. Since we have (2) algorithms and this package correctly handles both, this is a necessary addition to the package and offers immense value.

Context of Change

  • Adds a new static method to the Wallet class to derive an XRPL wallet using a private key.
  /**
   * Derives a wallet from a private key.
   *
   * @param privateKey - A string used to generate a keypair (publicKey/privateKey) to derive a wallet.
   * @returns A Wallet derived from a private key.
   */
  public static fromPrivateKey(privateKey: string): Wallet {
    return new Wallet(derivePublicKey(privateKey), privateKey)
  }

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)

Did you update HISTORY.md?

  • No, this change does not impact library users

Test Plan

  • Added a few tests in the Wallet folder to test derivations from mocked keypairs. These tests ensure that the correct public address and public key are generated from the private key.
  • Ensure that the library correctly handles a malformed private key and properly throws an error

Copy link
Contributor

coderabbitai bot commented Aug 14, 2025

Walkthrough

Adds private-key-to-public-key derivation across ripple-keypairs, new scheme methods to derive a KeyPair from a private key, exposes derivePublicKey in package API, adds Wallet.fromPrivateKey in xrpl, and includes tests for secp256k1 and ed25519 private-key derivation and validation.

Changes

Cohort / File(s) Summary of Changes
Core keypairs API
packages/ripple-keypairs/src/index.ts
Adds derivePublicKey(privateKey) that detects the signing algorithm and delegates to scheme-specific deriveKeypairFromPrivateKey; exports the new function.
Signing schemes
packages/ripple-keypairs/src/signing-schemes/ed25519/index.ts, packages/ripple-keypairs/src/signing-schemes/secp256k1/index.ts
Adds deriveKeypairFromPrivateKey(privateKey) to each scheme: accepts prefixed/plain hex, normalizes input, derives the corresponding public key, and returns { privateKey, publicKey }.
Types
packages/ripple-keypairs/src/types.ts
Extends SigningScheme interface with deriveKeypairFromPrivateKey(privateKey: HexString): KeyPair.
Wallet API
packages/xrpl/src/Wallet/index.ts
Adds static fromPrivateKey(privateKey: string): Wallet which validates input, calls derivePublicKey, and constructs a Wallet with derived publicKey and provided privateKey.
Tests
packages/xrpl/test/wallet/index.test.ts
New tests for Wallet.fromPrivateKey covering secp256k1 and ed25519 derivation and invalid private-key handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A nibble, a nib, a byte I keep,
From secret seeds to public leap.
ED or 00, I sniff the line,
I hop to keys both sharp and fine.
Wallets wake where shadows play—
Thump-thump! A rabbit signs away. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately describes the primary change — adding a method to derive a wallet from a private key — and matches the added Wallet.fromPrivateKey and supporting derivePublicKey functionality described in the changeset. It is concise and specific enough for a reviewer to understand the main purpose. The "Re:" prefix is minor noise but does not make the title misleading.
Description Check ✅ Passed The PR description follows the repository template and includes a High Level Overview, Context of Change with a code snippet, populated Type of Change checkboxes, a HISTORY.md response, and a Test Plan describing the added tests; these sections sufficiently explain the change and its verification. The description maps clearly to the raw_summary and pr_objectives and is detailed enough for review. No required template sections appear to be missing.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91557dd and eb46241.

📒 Files selected for processing (1)
  • packages/xrpl/src/Wallet/index.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/xrpl/src/Wallet/index.ts

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
packages/ripple-keypairs/src/types.ts (1)

21-22: Clarify method contract and canonicalization expectations

Please document the accepted formats (prefixes/lengths) for each algorithm and whether implementations must canonicalize the returned privateKey. This avoids ambiguity across backends and downstream callers.

Apply this diff to add TSDoc:

-  deriveKeypairFromPrivateKey: (privateKey: HexString) => KeyPair
+  /**
+   * Derive a keypair from a private key.
+   * Implementations should accept:
+   *  - secp256k1: '00' + 64-hex (66 chars) or raw 64-hex.
+   *  - ed25519: 'ED' + 64-hex (66 chars). Raw 64-hex should be rejected to avoid ambiguity with secp256k1.
+   *
+   * Returned privateKey SHOULD be canonicalized with the correct prefix for the algorithm to ensure
+   * compatibility with downstream APIs (e.g., ed25519 signing requires the 'ED' prefix).
+   */
+  deriveKeypairFromPrivateKey: (privateKey: HexString) => KeyPair
packages/ripple-keypairs/src/index.ts (1)

98-110: Simplify by delegating via getSigningScheme

This avoids duplicating algorithm branches and keeps the implementation aligned with other APIs (sign/verify).

Apply this diff:

-function derivePublicKey(privateKey: string): string {
-  const algorithm = getAlgorithmFromPrivateKey(privateKey)
-
-  if (algorithm === 'ecdsa-secp256k1') {
-    return secp256k1.deriveKeypairFromPrivateKey(privateKey).publicKey
-  }
-  // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-  if (algorithm === 'ed25519') {
-    return ed25519.deriveKeypairFromPrivateKey(privateKey).publicKey
-  }
-  throw new Error('Unknown signing scheme algorithm')
-}
+function derivePublicKey(privateKey: string): string {
+  const algorithm = getAlgorithmFromPrivateKey(privateKey)
+  const scheme = getSigningScheme(algorithm)
+  return scheme.deriveKeypairFromPrivateKey(privateKey).publicKey
+}
packages/xrpl/test/wallet/index.test.ts (1)

456-461: Consider asserting the error class/message to prevent false positives

Right now we only assert that an error is thrown. If the implementation throws a different error (e.g., Algorithm mismatch vs. invalid hex), these tests would still pass. When feasible, narrow this with a regex or specific error class (e.g., ValidationError), similar to other tests in this file.

Example:

assert.throws(
  () => Wallet.fromPrivateKey(...),
  /invalid|malformed|unknown signing scheme/i,
)

If the message is stable, prefer a precise regex.

Would you like me to align this to the actual error text thrown by derivePublicKey() so the tests are resilient?

Also applies to: 477-482

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between de28f40 and 91557dd.

📒 Files selected for processing (6)
  • packages/ripple-keypairs/src/index.ts (2 hunks)
  • packages/ripple-keypairs/src/signing-schemes/ed25519/index.ts (1 hunks)
  • packages/ripple-keypairs/src/signing-schemes/secp256k1/index.ts (1 hunks)
  • packages/ripple-keypairs/src/types.ts (1 hunks)
  • packages/xrpl/src/Wallet/index.ts (2 hunks)
  • packages/xrpl/test/wallet/index.test.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/xrpl/src/Wallet/index.ts (2)
packages/xrpl/src/index.ts (1)
  • Wallet (12-12)
packages/xrpl/src/errors.ts (1)
  • ValidationError (156-156)
🔇 Additional comments (2)
packages/xrpl/src/Wallet/index.ts (1)

20-20: Import looks correct

Importing derivePublicKey where used is correct and consistent with the rest of the module.

packages/ripple-keypairs/src/index.ts (1)

123-124: Good addition to the public API

Exporting derivePublicKey alongside deriveAddress/deriveNodeAddress makes sense and keeps xrpl Wallet usage minimal.

Comment on lines +22 to +42
deriveKeypairFromPrivateKey(privateKey: string): {
privateKey: string
publicKey: string
} {
assert.ok(
privateKey.startsWith(ED_PREFIX)
? privateKey.length === 66
: privateKey.length === 64,
'Invalid ed25519 private key length',
)

const normalizedPrivateKey = privateKey.startsWith(ED_PREFIX)
? privateKey.slice(2)
: privateKey

const buffer = Buffer.from(normalizedPrivateKey, 'hex')

const publicKey = ED_PREFIX + bytesToHex(nobleEd25519.getPublicKey(buffer))
return { privateKey, publicKey }
},

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Require ED prefix and avoid Buffer for isomorphic support

  • ed25519 keys must carry the ED prefix across this codebase (sign() enforces length 66). Accepting raw 64-hex here is inconsistent and creates ambiguity with secp256k1 detection.
  • Replace Buffer usage with hexToBytes for browser compatibility.
  • Add strict hex validation.

Apply this diff within the selected range:

   deriveKeypairFromPrivateKey(privateKey: string): {
     privateKey: string
     publicKey: string
   } {
-    assert.ok(
-      privateKey.startsWith(ED_PREFIX)
-        ? privateKey.length === 66
-        : privateKey.length === 64,
-      'Invalid ed25519 private key length',
-    )
-
-    const normalizedPrivateKey = privateKey.startsWith(ED_PREFIX)
-      ? privateKey.slice(2)
-      : privateKey
-
-    const buffer = Buffer.from(normalizedPrivateKey, 'hex')
-
-    const publicKey = ED_PREFIX + bytesToHex(nobleEd25519.getPublicKey(buffer))
-    return { privateKey, publicKey }
+    assert.ok(
+      privateKey.startsWith(ED_PREFIX) && privateKey.length === 66,
+      'ed25519 private key must be 33 bytes including "ED" prefix',
+    )
+    const normalizedPrivateKey = privateKey.slice(2)
+    assert.ok(
+      /^[0-9a-fA-F]{64}$/.test(normalizedPrivateKey),
+      'private key must be 32-byte hex',
+    )
+    const privBytes = hexToBytes(normalizedPrivateKey)
+    const publicKey =
+      ED_PREFIX + bytesToHex(nobleEd25519.getPublicKey(privBytes))
+    return { privateKey, publicKey }
   },

And update the import (outside this hunk):

// change this:
import { bytesToHex } from '@xrplf/isomorphic/utils'
// to this:
import { bytesToHex, hexToBytes } from '@xrplf/isomorphic/utils'
🤖 Prompt for AI Agents
In packages/ripple-keypairs/src/signing-schemes/ed25519/index.ts around lines 22
to 42, enforce that private keys must include the ED_PREFIX (no accepting 64-hex
raw keys), validate the hex string strictly (only [0-9a-fA-F] and exact length
after prefix), convert the normalized hex to bytes using hexToBytes instead of
Buffer for isomorphic/browser support, and return the publicKey prefixed with
ED_PREFIX; also update the module import (outside this hunk) to import
hexToBytes along with bytesToHex as requested.

Comment on lines +33 to +52
deriveKeypairFromPrivateKey(privateKey: string): {
privateKey: string
publicKey: string
} {
assert.ok(
(privateKey.length === 66 && privateKey.startsWith(SECP256K1_PREFIX)) ||
privateKey.length === 64,
'Invalid private key length or format',
)
const normalizedPrivateKey =
privateKey.length === 66 && privateKey.startsWith(SECP256K1_PREFIX)
? privateKey.slice(2)
: privateKey

const buffer = Buffer.from(normalizedPrivateKey, 'hex')

const publicKey = bytesToHex(nobleSecp256k1.getPublicKey(buffer, true))
return { privateKey, publicKey }
},

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid Node.js Buffer; use isomorphic hexToBytes and validate hex

Using Buffer breaks browser environments. This package is isomorphic; prefer hexToBytes. Also add a strict hex check to fail fast on malformed input.

Apply this diff within the selected range:

   } {
     assert.ok(
       (privateKey.length === 66 && privateKey.startsWith(SECP256K1_PREFIX)) ||
         privateKey.length === 64,
       'Invalid private key length or format',
     )
-    const normalizedPrivateKey =
+    const normalizedPrivateKey =
       privateKey.length === 66 && privateKey.startsWith(SECP256K1_PREFIX)
         ? privateKey.slice(2)
         : privateKey
-
-    const buffer = Buffer.from(normalizedPrivateKey, 'hex')
-
-    const publicKey = bytesToHex(nobleSecp256k1.getPublicKey(buffer, true))
+    assert.ok(
+      /^[0-9a-fA-F]{64}$/.test(normalizedPrivateKey),
+      'private key must be 32-byte hex',
+    )
+    const privBytes = hexToBytes(normalizedPrivateKey)
+    const publicKey = bytesToHex(nobleSecp256k1.getPublicKey(privBytes, true))
     return { privateKey, publicKey }
   },

And update the import (outside this hunk):

// change this:
import { bytesToHex } from '@xrplf/isomorphic/utils'
// to this:
import { bytesToHex, hexToBytes } from '@xrplf/isomorphic/utils'
🤖 Prompt for AI Agents
In packages/ripple-keypairs/src/signing-schemes/secp256k1/index.ts around lines
33 to 52, the code uses Node's Buffer to decode a hex private key and lacks
strict hex validation; replace Buffer.from(...) with the isomorphic
hexToBytes(privateHex) function (and update the import to include hexToBytes),
and add a strict hex format check (e.g., /^[0-9a-fA-F]+$/ with expected length
64) to fail fast on malformed input before converting and deriving the public
key.

Comment on lines +177 to +191
/**
* Derives a wallet from a private key.
*
* @param privateKey - A string used to generate a keypair (publicKey/privateKey) to derive a wallet.
* @returns A Wallet derived from a private key.
*
* @throws ValidationError if private key is not a valid string
*/
public static fromPrivateKey(privateKey: string): Wallet {
if (!privateKey || typeof privateKey !== 'string') {
throw new ValidationError('privateKey must be a non-empty string')
}
return new Wallet(derivePublicKey(privateKey), privateKey)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden validation and normalize error surface

  • Ambiguous input: ed25519 requires an ED-prefixed private key for downstream APIs. Accepting raw 64-hex for ed25519 leads to mis-detection as secp256k1 and subtle bugs. At minimum, make the error explicit.
  • Wrap ripple-keypairs errors in ValidationError to present a consistent API surface.

Apply this diff:

   public static fromPrivateKey(privateKey: string): Wallet {
-    if (!privateKey || typeof privateKey !== 'string') {
-      throw new ValidationError('privateKey must be a non-empty string')
-    }
-    return new Wallet(derivePublicKey(privateKey), privateKey)
+    if (typeof privateKey !== 'string' || privateKey.length === 0) {
+      throw new ValidationError('privateKey must be a non-empty string')
+    }
+    try {
+      const publicKey = derivePublicKey(privateKey)
+      return new Wallet(publicKey, privateKey)
+    } catch (e) {
+      // Note: ed25519 private keys must be 'ED' prefixed (66 hex chars).
+      // secp256k1 private keys may be '00' prefixed (66 hex chars) or raw 64-hex.
+      throw new ValidationError(
+        `Invalid or unsupported privateKey format${
+          e instanceof Error ? `: ${e.message}` : ''
+        }`,
+      )
+    }
   }

Also update the JSDoc to note prefix requirements for each algorithm.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Derives a wallet from a private key.
*
* @param privateKey - A string used to generate a keypair (publicKey/privateKey) to derive a wallet.
* @returns A Wallet derived from a private key.
*
* @throws ValidationError if private key is not a valid string
*/
public static fromPrivateKey(privateKey: string): Wallet {
if (!privateKey || typeof privateKey !== 'string') {
throw new ValidationError('privateKey must be a non-empty string')
}
return new Wallet(derivePublicKey(privateKey), privateKey)
}
/**
* Derives a wallet from a private key.
*
* @param privateKey - A string used to generate a keypair (publicKey/privateKey) to derive a wallet.
* @returns A Wallet derived from a private key.
*
* @throws ValidationError if private key is not a valid string
*/
public static fromPrivateKey(privateKey: string): Wallet {
if (typeof privateKey !== 'string' || privateKey.length === 0) {
throw new ValidationError('privateKey must be a non-empty string')
}
try {
const publicKey = derivePublicKey(privateKey)
return new Wallet(publicKey, privateKey)
} catch (e) {
// Note: ed25519 private keys must be 'ED' prefixed (66 hex chars).
// secp256k1 private keys may be '00' prefixed (66 hex chars) or raw 64-hex.
throw new ValidationError(
`Invalid or unsupported privateKey format${
e instanceof Error ? `: ${e.message}` : ''
}`,
)
}
}
🤖 Prompt for AI Agents
In packages/xrpl/src/Wallet/index.ts around lines 177 to 191, the fromPrivateKey
method currently accepts ambiguous inputs and can mis-detect ed25519 keys;
update validation to require and validate algorithm-specific formats (e.g.
require ED-prefixed private keys for ed25519 and explicit, validated hex format
for secp256k1), throw a ValidationError with an explicit message when the
prefix/format is wrong, and wrap any underlying ripple-keypairs errors in a
ValidationError so the API surface is consistent; also update the JSDoc to
document the prefix/format requirements for each algorithm.

Comment on lines +440 to +484
describe('from PrivateKey', function () {
describe('using secp256k1 private key', function () {
const mockWallet_secp256k1 = {
address: 'rhvh5SrgBL5V8oeV9EpDuVszeJSSCEkbPc',
publicKey:
'030E58CDD076E798C84755590AAF6237CA8FAE821070A59F648B517A30DC6F589D',
privateKey:
'00141BA006D3363D2FB2785E8DF4E44D3A49908780CB4FB51F6D217C08C021429F',
}

it('derive keypair from private key', function () {
const wallet = Wallet.fromPrivateKey(mockWallet_secp256k1.privateKey)
assert.equal(wallet.address, mockWallet_secp256k1.address)
assert.equal(wallet.publicKey, mockWallet_secp256k1.publicKey)
})

it('throws error for malformed secp256k1 private key', function () {
assert.throws(() =>
Wallet.fromPrivateKey(mockWallet_secp256k1.privateKey.slice(0, 10)),
)
})
})

describe('using ed25519 private key', function () {
const mockWallet_ed25519 = {
address: 'rszPLM97iS8mFTndKQNexGhY1N9ionLVAx',
publicKey:
'EDFD5C3E305FDEB97A89FC39ED333A710A7ED35E3471443C4989F9E3B8F023488D',
privateKey:
'EDDA8694C151CE30E8A2C91884E26BC11A75514E3A27EE6CE4615FABA3DCBE1429',
}
it('derive keypair from private key', function () {
const wallet = Wallet.fromPrivateKey(mockWallet_ed25519.privateKey)
assert.equal(wallet.address, mockWallet_ed25519.address)
assert.equal(wallet.publicKey, mockWallet_ed25519.publicKey)
})

it('throws error for malformed ed25519 private key', function () {
assert.throws(() =>
Wallet.fromPrivateKey(mockWallet_ed25519.privateKey.slice(0, 10)),
)
})
})
})

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 14, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden fromPrivateKey tests: fix suite name, improve consistency, and extend coverage for accepted inputs and invalid cases

Great addition; this validates the new API across both curves. A few improvements will make this suite more robust and consistent with the rest of the file:

  • Consistency/naming: use fromPrivateKey (no space) to match method name; use camelCase for mock constants; align test titles with “derives a wallet…” phrasing used elsewhere.
  • Assertions: also assert privateKey is preserved, classicAddress equals address, and seed is undefined when constructed from a privateKey.
  • Coverage: add tests for accepted input forms:
    • secp256k1 64-char hex without the leading "00" prefix
    • ed25519 without the "ED" prefix
  • Negative cases: add tests for empty string and non-hex input.

Apply this diff within this block:

-  describe('from PrivateKey', function () {
+  describe('fromPrivateKey', function () {
     describe('using secp256k1 private key', function () {
-      const mockWallet_secp256k1 = {
+      const mockWalletSecp256k1 = {
         address: 'rhvh5SrgBL5V8oeV9EpDuVszeJSSCEkbPc',
         publicKey:
           '030E58CDD076E798C84755590AAF6237CA8FAE821070A59F648B517A30DC6F589D',
         privateKey:
           '00141BA006D3363D2FB2785E8DF4E44D3A49908780CB4FB51F6D217C08C021429F',
       }
 
-      it('derive keypair from private key', function () {
-        const wallet = Wallet.fromPrivateKey(mockWallet_secp256k1.privateKey)
-        assert.equal(wallet.address, mockWallet_secp256k1.address)
-        assert.equal(wallet.publicKey, mockWallet_secp256k1.publicKey)
+      it('derives a wallet from private key', function () {
+        const wallet = Wallet.fromPrivateKey(mockWalletSecp256k1.privateKey)
+        assert.equal(wallet.address, mockWalletSecp256k1.address)
+        assert.equal(wallet.publicKey, mockWalletSecp256k1.publicKey)
+        assert.equal(wallet.privateKey, mockWalletSecp256k1.privateKey)
+        assert.equal(wallet.classicAddress, wallet.address)
+        assert.isUndefined((wallet as unknown as { seed?: string }).seed)
       })
 
+      it('accepts 64-char secp256k1 private key without 00 prefix', function () {
+        const unprefixed = mockWalletSecp256k1.privateKey.slice(2)
+        const wallet = Wallet.fromPrivateKey(unprefixed)
+        assert.equal(wallet.address, mockWalletSecp256k1.address)
+        assert.equal(wallet.publicKey, mockWalletSecp256k1.publicKey)
+      })
+
       it('throws error for malformed secp256k1 private key', function () {
         assert.throws(() =>
-          Wallet.fromPrivateKey(mockWallet_secp256k1.privateKey.slice(0, 10)),
+          Wallet.fromPrivateKey(mockWalletSecp256k1.privateKey.slice(0, 10)),
         )
       })
     })
 
     describe('using ed25519 private key', function () {
-      const mockWallet_ed25519 = {
+      const mockWalletEd25519 = {
         address: 'rszPLM97iS8mFTndKQNexGhY1N9ionLVAx',
         publicKey:
           'EDFD5C3E305FDEB97A89FC39ED333A710A7ED35E3471443C4989F9E3B8F023488D',
         privateKey:
           'EDDA8694C151CE30E8A2C91884E26BC11A75514E3A27EE6CE4615FABA3DCBE1429',
       }
-      it('derive keypair from private key', function () {
-        const wallet = Wallet.fromPrivateKey(mockWallet_ed25519.privateKey)
-        assert.equal(wallet.address, mockWallet_ed25519.address)
-        assert.equal(wallet.publicKey, mockWallet_ed25519.publicKey)
+      it('derives a wallet from private key', function () {
+        const wallet = Wallet.fromPrivateKey(mockWalletEd25519.privateKey)
+        assert.equal(wallet.address, mockWalletEd25519.address)
+        assert.equal(wallet.publicKey, mockWalletEd25519.publicKey)
+        assert.equal(wallet.privateKey, mockWalletEd25519.privateKey)
+        assert.equal(wallet.classicAddress, wallet.address)
+        assert.isUndefined((wallet as unknown as { seed?: string }).seed)
       })
 
+      it('accepts ed25519 private key without ED prefix', function () {
+        const unprefixed = mockWalletEd25519.privateKey.slice(2)
+        const wallet = Wallet.fromPrivateKey(unprefixed)
+        assert.equal(wallet.address, mockWalletEd25519.address)
+        assert.equal(wallet.publicKey, mockWalletEd25519.publicKey)
+      })
 
       it('throws error for malformed ed25519 private key', function () {
         assert.throws(() =>
-          Wallet.fromPrivateKey(mockWallet_ed25519.privateKey.slice(0, 10)),
+          Wallet.fromPrivateKey(mockWalletEd25519.privateKey.slice(0, 10)),
         )
       })
     })
+
+    it('throws error for empty private key', function () {
+      assert.throws(() => Wallet.fromPrivateKey(''))
+    })
+
+    it('throws error for non-hex private key', function () {
+      assert.throws(() => Wallet.fromPrivateKey('not-hex'))
+    })
   })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe('from PrivateKey', function () {
describe('using secp256k1 private key', function () {
const mockWallet_secp256k1 = {
address: 'rhvh5SrgBL5V8oeV9EpDuVszeJSSCEkbPc',
publicKey:
'030E58CDD076E798C84755590AAF6237CA8FAE821070A59F648B517A30DC6F589D',
privateKey:
'00141BA006D3363D2FB2785E8DF4E44D3A49908780CB4FB51F6D217C08C021429F',
}
it('derive keypair from private key', function () {
const wallet = Wallet.fromPrivateKey(mockWallet_secp256k1.privateKey)
assert.equal(wallet.address, mockWallet_secp256k1.address)
assert.equal(wallet.publicKey, mockWallet_secp256k1.publicKey)
})
it('throws error for malformed secp256k1 private key', function () {
assert.throws(() =>
Wallet.fromPrivateKey(mockWallet_secp256k1.privateKey.slice(0, 10)),
)
})
})
describe('using ed25519 private key', function () {
const mockWallet_ed25519 = {
address: 'rszPLM97iS8mFTndKQNexGhY1N9ionLVAx',
publicKey:
'EDFD5C3E305FDEB97A89FC39ED333A710A7ED35E3471443C4989F9E3B8F023488D',
privateKey:
'EDDA8694C151CE30E8A2C91884E26BC11A75514E3A27EE6CE4615FABA3DCBE1429',
}
it('derive keypair from private key', function () {
const wallet = Wallet.fromPrivateKey(mockWallet_ed25519.privateKey)
assert.equal(wallet.address, mockWallet_ed25519.address)
assert.equal(wallet.publicKey, mockWallet_ed25519.publicKey)
})
it('throws error for malformed ed25519 private key', function () {
assert.throws(() =>
Wallet.fromPrivateKey(mockWallet_ed25519.privateKey.slice(0, 10)),
)
})
})
})
describe('fromPrivateKey', function () {
describe('using secp256k1 private key', function () {
const mockWalletSecp256k1 = {
address: 'rhvh5SrgBL5V8oeV9EpDuVszeJSSCEkbPc',
publicKey:
'030E58CDD076E798C84755590AAF6237CA8FAE821070A59F648B517A30DC6F589D',
privateKey:
'00141BA006D3363D2FB2785E8DF4E44D3A49908780CB4FB51F6D217C08C021429F',
}
it('derives a wallet from private key', function () {
const wallet = Wallet.fromPrivateKey(mockWalletSecp256k1.privateKey)
assert.equal(wallet.address, mockWalletSecp256k1.address)
assert.equal(wallet.publicKey, mockWalletSecp256k1.publicKey)
assert.equal(wallet.privateKey, mockWalletSecp256k1.privateKey)
assert.equal(wallet.classicAddress, wallet.address)
assert.isUndefined((wallet as unknown as { seed?: string }).seed)
})
it('accepts 64-char secp256k1 private key without 00 prefix', function () {
const unprefixed = mockWalletSecp256k1.privateKey.slice(2)
const wallet = Wallet.fromPrivateKey(unprefixed)
assert.equal(wallet.address, mockWalletSecp256k1.address)
assert.equal(wallet.publicKey, mockWalletSecp256k1.publicKey)
})
it('throws error for malformed secp256k1 private key', function () {
assert.throws(() =>
Wallet.fromPrivateKey(mockWalletSecp256k1.privateKey.slice(0, 10)),
)
})
})
describe('using ed25519 private key', function () {
const mockWalletEd25519 = {
address: 'rszPLM97iS8mFTndKQNexGhY1N9ionLVAx',
publicKey:
'EDFD5C3E305FDEB97A89FC39ED333A710A7ED35E3471443C4989F9E3B8F023488D',
privateKey:
'EDDA8694C151CE30E8A2C91884E26BC11A75514E3A27EE6CE4615FABA3DCBE1429',
}
it('derives a wallet from private key', function () {
const wallet = Wallet.fromPrivateKey(mockWalletEd25519.privateKey)
assert.equal(wallet.address, mockWalletEd25519.address)
assert.equal(wallet.publicKey, mockWalletEd25519.publicKey)
assert.equal(wallet.privateKey, mockWalletEd25519.privateKey)
assert.equal(wallet.classicAddress, wallet.address)
assert.isUndefined((wallet as unknown as { seed?: string }).seed)
})
it('accepts ed25519 private key without ED prefix', function () {
const unprefixed = mockWalletEd25519.privateKey.slice(2)
const wallet = Wallet.fromPrivateKey(unprefixed)
assert.equal(wallet.address, mockWalletEd25519.address)
assert.equal(wallet.publicKey, mockWalletEd25519.publicKey)
})
it('throws error for malformed ed25519 private key', function () {
assert.throws(() =>
Wallet.fromPrivateKey(mockWalletEd25519.privateKey.slice(0, 10)),
)
})
})
it('throws error for empty private key', function () {
assert.throws(() => Wallet.fromPrivateKey(''))
})
it('throws error for non-hex private key', function () {
assert.throws(() => Wallet.fromPrivateKey('not-hex'))
})
})

Copy link
Collaborator

Choose a reason for hiding this comment

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

@interc0der The following two test cases are useful additions: accepts ed25519/secp256k1 private key without ED/00 prefix

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

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