Skip to content
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

[Minor Versioning] - Add deserialize_for_version trait #3309

Open
8 tasks
nomaxg opened this issue Jun 11, 2024 · 1 comment
Open
8 tasks

[Minor Versioning] - Add deserialize_for_version trait #3309

nomaxg opened this issue Jun 11, 2024 · 1 comment

Comments

@nomaxg
Copy link

nomaxg commented Jun 11, 2024

What is this task and why do we need to work on it?

After we introduce minor version upgradability to the sequencer, we will need a trait that deserializes types such as the Header + Payload by version.

What work will need to be done to complete this task?

  • Create a deserialize_for_version trait that can be implemented in sequencer
  • Remove Delta trait entirely (we ended up not dealing with state deltas generically, since this can just be a concrete type within the sequencer). This is important because currently we have Delta in the Decide event, which requires it to be serializable, but we don't need this
  • Remove ValidatedState from Decide event. Same as above, we don't use this anymore in the sequencer and we don't want ValidatedState to be serializable.
  • Remove Serialize, Deserialize trait bounds from ValidatedState trait (this should be trivial after the above)
  • Replace Deserialize trait bound with DeserializeVersion in remaining HotShot traits (BlockHeader, BlockPayload, any others?)
  • Wherever we need to deserialize messages containing these objects, we need to use deserialize_for_version and make sure we have the consensus version plumbed into that part of the code. This is the part I'm not sure about, might be hard.

Are there any other details to include?

No response

What are the acceptance criteria to close this issue?

  • Add deserialize_for_version test that serialize some mock types to different representation depending on some version enum
  • No HotShot traits require Deserialize anymore

Branch work will be merged to (if not the default branch)

No response

@jbearer
Copy link
Member

jbearer commented Jun 27, 2024

Actually BlockPayload already doesn't use serde, it has its own encode/from_bytes methods, so we can just add a version parameter to from_bytes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants