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 Byron Bech32 support #2605

Merged
merged 1 commit into from
Apr 14, 2021
Merged

Remove Byron Bech32 support #2605

merged 1 commit into from
Apr 14, 2021

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Apr 14, 2021

We use the prefix to distinguish keys, but according to the code, Byron and Shelley share the same prefix:

instance SerialiseAsBech32 (VerificationKey ByronKey) where
    bech32PrefixFor         _ =  "addr_xvk"
    bech32PrefixesPermitted _ = ["addr_xvk"]
instance SerialiseAsBech32 (VerificationKey PaymentExtendedKey) where
    bech32PrefixFor         _ =  "addr_xvk"
    bech32PrefixesPermitted _ = ["addr_xvk"]

We use the following list when deserialising:

    bech32Types =
      [ FromSomeType (AsVerificationKey AsByronKey)
                     AByronVerificationKey
      , FromSomeType (AsVerificationKey AsPaymentKey)
                     APaymentVerificationKey
      , FromSomeType (AsVerificationKey AsPaymentExtendedKey)
                     APaymentExtendedVerificationKey
      ]

So when deserialising a key like addr_xvk18359zklg0x68ru6yhxyvsv3slt6wscs00mzsc45jrv9840dfutj5uxg6z6rp4w2pany4kjgg5tly8s5xv7fuyq5tf8kw9ehr7cy4lzcenyhck, it thinks the keys are Byron because Byron appears first in the list and shares the same prefix as Shelley.

Having an instance for Byron declare that Byron keys have the addr_xvk prefix seems wrong. This PR removes support for serialising and deserialising Byron keys with that prefix.

@newhoggy
Copy link
Contributor Author

This fixes #2596

With this change the following works as expected:

$ cardano-cli address build --payment-verification-key addr_xvk18359zklg0x68ru6yhxyvsv3slt6wscs00mzsc45jrv9840dfutj5uxg6z6rp4w2pany4kjgg5tly8s5xv7fuyq5tf8kw9ehr7cy4lzcenyhck --testnet-magic 1097911063
addr_test1vptgn6nxuhfc0rt3s7xydxlczm2rncn7dhcqt4jp4d9edqqax2mf6%
$ cardano-cli address info --address addr_test1vptgn6nxuhfc0rt3s7xydxlczm2rncn7dhcqt4jp4d9edqqax2mf6

Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

So now the only way to use Byron keys is via a file (using text envelope format) right?

@newhoggy
Copy link
Contributor Author

So now the only way to use Byron keys is via a file (using text envelope format) right?

Yep.

@newhoggy
Copy link
Contributor Author

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 14, 2021

@iohk-bors iohk-bors bot merged commit 4fc844c into master Apr 14, 2021
@iohk-bors iohk-bors bot deleted the remove-byron-bech32-support branch April 14, 2021 15:24
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.

3 participants