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: revoke in issuer #918

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions packages/credentials/src/issuer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ import type { CTypeLoader } from './ctype/index.js'
import type { IssuerOptions, SubmitOverride } from './interfaces.js'

export type { IssuerOptions }
import * as Kilt from '@kiltprotocol/sdk-js'
import * as KiltChain from '@kiltprotocol/chain-helpers'
Copy link
Contributor

Choose a reason for hiding this comment

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

As with the other PR, no imports from sdk-js here and no blanket imports.

Copy link
Author

Choose a reason for hiding this comment

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

Done at: ccac9b9

import type {
KiltAddress,
SignerInterface,
KeyringPair,
DidUrl,
} from '@kiltprotocol/types'
import {authorizeTx} from "@kiltprotocol/did"
Copy link
Contributor

Choose a reason for hiding this comment

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

make sure you run prettier for auto-formatting

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: ccac9b9

import { base58Decode } from '@polkadot/util-crypto'

/**
* Creates a new credential document as a basis for issuing a credential.
Expand Down Expand Up @@ -134,3 +144,64 @@ export async function issue({
)
}
}

/**
* Revokes a Kilt credential on the blockchain, making it invalid.
*
* @param attester The DID URL of the attester revoking the credential
* @param submitterAccount The account that will submit and pay for the transaction
* @param signCallback Callback function to sign the DID authorization
* @param credential The Verifiable Credential to be revoked
* @returns An object containing:
* - success: Boolean indicating if revocation was successful
* - error?: Array of error messages if revocation failed
* - info: Object containing blockchain transaction details:
* - blockNumber?: Block number where transaction was included
* - blockHash?: Hash of the block
* - transactionHash?: Hash of the transaction
*/
export async function revoke(
attester: DidUrl,
submitterAccount: KeyringPair,
signCallback: any,
credential: VerifiableCredential,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to only see two parameters here:

Suggested change
attester: DidUrl,
submitterAccount: KeyringPair,
signCallback: any,
credential: VerifiableCredential,
credential: VerifiableCredential,
issuer: IssuerOptions,

This is enough for issuance, and will also be enough for revocation.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: ccac9b9

): Promise<{success: boolean, error?: Array<string>, info: {blockNumber?: string, blockHash?: string, transactionHash?: string}}> {
try {
const api = Kilt.ConfigService.get('api')
const rootHash = credential.id?.split(':').pop()!
const decodedroothash = base58Decode(rootHash);

const revokeTx = api.tx.attestation.revoke(decodedroothash, null) as any
const [submitter] = (await Kilt.getSignersForKeypair({
keypair: submitterAccount,
type: 'Ed25519',
})) as Array<SignerInterface<'Ed25519', KiltAddress>>

const authorizedTx = await authorizeTx(
attester,
revokeTx,
signCallback,
submitter.id
)

const response = await KiltChain.Blockchain.signAndSubmitTx(authorizedTx, submitterAccount) as Object
const responseObj = JSON.parse(JSON.stringify(response));

return {
success: true,
info: {
blockNumber: responseObj.blockNumber,
blockHash: responseObj.status.finalized,
transactionHash: responseObj.txHash
}
}

} catch (error: unknown) {
const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred';
return {
success: false,
error: [errorMessage],
info: {}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to make sure we can support future credential- and proof types without breaking the function interfaces, this function should be set up to detect what kind of revocation status method we are dealing with, and if it's a of the KiltRevocationStatusV1 type, call the right logic to revoke the credential.

Can you move the implementation to packages/credentials/src/V1/KiltRevocationStatusV1.ts, and in the function here only check which type of credentialStatus method we are dealing with, call the function implemented in KiltRevocationStatusV1.ts if it's the right one, and error if not? the issue function above should give you an idea.

Btw all the info you need for revocation is contained in the credentialStatus id, which is composed as follows:
${chainId}/kilt:attestation/${base58Encode(rootHash)}
Because we have the chain id here as well, which contains the genesis hash of the chain on which the credential was attested, you can even check if we are connected to the right network (i.e., peregrine vs spiritnet)! See the check function in KiltRevocationStatusV1.ts for an example.

Copy link
Author

Choose a reason for hiding this comment

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

I am working on a second commit for this.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: 110a5cc

}
18 changes: 18 additions & 0 deletions tests/bundle/bundle-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,24 @@ async function runAll() {

console.log('Did deactivated')

// ┏━━━━━━━━━━━━━━━━━━┓
// ┃ Revoke credential┃
// ┗━━━━━━━━━━━━━━━━━━┛
//
// Revoke a previously issued credential on chain
const revokeCredentialResult = await Kilt.Issuer.revoke(
didDocument.id,
submitter,
signers,
credential
)

if (!revokeCredentialResult.success) {
throw new Error(`revoke credential failed: ${revokeCredentialResult.error?.join(', ')}`)
}

console.log('credential revoked')

// Release the connection to the blockchain.
await api.disconnect()

Expand Down
Loading