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

Add spec up spec #162

Merged
merged 11 commits into from
Aug 9, 2024
Merged

Add spec up spec #162

merged 11 commits into from
Aug 9, 2024

Conversation

decentralgabe
Copy link
Member

@decentralgabe decentralgabe commented Jul 19, 2024

Fix #131

This is the first step in making web5 spec more formal, yay!

Next steps:

  • Make sure there is clear normative language (MUST, MUST NOT, etc) around features that we want implemented and tested
  • Open issues for all the TODOs in the spec & add more TODOs
  • Make sure all features are covered in the spec, and see if there are sections we can refine or remove (like that around key management)
  • Consider whether we want to add anything around the API to the spec (cc: @KendallWeihe @frankhinek)
  • Remove the README and generate a GH pages doc...then cut a first version we can publish

Separately, once we agree on the language in the spec and the features we can create a related spec for conformance testing. Here we can note the features, the associated test vectors, and whether the implementations we have are conformant against them.

Open to any & all feedback.

@decentralgabe decentralgabe marked this pull request as ready for review July 31, 2024 05:57
@decentralgabe
Copy link
Member Author

It might be easiest for you to review this if you check out the branch and render the spec yourself.

You can do that with ...

git fetch
git checkout origin/spec-up
cd spec
npm i
npm run build
open build/index.html

Comment on lines +40 to +43
1. **Web1**: Read-only, static websites
2. **Web2**: Read-write, interactive platforms
3. **Web3**: Read-write-own, blockchain-based applications
4. **Web5**: Decentralized, user-centric internet, without the need for a blockchain
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

spec/spec.md Outdated

1. Key Manager Interface
2. In-Memory Key Manager
3. AWS KMS
Copy link
Contributor

Choose a reason for hiding this comment

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

are we 100% sure we need this / its required for the spec?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied from the current readme I'd love to kill this or just define a key manager interface

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel strongly we should kill this as well as (4) and (5) below. I do feel that we need to add support for the 3 I'm proposing removing here, but I lack confidence the 3 belong here in this place at this time. Perhaps we could add a :::note with commentary which describes our intention of covering all major platforms' key management at a later date.

* Data Signing

::: note
[AWS KMS](https://docs.aws.amazon.com/kms/latest/developerguide/asymmetric-key-specs.html#key-spec-ecc) does not support [[ref:Ed25519]]. At some point in the future, we can consider introducing support for key wrapping.
Copy link
Contributor

Choose a reason for hiding this comment

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

especially since it wont event work lol

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm can you expand?


[[ref:JSON-SCHEMA]] support is ****REQUIRED**** for multiple features throughout [[ref:Web5 SDKs]], including, but not limited to support with [[ref:verifiable credentials]], [VC JSON Schemas](#vc-json-schema), and for use in validating the data models this specification has defined.

Conformant implementations of [[ref:Web5 SDKs]] ****MUST**** support at least [[ref:JSON-SCHEMA-DRAFT-7]] and [[ref:JSON-SCHEMA-2020-12]].
Copy link
Contributor

Choose a reason for hiding this comment

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

Chad says JSON-SCHEMA-2020-12 is backwards compatible with JSON-SCHEMA-DRAFT-7, so just writing in JSON-SCHEMA-DRAFT-7 should be sufficient right?

Copy link
Contributor

Choose a reason for hiding this comment

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

And if someone used features from JSON-SCHEMA-2020-12 then it may or may not be compatible with SCHEMA-DRAFT-7... lol so it seems like everyone will / should only use SCHEMA-DRAFT-7

Copy link
Member Author

Choose a reason for hiding this comment

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

we should be using 2020-12 since it's the latest draft, which is being used as the input document to a LTS type draft.

more info here and here which notes that if you're using 2020-12 you'll be fine (we're not using breaking features)

it should be straightforward to convert our schemas to 2020-12, I've done so in the past using this tool https://github.com/sourcemeta/alterschema

spec/spec.md Outdated

| Method | Creation | Resolution | Note |
|--------|----------|------------|------|
| [[ref:did:web]] | ❌ | ✅ | - |
Copy link
Contributor

Choose a reason for hiding this comment

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

We can create a did web document (have this in the cli).. but yea they would have to host it on their domain, so the philosophical question: is a did:web created if its not hosted on a domain?

Copy link
Member Author

Choose a reason for hiding this comment

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

created yes, published no

Copy link
Contributor

Choose a reason for hiding this comment

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

okay so we should change this to

Suggested change
| [[ref:did:web]] | || - |
| [[ref:did:web]] | || - |

yeah?

perhaps we should add a column for Publish?

Copy link
Member Author

Choose a reason for hiding this comment

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

good call, yes

spec/spec.md Outdated

Web5 supports the following DID methods:

| Method | Creation | Resolution | Note |
Copy link
Contributor

Choose a reason for hiding this comment

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

add update column here?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch


#### Credential Status

Conformant [[ref:Web5 SDKs]] ****MUST**** support Credential Status as specified by [[ref:STATUS-LIST-2021]] for usage with the W3C Verifiable Credential Data Model 1.1 [[ref:VC-DATA-MODEL-1.1]].
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

there are some weird things that will break if we do, mostly incompatibility with the 1.1 data model since bitstring is designed to be used with 2.0

spec/spec.md Outdated Show resolved Hide resolved
spec/spec.md Outdated
Comment on lines 101 to 104
| Key Type | Algorithm | Function |
|----------|-----------|----------|
| [[ref:secp256k1]] | `ES256K` [[spec:RFC8812]] | Signing and Verification |
| [[ref:Ed25519]] | `EdDSA` [[spec:RFC8032]] | Signing and Verification |
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I was planning on adding support for the four key types from here https://did-dht.com/registry/index.html#key-type-index

Copy link
Member Author

Choose a reason for hiding this comment

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

excellent, I'll add these

spec/spec.md Outdated Show resolved Hide resolved
spec/spec.md Outdated
| [[ref:did:web]] | ❌ | ✅ | - |
| [[ref:did:jwk]] | ✅ | ✅ | - |
| [[ref:did:dht]] | ✅ | ✅ | This is the "default" method for [[ref:Web5 SDKs]]. |
| [[ref:did:key]] | ⚠️ | ⚠️ | This method has been implemented in both Kotlin and TypeScript, with no plans for support in other languages. |
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 probably just remove this line altogether

The new KT SDK (from Rust Core) won't have this

decentralgabe and others added 3 commits July 31, 2024 13:39
Co-authored-by: Kendall Weihe <[email protected]>
Co-authored-by: Kendall Weihe <[email protected]>
@decentralgabe
Copy link
Member Author

@KendallWeihe @nitro-neal @frankhinek @mistermoe I have incorporated all feedback. I'd like to get this merged and publishing as-is.

Then I will work on a follow up PR for aligning normative statements (MUSTs) to test vectors we have, in addition to removing the README and GH-pageifying the spec.

Copy link
Contributor

@nitro-neal nitro-neal 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! The big thing I would want to see in the future is a "Recommended vc-jwt verification and parsing .

Basically this part:
https://github.com/TBD54566975/web5-rs/blob/main/crates/web5/src/credentials/verifiable_credential_1_1.rs#L271

where a vc jwt is still valid if it has the jwt parts but not the internal vc parts, like it is valid if it hass iss and now vc.issuer (and when we build it internally we add vc.issuer from the iss

Basically this part
image

Maybe an example section of a "minimum viable vc-jwt" and a "full featured vc jwt"

@decentralgabe
Copy link
Member Author

thanks @nitro-neal I added links to that section of the spec and an issue marker for #134

@decentralgabe decentralgabe merged commit 7240e88 into main Aug 9, 2024
@decentralgabe decentralgabe deleted the spec-up branch August 9, 2024 17:07
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.

Move to spec up and cut a verison
5 participants