Shelley: do the chainChecks in validateEnvelope #1963
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This includes #1928 until it is merged. UPDATE: merged |
mrBliss
force-pushed
the
mrBliss/shelley-chainChecks
branch
from
8c96bc2 to
682f893
Compare
mrBliss
commented
Apr 17, 2020
Comment on lines
323
to
329
| newtype ShelleyOtherHeaderEnvelopeError c = ShelleyOtherHeaderEnvelopeError { | ||
| unShelleyOtherHeaderEnvelopeError :: STS.PredicateFailure (STS.CHAIN c) | ||
| } | ||
| deriving (Eq, Show) | ||
| -- TODO fix thunks | ||
| deriving NoUnexpectedThunks | ||
| via OnlyCheckIsWHNF "ShelleyOtherHeaderEnvelopeError" (ShelleyOtherHeaderEnvelopeError c) |
There was a problem hiding this comment.
Let's make this an orphan instance of NoUnexpectedThunks instead, so that I'm forced to remove it when I add the right one in cardano-ledger-specs.
mrBliss
force-pushed
the
mrBliss/shelley-chainChecks
branch
from
682f893 to
f39c686
Compare
Instead of `Text`, we now allow a block-specific `OtherHeaderEnvelopeError`. We'll use this ability to do more header checks for Shelley without having to convert the errors to `Text`.
In particular, replace `BlockConfig` with `TopLevelConfig`, and add `LedgerView`. We need this extra data to do some more checks for Shelley.
In the Shelley ledger, the `CHAIN` rule is used to apply a whole block. In consensus, we split up the application of a block to the ledger into separate steps that are performed together by `applyExtLedgerState`: * `applyChainTick`: executes the `TICK` transition * `validateHeader`: - `validateEnvelope`: default checks - `updateConsensusState`: executes the `PRTCL` transition * `applyLedgerBlock`: executes the `BBODY` transition and the remaining checks that are part of the `CHAIN` transition The unfortunate thing was that we had to copy-paste some checks part of the `CHAIN` transition that were not part of any subtransition. The Shelley has recently factored out these checks into the `chainChecks` function. This commit takes advantage of this change. The new situation: * `applyChainTick`: executes the `TICK` transition * `validateHeader`: - `validateEnvelope`: default checks + executes the `chainChecks` - `updateConsensusState`: executes the `PRTCL` transition * `applyLedgerBlock`: executes the `BBODY` transition Now all checks and transitions of the `CHAIN` rule nicely map onto separate steps in consensus. Note that `chainChecks` can be done on the header, so this change means that we do these checks earlier, in the ChainSyncClient, instead of after downloading the block and applying it to the ledger with `applyExtLedgerState`.
mrBliss
force-pushed
the
mrBliss/shelley-chainChecks
branch
from
f39c686 to
5b20469
Compare
|
bors merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the Shelley ledger, the
CHAINrule is used to apply a whole block. In consensus, we split up the application of a block to the ledger into separate steps that are performed together byapplyExtLedgerState:applyChainTick: executes theTICKtransitionvalidateHeader:validateEnvelope: default checksupdateConsensusState: executes thePRTCLtransitionapplyLedgerBlock: executes theBBODYtransition and the remaining checks that are part of theCHAINtransitionThe unfortunate thing was that we had to copy-paste some checks part of the
CHAINtransition that were not part of any subtransition.The Shelley has recently factored out these checks into the
chainChecksfunction. This commit takes advantage of this change. The new situation:applyChainTick: executes theTICKtransitionvalidateHeader:validateEnvelope: default checks + executes thechainChecksupdateConsensusState: executes thePRTCLtransitionapplyLedgerBlock: executes theBBODYtransitionNow all checks and transitions of the
CHAINrule nicely map onto separate steps in consensus.Note that
chainCheckscan be done on the header, so this change means that we do these checks earlier, in the ChainSyncClient, instead of after downloading the block and applying it to the ledger withapplyExtLedgerState.To make this possible, some refactorings were needed:
HeaderValidation: export defaultValidateEnvelope
HeaderValidation: replace OtherEnvelopeError with a type family
Instead of
Text, we now allow a block-specificOtherHeaderEnvelopeError. We'll use this ability to do more header checks for Shelley without having to convert the errors toText.HeaderValidation: pass more info to validateEnvelope
In particular, replace
BlockConfigwithTopLevelConfig, and addLedgerView. We need this extra data to do some more checks for Shelley.