Change watchEpochStateUpdate callback type #5855
Conversation
| @@ -268,21 +268,21 @@ watchEpochStateUpdate | |||
| :: forall m a. (HasCallStack, MonadIO m, MonadTest m, MonadAssertion m) | |||
| => EpochStateView -- ^ The info to access the epoch state | |||
| -> EpochInterval -- ^ The maximum number of epochs to wait | |||
| -> ((AnyNewEpochState, SlotNo, BlockNo) -> m (Maybe a)) -- ^ The guard function (@Just@ if the condition is met, @Nothing@ otherwise) | |||
| -> m (Maybe a) | |||
| -> ((AnyNewEpochState, SlotNo, BlockNo) -> m (LedgerStateCondition, a)) -- ^ The guard function (@Just@ if the condition is met, @Nothing@ otherwise) | |||
There was a problem hiding this comment.
LedgerStateCondition needs to be renamed to a more general name. ConditionCheckResult perhaps? Will do in a follow-up PR to API.
| pure $ maybeExtractGovernanceActionIndex (fromString governanceActionTxId) anyNewEpochState | ||
| H.nothingFailM . fmap snd . watchEpochStateUpdate epochStateView (L.EpochInterval 1) $ \(anyNewEpochState, _, _) -> do | ||
| let r = maybeExtractGovernanceActionIndex (fromString governanceActionTxId) anyNewEpochState | ||
| pure (if isJust r then ConditionMet else ConditionNotMet, r) |
There was a problem hiding this comment.
Writing ifs by hand is error-prone. This will be simplified in a follow-up pr to API. We need Bool <-> LedgerStateCondition conversion functions
carbolymer
force-pushed
the
mgalazyn/test/change-watching-function-type
branch
from
4042040
to
a426763
Compare
carbolymer
force-pushed
the
mgalazyn/test/change-watching-function-type
branch
from
a426763
to
ca8bcd8
Compare
| mResult <- watchEpochStateUpdate epochStateView maxWait checkForVotes | ||
| when (isNothing mResult) $ | ||
| (cond, ()) <- watchEpochStateUpdate epochStateView maxWait checkForVotes | ||
| when (cond == ConditionNotMet) $ |
There was a problem hiding this comment.
I'm not a big fan of those == on a union type, for long term maintenance; but in this specific case I think it's fine. It's unlikely we'll add a third case to LedgerStateCondition.
| if isCommitteePresent == Just True | ||
| then (ConditionMet, ()) | ||
| else (ConditionNotMet, ()) |
There was a problem hiding this comment.
| if isCommitteePresent == Just True | |
| then (ConditionMet, ()) | |
| else (ConditionNotMet, ()) | |
| case isCommitteePresent of | |
| Just True -> (ConditionMet, ()) | |
| _ -> (ConditionNotMet, ()) |
Too bad Haskell doesn't have arm sharing here 🙃
|
This adds considerable complexity and opportunity for error, and I am not sure the added complexity is worth it. In any case, if we need the added flexibility, we could just have a restricted version of |
|
This PR is stale because it has been open 45 days with no activity. |
Description
This PR changes
watchEpochStateUpdatecallback type from a sum typeMaybe ato a product(LedgerStateCondition, a), which allows to return from the function, when the condition is not satisfied, but the deadline epoch has been reached.Checklist
See Runnings tests for more details
CHANGELOG.mdfor affected package.cabalfiles are updatedhlint. See.github/workflows/check-hlint.ymlto get thehlintversionstylish-haskell. See.github/workflows/stylish-haskell.ymlto get thestylish-haskellversionghc-8.10.7andghc-9.2.7Note on CI
If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.