DEV-1074: Add DID resolution implementation #1

Toktar · 2022-03-25T10:31:46Z

No description provided.

lampkin-diet · 2022-04-13T08:45:34Z

main.go

+	requestService := services.NewRequestService(ledgerService)
+
+	// Routes
+	e.GET("/identifier/:did", func(c echo.Context) error {


Should we create a const variable here? Or maybe as a config parameter

The recommended way of doing this in Echo is to define a list of routes as a JSON file. https://echo.labstack.com/guide/routing/#list-routes

This keeps things extensible

lampkin-diet · 2022-04-13T08:47:13Z

main.go

+	})
+
+	// Start server
+	e.Logger.Fatal(e.Start(":1313"))


I guess this value is a port number, maybe the same issue? Config parameter or const.

Yes I agree: define what backends you need to connect to and what ports you expose using Docker compose.

main.go

services/request_service.go

services/diddoc_service.go

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

ankurdotb

@anikitinDSR Could you please bring in the Universal Resolver Driver folders in here as well and start incorporating that in?

ankurdotb · 2022-04-13T11:38:17Z

config.yml

@@ -0,0 +1,4 @@
+networks: 
+    mainnet: rpc.cheqd.net:443
+    testnet: 127.0.0.1:9090


Testnet will be grpc.cheqd.network:443

ankurdotb · 2022-04-13T11:42:15Z

main.go

+	requestService := services.NewRequestService(ledgerService)
+
+	// Routes
+	e.GET("/identifier/:did", func(c echo.Context) error {


The recommended way of doing this in Echo is to define a list of routes as a JSON file. https://echo.labstack.com/guide/routing/#list-routes

This keeps things extensible

ankurdotb · 2022-04-13T11:44:32Z

main.go

+	})
+
+	// Start server
+	e.Logger.Fatal(e.Start(":1313"))


Yes I agree: define what backends you need to connect to and what ports you expose using Docker compose.

ankurdotb · 2022-04-13T11:49:37Z

main.go

+	var cfg Config
+	decoder := yaml.NewDecoder(f)
+	err = decoder.Decode(&cfg)
+	if err != nil {
+		return Config{}, err
+	}
+
+	return cfg, nil
+}


Maybe do this in Docker? Do you need it internally?

config.yml will be passed from docker-compose as a config parameter.

ankurdotb · 2022-04-13T11:56:04Z

types/resolution_metadata.go

+		contentType,
+		resolutionError,
+		time.Now().UTC().Format(time.RFC3339),
+	}


This should be XMLDatetime, not sure of this output will be that or not

Yes, it is XMLDatetime
I used the same way for Cheqd Node: https://github.com/cheqd/cheqd-node/blob/9e07d8e61b304eb0883c2bdfce63fc1b4a23a381/x/cheqd/types/stateValue.go#L35

…1074-dokerfile Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Make a dockerfile for did-resolver

…1074-did-resolver

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

…1074-dokerfile Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

ghost · 2022-04-14T16:20:55Z

main.go

+	})
+
+	// Start server
+	e.Logger.Fatal(e.Start(*didResolutionPort))


Can we add an IP to listen on? This will remove the need of having a reverse proxy server to serve the traffic?
What I mean is this:
e.Logger.Fatal(e.Start("0.0.0.0"+*didResolutionPort))

And let's put this value into config as well

ghost · 2022-04-14T16:23:01Z

docker-compose.yml

+    build:
+      dockerfile: Dockerfile


The docker-compose doesn't seem to work, since we need a context field here. I don't think we need to explicitly define which Dockerfile to use, since it uses the default Dockerfile name, and that should be the only one we're gonna use.
I think this is all we need:
build: .

Hey! Does this changes cover you request?
https://github.com/cheqd/cheqd-did-resolver/pull/3/files
Especially about docker-compose file

I'm not so good with Docker Swarm. I think that docker-compose, by default, doesn't support copying/mounting files into containers. Maybe handling this through the COPY command in the Dockerfile is not so bad in this case, since we can update the image when something changes.
Alternatively, maybe we could set fixed values to the network endpoints, and use env vars to overwrite them? It gets much easier to overwrite values like this during the deployment process.

askolesov

Good job! Thanks. The only more thing is I wouldn't leave commented pieces of code. Great work anyway.

go.mod

main.go

services/ledger_service.go

services/request_service.go

types/resolution_metadata.go

[DEV-1074] Dockerfile improvements

…1074-did-resolver

askolesov

Thanks for the changes. Few things from me:

Running as a root in docker is a security issue.
Should we allow DIDUrls?

Cool tests btw!

.github/workflows/build.yml

docker/Dockerfile

main.go

services/ledger_service.go

services/request_service.go

askolesov

Thanks for the edits! LGTM

DEV-1074: Add base structure

898a6a7

Toktar marked this pull request as draft March 25, 2022 10:31

Toktar added 3 commits April 5, 2022 19:57

DEV-1074: Added DIDDoc marshalling

fe3b4a7

DEV-1074: add gRPC connection func

ce77811

DEV-1074: fix gRPC error, update tests

2db1d21

lampkin-diet reviewed Apr 13, 2022

View reviewed changes

services/diddoc_service.go Outdated Show resolved Hide resolved

[DEV-1074] Make a dockerfile for did-resolver

9290a99

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

ankurdotb reviewed Apr 13, 2022

View reviewed changes

Toktar and others added 7 commits April 13, 2022 19:32

DEV-1074: add app parameters

daed391

Merge remote-tracking branch 'origin/dev-1074-did-resolver' into DEV-…

57eea48

…1074-dokerfile Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] add docker-compose

6e804f9

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

DEV-1074: open ports in docker compose

d4c23e4

Merge pull request #2 from cheqd/DEV-1074-dokerfile

8d7e9a8

[DEV-1074] Make a dockerfile for did-resolver

DEV-1074: fix "not found" status issue

d48c0be

Merge remote-tracking branch 'origin/dev-1074-did-resolver' into dev-…

dc4a663

…1074-did-resolver

Toktar marked this pull request as ready for review April 14, 2022 13:22

lampkin-diet approved these changes Apr 14, 2022

View reviewed changes

Andrew Nikitin added 10 commits April 14, 2022 17:21

[DEV-1074] Impove docker-compose

9673e94

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Remove

074ffaf

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

Merge remote-tracking branch 'origin/dev-1074-did-resolver' into DEV-…

ea088ed

…1074-dokerfile Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Add a comment

75483eb

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Add github actions

f580e9b

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] debug

c5c8047

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Fix workflow calling

d739d0e

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Another attempt

abaa859

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Change workflow name

bc719ae

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

[DEV-1074] Remove unused action

35b34cf

Signed-off-by: Andrew Nikitin <andrew.nikitin@cheqd.io>

ghost reviewed Apr 14, 2022

View reviewed changes

askolesov reviewed Apr 14, 2022

View reviewed changes

Toktar added 4 commits April 15, 2022 12:04

Merge pull request #3 from cheqd/DEV-1074-dokerfile

e394b5f

[DEV-1074] Dockerfile improvements

DEV-1074: improve error handling

1b91d7c

Merge remote-tracking branch 'origin/dev-1074-did-resolver' into dev-…

bf30610

…1074-did-resolver

DEV-1074: Add request_service_test.go

cc5c589

askolesov self-requested a review April 19, 2022 12:05

askolesov reviewed Apr 19, 2022

View reviewed changes

.github/workflows/build.yml Outdated Show resolved Hide resolved

docker/Dockerfile Outdated Show resolved Hide resolved

main.go Outdated Show resolved Hide resolved

services/ledger_service.go Show resolved Hide resolved

services/request_service.go Outdated Show resolved Hide resolved

Toktar and others added 6 commits April 19, 2022 17:41

DEV-1074: Fix docker security issue nad other small changes

87d2b34

DEV-1074: Fix DID validation

ce94577

Add configuration reading from .env file

9c60d38

DEV-1074: Update Dockerfile

9ffa81c

DEV-1074: fix bag in configs

24c31bb

DEV-1074: fix a bag in error handling

60b8005

askolesov approved these changes Apr 20, 2022

View reviewed changes

Toktar merged commit f54ec2f into main Apr 20, 2022

askolesov deleted the dev-1074-did-resolver branch May 13, 2022 09:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEV-1074: Add DID resolution implementation #1

DEV-1074: Add DID resolution implementation #1

Toktar commented Mar 25, 2022

lampkin-diet Apr 13, 2022

ankurdotb Apr 13, 2022

lampkin-diet Apr 13, 2022

ankurdotb Apr 13, 2022

ankurdotb left a comment

ankurdotb Apr 13, 2022

ankurdotb Apr 13, 2022

ankurdotb Apr 13, 2022

ankurdotb Apr 13, 2022

lampkin-diet Apr 14, 2022

ankurdotb Apr 13, 2022

Toktar Apr 13, 2022

ghost Apr 14, 2022

askolesov Apr 15, 2022

ghost Apr 14, 2022

lampkin-diet Apr 14, 2022

ghost Apr 14, 2022

askolesov left a comment

askolesov left a comment

askolesov left a comment

DEV-1074: Add DID resolution implementation #1

DEV-1074: Add DID resolution implementation #1

Conversation

Toktar commented Mar 25, 2022

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

ankurdotb left a 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

askolesov left a comment

Choose a reason for hiding this comment

askolesov left a comment

Choose a reason for hiding this comment

askolesov left a comment

Choose a reason for hiding this comment