feat: Improved/simplified validator code by Peeja · Pull Request #7 · fil-forge/ucantone

Peeja · 2026-05-11T22:07:38Z

This aligns the validator code closer to UCAN 1.0 and removes some of (I think) the vestiges of the old go-ucanto. The main work is in validator.Validate, which replaces validator.Access. It now takes dramatically fewer arguments and options:

❌ No longer needs:
- authority: Handled by DID resolution. (Also, stops giving the authority unproven access to do and delegate anything to any subject, which is off-spec and not required for Forge.)
- capability: Not relevant to validation; all invocations are validated the same way.
- canIssue: Defined by UCAN itself, not configurable.
- parsePrincipal: Folded into DID resolution.
- validateAuthorization: Was never actually called (🙈).
✅ Retains:
- validationTime: Time is time is time.
- verifyNonStandardSignature: Attestation is still a thing.
🔧 Modifies:
- resolveDIDKey: becomes resolveDIDVerifier, which resolves a DID to a verifier. This reflects the actual DID operation: resolving a DID to a document and finding verification methods. No longer uses intermediate fictitious did:key DIDs to represent keys that aren't actually DIDs. Returns a single Verifier, even though a DID may have multiple verification methods; we can construct a Verifier which composes multiple Verifiers if needed.

This change introduces a second thing called "capability" which is actually what the UCAN spec calls a "capability" rather than what go-ucanto and the original TS version called a "capability". We can replace "capability" with "command" now, but that's significant enough to warrant its own PR.

alanshaw

👏 I really like this! There's some blocking comments, but this makes a lot more sense now - thank you.

alanshaw · 2026-05-12T11:11:04Z

+	case ed25519.Code:
+		return ed25519.Decode(bytes)
+	case secp256k1.Code:
+		return secp256k1.Decode(bytes)


This forces consumers to bundle code for both ed25519 and secp256k1 even if they are not using both.

I would either move this to a separate package or create a registry pattern (similar to varsig) where ed25519 is supported by default but you can add secp256k1 support if you need it.

Oh, yeah, great call.

alanshaw · 2026-05-12T11:12:41Z

+		Header:                h,
+		TokenPayload1_0_0_rc1: tokenPayload,
+	}, nil
+}


You might want to rebase against #8 as this exposes this method and it's now not nearly as complicated as it needs to be here.

alanshaw · 2026-05-12T11:21:31Z


-// NopValidateAuthorization is a [ValidateAuthorizationFunc] that does no
-// validation and returns nil.
-func NopValidateAuthorization(ctx context.Context, auth Authorization) error {


Why does this get removed? Was previously intended to be for checking revocations.

Only because it was never called. Totally reasonable to have, we just have to actually use it. Although I might suggest we make it explicitly about revocation, since "validation" is what the whole thing is about.

Totally fine to rename - I don't know why it was named so generically but we will need this at some point. I think it's worth leaving it in.

Since it's not part of the signature, just an option, can we leave it out until we actually use it? We'll know how to write it properly then, and until then it's just a lie.

until then it's just a lie.

To clarify, UCAN revocation was implemented and used in the JS upload service and client. I think it's a bit misleading to call it a lie. It has not been implemented in Sprue or Guppy yet, but support here is a prerequisite.

Please at least open an issue to re-add support.

When I say it's a lie, what I mean is that having it in the code implies that it does something, but it doesn't. I don't want Ucantone to promise its consumer it'll do something, especially when it comes to security, and then not do it. It's absolutely a requirement, but I don't want to act like we've already implemented it before we do.

Oh: I think I may have been unclear. I'm not saying that no consumers used it. I'm saying it literally does nothing. That function was never called, even if you provided one.

alanshaw · 2026-05-12T11:29:37Z

 // invocation against proof policies.
-func New[A Arguments](cmd ucan.Command, options ...capability.Option) (*Capability[A], error) {
-	cap, err := capability.New(cmd, options...)
+func New[A Arguments](cmd ucan.Command) (*Capability[A], error) {


Can this be removed now?

Yep, just didn't want to throw it in the same PR.

alanshaw · 2026-05-12T11:29:46Z

Can this be removed now?

frrist

One small drive-by comment. As Alan mentioned, after rebasing on main much of the seralization code should be a bit simpler. You'll also find the changes you've made to the did type have already landed in main: Method(), Identifier(), Defined(), and an Undef type.

frrist · 2026-05-14T16:06:11Z

+	if !strings.HasPrefix(str, did.KeyPrefix) {
+		return nil, fmt.Errorf("must start with '%s'", did.KeyPrefix)
+	}
+	code, bytes, err := multibase.Decode(str[len(did.KeyPrefix):])
+	if err != nil {
+		return nil, err
+	}
+	if code != multibase.Base58BTC {
+		return nil, errors.New("not Base58BTC encoded")
+	}
+
+	keyTypeCode, _, err := varint.FromUvarint(bytes)
+	if err != nil {
+		return nil, fmt.Errorf("reading uvarint: %w", err)
+	}


Could we instead call did.Parse on the input string, this keeps all did parsing code in one place, then make assertions on it using the did.Method() and did.Identifier() methods?

Ports the DID resolvers from `go-libstoracha` and 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.

Signature verification is the same operation for every token, parameterized by token type's Signature Payload definition.

Also, add a few more missing tests

Previously only the `isOrdered` operations handled a `int`/`int64` mismatch. `OpEqual` needs to handle non-numeric things also, so it took a different code path which missed the normalization. Now the normalization happens outside of `isOrdered` to cover that.

Consumers don't have to pull in all encryption packages.

Peeja · 2026-05-18T02:49:05Z

Woof, that was quite a rebase, but came out way better. #8 is awesome, @frrist!

I added a bunch of error types, and I'm not sure if that's the right way to go, or overdoing it. But that's easy to tweak, at least; the actual meat of the PR should be ready to go now.

alanshaw

This is great, such a big improvement. I'm happy for this to merge provided all the feedback can be addressed in some form.

Please could you also quickly update the server documentation on the README - https://github.com/fil-forge/ucantone#server ? 🙏

alanshaw · 2026-05-18T08:33:49Z

@@ -73,13 +68,11 @@ func (d *Dispatcher) Execute(req execution.Request) (execution.Response, error)
 	opts := []validator.Option{validator.WithMetadata(req.Metadata())}


Just a note here - if our server is using a did:web there's an implicit requirement that we pass in a DID document resolver or map resolver that includes the server DID (preferably the latter since we actually know the did:key) so that it can resolve any delegations that it has issued. It would be nice if that was taken care of automatically or at least explicitly documented - it's a bit of a footgun.

Yeah, that makes sense. What do you mean by "automatically", though?

Previously we passed authority that would have allowed us to do that...automatically.

alanshaw · 2026-05-18T08:39:20Z

-	}
-}
-
-func WithAuthorizationValidator(validateAuthorization ValidateAuthorizationFunc) Option {


If you're going to remove it then please create an issue to add support for revocation checking during validation.

alanshaw · 2026-05-18T08:43:54Z

Rename tp command_definition_test.go?

Good catch!

alanshaw · 2026-05-18T08:48:13Z


 // SigPayload returns the decoded signature payload (varsig header + token payload).
-func (d *Delegation) SigPayload() *ddm.SigPayloadModel {
+func (d *Delegation) SigPayload() cbg.CBORMarshaler {


Why does this change? It's not part of any interface IIRC.

Oh, good call. I had added it to an interface and then realized it wasn't actually useful now that we have SignedBytes() instead, and never switched this back.

alanshaw · 2026-05-18T08:50:06Z


 // SigPayload returns the decoded signature payload (varsig header + token payload).
-func (inv *Invocation) SigPayload() *idm.SigPayloadModel {
+func (inv *Invocation) SigPayload() cbg.CBORMarshaler {


Why does this change?

alanshaw · 2026-05-18T09:00:20Z

+
+// ResolveDIDKeyVerifier is a [DIDVerifierResolverFunc] that only supports `did:key`
+// DIDs and returns an error for any other DID method.
+func ResolveDIDKeyVerifier(ctx context.Context, d did.DID) (ucan.Verifier, error) {


This is table stakes. There should at least be some warning that when passing a custom verifier via WithVerifierResolver that it MUST support this baseline resolution. Or perhaps the validator should automatically do this?

I want to avoid baking did:key resolution into the validator as anything more than a default. did:keys are extremely common, but they're not actually required for UCAN at all. The entire spec works if that method doesn't exist in the universe. But accidentally dropping support for them would almost certainly cause your validator to be useless in practice, so it's certainly a footgun.

I've made it easier to compose resolvers in 6101d63 and provided a convenience option to tighten that up even more in practice. How's that look?

alanshaw · 2026-05-18T09:15:42Z


-// NopValidateAuthorization is a [ValidateAuthorizationFunc] that does no
-// validation and returns nil.
-func NopValidateAuthorization(ctx context.Context, auth Authorization) error {


until then it's just a lie.

To clarify, UCAN revocation was implemented and used in the JS upload service and client. I think it's a bit misleading to call it a lie. It has not been implemented in Sprue or Guppy yet, but support here is a prerequisite.

Please at least open an issue to re-add support.

alanshaw · 2026-05-18T09:21:35Z

 }

-func TestFixtures(t *testing.T) {
-	fixturesFile, err := os.Open("./internal/fixtures/invocations.json")


Can we please continue to validate against the fixtures? That way if new fixtures are added to the list we can bring them in here and easily test against the new cases.

Oof, yes! This was a bad merge when I moved the new implementation in to replace the old one. It should absolutely still have the fixture tests.

alanshaw · 2026-05-18T09:24:35Z

-}
-
-func WithAuthorizationValidator(validateAuthorization ValidateAuthorizationFunc) Option {
+func WithVerifierResolver(resolveDIDKey DIDVerifierResolverFunc) Option {


I would include "DID" in this option name, also the parameter technically doesn't have to resolve to DID key, it just has to be able to produce a verifier. Attestations for example 😉

Suggested change

func WithVerifierResolver(resolveDIDKey DIDVerifierResolverFunc) Option {

func WithDIDVerifierResolver(resolveDID DIDVerifierResolverFunc) Option {

These were genericised for an interface change that never came to pass.

Also removes `verifier.Parse(string)`, which was only ever called after stringifying an already-parsed DID, in favor of `verifier.FromDIDKey(did.DID)`.

As a consequence, realigns errors with expected errors from fixtures.

Peeja · 2026-05-18T19:10:25Z

@alanshaw Addressed and merged! I'd love your eyes on that last round of changes, just in case.

These were removed in #7 but are used in `libforge`, `indexing-service` and `piri`. The parse functions per key type allow you to support multiple keys but restrict parsing to a specific one. They also return the verifier type for the specific key (not the interface), allowing users to access type specific methods (in the future).

Peeja requested a review from alanshaw May 11, 2026 22:07

alanshaw requested changes May 12, 2026

View reviewed changes

frrist reviewed May 14, 2026

View reviewed changes

alanshaw mentioned this pull request May 14, 2026

feat: port DID resolvers fil-forge/libforge#17

Merged

Peeja added 9 commits May 17, 2026 13:40

feat!: Capabilities no longer have base policies

188a1ea

refactor: Any Token can be sig-verified

6920a79

Signature verification is the same operation for every token, parameterized by token type's Signature Payload definition.

feat: validator2 validates invocations

e410296

feat: validator2 supports non-standard signatures

a958e28

feat: validator2 correctly disallows root powerline proofs

1d8ca32

Also, add a few more missing tests

feat: Replace validator with new implementation

f30e83a

refactor: "Attenuate" is a better verb than "constrain"

9e53a47

feat: Fully replace previous "capability" with "command"

100fd6e

Peeja force-pushed the peeja/breaking/adjust-validator branch from 1544b5e to a2cc064 Compare May 18, 2026 02:19

Peeja added 3 commits May 17, 2026 22:33

feat: Better errors

9385a58

feat: Verifier code uses a registry

e55e185

Consumers don't have to pull in all encryption packages.

refactor: Use did.Parse() in verifier.Parse()

56144ff

Peeja force-pushed the peeja/breaking/adjust-validator branch from a2cc064 to 56144ff Compare May 18, 2026 02:46

Peeja requested review from alanshaw and frrist May 18, 2026 02:47

alanshaw approved these changes May 18, 2026

View reviewed changes

Peeja added 6 commits May 18, 2026 12:39

refactor: capability_definition_test → command_definition_test

c1df18d

refactor: Restore concrete returns of SigPayload()

141e63b

These were genericised for an interface change that never came to pass.

feat: Easier to compose verifier resolvers by method

6101d63

Also removes `verifier.Parse(string)`, which was only ever called after stringifying an already-parsed DID, in favor of `verifier.FromDIDKey(did.DID)`.

refactor: Run invocation fixtures again in tests

d46a6e9

As a consequence, realigns errors with expected errors from fixtures.

refactor: Clearer name: WithDIDVerifierResolver()

6ddcf07

docs: Update README with capability → command

a729b12

Peeja merged commit 2076d20 into main May 18, 2026
1 check passed

alanshaw mentioned this pull request May 19, 2026

fix: re-add verifier parse functions #14

Merged

		@@ -73,13 +68,11 @@ func (d *Dispatcher) Execute(req execution.Request) (execution.Response, error)
		opts := []validator.Option{validator.WithMetadata(req.Metadata())}

	func WithVerifierResolver(resolveDIDKey DIDVerifierResolverFunc) Option {
	func WithDIDVerifierResolver(resolveDID DIDVerifierResolverFunc) Option {

Conversation

Peeja commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alanshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frrist left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Peeja commented May 18, 2026

Uh oh!

alanshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanshaw May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Peeja commented May 18, 2026

Uh oh!

Peeja commented May 11, 2026 •

edited

Loading

alanshaw May 18, 2026 •

edited

Loading