-
Notifications
You must be signed in to change notification settings - Fork 93
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.
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
support hashable-1.4.3 #1717
support hashable-1.4.3 #1717
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,9 +223,8 @@ someChainId v = someChainIdAt v maxBound | |
-- chain ids for the chainweb at the given height. | ||
-- | ||
someChainIdAt :: HasCallStack => HasChainwebVersion v => v -> BlockHeight -> ChainId | ||
someChainIdAt v h = head . toList $ chainIdsAt v h | ||
-- 'head' is guaranteed to succeed because the empty graph isn't a valid chain | ||
-- graph. | ||
someChainIdAt v h = minimum $ chainIdsAt v h | ||
-- guaranteed to succeed because the empty graph isn't a valid chain graph. | ||
Comment on lines
+226
to
+227
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have fixed the value to value with the previous version of |
||
{-# INLINE someChainIdAt #-} | ||
|
||
isGraphChange :: HasChainwebVersion v => v -> BlockHeight -> Bool | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,15 +21,14 @@ module Chainweb.Test.Cut.TestBlockDb | |
|
||
import Control.Concurrent.MVar | ||
import Control.Monad.Catch | ||
import Data.Bifunctor (first) | ||
import qualified Data.HashMap.Strict as HM | ||
|
||
import Chainweb.BlockHeader | ||
import Chainweb.BlockHeaderDB | ||
import Chainweb.ChainId | ||
import Chainweb.Cut | ||
import Chainweb.Test.Utils (testRocksDb) | ||
import Chainweb.Test.Cut (GenBlockTime, testMine') | ||
import Chainweb.Test.Cut (GenBlockTime, testMine', MineFailure(BadAdjacents)) | ||
import Chainweb.Payload | ||
import Chainweb.Payload.PayloadStore | ||
import Chainweb.Payload.PayloadStore.RocksDB | ||
|
@@ -64,13 +63,34 @@ mkTestBlockDb cv rdb = do | |
return $! TestBlockDb wdb pdb initCut | ||
|
||
-- | Add a block. | ||
addTestBlockDb :: TestBlockDb -> Nonce -> GenBlockTime -> ChainId -> PayloadWithOutputs -> IO () | ||
-- | ||
-- Returns False when mining fails due to BadAdjacents, which usually means that | ||
-- the chain is blocked. Retry with another chain! | ||
-- | ||
addTestBlockDb | ||
:: TestBlockDb | ||
-> Nonce | ||
-> GenBlockTime | ||
-> ChainId | ||
-> PayloadWithOutputs | ||
-> IO Bool | ||
addTestBlockDb (TestBlockDb wdb pdb cmv) n gbt cid outs = do | ||
c <- takeMVar cmv | ||
r <- testMine' wdb n gbt (_payloadWithOutputsPayloadHash outs) cid c | ||
(T2 _ c') <- fromEitherM $ first (userError . show) $ r | ||
casInsert pdb outs | ||
putMVar cmv c' | ||
case r of | ||
-- success | ||
Right (T2 _ c') -> do | ||
casInsert pdb outs | ||
putMVar cmv c' | ||
return True | ||
|
||
-- mining failed, probably because chain is blocked | ||
Left BadAdjacents -> do | ||
putMVar cmv c | ||
return False | ||
Comment on lines
+88
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before the function would fail with an In practice there are tests where it can happen that the mining of a blocked chain is attempted. It was by pure chance that this didn't showed up before. The latest version of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about applying a "if comments on github make it easy to understand the PR, they should be in the code" rule here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure in case of this comments, since they talk an old version of the code and the diff with the new version. I think, PR comments my be a better place for those. But, generally, I agree, that comments that explain the behavior of the new code should be code comments. |
||
|
||
-- something went wrong | ||
Left e -> throwM $ userError (show e) | ||
|
||
-- | Get header for chain on current cut. | ||
getParentTestBlockDb :: TestBlockDb -> ChainId -> IO BlockHeader | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,7 +62,6 @@ import Chainweb.Test.TestVersions | |
import Chainweb.Time | ||
import Chainweb.Utils | ||
import Chainweb.Version | ||
import Chainweb.Version.Utils | ||
import Chainweb.WebPactExecutionService | ||
|
||
import Chainweb.Storage.Table (casLookupM) | ||
|
@@ -71,7 +70,8 @@ testVersion :: ChainwebVersion | |
testVersion = slowForkingCpmTestVersion peterson | ||
|
||
cid :: ChainId | ||
cid = someChainId testVersion | ||
cid = unsafeChainId 9 | ||
-- several tests in this file expect chain 9 | ||
Comment on lines
+73
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the new hashable version the value of |
||
|
||
data MultiEnv = MultiEnv | ||
{ _menvBdb :: !TestBlockDb | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
results: | ||
coinbase: eyJnYXMiOjAsInJlc3VsdCI6eyJzdGF0dXMiOiJzdWNjZXNzIiwiZGF0YSI6IldyaXRlIHN1Y2NlZWRlZCJ9LCJyZXFLZXkiOiJJa1l3UXpsdFJUVkpNM2RsTFRsR05uRmxSV3RaYldSaWVYQXdUMk5sYUhKZmIwWkpNbXB1YkRVdGQwa2kiLCJsb2dzIjoibFdaVWZjX0dKdGxUUy1LeDAtYm00aXRfSWE5bnNGZG5jZ2NwSE0td091OCIsIm1ldGFEYXRhIjpudWxsLCJjb250aW51YXRpb24iOm51bGwsInR4SWQiOjd9 | ||
coinbase: eyJnYXMiOjAsInJlc3VsdCI6eyJzdGF0dXMiOiJzdWNjZXNzIiwiZGF0YSI6IldyaXRlIHN1Y2NlZWRlZCJ9LCJyZXFLZXkiOiJJbXRNTUVWWWEyc3lkMlJrVFZkeU1sRkJUVU5YUkRaelFqZHBNR3hPVTFZM2VGWm5XSGN5YWxBelNFMGkiLCJsb2dzIjoibFdaVWZjX0dKdGxUUy1LeDAtYm00aXRfSWE5bnNGZG5jZ2NwSE0td091OCIsIm1ldGFEYXRhIjpudWxsLCJjb250aW51YXRpb24iOm51bGwsInR4SWQiOjd9 | ||
minerData: eyJhY2NvdW50IjoiTm9NaW5lciIsInByZWRpY2F0ZSI6IjwiLCJwdWJsaWMta2V5cyI6W119 | ||
outputsHash: CVJw7e4GM7lv0fTF6UQ_Bh9AskyHw2TWzgRLUyyKRqo | ||
payloadHash: k_2HP1fYNLkKZQfXaijxIVlrN4hoxkG9--s060A9He4 | ||
outputsHash: BRFFHFEHv4IbjOUrIkWgp2N0eU-6mijYqRU1pcGVHJ8 | ||
payloadHash: h490ZOFfkwbnGOFgAtxj8v7B-u2BJiMzmySXUwB2Fq4 | ||
transactions: [] | ||
transactionsHash: lL9ztEiU-NwzrlTpBbvhT4M1l5Shsht94OwFyhBaFD0 | ||
test-group: new-block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this update is unrelated, but nice to have.