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

Move modules to make room for "random" AD scheme #579

Merged
merged 1 commit into from
Jul 26, 2019
Merged

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Jul 25, 2019

Relates to #553

Overview

  • Created modules Cardano.Wallet.Primitive.Address{Derivation,Discovery}.{Random,Sequential}
  • Left the common code in Cardano.Wallet.Primitive.Address{Derivation,Discovery}
  • Renamed test modules along the same lines.
  • Adjust the rest of the application to import AD.Sequential where it was previously importing the AD module.

Comments

  • keyToAddress will have slightly different parameters between Sequential and Random schemes. I haven't done anything to address that, and just left the parameters as they are for Sequential.

Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

Looks good to me! I like the simple interface of Cardano.Wallet.Primitive.AddressDiscovery we get now.

This might be a good opportunity to even more critically examine the documentation / structure of code sections etc. I might have a look.

-- ensure that the following implementation matches with other wallet softwares
-- (like Yoroi/Icarus or the cardano-cli)
-- ensure that the implementations match with other Cardano wallets
-- (like cardano-sl, Yoroi/Icarus, or cardano-cli)
Copy link
Member

Choose a reason for hiding this comment

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

Updating the module level comments ✨

-- schemes implemented are:
--
-- * 'Cardano.Wallet.Primitive.AddressDiscovery.Sequential'
-- * 'Cardano.Wallet.Primitive.AddressDiscovery.Random'
Copy link
Member

Choose a reason for hiding this comment

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

👍

@rvl
Copy link
Contributor Author

rvl commented Jul 26, 2019

Thanks

@rvl rvl merged commit afeaf12 into master Jul 26, 2019
@rvl rvl deleted the rvl/553/ad-moving branch July 26, 2019 00:50
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