Skip to content

feat: add bbs-2023 and ecdsa-sd-2023 key generation and did support#67

Merged
nghaninn merged 3 commits into
alphafrom
feat/new-crypto-suite-key-generation
Jul 31, 2025
Merged

feat: add bbs-2023 and ecdsa-sd-2023 key generation and did support#67
nghaninn merged 3 commits into
alphafrom
feat/new-crypto-suite-key-generation

Conversation

@rongquan1
Copy link
Copy Markdown
Contributor

@rongquan1 rongquan1 commented Jul 28, 2025

Summary by CodeRabbit

  • New Features

    • Added support for BBS-2023 and ECDSA-SD-2023 cryptographic suites, enabling issuance of DIDs and key pairs using modern Multikey verification methods.
    • DID Documents can now include Multikey verification methods with multibase-encoded public and secret keys.
    • Expanded documentation and examples to reflect new key types and formats.
  • Bug Fixes

    • Improved handling and validation of seeds for supported key types.
  • Tests

    • Added comprehensive tests for BBS-2023 and ECDSA-SD-2023 key generation, DID issuance, and mixed cryptosuite scenarios.
  • Chores

    • Updated dependencies and TypeScript configurations to support new cryptographic libraries and module declarations.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jul 28, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This set of changes introduces support for modern cryptosuites—BBS-2023 and ECDSA-SD-2023—across the DID Web issuer package. It adds new key pair generation modules, updates types and enums, enhances the main key generation logic, extends DID document creation, and broadens test coverage. TypeScript configurations and module declarations are also updated for compatibility.

Changes

Cohort / File(s) Change Summary
Modern Cryptosuite Key Generation
packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts, packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts, packages/w3c-issuer/src/did-web/keyPair/index.ts
Added modules for BBS-2023 and ECDSA-SD-2023 key pair generation; updated main key generation logic to support modern cryptosuites with a new dispatch function and improved error handling.
Types and Enums for Crypto Suites
packages/w3c-issuer/src/lib/types.ts, packages/w3c-issuer/src/did-web/keyPair/types.ts, packages/w3c-issuer/src/did-web/wellKnown/types.ts
Introduced CryptoSuite enum, extended VerificationType with Multikey, updated key pair and context types, and added new types for modern cryptosuite key pairs.
DID Document and Issuance Updates
packages/w3c-issuer/src/did-web/wellKnown/generate.ts, packages/w3c-issuer/src/did-web/wellKnown/index.ts
Enhanced DID document generation and issuance to support Multikey verification methods, updated logic for handling new key pair types, and clarified JSDoc.
Test Coverage for Modern Cryptosuites
packages/w3c-issuer/src/did-web/keyPair/index.test.ts, packages/w3c-issuer/src/did-web/wellKnown/generate.test.ts, packages/w3c-issuer/src/did-web/wellKnown/index.test.ts, apps/w3c-cli/tests/main.test.ts
Added and expanded tests for BBS-2023 and ECDSA-SD-2023 key pair generation, DID issuance, and document generation, including mixed cryptosuite scenarios and updated assertions.
Documentation Update
packages/w3c-issuer/src/did-web/README.md
Updated example DID Document to use Multikey verification method and modern context.
Dependency and TypeScript Declaration Management
packages/w3c-issuer/package.json, packages/declaration.d.ts
Added dependencies for new cryptosuite libraries and TypeScript module declarations for external packages.
TypeScript Project Configuration
packages/w3c-issuer/tsconfig.json, packages/w3c-issuer/tsconfig.build.json, packages/w3c-credential-status/tsconfig.json, packages/w3c-vc/tsconfig.json, packages/w3c/tsconfig.json
Included ../declaration.d.ts in project compilation context for all affected packages.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI/API
    participant KeyGen
    participant WellKnown
    participant DIDDoc

    User->>CLI/API: Request DID issuance (specify cryptosuite)
    CLI/API->>KeyGen: generateKeyPair({type: CryptoSuite.Bbs2023 or EcdsaSd2023})
    KeyGen->>KeyGen: Dispatch to appropriate generator
    KeyGen->>CLI/API: Return generated key pair (Multikey)
    CLI/API->>WellKnown: generateWellKnownDid({keyPair: Multikey})
    WellKnown->>DIDDoc: Construct DID Document with Multikey verification method
    DIDDoc-->>WellKnown: Return DID Document
    WellKnown-->>CLI/API: Return DID Document
    CLI/API-->>User: Output DID Document and key material
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

In the warren, cryptosuites bloom anew,
Multikeys hop in, with secrets to strew.
BBS and ECDSA, now part of the game,
DID docs grow stronger, never the same.
TypeScript burrows deeper, configs in tow—
Oh, what a leap for the rabbits below!
🐇✨

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/new-crypto-suite-key-generation

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@rongquan1 rongquan1 requested a review from nghaninn July 29, 2025 02:15
@nghaninn
Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jul 29, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@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: 8

🔭 Outside diff range comments (1)
packages/w3c-issuer/src/did-web/keyPair/index.ts (1)

91-100: Clarify legacy-only Base58 seed validation

The validation block is correctly limited to Bls12381G2Key2020 because only legacy BLS keys expose privateKeyBase58/publicKeyBase58 for direct comparison. Modern BBS-2023 keys are always returned as multibase-encoded strings (secretKeyMultibase/publicKeyMultibase) and do not expose Base58 keys, so seed validation via Base58 comparison isn’t possible.

Please update the comment above the validation to make this explicit. For example:

-  // If seed is provided, validate against provided keys (legacy format only)
+  // If seed is provided, validate against provided Base58 keys (only for legacy BLS).
+  // BBS-2023 keys use multibase encoding (secretKeyMultibase/publicKeyMultibase) and cannot be validated via Base58.

Location:

  • packages/w3c-issuer/src/did-web/keyPair/index.ts @ line 90–91
🧹 Nitpick comments (6)
packages/declaration.d.ts (1)

1-2: Consider adding more detailed type declarations for better type safety.

While these basic module declarations enable compilation, they essentially treat the imported modules as any type. Consider adding more specific type declarations to improve type safety and developer experience.

For example:

-declare module '@digitalbazaar/bls12-381-multikey';
-declare module '@digitalbazaar/ecdsa-multikey';
+declare module '@digitalbazaar/bls12-381-multikey' {
+  export function generate(options?: { seed?: Uint8Array }): Promise<any>;
+  // Add other expected exports
+}
+declare module '@digitalbazaar/ecdsa-multikey' {
+  export function generate(options: { curve: string }): Promise<any>;
+  // Add other expected exports
+}
packages/w3c-issuer/src/did-web/wellKnown/generate.ts (2)

42-45: Improve type safety for Multikey key pair handling.

The repeated type casting to union types could be made more type-safe and readable.

-        (newKeyPair.type === VerificationType.Multikey &&
-          s?.publicKeyMultibase &&
-          s?.publicKeyMultibase ===
-            (newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair)?.publicKeyMultibase)
+        (newKeyPair.type === VerificationType.Multikey &&
+          s?.publicKeyMultibase &&
+          'publicKeyMultibase' in newKeyPair &&
+          s?.publicKeyMultibase === newKeyPair.publicKeyMultibase)

97-103: Extract repeated type casting for better maintainability.

The type casting logic is repeated and could be extracted to improve readability and maintainability.

+  // Helper function to check if key pair has multibase key
+  const isMultikeyKeyPair = (kp: KeyPair): kp is Bbs2023KeyPair | EcdsaSd2023KeyPair => {
+    return kp.type === VerificationType.Multikey && 'publicKeyMultibase' in kp;
+  };
+
   } else if (
-    newKeyPair.type === VerificationType.Multikey &&
-    (newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair).publicKeyMultibase
+    isMultikeyKeyPair(newKeyPair)
   ) {
-    newVerificationMethod.publicKeyMultibase = (
-      newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair
-    ).publicKeyMultibase;
+    newVerificationMethod.publicKeyMultibase = newKeyPair.publicKeyMultibase;
   }
packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts (1)

15-17: Verify seed validation approach

The current validation only checks for truthy values. Consider using more specific validation to ensure the seed is a valid Uint8Array.

  if (!seed) {
-   throw new Error('Invalid seed');
+   throw new Error('Seed is required for BBS-2023 key generation');
  }
+ 
+ if (!(seed instanceof Uint8Array)) {
+   throw new Error('Seed must be a Uint8Array');
+ }
packages/w3c-issuer/src/did-web/keyPair/index.ts (2)

17-29: Consider documenting why ECDSA-SD-2023 ignores keyPairOptions

The generateEcdsaSd2023KeyPair function is called without any parameters even though keyPairOptions is available. This asymmetry with generateBbs2023KeyPair could be confusing. Consider adding a comment explaining that ECDSA-SD-2023 doesn't support deterministic key generation from seeds.

 switch (type) {
   case CryptoSuite.Bbs2023:
     return await generateBbs2023KeyPair(keyPairOptions);
   case CryptoSuite.EcdsaSd2023:
+    // ECDSA-SD-2023 doesn't support seed-based generation
     return await generateEcdsaSd2023KeyPair();
   default:
     throw new Error(`Unsupported modern cryptosuite: ${type}`);
 }

75-89: Consider optimizing the cryptosuite type check

Using Object.values(CryptoSuite).includes() creates an array on every function call. Since TypeScript already provides type safety, consider using a more direct approach.

 // Check if it's a modern cryptosuite
-if (Object.values(CryptoSuite).includes(type as CryptoSuite)) {
+if (type === CryptoSuite.Bbs2023 || type === CryptoSuite.EcdsaSd2023) {
   generatedKeyPair = await generateModernCryptosuite(type as CryptoSuite, keyPairOptions);
 } else {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2511e38 and 0e171e5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (20)
  • apps/w3c-cli/tests/main.test.ts (3 hunks)
  • packages/declaration.d.ts (1 hunks)
  • packages/w3c-credential-status/tsconfig.json (1 hunks)
  • packages/w3c-issuer/package.json (1 hunks)
  • packages/w3c-issuer/src/did-web/README.md (1 hunks)
  • packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts (1 hunks)
  • packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts (1 hunks)
  • packages/w3c-issuer/src/did-web/keyPair/index.test.ts (2 hunks)
  • packages/w3c-issuer/src/did-web/keyPair/index.ts (3 hunks)
  • packages/w3c-issuer/src/did-web/keyPair/types.ts (1 hunks)
  • packages/w3c-issuer/src/did-web/wellKnown/generate.test.ts (1 hunks)
  • packages/w3c-issuer/src/did-web/wellKnown/generate.ts (3 hunks)
  • packages/w3c-issuer/src/did-web/wellKnown/index.test.ts (3 hunks)
  • packages/w3c-issuer/src/did-web/wellKnown/index.ts (3 hunks)
  • packages/w3c-issuer/src/did-web/wellKnown/types.ts (3 hunks)
  • packages/w3c-issuer/src/lib/types.ts (3 hunks)
  • packages/w3c-issuer/tsconfig.build.json (1 hunks)
  • packages/w3c-issuer/tsconfig.json (1 hunks)
  • packages/w3c-vc/tsconfig.json (1 hunks)
  • packages/w3c/tsconfig.json (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts (1)
packages/w3c-issuer/src/did-web/keyPair/types.ts (1)
  • DidWebGeneratedKeyPair (12-24)
packages/w3c-issuer/src/did-web/keyPair/index.ts (4)
packages/w3c-issuer/src/lib/types.ts (2)
  • GenerateKeyPairOptions (39-39)
  • GeneratedKeyPair (41-41)
packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts (1)
  • generateBbs2023KeyPair (12-39)
packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts (1)
  • generateEcdsaSd2023KeyPair (12-31)
packages/w3c-issuer/src/did-web/keyPair/bls12381.ts (1)
  • generateBls12381KeyPair (12-34)
packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts (1)
packages/w3c-issuer/src/did-web/keyPair/types.ts (2)
  • DidWebGenerateKeyPairOptions (3-10)
  • DidWebGeneratedKeyPair (12-24)
🪛 Gitleaks (8.27.2)
packages/w3c-issuer/src/did-web/wellKnown/index.test.ts

41-41: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/w3c-issuer/src/did-web/wellKnown/generate.test.ts

227-227: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


309-309: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


392-392: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (27)
packages/w3c-issuer/package.json (1)

31-38: Double-check new crypto deps for ESM/CJS compatibility and audit status.

@digitalbazaar/bls12-381-multikey (v2) and @digitalbazaar/ecdsa-multikey (v1) ship pure-ESM. Your bundle script (tsup) must emit ESM (or use dynamic import/moduleResolution: "nodenext").
Also run an npm audit/npm audit --production to ensure no known CVEs are introduced.

If all clear, nothing else to change.

packages/w3c-credential-status/tsconfig.json (1)

5-5: Confirm base config’s rootDir.

If ../../tsconfig.base.json sets rootDir (common in monorepos) the same “outside rootDir” error will occur here.
Run tsc -p packages/w3c-credential-status once to verify; adjust rootDir or drop it exactly as for w3c-issuer/tsconfig.build.json if needed.

packages/w3c-issuer/tsconfig.json (1)

6-6: Nice addition – brings shared declaration typings into this package.

No immediate issues; rootDir is not set so the extra file is inside the computed root.

packages/w3c-vc/tsconfig.json (1)

6-6: Mirror the rootDir check here as well.

Same caution as in w3c-credential-status: if the base config fixes rootDir, importing ../declaration.d.ts will violate it. Quick tsc -p packages/w3c-vc run will surface the issue.

packages/w3c/tsconfig.json (1)

6-6: LGTM! Configuration change aligns with monorepo consistency.

The addition of the shared declaration file to the TypeScript compilation context is appropriate and maintains consistency with other packages in the monorepo.

packages/w3c-issuer/src/did-web/README.md (1)

24-45: Excellent documentation update reflecting modern DID standards.

The updated example correctly demonstrates:

  • Modern @context array with multikey security context
  • Multikey verification type usage
  • publicKeyMultibase encoding format
  • Proper assertionMethod inclusion

The documentation clearly shows the migration path from legacy formats to modern standards.

packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts (3)

1-11: LGTM: Clean imports and function structure

The imports are appropriate for BBS-2023 key generation, type definitions are correct, and the function signature follows the expected pattern from the DidWebGeneratedKeyPair interface.


31-38: LGTM: Proper return object construction

The return object correctly implements the DidWebGeneratedKeyPair interface with:

  • Multikey verification type for modern cryptosuites
  • Properly exported multibase-encoded keys
  • Consistent structure with other key pair generators

33-33: No change needed: seedBase58 should exclude the multibase “z” prefix

The .slice(1) call consistently removes the multibase prefix (the leading “z”) and matches how BLS‐12381 and BBS‐2023 seeds are handled elsewhere. All existing tests (in both keyPair and wellKnown modules) expect seedBase58 values without the “z” prefix, so this behavior is intentional and correct.

packages/w3c-issuer/src/did-web/keyPair/types.ts (2)

8-9: LGTM: Appropriate multibase key support

The addition of secretKeyMultibase and publicKeyMultibase optional properties correctly supports modern cryptosuites like BBS-2023 and ECDSA-SD-2023 that use multibase encoding.


18-19: LGTM: Consistent type definition updates

The Pick operation is correctly updated to include the new multibase key properties, maintaining consistency between the input options and generated key pair types.

apps/w3c-cli/tests/main.test.ts (2)

294-325: LGTM: Comprehensive mock implementation

The mock implementation provides a well-structured test fixture with:

  • Proper DID document structure with all required fields
  • Consistent use of enum values for verification types
  • Realistic key data for testing scenarios

This enhances test coverage and aligns with the updated type system.


348-348: LGTM: Consistent enum usage in assertions

The test assertions correctly use w3cIssuer.VerificationType.Bls12381G2Key2020 enum values instead of string literals, ensuring type safety and consistency with the updated type system.

Also applies to: 359-359

packages/w3c-issuer/src/did-web/keyPair/index.test.ts (3)

4-4: LGTM: Appropriate import addition

The import of CryptoSuite correctly supports testing of the new modern cryptosuites alongside the existing VerificationType.


120-164: LGTM: Comprehensive BBS2023 test coverage

The BBS2023 test suite provides excellent coverage:

  • Basic key generation without seed
  • Seeded key generation
  • Deterministic behavior verification
  • Proper multibase encoding validation with expected prefixes

The tests correctly verify that BBS-2023 keys use Multikey verification type and multibase encoding.


177-179: Confirmed correct multibase prefixes for ECDSA-SD-2023 keys

The W3C Multikey spec mandates base58-btc (“z”) as the multibase prefix for both public and secret ECDSA-SD-2023 keys. For P-256 public keys, the 0x1200 multicodec prefix encodes to a base58 string beginning with “Dn”, so publicKeyMultibase values should indeed start with zDn. The secretKeyMultibase only needs to start with z. No changes required.

packages/w3c-issuer/src/lib/types.ts (4)

3-6: LGTM: Well-structured CryptoSuite enum

The new CryptoSuite enum follows good naming conventions with kebab-case values and includes the modern cryptographic suites mentioned in the PR objectives.


18-18: LGTM: Appropriate Multikey verification type addition

The addition of Multikey to the VerificationType enum correctly supports modern cryptosuites that use multibase-encoded keys.


36-36: LGTM: Flexible type union for key pairs

The union type VerificationType | CryptoSuite correctly enables the type system to support both legacy verification types and modern cryptosuites, providing backward compatibility while enabling new features.


32-32: Context URL Correct
Confirmed that https://w3id.org/security/multikey/v1 is the official W3C context URL for the Multikey verification method (per the Verifiable Credential Data Integrity 1.0 specification). No changes needed.

packages/w3c-issuer/src/did-web/wellKnown/generate.test.ts (1)

221-421: Comprehensive test coverage for new cryptosuites

The test additions for BBS-2023 and ECDSA-SD-2023 cryptosuites are well-structured and provide thorough coverage. The tests properly verify:

  • DID document generation with Multikey verification type
  • Context inclusion for modern cryptosuites
  • Duplicate key detection using publicKeyMultibase
  • Compatibility between legacy and modern cryptosuites
packages/w3c-issuer/src/did-web/keyPair/index.ts (1)

58-71: Good implementation of type-specific seed handling

The code correctly restricts seed preparation to only the cryptosuite types that support deterministic key generation (BLS12381G2Key2020 and BBS-2023). This prevents unnecessary random seed generation for ECDSA types.

packages/w3c-issuer/src/did-web/wellKnown/index.ts (1)

47-57: Well-structured Multikey implementation

The code correctly handles the modern Multikey format with appropriate context, multibase-encoded keys, and conditional seed inclusion for BBS-2023.

packages/w3c-issuer/src/did-web/wellKnown/index.test.ts (2)

194-255: Comprehensive test coverage for BBS-2023

The tests thoroughly verify BBS-2023 functionality including:

  • Key generation with and without seeds
  • Correct Multikey type usage
  • Proper multibase encoding
  • Context inclusion
  • Key pair property validation

289-307: Excellent test for verifying non-deterministic key generation

This test effectively verifies that ECDSA-SD-2023 generates different keys on each call, which is the expected behavior for this cryptosuite that doesn't support seed-based generation.

packages/w3c-issuer/src/did-web/wellKnown/types.ts (2)

50-58: Well-designed type structure for Multikey support

The MultikeyKeyPair type and its aliases provide a clean abstraction for modern cryptosuites. The optional @context field and fixed Multikey type ensure type safety while maintaining flexibility.


75-81: Type definitions correctly reflect cryptosuite capabilities

The private key pair types accurately represent the differences between BBS-2023 (which supports seed-based generation) and ECDSA-SD-2023 (which doesn't), ensuring type safety at compile time.

Comment thread packages/w3c-issuer/src/did-web/keyPair/bbs2023.ts
Comment on lines +12 to +16
export const generateEcdsaSd2023KeyPair = async (): Promise<DidWebGeneratedKeyPair> => {
// Generate ECDSA key pair using P-256 curve for ECDSA-SD-2023
const ecdsaKeyPair = await EcdsaMultikey.generate({
curve: 'P-256',
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for key generation failure.

The key generation process could fail, but there's no error handling to manage potential failures from the external library.

 export const generateEcdsaSd2023KeyPair = async (): Promise<DidWebGeneratedKeyPair> => {
-  // Generate ECDSA key pair using P-256 curve for ECDSA-SD-2023
-  const ecdsaKeyPair = await EcdsaMultikey.generate({
-    curve: 'P-256',
-  });
+  try {
+    // Generate ECDSA key pair using P-256 curve for ECDSA-SD-2023
+    const ecdsaKeyPair = await EcdsaMultikey.generate({
+      curve: 'P-256',
+    });
+  } catch (error) {
+    throw new Error(`Failed to generate ECDSA-SD-2023 key pair: ${error.message}`);
+  }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts around lines 12 to 16,
the key generation using EcdsaMultikey.generate lacks error handling. Wrap the
key generation call in a try-catch block to catch any exceptions thrown by the
external library. In the catch block, handle the error appropriately, such as
logging the error or rethrowing it with additional context, to ensure failures
are managed gracefully.

Comment on lines +19 to +22
const exportedKeys = await ecdsaKeyPair.export({
publicKey: true,
secretKey: true,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for key export operation.

The key export operation could also fail and should be wrapped in error handling.

-  // Export the key pair to get multibase-encoded keys
-  const exportedKeys = await ecdsaKeyPair.export({
-    publicKey: true,
-    secretKey: true,
-  });
+  // Export the key pair to get multibase-encoded keys
+  let exportedKeys;
+  try {
+    exportedKeys = await ecdsaKeyPair.export({
+      publicKey: true,
+      secretKey: true,
+    });
+  } catch (error) {
+    throw new Error(`Failed to export ECDSA-SD-2023 keys: ${error.message}`);
+  }
📝 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
const exportedKeys = await ecdsaKeyPair.export({
publicKey: true,
secretKey: true,
});
// Export the key pair to get multibase-encoded keys
let exportedKeys;
try {
exportedKeys = await ecdsaKeyPair.export({
publicKey: true,
secretKey: true,
});
} catch (error) {
throw new Error(`Failed to export ECDSA-SD-2023 keys: ${error.message}`);
}
🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts around lines 19 to 22,
the key export operation using ecdsaKeyPair.export is not wrapped in error
handling. To fix this, wrap the export call in a try-catch block to catch any
errors that may occur during the export process. In the catch block, handle the
error appropriately, such as logging the error or rethrowing it with additional
context.

Comment on lines +24 to +28
const keyPair: DidWebGeneratedKeyPair = {
type: VerificationType.Multikey,
secretKeyMultibase: exportedKeys.secretKeyMultibase,
publicKeyMultibase: exportedKeys.publicKeyMultibase,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate exported key format before returning.

Add validation to ensure the exported keys are properly formatted before constructing the return object.

   const keyPair: DidWebGeneratedKeyPair = {
     type: VerificationType.Multikey,
+    ...(exportedKeys.secretKeyMultibase && { secretKeyMultibase: exportedKeys.secretKeyMultibase }),
+    ...(exportedKeys.publicKeyMultibase && { publicKeyMultibase: exportedKeys.publicKeyMultibase }),
-    secretKeyMultibase: exportedKeys.secretKeyMultibase,
-    publicKeyMultibase: exportedKeys.publicKeyMultibase,
   };
+
+  // Validate that required keys are present
+  if (!keyPair.publicKeyMultibase || !keyPair.secretKeyMultibase) {
+    throw new Error('Generated key pair is missing required multibase keys');
+  }
📝 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
const keyPair: DidWebGeneratedKeyPair = {
type: VerificationType.Multikey,
secretKeyMultibase: exportedKeys.secretKeyMultibase,
publicKeyMultibase: exportedKeys.publicKeyMultibase,
};
const keyPair: DidWebGeneratedKeyPair = {
type: VerificationType.Multikey,
...(exportedKeys.secretKeyMultibase && { secretKeyMultibase: exportedKeys.secretKeyMultibase }),
...(exportedKeys.publicKeyMultibase && { publicKeyMultibase: exportedKeys.publicKeyMultibase }),
};
// Validate that required keys are present
if (!keyPair.publicKeyMultibase || !keyPair.secretKeyMultibase) {
throw new Error('Generated key pair is missing required multibase keys');
}
🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/keyPair/ecdsaSd2023.ts around lines 24 to 28,
the exported keys are used directly without validation. Add checks to verify
that exportedKeys.secretKeyMultibase and exportedKeys.publicKeyMultibase are in
the expected format (e.g., non-empty strings or valid multibase format) before
constructing the keyPair object. If validation fails, handle the error
appropriately to prevent returning malformed keys.

Comment on lines +166 to +196
describe('ECDSASD2023 Key Generation', () => {
it('should generate ECDSASD2023 key pair without seed', async () => {
const keyPair = await generateKeyPair({
type: CryptoSuite.EcdsaSd2023,
});

expect(keyPair.type).toBe('Multikey');
expect(typeof keyPair.secretKeyMultibase).toBe('string');
expect(keyPair.secretKeyMultibase?.length).toBeGreaterThan(1);
expect(typeof keyPair.publicKeyMultibase).toBe('string');
expect(keyPair.publicKeyMultibase?.length).toBeGreaterThan(1);
// Keys should be multibase encoded (start with 'z')
expect(keyPair.secretKeyMultibase).toMatch(/^z/);
expect(keyPair.publicKeyMultibase).toMatch(/^zDn/);
});

it('should generate different ECDSASD2023 keys on each call', async () => {
const keyPair1 = await generateKeyPair({
type: CryptoSuite.EcdsaSd2023,
});

const keyPair2 = await generateKeyPair({
type: CryptoSuite.EcdsaSd2023,
});

// ECDSA-SD-2023 keys are random - should be different each time
expect(keyPair1.secretKeyMultibase).not.toBe(keyPair2.secretKeyMultibase);
expect(keyPair1.publicKeyMultibase).not.toBe(keyPair2.publicKeyMultibase);
});
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix test structure: ECDSASD2023 nested incorrectly

The ECDSASD2023 test suite is nested inside the BBS2023 describe block, which creates incorrect test organization.

Apply this diff to fix the test structure:

      });
-
+    });
+
     describe('ECDSASD2023 Key Generation', () => {
       it('should generate ECDSASD2023 key pair without seed', async () => {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/keyPair/index.test.ts around lines 166 to
196, the ECDSASD2023 describe block is incorrectly nested inside the BBS2023
describe block, causing poor test organization. To fix this, move the entire
ECDSASD2023 describe block out of the BBS2023 describe block so that both are
siblings at the same level, ensuring proper separation and clarity of test
suites.

Comment on lines +97 to +103
} else if (
newKeyPair.type === VerificationType.Multikey &&
(newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair).publicKeyMultibase
) {
newVerificationMethod.publicKeyMultibase = (
newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair
).publicKeyMultibase;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for required publicKeyMultibase property.

Ensure the publicKeyMultibase property exists before assigning it to the verification method.

   } else if (
     newKeyPair.type === VerificationType.Multikey &&
     (newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair).publicKeyMultibase
   ) {
+    const multikeyKeyPair = newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair;
+    if (!multikeyKeyPair.publicKeyMultibase) {
+      throw new Error('Multikey key pair missing required publicKeyMultibase property');
+    }
-    newVerificationMethod.publicKeyMultibase = (
-      newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair
-    ).publicKeyMultibase;
+    newVerificationMethod.publicKeyMultibase = multikeyKeyPair.publicKeyMultibase;
   }
📝 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
} else if (
newKeyPair.type === VerificationType.Multikey &&
(newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair).publicKeyMultibase
) {
newVerificationMethod.publicKeyMultibase = (
newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair
).publicKeyMultibase;
} else if (
newKeyPair.type === VerificationType.Multikey &&
(newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair).publicKeyMultibase
) {
const multikeyKeyPair = newKeyPair as Bbs2023KeyPair | EcdsaSd2023KeyPair;
if (!multikeyKeyPair.publicKeyMultibase) {
throw new Error('Multikey key pair missing required publicKeyMultibase property');
}
newVerificationMethod.publicKeyMultibase = multikeyKeyPair.publicKeyMultibase;
}
🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/wellKnown/generate.ts around lines 97 to 103,
add a validation step to check that the publicKeyMultibase property exists on
newKeyPair before assigning it to newVerificationMethod.publicKeyMultibase. If
the property is missing, handle the case appropriately, such as throwing an
error or skipping the assignment, to ensure the property is always valid when
used.

Comment on lines +35 to 58
let keyPairs: PrivateKeyPair;

if (generatedKeyPair.type === VerificationType.Bls12381G2Key2020) {
// Legacy BLS
keyPairs = {
...keyPairs,
id: `${did}#keys-${keyId}`,
type: VerificationType.Bls12381G2Key2020,
controller: did,
seedBase58: generatedKeyPair?.seedBase58,
privateKeyBase58: generatedKeyPair?.privateKeyBase58,
publicKeyBase58: generatedKeyPair?.publicKeyBase58,
};
} else if (generatedKeyPair.type === VerificationType.Multikey) {
// Modern cryptosuites (BBS-2023, ECDSA-SD-2023)
keyPairs = {
'@context': VerificationContext[VerificationType.Multikey],
id: `${did}#keys-${keyId}`,
type: VerificationType.Multikey,
controller: did,
secretKeyMultibase: generatedKeyPair?.secretKeyMultibase,
publicKeyMultibase: generatedKeyPair?.publicKeyMultibase,
...(generatedKeyPair?.seedBase58 && { seedBase58: generatedKeyPair.seedBase58 }), // Only for BBS-2023
};
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for unexpected verification types

The code doesn't handle the case where generatedKeyPair.type is neither Bls12381G2Key2020 nor Multikey. This would leave keyPairs undefined and cause a runtime error.

 let keyPairs: PrivateKeyPair;

 if (generatedKeyPair.type === VerificationType.Bls12381G2Key2020) {
   // Legacy BLS
   keyPairs = {
     // ... existing code
   };
 } else if (generatedKeyPair.type === VerificationType.Multikey) {
   // Modern cryptosuites (BBS-2023, ECDSA-SD-2023)
   keyPairs = {
     // ... existing code
   };
+} else {
+  throw new Error(`Unsupported verification type: ${generatedKeyPair.type}`);
 }
📝 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
let keyPairs: PrivateKeyPair;
if (generatedKeyPair.type === VerificationType.Bls12381G2Key2020) {
// Legacy BLS
keyPairs = {
...keyPairs,
id: `${did}#keys-${keyId}`,
type: VerificationType.Bls12381G2Key2020,
controller: did,
seedBase58: generatedKeyPair?.seedBase58,
privateKeyBase58: generatedKeyPair?.privateKeyBase58,
publicKeyBase58: generatedKeyPair?.publicKeyBase58,
};
} else if (generatedKeyPair.type === VerificationType.Multikey) {
// Modern cryptosuites (BBS-2023, ECDSA-SD-2023)
keyPairs = {
'@context': VerificationContext[VerificationType.Multikey],
id: `${did}#keys-${keyId}`,
type: VerificationType.Multikey,
controller: did,
secretKeyMultibase: generatedKeyPair?.secretKeyMultibase,
publicKeyMultibase: generatedKeyPair?.publicKeyMultibase,
...(generatedKeyPair?.seedBase58 && { seedBase58: generatedKeyPair.seedBase58 }), // Only for BBS-2023
};
}
let keyPairs: PrivateKeyPair;
if (generatedKeyPair.type === VerificationType.Bls12381G2Key2020) {
// Legacy BLS
keyPairs = {
id: `${did}#keys-${keyId}`,
type: VerificationType.Bls12381G2Key2020,
controller: did,
seedBase58: generatedKeyPair?.seedBase58,
privateKeyBase58: generatedKeyPair?.privateKeyBase58,
publicKeyBase58: generatedKeyPair?.publicKeyBase58,
};
} else if (generatedKeyPair.type === VerificationType.Multikey) {
// Modern cryptosuites (BBS-2023, ECDSA-SD-2023)
keyPairs = {
'@context': VerificationContext[VerificationType.Multikey],
id: `${did}#keys-${keyId}`,
type: VerificationType.Multikey,
controller: did,
secretKeyMultibase: generatedKeyPair?.secretKeyMultibase,
publicKeyMultibase: generatedKeyPair?.publicKeyMultibase,
...(generatedKeyPair?.seedBase58 && { seedBase58: generatedKeyPair.seedBase58 }), // Only for BBS-2023
};
} else {
throw new Error(`Unsupported verification type: ${generatedKeyPair.type}`);
}
🤖 Prompt for AI Agents
In packages/w3c-issuer/src/did-web/wellKnown/index.ts around lines 35 to 58, the
code assigns keyPairs based on generatedKeyPair.type but does not handle
unexpected types, which can leave keyPairs undefined and cause runtime errors.
Add an else clause after the existing if-else blocks that throws an error or
handles the unexpected verification type explicitly to ensure keyPairs is always
defined or the error is caught early.

Comment on lines 7 to 11
"rootDir": ".",
"baseUrl": "."
},
"include": ["src/**/*.ts"],
"include": ["src/**/*.ts", "../declaration.d.ts"],
"exclude": ["node_modules", ".coverage", "src/**/*.spec.ts", "src/**/*.test.ts"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Compilation will fail because rootDir does not cover ../declaration.d.ts.

rootDir is fixed to ".", but the newly-included file lives one directory above this tsconfig (packages/declaration.d.ts).
When tsc encounters a source outside rootDir it raises TS6059 (“File is not under 'rootDir'”). The build will therefore break for anyone invoking tsc -p tsconfig.build.json.

Minimal fix – drop rootDir (let TypeScript compute the common path) or widen it:

-    "rootDir": ".",
+    // rootDir removed so files outside the package (e.g. ../declaration.d.ts)
+    // are still accepted by tsc

Alternatively:

-    "rootDir": ".",
+    "rootDir": "..",

but this changes the path layout of emitted files.
Pick the first option unless you need a fixed root.

📝 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
"rootDir": ".",
"baseUrl": "."
},
"include": ["src/**/*.ts"],
"include": ["src/**/*.ts", "../declaration.d.ts"],
"exclude": ["node_modules", ".coverage", "src/**/*.spec.ts", "src/**/*.test.ts"]
{
"compilerOptions": {
// rootDir removed so files outside the package (e.g. ../declaration.d.ts)
// are still accepted by tsc
"baseUrl": "."
},
"include": ["src/**/*.ts", "../declaration.d.ts"],
"exclude": ["node_modules", ".coverage", "src/**/*.spec.ts", "src/**/*.test.ts"]
}
🤖 Prompt for AI Agents
In packages/w3c-issuer/tsconfig.build.json around lines 7 to 11, the rootDir is
set to ".", but the included file "../declaration.d.ts" is outside this
directory, causing a TS6059 error. To fix this, remove the rootDir setting
entirely so TypeScript can automatically determine the common root directory,
ensuring all included files are covered without changing the output structure.

@nghaninn nghaninn merged commit 1ad1690 into alpha Jul 31, 2025
11 checks passed
@nghaninn nghaninn deleted the feat/new-crypto-suite-key-generation branch July 31, 2025 02:42
nghaninn pushed a commit that referenced this pull request Jul 31, 2025
# [1.3.0-alpha.1](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-issuer@1.2.1...@trustvc/w3c-issuer@1.3.0-alpha.1) (2025-07-31)

### Features

* add bbs-2023 and ecdsa-sd-2023 key generation and did support ([#67](#67)) ([1ad1690](1ad1690))
@nghaninn
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Jul 31, 2025
@nghaninn
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Jul 31, 2025
@nghaninn
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Jul 31, 2025
# [1.3.0-alpha.1](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-vc@1.2.7-alpha.3...@trustvc/w3c-vc@1.3.0-alpha.1) (2025-07-31)

### Features

* add bbs-2023 and ecdsa-sd-2023 key generation and did support ([#67](#67)) ([1ad1690](1ad1690))
@nghaninn
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Jul 31, 2025
# [1.3.0-alpha.1](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-cli@1.2.7-alpha.3...@trustvc/w3c-cli@1.3.0-alpha.1) (2025-07-31)

### Features

* add bbs-2023 and ecdsa-sd-2023 key generation and did support ([#67](#67)) ([1ad1690](1ad1690))
@nghaninn
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

nghaninn pushed a commit that referenced this pull request Jul 31, 2025
# [1.3.0-alpha.1](https://github.com/TrustVC/w3c/compare/@trustvc/w3c@1.2.7-alpha.3...@trustvc/w3c@1.3.0-alpha.1) (2025-07-31)

### Features

* add bbs-2023 and ecdsa-sd-2023 key generation and did support ([#67](#67)) ([1ad1690](1ad1690))
@nghaninn
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants