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

Review multibase vs base58 key representation #3

Closed
OR13 opened this issue Sep 18, 2020 · 11 comments
Closed

Review multibase vs base58 key representation #3

OR13 opened this issue Sep 18, 2020 · 11 comments

Comments

@OR13
Copy link
Contributor

OR13 commented Sep 18, 2020

@dlongley I recall you may have objected to the idea of publicKeyMultibase vs publicKeyBase58.

Would love to hear your thoughts.

My initial thoughts are that publicKeyMultibase might be easier to align with IPLD in the future.

@peacekeeper
Copy link
Contributor

Is Example 4 correct?

If you have

"publicKeyBase58": "dbDmZLTWuEYYZNHFLKLoRkEX4sZykkSLNQLXvMUyMB1",

then shouldn't that be equivalent to

"publicKeyMultibase": "zdbDmZLTWuEYYZNHFLKLoRkEX4sZykkSLNQLXvMUyMB1",

?

But the example currently seems to show two completely different key pairs.

@dlongley
Copy link
Contributor

dlongley commented Nov 25, 2020

I don't mind publicKeyMultibase ... I'm just worried about people needing to support all your base. If we do publicKeyMultibase, someone is going to start using things other than base58 and this will lead to annoying interop problems. We need some kind of brakes on what gets supported.

For example -- we could say that the linked data key type can say which bases are supported (ideally just one).

@OR13
Copy link
Contributor Author

OR13 commented Nov 30, 2020

Agree, I think throwing an error in the library when an unsupported base is detected is the best solution.

And let the signature suite itself define the supported bases, so that support for a suite automatically causes libraries to support popular bases.

@peacekeeper
Copy link
Contributor

But again, Example 4 shows two different keys, even though they both have the id https://example.com/issuer/123#key-0, right?

If someone confirms that, I could try to fix the example(s).

@OR13
Copy link
Contributor Author

OR13 commented Dec 2, 2020

IIRC they are supposed to be the same key encoded 2 different ways, or that was my intention.

the z tells you its base58btc multicodec. My intention was to provide examples that showed how to upgrade an Ed25519VerificationKey2018.

It looks like I did the math wrong.

@OR13
Copy link
Contributor Author

OR13 commented Dec 2, 2020

its also possible that the prefixing shifted the base58 encoding of the binary, and thats why the character strings look different.

@OR13
Copy link
Contributor Author

OR13 commented Dec 2, 2020

the desired behavior is to show both encodings for the same binary key material.

@clehner
Copy link
Member

clehner commented Jun 3, 2021

Hi @OR13, @peacekeeper,
I notice this encoding issue also.

The example publicKeyMultibase and privateKeyMultibase values are doubly-encoded with multibase/base58btc. Decoding them yields what I think are the correct values, that is the original base58 strings with "z" prepended.

@OR13
Copy link
Contributor Author

OR13 commented Jun 3, 2021

@clehner I suspect the test vectors are incorrect, I built them a long time ago, and AFAIK nobody has tested them until you :)

Please open a PR to correct them, I am also unsure if the byte prefix for private key is correct... be careful.

@clehner clehner mentioned this issue Jun 4, 2021
@clehner
Copy link
Member

clehner commented Jun 4, 2021

@OR13 opened: #10. I find the VC test vector correct after fixing the multibase encoding. The VP I had to regenerate.
The private key is 64 bytes, a common encoding of a Ed25519 private key, where the last 32 bytes is the public key.

@msporny
Copy link
Member

msporny commented Jan 29, 2023

Looks like publicKeyMultibase won. There are no plans to keep publicKeyBase58 going AFAIK:

Closing.

@msporny msporny closed this as completed Jan 29, 2023
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

5 participants