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

dynamic versioning #1878

Merged
merged 18 commits into from
Aug 22, 2024
Merged

dynamic versioning #1878

merged 18 commits into from
Aug 22, 2024

Conversation

imabdulbasit
Copy link
Contributor

@imabdulbasit imabdulbasit commented Aug 20, 2024

This PR:

  • Adds both base and upgrade versions to the genesis.toml file and deserializes them from string format into the Version struct.
  • Initializes the base and upgrade versions in binaries from the genesis.toml file.
  • separates the API version from the protocol versions. The SequencerApiVersion is currently hardcoded as V0.1 for all APIs.
  • Renames the main function to run in binaries that require a protocol version, run() accepts a generic parameter V that implements the Versions trait. This allows matching and initializing versions from the genesis.toml file using the appropriate type. This can be replaced with a macro
  • adds message compat test for each version

Files to review:
all files mostly but primary focus on genesis.toml config, SequencerVersions type and main functions in all binaries.

test:
Run the demo native locally to ensure everything functions as expected.

@imabdulbasit imabdulbasit changed the title WIP - dynamic versioning dynamic versioning Aug 20, 2024
@imabdulbasit imabdulbasit marked this pull request as ready for review August 20, 2024 19:30
@imabdulbasit imabdulbasit marked this pull request as draft August 21, 2024 01:54
@imabdulbasit imabdulbasit marked this pull request as ready for review August 21, 2024 09:00
Copy link
Contributor

@tbro tbro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. There is a little bit of duplication w/ #1872, so I'd like to look through one more time after that gets merged. Should be a smaller change set at that time.

builder/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@tbro tbro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a couple of minor comments regarding the aliases.

types/src/v0/mod.rs Show resolved Hide resolved

#[derive(Debug, Copy, Clone)]
pub struct SequencerVersions;
pub type BaseV01UpgradeV02 = SequencerVersions<StaticVersion<0, 1>, StaticVersion<0, 2>>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are defining aliases for these, it might be a little easier to keep in sync if we use the alias everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is used for test network mostly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I should rename it to MockSequencerVersions

@imabdulbasit imabdulbasit merged commit 18b96df into main Aug 22, 2024
15 checks passed
@imabdulbasit imabdulbasit deleted the ab/dynamic-versioning branch August 22, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants