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

Add ed25519 public key codec #46

Merged
merged 2 commits into from
May 5, 2017
Merged

Conversation

JustinDrake
Copy link
Contributor

See ipfs/notes#241 (comment)

I suggest using the code 0xed so that's it's visually obvious and easy to remember it's associated with ed25519.

See ipfs/notes#241 (comment)

I suggest using the code `0xed` so that's it's visually obvious and easy to remember it's associated with ed25519.
@JustinDrake
Copy link
Contributor Author

I'm a little bit confused as to whether or not this is a valid varint codec.

@Kubuxu
Copy link
Member

Kubuxu commented Apr 22, 2017

@JustinDrake values in this table are not varint encoded. Varint encoded values might look different.

@JustinDrake
Copy link
Contributor Author

@Kubuxu I see. Then maybe 0xed is not the wisest of choices because when varint encoded I think it will take two bytes instead of just one (because the most significant bit of the first nibble e is 1). What do you think?

@Kubuxu
Copy link
Member

Kubuxu commented Apr 24, 2017

I am ok with it being 2bytes. It is just one byte extra and it is not used for content addressing that frequently (1 byte in case of sha would make a difference).

@whyrusleeping second opinion.

table.csv Outdated
@@ -200,3 +200,4 @@ stellar-tx, Stellar Tx, 0xd1

torrent-info, Torrent file info field (bencoded), 0x7b
torrent-file, Torrent file (bencoded), 0x7c
ed25519, Ed25519 public key, 0xed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change the name to ed25519-pub.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

JustinDrake added a commit to JustinDrake/go-multicodec-packed that referenced this pull request May 5, 2017
@ghost
Copy link

ghost commented May 5, 2017

I think this LGTM

@Kubuxu Kubuxu merged commit 5e3b03e into multiformats:master May 5, 2017
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

Successfully merging this pull request may close these issues.

2 participants