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

describe query parameter passing for public keys #37

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bumblefudge
Copy link
Collaborator

@bumblefudge bumblefudge commented Aug 8, 2023

As per the CASA meeting today, trying to update did-pkh to reflect the complexity of sub-methods requiring recovery or pk-passing to generate DID documents. @chunningham i can't assign you because it's a CCG repo but feel free to review as well.

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Editorial grammar, punctuation, and markup...

corresponding to the blockchain ecosystem within which it is valid. No DID
Document should be returned for a DID URL that is not a valid [CAIP-10][] URI.

In some contexts, such as in web-based interfaces for Solana or Tezos wallets, a
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In some contexts, such as in web-based interfaces for Solana or Tezos wallets, a
In some contexts, such as web-based interfaces for Solana or Tezos wallets, a

In some contexts, such as in web-based interfaces for Solana or Tezos wallets, a
public key is traditionally passed by wallets to counterparties as part of
authentication, without which signatures cannot be verified and submitted to
public ledgers. In other contexts, such as in web-based interfaces for Ethereum
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public ledgers. In other contexts, such as in web-based interfaces for Ethereum
public ledgers. In other contexts, such as web-based interfaces for Ethereum

wallets, a public key is cryptographically [recoverable][EC Recovery for
secp256k1] from a signature over a known message, but unknowable without one. In
all of these cases, a DID URL containing only the public-key hash will not be
enough to recover the corresponding public key and thus generate a useful DID
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
enough to recover the corresponding public key and thus generate a useful DID
enough to recover the corresponding public key and use it to generate a useful DID

Comment on lines +176 to +178
document; instead, a query parameter `pk` (i.e. `?pk=`) can be appended to a DID
PKH URL such that a DID document can be generated for that DID PKH URL. For a
valid DID URL (i.e. one conforming to [CAIP-10]) without the additional
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
document; instead, a query parameter `pk` (i.e. `?pk=`) can be appended to a DID
PKH URL such that a DID document can be generated for that DID PKH URL. For a
valid DID URL (i.e. one conforming to [CAIP-10]) without the additional
document; instead, the query parameter `pk` can be appended to a DID
PKH URL (i.e., `?pk=`), and a DID document can be generated for that DID PKH URL. For a
valid DID URL (i.e., one conforming to [CAIP-10]) without the additional

document; instead, a query parameter `pk` (i.e. `?pk=`) can be appended to a DID
PKH URL such that a DID document can be generated for that DID PKH URL. For a
valid DID URL (i.e. one conforming to [CAIP-10]) without the additional
information needed to generate a DID Document, an "empty" DID document
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
information needed to generate a DID Document, an "empty" DID document
information needed to generate a DID document, an "empty" DID document

that Spruce's implementation also supports for backwards-compatibility with
credentials already issued look like this:
namespacing rather than referring directly to the CAIP naming convention. It
also defaulted to main-net for `did:pkh:eth` in the absence of an explicit
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
also defaulted to main-net for `did:pkh:eth` in the absence of an explicit
also defaulted to `main-net` for `did:pkh:eth` in the absence of an explicit

credentials already issued look like this:
namespacing rather than referring directly to the CAIP naming convention. It
also defaulted to main-net for `did:pkh:eth` in the absence of an explicit
chainID. As the scope has grown of this project, and with forward compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
chainID. As the scope has grown of this project, and with forward compatibility
`chainID`. As the scope of this project has grown, and with forward compatibility

namespacing rather than referring directly to the CAIP naming convention. It
also defaulted to main-net for `did:pkh:eth` in the absence of an explicit
chainID. As the scope has grown of this project, and with forward compatibility
in mind, both of these patterns have been removed, required explicitly naming
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
in mind, both of these patterns have been removed, required explicitly naming
in mind, both of these patterns have been removed, now requiring explicitly naming

Comment on lines +277 to +278
implementation also supports for backwards-compatibility with credentials
already issued look like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
implementation also supports for backwards-compatibility with credentials
already issued look like this:
implementation also supports for backwards-compatibility with previously
issued credentials look like this:

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

3 participants