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

Remove support for storage layout v9 #2372

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Conversation

frederikrothenberger
Copy link
Member

@frederikrothenberger frederikrothenberger commented Mar 18, 2024

This PR drops support for storage layout v9. The following simplifications are implemented as well:

  • Drop the Option fields from PersistentState. These were only kept for stable memory compatibility with v8.
  • Make loading the PersistentState infallible.
  • Moved initialization of default values from StorablePersistentState to PersistentState.

🟡 Some screens were changed

This PR drops support for storage layout v9. The following
simplifications are implemented as well:
* Drop the `Option` fields from `PersistentState`. These were only kept
  for stable memory compatibility with v8.
* Make loading the `PersistentState` infallible.
* Moved initialization of default values from `StorablePersistentState`
  to `PersistentState`.
@@ -16,10 +16,7 @@ use std::cmp::min;
/// There is a maximum of `max_tokens` tokens, when reached the tokens not increase any further.
/// This is the maximum number of calls that can be handled in a burst.
pub fn process_rate_limit() {
let Some(config) = state::persistent_state(|ps| ps.registration_rate_limit.clone()) else {
// rate limit disabled -> nothing to do
Copy link
Member Author

Choose a reason for hiding this comment

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

Rate limit can no longer be not enabled.

@@ -138,106 +138,103 @@ fn persistent_state_metrics(
w: &mut MetricsEncoder<Vec<u8>>,
persistent_state: &PersistentState,
) -> Result<(), std::io::Error> {
if let Some(ref register_rate_limit_config) = persistent_state.registration_rate_limit {
let register_rate_limit_config = &persistent_state.registration_rate_limit;
Copy link
Member Author

Choose a reason for hiding this comment

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

The diff in this file is just unnesting if let Some(..) = ... { constructs

// immediately migrate the persistent state to the new layout
state::storage_borrow_mut(|storage| storage.migrate_persistent_state());
// load the persistent state after initializing storage, otherwise the memory address to load it from cannot be calculated
state::load_persistent_state();
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved loading the persistent state upwards, because logically it seems to make more sense. In the future, I'll include it in the init_from_stable_memory function to make it a single action.

Copy link
Collaborator

@przydatek przydatek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

src/internet_identity/src/storage.rs Outdated Show resolved Hide resolved
src/internet_identity/src/storage.rs Outdated Show resolved Hide resolved
@frederikrothenberger frederikrothenberger added this pull request to the merge queue Mar 19, 2024
Merged via the queue into main with commit 90c8902 Mar 19, 2024
63 checks passed
@frederikrothenberger frederikrothenberger deleted the frederik/remove-v8 branch March 19, 2024 09:50
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