TBD54566975 / web5-go

Apache License 2.0
10 stars 6 forks source link

Add DHT create support #33

Closed mihai-chiorean closed 6 months ago

mihai-chiorean commented 6 months ago

Summary

The main goal of this PR is to implement DHT Create operation, however this comes with a few changes to the diddht package.

The DHT method requires a relay/gateway - hence it makes an HTTP request, so it needs a client. We've opted to have a default client, but also open the API for consumers to pass their own client.

DID DHT API

There are 4 methods exposed by the package for creating and resolving dids:

Resolve API is part of a Resolver which can be instantiated with NewResolver(relayURL string, client *http.Client) *Resolver.

Create API takes some options as input; Available options are:

[!TIP] A possible improvement here is to have a default resolver in the didddht package, which is now implemented in the dids package. This would make the API more consistent with Create

Package structure

The diddht package has some [internal/](https://go.dev/doc/go1.4#internalpackages) packages that are strictly used to implement the DHT method spec.

TODO

mistermoe commented 6 months ago

The DHT method requires a relay/gateway - hence it makes an HTTP request, so it needs a client. We've opted to have a default client, but also open the API for consumers to pass their own client.

do you mean http client here or relay url? in the code i see optionality for relay urls but not for http clients.

KendallWeihe commented 6 months ago

Apologies I'm late to the show here... looking over now... 👀

mihai-chiorean commented 6 months ago

The DHT method requires a relay/gateway - hence it makes an HTTP request, so it needs a client. We've opted to have a default client, but also open the API for consumers to pass their own client.

do you mean http client here or relay url? in the code i see optionality for relay urls but not for http clients.

func WithRelay(relayURL string, client *http.Client) CreateOption { I'd consider this as configuring an optional relay, including custom relay url and client. Or were you referring to having independent defaults for each and being able to "option" just 1 of them?

mistermoe commented 6 months ago

The DHT method requires a relay/gateway - hence it makes an HTTP request, so it needs a client. We've opted to have a default client, but also open the API for consumers to pass their own client.

do you mean http client here or relay url? in the code i see optionality for relay urls but not for http clients.

func WithRelay(relayURL string, client *http.Client) CreateOption { I'd consider this as configuring an optional relay, including custom relay url and client. Or were you referring to having independent defaults for each and being able to "option" just 1 of them?

was referring to the latter

mihai-chiorean commented 6 months ago

The DHT method requires a relay/gateway - hence it makes an HTTP request, so it needs a client. We've opted to have a default client, but also open the API for consumers to pass their own client.

do you mean http client here or relay url? in the code i see optionality for relay urls but not for http clients.

func WithRelay(relayURL string, client *http.Client) CreateOption { I'd consider this as configuring an optional relay, including custom relay url and client. Or were you referring to having independent defaults for each and being able to "option" just 1 of them?

was referring to the latter

Honestly I think the current optionality is good enough, maybe with some extra comment to call out the use of http.DefaultClient and our default relay url

decentralgabe commented 6 months ago

general comment not sure if you already have this - but these two vectors should be covered by tests here https://did-dht.com/#test-vectors