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.

Sign up for GitHub

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

Jump to bottom

fix: change 'owner' to 'controller' to follow W3C Spec (#75) #81

Merged
merged 1 commit into from
Aug 24, 2020
Merged

fix: change 'owner' to 'controller' to follow W3C Spec (#75) #81

merged 1 commit into from
Aug 24, 2020

Conversation

PeterTheOne
Copy link
Contributor

see: #75

mirceanis
mirceanis requested changes Aug 21, 2020
View reviewed changes
Copy link
Member

@mirceanis mirceanis left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution.
It looks like this PR needs to be rebased or recreated because of conflicts caused by #73
I assume it won't be hard to redo since it seems to be a replace-all

Also, if anyone is using the did:ethr:...#owner public key id then it will be a breaking change for them, meaning that if there's a JWT that declares the did:ethr:0x...#owner in their header, it will become invalid after this change.

@PeterTheOne
Copy link
Contributor Author

It looks like this PR needs to be rebased or recreated because of conflicts caused by #73
I assume it won't be hard to redo since it seems to be a replace-all

done. It is basically a replace-all with the contract unchanged.

Also, if anyone is using the did:ethr:...#owner public key id then it will be a breaking change for them, meaning that if there's a JWT that declares the did:ethr:0x...#owner in their header, it will become invalid after this change.

Correct. Should we do something to stay backwards compatible? Maybe we could duplicate entries and have both #owner and #controller?

@mirceanis
Copy link
Member

Correct. Should we do something to stay backwards compatible? Maybe we could duplicate entries and have both #owner and #controller?

I guess that would add too much complexity that would later have to be supported for a very unlikely edge case.
It makes more sense to release a new major version with this, respecting semantic-release.

mirceanis
mirceanis approved these changes Aug 24, 2020
View reviewed changes
Copy link
Member

@mirceanis mirceanis left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution

@mirceanis mirceanis added this to todo in typescript libs did-core spec compliance via automation Aug 24, 2020
@mirceanis mirceanis merged commit af37b3f into decentralized-identity:master Aug 24, 2020
typescript libs did-core spec compliance automation moved this from todo to done Aug 24, 2020
uport-automation-bot pushed a commit that referenced this pull request Aug 24, 2020
@semantic-release-bot
chore(release): 3.0.0 [skip ci]
b825b47 
# [3.0.0](2.4.0...3.0.0) (2020-08-24)

### Bug Fixes

* change 'owner' to 'controller' to follow W3C Spec ([#75](#75)) ([#81](#81)) ([af37b3f](af37b3f))

### BREAKING CHANGES

* JWTs that refer to the `did:ethr:...#owner` key in their header may be considered invalid after this upgrade, as the key id is now `did:ethr:...#controller`
@mirceanis mirceanis mentioned this pull request Aug 24, 2020
Closed
@uport-automation-bot
Copy link
Collaborator

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

veramo-bot pushed a commit to veramolabs/ens-did-resolver that referenced this pull request Jul 10, 2022
@semantic-release-bot
chore(release): 1.0.0 [skip ci]
889b3ff 
# 1.0.0 (2022-07-10)

### Bug Fixes

* change 'owner' to 'controller' to follow W3C Spec ([decentralized-identity#75](https://github.com/veramolabs/ens-did-resolver/issues/75)) ([decentralized-identity#81](https://github.com/veramolabs/ens-did-resolver/issues/81)) ([af37b3f](af37b3f))
* ignore query string when interpreting identifiers ([decentralized-identity#123](https://github.com/veramolabs/ens-did-resolver/issues/123)) ([5508f8a](5508f8a)), closes [decentralized-identity#122](https://github.com/veramolabs/ens-did-resolver/issues/122)
* maintenance of dependencies, bots and build scripts ([decentralized-identity#136](https://github.com/veramolabs/ens-did-resolver/issues/136)) ([0d3fcf7](0d3fcf7))
* remove unused dependency ([#4](#4)) ([a97c826](a97c826))
* removed redundant code ([ca4d101](ca4d101))
* reverse events to have consistent order ([decentralized-identity#87](https://github.com/veramolabs/ens-did-resolver/issues/87)) ([08b9692](08b9692)), closes [/github.com/decentralized-identity/issues/86#issuecomment-699961595](https://github.com//github.com/decentralized-identity/ethr-did-resolver/issues/86/issues/issuecomment-699961595)
* strip milliseconds from dateTime strings ([decentralized-identity#129](https://github.com/veramolabs/ens-did-resolver/issues/129)) ([3e958af](3e958af)), closes [decentralized-identity#126](https://github.com/veramolabs/ens-did-resolver/issues/126)
* use rpcUrl in controller config ([decentralized-identity#128](https://github.com/veramolabs/ens-did-resolver/issues/128)) ([5302536](5302536)), closes [decentralized-identity#127](https://github.com/veramolabs/ens-did-resolver/issues/127)
* **deps:** update dependency buffer to v6 ([decentralized-identity#93](https://github.com/veramolabs/ens-did-resolver/issues/93)) ([e1dc861](e1dc861))
* **deps:** update dependency did-resolver to v1.1.0 ([ab47058](ab47058))
* **deps:** update dependency did-resolver to v2 ([decentralized-identity#68](https://github.com/veramolabs/ens-did-resolver/issues/68)) ([831ec17](831ec17))
* **deps:** update dependency did-resolver to v2.1.0 ([b26d387](b26d387))
* **deps:** update dependency did-resolver to v2.1.1 ([1a4cbca](1a4cbca))
* **deps:** update dependency did-resolver to v2.1.2 ([8c2294e](8c2294e))
* **deps:** update dependency ethjs-contract to ^0.2.0 ([b667ce6](b667ce6))
* **deps:** use Resolvable type from did-resolver ([d213ae6](d213ae6))
* **types:** simplify type exports ([decentralized-identity#101](https://github.com/veramolabs/ens-did-resolver/issues/101)) ([90ca9b5](90ca9b5))
* remove ejs module distribution ([780ec08](780ec08)), closes [decentralized-identity#39](https://github.com/veramolabs/ens-did-resolver/issues/39)
* require a configuration to be used when initializing the resolver ([3adc029](3adc029))

### Features

* add `assertionMethod` by default to didDocument ([decentralized-identity#124](https://github.com/veramolabs/ens-did-resolver/issues/124)) ([11b2096](11b2096)), closes [decentralized-identity#117](https://github.com/veramolabs/ens-did-resolver/issues/117) [decentralized-identity#115](https://github.com/veramolabs/ens-did-resolver/issues/115)
* add ability to use a compressed publicKey as identifier ([decentralized-identity#73](https://github.com/veramolabs/ens-did-resolver/issues/73)) ([e257eb3](e257eb3)), closes [decentralized-identity#56](https://github.com/veramolabs/ens-did-resolver/issues/56)
* add encryption key support for ethr-did-documents ([dff7b0f](dff7b0f)), closes [decentralized-identity#52](https://github.com/veramolabs/ens-did-resolver/issues/52)
* add encryption key support for ethr-did-documents ([2f5825c](2f5825c)), closes [decentralized-identity#52](https://github.com/veramolabs/ens-did-resolver/issues/52)
* Add types declaration stubb ([05944b1](05944b1))
* export `EthrDidController` helper class ([decentralized-identity#120](https://github.com/veramolabs/ens-did-resolver/issues/120)) ([745100d](745100d))
* import instead of require networks.json ([50c0832](50c0832))
* Initial version ([#1](#1)) ([d7a3cf8](d7a3cf8))
* upgrade to latest did core spec ([decentralized-identity#99](https://github.com/veramolabs/ens-did-resolver/issues/99)) ([decentralized-identity#109](https://github.com/veramolabs/ens-did-resolver/issues/109)) ([d46eea3](d46eea3)), closes [decentralized-identity#105](https://github.com/veramolabs/ens-did-resolver/issues/105) [decentralized-identity#95](https://github.com/veramolabs/ens-did-resolver/issues/95) [decentralized-identity#106](https://github.com/veramolabs/ens-did-resolver/issues/106) [decentralized-identity#83](https://github.com/veramolabs/ens-did-resolver/issues/83) [decentralized-identity#85](https://github.com/veramolabs/ens-did-resolver/issues/85) [decentralized-identity#83](https://github.com/veramolabs/ens-did-resolver/issues/83) [decentralized-identity#85](https://github.com/veramolabs/ens-did-resolver/issues/85) [decentralized-identity#95](https://github.com/veramolabs/ens-did-resolver/issues/95) [decentralized-identity#105](https://github.com/veramolabs/ens-did-resolver/issues/105) [decentralized-identity#106](https://github.com/veramolabs/ens-did-resolver/issues/106)
* upgrade to latest did core spec ([decentralized-identity#99](https://github.com/veramolabs/ens-did-resolver/issues/99)) ([decentralized-identity#109](https://github.com/veramolabs/ens-did-resolver/issues/109)) ([decentralized-identity#111](https://github.com/veramolabs/ens-did-resolver/issues/111)) ([2a023b1](2a023b1)), closes [decentralized-identity#105](https://github.com/veramolabs/ens-did-resolver/issues/105) [decentralized-identity#95](https://github.com/veramolabs/ens-did-resolver/issues/95) [decentralized-identity#106](https://github.com/veramolabs/ens-did-resolver/issues/106) [decentralized-identity#83](https://github.com/veramolabs/ens-did-resolver/issues/83) [decentralized-identity#85](https://github.com/veramolabs/ens-did-resolver/issues/85) [decentralized-identity#83](https://github.com/veramolabs/ens-did-resolver/issues/83) [decentralized-identity#85](https://github.com/veramolabs/ens-did-resolver/issues/85) [decentralized-identity#95](https://github.com/veramolabs/ens-did-resolver/issues/95) [decentralized-identity#105](https://github.com/veramolabs/ens-did-resolver/issues/105) [decentralized-identity#106](https://github.com/veramolabs/ens-did-resolver/issues/106)
* use only named exports ([decentralized-identity#31](https://github.com/veramolabs/ens-did-resolver/issues/31)) ([a558e14](a558e14))
* versioning ([decentralized-identity#121](https://github.com/veramolabs/ens-did-resolver/issues/121)) ([b794d69](b794d69)), closes [decentralized-identity#119](https://github.com/veramolabs/ens-did-resolver/issues/119) [decentralized-identity#118](https://github.com/veramolabs/ens-did-resolver/issues/118) [decentralized-identity#119](https://github.com/veramolabs/ens-did-resolver/issues/119) [decentralized-identity#118](https://github.com/veramolabs/ens-did-resolver/issues/118)

### BREAKING CHANGES

* The return type is `DIDResolutionResult` which wraps a `DIDDocument`.
* No errors are thrown during DID resolution. Please check `result.didResolutionMetadata.error` instead.
* This DID core spec requirement will break for users expecting `publicKey`, `ethereumAddress`, `Secp256k1VerificationKey2018` entries in the DID document. They are replaced with `verificationMethod`, `blockchainAccountId` and `EcdsaSecp256k1VerificationKey2019` and `EcdsaSecp256k1RecoveryMethod2020` depending on the content.
* JWTs that refer to the `did:ethr:...#owner` key in their header may be considered invalid after this upgrade, as the key id is now `did:ethr:...#controller`
* this removes the fallback hardcoded RPC URLs and will fail early when a wrong configuration (or none) is provided to `getResolver()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mirceanis mirceanis mirceanis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
typescript libs did-core spec compliance
  
done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@PeterTheOne @mirceanis @uport-automation-bot