feat: DidDoc versioning [DEV-1892] #443

askolesov · 2022-11-09T08:06:03Z

No description provided.

ankurdotb · 2022-11-09T08:06:06Z

Task linked: DEV-1892 Implement DIDDoc versioning on cheqd node

…into did-versioning

Updated text of tests

Updated test case descriptions

ankurdotb

Comments outside of these ones below (since I didn't get to see the protobufs one before merging / looking at everything else in general):

Resource protobufs

[ x ] Checksum: Why bytes, not a string? I would prefer just storing the SHA256 checksum rather than in bytes (this is a more commonly understood way of doing it) -> Bytes formatted on display
[ x ] Does it make sense to have a namespace property for resources too? -> Let resolver handle it
[ x ] Unsure if I missed this but I think I did show it, apologies if not: an optional URI property in resources with a repeated map of description and URI. One of these will be filled by default with the current resource URI. -> Keep it as also known as, let user define a map with description and URI as properties

DIDs

Transactions

create-did: Maybe change this to expect a file? Instead of passing verification methods and their private keys one by one. (Also rename to create-did instead) -> All identity (not fee etc) provided as a JSON file
Same for update-did

Tests

Looking at the create DID tests, I think it's pretty clear we need perhaps an ADR on update operations and external controllers. The rules implemented for what's allowed vs not for external controllers and updates is up to a DID method to choose, and I'm not a 100% sure some of the current ones are very clear.
Probably need more happy/unhappy path scenarios but this be in a different PR

diddoc_service_test: The services don't have namespace?
diddoc_service_test: "Base DID is not the same as in ID" <- this doesn't have to be the same as the base DID, no rule says so.
Verification method/material tests: No namespace?
Verification method tests: "Id does not have expected DID as a base" why does the base need to be the same?
Tbh I didn't do all of them since I started seeing a lot of scenarios that were confusing

Types

Do we even need to bother keeping the types/v1 folder?

General comments

I skipped a lot of tests near the end since that seems like a beast on its own, and it's probably worth resolving the top-level questions first (since they may be invalid or explainable), and then perhaps revisit the tests. There's also a questions of tests in general vs tests for just versioning.

proto/cheqd/did/v2/genesis.proto

proto/cheqd/did/v2/query.proto

ankurdotb · 2022-11-09T15:49:24Z

proto/cheqd/did/v2/tx.proto

@@ -62,7 +63,8 @@ message MsgUpdateDidDocPayload {
  repeated string key_agreement = 9;
  repeated string also_known_as = 10;
  repeated Service service = 11;
-  string version_id = 12;
+  string previous_version_id = 12;


Is this really necessary?

Remove this

tests/integration/cli/tx.go

x/did/client/cli/tx_deactivate_diddoc.go

x/did/keeper/msg_server_deactivate_diddoc.go

x/did/types/diddoc_metadata.go

ankurdotb · 2022-11-09T17:08:07Z

x/did/types/genesis.pb.go

+	// 325 bytes of a gzipped FileDescriptorProto
+	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x50, 0x41, 0x4b, 0xf3, 0x40,
+	0x10, 0xed, 0x7e, 0x85, 0x4f, 0xbb, 0x4d, 0x45, 0x72, 0xd0, 0xda, 0xc3, 0x12, 0x2a, 0x62, 0x2f,
+	0x26, 0x10, 0xc1, 0x8b, 0xa7, 0x4a, 0xd1, 0x93, 0x22, 0x29, 0x28, 0x78, 0x09, 0xdb, 0xcc, 0xb4,
+	0x5d, 0xb0, 0xd9, 0xd8, 0x1d, 0x83, 0xfe, 0x0b, 0x7f, 0x89, 0xbf, 0xc3, 0x63, 0x8f, 0x1e, 0xa5,
+	0xfd, 0x23, 0xe2, 0x6e, 0xab, 0x16, 0xbc, 0x0c, 0xc3, 0x7b, 0x8f, 0xf7, 0x66, 0x1e, 0x6f, 0x65,
+	0x63, 0x7c, 0x80, 0x08, 0x14, 0x44, 0x65, 0x1c, 0x8d, 0x30, 0x47, 0xa3, 0x4c, 0x58, 0x4c, 0x35,
+	0x69, 0xdf, 0xb3, 0x5c, 0x08, 0x0a, 0xc2, 0x32, 0x6e, 0xed, 0xad, 0x29, 0x41, 0x01, 0xe8, 0xcc,
+	0x09, 0x5b, 0x3b, 0x6b, 0xd4, 0x10, 0xd1, 0xe1, 0xed, 0x92, 0x6f, 0xf7, 0x14, 0xf4, 0x74, 0x76,
+	0x83, 0x53, 0xa3, 0x74, 0xde, 0x47, 0xf2, 0x0f, 0xf8, 0xd6, 0xbd, 0x24, 0x34, 0x94, 0x96, 0x0e,
+	0x6c, 0xb2, 0x80, 0x75, 0x6a, 0x49, 0xc3, 0xa1, 0x4b, 0xa5, 0x7f, 0xca, 0x37, 0x41, 0x41, 0x0a,
+	0x3a, 0x33, 0xcd, 0x7f, 0x41, 0xb5, 0x53, 0x8f, 0x83, 0xf0, 0xf7, 0x39, 0xa1, 0x33, 0xbe, 0x55,
+	0x34, 0xbe, 0x44, 0x92, 0x20, 0x49, 0x26, 0x1b, 0x60, 0x31, 0xd3, 0x7e, 0x65, 0xdc, 0xbb, 0x70,
+	0xaf, 0xf4, 0x49, 0x12, 0xfa, 0xfb, 0xbc, 0xf1, 0xe5, 0x96, 0xcb, 0x09, 0x9a, 0x42, 0x66, 0xb8,
+	0xcc, 0xf4, 0x40, 0xc1, 0xd5, 0x0a, 0xf3, 0xbb, 0xdc, 0x5b, 0x9e, 0x94, 0x1a, 0xa4, 0x55, 0xac,
+	0xf8, 0x2b, 0xf6, 0xe7, 0x9f, 0xa4, 0x5e, 0x7e, 0xef, 0xc6, 0x3f, 0xe1, 0x7c, 0x88, 0x98, 0x16,
+	0x72, 0x2a, 0x27, 0xa6, 0x59, 0x0d, 0x58, 0xa7, 0x1e, 0xef, 0xae, 0x1b, 0x9c, 0x23, 0x5e, 0x5b,
+	0x3a, 0xa9, 0x0d, 0x57, 0xeb, 0x59, 0xf7, 0x6d, 0x2e, 0xd8, 0x6c, 0x2e, 0xd8, 0xc7, 0x5c, 0xb0,
+	0x97, 0x85, 0xa8, 0xcc, 0x16, 0xa2, 0xf2, 0xbe, 0x10, 0x95, 0xbb, 0xc3, 0x91, 0xa2, 0xf1, 0xe3,
+	0x20, 0xcc, 0xf4, 0x24, 0x72, 0x2d, 0xdb, 0x79, 0x94, 0x6b, 0xc0, 0xe8, 0xc9, 0x56, 0x4e, 0xcf,
+	0x05, 0x9a, 0xc1, 0x7f, 0x5b, 0xf9, 0xf1, 0x67, 0x00, 0x00, 0x00, 0xff, 0xff, 0xed, 0xeb, 0xc7,
+	0x91, 0xd1, 0x01, 0x00, 0x00,
+}
+


What is this and why does it have such a weird name?

This is internals of generated proto. Looks like it keeps bytes of the original .proto file. Hard to say why they do it.

It's a way to pass .proto field definitions typed to descriptors, as bytes and parse them later, during the decoding process regardless of language.

askolesov · 2022-11-15T06:40:04Z

I've fixed all notes except tests. Let's review tests in a separate PR.

Do we even need to bother keeping the types/v1 folder?

That's the whole point of migration. We need to:

Read old value
Decode it using old models <- that't where v1 is used
Transcode to v2 models
Persist

Eengineer1 · 2022-11-15T12:15:19Z

And actually v1 is an essential part of the migration / upgrade tests PR.

Eengineer1

@askolesov Looks great. Thanks!

Left some concerns regarding the immediate impact on migration / upgrade test suite.

Eengineer1 · 2022-11-15T12:37:59Z

tests/integration/cli/tx.go

@@ -51,80 +52,90 @@ func Tx(module, tx, from string, txArgs ...string) (sdk.TxResponse, error) {
 	return resp, nil
 }

-func CreateDidDoc(payload types.MsgCreateDidDocPayload, signInputs []cli.SignInput, from string) (sdk.TxResponse, error) {
+func CreateDidDoc(tmpDit string, payload types.MsgCreateDidDocPayload, signInputs []cli.SignInput, from string) (sdk.TxResponse, error) {


@askolesov Simple typo fix.

askolesov added 4 commits November 7, 2022 19:18

Diddoc versioning implementation

98b0609

Fix unit tests

a132d83

Fix integration tests

0dc7483

Fix linter isssues

19c7c4a

askolesov and others added 4 commits November 9, 2022 13:21

Merge branch 'develop' into did-versioning

3bc2eaf

Implement getAllDidDocVersions query

ae6a00c

Cleanup

5586068

Merge branch 'did-versioning' of https://github.com/cheqd/cheqd-node …

5c0e5a1

…into did-versioning

askolesov requested review from lampkin-diet, Eengineer1 and ankurdotb November 9, 2022 10:11

ankurdotb added 7 commits November 9, 2022 16:16

Fix typo in latestVersionIterator

b9a578d

Update create DID test descriptions

59a2f3c

Fix typo in CreateDidDocWithExternalControllers

3c4afbe

Updated test descriptions for update

29ad40c

Update descriptions for DID deactivate tests

3982a6d

Update diddoc_diddoc_test.go

0f252b3

Updated text of tests

Update diddoc_service_test.go

8151e52

Updated test case descriptions

ankurdotb requested changes Nov 9, 2022

View reviewed changes

ankurdotb and others added 5 commits November 9, 2022 17:20

VM test descriptions

d827cd8

Update query.proto

cd43704

Remove previous_version_id + rename msgs

3d09553

Improve resource -> also know as property

da4ebba

Fix integration tests

f6f8225

askolesov requested a review from ankurdotb November 15, 2022 06:36

Merge branch 'develop' into did-versioning

8a69318

askolesov merged commit 512423a into develop Nov 15, 2022

askolesov deleted the did-versioning branch November 15, 2022 12:43

Eengineer1 approved these changes Nov 15, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: DidDoc versioning [DEV-1892] #443

feat: DidDoc versioning [DEV-1892] #443

askolesov commented Nov 9, 2022

ankurdotb commented Nov 9, 2022

ankurdotb left a comment •

edited

Loading

ankurdotb Nov 9, 2022

ankurdotb Nov 10, 2022 •

edited

Loading

askolesov Nov 15, 2022

ankurdotb Nov 9, 2022

askolesov Nov 11, 2022

Eengineer1 Nov 15, 2022

askolesov commented Nov 15, 2022

Eengineer1 commented Nov 15, 2022

Eengineer1 left a comment

Eengineer1 Nov 15, 2022

feat: DidDoc versioning [DEV-1892] #443

feat: DidDoc versioning [DEV-1892] #443

Conversation

askolesov commented Nov 9, 2022

ankurdotb commented Nov 9, 2022

ankurdotb left a comment • edited Loading

Choose a reason for hiding this comment

Resource protobufs

DIDs

Transactions

Tests

Types

General comments

ankurdotb Nov 9, 2022

Choose a reason for hiding this comment

ankurdotb Nov 10, 2022 • edited Loading

Choose a reason for hiding this comment

askolesov Nov 15, 2022

Choose a reason for hiding this comment

ankurdotb Nov 9, 2022

Choose a reason for hiding this comment

askolesov Nov 11, 2022

Choose a reason for hiding this comment

Eengineer1 Nov 15, 2022

Choose a reason for hiding this comment

askolesov commented Nov 15, 2022

Eengineer1 commented Nov 15, 2022

Eengineer1 left a comment

Choose a reason for hiding this comment

Eengineer1 Nov 15, 2022

Choose a reason for hiding this comment

ankurdotb left a comment •

edited

Loading

ankurdotb Nov 10, 2022 •

edited

Loading