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

Multiple records found for given query for recordType: DidCommMessageRecord #2142

Open
GHkrishna opened this issue Jan 6, 2025 · 3 comments

Comments

@GHkrishna
Copy link
Contributor

GHkrishna commented Jan 6, 2025

What

When trying to issue credential in bulk, the issuer seems to be getting multiple records for DidCommMessageRecord in state accept-request

Here is a sample query which return a duplicate record at DidCommMessageRepository

{"associatedRecordId":"d1f0f8ae-a0a7-4d73-ac67-ee5fb824d264","messageName":"request-credential","protocolName":"issue-credential","protocolMajorVersion":"2","role":"receiver"}

which return the following records,

Records found

Some Javascript

[
  {
      "_tags": {
          "associatedRecordId": "d1f0f8ae-a0a7-4d73-ac67-ee5fb824d264",
          "messageId": "a3315fe8-f171-4f5e-a2c0-f80f93f4d237",
          "messageName": "request-credential",
          "messageType": "https://didcomm.org/issue-credential/2.0/request-credential",
          "protocolMajorVersion": "2",
          "protocolMinorVersion": "0",
          "protocolName": "issue-credential",
          "role": "receiver",
          "threadId": "52317613-a1b4-44a1-af91-a91f89eafe71"
      },
      "metadata": {},
      "id": "480344cc-830e-4c13-8903-c67f9f37988a",
      "createdAt": "2025-01-03T07:21:27.598Z",
      "associatedRecordId": "d1f0f8ae-a0a7-4d73-ac67-ee5fb824d264",
      "role": "receiver",
      "message": {
          "@type": "https://didcomm.org/issue-credential/2.0/request-credential",
          "@id": "a3315fe8-f171-4f5e-a2c0-f80f93f4d237",
          "formats": [
              {
                  "attach_id": "636d24b7-9a9d-4951-9d9b-d7769c70c815",
                  "format": "aries/ld-proof-vc-detail@v1.0"
              }
          ],
          "requests~attach": [
              {
                  "@id": "636d24b7-9a9d-4951-9d9b-d7769c70c815",
                  "mime-type": "application/json",
                  "data": {
                      "base64": "eyJjcmVkZW50aWFsIjp7IkBjb250ZXh0IjpbImh0dHBzOi8vd3d3LnczLm9yZy8yMDE4L2NyZWRlbnRpYWxzL3YxIiwiaHR0cHM6Ly9naGtyaXNobmEuZ2l0aHViLmlvL3NjaGVtYXMvQ1JFREVCTC93My0yMDE4LWV4YW1wbGVzLXYxLmpzb24iXSwidHlwZSI6WyJWZXJpZmlhYmxlQ3JlZGVudGlhbCIsIlVuaXZlcnNpdHlEZWdyZWVDcmVkZW50aWFsIl0sImlzc3VlciI6eyJpZCI6ImRpZDprZXk6ejZNa3EzNU5vWks0YTVjYzMxVDdNd1pmMVYzVkNlN3RjQjdSM3dNZGMzZnJnQkpqIn0sImlzc3VhbmNlRGF0ZSI6IjIwMTktMTAtMTJUMDc6MjA6NTAuNTJaIiwiY3JlZGVudGlhbFN1YmplY3QiOnsiaWQiOiJkaWQ6a2V5Ono2TWtvQ3FwREpyNjVpYjd0YTIxdzVERkh4azlONmdnZnlheTRqVmpKQlhuVXdvdyIsImRlZ3JlZSI6eyJ0eXBlIjoiQmFjaGVsb3JEZWdyZWUiLCJuYW1lIjoiQmFjaGVsb3Igb2YgU2NpZW5jZSBhbmQgQXJ0cyJ9fX0sIm9wdGlvbnMiOnsicHJvb2ZUeXBlIjoiRWQyNTUxOVNpZ25hdHVyZTIwMTgiLCJwcm9vZlB1cnBvc2UiOiJhc3NlcnRpb25NZXRob2QifX0="
                  }
              }
          ],
          "~attach": [],
          "~thread": {
              "thid": "52317613-a1b4-44a1-af91-a91f89eafe71"
          }
      },
      "updatedAt": "2025-01-03T07:21:27.598Z"
  },
  {
      "_tags": {
          "associatedRecordId": "d1f0f8ae-a0a7-4d73-ac67-ee5fb824d264",
          "messageId": "6029912c-481d-4017-9ade-108bf70ff734",
          "messageName": "request-credential",
          "messageType": "https://didcomm.org/issue-credential/2.0/request-credential",
          "protocolMajorVersion": "2",
          "protocolMinorVersion": "0",
          "protocolName": "issue-credential",
          "role": "receiver",
          "threadId": "52317613-a1b4-44a1-af91-a91f89eafe71"
      },
      "metadata": {},
      "id": "edd13bc9-6013-4841-8dcc-55d833f0fe83",
      "createdAt": "2025-01-03T07:21:27.345Z",
      "associatedRecordId": "d1f0f8ae-a0a7-4d73-ac67-ee5fb824d264",
      "role": "receiver",
      "message": {
          "@type": "https://didcomm.org/issue-credential/2.0/request-credential",
          "@id": "6029912c-481d-4017-9ade-108bf70ff734",
          "formats": [
              {
                  "attach_id": "cf52b7eb-550b-4ed2-9508-e84b45b4675a",
                  "format": "aries/ld-proof-vc-detail@v1.0"
              }
          ],
          "requests~attach": [
              {
                  "@id": "cf52b7eb-550b-4ed2-9508-e84b45b4675a",
                  "mime-type": "application/json",
                  "data": {
                      "base64": "eyJjcmVkZW50aWFsIjp7IkBjb250ZXh0IjpbImh0dHBzOi8vd3d3LnczLm9yZy8yMDE4L2NyZWRlbnRpYWxzL3YxIiwiaHR0cHM6Ly9naGtyaXNobmEuZ2l0aHViLmlvL3NjaGVtYXMvQ1JFREVCTC93My0yMDE4LWV4YW1wbGVzLXYxLmpzb24iXSwidHlwZSI6WyJWZXJpZmlhYmxlQ3JlZGVudGlhbCIsIlVuaXZlcnNpdHlEZWdyZWVDcmVkZW50aWFsIl0sImlzc3VlciI6eyJpZCI6ImRpZDprZXk6ejZNa3EzNU5vWks0YTVjYzMxVDdNd1pmMVYzVkNlN3RjQjdSM3dNZGMzZnJnQkpqIn0sImlzc3VhbmNlRGF0ZSI6IjIwMTktMTAtMTJUMDc6MjA6NTAuNTJaIiwiY3JlZGVudGlhbFN1YmplY3QiOnsiaWQiOiJkaWQ6a2V5Ono2TWtvQ3FwREpyNjVpYjd0YTIxdzVERkh4azlONmdnZnlheTRqVmpKQlhuVXdvdyIsImRlZ3JlZSI6eyJ0eXBlIjoiQmFjaGVsb3JEZWdyZWUiLCJuYW1lIjoiQmFjaGVsb3Igb2YgU2NpZW5jZSBhbmQgQXJ0cyJ9fX0sIm9wdGlvbnMiOnsicHJvb2ZUeXBlIjoiRWQyNTUxOVNpZ25hdHVyZTIwMTgiLCJwcm9vZlB1cnBvc2UiOiJhc3NlcnRpb25NZXRob2QifX0="
                  }
              }
          ],
          "~attach": [],
          "~thread": {
              "thid": "52317613-a1b4-44a1-af91-a91f89eafe71"
          }
      },
      "updatedAt": "2025-01-03T07:21:27.345Z"
  }
]

I'm unsure if this is an implementation issue (##), or if we should simply return the first found record or add a separate findByQuery in DidCommRepository

(##)I think this might an implementation issue because a request even received twice should not create records with similar associateId and threadId

I was thinking if something similar to this #1495 might be done

@TimoGlastra
Copy link
Contributor

Hmm. In this case, why is the same record being stored twice? Something must trigger the issuer to receive/process the same request credential message twice.

I think it should usually catch this as it's a race condition. This is a limitation of Askar as we can't really prevent this with an unique constraint on the database level.

Could you debug where in the code these two messages are stored? And why it's called twice? We store the didcomm message record using saveOrUpdate (

public async saveOrUpdateAgentMessage(agentContext: AgentContext, options: SaveAgentMessageOptions) {
), and that's probably called twice in rapid succession. Using locking from Askar would already help with this (we fetch the record for update). but i'm still curious why it's being retrieved twice in rapid successions.

Happy to dive into this more if you can provide more details

@GHkrishna
Copy link
Contributor Author

Happy to dive into this more if you can provide more details

Thank you for the enthusiasm

Something must trigger the issuer to receive/process the same request credential message twice.

I think it should usually catch this as it's a race condition.

We are not explicitly interfering as such with the process, once we initiate and create an offer(using existing connection), the flow completes as usual between issuer and holder(connected via mediator).
PS: If its not the issuer, there are chances that the offer created from the issuer reaches holder more than once(from the mediator), which somehow then respond to issuer in a split time frame (in agreement to your statement, this scenario is however not convincing enough to cause duplicate records at Issuer)

Could you debug where in the code these two messages are stored? And why it's called twice? We store the didcomm message record using saveOrUpdate, and that's probably called twice in rapid succession.

Yess, I'll debug it further, however, this might take some time as reproducing this requires a significant number of parallel holders completing the flow multiple times

Using locking from Askar would already help with this (we fetch the record for update).

Okay, so this shouldn't require any additional configurations, right?

I'll debug and share the logs soon.

@TimoGlastra
Copy link
Contributor

Okay, so this shouldn't require any additional configurations, right?

Currently we don't support transactions and locking yet. It should be relatively simple to support:

  • Add option lock / or similar when fetching a record (which in turn should enable forUpdate=true on askar record)
  • Add support for transaction. We currently have withSession on askar wallet. We can use withTransaction ,but we should allow then to pass a session to the storage service. However because it is generic, we need to think how we can create a Credo session. or we should somehow do evertyhing within a callback using a transaction, without exposing the askar seession.

Something like:

storageService.transaction((sessionStorageService) => {
  const existing = await sessionStorageService.findByID('id', {lock: true})

  if (!existing) {
       sessionStorageService.save()
  } else {
     sessionStorageService.update()
  }
  
})

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

No branches or pull requests

2 participants