feat: port DID resolvers#17
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Ports DID resolution utilities from go-libstoracha into libforge and updates multiple APIs to align with the latest ucantone types (notably did.DID and cid.Cid).
Changes:
- Add a new
didresolverpackage (map-based, tiered, HTTP-based, and cached resolvers) with tests. - Update UCAN proof-chain + attestation helpers to use
did.DIDandcid.Cidinstead of prior principal/link types. - Update receipt client + blob capability datamodels to use
cid.Cid, and bump Go module dependencies accordingly.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ucan/proof_chain.go | Switch proof-chain APIs from principals/links to did.DID + cid.Cid. |
| ucan/proof_chain_test.go | Update proof-chain tests for did.DID/did.Undef and cid.Cid. |
| ucan/attestations.go | Update attestation lookup APIs to use did.DID for authority and matching. |
| ucan/attestations_test.go | Update attestation tests to use did.DID and cid.Cid. |
| receipt/client.go | Change receipt fetching/polling to accept task IDs as cid.Cid. |
| go.mod | Bump ucantone and add direct deps for DID resolver support. |
| go.sum | Update dependency checksums for the new module versions. |
| didresolver/tieredresolver.go | New tiered DID resolver that tries multiple resolution strategies. |
| didresolver/tieredresolver_test.go | Tests for tier fallthrough, error joining, context propagation. |
| didresolver/mapresolver.go | New static mapping DID resolver. |
| didresolver/mapresolver_test.go | Tests for static mapping resolver. |
| didresolver/httpresolver.go | New HTTP DID:web resolver fetching /.well-known/did.json. |
| didresolver/httpresolver_test.go | Extensive tests for HTTP resolution, error modes, and context formats. |
| didresolver/cacheresolver.go | New caching wrapper for DID resolvers. |
| didresolver/cacheresolver_test.go | Tests for caching behavior, expiry, concurrency, and composition. |
| capabilities/blob/replica/datamodel/transfer.go | Replace ucan.Link fields with cid.Cid for replica transfer models. |
| capabilities/blob/replica/datamodel/allocate.go | Replace ucan.Link fields with cid.Cid for replica allocate models. |
| capabilities/blob/datamodel/replicate.go | Replace ucan.Link field with cid.Cid for replicate arguments. |
| capabilities/blob/datamodel/allocate.go | Replace ucan.Link field with cid.Cid for allocate arguments. |
| capabilities/blob/datamodel/accept.go | Replace ucan.Link field with cid.Cid for accept output model. |
Comments suppressed due to low confidence (1)
ucan/proof_chain.go:28
- This comment says the subject parameter may be nil to indicate powerline, but the function type now uses did.DID and call sites use did.Undef. Update the comment to avoid suggesting nil is a valid value.
// DelegationListerFunc lists delegations for the given audience, command, and
// subject. It differs from [DelegationMatcherFunc] in that it only retrieves
// delegations for the EXACT audience, command and subject.
//
// Note: the subject parameter MAY be nil to indicate powerline.
type DelegationListerFunc func(ctx context.Context, aud did.DID, cmd ucan.Command, sub did.DID) iter.Seq2[ucan.Delegation, error]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+21
| @@ -16,20 +18,20 @@ import ( | |||
| // powerline delegations (with nil subject) and delegations where command is a | |||
| // matching parent of the passed command e.g. if passed command is "/read/file", | |||
| // delegations with command "/read", and "/" may be returned. | |||
| type DelegationMatcherFunc func(ctx context.Context, aud ucan.Principal, cmd ucan.Command, sub ucan.Subject) iter.Seq2[ucan.Delegation, error] | |||
| type DelegationMatcherFunc func(ctx context.Context, aud did.DID, cmd ucan.Command, sub did.DID) iter.Seq2[ucan.Delegation, error] | |||
| } | ||
| return verifier, nil | ||
| } | ||
| return nil, verrs.NewDIDKeyResolutionError(input, fmt.Errorf("not resolvable by any tier: %w", errs)) |
Comment on lines
+23
to
+24
| if out, found := c.cache.Get(input.String()); found { | ||
| return out.(ucan.Verifier), nil |
| }, | ||
| { | ||
| name: "domain too long", | ||
| did: "did:web:" + string(make([]byte, 254)), |
frrist
reviewed
May 14, 2026
Comment on lines
+207
to
+223
| endpoint, ok := r.webKeys[input] | ||
| if !ok { // if not in allowed web keys, try globs | ||
| for _, g := range r.cfg.globs { | ||
| ok = g.Match(strings.TrimPrefix(input.String(), didWebPrefix)) | ||
| if ok { | ||
| u, err := WellKnownEndpointFromDID(input, r.cfg.insecure) | ||
| if err != nil { | ||
| return nil, verrs.NewDIDKeyResolutionError(input, fmt.Errorf("invalid DID: %w", err)) | ||
| } | ||
| endpoint = u | ||
| break | ||
| } | ||
| } | ||
| } | ||
| if !ok { | ||
| return nil, verrs.NewDIDKeyResolutionError(input, fmt.Errorf("resolution via HTTP not permitted")) | ||
| } |
Member
There was a problem hiding this comment.
I like the globs pattern as a policy to allow/deny arbitrary HTTP GET calls to URLs. But I think the introduction of webKeys in the constructor is a mistake (on my part). My preference would be:
- By default the
HTTPResolverwill fetch the did:key for any did:web key it's asked to resolve. - Support an optional policy (the glob pattern you have) that permits a fetch to did:web's matching a specific pattern, rejecting anything outside that pattern.
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
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.
Ports the DID resolvers from
go-libstorachaand deals with the fallout of upgrading to latest ucantone.It has a soft dependency on fil-forge/ucantone#7 in that you'll need to wrap the resolvers with something to make them look like what is required by ucantone right now.