Remove dead code related to accounts migration #6311
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-5
branch
2 times, most recently
from
694bf91 to
fb76dca
Compare
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-4
branch
from
ad26122 to
19088a5
Compare
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-5
branch
2 times, most recently
from
e35a5b4 to
b1a498c
Compare
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-4
branch
from
19088a5 to
b6bbedc
Compare
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-5
branch
from
b1a498c to
08416b5
Compare
|
The diff looks much larger than what the PR description describes. Did you make changes to the merge base? |
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-4
branch
4 times, most recently
from
3930cab to
8f323c8
Compare
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-5
branch
2 times, most recently
from
a34a6a6 to
00dc316
Compare
jasonz-dfinity
changed the title
I started removing the proxy code and the diffs just got bigger and bigger. I repurposed this PR for those that can already by removed without changing the AccountsDbProxy. The next PR will be about AccountsDbProxy. PTAL |
dskloetd
approved these changes
Feb 4, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 4, 2025
…_db (#6302) # Motivation The `AccountsDb::Map(AccountsDbAsMap)` enum value (i.e. representing accounts as a map in heap memory) is deprecated, and we'd like to remove this case (and only have the branch of code for the `AccountsDb::UnboundedStableBTreeMap(AccountsDbAsUnboundedStableBTreeMap<>)`). However, currently it's impossible, since `AccountsStore::decode` calls `AccountsDbAsProxy::default()`, which is an instance of the deprecated type. The `accounts_db` is later [replaced](https://sourcegraph.com/github.com/dfinity/nns-dapp@395ad2a74c9309b2f6b943bce3964921904d6e36/-/blob/rs/backend/src/state.rs?L147) so that the production code indeed uses the `StableBTreeMap`. Therefore, this PR aims at removing the default-then-replace approach, by initializing the `AccountsDb::UnboundedStableBTreeMap` directly. # Changes - Inline and remove `State::save_heap_to_managed_memory` and `State::recover_heap_from_managed_memory`, since the logic is simple (so the indirection is no longer necessary) - Move some of the logic from `State::new_restored` into `State::decode` and remove the `default`/`replace_accounts_db` logic (which is the main goal of the PR) - Move the `accounts_store` initialization logic from `State::new` to `AccountsStore::new`, to keep StableBTreeMap initialization logic close to each other # Tests N/A since there are no functionality changes. The test in `rs/backend/src/state/tests.rs` should (still) cover the code being changed in this PR. # Related PRs [Previous](#6301) | [Next](#6311)
jasonz-dfinity
force-pushed
the
jason/NNS1-3030-5
branch
from
00dc316 to
e01349f
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 4, 2025
# Motivation There are currently 3 layers of indirection: `AccountsStore`->`AccountsDbAsProxy`->`AccountsDb`->`AccountsDbAsUnboundedStableBTreeMap`->`StableBTreeMap`. This PR inlines the `AccountsDbAsUnboundedStableBTreeMap` into `AccountsStore` directly, since each layer is very thin. # Changes * Change `AccountsStore::accounts_db` from `AccountsDbAsProxy` to `StableBTreeMap` * Inline the all the function calls related to `StableBTreeMap` * Refactor `impl PartialEq/Eq/Default` from `AccountsDbAsProxy` to `AccountsStore` (since the former doesn't exist anymore, and the latter can no longer derive because of `StableBTreeMap` does not have those traits) # Tests N/A # Related PR [Previous](#6311)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Some of the migration code is already dead. Either they are marked as
#allow[dead_code], or they are structs/enums not used anymore.Changes
SchemaLabeland serialization/deserialization code around itstep_migrationcanister method (it's an internal method since it checks the caller, so no backward compatibility problems)AccountsDbAsProxy::migrationis alwaysNone, remove the field and simplifies the code pathsTests
N/A
Related PRs
Previous | Next