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

Port and review the Address discovery (AddressPool + AddressPoolGap modules) from cardano-wallet #22

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

Port and review the Address discovery (AddressPool + AddressPoolGap modules) from cardano-wallet #22

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

Comments

@KtorZ
Copy link
Member

KtorZ commented Mar 4, 2019 •
edited
Loading

Context

Similarly to #21, we do want to port existing logic from cardano-wallet-legacy onto cardano-wallet, from the following modules:

Also, we did initially embed in AddressPool some logic to create new addresses. This could actually be removed in order to make the AddressPool a plain data-structure that can be easily serialized / deserialized (much more like done in: https://github.com/KtorZ/wallet-prototype/blob/master/app/Main.hs#L719-L730

We do not need any particular abstraction on how addresses are derived at this stage and we can only consider AddressPool and AddressPoolGap in the context of sequential derivation with BIP-44.

Decision

  • We must port modules described above to cardano-wallet and remove any dependency to cardano-sl
  • We should merge AddressPool and AddressPoolGap into one module
  • We should remove unnecessary abstraction over the address derivation (as described above)

Development Plan

  • I intend to import and merge together the AddressPoolGap and AddressPool modules from cardano-wallet, alongside with their testing code.
  • I plan to align types with primitives and wrappers provided in AddressDerivation & Cardano.Wallet.Primitive
  • I intend to make AddressPool a plain data-structure (as it is in the wallet prototype) to be more idiomatic haskell.

PR

Number Base
#51 KtorZ/#21/ed25519-address-derivation

QA

  • The module define two main data-structure (cf haddock

    • AddressPoolGap: to represent the number of consecutive undiscovered addresses
    • AddressPool: to keep track of a set of addresses for a given account and change chain

  • Properties are define for both structures as follows:

    • AddressPoolGap
    • AddressPool
      The ones for AddressPoolGap are rather straightforward and just make sure that our wrapper type has the correct properties and define correct bounds. Not that, I've removed the Num, Real and Integral instances from the legacy code because this was just giving too much power to the wrapper, and wasn't really needed after all. An Enum instance is enough in most cases.
@KtorZ KtorZ added this to the Support Wallet Creation milestone Mar 4, 2019
@KtorZ KtorZ self-assigned this Mar 11, 2019
KtorZ added a commit that referenced this issue Mar 11, 2019
@KtorZ
refactoring #22 | review pushNextBlocks indentation
8adc703
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
This was referenced Mar 11, 2019
Merged
Merged
KtorZ added a commit that referenced this issue Mar 13, 2019
@KtorZ
Merge pull request #51 from input-output-hk/KtorZ/#22/port-address-di…
b554949 
…scovery

Address Discovery (Sequential)
@piotr-iohk
Copy link
Contributor

lgtm! 👍

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

@KtorZ KtorZ

Labels
None yet
Projects
None yet
Milestone
Support Wallet Creation
Development

No branches or pull requests
2 participants
@KtorZ @piotr-iohk