-
Notifications
You must be signed in to change notification settings - Fork 95
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
miscelaneous small changes #2087
Conversation
-- TODO: Shouldn't this type guarantee that the map is total, i.e. that there | ||
-- exists a value for each chain? |
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.
Hmmmmm... maybe we can do this, yes. Please leave this comment here.
|
||
type instance Index (ChainMap a) = ChainId | ||
type instance IxValue (ChainMap a) = a | ||
|
||
instance IxedGet (ChainMap a) where | ||
ixg i = atChain i |
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.
❤️
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.
We should probably even have an Ixed
instance.
@@ -29,7 +29,6 @@ module Chainweb.BlockHeaderDB.Internal | |||
( | |||
-- * Internal Types | |||
RankedBlockHeader(..) |
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.
Do we still need RankedBlockHeader
with the Ranked
newtype? I suppose if we make it wrap a Ranked BlockHeader
, that will waste some memory by duplicating the height.
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.
Yeah, that's the reasoning for not using Ranked
with BlockHeader
.
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.
I see that this:
- adds a
Ranked
type, which should be able to replace a couple newtypes like forRankedBlockHash
. - adds some extra instances for
ChainMap
and a salient comment. - splits
BlockPayloadHash
into its own file - of course this is required by EVM. - adds some other utilities and missing instances.
Co-authored-by: chessai <[email protected]> Co-authored-by: Edmund Noble <[email protected]>
miner rewards
A number of small unrelated changes. It is easiest to review on a per commit basis.