[2.0.0-dev] when outputting K1 public keys, always use `PUB_K1_` format #1561

spoonincode · 2025-05-29T04:13:15Z

Change nodeos/cleos/keosd to output K1 public keys in the PUB_K1_ format instead of the "legacy" EOS... format. Legacy K1 public keys that begin with EOS are still accepted as input, but spring will only output these keys as PUB_K1.... This effects everything from cleos wallet keys to nodeos' RPC replies (which means even an old cleos will still display keys as PUB_K1... since nodeos will have returned those part of its reply).

Two commands have been added to cleos convert: print_public_key_as_legacy and print_public_key_as_new. These commands will always print the given key in the respective format. (only K1 keys different between legacy and new).

Work on #1518

Related: VaultaFoundation/mandel#44 EOSIO/eos#5155

heifner · 2025-05-29T12:30:32Z

This LGTM.

An ability to convert between the two formats seems like it would be useful. I would expect people to want a easy way to convert the EOS... to PUB_K1.... Not sure anyone would want the other way. Seems like a cleos feature, not a spring-util.

linh2931 · 2025-05-29T12:44:54Z

This LGTM.

An ability to convert between the two formats seems like it would be useful. I would expect people to want a easy way to convert the EOS... to PUB_K1.... Not sure anyone would want the other way. Seems like a cleos feature, not a spring-util.

Why not spring-util? The conversion does not need nodeos to be running.

heifner · 2025-05-29T13:00:55Z

Why not spring-util? The conversion does not need nodeos to be running.

I see spring-util as a tool for node operators. I see cleos as a tool for general users. Converting of keys is a general user feature.

spoonincode · 2025-05-30T15:16:52Z

I tend to think cleos is the right spot too. Interestingly we already have a convert subcommand group that is named appropriately for where we could stuff it in

spoonincode · 2025-05-30T20:24:54Z

programs/cleos/main.cpp

+   });
+   // output public key in new format
+   auto public_key_to_new = convert->add_subcommand("print_public_key_as_new", localized("Output a public key in new format (PUB_K1, PUB_R1, PUB_WA)"));
+   public_key_to_new->add_option("public_key", public_key_to_convert, localized("Public key to output in new format"))->required();


I wanted to come up with something better then 'new format' but 🤷‍♀️

generic ?

I think print_ is a bit verbose. Maybe convert public_key_as_legacy. Seems like print is implied.

Does convert imply that only one input format is allowed? We can get around naming the "new format" entirely by saying something like convert_public_key_from_legacy and convert_public_key_to_legacy. I was initially a little worried about using this phrasing because I want convert_public_key_from_legacy to pass through "new format" keys too (i.e. it doesn't require the input key to be legacy), because that makes it more useful in scripting environments imo

Usage: ./cleos convert [OPTIONS] SUBCOMMAND Options: -h,--help Print this help message and exit --help-all Show all help Subcommands: pack_transaction From plain signed JSON to packed form unpack_transaction From packed to plain signed JSON form pack_action_data From JSON action data to packed form unpack_action_data From packed to JSON action data form

Existing convert is what it is converting to.
To be consistent I think it would be:

legacy_public_key From existing public key to legacy form, i.e. EOS.. public_key From existing public key to canonical form, i.e. PUB_..

Other ideas instead of canonical: new, generic, standard, current

What I don't like about "new", "current", "updated", etc is that it implies it's.. well.. new. But it's not new it has been there since 1.0. "standard" may imply something more broadly standardized (think 'industry standard') so I don't think that's the best choice either.
So I would probably lean to "canonical". My one concern with it is that in this ball park there is already the concept of 'canonical signatures' and maybe that's overusing the word but oh well -- if it's most appropriate it's most appropriate.
I also considered "normalized" but I think canonical is still more appropriate than that.

doesn't seem right. just let exceptions escape (and cause failure) instead

spoonincode · 2025-06-02T20:21:03Z

There are quite a few EOS... keys in the documentation; and in bios boot tut. I should modify these as part of this PR

heifner · 2025-06-04T14:21:36Z

I don't see any reason this can't target main instead of release/2.0.

spoonincode · 2025-06-04T15:16:37Z

Don't think it's too breaking for a minor release? I'm not sure I can think of anything downstream it'll specifically break. Anyone who does some sort of string validation that a public key is EOS will have a problem, but hopefully no one is doing that any longer. Since 1.3/1.4 would not be a required upgrade it may be surprising for users to encounter difference responses on different endpoints if they continue to use some 1.0-1.2 endpoints. (but that's something that would have occurred during 2.0 upgrade too, though more limited in time where there would be a mix of 1.x and 2.0)

You can bring it up in the team call.

heifner · 2025-06-04T15:21:48Z

Don't think it's too breaking for a minor release?

Actually, I forgot the default changed. I was thinking only of the new cleos functionality. Probably better to wait until 2.0.

spoonincode · 2025-06-04T15:33:33Z

huh well yeah we could still split out the cleos convert functionality to main to be able to point to that as part of 1.3 deprecation notice. But I'm not sure it's really worth it.

heifner · 2025-06-04T15:34:52Z

huh well yeah we could still split out the cleos convert functionality to main to be able to point to that as part of 1.3 deprecation notice. But I'm not sure it's really worth it.

Probably not, lived without it this long.

linh2931

Please update the PR description, as now the commands are cleos convert legacy_public_key and cleos convert public_key.

linh2931 · 2025-06-05T14:07:32Z

Maybe it is too late. I feel the command names cleos convert legacy_public_key and cleos convert public_key do not tell whether they are converting to or from legacy_public_key / public_key. cleos convert to_legacy_public_key and cleos convert to_public_key more clear? Or cleos convert to legacy_public_key and cleos convert to public_key? But the to adds an extra level of convert. We can even have a convert from for other use cases in the future.

spoonincode · 2025-06-05T14:29:27Z

Not too late for changes I still need to go through and handle this (I expected to do this sooner which is why I didn't move to draft but now I'm not sure when I'll come back around to finish it off)

There are quite a few EOS... keys in the documentation; and in bios boot tut. I should modify these as part of this PR

Maybe it is too late. I feel the command names cleos convert legacy_public_key and cleos convert public_key do not tell whether they are converting to or from legacy_public_key / public_key. cleos convert to_legacy_public_key and cleos convert to_public_key more clear? Or cleos convert to legacy_public_key and cleos convert to public_key? But the to adds an extra level of convert. We can even have a convert from for other use cases in the future.

Yeah it's tough... I feel like currently it's no worse than existing subcommands here though. convert pack_transaction -- without knowing anything else does that convert from or to a packed transaction? The help text clarifies what exactly is from and to. And the name of the subcommand is what it is converted to. So we're consistent in that regard.

linh2931 · 2025-06-05T15:04:35Z

Not too late for changes I still need to go through and handle this (I expected to do this sooner which is why I didn't move to draft but now I'm not sure when I'll come back around to finish it off)

There are quite a few EOS... keys in the documentation; and in bios boot tut. I should modify these as part of this PR

Maybe it is too late. I feel the command names cleos convert legacy_public_key and cleos convert public_key do not tell whether they are converting to or from legacy_public_key / public_key. cleos convert to_legacy_public_key and cleos convert to_public_key more clear? Or cleos convert to legacy_public_key and cleos convert to public_key? But the to adds an extra level of convert. We can even have a convert from for other use cases in the future.

Yeah it's tough... I feel like currently it's no worse than existing subcommands here though. convert pack_transaction -- without knowing anything else does that convert from or to a packed transaction? The help text clarifies what exactly is from and to. And the name of the subcommand is what it is converted to. So we're consistent in that regard.

Yes, the existing convert commands do not indicate to/from which sometimes is confusing. Maybe for new commands we introduce a new convert_to subcommand?

spoonincode · 2025-06-05T16:23:47Z

But subcommands are shown in cleos --help so having both a convert and convert_to listed there seems bloaty and noisy. I thought it was really nice how we were able to stuff these new public key conversions in to an existing top level subcommand that makes contextual sense for them -- it didn't expand the top level subcommand list any

spoonincode · 2025-06-10T16:24:27Z

Not too late for changes I still need to go through and handle this (I expected to do this sooner which is why I didn't move to draft but now I'm not sure when I'll come back around to finish it off)

actually now that I stumbled on accounts.json I'll do a separate PR that scrubs all legacy keys, since it'll be touching dozens of megabytes

spoonincode added 2 commits May 28, 2025 16:36

output public k1 keys with PUB_K1_ prefix

cc6801f

update tests with new PUB_K1 default

5997070

spoonincode added 2 commits May 30, 2025 15:17

add public_key::to_legacy_string()

a146cc4

add cleos commands to convert between legacy and new pubkey format

99549f1

spoonincode commented May 30, 2025

View reviewed changes

spoonincode marked this pull request as ready for review May 30, 2025 20:26

spoonincode added 2 commits June 2, 2025 13:55

use 'canonical' term for the "new" key format

243b262

don't mess with testSuccessful in public_key_convert_test()

ec6f8ca

doesn't seem right. just let exceptions escape (and cause failure) instead

heifner approved these changes Jun 2, 2025

View reviewed changes

linh2931 approved these changes Jun 5, 2025

View reviewed changes

spoonincode merged commit 5eb0162 into release/2.0 Jun 23, 2025
28 checks passed

spoonincode deleted the PUBK1_20 branch June 23, 2025 21:46

[2.0.0-dev] when outputting K1 public keys, always use PUB_K1_ format #1561

[2.0.0-dev] when outputting K1 public keys, always use PUB_K1_ format #1561

Uh oh!

Conversation

spoonincode commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

heifner commented May 29, 2025

Uh oh!

linh2931 commented May 29, 2025

Uh oh!

heifner commented May 29, 2025

Uh oh!

spoonincode commented May 30, 2025

Uh oh!

spoonincode May 30, 2025

Choose a reason for hiding this comment

Uh oh!

heifner May 30, 2025

Choose a reason for hiding this comment

Uh oh!

heifner May 30, 2025

Choose a reason for hiding this comment

Uh oh!

spoonincode Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

heifner Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

spoonincode Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

spoonincode commented Jun 2, 2025

Uh oh!

heifner commented Jun 4, 2025

Uh oh!

spoonincode commented Jun 4, 2025

Uh oh!

heifner commented Jun 4, 2025

Uh oh!

spoonincode commented Jun 4, 2025

Uh oh!

heifner commented Jun 4, 2025

Uh oh!

linh2931 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linh2931 commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spoonincode commented Jun 5, 2025

Uh oh!

linh2931 commented Jun 5, 2025

Uh oh!

spoonincode commented Jun 5, 2025

Uh oh!

spoonincode commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[2.0.0-dev] when outputting K1 public keys, always use `PUB_K1_` format #1561

[2.0.0-dev] when outputting K1 public keys, always use `PUB_K1_` format #1561

spoonincode commented May 29, 2025 •

edited

Loading

linh2931 left a comment •

edited

Loading

linh2931 commented Jun 5, 2025 •

edited

Loading