fix: add derivation support for ecdsa bitstring vc#81
Merged
Conversation
WalkthroughAdds a fallback verification path in credential status verification: on a specific failure, derive a credential first and re-run verification. Updates error handling to separate base error message from detailed logs. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Verifier as CredentialStatusVerifier
participant VC as verifyCredential(...)
participant Deriver as deriveCredential(...)
Client->>Verifier: verify(vcStatusList, options)
Verifier->>VC: verifyCredential(vcStatusList, options)
VC-->>Verifier: Error (may contain "Use deriveCredential() first")
alt Error contains DERIVE_CREDENTIAL_ERROR
Verifier->>Deriver: deriveCredential(vcStatusList, [])
Deriver-->>Verifier: { derived }
Verifier->>VC: verifyCredential(derived, options)
alt Verification succeeds
VC-->>Verifier: Success result
Verifier-->>Client: Success result
else Verification fails
VC-->>Verifier: Error
Note over Verifier: Log base + detailed error\nThrow base error message
Verifier-->>Client: Error (base message)
end
else Other error
Note over Verifier: Log base + detailed error\nThrow base error message
Verifier-->>Client: Error (base message)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
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.
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. ✨ 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. Comment |
|
Contributor
Author
|
@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: 2
🧹 Nitpick comments (2)
packages/w3c-vc/src/lib/verify/credentialStatus/index.ts (2)
55-57: Brittle substring match; compute a needsDerivation flag using a structured code when availableRelying on the literal text "Use deriveCredential() first" is fragile and could break on message changes or localization. Compute a single needsDerivation flag that prefers a structured code and only falls back to substring matching.
Apply:
const DERIVE_CREDENTIAL_ERROR = 'Use deriveCredential() first'; -let vcStatusListVerificationResult = await verifyCredential(vcStatusList, options); +let vcStatusListVerificationResult = await verifyCredential(vcStatusList, options); +const needsDerivation = + !vcStatusListVerificationResult?.verified && + ( + // Prefer a structured discriminator if available + (vcStatusListVerificationResult as any).code === 'VC_NEEDS_DERIVATION' || + // Fallback to substring match + vcStatusListVerificationResult.error?.includes(DERIVE_CREDENTIAL_ERROR) + );
12-12: Introduce structured error codes for verifyCredential
Replace brittle.error.includes('Use deriveCredential() first')checks inpackages/w3c-vc/src/lib/verify/credentialStatus/index.tswith a dedicated machine-readable error code or custom error class onVerificationResult(e.g.result.code === 'VC_NEEDS_DERIVATION').
📜 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 (1)
packages/w3c-vc/src/lib/verify/credentialStatus/index.ts(2 hunks)
Comment on lines
+58
to
+66
| // Handle ECDSA-SD-2023 base credentials that need derivation before verification | ||
| if ( | ||
| !vcStatusListVerificationResult?.verified && | ||
| vcStatusListVerificationResult.error?.includes(DERIVE_CREDENTIAL_ERROR) | ||
| ) { | ||
| const derivedResult = await deriveCredential(vcStatusList, []); | ||
| vcStatusListVerificationResult = await verifyCredential(derivedResult.derived, options); | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Derivation path: pass documentLoader, track the verified VC, and base downstream decoding on it
- Include options.documentLoader in deriveCredential for consistency.
- Track which artifact (original vs derived) was actually verified, then decode/use that, not the pre-verified original.
- Guard against absent derived.derived.
This avoids computing status from an object that wasn’t the one you verified.
If deriveCredential accepts options, confirm signature across the repo:
Suggested changes:
-// Handle ECDSA-SD-2023 base credentials that need derivation before verification
-if (
- !vcStatusListVerificationResult?.verified &&
- vcStatusListVerificationResult.error?.includes(DERIVE_CREDENTIAL_ERROR)
-) {
- const derivedResult = await deriveCredential(vcStatusList, []);
- vcStatusListVerificationResult = await verifyCredential(derivedResult.derived, options);
-}
+// Track which VC we ultimately verified
+let verifiedVC: SignedVerifiableCredential = vcStatusList;
+// Handle ECDSA-SD-2023 base credentials that need derivation before verification
+if (needsDerivation) {
+ const derivedResult = await deriveCredential(
+ vcStatusList,
+ [], // TODO: confirm minimal reveal frame is correct for status-list VCs
+ { documentLoader: options?.documentLoader } as any
+ );
+ const derivedVC = derivedResult?.derived;
+ if (derivedVC) {
+ verifiedVC = derivedVC;
+ vcStatusListVerificationResult = await verifyCredential(derivedVC, options);
+ }
+}Follow-up (outside this hunk): move decoding/asserts to run against verifiedVC after verification:
// After verification succeeds:
const statusListCS = verifiedVC.credentialSubject as VCBitstringCredentialSubject;
assertStatusPurposeMatches(statusListCS, statusPurpose);
const bitstringStatusList = await StatusList.decode(statusListCS);
assertStatusListIndexWithinRange(bitstringStatusList, index);
const status = bitstringStatusList.getStatus(index);🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\bderiveCredential\s*\(' --type=tsLength of output: 7578
🏁 Script executed:
#!/bin/bash
# Locate deriveCredential definition in w3c-vc.ts to confirm its signature
rg -nP 'export\s+(async\s+)?function\s+deriveCredential' -n packages/w3c-vc/src/lib/w3c-vc.ts
# Show surrounding lines to inspect parameters
sed -n '330,380p' packages/w3c-vc/src/lib/w3c-vc.tsLength of output: 2143
🏁 Script executed:
#!/bin/bash
# Find the deriveCredential declaration to inspect its parameters
rg -nP 'export\s+(async\s+)?function\s+deriveCredential\b' -n packages/w3c-vc/src/lib
rg -nP 'const\s+deriveCredential\b' -n packages/w3c-vc/src/libLength of output: 77
🏁 Script executed:
#!/bin/bash
sed -n '380,440p' packages/w3c-vc/src/lib/w3c-vc.tsLength of output: 2703
🏁 Script executed:
#!/bin/bash
sed -n '1,100p' packages/w3c-vc/src/lib/verify/credentialStatus/index.tsLength of output: 3555
Track and Verify Derived Credential Before Decoding
- Pass the incoming loader into deriveCredential, e.g.
const derivedResult = await deriveCredential(vcStatusList, [], { documentLoader: options?.documentLoader }); - Introduce
let verifiedVC = vcStatusList; on a DERIVE_CREDENTIAL_ERROR verify failure, guardderivedResult.derived, setverifiedVC = derivedResult.derived, then re-verify. - Move
assertStatusPurposeMatches,StatusList.decode,assertStatusListIndexWithinRangeandbitstringStatusList.getStatusto after the successful verification ofverifiedVCso you only decode a signature-verified credential.
🤖 Prompt for AI Agents
In packages/w3c-vc/src/lib/verify/credentialStatus/index.ts around lines 58 to
66, handle derived credentials and decoding only after signature verification:
pass the incoming document loader into deriveCredential (e.g.
deriveCredential(vcStatusList, [], { documentLoader: options?.documentLoader
})), introduce let verifiedVC = vcStatusList and when
vcStatusListVerificationResult indicates a DERIVE_CREDENTIAL_ERROR check that
derivedResult.derived exists, set verifiedVC = derivedResult.derived and re-run
verifyCredential(verifiedVC, options); only after the successful verification of
verifiedVC move/assert assertStatusPurposeMatches, call StatusList.decode and
assertStatusListIndexWithinRange and use bitstringStatusList.getStatus so
decoding and index checks happen on a signature-verified credential.
Comment on lines
+67
to
76
| // Throw error if verification still fails | ||
| if (!vcStatusListVerificationResult?.verified) { | ||
| console.error( | ||
| `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}. Error: ${vcStatusListVerificationResult.error}`, | ||
| ); | ||
| throw new Error( | ||
| `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}`, | ||
| ); | ||
| const errorMessage = `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}`; | ||
| const detailedError = vcStatusListVerificationResult.error | ||
| ? `. Error: ${vcStatusListVerificationResult.error}` | ||
| : ''; | ||
|
|
||
| console.error(errorMessage + detailedError); | ||
| throw new Error(errorMessage); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve error details and avoid logging from library code
- The message currently embeds a boolean (verified) and drops the underlying error for callers. Throw the combined message, not just the base string.
- Prefer not to console.error in library code; let callers control logging.
Apply:
-// Throw error if verification still fails
if (!vcStatusListVerificationResult?.verified) {
- const errorMessage = `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}`;
- const detailedError = vcStatusListVerificationResult.error
- ? `. Error: ${vcStatusListVerificationResult.error}`
- : '';
-
- console.error(errorMessage + detailedError);
- throw new Error(errorMessage);
+ const baseMessage = 'Failed to verify Credential Status VC';
+ const detailed = vcStatusListVerificationResult.error
+ ? `: ${vcStatusListVerificationResult.error}`
+ : '';
+ throw new Error(baseMessage + detailed);
}📝 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
| // Throw error if verification still fails | |
| if (!vcStatusListVerificationResult?.verified) { | |
| console.error( | |
| `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}. Error: ${vcStatusListVerificationResult.error}`, | |
| ); | |
| throw new Error( | |
| `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}`, | |
| ); | |
| const errorMessage = `Failed to verify Credential Status VC: ${vcStatusListVerificationResult.verified}`; | |
| const detailedError = vcStatusListVerificationResult.error | |
| ? `. Error: ${vcStatusListVerificationResult.error}` | |
| : ''; | |
| console.error(errorMessage + detailedError); | |
| throw new Error(errorMessage); | |
| } | |
| if (!vcStatusListVerificationResult?.verified) { | |
| const baseMessage = 'Failed to verify Credential Status VC'; | |
| const detailed = vcStatusListVerificationResult.error | |
| ? `: ${vcStatusListVerificationResult.error}` | |
| : ''; | |
| throw new Error(baseMessage + detailed); | |
| } |
🤖 Prompt for AI Agents
In packages/w3c-vc/src/lib/verify/credentialStatus/index.ts around lines 67 to
76, the code logs to console and throws an error message that only contains a
boolean; instead, remove the console.error call and throw an Error that includes
the combined detailed message (use vcStatusListVerificationResult.error when
present to append error details to the message), e.g. build a single error
string containing the verification state and underlying error and throw new
Error(combinedMessage) so callers receive the full error info and library code
does not perform direct logging.
RishabhS7
approved these changes
Sep 10, 2025
nghaninn
pushed a commit
that referenced
this pull request
Sep 10, 2025
# [1.3.0-alpha.8](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-issuer@1.3.0-alpha.7...@trustvc/w3c-issuer@1.3.0-alpha.8) (2025-09-10) ### Bug Fixes * add derivation support for ecdsa bitstring vc ([#81](#81)) ([5130d91](5130d91))
|
🎉 This PR is included in version 1.3.0-alpha.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
nghaninn
pushed a commit
that referenced
this pull request
Sep 10, 2025
# [1.3.0-alpha.10](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-context@1.3.0-alpha.9...@trustvc/w3c-context@1.3.0-alpha.10) (2025-09-10) ### Bug Fixes * add derivation support for ecdsa bitstring vc ([#81](#81)) ([5130d91](5130d91))
|
🎉 This PR is included in version 1.3.0-alpha.10 🎉 The release is available on: Your semantic-release bot 📦🚀 |
nghaninn
pushed a commit
that referenced
this pull request
Sep 10, 2025
# [1.3.0-alpha.10](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-credential-status@1.3.0-alpha.9...@trustvc/w3c-credential-status@1.3.0-alpha.10) (2025-09-10) ### Bug Fixes * add derivation support for ecdsa bitstring vc ([#81](#81)) ([5130d91](5130d91))
|
🎉 This PR is included in version 1.3.0-alpha.10 🎉 The release is available on: Your semantic-release bot 📦🚀 |
nghaninn
pushed a commit
that referenced
this pull request
Sep 10, 2025
# [1.3.0-alpha.11](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-vc@1.3.0-alpha.10...@trustvc/w3c-vc@1.3.0-alpha.11) (2025-09-10) ### Bug Fixes * add derivation support for ecdsa bitstring vc ([#81](#81)) ([5130d91](5130d91))
|
🎉 This PR is included in version 1.3.0-alpha.11 🎉 The release is available on: Your semantic-release bot 📦🚀 |
nghaninn
pushed a commit
that referenced
this pull request
Sep 10, 2025
# [1.3.0-alpha.11](https://github.com/TrustVC/w3c/compare/@trustvc/w3c-cli@1.3.0-alpha.10...@trustvc/w3c-cli@1.3.0-alpha.11) (2025-09-10) ### Bug Fixes * add derivation support for ecdsa bitstring vc ([#81](#81)) ([5130d91](5130d91))
|
🎉 This PR is included in version 1.3.0-alpha.11 🎉 The release is available on: Your semantic-release bot 📦🚀 |
nghaninn
pushed a commit
that referenced
this pull request
Sep 10, 2025
# [1.3.0-alpha.11](https://github.com/TrustVC/w3c/compare/@trustvc/w3c@1.3.0-alpha.10...@trustvc/w3c@1.3.0-alpha.11) (2025-09-10) ### Bug Fixes * add derivation support for ecdsa bitstring vc ([#81](#81)) ([5130d91](5130d91))
|
🎉 This PR is included in version 1.3.0-alpha.11 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit