publish genesis state #133

ilya-korotya · 2023-09-12T16:36:03Z

No description provided.

Kolezhniuk · 2023-09-13T07:21:14Z

src/credentials/credential-wallet.ts

+        credentialStatus.id = request.revocationOpts.issuerState
+          ? `${request.revocationOpts.id}/node?state=${request.revocationOpts.issuerState}`
+          : `${request.revocationOpts.id}`;
+        break;


I would expect default keyword which throws Error

For SparseMerkleTreeProof, Iden3commRevocationStatusV1, Iden3ReverseSparseMerkleTreeProof types we have to modify the id field. All other types remain unchanged

Please add default keyword to switch statement which will throw an Error in case for some reasons request.revocationOpts.type none of available cases

why do we need to modify id for Iden3commRevocationStatusV1. This is not right! Agent URL must remain the same.

https://iden3-communication.io/status/overview/#iden3commrevocationstatusv10:~:text=Send%20the%20constructed%20revocation%20status%20request%20message%20in%20plain%20text%20format%20to%20the%20issuer%27s%20agent%20endpoint%2C%20which%20is%20specified%20in%20the%20id%20field.%20The%20communication%20will%20take%20place%20using%20the%20iden3comm%20protocol.

How I understand this regexp cuts end '/'. We should cut the last '/' for the agent endpoint also to be sure that SDK will not create a URL like https/agent//

@Kolezhniuk @vmidylli about the default case. I disagree that we should return an error if we have an unknown type.

What if the user implements a custom revocation type?

We have strict typing to protect against dummy users.

SDK should be flexible. All validation should be on the user side. Because we cannot know all the use cases of this SDK

@ilya-korotya ok, in case we don't need to throw an Error, I would expect default statement in any case where you need to use switch to make logic flow explicitly

default: return credentialStatus;

/** * Builds credential status * @param {CredentialRequest} request * @returns `CredentialStatus` */ private buildCredentialStatus(request: CredentialRequest): CredentialStatus { const credentialStatus: CredentialStatus = { id: request.revocationOpts.id, type: request.revocationOpts.type, revocationNonce: request.revocationOpts.nonce }; switch (request.revocationOpts.type) { case CredentialStatusType.SparseMerkleTreeProof: case CredentialStatusType.Iden3commRevocationStatusV1: return { ...credentialStatus, id: `${request.revocationOpts.id.replace(/\/$/, '')}/${request.revocationOpts.nonce}` }; case CredentialStatusType.Iden3ReverseSparseMerkleTreeProof: return { ...credentialStatus, id: request.revocationOpts.issuerState ? `${request.revocationOpts.id}/node?state=${request.revocationOpts.issuerState}` : `${request.revocationOpts.id}` }; default: return credentialStatus; } }

Kolezhniuk · 2023-09-13T07:26:54Z

src/credentials/status/reverse-sparse-merkle-tree.ts

+          claimsTreeRoot: issuerData.state.claimsTreeRoot
+        }
+      };
+    } else {


you don't need else here

Kolezhniuk · 2023-09-13T07:38:50Z

tests/identity/id.test.ts

@@ -21,6 +21,8 @@ import { CredentialStatusResolverRegistry } from '../../src/credentials';
 import { RHSResolver } from '../../src/credentials';

 describe('identity', () => {
+  const rhsURL = process.env.RHS_URL as string;
+  let credWallet: CredentialWallet;


Please use ICredentialWallet

Kolezhniuk · 2023-09-13T07:44:13Z

tests/identity/id.test.ts

+    const issuerCred = await wallet.issueCredential(issuerDID, claimReq);
+    issuerCred.credentialStatus.id = rhsURL;
+
+    credWallet.getRevocationStatusFromCredential(issuerCred);


I guess await missed here

Kolezhniuk · 2023-09-18T08:48:30Z

src/credentials/credential-wallet.ts

+        credentialStatus.id = request.revocationOpts.issuerState
+          ? `${request.revocationOpts.id}/node?state=${request.revocationOpts.issuerState}`
+          : `${request.revocationOpts.id}`;
+        break;


Please add default keyword to switch statement which will throw an Error in case for some reasons request.revocationOpts.type none of available cases

vmidyllic · 2023-09-18T08:50:28Z

src/credentials/credential-wallet.ts

+        credentialStatus.id = request.revocationOpts.issuerState
+          ? `${request.revocationOpts.id}/node?state=${request.revocationOpts.issuerState}`
+          : `${request.revocationOpts.id}`;
+        break;


why do we need to modify id for Iden3commRevocationStatusV1. This is not right! Agent URL must remain the same.

https://iden3-communication.io/status/overview/#iden3commrevocationstatusv10:~:text=Send%20the%20constructed%20revocation%20status%20request%20message%20in%20plain%20text%20format%20to%20the%20issuer%27s%20agent%20endpoint%2C%20which%20is%20specified%20in%20the%20id%20field.%20The%20communication%20will%20take%20place%20using%20the%20iden3comm%20protocol.

vmidyllic · 2023-09-18T09:02:34Z

src/identity/identity-wallet.ts

+      const revocationTree = await this._storage.mt.getMerkleTreeByIdentifierAndType(
+        did.string(),
+        MerkleTreeType.Revocations
+      );
+
+      const rootOfRootsTree = await this._storage.mt.getMerkleTreeByIdentifierAndType(
+        did.string(),
+        MerkleTreeType.Roots
+      );
+
+      const trees: TreesModel = {
+        state: currentState,
+        claimsTree: claimsTree,
+        revocationTree: revocationTree,
+        rootsTree: rootOfRootsTree
+      };
+      await pushHashesToRHS(currentState, trees, opts.revocationOpts.id);


can't we just call publishStateToRHS here?

Yes. I did it

vmidyllic · 2023-09-21T09:16:56Z

src/credentials/rhs.ts

@@ -35,7 +35,7 @@ export interface TreesModel {
 * @param {number[]} [revokedNonces] - revoked nonces since last published info
 * @returns void
 */
-export async function pushHashesToRHS(
+export async function publishStateToRHS(


are you sure that we want to check public interface here? I think it is better to live it as is.

@vmidyllic we discussed it. And we decided to add export to this function and call it inside on identity-wallet.ts

it was exported already, so this is a problem of renaming already exported function

I returned old name

Kolezhniuk · 2023-09-20T10:01:36Z

src/credentials/credential-wallet.ts

+    };
+
+    switch (request.revocationOpts.type) {
+      case (CredentialStatusType.SparseMerkleTreeProof,


better to write

case CredentialStatusType.SparseMerkleTreeProof: case CredentialStatusType.Iden3commRevocationStatusV1:

Is already fixed

Kolezhniuk · 2023-09-21T15:47:59Z

src/credentials/credential-wallet.ts

+        credentialStatus.id = request.revocationOpts.issuerState
+          ? `${request.revocationOpts.id}/node?state=${request.revocationOpts.issuerState}`
+          : `${request.revocationOpts.id}`;
+        break;


/** * Builds credential status * @param {CredentialRequest} request * @returns `CredentialStatus` */ private buildCredentialStatus(request: CredentialRequest): CredentialStatus { const credentialStatus: CredentialStatus = { id: request.revocationOpts.id, type: request.revocationOpts.type, revocationNonce: request.revocationOpts.nonce }; switch (request.revocationOpts.type) { case CredentialStatusType.SparseMerkleTreeProof: case CredentialStatusType.Iden3commRevocationStatusV1: return { ...credentialStatus, id: `${request.revocationOpts.id.replace(/\/$/, '')}/${request.revocationOpts.nonce}` }; case CredentialStatusType.Iden3ReverseSparseMerkleTreeProof: return { ...credentialStatus, id: request.revocationOpts.issuerState ? `${request.revocationOpts.id}/node?state=${request.revocationOpts.issuerState}` : `${request.revocationOpts.id}` }; default: return credentialStatus; } }

publish genesis state

2660c56

ilya-korotya requested review from Kolezhniuk and vmidyllic September 12, 2023 16:44

Kolezhniuk reviewed Sep 13, 2023

View reviewed changes

fix comments

326f22c

Kolezhniuk requested changes Sep 18, 2023

View reviewed changes

vmidyllic reviewed Sep 18, 2023

View reviewed changes

fix comments

028425e

vmidyllic reviewed Sep 21, 2023

View reviewed changes

call publishStateToRHS instead of pushHashesToRHS

21ee577

Kolezhniuk reviewed Sep 21, 2023

View reviewed changes

fixs

8b8797c

ilya-korotya requested review from vlmold, vmidyllic and Kolezhniuk September 26, 2023 12:56

vmidyllic approved these changes Sep 26, 2023

View reviewed changes

Kolezhniuk approved these changes Sep 26, 2023

View reviewed changes

vmidyllic merged commit 5ffd319 into main Sep 26, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

publish genesis state #133

publish genesis state #133

ilya-korotya commented Sep 12, 2023

Kolezhniuk Sep 13, 2023

ilya-korotya Sep 13, 2023 •

edited

Loading

Kolezhniuk Sep 18, 2023

vmidyllic Sep 18, 2023

ilya-korotya Sep 18, 2023

ilya-korotya Sep 18, 2023

Kolezhniuk Sep 20, 2023

Kolezhniuk Sep 21, 2023

Kolezhniuk Sep 13, 2023

Kolezhniuk Sep 13, 2023

Kolezhniuk Sep 13, 2023

Kolezhniuk Sep 18, 2023

vmidyllic Sep 18, 2023

vmidyllic Sep 18, 2023

ilya-korotya Sep 21, 2023

vmidyllic Sep 21, 2023

ilya-korotya Sep 21, 2023

vlmold Sep 21, 2023

ilya-korotya Sep 21, 2023

Kolezhniuk Sep 20, 2023

ilya-korotya Sep 22, 2023

Kolezhniuk Sep 21, 2023

publish genesis state #133

publish genesis state #133

Conversation

ilya-korotya commented Sep 12, 2023

Choose a reason for hiding this comment

ilya-korotya Sep 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilya-korotya Sep 13, 2023 •

edited

Loading