[#526] Implement multiple validation modes #548

intricate · 2019-06-11T21:34:37Z

Closes #526

cardano-ledger/src/Cardano/Chain/Block/Validation.hs

erikd · 2019-06-11T22:25:44Z

cardano-ledger/test/Test/Cardano/Chain/Elaboration/Block.hs

-    , Update.ppMaxBlockSize     = 748 * pps ^. maxBkSz
-    , Update.ppMaxHeaderSize    = 95 * pps ^. maxHdrSz
+    , Update.ppMaxBlockSize     = 832 * pps ^. maxBkSz
+    , Update.ppMaxHeaderSize    = 569 * pps ^. maxHdrSz


What are these changes about?

I'll have to add a comment explaining these. These seem to be the minimum constant multipliers that I can use for elaborating abstract size limits such that all tests still pass without returning "block too large" or "block header too large" errors.

In cardano-ledger-specs, Sizes are represented differently than they are in the concrete implementation (where they're represented in actual bytes) and I'm unsure as to how we can accurately convert between the two.

cardano-ledger/test/Test/Cardano/Chain/Elaboration/Update.hs

intricate · 2019-06-12T03:39:08Z

cardano-ledger/test/Test/Cardano/Chain/UTxO/ValidationMode.hs

+            abstractTxWits
+
+      -- Validate the generated concrete transaction
+      let pm = Dummy.aProtocolMagic


Will address these Dummy.aProtocolMagics tomorrow.

cardano-ledger/test/Test/Cardano/Chain/Block/Model.hs

cardano-ledger/test/Test/Cardano/Chain/UTxO/ValidationMode.hs

dnadales · 2019-06-12T06:06:51Z

I'm a bit confused by this, so sorry for the silly question:

There should be three scenarios: NewSignal/NewState, OldSignal/NewState, OldSignal/OldState

Do we care about these scenarios? I thought this was about checking whether we would perform validation or not when applying a block.

cardano-ledger/src/Cardano/Chain/Block/ValidationMode.hs

ruhatch

Okay, so quite a few changes here, but the general structure is good.

cardano-ledger/src/Cardano/Chain/Block/Validation.hs

cardano-ledger/src/Cardano/Chain/Block/ValidationMode.hs

cardano-ledger/test/Test/Cardano/Chain/Block/ValidationMode.hs

cardano-ledger/test/Test/Cardano/Chain/UTxO/ValidationMode.hs

cardano-ledger/src/Cardano/Chain/Epoch/Validation.hs

cardano-ledger/src/Cardano/Chain/UTxO/ValidationMode.hs

cardano-ledger/src/Cardano/Chain/Block/Validation.hs

erikd · 2019-06-14T02:37:55Z

cardano-ledger/src/Cardano/Chain/Block/Validation.hs

 foldUTxO env utxo blocks = S.foldM_
  (foldUTxOBlock env)
  (pure utxo)
  pure
-  (hoist (withExceptT ErrorParseError) blocks)
+  (pure (hoist (withExceptT ErrorParseError) blocks))


Something about pure and hoist being used like that strikes me as odd. Its almost as if you need a transformers version of Control.Monad.join.

erikd · 2019-06-14T02:43:12Z

cardano-ledger/src/Cardano/Chain/Epoch/Validation.hs

 foldChainValidationState config chainValState blocks = S.foldM_
  (\cvs block ->
    withExceptT (EpochChainValidationError (blockOrBoundarySlot block))
      $ updateChainBlockOrBoundary config cvs block
  )
  (pure chainValState)
-  pure $ hoist (withExceptT EpochParseError) blocks
+  pure (pure (hoist (withExceptT EpochParseError) blocks))


Like above, pure . pure . hoist seems weird.

cardano-ledger/src/Cardano/Chain/Block/Validation.hs

cardano-ledger/src/Cardano/Chain/UTxO/Validation.hs

intricate · 2019-06-17T04:55:17Z

cardano-ledger/src/Cardano/Chain/ValidationMode.hs

+--
+-- If we could find some way to convert the error type in a 'MonadError' then
+-- we could re-write 'wrapError' in a more abstract way.
+wrapExceptTError :: MonadError e' m => ExceptT e m a -> (e -> e') ->  m a


@ruhatch @dnadales: Is it cool if I move this into cardano-prelude or do you guys perhaps have a more clever way of doing this (see comment above this function definition)?

See IntersectMBO/cardano-ledger#548 (comment)

@intricate what about:

wrapErrorWithValidationMode :: (MonadError e' m, MonadReader ValidationMode m) => ReaderT ValidationMode (Either e a) -> (e -> e') -> m a wrapErrorWithValidationMode readerAction = do validationMode <- ask runReaderT readerAction validationMode >>= \case Left err -> throwError $ wrapper err Right x -> pure x

Let me know if there are any type errors with that, but I think it should work

Thanks!

Only had to make a slight change to type sig

wrapErrorWithValidationMode :: (MonadError e' m, MonadReader ValidationMode m) => ReaderT ValidationMode (Either e) a -> (e -> e') -> m a

cardano-ledger/src/Cardano/Chain/UTxO/Validation.hs

cardano-ledger/src/Cardano/Chain/ValidationMode.hs

ruhatch · 2019-06-17T12:18:33Z

cardano-ledger/src/Cardano/Chain/ValidationMode.hs

+--
+-- If we could find some way to convert the error type in a 'MonadError' then
+-- we could re-write 'wrapError' in a more abstract way.
+wrapExceptTError :: MonadError e' m => ExceptT e m a -> (e -> e') ->  m a


@intricate what about:

wrapErrorWithValidationMode :: (MonadError e' m, MonadReader ValidationMode m) => ReaderT ValidationMode (Either e a) -> (e -> e') -> m a wrapErrorWithValidationMode readerAction = do validationMode <- ask runReaderT readerAction validationMode >>= \case Left err -> throwError $ wrapper err Right x -> pure x

ruhatch · 2019-06-17T12:18:56Z

cardano-ledger/src/Cardano/Chain/ValidationMode.hs

+--
+-- If we could find some way to convert the error type in a 'MonadError' then
+-- we could re-write 'wrapError' in a more abstract way.
+wrapExceptTError :: MonadError e' m => ExceptT e m a -> (e -> e') ->  m a


Let me know if there are any type errors with that, but I think it should work

…ition

ruhatch

Looks really good I think - worth several rounds of review :)

ruhatch · 2019-06-20T10:18:17Z

bors r+

548: [#526] Implement multiple validation modes r=ruhatch a=intricate Closes #526 Co-authored-by: Luke Nadur <[email protected]>

iohk-bors · 2019-06-20T10:25:36Z

Build succeeded

intricate self-assigned this Jun 11, 2019

erikd reviewed Jun 11, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/Block/Validation.hs Outdated Show resolved Hide resolved

erikd reviewed Jun 11, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/Block/Validation.hs Outdated Show resolved Hide resolved

intricate force-pushed the intricate/validation-modes branch from 5706b8e to bcffa9a Compare June 11, 2019 22:20

erikd reviewed Jun 11, 2019

View reviewed changes

intricate force-pushed the intricate/validation-modes branch from bcffa9a to 0b64a31 Compare June 11, 2019 22:31

intricate added DO NOT MERGE WIP labels Jun 12, 2019

intricate force-pushed the intricate/validation-modes branch from 0b64a31 to d6deeb1 Compare June 12, 2019 01:49

intricate requested review from ruhatch, dnadales and mhuesch June 12, 2019 02:09

intricate added this to the Consensus/Ledger Interface Additions milestone Jun 12, 2019

intricate changed the title ~~Implement multiple validation modes~~ [#526] Implement multiple validation modes Jun 12, 2019

intricate commented Jun 12, 2019

View reviewed changes

cardano-ledger/test/Test/Cardano/Chain/Block/Model.hs Outdated Show resolved Hide resolved

intricate commented Jun 12, 2019

View reviewed changes

cardano-ledger/test/Test/Cardano/Chain/UTxO/ValidationMode.hs Outdated Show resolved Hide resolved

dnadales reviewed Jun 12, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/Block/ValidationMode.hs Outdated Show resolved Hide resolved

dnadales reviewed Jun 12, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/Block/ValidationMode.hs Outdated Show resolved Hide resolved

ruhatch suggested changes Jun 12, 2019

View reviewed changes

intricate force-pushed the intricate/validation-modes branch 6 times, most recently from 4b4445f to da3046c Compare June 14, 2019 02:20

erikd reviewed Jun 14, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/Block/Validation.hs Outdated Show resolved Hide resolved

erikd reviewed Jun 14, 2019

View reviewed changes

intricate force-pushed the intricate/validation-modes branch from da3046c to ff7c06b Compare June 14, 2019 03:41

dnadales reviewed Jun 14, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/Block/Validation.hs Outdated Show resolved Hide resolved

dnadales reviewed Jun 14, 2019

View reviewed changes

cardano-ledger/src/Cardano/Chain/UTxO/Validation.hs Outdated Show resolved Hide resolved

intricate force-pushed the intricate/validation-modes branch 2 times, most recently from 791ad88 to bd6b851 Compare June 17, 2019 01:59

intricate commented Jun 17, 2019

View reviewed changes

ruhatch suggested changes Jun 17, 2019

View reviewed changes

intricate force-pushed the intricate/validation-modes branch 2 times, most recently from 1ec04bb to 5a0c845 Compare June 17, 2019 23:26

intricate removed DO NOT MERGE WIP labels Jun 18, 2019

intricate force-pushed the intricate/validation-modes branch from 5a0c845 to 0737b86 Compare June 18, 2019 06:35

intricate marked this pull request as ready for review June 18, 2019 06:35

intricate force-pushed the intricate/validation-modes branch 3 times, most recently from 1418098 to 80a4361 Compare June 18, 2019 06:47

intricate requested a review from ruhatch June 18, 2019 13:56

intricate added 6 commits June 18, 2019 17:24

[#526] Add TxValidationMode data type

dafcecd

[#526] Add BlockValidationMode data type

4b788bb

[#526] Implement validation modes

ac24dae

[#526] Add tests for multiple validation modes

4d315c4

[#526] Remove MonadError constraint from registerEpoch and epochTrans…

3265e42

…ition

[#526] Add ValidationMode and utilize with ReaderT

b764abe

intricate force-pushed the intricate/validation-modes branch from 80a4361 to b764abe Compare June 18, 2019 22:25

ruhatch approved these changes Jun 20, 2019

View reviewed changes

iohk-bors bot added a commit that referenced this pull request Jun 20, 2019

Merge #548

7534ee3

548: [#526] Implement multiple validation modes r=ruhatch a=intricate Closes #526 Co-authored-by: Luke Nadur <[email protected]>

iohk-bors bot merged commit b764abe into master Jun 20, 2019

iohk-bors bot deleted the intricate/validation-modes branch June 20, 2019 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#526] Implement multiple validation modes #548

[#526] Implement multiple validation modes #548

intricate commented Jun 11, 2019 •

edited

Loading

erikd Jun 11, 2019

intricate Jun 11, 2019 •

edited

Loading

intricate Jun 12, 2019

dnadales commented Jun 12, 2019

ruhatch left a comment

erikd Jun 14, 2019

erikd Jun 14, 2019

intricate Jun 17, 2019 •

edited

Loading

dnadales Jun 17, 2019

ruhatch Jun 17, 2019 •

edited

Loading

ruhatch Jun 17, 2019

intricate Jun 18, 2019

ruhatch Jun 17, 2019 •

edited

Loading

ruhatch Jun 17, 2019

ruhatch left a comment

ruhatch commented Jun 20, 2019

iohk-bors bot commented Jun 20, 2019

[#526] Implement multiple validation modes #548

[#526] Implement multiple validation modes #548

Conversation

intricate commented Jun 11, 2019 • edited Loading

Choose a reason for hiding this comment

intricate Jun 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnadales commented Jun 12, 2019

ruhatch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intricate Jun 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruhatch Jun 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruhatch Jun 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruhatch left a comment

Choose a reason for hiding this comment

ruhatch commented Jun 20, 2019

iohk-bors bot commented Jun 20, 2019

Build succeeded

intricate commented Jun 11, 2019 •

edited

Loading

intricate Jun 11, 2019 •

edited

Loading

intricate Jun 17, 2019 •

edited

Loading

ruhatch Jun 17, 2019 •

edited

Loading

ruhatch Jun 17, 2019 •

edited

Loading