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

Local Store Fails to Update Added Key #1362

Open
radleylewis opened this issue Mar 22, 2024 · 2 comments
Open

Local Store Fails to Update Added Key #1362

radleylewis opened this issue Mar 22, 2024 · 2 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@radleylewis
Copy link
Contributor

Bug severity
4

Describe the bug
When adding a key, using the didManagerAddKey method, the Identifier is being successfully transmitted to the network but is not stored faithfully in the local store (i.e. the resolved DIDDoc from the network includes the X25519 key being added, however, the local store using didManagerGet resolves a DIDDoc that does not included the added key).

To Reproduce
Steps to reproduce the behaviour:
I have create a repository here which isolates the relevant logical calls and configures a minimal veramo agent to demonstrate the issue. There are instructions included in the README.

Observed behaviour
When using a db connection to sqlite or postgres the local DIDDoc does not include the added key (despite it having been transmitted successfully to the network and available on the network resolvable DIDDoc). When using the MemoryDidStore and MemoryKeyStore (as opposed to the keystores with the persistent db connection), the correct DIDDoc is returned (i.e. the local DIDDoc includes the added X25519 key).

Expected behaviour
The local DIDDoc should reflect the updated changes (added key) when using the persistent database connections (sqlite, postgres).

Details
Expected Local DIDDoc (includes two keys: secp256k1, x25519 just added):

{
  "did": "did:ethr:sepolia:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d",
  "controllerKeyId": "0471ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d3fe249e26270596f79915e6743661e4a2f587fdfcd442fd524ce8d0ddd65e35d",
  "provider": "did:ethr:sepolia",
  "services": [],
  "keys": [
    {
      "kid": "0471ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d3fe249e26270596f79915e6743661e4a2f587fdfcd442fd524ce8d0ddd65e35d",
      "type": "Secp256k1",
      "kms": "local",
      "publicKeyHex": "0471ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d3fe249e26270596f79915e6743661e4a2f587fdfcd442fd524ce8d0ddd65e35d",
      "meta": {
        "algorithms": [
          "ES256K",
          "ES256K-R",
          "eth_signTransaction",
          "eth_signTypedData",
          "eth_signMessage",
          "eth_rawSign"
        ]
      }
    },
    {
      "kms": "local",
      "kid": "didcomm-enc-key",
      "type": "X25519",
      "privateKeyHex": "87e81ef7539e97c5857b513e26718eeb84111f92f50d0f76d561b401bb22c5d1e2789bf35f6ed4864c69da6bbba1d0112d67ee5bc2292d5937115ac587daf529",
      "publicKeyHex": "e2789bf35f6ed4864c69da6bbba1d0112d67ee5bc2292d5937115ac587daf529",
      "meta": {
        "algorithms": [
          "ECDH",
          "ECDH-ES",
          "ECDH-1PU"
        ]
      }
    }
  ]
}

Actual Output (missing x25519 key):

{
  "did": "did:ethr:sepolia:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d",
  "controllerKeyId": "0471ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d3fe249e26270596f79915e6743661e4a2f587fdfcd442fd524ce8d0ddd65e35d",
  "provider": "did:ethr:sepolia",
  "services": [],
  "keys": [
    {
      "kid": "0471ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d3fe249e26270596f79915e6743661e4a2f587fdfcd442fd524ce8d0ddd65e35d",
      "type": "Secp256k1",
      "kms": "local",
      "publicKeyHex": "0471ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d3fe249e26270596f79915e6743661e4a2f587fdfcd442fd524ce8d0ddd65e35d",
      "meta": {
        "algorithms": [
          "ES256K",
          "ES256K-R",
          "eth_signTransaction",
          "eth_signTypedData",
          "eth_signMessage",
          "eth_rawSign"
        ]
      }
    }
  ]
}

Additional context
Repo to reproduce

Versions (please complete the following information):

  • Veramo: 5.6.0
  • Browser: N/A
  • Node Version: 21.4.0
@radleylewis radleylewis added the bug Something isn't working label Mar 22, 2024
@radleylewis
Copy link
Contributor Author

I want to provide an update on this bug. Specifically, I have identified the variation in the behaviour between using the memory stores and the persistent databases (sqlite, postgres etc).

The issue pertains to the DIDStore. When the MemoryDIDStore saves an identifier, it is not concerned with any relations (i.e. keys, services) and so simply applies the save in importDID as follows:

  async importDID(args: IIdentifier) {
    const identifier = { ...args }
    for (const key of identifier.keys) {
      if (key.privateKeyHex) {
        delete key.privateKeyHex
      }
    }
    this.identifiers[args.did] = identifier // this line here
    return true
  }

However, when dealing with the persistent datastores, if the key itself has not first been imported using agent.keyManagerImport then the operation will fail. So any call to didManagerAddKey needs to be preempted with keyManagerImport.

Its arguable that this is not a bug, however, it isn't at all intuitive and I was stuck on this for a fair while. I am happy to raise a PR with a fix, however the typing in the didStore doesn't allow for easy patching.

Copy link

stale bot commented May 30, 2024

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.

@stale stale bot added the wontfix This will not be worked on label May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant