Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.10 #132

Merged
merged 65 commits into from
May 9, 2023
Merged

v0.10 #132

merged 65 commits into from
May 9, 2023

Conversation

expede
Copy link
Member

@expede expede commented Nov 23, 2022

Preview 📜

Closes

Removed/Deferred from Scope

  • Way to connect delegation chains #130
    • By definition, this needs to live outside of the initial delegation
    • Several ways to do this (e.g. look in a DB, in a CAR file, some other format)
    • IMO this is super valuable, but is not in the minimal subset that everyone needs to support
    • As such it probably should have its own spec(s), removing from this PR
  • Should we support non DID principals ? #139
    • Discussed with several people
    • No clear benefit (e.g. entirely posisble to do a did:mailto)
    • Opening to URI is a mostly one-way door
      • i.e. It's much harder to reverse than add later if it's found to be critical

@cla-bot cla-bot bot added the cla-signed label Nov 23, 2022
README.md Outdated Show resolved Hide resolved
Co-authored-by: Irakli Gozalishvili <contact@gozala.io>
Signed-off-by: Brooklyn Zelenka <be.zelenka@gmail.com>
README.md Outdated Show resolved Hide resolved
Gozala and others added 3 commits November 25, 2022 15:57
Signed-off-by: Irakli Gozalishvili <contact@gozala.io>
@oed
Copy link

oed commented Jan 18, 2023

Why is the prf field not a IPLD link? My assumption had been that it is, but looks like it's just a CID string.

Valid DagJSON would be:

"prf": [
  { "/": "bafkreihogico5an3e2xy3fykalfwxxry7itbhfcgq6f47sif6d7w6uk2ze" },
  { "/": "bafkreiemaanh3kxqchhcdx3yckeb3xvmboztptlgtmnu5jp63bvymxtlva" }
]

Maybe there's a reason for this that I'm missing?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dholms dholms left a comment

Choose a reason for hiding this comment

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

This all looks great to me! 🙌

Thanks for the quick back & forth on a couple things!

Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Looks great! This spec has come out really nicely. ✨ Thanks for all of the work pulling our voices together!

Copy link
Member

@icidasset icidasset left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@hugomrdias hugomrdias left a comment

Choose a reason for hiding this comment

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

LGTM left a minor typo suggestion in there.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Hugo Dias <hugomrdias@gmail.com>
Signed-off-by: Brooklyn Zelenka <be.zelenka@gmail.com>
Copy link
Contributor

@Gozala Gozala left a comment

Choose a reason for hiding this comment

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

Looks good to me. I do wish we could resolve some of the ability taxonomy, but perhaps a next revision where it should occur.

]
{
"ucan:bafkreihogico5an3e2xy3fykalfwxxry7itbhfcgq6f47sif6d7w6uk2ze": {"ucan/*": [{}]},
"ucan:*": {"ucan/*": [{}]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need ucan/ prefix on the ability here ? We used to have special * ability to denote "all" abilities which was far more intuitive than ucan/*. If it really need to be name-spaced I would suggest */* as it feels far more intuitive that * and */* would include crud/read than ucan/*.

Copy link
Contributor

Choose a reason for hiding this comment

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

We also use { with: "ucan:*", can: "*" } when linking devices which I think would be considered invalid ? If so it is probably worth calling out what it would imply.

end
end

... --> *
Copy link
Contributor

Choose a reason for hiding this comment

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

I would really like if the hierarchy above was actually part of the spec, meaning that it would be really useful if it was specified that msg/* includes msg/send and * included msg/* and crud/* etc.. Which as I understood from our side channel conversation is not implied

Copy link
Member Author

Choose a reason for hiding this comment

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

At the level of the UCAN spec, we're leaving it by convention for now. Not all namespaces have a .../*, but I do think that we want to start writing related specs for essentially a stdlib of common abilities and resources 💯

@burdiyan
Copy link

burdiyan commented Apr 25, 2023

Love the new structure for attenuation! Feels much better than previous list of with and can. I guess this addresses my comment in the Discussion section.


The DID spec [permits an arbitrary DID method to specify the use of the URI path segment](https://www.w3.org/TR/did-core/#path), the `own` scheme separates out the reference of owned resources rather than a DID document itself. Other URI schemes (such as `dns` and `telnet`) define their own syntactic rules. `own` makes it clear that the contained DID is the "owner" of the resource in the path, fragments, and queries.
`ucan:*` represents all of the UCANs in the current proofs array. If selecting a particular proof (i.e. not the wildcard), then its CID MUST be used. In the case of selecting a particular proof, the validator MUST check that the delegated content address is listed in the proofs (`prf`) field.
Copy link
Member

Choose a reason for hiding this comment

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

@expede Bit confused by this section. This paragraph says:

ucan:* represents all of the UCANs in the current proofs array.

But the table above says:

ucan:* All possible provable UCANs
ucan:./*All in this UCAN's proofs

Does the syntax in the paragraph need to be adjusted to ucan:./*?
Or am I confused about what ucan:* actually means?
I assume it means more than just the current proofs array?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah indeed, we should make this consistent — apologies. It's also something that I don't love about this syntax: it's not very salient despite the meaning being quite different

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

Successfully merging this pull request may close these issues.

None yet

9 participants