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.

Sign up for GitHub

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

Jump to bottom

Change pubkey/privatekey type name from ostracon to tendermint #445

Closed
1 of 4 tasks
zemyblue opened this issue Jul 22, 2022 · 4 comments · Fixed by #447
Closed
1 of 4 tasks

Change pubkey/privatekey type name from ostracon to tendermint #445

zemyblue opened this issue Jul 22, 2022 · 4 comments · Fixed by #447
Assignees
@torao
Labels
C: proposal Classification: Proposal for specification, algorithm, architecture, or communication

Comments

@zemyblue
Copy link
Member

zemyblue commented Jul 22, 2022 •
edited
Loading

Summary

Change pubkey/privatekey type name from ostracon to tendermint

Problem Definition

And maybe we need to change protobuf data or path.

Proposal

We changed the key type name from #287 . But this change make to be difficult to use other cosmos-sdk compatibility tools for example cosmjs, keplr, pingpub and etc. But if we revert this type name, we can use it without any porting.

So I proposal this change.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@zemyblue zemyblue changed the title Change pubkey/privatekey type name from ostracon to `tendermint Change pubkey/privatekey type name from ostracon to tendermint Jul 22, 2022
@torao torao added the C: proposal Classification: Proposal for specification, algorithm, architecture, or communication label Jul 22, 2022
@torao torao assigned Kynea0b and torao and unassigned Kynea0b Jul 22, 2022
torao added a commit to torao/ostracon that referenced this issue Jul 26, 2022
@torao
change PubKey/PrivKey prefixes of "ostracon/" to "tendermint (Finschi…
4eb4729 
…a#445)

1. The private key may remain "pstracon/PrivKeyXxx" as it's never exposed to outside world. However, it makes no sense to keep the ostracon prefix even with asymmetric naming, so it should be modified with "tendermint/PrivKeyXxx"
2. BLS12 and Composite key types are not compatible with current Cosmos assets to begin with.  To make this clear, these Ostracon-specific prefixes are left as "ostracon/".
@torao torao mentioned this issue Jul 26, 2022
Merged
@torao
Copy link
Contributor

torao commented Jul 26, 2022 •
edited
Loading

@zemyblue
For BLS and Composite keys, I think it best to leave the ostracon/ prefix to distinguish Ostracon-specific features since BLS keys (and Composite keys introduced in relation to BLS keys), among etc., aren't compatible with the Cosmos SDK tools, to begin with. If they'll try to support the BLS and Ostracon implementation is adopted, then they would support ostracon/ key names.

Can the modified PR be tested with cosmjs, keplr and other modules mentioned above to make sure the modification works well before merging it into main branch?

@zemyblue
Copy link
Member Author

@zemyblue I think it best to leave the ostracon/ prefix to distinguish Ostracon-specific features since BLS keys (and Composite keys introduced in relation to BLS keys), among etc., aren't compatible with the Cosmos SDK tools, to begin with. If they'll try to support the BLS and Ostracon implementation is adopted, then they would support ostracon/ key names.

Can the modified PR be tested with cosmjs, keplr and other modules mentioned above to make sure the modification works well before merging it into main branch?

Thank you. I also think so. We don't need to change the BLS's key name, composite keys also.

And I am testing we can use cosmjs without modifying. When I tested in last two days, we can use cosmjs directly, but we need to add plugin options for the Ostracon's validator_info. In the Ostracon's validator_info, staking_power is added that is difference with Tendermint. Tendermint's validator_info has voting_power. I need more testing.

@torao
Copy link
Contributor

torao commented Jul 26, 2022

I see. So we can't confirm it works yet now, can it?
Then I’ll merge PR #445 into main branch after @kokeshiM0chi approves it.

@zemyblue
Copy link
Member Author

Yes, we can. I think #447 PR does not have any problem. And I already ready the PR to apply this change in lbm-sdk. So, I also think no problem it's good to merge after approving enough.

torao added a commit that referenced this issue Jul 27, 2022
@torao
change PubKey/PrivKey prefixes of "ostracon/" to "tendermint (#445) (#…
18bdbd7 
…447)

1. The private key may remain "pstracon/PrivKeyXxx" as it's never exposed to outside world. However, it makes no sense to keep the ostracon prefix even with asymmetric naming, so it should be modified with "tendermint/PrivKeyXxx"
2. BLS12 and Composite key types are not compatible with current Cosmos assets to begin with.  To make this clear, these Ostracon-specific prefixes are left as "ostracon/".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@torao torao

Labels
C: proposal Classification: Proposal for specification, algorithm, architecture, or communication
Projects
None yet
Milestone
No milestone
3 participants
@torao @zemyblue @Kynea0b