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

Light client contract related changes #3655

Merged
merged 18 commits into from
Sep 6, 2024
Merged

Conversation

philippecamacho
Copy link
Contributor

@philippecamacho philippecamacho commented Sep 6, 2024

Part of EspressoSystems/espresso-sequencer#1938

This PR:

The purpose of this PR is to update the GenericLightClientState struct (removing 4 fields) and introduce a new struct
GenericStakeTableState to represent the stake table state (which is fixed). These changes are required to account for a refactoring of the Light Client contract which purpose is to remove all logic related to epochs. See EspressoSystems/espresso-sequencer#1938 for more context.

Key places to review:

Only crates/types/src/light_client.rs as other changes are due to running cargo fmt.

As usual (see README.md)

@philippecamacho philippecamacho changed the title Lc contract updates Light client contract related changes Sep 6, 2024
@philippecamacho philippecamacho merged commit f532baa into main Sep 6, 2024
33 of 35 checks passed
@philippecamacho philippecamacho deleted the lc-contract-updates branch September 6, 2024 16:41
Comment on lines +249 to +255
self.0[5]
}

/// Return the threshold
#[must_use]
pub fn threshold(&self) -> F {
self.0[6]
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these magic numbers are kind of tricky to reason about, can we just use a named struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alxiong @alysiahuggins do you remember if there is any particular reason why we don't use a named struct?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is the decision made by @mrain and i recall the public input right now is simple enough to keep track of?
i also personally prefer a named struct, less error prone.

but i tried to make minimal changes for this task, so kept the legacy way. Can update in the future as tech debt.

@philippecamacho philippecamacho restored the lc-contract-updates branch September 6, 2024 21:20
@Ancient123 Ancient123 deleted the lc-contract-updates branch October 7, 2024 21:34
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.

6 participants