feat(alpenglow): introducing BlockComponent#10090
Conversation
ksn6
force-pushed
the
upstream-block-component
branch
from
f56e006 to
d60a792
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #10090 +/- ##
=========================================
- Coverage 82.6% 82.5% -0.1%
=========================================
Files 844 845 +1
Lines 316634 316876 +242
=========================================
+ Hits 261631 261707 +76
- Misses 55003 55169 +166 🚀 New features to boost your workflow:
|
entry/src/block_component.rs
Outdated
| const TYPE_META: TypeMeta = match T::TYPE_META { | ||
| TypeMeta::Static { size, zero_copy } => TypeMeta::Static { | ||
| size: size + std::mem::size_of::<u16>(), | ||
| zero_copy, |
There was a problem hiding this comment.
I don't think LengthPrefixed should ever be zero_copy: true.
Presumably this design anticipates variable length payloads, given that we write an explicit length prefix in serialization. This would disqualify it for zero-copy generally from a design perspective.
Structurally, LengthPrefixed's layout wouldn't support zero-copy either, as it doesn't actually house a length value (it's ignored in deserialization and not specified in the struct definition). As such, a raw byte read of size_of::<u16>() + size_of::<T>() bytes would be invalid to initialize T by definition.
There was a problem hiding this comment.
Sure, makes sense - setting zero_copy to false in the write and read cases impl's.
entry/src/block_component.rs
Outdated
| type Src = Self; | ||
|
|
||
| const TYPE_META: TypeMeta = match T::TYPE_META { | ||
| TypeMeta::Static { size, .. } => TypeMeta::Static { |
There was a problem hiding this comment.
It's probably not static either right? Or am I misunderstanding?
There was a problem hiding this comment.
I guess if you KNEW up-front the T that you're deserializing you could conceivably use static. But presumably for the intended use-case T must be treated as opaque? I.e., should LengthPrefixed actually contain a blob of bytes rather than some T (e.g., Vec<u8>), or is the case that for the purposes of this module we'll always know T, but downstream consumption code will have to treat it as opaque?
There was a problem hiding this comment.
Discussed offline - we'll want to keep this as-is, since TYPE_META is inferred based on T and LengthPrefixed is purely used for serialization purposes.
There was a problem hiding this comment.
Turns out we can simplify this substantially and get zero-copy going as well. Thanks @cpubot !
ksn6
force-pushed
the
upstream-block-component
branch
from
18e023d to
d8cbc59
Compare
ksn6
force-pushed
the
upstream-block-component
branch
from
6f07ea2 to
d5ad9a6
Compare
4 participants
Problem
We need a unified, extensible data structure to represent the components of a Solana block, covering both normal transaction entries and special markers.
The structure must support a number of Alpenglow-related needs, e.g.:
Markers for Fast Leader Handover: SIMD-0337: Markers for Alpenglow Fast Leader Handover solana-foundation/solana-improvement-documents#337
Reasoning about the Alpenglow clock: SIMD-0363: Simple Alpenglow Clock solana-foundation/solana-improvement-documents#363
Block footer markers: SIMD-0307: Add Block Footer solana-foundation/solana-improvement-documents#307
Validator tickets: SIMD-0357: Alpenglow VAT implementation solana-foundation/solana-improvement-documents#357
Storing double-Merkle roots
... all the while remaining flexible for new marker types in the future.
Summary of Changes
We introduce a new enum,
BlockComponent, which represents one logical element within a block. This code was originally designed in thealpenglowrepository.