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

feat: add new DID runtime APIs to aid with DID deletion #852

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

ntn-x2
Copy link
Member

@ntn-x2 ntn-x2 commented Jan 20, 2025

Fixes https://github.com/KILTprotocol/ticket/issues/3742.

The PR introduces two new runtime APIs: one that returns an enum of "linked resources", which some clients might find useful to understand why a DID deletion failed, and one that returns a list of RuntimeCalls, that could be extended with a did.delete() call, put together into a utility.batch*, and submitted to the chain, and be sure the DID deletion will work as expected (as long as all the DID origin requirements are met).

@ntn-x2 ntn-x2 self-assigned this Jan 20, 2025
@ntn-x2 ntn-x2 requested review from Ad96el and rflechtner and removed request for Ad96el January 20, 2025 13:54
Copy link
Contributor

@rflechtner rflechtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable! The enums can serve as the missing error cases to show to users via a UI to help users identify which clean up steps they need to perform. And for the transactions I would assume the client just needs logic to batch them in a DID-origin transaction, which the sdk already has. Did you try this out?

Comment on lines +31 to +33
Web3NameAccount(LinkableAccountId),
/// An account linked to the DID and resolvable by or to a Dotname.
DotNameAccount(LinkableAccountId),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's because we have the two instances of the linking pallet?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly.

@ntn-x2
Copy link
Member Author

ntn-x2 commented Jan 27, 2025

And for the transactions I would assume the client just needs logic to batch them in a DID-origin transaction, which the sdk already has. Did you try this out?

Correct, as long as the batch also includes the did.delete tx at the end as well, which is of course not included in the result of this runtime call. I did test this locally with a peregrine-based chopsticks instance, but notice this won't work with the standalone runtime that the SDK uses for integration tests: or rather, the runtime call will always return [] which means the delete operation will never fail.

As for testing, I modified locally the Web3name integration test should be possible to remove a w3n by the owner did with the following, which worked as expected:

const deletionCalls: [SubmittableExtrinsic] =
      await api.call.did.linkedResourcesDeletionCalls(
        Did.toChain(w3nCreator.uri)
      )

    const tx = api.tx.utility.batchAll([
      // Old call
      // api.tx.web3Names.releaseByOwner(),
      ...deletionCalls,
      api.tx.did.delete(0),
    ])
    const authorizedTx = await Did.authorizeTx(
      w3nCreator.uri,
      tx,
      w3nCreatorKey.getSignCallback(w3nCreator),
      paymentAccount.address
    )
    await submitTx(authorizedTx, paymentAccount)

Commeting out the ...deletionCalls and only leaving api.tx.did.delete(0) fails with a CannotDelete error, which is also expected.

I modified the type definitions locally with the following:

const tmpCalls = Object.assign(Object.assign({}, batchedDidApiCalls), { linked_resources: {
        description: '',
        params: [
            {
                name: 'did',
                type: 'AccountId32',
            },
        ],
        type: 'Vec<LinkedDidResource>',
    }, linked_resources_deletion_calls: {
        description: '',
        params: [
            {
                name: 'did',
                type: 'AccountId32',
            },
        ],
        type: 'Vec<RuntimeCall>',
    }});

and properly defined the LinkedDidResource type. I will work on a PR that adds these to the repo.

EDIT: PR for the types repo: https://github.com/KILTprotocol/types-augment/pull/26.

@ntn-x2 ntn-x2 force-pushed the aa/did-deletion-api branch from 471c1d0 to cbf2228 Compare January 27, 2025 12:23
@ntn-x2 ntn-x2 force-pushed the aa/did-deletion-api branch from cbf2228 to 2483ab0 Compare January 29, 2025 08:32
@ntn-x2 ntn-x2 merged commit 3ff5264 into develop Jan 29, 2025
2 checks passed
@ntn-x2 ntn-x2 deleted the aa/did-deletion-api branch January 29, 2025 08:32
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

Successfully merging this pull request may close these issues.

3 participants