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

Porting and cleaning up Cardano.Mnemonic module (incl. corresponding tests) #16

Closed
KtorZ opened this issue Mar 4, 2019 · 1 comment
Closed

Porting and cleaning up Cardano.Mnemonic module (incl. corresponding tests) #16

KtorZ opened this issue Mar 4, 2019 · 1 comment
Assignees
@paweljakubas
Milestone
Support Wallet Cr...

Comments

@KtorZ
Copy link
Member

KtorZ commented Mar 4, 2019 •
edited
Loading

Context

As part of this rewrite of the wallet backend, we do not want to depend on any of the code from cardano-sl; Yet, there are a few bits that were made in a fairly isolated manner that are still very relevant to the wallet This is the case of:

https://github.com/input-output-hk/cardano-sl/tree/develop/mnemonic

Minus a few bits that are strictly related to cardano-sl like:

Decision

We do want to port this module onto cardano-wallet, review the style to make it consistent with our coding standards, and remove the bits that aren't relevant to this new code. We do want to make this a module of cardano-wallet (and not a package as it is now), under Cardano.Wallet.Mnemonic.

Tests should also share the same faith and be ported in their rightful place.

Development Plan

PR

Number Base
#47 master
#58 master

QA

The PR introduces Cardano.Wallet.Mnemonics module which delivers mnemonic creation. From the point of view of testing there are two aspects :
(a) unit tests that are in located in Cardano.Wallet,MnemonicsSpec module.
(b) executable that uses this and generates Mnemonic 15 mnemonics

$ stack build
[pawel@arch cardano-wallet]$ stack exec cardano-generate-mnemonic
["love","dutch","usual","uphold","whale","certain","merry","horror","bulb","injury","school","crane","grass","police","beauty"]
@KtorZ KtorZ added this to the Support Wallet Creation milestone Mar 4, 2019
@paweljakubas paweljakubas self-assigned this Mar 8, 2019
KtorZ added a commit that referenced this issue Mar 11, 2019
@KtorZ
refactoring #16 | remove unecessary IO in 'writeToIORefAction'
05af556
paweljakubas added a commit that referenced this issue Mar 11, 2019
@paweljakubas
[3] Adding bracket
69e2083 
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes
paweljakubas added a commit that referenced this issue Mar 11, 2019
@paweljakubas
[3] Adding bracket
a423b02 
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes

[3] hlint suggestion
paweljakubas added a commit that referenced this issue Mar 11, 2019
@paweljakubas
[3] Adding bracket
33cd68f 
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes

[3] hlint suggestion

[3] add .weeder.yaml to omit duplicateMaybes and groups to be announce during weeder execution

[3] undo weeder ignore file plus remove pragma and unnecessary exports
paweljakubas added a commit that referenced this issue Mar 11, 2019
@paweljakubas
[3] Adding bracket
a2f3c36 
[3] Adding ticking function test and downloading block logic

[3] Block syncer working ok plus additional tests

[3] Cleaning the code

[3] cabal fix

[3] Complete refactoring to the review

[3] Final refactoring

[3] cabal fix

[3] hlint and weeding

[3] add killing thread at the end of test

[3] Replace IORef with MVar

refactoring #1 | inline intermediate functions to see more clearly + remove debug console prints

refactoring #2 | review indentation of 'where' clause

refactoring #3 | inline loop and remove test intrumentation from test logic

refactoring #4 | define generator for ticking args in a declarative manner

refactoring #5 | purify tickingFunctionTest and make it a monadic property

refactoring #6 | review naming in Arbitrary TickingArgs

refactoring #7 | use guards in mkConsecutiveTestBlocks

refactoring #8 | define single block generator from previous block

refactoring #9 | use 'fromPreviousBlock' and start loop with an already initialize list

refactoring #10 | purify mkConsecutiveTestBlocks by defining a test hash function

refactoring #11 | switch argument positions in mkConsecutiveTestBlocks

refactoring #12 | replace loop with built-in list 'iterate'

refactoring #13 | define Arbitrary instance for creating consecutive blocks

refactoring #14 | replace mkConsecutiveBlocks with a property parameter

refactoring #15 | use 'newMVar' instead or 'newEmptyMVar' + 'putMVar'

refactoring #16 | remove unecessary IO in 'writeToIORefAction'

refactoring #17 | replace takeMVar + putMVar with modifyMVar

refactoring #18 | review naming for 'writeToIORefAction' --> 'reader'

refactoring #19 | use a 'Map.lookup' instead of 'List.filter' + pattern-match

refactoring #20 | remove 'BlocksConsumed' wrapper

refactoring #21 | generalize reader with polymorphic parametrism

refactoring #22 | review pushNextBlocks indentation

refactoring #23 | group case pattern matches using tuple

refactoring #24 | remove 'Hash BlockHeader' from the block to inject

refactoring #25 | use synchronization lock instead of computed times

refactoring #26 | Move generation of duplicated blocks onto 'Arbitrary Blocks'

refactoring #27 | remove 'chunkSizes' in a favor of inline random selection

refactoring #28 | remove 'DeliveryMode' in favor of the most general case

refactoring #29 | cleanup wrapper types

refactoring #30 | generalize pushNextBlocks with parametric polymorphism

refactoring #31 | rename pushNextBlocks to 'writer'

refactoring #32 | define reader on Block instead of BlockHeaderHash

refactoring #33 | replace old reader with reader'

refactoring #34 | move creation of writer MVar inside writer action

refactoring #35 | remove header hash from 'Blocks'

refactoring #36 | rename 'consecutiveBlocks' into 'blocks'

refactoring #37 | re-organize module to separate effectful logic from declarations

refactoring #38 | move waiting logic into dedicated function

refactoring #39 | move 'done' and 'readerChan' initialization into reader and writer

refactoring #40 | use Millisecond instead of Second for shorter tests

refactoring #41 | Move creation of blocks from writer to 'Arbitrary Blocks'

[3] fix line width

[3] aligning the code with other code changes

[3] hlint suggestion

[3] add .weeder.yaml to omit duplicateMaybes and groups to be announce during weeder execution

[3] undo weeder ignore file plus remove pragma and unnecessary exports

[3] remove not needed imports
@paweljakubas paweljakubas mentioned this issue Mar 11, 2019
Merged
4 tasks
@piotr-iohk piotr-iohk mentioned this issue Mar 13, 2019
Merged
1 task
@piotr-iohk
Copy link
Contributor

lgtm 👍
(I have added few more tests and adjusted API spec in #58)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@paweljakubas paweljakubas

Labels
None yet
Projects
None yet
Milestone
Support Wallet Creation
Development

No branches or pull requests
3 participants
@KtorZ @paweljakubas @piotr-iohk