Skip to content

feat: resolve did:plc#477

Open
fforbeck wants to merge 2 commits intomainfrom
feat/did-plc
Open

feat: resolve did:plc#477
fforbeck wants to merge 2 commits intomainfrom
feat/did-plc

Conversation

@fforbeck
Copy link
Member

@fforbeck fforbeck commented May 27, 2025

DID PLC Resolver

Overview

Created a DID resolver helper to support both Web and PLC DID resolution. The resolver uses an optional options object and has dedicated resolution functions for each DID method that can be easily extended.

Main Changes

  • Refactored createDidResolver to use optional options object
  • Added separate resolveDIDWeb and resolveDIDPlc functions
  • Added proper parsing of PLC verification methods using multikey format

Usage

const resolver = createDidResolver({
  principalMapping, // optional mapping of Web DIDs to DID keys
  plcClient        // optional PLC client for PLC DID resolution
})

// Can also be called with no options
const resolver = createDidResolver()

Related: storacha/project-tracking#463

@alanshaw
Copy link
Member

@fforbeck do you want to update the dependencies?

@fforbeck
Copy link
Member Author

@fforbeck do you want to update the dependencies?

sure thing @alanshaw - but I need this PR merged and released first: storacha/upload-service#273

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

I left a few suggestions - would be good to get the options jsdoc changes done but everything else non-blocking :)

const bytes = base58btc.decode(publicKeyMultibase) // assumes 'z'-prefix
const [code] = varint.decode(bytes)
switch (code) {
case 0xed: { // Ed25519
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
case 0xed: { // Ed25519
case ed25519.Verifier.code: {



/**
* @typedef {Record<`did:web:${string}`, `did:key:${string}`>} PrincipalMapping
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to use DID<'web'> and DID<'key'> types?

* @param {object} options
* @param {PrincipalMapping} options.principalMapping - Mapping of known WebDIDs to their corresponding DIDKeys
* @param {import('@storacha/did-plc').PlcClient} options.plcClient - PLC client to use for resolving PLC DIDs
* @returns {(did: `did:${string}:${string}`) => Promise<import('@ucanto/interface').Result<`did:key:${string}`[], DIDResolutionError>>}
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to use DID<'web'> and DID<'key'> types?

Comment on lines +20 to +22
* @param {object} options
* @param {PrincipalMapping} options.principalMapping - Mapping of known WebDIDs to their corresponding DIDKeys
* @param {import('@storacha/did-plc').PlcClient} options.plcClient - PLC client to use for resolving PLC DIDs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param {object} options
* @param {PrincipalMapping} options.principalMapping - Mapping of known WebDIDs to their corresponding DIDKeys
* @param {import('@storacha/did-plc').PlcClient} options.plcClient - PLC client to use for resolving PLC DIDs
* @param {object} [options]
* @param {PrincipalMapping} [options.principalMapping] - Mapping of known WebDIDs to their corresponding DIDKeys
* @param {import('@storacha/did-plc').PlcClient} [options.plcClient] - PLC client to use for resolving PLC DIDs

* @param {`did:${string}:${string}`} did
*/
return async (did) => {
if (Schema.did({ method: 'web' }).is(did) && options.principalMapping) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (Schema.did({ method: 'web' }).is(did) && options.principalMapping) {
if (Schema.did({ method: 'web' }).is(did) && options?.principalMapping) {

return resolveDIDWeb(did, options.principalMapping)
}

if (Schema.did({ method: 'plc' }).is(did) && options.plcClient) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (Schema.did({ method: 'plc' }).is(did) && options.plcClient) {
if (Schema.did({ method: 'plc' }).is(did) && options?.plcClient) {

}
}
} catch (err) {
console.error(`unable to parse public key from verification method ${publicKeyMultibase}`, err)
Copy link
Member

Choose a reason for hiding this comment

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

Still just a warning...

Suggested change
console.error(`unable to parse public key from verification method ${publicKeyMultibase}`, err)
console.warn(`unable to parse public key from verification method ${publicKeyMultibase}`, err)

const [code] = varint.decode(bytes)
switch (code) {
case 0xed: { // Ed25519
return ed25519.Verifier.parse(publicKeyMultibase)
Copy link
Member

Choose a reason for hiding this comment

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

You return null either way (if it's unsupported or invalid), I'm not sure the complexity is worth it here just to log the right message. I'd just parse and change the log message in the catch to unsupported or invalid verification method: ${publicKeyMultibase}.

@alanshaw
Copy link
Member

@fforbeck do you want to update the dependencies?

sure thing @alanshaw - but I need this PR merged and released first: storacha/upload-service#273

Hmm, I meant the ucanto dependencies that are referenced as file: here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants