Add block decoder #28

rvl · 2019-03-05T05:50:30Z

Relates to #11

Overview

Copy and slightly modify the CBOR block decoder from the prototype.
Add some tests using mainnet blocks.
This branch takes the Cardano.Wallet.Primitive module from Wallet Primitive Types #25, so that should be merged first, then this branch rebased.

Todo

Remove TODO line from decoders.
Add test for a testnet block.
~~Add test for Yoroi style addresses~~ (actually is ok, because addresses aren't decoded).
Wait for Wallet Primitive Types #25 to be merged.

KtorZ

Nice tests 👍
A few remarks, mostly on the coding style & standards.

KtorZ · 2019-03-05T07:34:51Z

src/Cardano/Wallet/Binary.hs

+import qualified Data.ByteString.Lazy             as BL
+import           Data.Word                        (Word16, Word64)
+import           Data.Set                         (Set)
+import qualified Data.Set                         as Set


Did you configure your editor to pick up and apply the stylish-haskell configuration 🤔 ?

src/Cardano/Wallet/Binary.hs

KtorZ · 2019-03-05T07:37:29Z

src/Cardano/Wallet/Binary/Helpers.hs

+--
+inspectNextToken :: CBOR.Decoder s ()
+inspectNextToken =
+  CBOR.peekTokenType >>= flip traceShow (return ())


HLint is probably going to complain about the traceShow, we may require an annotation up there ^^^

src/Cardano/Wallet/Binary/Helpers.hs

KtorZ · 2019-03-05T07:40:05Z

test/unit/Cardano/Wallet/BinarySpec.hs

+hash16 :: ByteString -> Hash a
+hash16 = either bomb Hash . convertFromBase Base16
+    where
+        bomb msg = error ("Could not decode test string: " <> msg)


test/unit/Cardano/Wallet/BinarySpec.hs

src/Cardano/Wallet/Binary.hs

test/unit/Cardano/Wallet/BinarySpec.hs

src/Cardano/Wallet/Binary.hs

test/unit/Cardano/Wallet/BinarySpec.hs

jonathanknowles

@rvl When running weeder on your branch, it claims that:

= Package cardano-wallet =

== Section library ==
Redundant build-depends entry
* base58-bytestring
* cardano-crypto
* cryptonite
* digest
* memory

Perhaps remove these from the cabal file?

src/Cardano/Wallet/Binary.hs

points above were addressed 👍

KtorZ · 2019-03-06T09:54:05Z

Rebased on top of latest master 👍

…case

[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] 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] 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] 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 self-assigned this Mar 5, 2019

KtorZ mentioned this pull request Mar 5, 2019

Implement necessary CBOR decoders (block and block header) #11

Closed

9 tasks

rvl added this to the Receive And Process Blocks (cardano-http-bridge) milestone Mar 5, 2019

rvl requested a review from jonathanknowles March 5, 2019 07:13

rvl assigned jonathanknowles Mar 5, 2019

rvl marked this pull request as ready for review March 5, 2019 07:13

KtorZ suggested changes Mar 5, 2019

View reviewed changes