-
Notifications
You must be signed in to change notification settings - Fork 214
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
Light NetworkLayer: watchNodeTip/currentNodeTip #3169
Conversation
2393dd7
to
889e2f7
Compare
869b175
to
d3a0f20
Compare
There was a problem hiding this 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! 👍 Returning the genesis block instead of throwing an error (if appropriate!) could be worth it, though. 🤔
parseBlockHeader blockHash = | ||
case fromText (BF.unBlockHash blockHash) of | ||
Right hash -> pure hash | ||
Left tde -> throwError $ InvalidBlockHash blockHash tde |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to use some helper function of type :: Either e a -> ExceptT m e' a
here? 🤔
(That's the issue with explicit error handling: Large portions of the code become code that converts error types. 😅 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used to use hoist-error
library for these kind of tasks https://hackage.haskell.org/package/hoist-error-0.2.1.0
Ideally I'd introduce it to our codebase but its associated with additional effort of communicating and convincing team that its worth it and I don't feel like I am ready to kick-off such process 🤷🏻♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. 🤷🏻♂️ hoist-error
may be a bit overkill, but I had the feeling that there could be a polymorphic function in base
that does part of the job. E.g. Data.Bifunctor.first
specializes to first :: (e -> e') -> Either e a -> Either e' a
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Unisay @HeinrichApfelmus well, we have also
mapLeft :: (a -> c) -> Either a b -> Either c b
from https://hackage.haskell.org/package/either-5.0.1.1/docs/Data-Either-Combinators.html
Nothing -> throwError $ NoSlotError block | ||
blockHeight <- case _blockHeight of | ||
Just height -> pure $ Quantity $ fromIntegral height | ||
Nothing -> throwError $ NoBlockHeight block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 I think the Nothing
case indicates the genesis block, but I'm not entirely certain. I recently learned that in the Byron era, there exist Epoch Boundary Blocks (EBBs) which are weird. Could you check (or google) what value Blockfrost returns for the genesis block and these special blocks?
The Cardano mainnet genesis is:
https://explorer.cardano.org/en/block?id=5f20df933584822601f9e3f8c024eb5eb252fe8cefb24d1317dc3d432e940ebb
The block after genesis is one of those fabled EEBs:
https://explorer.cardano.org/en/block?id=89d9b5a5b8ddc8d7e5a6795e9774d97faf1efea59b2caf7eaf9f8c5b32059df4
This PR adds 2 functions to the new
light-mode
implementation of theNetworkLayer
:currentNodeTip
: calls to the Blockfrost APIgetLatestBlock
, translates Blockfrost types to ours;watchNodeTip
: same ascurrentNodeTip
but looping forever in a separate thread invoking a callback function;I have tested it manually in REPL
Comments
Issue Number
ADP-1424