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

First draft #2

Merged
merged 14 commits into from
Feb 21, 2023
Merged

First draft #2

merged 14 commits into from
Feb 21, 2023

Conversation

oed
Copy link
Collaborator

@oed oed commented Nov 28, 2022

No description provided.

@oed oed requested a review from bumblefudge November 28, 2022 20:04
@oed
Copy link
Collaborator Author

oed commented Nov 28, 2022

@expede @Gozala tagging you for a review as well!

@expede
Copy link
Contributor

expede commented Dec 1, 2022

Stashing this here:

📚 Preview of latest

methods/did:.md Show resolved Hide resolved
methods/did:.md Outdated Show resolved Hide resolved
@expede
Copy link
Contributor

expede commented Dec 2, 2022

Looking awesome overall! Just a question about string encoding, but otherwise this looks great 😁

README.md Outdated Show resolved Hide resolved
@oed
Copy link
Collaborator Author

oed commented Dec 2, 2022

Note to self, todo:

  • Use ascii for URL component
  • Add editors and authors (like in varsig)

@cla-bot
Copy link

cla-bot bot commented Dec 5, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Joel Thorstensson.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Dec 5, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Joel Thorstensson.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Dec 5, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Joel Thorstensson.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot added the cla-signed label Dec 5, 2022
@oed
Copy link
Collaborator Author

oed commented Dec 5, 2022

Thoughts on maybe grabbing 0x0d instead of 0x0d1d. Seems likely that DIDs will get widely used and would warrant a low number?

@Gozala
Copy link
Contributor

Gozala commented Dec 5, 2022

Thoughts on maybe grabbing 0x0d instead of 0x0d1d. Seems likely that DIDs will get widely used and would warrant a low number?

Sounds good to me, mind submitting PR to https://github.com/multiformats/multicodec/ with proposed code, I suspect reviewers may have more informed feedback there. We have never registered 0x0d1d either so it's a good time to reconsider code

@oed oed requested review from expede and bumblefudge and removed request for expede December 12, 2022 14:25
@oed
Copy link
Collaborator Author

oed commented Dec 12, 2022

Updated all examples, should be ready for a final review. Based on the conversation in the multicodec repo it looks like we will use varint encoded 0x0d1d.

@oed oed requested a review from expede December 12, 2022 14:27

* `multidid-code` - the value `0x0d1d` encoded as a varint
* `method-code` - a varint encoded multicode for the [DID Method](https://www.w3.org/TR/did-core/#a-simple-example)
* `method-bytes` - data representing the method-id
Copy link
Contributor

Choose a reason for hiding this comment

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

I have not got through the rest of the document, but I do find it bit confusing that there is both method code and method bytes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The method code identifies which DID method it is. The method bytes is the rest of the data for the method specific identifier. Lmk if you think this can be clarified!

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh it’s actual id, was just confused by the name (perhaps it should be called whatever did:core calls those instead)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also unless it’s length prefixed it would be impossible to tell where this fragment ends and next one starts

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also unless it’s length prefixed it would be impossible to tell where this fragment ends and next one starts

This is not the case. The method-code should tell the implementer how long the expected bytestring is. See Key DID for an example.

Oh it’s actual id, was just confused by the name (perhaps it should be called whatever did:core calls those instead)

Yes, I think this makes sense. It's slightly nuanced though since in Key DID what we use as method-code is itself part of the "method-specific-identifier"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what you mean is that <method-code><method-bytes> is it's own multiformat, meaning that method-bytes may contain additional varint tags in them when necessary e.g. did:web will have to length-prefix it because unlike did:key there's no way to infer the length.

I think it is worth mentioning this somehow in the spec, maybe even an example of did:web would be useful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't exactly know what you mean by "its own multiformat". Can you maybe make a suggestion with the edit you are thinking about?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might be in agreement here and just lack formal language to describe these things. In the comment above I proposed IPLD Schema extension syntax to describe this

methods/did:.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
methods/did:.md Outdated Show resolved Hide resolved
@@ -0,0 +1,47 @@
# did:key

The [Key DID method](https://w3c-ccg.github.io/did-method-key) encoding leverages the existing multicodes used by the Key DID method.
Copy link
Contributor

Choose a reason for hiding this comment

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

My main problem is that unlike https://github.com/ucan-wg/ucan-ipld/#21-principal we have to tag keys with additional 0x0d1d umbrella tag, rendering all our UCANs invalid.

This may not be a deal-breaker necessarily, however I'm not entirely convinced this approach is strictly better. While it more explicitly signals that value is a did:key as opposed to just public key, I would argue that both are just public keys formatted differently and given the context they are used in it's obvious which one is implied.

While it may be more challenging to specify multidid without umbrella tag, I still think it may be a good idea because that way one could go and implement that spec, while approach with <multidid-code><method-code> is practically impossible as there could be some unknown method-code making it unclear how to interpret method-bytes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A few problems I've observed with not having a multidid-code:

  • How do I know what I'm looking at is a DID and should be treated as such? (e.g. just a public key or a DID key)
  • In the new version of 3ID we are planning to use a multihash as the identifier, in this case how would I know that I'm looking at a multihash vs a DID?
  • For DID PKH the plan is to use 0xca, this is already registered as "CAIP-50 multi-chain account id", not as DID

is practically impossible as there could be some unknown method-code making it unclear how to interpret method-bytes.

How is this different from just having a bunch of different codes for each DID method? At least with a DID umbrella tag the implementation can know it's a DID and throw an appropriate error (e.g. method not supported)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, and the main thing that we wouldn't get without the 0x0d1d tag is a way to encode an entire DID URL.

For example you can specify a specific verification method on DID key by:

did:key:z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp#z6LShs9GGnqk85isEBzzshkuVWrVKsRp24GnDuHk8QWkARMW

Here, we refer to the x25519 Verification Method of a ed25519 Key DID.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think I'm ok with multidid requiring 0x0d1d and in contexts where it's clear what you mean from the field name like iss and aud in UCANs we can define them as union of MultiDID and DIDKey

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in UCANs we can define them as union of MultiDID and DIDKey

I understand why you'd want to support reading a DIDKey without the multidid code for backwards compatibility. I assume that all new writes would still use MiltiDID? Otherwise we might end up with different ways of writing the same capability based on implementation?

Copy link
Contributor

Choose a reason for hiding this comment

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

I need to think more about it. We have observed similar tension in UCAN invocation spec, where we need to tag invocations in context where you may be allowing others things and do not tag where only thing you allow is invocations.

If we end up allowing non DID principals in UCANs as discussed here I absolutely agree that prefixing with 0x0d1d is to go. On the other hand if we only allow DIDs I would argue that canonical representation should probably omit the tag.

That said I don't think it should affect this spec any way though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we end up allowing ucan-wg/spec#139 I absolutely agree that prefixing with 0x0d1d is to go.

Interesting point. In this case it might be worth adding the 0x0d1d prefix regardless for future compatibility in case non DID Principals become a thing?

@ukstv
Copy link

ukstv commented Jan 4, 2023

Really lovely format!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Multidid is a representation strategy for DIDs and DID URLs that is very compact and extensible. It allows any DID method to be represented as a string of bytes.

## Format

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should just use IPLD schema extension syntax to define multiformats, otherwise they're just too vague I'd suggest something like

type MultiDID union {
  | DID   "0x0d1d"
} representation varintprefix

type DID struct {
  # DID method-name + method-specific-id
  id                         Method

  # DID URL path, query & fragment are length prefixed bytes
  location               Location
} representation varintbytes

type Method union {
   # did:key
   Key          "0xe7"
   Key          "0xeb"
   Key          "0xec"
   Key          "0xed"
   Key          "0x1200"
   Key          "0x1201"
   Key          "0x1202"
   Key          "0x1205"

   # did:pkh
   PKH         "0xca"

   # did:*
   Raw         "0x55"
} representation varintprefix

type Location bytes representation lengthprefixed

That way we can go ahead and define Raw Key and PKH as separate schemas like this.

Copy link
Collaborator Author

@oed oed Feb 6, 2023

Choose a reason for hiding this comment

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

How about in a similar way as we define varsig?

<varint 0x0d1d><varint method_code><method_id_bytes><varint url_length><url_bytes>

I worry that using new words like varintprefix and lengthprefixed makes it harder for people who are not familiar with multiformats because they are not aware of how these layouts usually look like.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about in a similar way as we define varsig?

I was in fact going to suggest the same thing in varsig spec. I would much rather write somewhere what we mean by varintprefix and lengthprefixed than use syntax that has no definition at all.

That said those are just suggestions, it's up to you what makes most sense.

Ideally we'd even have visuals like ssb and dat protocols have

https://ssbc.github.io/scuttlebutt-protocol-guide/#rpc-protocol
https://dat-ecosystem-archive.github.io/how-dat-works/#mdns-response

And I was hoping we could generate such things from the extended schema


* `multidid-code` - the value `0x0d1d` encoded as a varint
* `method-code` - a varint encoded multicode for the [DID Method](https://www.w3.org/TR/did-core/#a-simple-example)
* `method-bytes` - data representing the method-id
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might be in agreement here and just lack formal language to describe these things. In the comment above I proposed IPLD Schema extension syntax to describe this


```
<multidid-code><method-code><method-bytes><url-length><url-bytes>
```
Copy link
Contributor

Choose a reason for hiding this comment

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

With the proposed syntax we could define Raw as follows

type Raw bytes

@@ -0,0 +1,33 @@
# did:*

The "any" DID method encoding allows any DID to be represented as a multidid by essentially putting the entire DID ASCII string in the URL portion of the multidid byte string.
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 personally expect that everything but path, query and fragment to go here instead, that way Location will be the same across all of the DID methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But then we would need two length varints. One for the DID itself and one for the URL component. Seems less efficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct it would have some overhead when you actually have path / query / fragment. On the flip-side you'll have more uniform structure.

If you don't want to pay that overhead I would define multidid as follows instead

type MultiDID union {
  | DID   "0x0d1d"
} representation varintprefix

type DID union {
  Key          "0xe7"
  # ....
  # did:pkh
  PKH         "0xca"
  # did:*
  Raw         "0x55"
} representation varintprefix

That way Key, PKH can have method bytes and url bytes, while Raw does no.

@@ -0,0 +1,47 @@
# did:key

The [Key DID method](https://w3c-ccg.github.io/did-method-key) encoding leverages the existing multicodes used by the Key DID method.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think I'm ok with multidid requiring 0x0d1d and in contexts where it's clear what you mean from the field name like iss and aud in UCANs we can define them as union of MultiDID and DIDKey

Co-authored-by: Irakli Gozalishvili <contact@gozala.io>
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@bumblefudge bumblefudge left a comment

Choose a reason for hiding this comment

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

LGTM except I'm confused by the exchange here - are UCAN implementations a consumer/target of this spec, or not?

@bumblefudge
Copy link
Collaborator

apologies for the delay, hoping the timeline and dependencies can be made clearer in today's meeting!


```
// did:example:123456
0x9d1a370e6578616d706c653a313233343536

Choose a reason for hiding this comment

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

@oed
https://github.com/chrisdickinson/varint seems to be consistent with the example strings.

npm i varint
var varint = require('varint')

did:example:123456
0x9d1a 37 0e 6578616d706c653a313233343536

multidid-code 0x0d1d

varint.encode(0x0d1d).map((x) => x.toString(16)).join("")  // '9d1a'

method-code Raw 55 // 0x37

varint.encode(55).map((x) => x.toString(16)).join("")  // '37'

method-bytes "" // raw has no method-bytes

url-length // 0x0e

"example:123456".length  // 14
varint.encode(14).map((x) => x.toString(16)).join("") // "e"
// warn: need to include the '0' padding 0x0e

url-bytes 0x6578616d706c653a313233343536

Array.from("example:123456", (x) => x.charCodeAt(0).toString(16)).join("")
// '6578616d706c653a313233343536'

// did:example:123456?versionId=1
0x9d1a 37 1a 6578616d706c653a3132333435363f76657273696f6e49643d31

    0x9d1a  // multidid-code 0x0d1d 
    0x37  // method-code 55 Raw
    0x1a  // url-length 26
    0x6578616d706c653a3132333435363f76657273696f6e49643d31  // "example:123456?versionId=1" url-bytes

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bumblefudge bumblefudge merged commit bdfd56b into main Feb 21, 2023

// did:key:zQ3shokFTS3brHcDQrn82RUDfCZESWL1ZdCEJwekUDPQiYBme
0x9d1ae70103874c15c7fda20e539c6e5ba573c139884c351188799f5458b4b41f7924f235cd
```

Choose a reason for hiding this comment

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

npm i multibase
const multibase = require('multibase')

<multidid-code><method-code><method-bytes><url-length><url-bytes>

// did:key:z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp
0x9d1a ed01 ed01 3b6a27bcceb6a42d62a3a8d02a6f0d73653215771de243a63ac048a18b59da29

0x9d1a  // multidid-code 0x0d1d
0xed01  // method-code 0xed ed25519-pub
0x3b6a27bcceb6a42d62a3a8d02a6f0d73653215771de243a63ac048a18b59da29  // method-bytes
"" // url-length
"" // url-bytes
varint.decode(multibase.decode("z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp")).toString(16)  // 'ed' ed25519-pub
varint.decode.bytes // 2 vint is first 2 bytes
Array.from(multibase.decode("z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp")).slice(2).map(i => i.toString(16)).padStart(2, '0').join("") // '3b6a27bcceb6a42d62a3a8d02a6f0d73653215771de243a63ac048a18b59da29'

// did:key:z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp#z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp
0x9d1a ed01 ed01 (I think this second ed01 a bug) 3b6a27bcceb6a42d62a3a8d02a6f0d73653215771de243a63ac048a18b59da2931237a364d6b6954427a31796d75657041513448454859534631483871754735474c5656515233646a6458336d446f6f5770

0x9d1a  // multidid-code 0x0d1d
0xed01  // method-code 0xed ed25519-pub
0x3b6a27bcceb6a42d62a3a8d02a6f0d73653215771de243a63ac048a18b59da29  // method-bytes
0x31 // url-length
0x237a364d6b6954427a31796d75657041513448454859534631483871754735474c5656515233646a6458336d446f6f5770 // url-bytes
Array.from(multibase.decode("z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp")).slice(2).map(i => i.toString(16).padStart(2, '0')).join("")  // '3b6a27bcceb6a42d62a3a8d02a6f0d73653215771de243a63ac048a18b59da29'
"#z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp".length  // 49

varint.encode(49).map((x) => x.toString(16).padStart(2, '0')).join("")  // '31'

Array.from("#z6MkiTBz1ymuepAQ4HEHYSF1H8quG5GLVVQR3djdX3mDooWp", (x) => x.charCodeAt(0).toString(16).padStart(2, '0')).join("") 
 // '237a364d6b6954427a31796d75657041513448454859534631483871754735474c5656515233646a6458336d446f6f5770'

// did:key:zQ3shokFTS3brHcDQrn82RUDfCZESWL1ZdCEJwekUDPQiYBme
0x9d1a e701 03874c15c7fda20e539c6e5ba573c139884c351188799f5458b4b41f7924f235cd

0x9d1a  // multidid-code 0x0d1d
0xe701  // method-code 0xed ed25519-pub
0x03874c15c7fda20e539c6e5ba573c139884c351188799f5458b4b41f7924f235cd  // method-bytes
"" // url-length
"" // url-bytes
varint.decode(multibase.decode("zQ3shokFTS3brHcDQrn82RUDfCZESWL1ZdCEJwekUDPQiYBme")).toString(16)  // 'e7' secp256k1-pub
varint.decode.bytes  // 2 vint is first 2 bytes

Array.from(multibase.decode("zQ3shokFTS3brHcDQrn82RUDfCZESWL1ZdCEJwekUDPQiYBme")).slice(2).map(i => i.toString(16).padStart(2, '0')).join("")  // 
'03874c15c7fda20e539c6e5ba573c139884c351188799f5458b4b41f7924f235cd'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, that looks like a bug to me @AaronGoldman. Could you open a PR to fix it?

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.

7 participants