[BUG] Resolver adds more keys than it should

[ismapolis]

Current Behavior

When i send a setAttribute transaction with key equals "did/pub/Secp256k1/enc/" then on the did document it adds an entry on assertionMethod and KeyAgreement. When i select veriKey instead of enc it works fine.

Expected Behavior

The resolver should just adds an entry on KeyAgreement.

Failure Information

Please help provide information about the failure.

Steps to Reproduce

I am using a react app for testing with some select elements for key purpose and encoding. For the blockchain I am using Ganache and Metamask as Web3 provider.
Here is a fragment of the code I use for emitting the transaction. "keyuse" and "encode" are the values from the select elements.

let key = "did/pub/Secp256k1/" + keyuse + "/" + encode;
console.log(key);
await contract.methods
      .setAttribute(
        accounts[0],
        stringToBytes32(key),
        "0x" + publicKey,
        31536000
      )
      .send({ from: accounts[0] });

Environment Details

I attach the react component code on zip.

index.zip

Failure Logs/Screenshots

DID initial doc

After I emit a transaction with enc value

And then when I try to revoke that verificationMethod

[mirceanis]

Sounds like a bug.. it should not be added to assertionMethod; just to keyAgreement.

[ismapolis]

Sounds like a bug.. it should not be added to assertionMethod; just to keyAgreement.

Hello I have been reviewing the code and it is not a bug, it is coded like that but I dont know why, maybe an older version of did:ethr specification?
Here it filters "sigAuth" and "enc" and adds the key to the corresponding section, but not for assertion.

Later all the verification methods are added to assertionMethod don't know why.

I even checked the tests and in the case of adding an "enc" key the test checks that it is added to KeyAgreement and assertionMethod.

I could fix it and make a PR If you approve it.

[mirceanis]

Sounds like a bug.. it should not be added to assertionMethod; just to keyAgreement.

Hello I have been reviewing the code and it is not a bug, it is coded like that but I dont know why, maybe an older version of did:ethr specification? Here it filters "sigAuth" and "enc" and adds the key to the corresponding section, but not for assertion.

Later all the verification methods are added to assertionMethod don't know why.

I even checked the tests and in the case of adding an "enc" key the test checks that it is added to KeyAgreement and assertionMethod.

I could fix it and make a PR If you approve it.

It's still a bug, even if it looks intentional.
The origin of the bug is from the early days of the DID spec, when keys didn't need to have a clear verification relationship. It's also common for authentication keys to also be usable as assertion keys, but it's debatable whether that should be the case with keyAgreement.

Generally, this DID method leans toward fewer transactions where possible, which is why all keys are automatically dumped in the assertion method relationship.

Fixing this would be a breaking change, however, and should also include an update to the spec.

[ismapolis]

Hello I would like to confirm this with you. The current spec states in key purposes that every key is added to its corresponding section not in more than one. The change I proposed wouldn't breaking. It would just update the code to the referenced spec.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[uport-automation-bot]

🎉 This issue has been resolved in version 10.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀