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 nextBlocks function #41

Merged
merged 1 commit into from
Mar 13, 2019
Merged

Add nextBlocks function #41

merged 1 commit into from
Mar 13, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Mar 8, 2019

Relates to #12.

Overview

The nextBlocks function is a member of the MonadChainProducer class.

The interface will need tweaking to work efficiently.

There is also a NetworkLayer type (copied from prototype) which the RustCardano backend uses.

Some slotting functions needed to be added to deal with fetching the correct range of blocks.

-> SlotId -- ^ Start point
-> SlotId -- ^ Latest slot available
-> [EpochIndex]
epochRange numBlocks (SlotId startEpoch startSlot) (SlotId tipEpoch _)
Copy link
Member

Choose a reason for hiding this comment

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

One issue I can think of:

Although we know how many slots are in an epoch, we don't know how many blocks are contained in a particular epoch (because any given slot may be missing its block). Or in other words, blocks and slots are not in 100% correspondence.

Isn't it therefore possible for this function to underestimate the range of epochs required?

Perhaps if the function were to state its assumption, that it only counts based on slots, and not blocks, then it would be clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you're right. It assumes slots not blocks.

src/Cardano/ChainProducer/RustHttpBridge/Client.hs Outdated Show resolved Hide resolved
src/Servant/Extra/ContentTypes.hs Show resolved Hide resolved
src/Servant/Extra/ContentTypes.hs Show resolved Hide resolved
test/unit/Cardano/ChainProducer/RustHttpBridgeSpec.hs Outdated Show resolved Hide resolved
src/Cardano/Wallet/Slotting.hs Outdated Show resolved Hide resolved
@rvl rvl changed the title WIP Add nextBlocks function Add nextBlocks function Mar 11, 2019
@rvl rvl marked this pull request as ready for review March 11, 2019 09:40
paweljakubas added a commit that referenced this pull request Mar 11, 2019
[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 pull request Mar 11, 2019
[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 pull request Mar 11, 2019
[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 pull request Mar 11, 2019
[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
@rvl rvl force-pushed the jskrvl/12/next-blocks branch 3 times, most recently from 06282fc to 06ff422 Compare March 12, 2019 03:58
Copy link
Member

@jonathanknowles jonathanknowles 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, though still need to bring the style in line with https://github.com/input-output-hk/cardano-wallet/wiki/Coding-Standards

src/Cardano/ChainProducer/RustHttpBridge/Client.hs Outdated Show resolved Hide resolved
src/Cardano/ChainProducer/RustHttpBridge/Client.hs Outdated Show resolved Hide resolved
src/Cardano/ChainProducer/RustHttpBridge/NetworkLayer.hs Outdated Show resolved Hide resolved
src/Cardano/ChainProducer/RustHttpBridge/NetworkLayer.hs Outdated Show resolved Hide resolved
src/Cardano/Wallet/Slotting.hs Outdated Show resolved Hide resolved
test/unit/Cardano/Wallet/BlockSyncerSpec.hs Outdated Show resolved Hide resolved
test/unit/Cardano/Wallet/SlottingSpec.hs Outdated Show resolved Hide resolved
src/Cardano/ChainProducer/RustHttpBridge/Client.hs Outdated Show resolved Hide resolved
@rvl rvl force-pushed the jskrvl/12/next-blocks branch 2 times, most recently from 5cb48bb to 2a23cda Compare March 13, 2019 01:14
This is a member of the MonadChainProducer class.

The interface will need tweaking to work efficiently.

There is also a NetworkLayer type (copied from prototype) which the
RustCardano backend uses.

Some slotting functions needed to be added to deal with fetching the
correct range of blocks.
@rvl rvl merged commit 1baea59 into master Mar 13, 2019
@rvl rvl deleted the jskrvl/12/next-blocks branch March 13, 2019 01:52
jamescheuk91 pushed a commit to jamescheuk91/cardano-sl that referenced this pull request Mar 28, 2019
3980: Bump cardano-crypto r=KtorZ a=akegalj

Fixes deriveXPub: bip32 hd wallet public key derivation
IntersectMBO/cardano-crypto#52

## Description

<!--- A brief description of this PR and the problem is trying to solve -->

This is a bump of `cardano-crypto` bug IntersectMBO/cardano-crypto#52 . It was discovered as a part of a task on `cardano-wallet` cardano-foundation/cardano-wallet#41

## Linked issue

<!--- Put here the relevant issue from YouTrack -->
cardano-foundation/cardano-wallet#41


Co-authored-by: Ante Kegalj <akegalj@gmail.com>
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