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

Remove hex as we can always use bech32 #282

Closed
paweljakubas opened this issue Nov 27, 2024 · 4 comments · Fixed by #291
Closed

Remove hex as we can always use bech32 #282

paweljakubas opened this issue Nov 27, 2024 · 4 comments · Fixed by #291
Assignees
Labels
haskell PR:ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG PR:IMPROVING CODE Mark a PR as an improvement, for auto-generated CHANGELOG

Comments

@paweljakubas
Copy link
Collaborator

paweljakubas commented Nov 27, 2024

Context

When using the library it was reported by several users in private communication that is not so seamless to convert between bech32 and hex as it could be. They stressed that it would be nice to have (a) uniform support through key command and also (b) separately be able to convert both ways.

Decision

Let's create command that accepts any key through STDIN

cardano-address key convert --hex
cardano-address key convert --bech32 prefix

Here also, for public keys --with-chain-code and without-chain-code.
For private keys -signing-key and chain-code.

Additionally we can adopt the approach done aleeady for private keys:

$ cardano-address key private --signing-key < drep.xsk
drep_sk1vpdsm49smzmdwhd4kjmm2mdyljjysm746rafjr7r8kgfanj849psw8pfm305g59wng0akw3qzppmfh6k5z7gx66h2vppu022m4eqajg5xmwma

$ cardano-address key private --signing-key --hex < drep.xsk
605b0dd4b0d8b6d75db5b4b7b56da4fca4486fd5d0fa990fc33d909ece47a943071c29dc5f4450ae9a1fdb3a201043b4df56a0bc836b5753021e3d4add720ec9

$ cardano-address key private --chain-code < drep.xsk
5a1df4df66655a4a9e5157d9212173b8e993b493baa5cf2db5a3c7dc98d8df3c

ie. add everywhere in key command ability of --hex and support additionally --bech32 prefix.And do this for all key commands and make sure the support is uniform. If those options are missing then bech32 prefix as according to CIP will be provided.

Acceptance Criteria


Development

QA

@abailly
Copy link
Contributor

abailly commented Nov 28, 2024

There's already a package and command-line tool available for manipulating bech32 encoding. Instead of incorporating this feature in the codebase, leading to more code to ship and maintain, could we not provide guidance to users in error messages or documentation?

@paweljakubas
Copy link
Collaborator Author

indeed, bech32 works fine. I use it everyday. I was thinking about wrapping it under the hood of proposed key convert command. It might be superfluous as people are already accustomed to bench32 (myself included). Nevertheless, I would still go with adding the --hex functionality where missing and might be of use. For example, below

cardano-address key private --signing-key --hex < drep.xsk
cardano-address key hash --hex < .pay.xvk

works, but there are some places where it is missing, like

cardano-address key child
cardano-address key from-recovery-phrase
cardano-address key public

Here, we should add optional FormatType and get the same experience. That would be my proposal.

@paweljakubas paweljakubas added haskell PR:IMPROVING CODE Mark a PR as an improvement, for auto-generated CHANGELOG PR:ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG labels Nov 28, 2024
@abailly
Copy link
Contributor

abailly commented Nov 28, 2024

OK, so the idea is to make commands more consistent? Then why not remove --hex parameter and point the user towards bech32? This is less work, reduces the amount of code we maintain, improves consistency of commands, and aligns better with unix philosophy (do one thing well, compose through input/output).

@paweljakubas
Copy link
Collaborator Author

hmm, having bech32 available and working perfectly, I start to incline towards cleaning and slimming cardano-addresses out of --hex . Very "linux in mind compability" proposal 💯 I am happy to handle that 👍

@paweljakubas paweljakubas self-assigned this Jan 2, 2025
@paweljakubas paweljakubas changed the title Add capability to convert between bech32 and hex Remove hex as we can always use bech32 Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
haskell PR:ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG PR:IMPROVING CODE Mark a PR as an improvement, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants