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.

Sign up for GitHub

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 MuxError handling in FoldBlocksError #548

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Add MuxError handling in FoldBlocksError #548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions cardano-api/internal/Cardano/Api/LedgerState.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ module Cardano.Api.LedgerState
, chainSyncClientPipelinedWithLedgerState

-- * Ledger state conditions
, LedgerStateCondition(..)
, ConditionResult(..)
, fromConditionResult
, toConditionResult
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not super satisfied with those names. Any suggestions are welcome here.

Copy link
Contributor

Choose a reason for hiding this comment

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

For fromConditionResult you could call it: isConditionMet or wasConditionMet. For toConditionResult, I am not sure what could be better, one idea would be simply conditionResult, because it is kind of a smart constructor. Or conditionResultMet?

Copy link
Contributor

Choose a reason for hiding this comment

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

But it doesn't look like it is being used

Copy link
Contributor

Choose a reason for hiding this comment

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

Those names are fine to me. Explicit and not too long. I'd keep them 👍

Copy link
Contributor Author

@carbolymer carbolymer Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

They're meant to be used in cardano-testnet if needed. Example usage:

fooCallbck = do
   ...
   pure $ toConditionResult isSkyBlue

or

fooCallbck = do
   ...
   pure $ conditionResult isSkyBlue

Not sure which one is more obvious. I think I'll stick to toConditionResult as it signals simple conversion.

, foldEpochState

-- * Errors
Expand Down Expand Up @@ -166,6 +168,7 @@ import Ouroboros.Consensus.Storage.Serialisation
import Ouroboros.Consensus.TypeFamilyWrappers (WrapLedgerEvent (WrapLedgerEvent))
import Ouroboros.Network.Block (blockNo)
import qualified Ouroboros.Network.Block
import Ouroboros.Network.Mux (MuxError)
import qualified Ouroboros.Network.Protocol.ChainSync.Client as CS
import qualified Ouroboros.Network.Protocol.ChainSync.ClientPipelined as CSP
import Ouroboros.Network.Protocol.ChainSync.PipelineDecision
Expand Down Expand Up @@ -356,13 +359,15 @@ data FoldBlocksError
= FoldBlocksInitialLedgerStateError !InitialLedgerStateError
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also change the name of this type to something like NewEpochStateError (not necessary for this PR).

| FoldBlocksApplyBlockError !LedgerStateError
| FoldBlocksIOException !IOException
| FoldBlocksMuxError !MuxError
deriving Show

instance Error FoldBlocksError where
prettyError = \case
FoldBlocksInitialLedgerStateError err -> prettyError err
FoldBlocksApplyBlockError err -> "Failed when applying a block:" <+> prettyError err
FoldBlocksIOException err -> "IOException:" <+> prettyException err
FoldBlocksMuxError err -> "FoldBlocks error:" <+> prettyException err

-- | Type that lets us decide whether to continue or stop
-- the fold from within our accumulation function.
Expand Down Expand Up @@ -406,7 +411,7 @@ foldBlocks
-- truncating the last k blocks before the node's tip.
-> t m a
-- ^ The final state
foldBlocks nodeConfigFilePath socketPath validationMode state0 accumulate = handleIOExceptions $ do
foldBlocks nodeConfigFilePath socketPath validationMode state0 accumulate = handleExceptions $ do
-- NOTE this was originally implemented with a non-pipelined client then
-- changed to a pipelined client for a modest speedup:
-- * Non-pipelined: 1h 0m 19s
Expand Down Expand Up @@ -1758,10 +1763,19 @@ constructGlobals sGen eInfo (Ledger.ProtVer majorPParamsVer _) =

--------------------------------------------------------------------------

data LedgerStateCondition
= ConditionMet
| ConditionNotMet
deriving (Show, Eq)
-- | Type isomorphic to bool, representing condition check result
data ConditionResult
= ConditionNotMet
| ConditionMet
deriving (Read, Show, Enum, Bounded, Ord, Eq)

toConditionResult :: Bool -> ConditionResult
toConditionResult False = ConditionNotMet
toConditionResult True = ConditionMet

fromConditionResult :: ConditionResult -> Bool
fromConditionResult ConditionNotMet = False
fromConditionResult ConditionMet = True

data AnyNewEpochState where
AnyNewEpochState
Expand Down Expand Up @@ -1791,7 +1805,7 @@ foldEpochState
-> ( AnyNewEpochState
-> SlotNo
-> BlockNo
-> StateT s IO LedgerStateCondition
-> StateT s IO ConditionResult
)
-- ^ Condition you want to check against the new epoch state.
--
Expand All @@ -1804,9 +1818,9 @@ foldEpochState
-- rollback. This is achieved by only calling the accumulator on states/blocks
-- that are older than the security parameter, k. This has the side effect of
-- truncating the last k blocks before the node's tip.
-> t m (LedgerStateCondition, s)
-> t m (ConditionResult, s)
-- ^ The final state
foldEpochState nodeConfigFilePath socketPath validationMode terminationEpoch initialResult checkCondition = handleIOExceptions $ do
foldEpochState nodeConfigFilePath socketPath validationMode terminationEpoch initialResult checkCondition = handleExceptions $ do
-- NOTE this was originally implemented with a non-pipelined client then
-- changed to a pipelined client for a modest speedup:
-- * Non-pipelined: 1h 0m 19s
Expand Down Expand Up @@ -1858,7 +1872,7 @@ foldEpochState nodeConfigFilePath socketPath validationMode terminationEpoch ini
Nothing -> modifyError FoldBlocksIOException . liftIO $ readMVar stateMv
where
protocols :: ()
=> MVar (LedgerStateCondition, s)
=> MVar (ConditionResult, s)
-> IORef (Maybe LedgerStateError)
-> Env
-> LedgerState
Expand All @@ -1874,7 +1888,7 @@ foldEpochState nodeConfigFilePath socketPath validationMode terminationEpoch ini
-- | Defines the client side of the chain sync protocol.
chainSyncClient :: Word16
-- ^ The maximum number of concurrent requests.
-> MVar (LedgerStateCondition, s)
-> MVar (ConditionResult, s)
-- ^ State accumulator. Written to on every block.
-> IORef (Maybe LedgerStateError)
-- ^ Resulting error if any. Written to once on protocol
Expand Down Expand Up @@ -2002,5 +2016,11 @@ atTerminationEpoch terminationEpoch events =
, currentEpoch' >= terminationEpoch
]

handleIOExceptions :: MonadIOTransError FoldBlocksError t m => ExceptT FoldBlocksError IO a -> t m a
handleIOExceptions = liftEither <=< liftIO . fmap (join . first FoldBlocksIOException) . try . runExceptT
handleExceptions :: MonadIOTransError FoldBlocksError t m
=> ExceptT FoldBlocksError IO a
-> t m a
handleExceptions = liftEither <=< liftIO . runExceptT . flip catches handlers
where
handlers = [ Handler $ throwError . FoldBlocksIOException
, Handler $ throwError . FoldBlocksMuxError
]
7 changes: 6 additions & 1 deletion cardano-api/internal/Cardano/Api/Orphans.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

module Cardano.Api.Orphans () where

import Cardano.Api.Pretty (Pretty (..), (<+>))
import Cardano.Api.Pretty (Pretty (..), prettyException, (<+>))
import Cardano.Api.Via.ShowOf

import Cardano.Binary (DecoderError (..))
Expand Down Expand Up @@ -74,6 +74,7 @@ import qualified Ouroboros.Consensus.Shelley.Eras as Consensus
import Ouroboros.Consensus.Shelley.Ledger.Block (ShelleyHash (..))
import qualified Ouroboros.Consensus.Shelley.Ledger.Query as Consensus
import Ouroboros.Network.Block (HeaderHash, Tip (..))
import Ouroboros.Network.Mux (MuxError)

import qualified Codec.Binary.Bech32 as Bech32
import qualified Codec.CBOR.Read as CBOR
Expand Down Expand Up @@ -474,3 +475,7 @@ instance Semigroup (Ledger.ConwayPParams StrictMaybe era) where

lastMappendWithTHKD :: (a -> Ledger.THKD g StrictMaybe b) -> a -> a -> Ledger.THKD g StrictMaybe b
lastMappendWithTHKD f a b = Ledger.THKD $ lastMappendWith (Ledger.unTHKD . f) a b

instance Pretty MuxError where
pretty err = "Mux layer error:" <+> prettyException err

4 changes: 3 additions & 1 deletion cardano-api/src/Cardano/Api.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,9 @@ module Cardano.Api (
chainSyncClientPipelinedWithLedgerState,

-- *** Ledger state conditions
LedgerStateCondition(..),
ConditionResult(..),
fromConditionResult,
toConditionResult,
AnyNewEpochState(..),
foldEpochState,
getAnyNewEpochState,
Expand Down
Loading