-
Notifications
You must be signed in to change notification settings - Fork 720
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
[CAD-3693] Expose events api via foldBlocks, chainSyncClientWithLedgerState, and chainSyncClientPipelinedWithLedgerState #3374
Conversation
… chainSyncClientPipelinedWithLedgerState
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.
API looks great. I've not checked anything else carefully.
@@ -222,11 +222,12 @@ foldBlocks | |||
-> ValidationMode | |||
-> a | |||
-- ^ The initial accumulator state. | |||
-> (Env -> LedgerState -> BlockInMode CardanoMode -> a -> IO a) | |||
-> (Env -> LedgerState -> [LedgerEvent] -> BlockInMode CardanoMode -> a -> IO 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.
Noice!
@@ -86,21 +86,21 @@ main = do | |||
-- | Defines the client side of the chain sync protocol. | |||
chainSyncClient | |||
:: ChainSyncClient | |||
(BlockInMode CardanoMode, Either Text LedgerState) | |||
(BlockInMode CardanoMode, Either Text (LedgerState, [LedgerEvent])) |
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.
👍
BTW, remind me what the Either Text
is about.
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.
We might get an error when applying blocks. The error will be either a HardForkLedgerError '[...]
from calling tickThenApplyLedgerResult
or an hash miss-match. I think I originally just used Text
here out of laziness/simplicity but am happy to use a structured error instead (I'd do that in a separate PR).
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.
A structured error would be great 👍
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.
LGTM!
@@ -86,21 +86,21 @@ main = do | |||
-- | Defines the client side of the chain sync protocol. | |||
chainSyncClient | |||
:: ChainSyncClient | |||
(BlockInMode CardanoMode, Either Text LedgerState) | |||
(BlockInMode CardanoMode, Either Text (LedgerState, [LedgerEvent])) |
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.
A structured error would be great 👍
bors merge |
Build succeeded: |
This simply exposes the
[LedgerEvent]
that comes with applying blocks. This should be useful for db-sync in the future.