Add startBlockSyncer #54
Conversation
rvl
force-pushed
the
rvl/52/start-block-syncer
branch
from
fb34c1f to
e77897a
Compare
rvl
force-pushed
the
rvl/52/start-block-syncer
branch
from
e77897a to
1dd3044
Compare
rvl
force-pushed
the
rvl/52/start-block-syncer
branch
2 times, most recently
from
abd3258 to
3880cd6
Compare
KtorZ
force-pushed
the
rvl/12/review-network-layer
branch
2 times, most recently
from
3dfd7e5 to
1bfdbff
Compare
So that we can remember the block we are up to.
This makes it simpler and better.
KtorZ
force-pushed
the
rvl/52/start-block-syncer
branch
from
3880cd6 to
f46f898
Compare
|
rebased on top of latest master. |
There was a problem hiding this comment.
LGTM. I generalized the tick function to make it now really just about ticking, using the state instead of the block header consumed. We'll move the responsibility of keeping track of blocks and duplicates to the network layer.
I've adjusted the file header comments and the tests to reflect that.
I've also moved the instantiation of a network layer outside of the startBlockSyncer function to leave the function really about, fetching blocks from a chain producer.
| @@ -117,6 +124,15 @@ data BlockHeader = BlockHeader | |||
|
|
|||
| instance NFData BlockHeader | |||
|
|
|||
| instance Buildable BlockHeader where | |||
| build (BlockHeader s prev) = mempty | |||
There was a problem hiding this comment.
Do we need mempty here?
There was a problem hiding this comment.
Not really. But it makes the alignment nicer below. With monoids or applicatives, I usually like starting with an empty case and then, on each newline, have a consistent alignment with <> ..., rather than having to do something like:
fn =
myFirstCase
<> MySecondCase
<> MyThirdCase
-- or
fn = myFirstCase
<> MySecondCase
<> MyThirdCaseIt's purely aesthetic 🤷♂️
| @@ -135,6 +151,11 @@ data Tx = Tx | |||
|
|
|||
| instance NFData Tx | |||
|
|
|||
| instance Buildable Tx where | |||
| build (Tx ins outs) = mempty | |||
There was a problem hiding this comment.
Do we need mempty here?
| -> (Block -> IO ()) | ||
| -> IO () | ||
| listen network action = do | ||
| tick getNextBlocks action 5000 (SlotId 0 0) |
There was a problem hiding this comment.
definitely replacing BlockHeadersConsumed with SlotId will make it easier to follow in the logs!!!
| ) | ||
| mapM duplicateMaybe blocks >>= fmap Blocks . groups . mconcat | ||
| let b0 = (blockHeaderHash h0, Block h0 mempty) | ||
| Blocks <$> groups (map snd $ take n $ iterate next b0) |
There was a problem hiding this comment.
does it going to produce duplicated Blocks ?
There was a problem hiding this comment.
Nope it's not. I've removed that responsibility from the ticking function for now, and we'll have to make this a responsibility of the network layer.
Relates to #52.
Overview
This combines
BlockSyncer.tickingFunctionandChainProducer.getNextBlocksinto a process that retrieves blocks fromcardano-http-bridge.Comments
It's pretty clear after looking at this feature "as a whole" that we must rework the code to address a few issues:
nextBlocksshould return more than the requested number of blocks if that makes sense (for example, returning the entire contents of an epoch pack file, or returning all the blocks from unstable epochs).ExceptT ErrGetNextBlocksand usethrowIOto throw theServant.Client.Core.ClientError(or whatever exception), and let the exception bubble up. (Because there's no way to recover from these errors). ⇒ fixed in Simplify NetworkLayer #63tickingFunctionThe following can go in a later PR:
BlockHeadersConsumedis unnecessary becausenextBlocksnever returns blocks before the start slot given to it.