Contact not added correctly after a previous deletion

[lenkan]

Steps to reproduce

1. Create wallet with identifier X and authorize the agent end role
2. Create wallet with identifier Y and authorize the agent end role
3. Resolve the OOBI of identifier Y from the wallet of identifier X, use an alias to ensure that the contact is added
4. GET /contacts/{prefix} => verify that the correct alias and id is returned.
5. DELETE /contacts/{prefix}
6. Resolve the OOBI of identifier Y again, also using an alias.
7. GET /contacts/{prefix} => verify that the correct alias and id is returned.

Actual result

HTTP 404 is returned.

Notes

See reproduction https://github.com/nordlei/vlei-sandbox/blob/main/src/issues/contact-not-added-after-deletion.test.ts

git clone [email protected]:nordlei/vlei-sandbox.git
cd vlei-sandbox
docker compose run --rm --build test npm start src/issues/contact-not-added-after-deletion.test.ts

It seems like the contact attributes are removed from the contact.

See test

import { beforeAll, expect, test } from "vitest";
import { TestWallet } from "./test-wallet.ts";
import { randomUUID } from "crypto";

const wallet1 = new TestWallet({
  alias: "alias1",
});

const wallet2 = new TestWallet({
  alias: "alias2",
});

function sleep(ms: number) {
  return new Promise((resolve) => setTimeout(resolve, ms));
}

beforeAll(async () => {
  await Promise.all([wallet1.init(), wallet2.init()]);
});

test("Resolve OOBI, verify contact", async () => {
  let operation = await wallet1.client.oobis().resolve(await wallet2.generateOobi(), wallet2.identifier.name);
  operation = await wallet1.client.operations().wait(operation);

  expect(operation).toMatchObject({ done: true });

  expect(await wallet1.client.contacts().get(wallet2.identifier.prefix)).toMatchObject({
    id: wallet2.identifier.prefix,
    alias: wallet2.identifier.name,
  });
});

test("Delete contact, resolve OOBI, get contact", async () => {
  await wallet1.client.contacts().delete(wallet2.identifier.prefix);

  let operation = await wallet1.client.oobis().resolve(await wallet2.generateOobi(), wallet2.identifier.name);
  operation = await wallet1.client.operations().wait(operation);

  expect(operation).toMatchObject({ done: true });

  // Need to add this sleep for the contact to become available, even though the operation is finished
  // await sleep(1000);

  expect(await wallet1.client.contacts().get(wallet2.identifier.prefix)).toMatchObject({
    id: wallet2.identifier.prefix,
    alias: wallet2.identifier.name,
  });
});

It looks like I need to add a delay after the operation has resolved before the contact is properly added to the list of contacts.

[iFergal]

The problem here is the long running operation is related to the OOBI and not the contact. The OOBI has been previously resolved (and is never removed (we don't unresolve for contact deletion)) so the operation is done: True immediately - while the actual second client call to oobi().resolve() is still processing again and creating the new contact.

I think the cleanest solution might be to have a new OpTypes.contact that is returned if the alias is specified instead of OpTypes.oobi. What do you think?

[lenkan]

The problem here is the long running operation is related to the OOBI and not the contact. The OOBI has been previously resolved (and is never removed (we don't unresolve for contact deletion)) so the operation is done: True immediately - while the actual second client call to oobi().resolve() is still processing again and creating the new contact.

I think the cleanest solution might be to have a new OpTypes.contact that is returned if the alias is specified instead of OpTypes.oobi. What do you think?

Thanks for putting some thought into it! I agree that would be cleaner. I think we can even take it one step further even - what if the "resolve oobi" is completely independent from contacts. If you want to add a contact you would POST / PUT a contact resource that contains alias and AID. The creation of contact would fail if the oobi is not yet resolved.

This is what we have to do often times anyway if we want to add more attributes to the contact.

If we do it that way, the resolving of an oobi would never affect the contact database.

What do you think?

[iFergal]

Yeah I wouldn't be opposed to that since the second call would be necessary anyway for the attributes, and the OOBI resolution is idempotent if it fails before creating the contact.

This is inherited from keripy though so we'd have to see if it makes sense with @pfeairheller and others.

[2byrds]

From the dev meeting today: There is a desire to distill keripy to the reference impl functionality. Contacts is not part of KERI and could be managed in KERIA. This is a similar conversation to other keripy related code like kli, witness, etc. It is a matter of community members contributing that migration out of keripy and into another project. If it is something useful across multiple projects (keripy users, KERIA users, etc) then a new intermediate project that could be used by all, is preferred. Perhaps a shared or monorepo could help to separate but compose these components.

[iFergal]

For now at minimum we could create the new op type. But if we are moving towards removing contacts from KERI in the long term, I think it makes sense to stop mixing OOBIs and contacts already as @lenkan has suggested.

[iFergal]

@lenkan I looked at this yesterday a little more closely. The POST/PUT contact APIs are synchronous and once you get the 200 everything is done, so there's no op type.

I could add a new op type, but not sure it's the right move until we know if OOBIs and contacts will be decoupled. Would rather not add it and then remove it later.

You could mimic it for now by not passing an alias to the OOBI resolution, and using POST to create it.

[kentbull]

The three PRs below are related to this PR, especially the quote by @lenkan

It seems like the contact attributes are removed from the contact.

• https://github.com/WebOfTrust/keripy/pull/944 (to 1.1.31)
• https://github.com/WebOfTrust/keripy/pull/942 (to 1.2.4)
• https://github.com/WebOfTrust/keripy/pull/943 (to main)

The OOBI resolution API, like Fergal mentioned, is not specific to a given identifier. This is because it is keystore-wide (Habery-wide) in KERIpy which is then inherited by KERIA for an Agent. Each Agent creates a Habery. And the contact data, other than the prefix and the OOBI URL, was being replaced by Organizer.replace() on every re-resolution of the OOBI. The use of Organizer.update() does a proper merge on re-resolve which preserves any contact data for a given prefix in the Habery database.

Since we have not yet decoupled contact management from OOBI resolution works with the PRs I made are you, @lenkan , comfortable closing this issue or is there something else that is broken?

I suggest that if we want to have the new Op type for contacts that we capture that as a separate feature request rather than leave this open if that is what is leaving this issue open.

[iFergal]

Given the normal contacts API is synchronous, I'm not sure it's worth adding an op type for contacts that are updated via an OOBI resolution, and might just be confusing.

I changed my code to always strip the alias from the URL, and update the contact directly once the OOBI resolution is done.