feat: delegation chain validation #13
Conversation
add all errors from JS Ucanto
core/policy/match.go
Outdated
| } | ||
|
|
||
| func matchStatement(statement Statement, node ipld.Node) bool { | ||
| switch statement.Kind() { |
There was a problem hiding this comment.
does it not make sense to make this a type switch? https://go.dev/tour/methods/16
asking cause it's a private method, you have access to the underlying types.
also, do you expect others outside the policy module to implement the statement interfaces? If not, I'd just skip them. several are also identical. also if it's a closed union, you can just make a private interface method all the underlying types implement, and since it's private the interface won't be implementable outside the module. Here's golang's own ast parser doing it... https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/go/ast/ast.go;l=851
There was a problem hiding this comment.
basically,
interface with private method and type switch is my go to for tagged unions, which don't really exist in go. (there's some debate about other solutions like visitor pattern, but I find them too verbose)
There was a problem hiding this comment.
FYI all the policy code got moved into go-ucan and I believe they changed it to something similar to what you suggested :)
core/schema/did.go
Outdated
| } | ||
|
|
||
| // DIDString read a string that is in DID format. | ||
| func DIDString() Reader[string, string] { | ||
| return &didstrreader | ||
| return didstrreader |
There was a problem hiding this comment.
ha loved that I convinced you on non pointer types :P
validator/lib.go
Outdated
| // returned that illustrates the valid path. If no valid path is found | ||
| // `Unauthorized` error is returned detailing all explored paths and where they | ||
| // proved to fail. | ||
| func Access[Caveats any](invocation invocation.Invocation, context ValidationContext[Caveats]) (result.Result[Authorization[Caveats], Unauthorized], error) { |
There was a problem hiding this comment.
Tracing these functions all the way back to the service, we we understand well the difference between returning a result.Error and a golang error? I see there is a HandlerExecutionError but I also find this whole distinction a bit cryptic.
I just want to make sure this makes sense. By default I would expect function returning a result type not to return an error. I'm also ok though if it makes sense.
There was a problem hiding this comment.
Ahh, yes at this point I'm not sure we're getting loads of benefit from returning a result type - it was useful when first porting the code but considering Go has typed errors in return values I think I'd be open to just using that as it's more ideomatic, and then wrap up a "Result" for the wire and use them in the receipts.
There was a problem hiding this comment.
honestly this would be my suggestion
| @@ -1,8 +1,12 @@ | |||
| package validator | |||
There was a problem hiding this comment.
Reading this in comparison to ucanto, as I understand it a lot of logic moved to the policy module in core, which I understand to be ucan 1.0 compatible. Is this validator also js-ucanto compatible? IOW has the DSL changed significantly?
There was a problem hiding this comment.
also, should we go ahead and port the delegation logic to js ucanto?
There was a problem hiding this comment.
Yep - there's a PR open for it that did not get finished yet! storacha/ucanto#344
## Supported forms testing This PR provides a set of testcases to verify that the `Selector` behaves properly according to the "Supported Forms" table proposed in a GitHub issue at ucan-wg/delegation#5 (comment). The test cases are divided into three groups: - Tests that should return one or more IPLD `datamodel.Node`s - Tests that should return "null" which is currently replaced by `nil` in Go - Tests that should return an `error` Currently all the `nil` and `error` testcases pass. There are ten `node` testcases that are failing with two having obvious causes: 1. The "Negative Index" test properly `Parse`s the `Selector` text but fails with an `index out of range [-1]` panic when `Select` is called. 2. The "Optional Iterator" test returns the expected values but also a `nil` value for the second element which isn't a `List`. This should probably be pruned during `Select`.
# Goals To feature complete blob index module in indexer-service, we need offset and length for each block when reading from a car, so that we can generate sharded dag index. # Implementation - Adds a small bit of CAR offfset math under the hood so that we can get block offsets and lengths - Adds a tests that verifies these can be used to read exact blocks out of a CAR file - Adds a small utility function to convert one Iterator to another (i.e. make an interator where each element is wrapped)
…a-network/go-ucanto into feat/delegation-chain-validation
use go 1.23 built in iterators which are more memory efficient and easier to work with
# Goals Go 1.23 is out, and it has iterators which are quite close to javascript generators, and largely obviates the need for the iterable module. # Implementation - copy a few iterable utility functions from a proposed golang issue to expand the iterable library with utility functions -- they can go away entirely later (that's why no tests) - No need for From / FromMap any more -- that can be done with slices.Values, and maps.Keys, maps.Values, and maps.All - The syntax is much nicer also :)
add mapped reader for doing additional conversion on schemas
…a-network/go-ucanto into feat/delegation-chain-validation
# Goals More expressive schema languages. Adds Link, URI, and Or readers from js ucanto. Adds a "Mapped" reader for additional transformations (mostly on structs) and an initial read -- the reason here being that particularly in the case of structs, IPLD schemas may not always provide a schema def that is specific enough. # Implementation - add Mapped schema and test - port js ucanto link schema and tests - port js ucanto uri schema and tests - port js ucanto or schema utility
No description provided.