Directly use deterministicMNManager instead of compat code in CMasternodeMan #2594
Conversation
…ministicMNManager
And remove them with direct use of deterministicMNManager
…e used directly Additionally, implement GetLastDsq in CMasternodeMan as a replacement for direct access to the masternode_info_t object. Will move this variable to another (PS specific) place later.
And adapt it to directly use deterministicMNManager
Has to be replaced with something new in the future.
…ticMNCPtr This leaves us with nMinProtocol unused, but this is ok as we will later remove that argument completely.
…ing DMNs now It's ok to remove this code as we're later going to completely remove it. Until then, this code is dead nevertheless.
Also remove GetFullMasternodeMap
This relies on mnInfo which is not present anymore as we directly use deterministicMNManager now.
| mn.nProtocolVersion << " " << | ||
| payeeStr << " " << | ||
| (int64_t)mn.lastPing.sigTime << " " << std::setw(8) << | ||
| (int64_t)(mn.lastPing.sigTime - mn.sigTime) << " " << std::setw(10) << |
There was a problem hiding this comment.
should keep << std::setw(10) <<
(can't create suggestions on deleted lines :/ )
src/rpc/masternode.cpp
Outdated
| " payee - Print Dash address associated with a masternode (can be additionally filtered,\n" | ||
| " partial match)\n" | ||
| " protocol - Print protocol of a masternode (can be additionally filtered, exact match)\n" | ||
| " keyid - Print the masternode (not collateral) key id\n" |
There was a problem hiding this comment.
Side note: keyid seems to be not used anywhere and keyID(Owner/Operator/Voting) neither listed in help nor accepted as a legit option in masternodelist 🙈
There was a problem hiding this comment.
I removed keyid for now. Not sure how to proceed with the missing options, as I'm not sure how we'll proceed with parallel support of protx list and masternode list.
There was a problem hiding this comment.
While reading the comments that came after this one I realized that the help text needed some more fixes nevertheless, including keyID(Owner/Operator/Voting). So, this is fixed now.
| payeeScript = GetScriptForDestination(mn.keyIDCollateralAddress); | ||
|
|
||
| auto mnList = deterministicMNManager->GetListAtChainTip(); | ||
| auto dmnToStatus = [&](const CDeterministicMNCPtr& dmn) { |
There was a problem hiding this comment.
Side note: feels like we should move the logic for IsValid() and IsPoSeBanned() and their string representation (this piece -> GetStatus()) into CDeterministicMNState. Basically, replicate the same interface we have for non-deterministic MNs already (and do not expose the logic to rpc or whatever).
There was a problem hiding this comment.
Hmm yeah sounds like a refactoring is needed here. I've put it onto my TODO list.
| } | ||
| return "UNKNOWN"; | ||
| }; | ||
| auto dmnToLastPaidTime = [&](const CDeterministicMNCPtr& dmn) { |
There was a problem hiding this comment.
Same here, this could be e.g. GetLastPaidTime() in CDeterministicMNState
Co-Authored-By: codablock <[email protected]>
1. Make strMode lower case before comparing it (keyIDXXX would otherwise be case sensitive, leading to confusion for users) 2. Remove "rank" and "keyid" mode from help 3. Add keyIDOwner/keyIDVoting/pubKeyOperator to help and strMode check 4. Remove "pubkey" from strMode check 5. Call ToString() on address instead of whole DMN state 6. Add missing std::setw(10) to "full" mode
|
Fixed and pushed code review fixes |
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
Slightly tested (synced, reindexed, tried a couple of masternode list commands).
ACK
It removes 2 unused flags: - BIP9CheckMasternodesUpgraded from CChainParams - check_mn_protocol from versionbitsinfo These flags have no meaning since 17c792c (PR dashpay#2594) The TODO in removed code is super-seeded by new way to validate MN version in hard-fork (see dashpay#5469 and DIP0023 for more details)
## Issue being fixed or feature implemented During implementation #5469 (master node hard-fork) I noticed that some parts of `CChainParams` are deprecated and can be removed. ## What was done? 1. removed methods from `CChainParams` that have no implementation at all: - UpdateSubsidyAndDiffParams - UpdateLLMQChainLocks - UpdateLLMQTestParams - UpdateLLMQDevnetParams 2. removed method `BIP9CheckMasternodesUpgraded` from `CChainParams` and a flag `check_mn_protocol` from `versionbitsinfo`. (to follow-up #2594) ## How Has This Been Tested? Run functional/unit tests. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
2 participants
This is extracted from #2566. It removes most uses of
CMasternodeManand replaces it with direct use ofdeterministicMNManager. I tried to avoid mixing in code removal as much as possible, but this was still required in some places wheredeterministicMNManagercan not provide the same (legacy) functionality. I also had to remove support for legacy operator keys in variousSign/Verifymethods as the legacy operator pubkey is not available anymore when directly usingdeterministicMNManager.This leaves
CMasternodeManand most of its code unused. It's now only used for governance and PS metadata. The next PR will remove all unused code and the PR after that will refactorCMasternodeManintoCMasternodeMetaMan(as seen in #2566).