-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
chore: drop legacy CDeterministicMNState
formats, evodb upgrade logic
#6579
Conversation
Co-authored-by: UdjinM6 <[email protected]>
WalkthroughThe pull request removes legacy support for migrating EvoDb databases and handling outdated formats within the deterministic masternode management code. The documentation has been updated to advise users upgrading from versions 19 and 20 to reindex their databases. In the code, functions and logic related to migrating or reading from older EvoDB formats have been eliminated. Specifically, migration methods in the deterministic masternode manager and compatibility constructors for legacy masternode states were removed, as well as constants and enumeration values related to older formats and upgrade errors. Modifications in the chainstate loading process also remove specific calls and error handling for EvoDb migrations, thereby streamlining the overall initialization, serialization, and state management processes. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/evo/deterministicmns.h (1)
76-79
: Consider providing more user guidance in the exception message.
Although throwing an exception is correct for dropping older formats, you might include additional steps or references within this message (e.g., pointing users to release notes) to help them troubleshoot reindex requirements more easily.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
doc/release-notes-6579.md
(1 hunks)src/evo/deterministicmns.cpp
(0 hunks)src/evo/deterministicmns.h
(2 hunks)src/evo/dmnstate.h
(0 hunks)src/init.cpp
(0 hunks)src/node/chainstate.cpp
(0 hunks)src/node/chainstate.h
(0 hunks)
💤 Files with no reviewable changes (5)
- src/node/chainstate.cpp
- src/node/chainstate.h
- src/evo/deterministicmns.cpp
- src/evo/dmnstate.h
- src/init.cpp
✅ Files skipped from review due to trivial changes (1)
- doc/release-notes-6579.md
🔇 Additional comments (3)
src/evo/deterministicmns.h (3)
84-84
: No further concerns on thepdmnState
field.
READWRITE forpdmnState
is consistent with dropping old states and uses the updated format logic correctly.
88-88
: Good approach for handling type properly based on version.
The version check ensures backward compatibility for older network peers by defaulting toMnType::Regular
.
214-214
: Logic looks good for deserializing and adding masternodes.
Creation ofCDeterministicMN
instances using the new format version is aligned with the removal of old formats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 39f76c8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 39f76c8
Additional Information
CDeterministicMNState_
{Oldformat
,mntype_format
} was introduced in dash#5039 and dash#5403, included in Dash Core v19 and v20 respectively.The above change has been in the codebase for more than two major releases and does not concern databases with a long shelf life (like wallets), this pull request removes them entirely.
Breaking Changes
Users upgrading from these versions are now expected to
-reindex
.Checklist: