VolatileDB: Unify and extend getters #1701
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
mrBliss
suggested changes
Feb 26, 2020
| @@ -304,7 +301,7 @@ transitionImpl model cmd _ = eventAfter $ lockstep model cmd | |||
| preconditionImpl :: Model Symbolic -> At CmdErr Symbolic -> Logic | |||
| preconditionImpl Model{..} (At (CmdErr cmd mbErrors)) = | |||
| compatibleWithError .&& case cmd of | |||
| GetPredecessor bids -> forall bids (`elem` bidsInModel) | |||
| GetBlockInfo bids -> forall bids (`elem` bidsInModel) | |||
There was a problem hiding this comment.
Remove this precondition, getBlockInfo should just return Nothing for blocks not in the VolatileDB.
| | Successors [Set BlockId] | ||
| | Predecessor [Predecessor] | ||
| | BlocksInfo [Maybe (BlockInfo BlockId)] |
There was a problem hiding this comment.
Suggested change
| | BlocksInfo [Maybe (BlockInfo BlockId)] | |
| | BlockInfos [Maybe (BlockInfo BlockId)] |
Comment on lines
231
to
234
| getBlockInfo :: VolDB m blk | ||
| -> STM m ( HeaderHash blk | ||
| -> Maybe (VolDB.BlockInfo (HeaderHash blk)) | ||
| ) |
There was a problem hiding this comment.
getBlockInfo
:: VolDB m blk
-> STM m (HeaderHash blk -> Maybe (VolDB.BlockInfo (HeaderHash blk)))| @@ -348,32 +354,31 @@ isReachable predecessor isMember i b = | |||
| -- See the documentation of 'Path'. | |||
| computePath | |||
| :: forall blk. HasHeader blk | |||
| => (HeaderHash blk -> WithOrigin (HeaderHash blk)) -- ^ @getPredecessor@ | |||
| -> (HeaderHash blk -> Bool) -- ^ @isMember@ | |||
| => (HeaderHash blk -> Maybe (WithOrigin (HeaderHash blk))) -- ^ @predecessor@ | |||
There was a problem hiding this comment.
There is nothing called predecessor anymore, so I suggest:
Suggested change
| => (HeaderHash blk -> Maybe (WithOrigin (HeaderHash blk))) -- ^ @predecessor@ | |
| => (HeaderHash blk -> Maybe (WithOrigin (HeaderHash blk))) | |
| -- ^ Return the predecessor |
kderme
force-pushed
the
kderme/getters
branch
from
e33f342 to
469fbb8
Compare
mrBliss
approved these changes
Feb 27, 2020
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Feb 27, 2020
1701: VolatileDB: Unify and extend getters r=mrBliss a=kderme Related issue: #1684 This is done: > Unify getIsMember and getPredecessor, and return BlockInfo This is not done yet: > Possibly unify it with getSuccessors too, e.g., using return a pair of both results. Co-authored-by: kderme <[email protected]>
Build failed |
|
bors r+ |
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.
Related issue: #1684
This is done:
This is not done yet: