feat: update document builder to data model 2.0#104
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughUpdates the Document Builder to W3C VC Data Model 2.0, adds a derive-first verification flow (notably for EcdsaSd2023), introduces SignOptions, renames issuance/expiration fields, and refactors context handling. Tests are overhauled for v2 and new crypto suites. Verification fragments update import paths. README and dependencies are aligned. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant DB as DocumentBuilder
participant W3C as W3C VC Lib
Note over DB: Build document (VC v2, contexts, fields)
Dev->>DB: sign(privateKey, cryptoSuite, options?)
DB->>W3C: signW3C(vc, cryptoSuite, options)
W3C-->>DB: signed VC
DB-->>Dev: signed VC
alt Selective disclosure
Dev->>DB: derive(revealedAttributes)
DB->>W3C: deriveW3C(signed VC, revealedAttributes)
W3C-->>DB: derived VC
DB-->>Dev: derived VC
end
Note over DB: Verification
Dev->>DB: verify()
alt EcdsaSd2023 and not derived
DB-->>Dev: throw Error("derive before verify")
else Derived or non-SD suite
DB->>W3C: verifyW3CSignature(vc)
W3C-->>DB: result
DB-->>Dev: result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/verify/fragments/issuer-identity/w3cIssuerIdentity.ts (1)
47-61: Handle issuer as string OR object; prevent false INVALID and runtime issues.VC DM 2.0 allows issuer to be a string DID or an object with an id. Current comparisons and DID resolution assume a string, which will incorrectly mark valid credentials INVALID and can pass a non-string into checkDidWebResolve.
Apply this diff to normalize issuer and guard early:
- const doc = document as SignedVerifiableCredential; - if (doc.proof?.verificationMethod?.split('#')[0] !== doc.issuer) { + const doc = document as SignedVerifiableCredential; + const issuerDid = + typeof doc.issuer === 'string' ? doc.issuer : (doc.issuer as { id?: string })?.id; + if (!issuerDid) { + return { + type: 'ISSUER_IDENTITY', + name: 'W3CIssuerIdentity', + data: false, + reason: { message: 'Issuer must be a DID string or an object with an id.' }, + status: 'INVALID', + }; + } + if (doc.proof?.verificationMethod?.split('#')[0] !== issuerDid) { return { type: 'ISSUER_IDENTITY', name: 'W3CIssuerIdentity', data: false, reason: { message: `Issuer and verification method do not match.`, }, status: 'INVALID', }; } - const resolutionResult = await checkDidWebResolve(doc.issuer, verifierOptions?.documentLoader); + const resolutionResult = await checkDidWebResolve(issuerDid, verifierOptions?.documentLoader);src/verify/fragments/document-integrity/ecdsaW3CSignatureIntegrity.ts (1)
47-57: Make derivation retry robust; avoid brittle string match; clarify flag naming.Relying on an exact error string 'Use deriveCredential() first' is fragile. Prefer a case-insensitive test and invert the flag to wasDerived for clarity.
Apply:
- let verificationResult = await verifyW3CSignature(document, verifierOptions); - let isDerived = true; + let verificationResult = await verifyW3CSignature(document, verifierOptions); + let wasDerived = false; // Handle derivation if needed - if ( - !verificationResult.verified && - verificationResult.error?.includes(DERIVE_CREDENTIAL_ERROR) - ) { + if ( + !verificationResult.verified && + !!verificationResult.error && + /derivecredential\(\)\s*first/i.test(verificationResult.error) + ) { const derivedCredential = await deriveCredential(document, []); verificationResult = await verifyW3CSignature(derivedCredential.derived, verifierOptions); - isDerived = false; + wasDerived = true; } @@ - message: isDerived - ? 'Document verified successfully' - : 'Document verified after derivation', + message: wasDerived + ? 'Document verified after derivation' + : 'Document verified successfully',Also applies to: 65-67
README.md (1)
292-321: Verification example still uses VC v1.1 contexts and fieldsThe verification sample still shows v1.1 contexts (2018/v1), issuanceDate/expirationDate, and Linked Data proof for BbsBlsSignature2020. Given this PR’s V2 focus, provide an additional V2 example (or replace this one) to avoid confusing users.
Proposed V2-flavored verification snippet:
- '@context': [ - 'https://www.w3.org/2018/credentials/v1', - 'https://w3c-ccg.github.io/citizenship-vocab/contexts/citizenship-v1.jsonld', - 'https://w3id.org/security/bbs/v1', - 'https://w3id.org/vc/status-list/2021/v1', - ], + '@context': [ + 'https://www.w3.org/ns/credentials/v2', + 'https://w3id.org/security/data-integrity/v2', + 'https://w3id.org/security/bbs/v1', + 'https://w3id.org/vc/status-list/2021/v1' + ], credentialSubject: { /* ... */ }, - issuanceDate: '2024-04-01T12:19:52Z', - expirationDate: '2029-12-03T12:19:52Z', + validFrom: '2024-04-01T12:19:52Z', + validUntil: '2029-12-03T12:19:52Z', - proof: { type: 'BbsBlsSignature2020', /* ... */ } + proof: { + type: 'DataIntegrityProof', + cryptosuite: 'BbsBlsSignature2020', + /* ... */ + }src/core/documentBuilder.ts (2)
176-211: Persist the signed VC back into the builder (avoid relying on in-place mutation)Currently, sign() sets isSigned but does not assign the signed VC to this.document. If signW3C doesn’t mutate the input in place, subsequent derive()/verify() will read a stale doc without proof. Make assignment explicit.
Apply:
const signedVC = await signW3C(this.document, privateKey, cryptoSuite, options); if (signedVC.error) throw new Error(`Signing Error: ${signedVC.error}`); - this.isSigned = true; - return signedVC.signed; + this.isSigned = true; + this.isDerived = false; + this.document = signedVC.signed; + return this.document;
118-146: Add status-list context when configuring verifiableDocumentTo align with VCDM 2.0 bitstring status lists, include the status-list 2021 context so produced VCs are self-describing.
Apply:
import { BBS_V1_URL, DATA_INTEGRITY_V2_URL, QRCODE_CONTEXT_URL, RENDER_CONTEXT_V2_URL, TR_CONTEXT_URL, VC_V1_URL, VC_V2_URL, + STATUS_LIST_2021_V1_URL, } from '@trustvc/w3c-context';} else if (isVerifiable) { this.selectedStatusType = 'verifiableDocument'; this.statusConfig = { id: `${config.url}#${config.index}`, type: 'BitstringStatusListEntry', statusPurpose: config.purpose || 'revocation', statusListIndex: config.index, statusListCredential: config.url, }; + this.addContext(STATUS_LIST_2021_V1_URL); } else {
🧹 Nitpick comments (12)
package.json (1)
124-129: Align @trustvc/w3c-issuer version with the rest of the @trustvc/w3c stack.*Other @trustvc/w3c* packages are at ^1.3.0-alpha.7. Keeping @trustvc/w3c-issuer at ^1.3.0-alpha.5 risks subtle type or runtime mismatches across tightly-coupled modules.
Apply this diff:
- "@trustvc/w3c-issuer": "^1.3.0-alpha.5", + "@trustvc/w3c-issuer": "^1.3.0-alpha.7",src/verify/fragments/document-integrity/w3cSignatureIntegrity.ts (1)
10-16: Clarify SKIPPED message to match both supported proof types.The test handles 'BbsBlsSignature2020' and 'BbsBlsSignatureProof2020', but the SKIPPED reason mentions only the former.
Apply:
- message: `Document either has no proof or proof.type is not 'BbsBlsSignature2020'.`, + message: `Document either has no proof or proof.type is neither 'BbsBlsSignature2020' nor 'BbsBlsSignatureProof2020'.`,src/verify/fragments/document-integrity/ecdsaW3CSignatureIntegrity.ts (2)
28-31: Support array-valued proofs in VC DM 2.0.proof can be a single object or an array. The current test will skip valid credentials that carry multiple proofs.
Apply:
- const doc = document as SignedVerifiableCredential; - return doc.proof?.type === 'DataIntegrityProof' && doc.proof?.cryptosuite === 'ecdsa-sd-2023'; + const doc = document as SignedVerifiableCredential; + const proofs = Array.isArray(doc.proof) ? doc.proof : [doc.proof]; + return proofs?.some( + (p) => p?.type === PROOF_TYPE && p?.cryptosuite === CRYPTOSUITE + );
10-12: Optional: reuse shared type guard.You already import utilities from @trustvc/w3c-vc elsewhere. Consider replacing the local isSignedVerifiableCredential with isSignedDocument from @trustvc/w3c-vc for consistency across fragments.
Would you like me to propose a small patch to switch to isSignedDocument here as well?
README.md (4)
739-743: Grammar fix: “an valid until date” → “a valid-until date (expiration)”Minor English issue.
Apply:
-You can set an valid until date (expiration) for the document. +You can set a valid-until date (expiration) for the document.
831-837: Heading and flow clarity: Derivation phrasingSlightly awkward phrasing. Suggest tightening.
Apply:
-##### Deriving the Document -To derive the document, provide the attributes to be revealed to the `derive` method. +##### Deriving the document +Provide the attributes to reveal to the `derive` method.
839-845: ECDSA derive-first requirement should be explicit hereVerification requires derive() first only for ECDSA-SD-2023. Call this out so BBS2020 users don’t think it’s mandatory for them.
Apply:
-##### Verify the Document -To verify the signature of the signed document, ensure the document is derived first and then call the `verify` method. +##### Verify the document +For ECDSA-SD-2023 credentials, call `derive()` first (derive-first verification). For BBS 2020, `derive()` is optional.
710-736: Placeholder in TS snippetUse a concrete number for index; placeholders in TS code blocks can mislead copy/paste.
Apply:
- index: <placeholder>, + index: 10,src/__tests__/core/documentBuilder.test.ts (2)
123-151: Derive with at least one pointer for stronger ECDSA-SD signalDeriving with an empty array passes, but using at least one pointer (e.g., '/type') will better exercise the SD path and reduce the risk of false positives if defaults change.
Apply:
- const derivedDocument = await documentBuilder.derive([]); + const derivedDocument = await documentBuilder.derive(['/type']);
334-345: BBS2020 derive-and-verify path is realisticExercising derive(contextDocument) for BBS2020 with the TR context is a solid integration check. Consider adding one assertion that the derived doc’s @context contains the TR context (to strengthen coupling to builder.addContext).
Suggested extra assertion:
const derivedDocument = await documentBuilder.derive(contextDocument); expect(derivedDocument).toBeDefined(); + expect(derivedDocument['@context']).toEqual( + expect.arrayContaining(['https://trustvc.io/context/transferable-records-context.json']), + );Also applies to: 353-374
src/core/documentBuilder.ts (2)
110-114: credentialSubject() parameter typeThe method currently takes Partial, which is wider than needed and masks type errors. Narrow to the credentialSubject shape.
Apply:
- credentialSubject(subject: Partial<VerifiableCredential>) { + credentialSubject(subject: VerifiableCredential['credentialSubject']) { if (this.isSigned) throw new Error('Configuration Error: Document is already signed.'); this.document.credentialSubject = subject; return this; }
68-76: Typo and naming convention for QR Code options
- JSDoc: “qrcoode” → “qrcode”.
- Exported interface name qrCode should be PascalCase (QRCode or QrCode) for consistency with TS conventions. Rename is a breaking change; consider aliasing the old name for back-compat.
Apply:
-/** - * Configuration for the qrcoode used in a Verifiable Credential document. +/** + * Configuration for the qrcode used in a Verifiable Credential document. * @property {string} uri - A unique identifier for the qrcode, typically a URL or URI. * @property {string} type - The type of the qrcode method (e.g., 'TrustVCQRCode'). */ -export interface qrCode { +export interface QRCode { uri: string; type: string; }- // Defines the qrcode for the document. - qrCode(method: qrCode) { + // Defines the qrcode for the document. + qrCode(method: QRCode) {To avoid breaking consumers, optionally export a type alias:
export type qrCode = QRCode; // backwards compatibilityAlso applies to: 168-174
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
README.md(7 hunks)package.json(1 hunks)src/__tests__/core/documentBuilder.test.ts(9 hunks)src/core/documentBuilder.ts(8 hunks)src/verify/fragments/document-integrity/ecdsaW3CSignatureIntegrity.ts(1 hunks)src/verify/fragments/document-integrity/w3cSignatureIntegrity.ts(1 hunks)src/verify/fragments/document-status/w3cCredentialStatus.ts(1 hunks)src/verify/fragments/issuer-identity/w3cIssuerIdentity.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~767-~767: There might be a mistake here.
Context: ...rfrom@trustvc/trustvc`. The builder uses ECDSA key for signing by default. ```t...
(QB_NEW_EN)
[grammar] ~831-~831: There might be a mistake here.
Context: ..., } } ``` ##### Deriving the Document To derive the document, provide the attr...
(QB_NEW_EN)
[grammar] ~839-~839: There might be a mistake here.
Context: ...ocument); ``` ##### Verify the Document To verify the signature of the signed do...
(QB_NEW_EN)
🔇 Additional comments (9)
src/verify/fragments/issuer-identity/w3cIssuerIdentity.ts (1)
4-4: Import refactor LGTM.Importing SignedVerifiableCredential from @trustvc/w3c-vc aligns with the package.json bumps and improves module clarity.
src/verify/fragments/document-integrity/w3cSignatureIntegrity.ts (1)
2-4: verifyW3CSignature import path is correctI’ve confirmed that
verifyW3CSignatureis exported as a named export insrc/w3c/verify.ts(line 11) and that the deep import (../../../w3c/verify) inw3cSignatureIntegrity.tsresolves properly. No further changes are needed.src/verify/fragments/document-status/w3cCredentialStatus.ts (1)
6-11: Import consolidation LGTM.Using @trustvc/w3c-vc for SignedVerifiableCredential, isSignedDocument, and verifyCredentialStatus keeps the fragments consistent with the v2 refactor.
src/verify/fragments/document-integrity/ecdsaW3CSignatureIntegrity.ts (1)
2-3: Import refactor LGTM.Explicit imports from @trustvc/w3c-vc and the internal verify helper improve clarity.
src/__tests__/core/documentBuilder.test.ts (4)
31-41: LGTM: ECDSA V2 initialization assertions are appropriateGood coverage ensuring V2 base context and data-integrity context are present after signing. This will catch regressions in context handling.
219-230: Good coverage for validUntil via string and DateThese tests concretely validate the v2 field migration from expirationDate → validUntil. Nice.
246-249: Context extension assertions are helpfulAsserting the presence of render-method-context-v2.json and qrcode-context.json ensures builder context augmentation remains stable.
Also applies to: 264-267
160-172: Great negative test for derive-first requirementThis precisely captures the ECDSA derive-first constraint and protects against regressions in verify().
src/core/documentBuilder.ts (1)
198-206: Context selection for crypto suitesCurrent logic adds data-integrity v2 only for ecdsa-sd-2023 and bbs v1 for BBS2020. Consider also including DI v2 for BBS when the proof type is DataIntegrityProof (if your signer emits DataIntegrityProof for BBS). If not, this is fine—flagging to verify consistency.
Please confirm what proof.type and contexts signW3C emits for 'BbsBlsSignature2020'. If it emits DataIntegrityProof, include DI v2:
- } else { - this.addContext(BBS_V1_URL); - } + } else { + this.addContext(BBS_V1_URL); + // If your BBS signer uses DataIntegrityProof, also: + // this.addContext(DATA_INTEGRITY_V2_URL); + }
| To sign the document, provide a `PrivateKeyPair` from `@trustvc/trustvc`. The builder uses ECDSA key for signing by default. | ||
|
|
||
| ```ts | ||
| const privateKey: PrivateKeyPair = { | ||
| id: 'did:example:456#key1', | ||
| controller: 'did:example:456', | ||
| type: VerificationType.Bls12381G2Key2020, | ||
| publicKeyBase58: 'your-public-key-base58', | ||
| privateKeyBase58: 'your-private-key-base58', | ||
| }; | ||
| '@context': 'https://w3id.org/security/multikey/v1', | ||
| id: 'did:web:example.com#multikey-1', | ||
| type: VerificationType.Multikey, | ||
| controller: 'did:web:example.com', | ||
| publicKeyMultibase: 'your-public-key-multibase', | ||
| secretKeyMultibase: 'your-secret-key-multibase', | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify article usage and import path for key type
- “The builder uses ECDSA key…” → should be “an ECDSA key” or “ECDSA keys.”
- Double-check the import path of PrivateKeyPair. README says to import from @trustvc/trustvc, but tests import from @trustvc/w3c-issuer. Please align.
Run to confirm what your package actually exports:
🏁 Script executed:
#!/bin/bash
# Inspect re-exports of PrivateKeyPair from the library root.
rg -nC2 'export\s+.*PrivateKeyPair' -g '!**/dist/**'Length of output: 218
Update README: correct ECDSA phrasing and import source
-
Change the sentence
“The builder uses ECDSA key for signing by default.”
to
“The builder uses an ECDSA key for signing by default.” -
Align the import path for
PrivateKeyPairwith the actual export: in your README replaceimport type { PrivateKeyPair } from '@trustvc/trustvc';
with
import type { PrivateKeyPair } from '@trustvc/w3c-issuer';
to match the re-export in
src/w3c/types.ts(see line 7).
🧰 Tools
🪛 LanguageTool
[grammar] ~767-~767: There might be a mistake here.
Context: ...rfrom@trustvc/trustvc`. The builder uses ECDSA key for signing by default. ```t...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around lines 767 to 777, update the phrasing and import source:
change the sentence "The builder uses ECDSA key for signing by default." to "The
builder uses an ECDSA key for signing by default." and change the import
statement to reference the actual export by replacing "import type {
PrivateKeyPair } from '@trustvc/trustvc';" with "import type { PrivateKeyPair }
from '@trustvc/w3c-issuer';" so the README matches src/w3c/types.ts re-exports.
| "https://www.w3.org/ns/credentials/v2", | ||
| "https://w3c-ccg.github.io/citizenship-vocab/contexts/citizenship-v2.jsonld", | ||
| "https://trustvc.io/context/render-method-context-v2.json", | ||
| "https://trustvc.io/context/qrcode-context.json", | ||
| "https://w3id.org/security/bbs/v1" | ||
| "https://w3id.org/security/data-integrity/v2" | ||
| ], |
There was a problem hiding this comment.
Status list entry type inconsistent with code (V2 uses BitstringStatusListEntry)
Your code sets the credentialStatus.type to "BitstringStatusListEntry" but the README’s signed output shows "StatusList2021Entry". Align the docs with the implementation and add the status-list context if required in examples.
Apply:
"credentialStatus": {
- "id": "https://example.com/status-list#<placeholder>",
- "type": "StatusList2021Entry",
+ "id": "https://example.com/status-list#<placeholder>",
+ "type": "BitstringStatusListEntry",
"statusPurpose": "revocation",
"statusListIndex": "<placeholder>",
"statusListCredential": "https://example.com/status-list"
},If examples rely on status lists, also include the context in the example’s @context array:
"@context": [
"https://www.w3.org/ns/credentials/v2",
"https://w3c-ccg.github.io/citizenship-vocab/contexts/citizenship-v2.jsonld",
"https://trustvc.io/context/render-method-context-v2.json",
"https://trustvc.io/context/qrcode-context.json",
"https://w3id.org/security/data-integrity/v2",
+ "https://w3id.org/vc/status-list/2021/v1"
],Also applies to: 810-816
🤖 Prompt for AI Agents
In README.md around lines 787-792 (and similarly 810-816), the example
credential output shows credentialStatus.type as "StatusList2021Entry" which is
inconsistent with the code that uses "BitstringStatusListEntry"; update the
examples to use "BitstringStatusListEntry" and add the appropriate status-list
context URL (e.g. the Status List 2021/2022 context) into the example @context
arrays so the status type is valid and the examples mirror the implementation.
| if (this.document.proof.cryptosuite === CryptoSuite.EcdsaSd2023 && !this.isDerived) { | ||
| throw new Error('Verification Error: Document is not derived yet. Use derive() first.'); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against missing proof in verify()
Accessing this.document.proof.cryptosuite can throw if proof is missing or the document wasn’t persisted. Use optional chaining and the SignedVerifiableCredential type where needed.
Apply:
- if (this.document.proof.cryptosuite === CryptoSuite.EcdsaSd2023 && !this.isDerived) {
+ const cryptosuite = (this.document as SignedVerifiableCredential)?.proof?.cryptosuite;
+ if (cryptosuite === CryptoSuite.EcdsaSd2023 && !this.isDerived) {
throw new Error('Verification Error: Document is not derived yet. Use derive() first.');
}📝 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.
| if (this.document.proof.cryptosuite === CryptoSuite.EcdsaSd2023 && !this.isDerived) { | |
| throw new Error('Verification Error: Document is not derived yet. Use derive() first.'); | |
| } | |
| const cryptosuite = (this.document as SignedVerifiableCredential)?.proof?.cryptosuite; | |
| if (cryptosuite === CryptoSuite.EcdsaSd2023 && !this.isDerived) { | |
| throw new Error('Verification Error: Document is not derived yet. Use derive() first.'); | |
| } |
🤖 Prompt for AI Agents
In src/core/documentBuilder.ts around lines 231 to 234, the code directly
accesses this.document.proof.cryptosuite which will throw if proof is missing;
update the guard to safely handle missing or non-persisted proof by using
optional chaining (e.g., this.document.proof?.cryptosuite) and narrow the
document type to SignedVerifiableCredential where appropriate; if proof is
absent, throw a clear verification error (e.g., "Verification Error: Missing
proof. Persist or attach proof before calling verify()") and only check the
cryptosuite when proof exists and the document is the expected
SignedVerifiableCredential type.
|
## [1.6.0-alpha.7](v1.6.0-alpha.6...v1.6.0-alpha.7) (2025-08-22) ### Features * update document builder to data model 2.0 ([#104](#104)) ([f04f153](f04f153))
|
🎉 This PR is included in version 1.6.0-alpha.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Summary
Document builder update to build w3c vc data model 2.0 document using ECDSA as default
Test cases
README update
Summary by CodeRabbit