SIMD-0402: Finalization Cert in Block Footer #402
Conversation
| +---------------------------------------+ | ||
| | block_user_agent (0-255 bytes) | | ||
| +---------------------------------------+ | ||
| | final_cert_len (2 bytes) | ← NEW |
There was a problem hiding this comment.
Could we have this be 1 byte, where it's 0x00 if there's not final_cert and 0x01 if there is a final_cert?
There was a problem hiding this comment.
hmm, 1 byte only encodes up to 255, we will have up to ~1300 bytes for 2000 validators, we can do length * 8, then we can encode the length in 1 byte, what will wincode prefer here?
There was a problem hiding this comment.
to clarify, I'm saying that final_cert_len shouldn't actually be a len; rather, it should purely denote a bool that states true or false as to whether there exists a final_cert
| If `final_cert` is present, it is serialized as a whole using | ||
| `wincode::serialize`. |
There was a problem hiding this comment.
we should explicitly write out the serialization format in tabular form as done above.
in particular:
-
notar_aggregate: Option<VotesAggregate>should use a singleu8to denote whether aVotesAggregateis present (i.e.,0x00if not present and0x01if present) -
by default
wincodewill use8bytes to serialize the length ofbitmap; we could probably get away with a fewer number of bytes, right? e.g., would 2 bytes work?
|
|
||
| ## Summary | ||
|
|
||
| This SIMD introduces addition of an Alpenglow finalization certificate to the |
There was a problem hiding this comment.
something is wrong here (grammatically), I don't quite get the sentence.
There was a problem hiding this comment.
maybe we want to be more specific? we want to say that we add a finalization certificate such that somebody who just observes blocks understands that they are finalized without having to understand the all-to-all communication between the validators.
There was a problem hiding this comment.
Thanks for the suggestion. Changed.
You can also make comments in the Google doc if that's easier.
| This shift removes the on-chain visibility previously provided by vote | ||
| transactions and vote account state, which zero-staked validators currently | ||
| rely on to infer validator delinquency. It also affects any party that depends | ||
| on the ability to observe votes on-chain. To address this, Alpenglow SIMD |
There was a problem hiding this comment.
(remove "SIMD", it sounds wrong)
| +---------------------------------------+ | ||
| | block_user_agent (0-255 bytes) | | ||
| +---------------------------------------+ | ||
| | final_cert_present (1 byte) | ← NEW |
There was a problem hiding this comment.
It might make sense to use this field to also indicate if the notar and skip reward certs are present. So we are not using additional bytes for them. Maybe we want to roll the reward certs into this SIMD?
There was a problem hiding this comment.
It is chosen this way because this is the natural translation of
struct {
final_cert: Option<FinalCertificate>,
}
from Rust in bincode/wincode serialization.
There was a problem hiding this comment.
I like @akhi3030 's suggestion, though more strongly prefer avoiding custom serde.
I'm checking with the wincode authors to see whether this is something we can support. For now, though, would rather use different bytes to denote whether different certs are present.
| +---------------------------------------+ | ||
| | block_user_agent_len (1 byte) | | ||
| +---------------------------------------+ | ||
| | block_user_agent (0-255 bytes) | |
There was a problem hiding this comment.
in previous SIMDs, the feedback was to keep this field in the end so that all fields have a fixed offset. This SIMD is introducing a new variable field. So at the least, we may want to swap things around so that all the fixed length fields come first and then all the variable length ones.
There was a problem hiding this comment.
See above, if we swap things around then natural bincode/wincode serialization doesn't work any more, we need to write customized serialization.
| +---------------------------------------+ | ||
| | notar_aggregate_signature (variable) | | ||
| +---------------------------------------+ | ||
| | notar_aggregate_bitmap_len (variable) | |
There was a problem hiding this comment.
notar_aggregate_bitmap_len should be 2 bytes
There was a problem hiding this comment.
Wrote variable basically because notar_aggregate is optional here.
5 participants
No description provided.