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

feat: hierarchical capability organisation #12

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Gozala
Copy link
Contributor

@Gozala Gozala commented Mar 5, 2024

No description provided.

@Gozala Gozala requested a review from vasco-santos March 13, 2024 19:58
@Gozala
Copy link
Contributor Author

Gozala commented Mar 13, 2024

@vasco-santos asking your feedback here as I think it might be worth introducing new capabilities already following proposed conventions

@Gozala Gozala requested a review from alanshaw March 14, 2024 08:48
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

I like a lot this proposal and I am in for it. I would like to see in the RFC a more clear direction path for the migration. I see you mean we should keep both, but my main idea was: "we are probably doing a big breaking change when UCAN 1.0 and invocation spec, that seems a great timing", even though I know some motivation was to ease client refactor.

Maybe a combo of the above could be an option, so flag as deprecate and remove on the timing I mention.

Also, would like to know a bit more info on what would be proposed for folks that already have given delegations for service? we need some kind of refresh of delegations


## Abstract

Currently UCAN capabilities described in [W3Up specification](https://github.com/web3-storage/specs) do not follow consistent organizational structure. To make things worse accounts on login delegate `*` capabilities and spaces delegate set of namespaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I think we must fix this

rfc/hierarchical-capabilities.md Outdated Show resolved Hide resolved
Comment on lines +20 to +23
/space/content/add # upload/add
/space/content/add/shard # store/add
/space/content/add/shard/allocate # store/allocate
/space/content/add/shard/write # S3/R2 PUT
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure about this proposal in specific. I would not consider upload/add to be a "parent" of store/add in a way. Probably I would put it into a different layer, like space/collection and drop shard from here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look at our client our upload function does bunch of store/add's and then does upload/add, that means I can not delegate one ability to perform this task. That is why I think /space/content/add should cover both although perhaps you are right and we need to have something like /space/content/add/root or something instead of making upload/add into /space/content/add.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, something more along the lines of /space/content/add/root, /space/content/add/pack I think would feel better

Copy link
Member

Choose a reason for hiding this comment

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

I think /content is superfluous since we're specifying what is being added/removed/listed/etc.

  • /space/add/root (upload/add)
  • /space/add/shard (store/add)
  • /space/add/shard/allocate (store/allocate)
  • /space/remove/shard (store/remove)
  • /space/list/root (upload/list)
  • /space/list/shard (store/list)

I also wonder if this is the correct order - I was thinking perhaps the verb should be at the end? Should the noun(s) drill down to the "action" (verb) from /? So you can read the capability right to left and it make sense?

  • /space/root/add - add a root to the space
  • /space/shard/add - add a shard to the space
  • /space/shard/allocate - allocate a shard to the space
  • /space/blob/add - add a blob to the space
  • /space/blob/list - list blobs in the space
  • /space/blob/remove - remove a blob from the space

It feels more likely to me that users would want to "wildcard-delegate" based on the thing (noun) not the action (verb)....probably not explaining that well, but I'd be hesitant to delegate /space/add/* - ability to add anything that exists or will exist in the future (narrow action, wide things), whereas /space/blob/* - any action with blobs specifically feels safer, despite widening of granted actions (narrow thing, wide actions).

Also, FWIW I'm not convinced we need or should be able to delegate a single capability to add an upload as well store a shard. That said you can still achieve that using the /noun/noun/verb style by delegating /space/* although you'd be being MUCH less restrictive on actions. Might be nice to have /space/*/add or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also wonder if this is the correct order - I was thinking perhaps the verb should be at the end? Should the noun(s) drill down to the "action" (verb) from /? So you can read the capability right to left and it make sense?

While I do share the sentiment problem is that when user wants to share ability to add content they would have to delegate several capabilities as you need be able to add shards and uploads etc...

Which is also why content/add contained all kinds of writes so user can just delegate white capability, or limit it to specific write capability where needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It feels more likely to me that users would want to "wildcard-delegate" based on the thing (noun) not the action (verb)....probably not explaining that well, but I'd be hesitant to delegate /space/add/* - ability to add anything that exists or will exist in the future (narrow action, wide things), whereas /space/blob/* - any action with blobs specifically feels safer, despite widening of granted actions (narrow thing, wide actions).

Well that is why I had /space/content/ to cover everything content related wheres /space/content/add covered all content writes and if you want to get very specific that /space/content/add/blob/.

So if we replace blob with content you get basically what you're suggesting here. However I wanted content to cover blobs (which are effectively shards) and DAGs consisting of blocks from shards.

Perhaps content is not best term for it ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think content/blob would be important because we want to have caps to add side indexes and so one to the space? While they coul in theory be a content themselves, I think making distinction would be good so that we can treat those more as hidden files in console

Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

(I did not mean to approve, but comment at this point :/)

Copy link
Member

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

I am convinced on the top level hierarchy, not as clearly convinced on the lower level hiearchy, though I guess I don't strongly object.

Consider me non-blocking on this though I encourage others to discuss what I raised if it feels relevant to you.

/space/content/edit/remove # upload/remove
/space/content/edit/remove/shard # store/remove

/space/shard/add # store/add
Copy link
Member

Choose a reason for hiding this comment

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

why would I use these two methods only, as opposed to the ones underneath space/content

asking cause I'm tempted to say just drop content entirely and make /space/add

I do wonder whether doing this kind of lower level hierarchy organization (as opposed to the /space & /accounts, which make 100% sense) is making assumptions about how folks want to delegate capabilities.

Copy link
Member

Choose a reason for hiding this comment

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

like someone wants to just offer /space/contend/add (i.e. upload/add) without delegating /space/content/add/shard -- maybe there is a usecase we just haven't thought of?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the last on was the thinking here, in fact that is more or less our setup. That said I think what we arrived at #12 (comment) might be a better option, because content/add gives you blanket ability and you could have sub namespaces for shards and roots.


This creates a serious challenge for the programs (like w3up client) that attempt to infer set of methods to expose based no delegated capabilities.

We roughly two roles in the sysntem `space` and `account` and their capability set do not overlap, so we can somewhat infer which methods to present in case of `*` ability by subject DID, but that hinges upon the fact that we use `did:key` for space and `did:mailto` for accounts. This assumbtion may be rendered obsolete in the future.
Copy link
Member

Choose a reason for hiding this comment

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

If I understand, we're infering methods based on the did type of the subject? I 100% agree let's not do this.

Gozala added a commit to storacha/specs that referenced this pull request Mar 22, 2024
Based on #114. PR is written with UCAN 1.0 format and assuming
storacha/RFC#12 however in terms of
immediate implementation I suggest that we instead

1. Use `blob/add` instead of `/space/content/add/blob`
2. Use `web3.storage/blob/*` in place of `/service/blob/` 

I suggest above because I think it would make more sense to transition
to storacha/RFC#12 once we have [email protected]
implemented, because I suspect links to tasks vs invocations are going
to be a pain to deal with otherwise. This will give us cleaner break.

In terms of implementing `/service/blob/accept` and specifically how
does client signal that they've completed upload I suggest that we do
whichever is easiest option from following two:

1. Make client sent second `blob/add` invocation after they've done
upload so we can perform a lookup.
2. Add another temp capability with empty output either very specific
like `blob/add/poll` or very general like `invocation/wake { task: CID }
`.

---------

Co-authored-by: Vasco Santos <[email protected]>
Comment on lines +20 to +23
/space/content/add # upload/add
/space/content/add/shard # store/add
/space/content/add/shard/allocate # store/allocate
/space/content/add/shard/write # S3/R2 PUT
Copy link
Member

Choose a reason for hiding this comment

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

I think /content is superfluous since we're specifying what is being added/removed/listed/etc.

  • /space/add/root (upload/add)
  • /space/add/shard (store/add)
  • /space/add/shard/allocate (store/allocate)
  • /space/remove/shard (store/remove)
  • /space/list/root (upload/list)
  • /space/list/shard (store/list)

I also wonder if this is the correct order - I was thinking perhaps the verb should be at the end? Should the noun(s) drill down to the "action" (verb) from /? So you can read the capability right to left and it make sense?

  • /space/root/add - add a root to the space
  • /space/shard/add - add a shard to the space
  • /space/shard/allocate - allocate a shard to the space
  • /space/blob/add - add a blob to the space
  • /space/blob/list - list blobs in the space
  • /space/blob/remove - remove a blob from the space

It feels more likely to me that users would want to "wildcard-delegate" based on the thing (noun) not the action (verb)....probably not explaining that well, but I'd be hesitant to delegate /space/add/* - ability to add anything that exists or will exist in the future (narrow action, wide things), whereas /space/blob/* - any action with blobs specifically feels safer, despite widening of granted actions (narrow thing, wide actions).

Also, FWIW I'm not convinced we need or should be able to delegate a single capability to add an upload as well store a shard. That said you can still achieve that using the /noun/noun/verb style by delegating /space/* although you'd be being MUCH less restrictive on actions. Might be nice to have /space/*/add or something?

# write+delete permission
/space/content/edit/add # upload/add
/space/content/edit/add/shard # store/add
/space/content/edit/add/shard/allocate # store/allocate
Copy link
Member

Choose a reason for hiding this comment

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

This does not makes sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please elaborate what exactly does not ? Are you suggesting that no one would want to give write + delete or that calling it edit is misleading or something else ?

Copy link
Member

Choose a reason for hiding this comment

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

The fact that there are 3 different verbs in 1 capability - edit, add and allocate - I do not understand what being delegated this capability would allow me to do.

@Gozala
Copy link
Contributor Author

Gozala commented Apr 10, 2024

Thinking more about this I think it would be a good idea to try model what user space as KV looks like and then define commands accordingly. I suspect that our commands will very likely start to resemble KV operations hence it would make senes to do that to inform our design.

vasco-santos added a commit to storacha/w3up that referenced this pull request May 30, 2024
Rename capabilities to minimize issues on migration for clients based on
storacha/RFC#12
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.

4 participants