feat: update sign and derive#114
Conversation
WalkthroughDocumentation and tests updated to add BBS-2023 support alongside ECDSA-SD-2023, deprecate BbsBlsSignature2020 (now errors), narrow derive APIs to accept Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Sign as signW3C
participant Suite as Crypto Suite\n(ECDSA-SD-2023 | BBS-2023)
participant VC as Verifiable Credential
App->>Sign: signW3C(VC, key, { suite, mandatoryPointers? })
alt suite == ECDSA-SD-2023 or BBS-2023
Note over Sign,Suite: Process pointers if provided
Sign->>Suite: Create DataIntegrityProof
Suite-->>Sign: proof
Sign-->>App: VC + proof
else suite == BbsBlsSignature2020
Sign-->>App: Error ("BbsBlsSignature2020 unsupported")
end
sequenceDiagram
autonumber
actor Verifier
participant Derive as deriveW3C
participant VC as Signed VC (v2.0)
Verifier->>Derive: deriveW3C(VC, revealedAttributes: string[])
Note over Derive: API narrowed — `revealedAttributes` is `string[]`
Derive-->>Verifier: Derived (selective disclosure) VC
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/core/documentBuilder.ts (1)
187-187: Consider creating an issue to track this TODO.The TODO indicates a planned restriction to BitStringStatusList with v2.0 for the document builder. If this represents planned work, consider creating an issue to track it.
Do you want me to generate an issue template for tracking this restriction?
src/__tests__/w3c/derive.test.ts (1)
11-61: Consider reducing test duplication with a parameterized approach.All four test cases follow the same pattern with only the fixture and description varying. You could refactor using
it.eachto reduce duplication:import { describe, expect, it } from 'vitest'; import { ECDSA_W3C_VERIFIABLE_DOCUMENT_V2_0, BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0, } from '../fixtures/fixtures'; import { deriveW3C } from 'src/w3c'; import { SignedVerifiableCredential } from '@trustvc/w3c-vc'; describe('W3C derive', () => { // credentialStatus is defined since the document has been signed with credentialStatus as mandatory parameter it.each([ ['ECDSA-SD-2023', ECDSA_W3C_VERIFIABLE_DOCUMENT_V2_0], ['BBS-2023', BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0], ])('should derive a W3C v2.0 document using %s without custom selective pointers', async (suite, fixture) => { const result = await deriveW3C( fixture as SignedVerifiableCredential, [], ); expect(result.derived).toBeDefined(); expect(result.derived.proof).toBeDefined(); expect(result.derived['@context']).toBeDefined(); expect(result.derived.credentialStatus).toBeDefined(); expect(result.derived.renderMethod).toBeUndefined(); expect(result.derived.qrCode).toBeUndefined(); }); it.each([ ['ECDSA-SD-2023', ECDSA_W3C_VERIFIABLE_DOCUMENT_V2_0], ['BBS-2023', BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0], ])('should derive a W3C v2.0 document using %s with custom selective pointers', async (suite, fixture) => { const result = await deriveW3C( fixture as SignedVerifiableCredential, ['/renderMethod', '/qrCode'], ); expect(result.derived).toBeDefined(); expect(result.derived.proof).toBeDefined(); expect(result.derived['@context']).toBeDefined(); expect(result.derived.credentialStatus).toBeDefined(); expect(result.derived.renderMethod).toBeDefined(); expect(result.derived.qrCode).toBeDefined(); }); });This approach maintains test clarity while reducing duplication, making it easier to add new cryptosuites in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
README.md(6 hunks)package.json(1 hunks)src/__tests__/core/documentBuilder.test.ts(4 hunks)src/__tests__/fixtures/fixtures.ts(1 hunks)src/__tests__/w3c/derive.test.ts(3 hunks)src/__tests__/w3c/sign.test.ts(4 hunks)src/core/documentBuilder.ts(2 hunks)src/w3c/derive.ts(1 hunks)src/w3c/sign.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/w3c/derive.ts (1)
src/w3c/types.ts (1)
SignedVerifiableCredential(4-4)
src/__tests__/w3c/sign.test.ts (2)
src/__tests__/fixtures/fixtures.ts (1)
BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0(1615-1679)src/w3c/sign.ts (1)
signW3C(13-23)
src/__tests__/w3c/derive.test.ts (2)
src/w3c/derive.ts (1)
deriveW3C(10-15)src/__tests__/fixtures/fixtures.ts (1)
BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0(1615-1679)
🪛 Gitleaks (8.28.0)
src/__tests__/fixtures/fixtures.ts
[high] 1629-1629: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1630-1630: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (13)
src/w3c/sign.ts (1)
9-10: LGTM! Documentation accurately reflects multi-suite support.The JSDoc update correctly documents that
mandatoryPointersapply to both ECDSA-SD-2023 and BBS-2023 cryptographic suites, aligning with the PR's objective to add BBS-2023 support.package.json (1)
125-129: Verify alpha version usage is intentional.The dependencies are pinned to alpha versions (^1.3.0-alpha.14, etc.) while a stable 2.0.7 release exists. Confirm this is intentional for accessing pre-release BBS-2023 features, and ensure the team plans to migrate to stable versions once they're released.
Based on learnings.
README.md (1)
158-227: LGTM! Clear documentation of BBS-2023 support.The documentation effectively:
- Explains the new BBS-2023 cryptosuite option
- Provides working examples with proper key formats
- Includes a clear deprecation notice for BbsBlsSignature2020
- Shows mandatory pointers usage for both suites
src/__tests__/w3c/sign.test.ts (2)
34-37: LGTM! Correctly validates BbsBlsSignature2020 deprecation.The test properly asserts that BbsBlsSignature2020 now returns an error with an appropriate message, ensuring users are directed to use modern cryptosuites.
55-68: LGTM! Comprehensive BBS-2023 test coverage.The test cases properly cover:
- Basic BBS-2023 signing
- BBS-2023 signing with mandatory pointers
- Correct key format (publicKeyMultibase/secretKeyMultibase)
- DataIntegrityProof verification
Also applies to: 102-133
src/__tests__/fixtures/fixtures.ts (2)
1629-1630: Static analysis false positive: tokenId is not a secret.The flagged values are blockchain token IDs (hex hashes), not API keys or secrets. These are safe to include in test fixtures as they're publicly visible on-chain identifiers.
1615-1679: No duplicate export detected. Only one export ofBBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0exists insrc/__tests__/fixtures/fixtures.ts.src/__tests__/core/documentBuilder.test.ts (2)
27-29: LGTM! Tests updated with clearer context and expectations.The test initialization now uses a specific Bill of Lading context and properly asserts its presence, making the test more explicit and maintainable.
Also applies to: 39-40, 55-55
283-289: LGTM! Focused test for BbsBlsSignature2020 deprecation.The test concisely validates that using the deprecated BbsBlsSignature2020 cryptosuite produces an appropriate error, replacing the previous multi-party flow test with a more maintainable assertion.
src/core/documentBuilder.ts (1)
213-213: LGTM! Type narrowing aligns with deriveW3C changes.The derive method signature correctly narrows the parameter type to
string[], consistent with the changes insrc/w3c/derive.ts. This provides a cleaner, more focused API for JSON pointer-based selective disclosure.src/w3c/derive.ts (1)
1-1: Confirm removal of ContextDocument support in deriveW3C is intentionalderiveW3C’s
revealedAttributesparameter narrowed fromContextDocument | string[]tostring[], causing a breaking change for any callers passing ContextDocument objects.src/__tests__/w3c/derive.test.ts (2)
11-35: LGTM: ECDSA test cases are well-structured.The ECDSA-SD-2023 test cases properly verify both scenarios (with and without custom selective pointers) and have appropriate expectations for mandatory vs. revealed fields.
37-61: LGTM: BBS-2023 test coverage successfully added.The new BBS-2023 test cases properly mirror the existing ECDSA-SD-2023 tests, ensuring consistent behavior across cryptosuites for both scenarios (with and without custom selective pointers).
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/__tests__/w3c/derive.test.ts (1)
42-65: Consider adding error case coverage.The parameterized test structure effectively covers the happy paths for both ECDSA-SD-2023 and BBS-2023. Consider adding test cases for error scenarios such as:
- Invalid selective pointers
- Malformed credentials
- Missing required fields
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/__tests__/w3c/derive.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/w3c/derive.test.ts (2)
src/__tests__/fixtures/fixtures.ts (2)
ECDSA_W3C_VERIFIABLE_DOCUMENT_V2_0(1420-1486)BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0(1615-1679)src/w3c/derive.ts (1)
deriveW3C(10-15)
🔇 Additional comments (4)
src/__tests__/w3c/derive.test.ts (4)
2-5: LGTM! Clean import of BBS-2023 fixture.The addition of
BBS2023_W3C_VERIFIABLE_DOCUMENT_V2_0aligns with the PR's objective to add BBS-2023 support and test coverage.
10-20: Excellent refactoring to parameterized tests.The test case configuration effectively supports both ECDSA-SD-2023 and BBS-2023 cryptosuites, improving test coverage while reducing duplication.
22-39: Well-structured scenario configuration.The scenario-based testing approach cleanly separates the two derive behaviors (with and without custom selective pointers) and their expected outcomes.
45-45: Confirm necessity of the type assertion
Thedocument as SignedVerifiableCredentialcast may be masking mismatches between the fixture’s literal type and the external interface. Verify the fixture fully satisfiesSignedVerifiableCredential(or export it with that type) rather than silencing potential type errors.
## [2.1.0](v2.0.7...v2.1.0) (2025-10-09) ### Features * update sign and derive ([#114](#114)) ([e4419a5](e4419a5))
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Summary
Changes
Summary by CodeRabbit
New Features
Deprecations
Documentation
Tests
Chores