Introduce HasBinaryBlockInfo and make it overridable in SerialiseHFC #2435
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
So that we later can refer from `Ouroboros.Consensus.Storage.ChainDB.Serialisation`. The dependency was previously in the wrong direction, `Common` should not rely on `ChainDB`.
Fixes #2239. Instead of having a `RunNode` method to get the `BinaryBlockInfo` of a block (offset and size of the header within the block), create a separate type class `HasBinaryBlockInfo` for it. This is another step in the clean-up process of `RunNode` that started in the weeks before the HFC was merged. This means we can provide a generic implementation of `HasBinaryBlockInfo` for `HardForkBlock` that takes the HFC wrapper into account. This wrapper adds a listlen 2 and a tag indicating the era to the start of each block, so we need to shift the offset of the header by 2 bytes. More importantly, we now allow overriding the `getBinaryBlockInfo` method in `SerialiseHFC`, just like we allow overriding the disk en/decoder for blocks. This allows us to do the right thing for the Byron era: no era wrapper for Byron, but start using era wrappers for the eras after it. This also fixes a bug: `cardano-node` running in pure Shelley mode uses `ShelleyHFC`, i.e., a `ShelleyBlock` wrapped in the HFC. Since the generic `RunNode` instance for `HardForkBlock '[blk]` forwards everything to the instance for `blk`, we were using the `BinaryBlockInfo` implementation for Shelley, which didn't account for the era wrapper in the header offset. Since we *were* using the era wrapper to serialise `ShelleyHFC` blocks on disk, this meant we were extracting the header from the on-disk blocks incorrectly, resulting in deserialisation failures when reading headers from disk. This refactoring allows us to fix it properly: now the defaults for `SerialiseHFC` do the right thing for `ShelleyHFC`, and for `ByronHFC` we forward to `ByronBlock`, just like for all the other methods. This means `ShelleyHFC` starts out with an era wrapper around blocks from the first era (to allow for easier transitions to later eras, which is the right thing to do from the start), but `ByronHFC`'s block format is binary compatible with `ByronBlock`.
|
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.
Fixes #2239.
Instead of having a
RunNodemethod to get theBinaryBlockInfoof ablock (offset and size of the header within the block), create a separate type
class
HasBinaryBlockInfofor it. This is another step in the clean-up processof
RunNodethat started in the weeks before the HFC was merged.This means we can provide a generic implementation of
HasBinaryBlockInfoforHardForkBlockthat takes the HFC wrapper into account. This wrapper adds alistlen 2 and a tag indicating the era to the start of each block, so we need to
shift the offset of the header by 2 bytes.
More importantly, we now allow overriding the
getBinaryBlockInfomethod inSerialiseHFC, just like we allow overriding the disk en/decoder for blocks.This allows us to do the right thing for the Byron era: no era wrapper for
Byron, but start using era wrappers for the eras after it.
This also fixes a bug:
cardano-noderunning in pure Shelley mode usesShelleyHFC, i.e., aShelleyBlockwrapped in the HFC. Since the genericRunNodeinstance forHardForkBlock '[blk]forwards everything to theinstance for
blk, we were using theBinaryBlockInfoimplementation forShelley, which didn't account for the era wrapper in the header offset. Since
we were using the era wrapper to serialise
ShelleyHFCblocks on disk, thismeant we were extracting the header from the on-disk blocks incorrectly,
resulting in deserialisation failures when reading headers from disk.
This refactoring allows us to fix it properly: now the defaults for
SerialiseHFCdo the right thing forShelleyHFC, and forByronHFCweforward to
ByronBlock, just like for all the other methods. This meansShelleyHFCstarts out with an era wrapper around blocks from the first era (toallow for easier transitions to later eras, which is the right thing to do from
the start), but
ByronHFC's block format is binary compatible withByronBlock.