Skip to content

feat: ValidateToken takes options, too#28

Open
Peeja wants to merge 2 commits into
mainfrom
petra/feat/validatetoken-takes-options
Open

feat: ValidateToken takes options, too#28
Peeja wants to merge 2 commits into
mainfrom
petra/feat/validatetoken-takes-options

Conversation

@Peeja

@Peeja Peeja commented May 27, 2026

Copy link
Copy Markdown
Contributor

This makes it practical to use on its own, outside of ValidateInvocation (for validating a delegation, for instance).

Required for fil-forge/sprue#15.

PR Dependency Tree

This tree was auto-generated by Charcoal

This makes it practical to use on its own, outside of
`ValidateInvocation` (for validating a delegation, for instance).
@Peeja Peeja requested review from alanshaw and frrist and removed request for alanshaw May 27, 2026 14:50
Comment thread validator/options.go Outdated
cfg := validationConfig{
resolveProof: ProofUnavailable,
resolveDIDVerifier: ResolveDIDKeyVerifier,
validationTime: ucan.UnixTimestamp(time.Now().Unix()),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: replace this with ucan.Now()

Comment thread validator/options.go Outdated
Comment on lines +78 to +84

// withConfig reuses an entire built [validationConfig].
func withConfig(cfg validationConfig) Option {
return func(vc *validationConfig) {
*vc = cfg
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

withConfig is wearing an Option "costume" to do something options aren't supposed to do: replace the whole config rather than tweaking a field. That has a few knock-on costs:

  • Order-dependent in a way the type can't express. withConfig only behaves correctly when it's alone; combined with any real option it either clobbers them or gets clobbered, silently.
  • Two ways to express the same intent (build from options vs. inject a pre-built struct) for one underlying need.

Can we instead split this, something like:

func ValidateToken(ctx context.Context, tok ucan.Token, options ...Option) error {
    return validateToken(ctx, tok, makeCfg(options...))
}

func validateToken(ctx context.Context, tok ucan.Token, cfg validationConfig) error {
    // existing body
}

Then ValidateInvocation and the proof-chain loop can call validateToken(ctx, …, cfg) directly, and withConfig goes away.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah! That's the way to do it. ✨

@Peeja Peeja requested a review from frrist June 1, 2026 16:54
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