fix: did, schema and cred-def controller#350
Merged
GHkrishna merged 6 commits intofeat/oidc-main-syncfrom Mar 26, 2026
Merged
Conversation
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
|
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 ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughActivated anoncreds schema and credential-definition controllers and routes, added related DTOs and OpenAPI models, refactored DID import/key handling for Bcovrin and Indicio, updated an enum and sample CLI config, and adjusted agent module imports. Changes
Sequence DiagramsequenceDiagram
participant Client
participant SchemaController
participant AnonCredsModule
participant IndyVDR
participant ErrorHandler
Client->>SchemaController: POST /anoncreds/schemas (CreateSchemaInput)
SchemaController->>SchemaController: validate endorsement & payload
SchemaController->>AnonCredsModule: registerSchema(payload)
AnonCredsModule->>IndyVDR: submit registration
IndyVDR-->>AnonCredsModule: return state (Wait/Action/Finished/Failed)
alt State == Failed
AnonCredsModule-->>SchemaController: Failed
SchemaController->>ErrorHandler: map → InternalServerError
else State == Wait
AnonCredsModule-->>SchemaController: Wait
SchemaController->>SchemaController: set HTTP 202
SchemaController-->>Client: return RegisterSchemaReturnStateWait
else State == Finished
AnonCredsModule-->>SchemaController: Finished (includes schemaId)
SchemaController->>SchemaController: normalize Indy schemaId (unqualified)
SchemaController-->>Client: return Finished response
end
Client->>SchemaController: GET /anoncreds/schemas/{schemaId}
SchemaController->>AnonCredsModule: resolveSchema(schemaId)
AnonCredsModule->>IndyVDR: query schema
IndyVDR-->>AnonCredsModule: schema + resolutionMetadata
alt resolutionMetadata.error present
AnonCredsModule-->>SchemaController: error metadata
SchemaController->>ErrorHandler: map → NotFound/BadRequest
else success
AnonCredsModule-->>SchemaController: schema + metadata
SchemaController-->>Client: return SchemaResponseDTO
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
shitrerohit
approved these changes
Mar 25, 2026
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
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: 6
🧹 Nitpick comments (8)
src/cliAgent.ts (1)
4-4: Consolidate duplicate imports from@hyperledger/indy-vdr-nodejs.Lines 4 and 57 both import from the same package—line 4 as a side-effect import and line 57 as a named import. Consolidate into a single top-level named import to avoid duplicate-import warnings and preserve initialization order.
♻️ Proposed fix
import '@openwallet-foundation/askar-nodejs' -import '@hyperledger/indy-vdr-nodejs' +import { indyVdr } from '@hyperledger/indy-vdr-nodejs' import type { AskarModuleConfigStoreOptions } from '@credo-ts/askar' @@ -import { indyVdr } from '@hyperledger/indy-vdr-nodejs'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cliAgent.ts` at line 4, There are duplicate imports of '@hyperledger/indy-vdr-nodejs' (a side-effect import "import '@hyperledger/indy-vdr-nodejs'" and a later named import); remove the redundant side-effect import and consolidate into a single top-level named import that includes the previously used identifiers from the later import (keep the same exported symbol names used elsewhere in cliAgent.ts), ensuring the consolidated import is placed at module top to preserve module initialization order and side effects.src/controllers/anoncreds/schema/SchemaController.ts (4)
31-31: Typo in variable name:schemBySchemaIdshould beschemaBySchemaId.✏️ Suggested fix
- const schemBySchemaId = await request.agent.modules.anoncreds.getSchema(schemaId) + const schemaBySchemaId = await request.agent.modules.anoncreds.getSchema(schemaId)Update subsequent references on lines 34-37, 40, and 44 accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/anoncreds/schema/SchemaController.ts` at line 31, Rename the misspelled local variable schemBySchemaId to schemaBySchemaId in the SchemaController.ts scope where you call request.agent.modules.anoncreds.getSchema(schemaId) and update all subsequent references (e.g., the checks and returns that currently use schemBySchemaId) so they reference schemaBySchemaId instead; ensure any early returns, logging, or property accesses that used schemBySchemaId are updated to the corrected identifier to avoid reference errors.
33-42: Condition logic has inconsistent null checking.The first disjunct
(schemBySchemaId && schemBySchemaId?.resolutionMetadata && ...)explicitly checks for truthiness, while the second disjunct relies solely on optional chaining. This works due to?.returningundefined, but the asymmetry is confusing.Consider simplifying to a consistent pattern:
♻️ Suggested refactor
- if ( - (schemBySchemaId && - schemBySchemaId?.resolutionMetadata && - schemBySchemaId?.resolutionMetadata?.error === SchemaError.NotFound) || - schemBySchemaId?.resolutionMetadata?.error === SchemaError.UnSupportedAnonCredsMethod - ) { + const resolutionError = schemaBySchemaId?.resolutionMetadata?.error + if ( + resolutionError === SchemaError.NotFound || + resolutionError === SchemaError.UnSupportedAnonCredsMethod + ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/anoncreds/schema/SchemaController.ts` around lines 33 - 42, The conditional checking schemBySchemaId/resolutionMetadata is inconsistent and confusing: normalize the guard by consistently checking resolutionMetadata before comparing its error to SchemaError values (use a single pattern like schemBySchemaId?.resolutionMetadata && (schemBySchemaId.resolutionMetadata.error === SchemaError.NotFound || schemBySchemaId.resolutionMetadata.error === SchemaError.UnSupportedAnonCredsMethod)); if true, throw the NotFoundError with the existing message fallback (retain schemaId usage). Update the condition in SchemaController.ts around the schemBySchemaId check so both branches use the same null-safe access pattern to resolutionMetadata and error.
105-120: Inconsistent return shapes between endorsed and non-endorsed schemas.For internally endorsed schemas (line 117), the method returns
schemaStatedirectly, while for externally endorsed schemas (line 119), it returns the fullcreateSchemaTxResult. The TODO acknowledges this, but this inconsistency can confuse API consumers who receive different response structures for the same endpoint.The
anyOfin the OpenAPI spec accommodates this, but consider documenting this behavior clearly or addressing the TODO before release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/anoncreds/schema/SchemaController.ts` around lines 105 - 120, The function returns different shapes depending on endorsement: when schema.endorse is false it returns createSchemaTxResult.schemaState, but when true it returns createSchemaTxResult, causing inconsistent API responses; fix by returning a uniform response (choose one shape) — e.g., always return createSchemaTxResult after mutating createSchemaTxResult.schemaState (use parseIndySchemaId, getUnqualifiedSchemaId to compute and set createSchemaTxResult.schemaState.schemaId) so both branches return the same object, or alternately always return schemaState in both branches; update the branch that currently returns schemaState (schema.endorse === false) to return the chosen unified object.
29-29: Formatting issues flagged by linter.Static analysis indicates parameter formatting should be split across multiple lines per Prettier configuration.
🔧 Suggested formatting fix
- public async getSchemaById(`@Request`() request: Req, `@Path`('schemaId') schemaId: SchemaId): Promise<SchemaResponseDTO> { + public async getSchemaById( + `@Request`() request: Req, + `@Path`('schemaId') schemaId: SchemaId, + ): Promise<SchemaResponseDTO> {- public async createSchema(`@Request`() request: Req, `@Body`() schema: CreateSchemaInput): Promise<RegisterSchemaReturn | RegisterSchemaReturnStateFinished> { + public async createSchema( + `@Request`() request: Req, + `@Body`() schema: CreateSchemaInput, + ): Promise<RegisterSchemaReturn | RegisterSchemaReturnStateFinished> {Also applies to: 61-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/anoncreds/schema/SchemaController.ts` at line 29, The linter flags parameter formatting in the controller: split long parameter lists onto multiple lines for methods like getSchemaById (public async getSchemaById(`@Request`() request: Req, `@Path`('schemaId') schemaId: SchemaId): Promise<SchemaResponseDTO>) and any other flagged methods (e.g., the one at line 61) so each decorator/parameter sits on its own line; update the method signatures to place each parameter/decorator on its own line to satisfy Prettier/linters across SchemaController.ts.src/controllers/did/DidController.ts (3)
203-205: Consider usingundefinedinstead of empty string for clarity.Passing
""as theseedparameter to trigger theprivateKeybranch inimportDidis unintuitive. Usingundefinedwould make the intent clearer and be more consistent with TypeScript conventions for optional parameters.Also, ESLint flagged the double quotes—use single quotes per project style.
Suggested fix
- await this.importDid(agent, didMethod, createDidOptions.did, "", createDidOptions.seed) + await this.importDid(agent, didMethod, createDidOptions.did, undefined, createDidOptions.seed)This requires updating the
importDidsignature to makeseedoptional:- private async importDid(agent: AgentType, didMethod: string, did: string, seed: string, privateKey?: string, keyId?: string) { + private async importDid(agent: AgentType, didMethod: string, did: string, seed?: string, privateKey?: string, keyId?: string) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/did/DidController.ts` around lines 203 - 205, The call to importDid currently passes an empty string ("") to force the privateKey branch which is unclear and violates project linting; change the call in DidController to pass undefined instead of an empty string (await this.importDid(agent, didMethod, createDidOptions.did, undefined, createDidOptions.seed)) and update the importDid function signature to accept seed as an optional parameter (e.g., seed?: string) so callers and type checking are consistent; also replace the double quotes with single quotes per ESLint rules and update any other callers of importDid to handle the optional seed accordingly.
318-341: Remove stale commented-out code.The commented block at lines 322-329 is dead code. The PR objective mentions "remove comments" for naming conventions—this block should also be removed for cleanliness.
Also, add trailing commas at lines 340, 356, and 363 per ESLint.
Suggested cleanup
private async createIndicioKey(agent: AgentType, createDidOptions: DidCreate) { if (!createDidOptions.seed) { throw new BadRequestError('Seed is required') } - // TODO: Remove comments afterwards - // const key = await agent.kms.createKey({ - // privateKey: TypedArrayEncoder.fromString(createDidOptions.seed), - // keyType: KeyAlgorithm.Ed25519, - // }) - - // const buffer = TypedArrayEncoder.fromBase58(key.publicKeyBase58) - // const did = TypedArrayEncoder.toBase58(buffer.slice(0, 16)) const privateJwk = transformSeedToPrivateJwk({ seed: TypedArrayEncoder.fromString(createDidOptions.seed), type: { crv: 'Ed25519', kty: 'OKP', }, }).privateJwk const key = await agent.kms.importKey({ - privateJwk + privateJwk, })
229-229: Same issue: Useundefinedinstead of""for consistency.Apply the same refactor as suggested for line 205.
Suggested fix
- await this.importDid(agent, didMethod, did, "", createDidOptions.seed) + await this.importDid(agent, didMethod, did, undefined, createDidOptions.seed)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/did/DidController.ts` at line 229, The call to this.importDid is passing an empty string literal for the key parameter which should be undefined for consistency; update the invocation in DidController (the await this.importDid(agent, didMethod, did, "", createDidOptions.seed) call) to pass undefined instead of "" so it matches the refactor used earlier and preserves the optional semantics of the importDid function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/cliConfig.json`:
- Line 15: Remove the unused isProduction property from the sample config and
any duplicate entries (lines noted 20 and 25) because AriesRestConfig only
expects indyLedger.{genesisTransactions, indyNamespace} and fetchLedgerData in
src/cliAgent.ts hardcodes isProduction:false; update samples/cliConfig.json to
delete all isProduction fields and ensure no code references rely on it (check
AriesRestConfig, fetchLedgerData, and any config parsing logic) so the sample
matches the actual config shape.
In `@src/controllers/anoncreds/schema/SchemaController.ts`:
- Around line 108-114: createSchemaTxResult.schemaState.schemaId is being
asserted as string and passed to parseIndySchemaId which can receive "undefined"
at runtime; replace the unsafe assertion by first checking that
createSchemaTxResult.schemaState.schemaId is defined (and non-empty) when the
schema state indicates Finished, and if missing either throw a clear error or
return a failure response before calling parseIndySchemaId; update the block
around parseIndySchemaId and getUnqualifiedSchemaId to use the validated value
(or bail out) so parseIndySchemaId and subsequent calls (indySchemaId,
getUnqualifiedSchemaId) never receive an "undefined" string.
In `@src/controllers/did/DidController.ts`:
- Around line 371-399: Refactor the nested ternary inside importDid: invert the
initial condition to handle the simple case when keyId is provided first, and
extract the JWK derivation into a helper (e.g., derivePrivateJwk) that takes
privateKey?: string and seed?: string and returns privateJwk; inside
derivePrivateJwk use explicit if/else: if privateKey -> call
transformPrivateKeyToPrivateJwk, if seed -> call transformSeedToPrivateJwk,
otherwise throw the "Either privateKey or seed is required" error; then call
derivePrivateJwk from importDid and use the returned privateJwk to import the
key and set _keyId (preserve use of agent.kms.importKey and _keyId assignment).
In `@src/controllers/types.ts`:
- Line 2: The file has Prettier formatting violations in the long import line
and the new schema type declarations for AnonCredsDidCommCredentialFormat,
LegacyIndyCredentialFormat and the RegisterSchemaReturnState* types; fix by
reformatting the import to a multi-line named import and reflowing the related
type declarations (RegisterSchemaReturnStateAction,
RegisterSchemaReturnStateFailed, RegisterSchemaReturnStateFinished,
RegisterSchemaReturnStateWait) so they comply with Prettier (wrap long lines,
add trailing commas where appropriate, and keep consistent indentation), then
run Prettier/ESLint to verify the errors are resolved.
In `@src/enums/enum.ts`:
- Around line 47-51: Add the missing trailing comma after the final enum member
to satisfy Prettier/ESLint: update the enum member list that contains
At_Submission, For_Session, On_File, Product_Eula, Service_Agreement by
appending a comma after 'Service_Agreement' so the list ends with , to avoid the
prettier/prettier lint error.
In `@src/routes/swagger.json`:
- Around line 7651-7706: The OpenAPI operationId values "GetSchemaById" and
"CreateSchema" on the paths "/anoncreds/schemas/{schemaId}" and
"/anoncreds/schemas" collide with existing operationIds (causing client
generation issues); rename them to unique identifiers (for example
"AnoncredsGetSchemaById" and "AnoncredsCreateSchema") and update any references
or generated-client expectations that use those operationIds so they match the
new names; modify the operationId fields in the swagger.json entries for
GetSchemaById and CreateSchema accordingly.
---
Nitpick comments:
In `@src/cliAgent.ts`:
- Line 4: There are duplicate imports of '@hyperledger/indy-vdr-nodejs' (a
side-effect import "import '@hyperledger/indy-vdr-nodejs'" and a later named
import); remove the redundant side-effect import and consolidate into a single
top-level named import that includes the previously used identifiers from the
later import (keep the same exported symbol names used elsewhere in
cliAgent.ts), ensuring the consolidated import is placed at module top to
preserve module initialization order and side effects.
In `@src/controllers/anoncreds/schema/SchemaController.ts`:
- Line 31: Rename the misspelled local variable schemBySchemaId to
schemaBySchemaId in the SchemaController.ts scope where you call
request.agent.modules.anoncreds.getSchema(schemaId) and update all subsequent
references (e.g., the checks and returns that currently use schemBySchemaId) so
they reference schemaBySchemaId instead; ensure any early returns, logging, or
property accesses that used schemBySchemaId are updated to the corrected
identifier to avoid reference errors.
- Around line 33-42: The conditional checking schemBySchemaId/resolutionMetadata
is inconsistent and confusing: normalize the guard by consistently checking
resolutionMetadata before comparing its error to SchemaError values (use a
single pattern like schemBySchemaId?.resolutionMetadata &&
(schemBySchemaId.resolutionMetadata.error === SchemaError.NotFound ||
schemBySchemaId.resolutionMetadata.error ===
SchemaError.UnSupportedAnonCredsMethod)); if true, throw the NotFoundError with
the existing message fallback (retain schemaId usage). Update the condition in
SchemaController.ts around the schemBySchemaId check so both branches use the
same null-safe access pattern to resolutionMetadata and error.
- Around line 105-120: The function returns different shapes depending on
endorsement: when schema.endorse is false it returns
createSchemaTxResult.schemaState, but when true it returns createSchemaTxResult,
causing inconsistent API responses; fix by returning a uniform response (choose
one shape) — e.g., always return createSchemaTxResult after mutating
createSchemaTxResult.schemaState (use parseIndySchemaId, getUnqualifiedSchemaId
to compute and set createSchemaTxResult.schemaState.schemaId) so both branches
return the same object, or alternately always return schemaState in both
branches; update the branch that currently returns schemaState (schema.endorse
=== false) to return the chosen unified object.
- Line 29: The linter flags parameter formatting in the controller: split long
parameter lists onto multiple lines for methods like getSchemaById (public async
getSchemaById(`@Request`() request: Req, `@Path`('schemaId') schemaId: SchemaId):
Promise<SchemaResponseDTO>) and any other flagged methods (e.g., the one at line
61) so each decorator/parameter sits on its own line; update the method
signatures to place each parameter/decorator on its own line to satisfy
Prettier/linters across SchemaController.ts.
In `@src/controllers/did/DidController.ts`:
- Around line 203-205: The call to importDid currently passes an empty string
("") to force the privateKey branch which is unclear and violates project
linting; change the call in DidController to pass undefined instead of an empty
string (await this.importDid(agent, didMethod, createDidOptions.did, undefined,
createDidOptions.seed)) and update the importDid function signature to accept
seed as an optional parameter (e.g., seed?: string) so callers and type checking
are consistent; also replace the double quotes with single quotes per ESLint
rules and update any other callers of importDid to handle the optional seed
accordingly.
- Line 229: The call to this.importDid is passing an empty string literal for
the key parameter which should be undefined for consistency; update the
invocation in DidController (the await this.importDid(agent, didMethod, did, "",
createDidOptions.seed) call) to pass undefined instead of "" so it matches the
refactor used earlier and preserves the optional semantics of the importDid
function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4853dfae-0882-4d05-8990-22b779d4d4b8
📒 Files selected for processing (8)
samples/cliConfig.jsonsrc/cliAgent.tssrc/controllers/anoncreds/schema/SchemaController.tssrc/controllers/did/DidController.tssrc/controllers/types.tssrc/enums/enum.tssrc/routes/routes.tssrc/routes/swagger.json
src/controllers/did/DidController.ts
Outdated
Comment on lines
+371
to
+399
| private async importDid(agent: AgentType, didMethod: string, did: string, seed: string, privateKey?: string, keyId?: string) { | ||
| let _keyId: string; | ||
|
|
||
| const { privateJwk } = transformSeedToPrivateJwk({ | ||
| type: { | ||
| crv: 'Ed25519', | ||
| kty: 'OKP', | ||
| }, | ||
| seed: TypedArrayEncoder.fromString(seed), | ||
| }) | ||
| if (!keyId) { | ||
| const { privateJwk } = privateKey ? transformPrivateKeyToPrivateJwk({ | ||
| type: { | ||
| crv: 'Ed25519', | ||
| kty: 'OKP', | ||
| }, | ||
| privateKey: TypedArrayEncoder.fromString(privateKey), | ||
| }) : seed ? transformSeedToPrivateJwk({ | ||
| seed: TypedArrayEncoder.fromString(seed), | ||
| type: { | ||
| crv: 'Ed25519', | ||
| kty: 'OKP', | ||
| } | ||
| }) : { | ||
| privateJwk: undefined | ||
| } | ||
|
|
||
| if (!privateJwk) { | ||
| throw new Error("Either privateKey or seed is required") | ||
| } | ||
|
|
||
| const key = await agent.kms.importKey({ privateJwk }) | ||
| _keyId = key.keyId | ||
| } else { | ||
| _keyId = keyId | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor nested ternary for readability.
The nested ternary at lines 375-389 is flagged by SonarCloud and is difficult to follow. Extract it into a helper or use explicit if-else blocks.
Also consider inverting the condition at line 374 to handle the simpler case (keyId provided) first, avoiding the negation.
Suggested refactor
- private async importDid(agent: AgentType, didMethod: string, did: string, seed: string, privateKey?: string, keyId?: string) {
- let _keyId: string;
-
- if (!keyId) {
- const { privateJwk } = privateKey ? transformPrivateKeyToPrivateJwk({
- type: {
- crv: 'Ed25519',
- kty: 'OKP',
- },
- privateKey: TypedArrayEncoder.fromString(privateKey),
- }) : seed ? transformSeedToPrivateJwk({
- seed: TypedArrayEncoder.fromString(seed),
- type: {
- crv: 'Ed25519',
- kty: 'OKP',
- }
- }) : {
- privateJwk: undefined
- }
-
- if (!privateJwk) {
- throw new Error("Either privateKey or seed is required")
- }
-
- const key = await agent.kms.importKey({ privateJwk })
- _keyId = key.keyId
- } else {
- _keyId = keyId
- }
+ private async importDid(
+ agent: AgentType,
+ didMethod: string,
+ did: string,
+ seed?: string,
+ privateKey?: string,
+ keyId?: string,
+ ) {
+ let _keyId: string
+
+ if (keyId) {
+ _keyId = keyId
+ } else {
+ const privateJwk = this.derivePrivateJwk(privateKey, seed)
+ const key = await agent.kms.importKey({ privateJwk })
+ _keyId = key.keyId
+ }Add a helper method:
private derivePrivateJwk(privateKey?: string, seed?: string) {
const ed25519Type = { crv: 'Ed25519' as const, kty: 'OKP' as const }
if (privateKey) {
return transformPrivateKeyToPrivateJwk({
type: ed25519Type,
privateKey: TypedArrayEncoder.fromString(privateKey),
}).privateJwk
}
if (seed) {
return transformSeedToPrivateJwk({
seed: TypedArrayEncoder.fromString(seed),
type: ed25519Type,
}).privateJwk
}
throw new Error('Either privateKey or seed is required')
}🧰 Tools
🪛 ESLint
[error] 371-371: Replace agent:·AgentType,·didMethod:·string,·did:·string,·seed:·string,·privateKey?:·string,·keyId?:·string with ⏎····agent:·AgentType,⏎····didMethod:·string,⏎····did:·string,⏎····seed:·string,⏎····privateKey?:·string,⏎····keyId?:·string,⏎··
(prettier/prettier)
[error] 372-372: Delete ;
(prettier/prettier)
[error] 375-375: Insert ⏎·······
(prettier/prettier)
[error] 376-376: Insert ····
(prettier/prettier)
[error] 377-377: Insert ····
(prettier/prettier)
[error] 378-378: Replace ·········· with ··············
(prettier/prettier)
[error] 379-379: Insert ····
(prettier/prettier)
[error] 380-380: Insert ····
(prettier/prettier)
[error] 381-381: Replace })·:·seed with ····})⏎········:·seed⏎·········
(prettier/prettier)
[error] 382-382: Insert ······
(prettier/prettier)
[error] 383-383: Replace ········ with ··············
(prettier/prettier)
[error] 384-384: Replace ·········· with ················
(prettier/prettier)
[error] 385-385: Insert ······
(prettier/prettier)
[error] 386-386: Replace ········} with ··············},
(prettier/prettier)
[error] 387-387: Replace ······}) with ············})⏎·········
(prettier/prettier)
[error] 388-388: Replace privateJwk:·undefined with ······privateJwk:·undefined,
(prettier/prettier)
[error] 389-389: Replace ······ with ············
(prettier/prettier)
[error] 392-392: Replace "Either·privateKey·or·seed·is·required" with 'Either·privateKey·or·seed·is·required'
(prettier/prettier)
[error] 394-394: Delete ··
(prettier/prettier)
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 374-374: Unexpected negated condition.
[warning] 381-389: Extract this nested ternary operation into an independent statement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/controllers/did/DidController.ts` around lines 371 - 399, Refactor the
nested ternary inside importDid: invert the initial condition to handle the
simple case when keyId is provided first, and extract the JWK derivation into a
helper (e.g., derivePrivateJwk) that takes privateKey?: string and seed?: string
and returns privateJwk; inside derivePrivateJwk use explicit if/else: if
privateKey -> call transformPrivateKeyToPrivateJwk, if seed -> call
transformSeedToPrivateJwk, otherwise throw the "Either privateKey or seed is
required" error; then call derivePrivateJwk from importDid and use the returned
privateJwk to import the key and set _keyId (preserve use of agent.kms.importKey
and _keyId assignment).
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
GHkrishna
changed the title
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
3 tasks
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/controllers/anoncreds/cred-def/CredentialDefinitionController.ts`:
- Around line 115-117: The failure branch for registerCredentialDefinitionResult
incorrectly throws a generic InternalServerError; update the block that checks
registerCredentialDefinitionResult.credentialDefinitionState.state ===
CredentialEnum.Failed to include the SDK-provided failure detail by reading
registerCredentialDefinitionResult.credentialDefinitionState.reason (or a
fallback like a short generic message if reason is missing) and include that
text in the InternalServerError message so callers/logs contain the actual
ledger/validation reason.
- Around line 129-140: The internal registration branch currently returns only
registerCredentialDefinitionResult.credentialDefinitionState, dropping jobId,
credentialDefinitionMetadata, and registrationMetadata and causing the response
shape to vary by endorse; instead, after you compute and replace
registerCredentialDefinitionResult.credentialDefinitionState (using
parseIndyCredentialDefinitionId and getUnqualifiedCredentialDefinitionId),
return the full registerCredentialDefinitionResult object so the envelope
(jobId/credentialDefinitionMetadata/registrationMetadata) is preserved; update
the controller/type signature back to a single
RegisterCredentialDefinitionReturn if needed and regenerate TSOA artifacts to
keep API contracts consistent.
In `@src/controllers/did/DidController.ts`:
- Around line 275-279: The code is currently importing the KMS key inside
createIndicioKey() before calling Indicio (axios.post), which can leave an
orphaned KMS key if remote registration fails; change flow so createIndicioKey
returns the generated key metadata (and keyId) without importing into the local
KMS (or add a flag to skip import), call axios.post(INDICIO_NYM_URL, key) and
only when that call returns success (res.data.statusCode === 200) call
importDid(agent, didMethod, key.did, createDidOptions.seed, undefined, keyId) to
import the key into the KMS; alternatively implement idempotent recovery in
importDid and createIndicioKey to clean up or retry imports on failure, but the
simplest fix is deferring the KMS import from createIndicioKey() until after the
successful axios.post.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 673c9b06-6723-4395-ac3d-8941ec147d08
📒 Files selected for processing (8)
samples/cliConfig.jsonsrc/controllers/anoncreds/cred-def/CredentialDefinitionController.tssrc/controllers/anoncreds/schema/SchemaController.tssrc/controllers/did/DidController.tssrc/controllers/types.tssrc/enums/enum.tssrc/routes/routes.tssrc/routes/swagger.json
✅ Files skipped from review due to trivial changes (1)
- samples/cliConfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/controllers/anoncreds/schema/SchemaController.ts
Comment on lines
+275
to
+279
| const { keyId, ...key } = await this.createIndicioKey(agent, createDidOptions) | ||
| const INDICIO_NYM_URL = process.env.INDICIO_NYM_URL as string | ||
| const res = await axios.post(INDICIO_NYM_URL, key) | ||
| if (res.data.statusCode === 200) { | ||
| await this.importDid(agent, didMethod, key.did, createDidOptions.seed) | ||
| await this.importDid(agent, didMethod, key.did, createDidOptions.seed, undefined, keyId) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the order of local and remote writes in the Indicio flow.
rg -n -C2 'createIndicioKey|kms\.importKey|INDICIO_NYM_URL|axios\.post|dids\.import' --type tsRepository: credebl/agent-controller
Length of output: 7328
🏁 Script executed:
sed -n '270,290p' src/controllers/did/DidController.tsRepository: credebl/agent-controller
Length of output: 879
🏁 Script executed:
sed -n '315,365p' src/controllers/did/DidController.tsRepository: credebl/agent-controller
Length of output: 1675
🏁 Script executed:
sed -n '390,425p' src/controllers/did/DidController.tsRepository: credebl/agent-controller
Length of output: 910
🏁 Script executed:
sed -n '265,295p' src/controllers/did/DidController.tsRepository: credebl/agent-controller
Length of output: 1192
🏁 Script executed:
# Check for try-catch or error handling in the broader context
sed -n '240,305p' src/controllers/did/DidController.ts | grep -E 'try|catch|throw|error|Error'Repository: credebl/agent-controller
Length of output: 422
Defer KMS key import until after Indicio registration succeeds.
The KMS key is imported at line 339 (inside createIndicioKey()) before the Indicio registration call at line 277. If the registration fails, the KMS key remains orphaned. If the subsequent local import at line 279 fails, the ledger has the DID but the local wallet doesn't. Reorder to import the KMS key only after confirming successful remote registration, or implement idempotent recovery for both the KMS and ledger sides on failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/controllers/did/DidController.ts` around lines 275 - 279, The code is
currently importing the KMS key inside createIndicioKey() before calling Indicio
(axios.post), which can leave an orphaned KMS key if remote registration fails;
change flow so createIndicioKey returns the generated key metadata (and keyId)
without importing into the local KMS (or add a flag to skip import), call
axios.post(INDICIO_NYM_URL, key) and only when that call returns success
(res.data.statusCode === 200) call importDid(agent, didMethod, key.did,
createDidOptions.seed, undefined, keyId) to import the key into the KMS;
alternatively implement idempotent recovery in importDid and createIndicioKey to
clean up or retry imports on failure, but the simplest fix is deferring the KMS
import from createIndicioKey() until after the successful axios.post.
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
|
RinkalBhojani
pushed a commit
that referenced
this pull request
Mar 28, 2026
* fix: did and schema controller Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: naming convention and remove comments Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: formatting Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: remove unused param Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: credential definition controller Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: error when cred-def failure Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> --------- Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
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
2 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.
What
Summary by CodeRabbit
New Features
Improvements
API / Docs
Configuration