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

[BUG] (?) Checksum eth addresses resolve to a different did doc than hex addresses #105

Closed
elmariachi111 opened this issue Jan 22, 2021 · 2 comments · Fixed by #111
Closed
Assignees
Labels
bug Something isn't working work-in-progress someone is already working on this

Comments

@elmariachi111
Copy link

Current Behavior

Resolving did:ethr:development:0xaca94ef8bd5ffee41947b4585a84bda5a3d3da6e leads to a different did doc than resolving did:ethr:development:0xACa94ef8bD5ffEE41947b4585a84BdA5a3d3DA6E, e.g. when you add did attributes using chain events. Adding a new key to the lower case version (the default) won't show up when resolving with the upper case version (the Metamask copy/paste default).

Expected Behavior

I would assume that to resolve to the same did doc since checksums are a plain eth security feature and out of scope of the did spec. But I could be wrong with that :D

Environment Details

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • node/browser version: 14.15.3
  • OS Version: Fedora 33
  • Device details: cli

Alternatives you considered

As said, since the DID strings are different, that might not be a "bug" but even expected behaviour, Nevertheless I'd expect the history scan to yield the same results for both variants.

@elmariachi111 elmariachi111 added the bug Something isn't working label Jan 22, 2021
@elmariachi111 elmariachi111 changed the title [BUG] (?) Checksum eth addresses resolve to a different did doc than binary addresses [BUG] (?) Checksum eth addresses resolve to a different did doc than hex addresses Jan 22, 2021
@mirceanis mirceanis self-assigned this Jan 22, 2021
@mirceanis mirceanis added the work-in-progress someone is already working on this label Jan 26, 2021
uport-automation-bot pushed a commit that referenced this issue Mar 15, 2021
# [3.1.0](3.0.3...3.1.0) (2021-03-15)

### Features

* upgrade to latest did core spec ([#99](#99)) ([#109](#109)) ([d46eea3](d46eea3)), closes [#105](#105) [#95](#95) [#106](#106) [#83](#83) [#85](#85) [#83](#83) [#85](#85) [#95](#95) [#105](#105) [#106](#106)
@uport-automation-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

mirceanis added a commit that referenced this issue Mar 15, 2021
* feat: refactor configuration options, add `chainID`
* fix: improve `kid` stability during attribute changes
* chore: cleanup in tests and local dependencies
* test(coverage): improve test coverage
* refactor(style): enable linting using `eslint`
* refactor: isolate some of the type definitions
* fix: test that checksum address resolves to identical DID document (#105)
* fix: setAttribute with sigAuth adds key to authentication section (#95)
* feat: enable base58 encoding for keys (#106)(#99)
* refactor: group types, defaults, and helper methods
* fix: add support for deactivated DIDs (#83)(#85)
* fix(deps): bump did-resolver version
* fix: update the default `@context` to be LD compatible (#99)
* docs: update readme and spec to reflect reality and current DID spec compliance (#99)
* fix: rename `publicKey` to `verificationMethod` (#99)
* fix: rename `ethereumAddress` to `blockchainAccountId` (#99)
* feat: no more errors thrown during resolution.

fixes #83
closes #85
fixes #95
fixes #105
closes #106
fixes #99

BREAKING CHANGE: The return type is `DIDResolutionResult` which wraps a `DIDDocument`.
BREAKING CHANGE: No errors are thrown during DID resolution. Please check `result.didResolutionMetadata.error` instead.
BREAKING CHANGE: 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.
mirceanis added a commit that referenced this issue Mar 15, 2021
* feat: refactor configuration options, add `chainID`
* fix: improve `kid` stability during attribute changes
* chore: cleanup in tests and local dependencies
* test(coverage): improve test coverage
* refactor(style): enable linting using `eslint`
* refactor: isolate some of the type definitions
* fix: test that checksum address resolves to identical DID document (#105)
* fix: setAttribute with sigAuth adds key to authentication section (#95)
* feat: enable base58 encoding for keys (#106)(#99)
* refactor: group types, defaults, and helper methods
* fix: add support for deactivated DIDs (#83)(#85)
* fix(deps): bump did-resolver version
* fix: update the default `@context` to be LD compatible (#99)
* docs: update readme and spec to reflect reality and current DID spec compliance (#99)
* fix: rename `publicKey` to `verificationMethod` (#99)
* fix: rename `ethereumAddress` to `blockchainAccountId` (#99)
* feat: no more errors thrown during resolution.

fixes #83
closes #85
fixes #95
fixes #105
closes #106
fixes #99

BREAKING CHANGE: The return type is `DIDResolutionResult` which wraps a `DIDDocument`.
BREAKING CHANGE: No errors are thrown during DID resolution. Please check `result.didResolutionMetadata.error` instead.
BREAKING CHANGE: 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.
mirceanis added a commit that referenced this issue Mar 15, 2021
* feat: refactor configuration options, add `chainID`
* fix: improve `kid` stability during attribute changes
* chore: cleanup in tests and local dependencies
* test(coverage): improve test coverage
* refactor(style): enable linting using `eslint`
* refactor: isolate some of the type definitions
* fix: test that checksum address resolves to identical DID document (#105)
* fix: setAttribute with sigAuth adds key to authentication section (#95)
* feat: enable base58 encoding for keys (#106)(#99)
* refactor: group types, defaults, and helper methods
* fix: add support for deactivated DIDs (#83)(#85)
* fix(deps): bump did-resolver version
* fix: update the default `@context` to be LD compatible (#99)
* docs: update readme and spec to reflect reality and current DID spec compliance (#99)
* fix: rename `publicKey` to `verificationMethod` (#99)
* fix: rename `ethereumAddress` to `blockchainAccountId` (#99)
* feat: no more errors thrown during resolution.

fixes #83
closes #85
fixes #95
fixes #105
closes #106
fixes #99

BREAKING CHANGE: The return type is `DIDResolutionResult` which wraps a `DIDDocument`.
BREAKING CHANGE: No errors are thrown during DID resolution. Please check `result.didResolutionMetadata.error` instead.
BREAKING CHANGE: 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.
uport-automation-bot pushed a commit that referenced this issue Mar 15, 2021
# [4.0.0](3.1.0...4.0.0) (2021-03-15)

### Features

* upgrade to latest did core spec ([#99](#99)) ([#109](#109)) ([#111](#111)) ([2a023b1](2a023b1)), closes [#105](#105) [#95](#95) [#106](#106) [#83](#83) [#85](#85) [#83](#83) [#85](#85) [#95](#95) [#105](#105) [#106](#106)

### 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.
@uport-automation-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

veramo-bot pushed a commit to veramolabs/ens-did-resolver that referenced this issue Jul 10, 2022
# 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
Labels
bug Something isn't working work-in-progress someone is already working on this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants