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

Did Document Orbit Manifests #83

Merged
merged 29 commits into from Mar 23, 2022
Merged

Did Document Orbit Manifests #83

merged 29 commits into from Mar 23, 2022

Conversation

chunningham
Copy link
Member

@chunningham chunningham commented Feb 18, 2022

This PR simplifies the Orbit Manifest model by rearranging the URI scheme and following the DID model more closely.

  • It reimplements orbit::OrbitMetadata as orbit::Manifest, which is derived directly from a DID Doc via From<Document>.
  • Following the DID model, OrbitMedata::controllers has been split in to Manifest::invokers and Manifest::delegators.
  • Orbit IDs and resource paths have changed from a scheme://path model (e.g. kepler://<orbit-id-CID>/s3/my_image.jpg) to an extensible pseudo-DID-url scheme (e.g. did:ens:alice.eth#orbit-1 -> kepler:ens:alice.eth://orbit-1:s3/my_image.jpg) implemented as resource::OrbitId and resource::ResourceId.
  • Because of the use of DIDs over CIDs, parameters have been removed from Action::Create.
  • The Allowlist, used for orbit creation without a signature, has been updated to return the OrbitId instead of a set of controllers.

The URIs are prefixed with kepler: to disambiguate from actual DIDs, as DID methods can define their own pathing semantics and restrictions.

@chunningham chunningham marked this pull request as ready for review March 14, 2022 14:50
@obstropolos obstropolos requested a review from wyc March 14, 2022 15:35
Copy link
Collaborator

@cobward cobward left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me, just a couple of style questions.

src/resource.rs Outdated Show resolved Hide resolved

tracing_try_init();
let d = base64::encode_config(
r#"["test.org wants you to sign in with your Ethereum account:\n0x6c3Ca9380307EEDa246B7606B43b33F3e0786C79\n\nAuthorize this provider to host your Orbit\n\nURI: peer:12D3KooWSJT2PD5c1rEAD959q9kChGcWUnkkUzku28uY5pqegkuW\nVersion: 1\nChain ID: 1\nNonce: 3A9S4Ar7YibfspTb2\nIssued At: 2022-03-16T15:03:36.775Z\nExpiration Time: 2022-03-16T15:05:36.775Z\nResources:\n- kepler:pkh:eip155:1:0x6c3Ca9380307EEDa246B7606B43b33F3e0786C79://default#peer","0x6909694c1afe49fbe9350da8f89333397657ae46d9898dccdef45a38cf39e8fd1527e81e79b52db3f2ad2239c07cab8888f4882faf453c032c0e4df3d9c4902d1b"]"#,
Copy link
Collaborator

Choose a reason for hiding this comment

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

URI: peer:12D3KooWSJT2PD5c1rEAD959q9kChGcWUnkkUzku28uY5pqegkuW

Just wondering if we want the kepler URL in here too? i.e. https://kepler.spruceid.xyz/peer/<peer-id> or maybe kepler://spruceid.xyz/peer/<peer-id>. Would be more meaningful for the user

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah we probably do, as it must be a URI (hence the peer: bolted on the front). I wanted to use a DID for it, but there isn't afaik a DID method for PeerIDs the same way there is for blockchain accounts (did:pkh) or JWKs (did:key).

Copy link
Collaborator

Choose a reason for hiding this comment

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

When I was looking at the URI spec it wasn't clear to me if the scheme needs to be registered for it to be a valid URI. Do you know?

Copy link
Member Author

Choose a reason for hiding this comment

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

technically, a scheme needs to be registered with IANA for the URI to be considered globally unique (in the sense that, someone writing another app should not use it unintentionally)

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.

None yet

2 participants