Skip to content

fix: nested ctype loading #931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions packages/credentials/src/V1/KiltCredentialV1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../../../../tests/testUtils/testData.js'
import {
credentialSchema,
fromInput,
validateStructure,
validateSubject,
} from './KiltCredentialV1.js'
Expand All @@ -32,6 +33,45 @@ it('it verifies valid claim against schema', async () => {
await expect(validateSubject(VC, { cTypes: [cType] })).resolves.not.toThrow()
})

it('it verifies valid claim against nested schema', async () => {
const nestedCType = CType.fromProperties('nested', {
prop: {
$ref: cType.$id,
},
})
const nestedVc = fromInput({
cType: nestedCType.$id,
claims: {
prop: {
name: 'Kurt',
},
},
subject: VC.credentialSubject.id,
issuer: VC.issuer,
})

await expect(
validateSubject(nestedVc, {
cTypes: [nestedCType, cType],
loadCTypes: false,
})
).resolves.not.toThrow()

await expect(
validateSubject(nestedVc, {
loadCTypes: CType.newCachingCTypeLoader([nestedCType, cType], () =>
Promise.reject()
),
})
).resolves.not.toThrow()

await expect(
validateSubject(nestedVc, { cTypes: [nestedCType], loadCTypes: false })
).rejects.toThrowErrorMatchingInlineSnapshot(
`"This credential is based on CType kilt:ctype:0xf0fd09f9ed6233b2627d37eb5d6c528345e8945e0b610e70997ed470728b2ebf whose definition has not been passed to the validator, while automatic CType loading has been disabled."`
)
})

it('it detects schema violations', async () => {
const credentialSubject = { ...VC.credentialSubject, name: 5 }
await expect(
Expand Down
51 changes: 35 additions & 16 deletions packages/credentials/src/V1/KiltCredentialV1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import {
jsonLdExpandCredentialSubject,
spiritnetGenesisHash,
} from './common.js'
import { CTypeLoader, newCachingCTypeLoader } from '../ctype/CTypeLoader.js'
import {
type CTypeLoader,
newCachingCTypeLoader,
} from '../ctype/CTypeLoader.js'

export {
credentialIdFromRootHash as idFromRootHash,
Expand Down Expand Up @@ -329,6 +332,13 @@ const cachingCTypeLoader = newCachingCTypeLoader()

/**
* Validates the claims in the VC's `credentialSubject` against a CType definition.
* Supports both nested and non-nested CType validation.
* For non-nested CTypes:
* - Validates claims directly against the CType schema.
* For nested CTypes:
* - Automatically detects nested structure through `$ref` properties.
* - Fetches referenced CTypes via the `loadCTypes` funtion, if not included in `cTypes`.
* - Performs validation against the main CType and all referenced CTypes.
*
* @param credential A {@link KiltCredentialV1} type verifiable credential.
* @param credential.credentialSubject The credentialSubject to be validated.
Expand All @@ -354,26 +364,13 @@ export async function validateSubject(
if (!credentialsCTypeId) {
throw new Error('credential type does not contain a valid CType id')
}
// check that we have access to the right schema
let cType = cTypes?.find(({ $id }) => $id === credentialsCTypeId)
if (!cType) {
if (typeof loadCTypes !== 'function') {
throw new Error(
`The definition for this credential's CType ${credentialsCTypeId} has not been passed to the validator and CType loading has been disabled`
)
}
cType = await loadCTypes(credentialsCTypeId)
if (cType.$id !== credentialsCTypeId) {
throw new Error('failed to load correct CType')
}
}

// normalize credential subject to form expected by CType schema
const expandedClaims: Record<string, unknown> =
jsonLdExpandCredentialSubject(credentialSubject)
delete expandedClaims['@id']

const vocab = `${cType.$id}#`
const vocab = `${credentialsCTypeId}#`
const claims = Object.entries(expandedClaims).reduce((obj, [key, value]) => {
if (!key.startsWith(vocab)) {
throw new Error(
Expand All @@ -385,6 +382,28 @@ export async function validateSubject(
[key.substring(vocab.length)]: value,
}
}, {})

// Turn CType loader & ctypes array into combined loader function
const combinedCTypeLoader = newCachingCTypeLoader(
cTypes,
typeof loadCTypes === 'function'
? loadCTypes
: (id) =>
Promise.reject(
new Error(
`This credential is based on CType ${id} whose definition has not been passed to the validator, while automatic CType loading has been disabled.`
)
)
)

const cType = await combinedCTypeLoader(credentialsCTypeId)

// Load all nested CTypes
const referencedCTypes = await CType.loadNestedCTypeDefinitions(
cType,
combinedCTypeLoader
)

// validates against CType (also validates CType schema itself)
CType.verifyClaimAgainstSchema(claims, cType)
CType.verifyClaimAgainstNestedSchemas(cType, referencedCTypes, claims)
}
75 changes: 70 additions & 5 deletions packages/credentials/src/ctype/CTypeLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
* found in the LICENSE file in the root directory of this source tree.
*/

import { ICType } from '@kiltprotocol/types'
import type { ICType } from '@kiltprotocol/types'
import { SDKErrors } from '@kiltprotocol/utils'

import { fetchFromChain } from './CType.chain.js'
import { isICType, verifyDataStructure } from './CType.js'

export type CTypeLoader = (id: ICType['$id']) => Promise<ICType>

const loadCType: CTypeLoader = async (id) => {
const chainCTypeLoader: CTypeLoader = async (id) => {
return (await fetchFromChain(id)).cType
}

Expand All @@ -20,10 +22,13 @@ const loadCType: CTypeLoader = async (id) => {
* Used in validating the credentialSubject of a {@link KiltCredentialV1} against the Claim Type referenced in its `type` field.
*
* @param initialCTypes An array of CTypes with which the cache is to be initialized.
* @returns A function that takes a CType id and looks up a CType definition in an internal cache, and if not found, tries to fetch it from the KILT blochchain.
* @param cTypeLoader A basic {@link CTypeLoader} to augment with a caching layer.
* Defaults to loading CType definitions from the KILT blockchain.
* @returns A function that takes a CType id and looks up a CType definition in an internal cache, and if not found, tries to fetch it from an external source.
*/
export function newCachingCTypeLoader(
initialCTypes: ICType[] = []
initialCTypes: ICType[] = [],
cTypeLoader = chainCTypeLoader
): CTypeLoader {
const ctypes: Map<string, ICType> = new Map()

Expand All @@ -32,9 +37,69 @@ export function newCachingCTypeLoader(
})

async function getCType(id: ICType['$id']): Promise<ICType> {
Copy link
Member

Choose a reason for hiding this comment

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

Randomly adding more soundness checks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved them from the code for loading nested ctype definitions, as they seemed misplaced there.

Copy link
Member

Choose a reason for hiding this comment

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

Ok fair enough.

const ctype: ICType = ctypes.get(id) ?? (await loadCType(id))
let ctype = ctypes.get(id)
if (ctype) {
return ctype
}
ctype = await cTypeLoader(id)
verifyDataStructure(ctype)
if (id !== ctype.$id) {
throw new SDKErrors.CTypeIdMismatchError(ctype.$id, id)
}
ctypes.set(ctype.$id, ctype)
return ctype
}
return getCType
}

/**
* Recursively traverses a (nested) CType's definition to load definitions of CTypes referenced within.
*
* @param cType A (nested) CType containg references to other CTypes.
* @param cTypeLoader A function with which to load CType definitions.
* @returns An array of CType definitions which were referenced in the original CType or in any of its composite CTypes.
*/
export async function loadNestedCTypeDefinitions(
cType: ICType,
cTypeLoader: CTypeLoader
): Promise<ICType[]> {
const fetchedCTypeIds = new Set<string>()
const fetchedCTypeDefinitions: ICType[] = []

// Don't fetch the original CType
fetchedCTypeIds.add(cType.$id)

async function extractRefsFrom(value: unknown): Promise<void> {
if (typeof value !== 'object' || value === null) {
return
}

if ('$ref' in value) {
const ref = (value as { $ref: unknown }).$ref
if (typeof ref === 'string' && ref.startsWith('kilt:ctype:')) {
const cTypeId = ref.split('#/')[0] as ICType['$id']

if (!fetchedCTypeIds.has(cTypeId)) {
fetchedCTypeIds.add(cTypeId)
const referencedCType = await cTypeLoader(cTypeId)

if (isICType(referencedCType)) {
fetchedCTypeDefinitions.push(referencedCType)
} else {
throw new Error(`Failed to load referenced CType: ${cTypeId}`)
}

await extractRefsFrom(referencedCType.properties)
}
}
return
}

// Process all values in the object. Also works for arrays
await Promise.all(Object.values(value).map(extractRefsFrom))
Copy link
Member

Choose a reason for hiding this comment

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

I had asked chat about this.

Not sure if we care to much about proformance. However, it gave me an interesting insight into this Promise.all and map.

await Promise.all(Object.values(value).map(extractRefsFrom))
  • The issue is that extractRefsFrom is async, but map() does not handle async functions well—it creates a list of Promise<void>, which Promise.all then waits for.
  • While this works in many cases, it may cause performance issues if many deep CType references are being processed simultaneously.
  • Solution: You may consider sequential processing for deeply nested structures using for...of:
for (const subValue of Object.values(value)) {
  await extractRefsFrom(subValue)
}

This prevents too many concurrent fetches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bizarre that chad thinks performance would be better if we process them sequentially. Async code exists exactly for this use case, where it doesn't make much sense to block execution while waiting for the full node to return historic blocks. In case there are issues with making too many requests at the same time, I'd recommend rate limiting on the ctype loader side, but not here.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is strange. There wasn't any issues on my side, but I like to run it through chat afterwards. Seeing what interesting things it comes up with. I sent the test results. It basically a pointless comment without the context of what we are doing in the project.

}

await extractRefsFrom(cType.properties)

return fetchedCTypeDefinitions
}
Loading