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

Add goldens for singleAddressFromKey #433

Closed
wants to merge 1 commit into from

Conversation

Anviking
Copy link
Collaborator

@Anviking Anviking commented Jun 18, 2019

#219

Overview

  • I added a bech32KeyToAddrGolden helper and three goldens testing singleAddressFromKey

Comments

  1. I opted to work with bech32 encoded strings instead of the underlying data for easy comparison with jcli.
  2. I would have slightly preferred to test keyToAddress. This would have required constructing a Key 'AddressK XPub from bech32, which is inconvenient. I have therefore opted to only test singleAddressFromKey.

An alternative approach I originally went with but abandoned was to generate the keys properly from mnemonics instead of decoding them from bech32.

@Anviking Anviking self-assigned this Jun 18, 2019
-- ^ corresponding testnet address ("ta...")
-> Text
-- ^ corresponding mainnet address ("ca...")
-> Spec
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reasoning for using Text -> Text -> Text:

  • The human readable parts of the bech32 strings already act as types (just not at compile-time)
  • Allows for easy copy-and-paste at call-site for comparison with jcli

@Anviking Anviking requested a review from KtorZ June 18, 2019 16:29
bech32KeyToAddrGolden
"ed25519_pk1yv0er3wlzvcauqj470tesdlcwy4dll9w7cvsvn4q30678gh44tnsxdh75c"
"ta1sv33lyw9mufnrhsz2hea0xphlpcj4hlu4mmpjpjw5z9ltcaz7k4wwywkd6j"
"ca1qv33lyw9mufnrhsz2hea0xphlpcj4hlu4mmpjpjw5z9ltcaz7k4ww0xfchg"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A caveat is that it is unreadable if you don't know of the meaning of the human readable parts.

A benefit from passing in Text directly is that bech32KeyToAddrGolden has more control, and can output the key in bech32 format despite there being no encodeBech32Key.

Maybe we should add encode/decodeBech32Key in AddressDerivation… 🤔

Copy link
Member

Choose a reason for hiding this comment

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

To make things more readable without impeding too much on the flexibility, you may use a record type and use record fields at the call-site 👍

@Anviking Anviking force-pushed the anviking/219/keyToAddr-goldens branch from da9e6b0 to 4982784 Compare June 18, 2019 16:46
else error $ "failed to decode key: " ++ hrp ++ " is not ed25519_pk"

cc :: ChainCode
cc = ChainCode "<ChainCode shouldn't be used by keyToAddress>"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:s #435

@Anviking Anviking closed this Jun 19, 2019
@KtorZ KtorZ deleted the anviking/219/keyToAddr-goldens branch June 21, 2019 18:55
@KtorZ KtorZ mentioned this pull request Jul 3, 2019
12 tasks
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.

None yet

2 participants