Introduce Header type family #616
Conversation
There was a problem hiding this comment.
Finally, the long awaited clean-up!
I'll push some commits that fix some issues. These should be squashed before merging.
The ChainDB still has to be updated. I don't mind that this is done before my work on the ChainDB is done, I'll just rebase. We need to be careful about the Readers, these will need to be instantiated both with blocks (node-to-client chain sync server) and headers (for the regular chain sync server).
| (SimpleHeader SimpleMockCrypto ext) | ||
| , ProtocolLedgerView (SimpleBlock SimpleMockCrypto ext) | ||
| -- TODO: ProtocolLedgerView implies SupportedBlock already..? | ||
| , SupportedBlock (SimpleBlock SimpleMockCrypto ext) |
There was a problem hiding this comment.
Huh, it compiles just fine if I remove this constraint.
| @@ -122,7 +122,7 @@ class ( Show (ChainState p) | |||
| type family ValidationErr p :: * | |||
|
|
|||
| -- | Blocks that the protocol can run on | |||
| @@ -134,10 +134,10 @@ class ( Show (ChainState p) | |||
| -- | |||
| -- Note: we do not assume that the candidate fragments fork less than @k@ | |||
| -- blocks back. | |||
| preferCandidate :: HasHeader b | |||
| preferCandidate :: HasHeader hdr | |||
There was a problem hiding this comment.
Should we use SupportedHeader here too?
Could/should we include HasHeader as a mandatory constraint for headers?
There was a problem hiding this comment.
We could, those we don't currently need it. But I guessthere is no reason not to add it.
| @@ -122,7 +122,7 @@ class ( Show (ChainState p) | |||
| type family ValidationErr p :: * | |||
|
|
|||
| -- | Blocks that the protocol can run on | |||
| type family SupportedBlock p :: * -> Constraint | |||
| type family SupportedHeader p :: * -> Constraint | |||
There was a problem hiding this comment.
Is this only meant for headers or do we also use it with blocks sometimes?
There was a problem hiding this comment.
Perhaps we might apply it to blocks in tests (like in the model of the chain DB where we don't distinguish).
| @@ -165,10 +165,10 @@ class ( Show (ChainState p) | |||
| -- | Apply a block | |||
| @@ -165,10 +165,10 @@ class ( Show (ChainState p) | |||
| -- | Apply a block | |||
| -- | |||
| -- TODO this will only be used with headers | |||
There was a problem hiding this comment.
This can be removed.
Same comment as above: is this only meant for headers or do we also use it with blocks sometimes?
| -- | Extract the part of the block that the signature should be computed over | ||
| blockSigned :: b -> Signed b | ||
| -- | Extract the part of the header that the signature should be computed over | ||
| headerSigned :: hdr -> Signed hdr | ||
|
|
||
| -- | Signatures are computed of the serialized form | ||
| -- | ||
| -- NOTE: This encoding is important: it determines what the signature looks | ||
| -- like. For backwards compatibility, the encoding of the parts of the block |
There was a problem hiding this comment.
"block" (instead of "header") is mentioned a few times in the docstring.
|
bors r+ |
|
👎 Rejected by code reviews |
|
bors r+ |
mrBliss
force-pushed
the
edsko/introduce-BlockWithHeader
branch
from
d9473e0
to
bd28e44
Compare
Canceled |
Co-authored-by: Thomas Winant <thomas@well-typed.com>
edsko
force-pushed
the
edsko/introduce-BlockWithHeader
branch
from
bd28e44
to
f7440da
Compare
No description provided.