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

Use []byte for service and instance hash (and all hash in the future) #1062

Closed
krhubert opened this issue Jun 18, 2019 · 5 comments
Closed

Comments

@krhubert
Copy link
Contributor

krhubert commented Jun 18, 2019

In go all hashes are represented with []byte. We should use []byte as type in source code. What is motivation for using string, as this is clearly the opposite direction as all golang packages (standard and non-standard)?

In the API receiving/returning string is fine, but otherwise []byte is the default type for such data.

@NicolasMahe
Copy link
Member

NicolasMahe commented Jun 18, 2019

I agree that internally (like in database) it should be []byte and on the API it should be string.

I would like to keep the encoding/decoding of string to byte inside the Engine. This should be done by helper function to not mix with hash in hex and hash in base58.

@NicolasMahe
Copy link
Member

NicolasMahe commented Jun 19, 2019

Actually it will be nicer to use only one encode/decode function. I really like base58 because it's the most compact with DNS valid characters.
@mesg-foundation/mesg-core-team do you agree? base58 for all hash?

@NicolasMahe NicolasMahe changed the title Use []byte for service and instance hash (and all hasesh in the future) Use []byte for service and instance hash (and all hash in the future) Jun 19, 2019
@krhubert
Copy link
Contributor Author

Also, does it make still make sense to use sid in grpc api if we will have an instance? How sid should behave then?

@antho1404
Copy link
Member

I think sid should disappear from the apis and apis should only accept hash, it's up to the client to resolve the sid to hash.
Either check all the services and filter the one with the sid and get the list of instances for this service and pick the wanted hash or we could provide api helper if needed.
Ultimately this sid should not be part of the service but more a name service on top of the services and services are only accessible with hash

@NicolasMahe
Copy link
Member

Closing this issue because PR #1072 has been merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants