Remove BuiltInFunctionOnMetaEnableEpoch and FixWaitingList #4936
Conversation
mariusmihaic
changed the title
| // IsTransferToMetaFlagEnabled returns true if builtInFunctionOnMetaFlag is enabled | ||
| // this is a duplicate for BuiltInFunctionOnMetaEnableEpoch needed for consistency into vm-common | ||
| // IsTransferToMetaFlagEnabled returns false | ||
| // This is used for consistency into vm-common |
There was a problem hiding this comment.
we should update and simplify on vm-common as well. We do not need this function
There was a problem hiding this comment.
Will add a task to delete it on vm-common as well then, but on a separate PR
| @@ -753,7 +753,7 @@ func (ihnc *indexHashedNodesCoordinator) computeNodesConfigFromList( | |||
| newNodesList := make([]Validator, 0) | |||
| auctionList := make([]Validator, 0) | |||
|
|
|||
| if ihnc.flagWaitingListFix.IsSet() && previousEpochConfig == nil { | |||
| if previousEpochConfig == nil { | |||
There was a problem hiding this comment.
if ihnc.flagStakingV4Started.IsSet() && previousEpochConfig == nil { ?
There was a problem hiding this comment.
After discussions with @AdoAdoAdo , we will delete this "previousEpochConfig" completely in the next PR anyway, but I'll add it for consistency
…akingV4' into EN-13754-remove-ls-flags-from-stakingV4
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## feat/staking-v4 #4936 +/- ##
==================================================
Coverage ? 70.87%
==================================================
Files ? 662
Lines ? 86493
Branches ? 0
==================================================
Hits ? 61299
Misses ? 20644
Partials ? 4550 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| found, shardId := searchInMap(previousEpochConfig.eligibleMap, currentValidator.PubKey()) | ||
| if found { | ||
| log.Debug("leaving node found in", "list", "eligible", "shardId", shardId) | ||
| eligibleMap[shardId] = append(eligibleMap[currentValidatorShardId], currentValidator) |
There was a problem hiding this comment.
...append(eligibleMap[shardId] ?
| found, shardId = searchInMap(previousEpochConfig.waitingMap, currentValidator.PubKey()) | ||
| if found { | ||
| log.Debug("leaving node found in", "list", "waiting", "shardId", shardId) | ||
| waitingMap[shardId] = append(waitingMap[currentValidatorShardId], currentValidator) |
There was a problem hiding this comment.
...append(waitingMap[shardId] ?
| return | ||
| } | ||
|
|
||
| log.Debug("leaving node not in eligible or waiting, probably was in auction/inactive/jailed", |
There was a problem hiding this comment.
currentValidatorShardId instead shardId. I think in this point shardId will have not value set, probably 0, as found were false in both searching above.
Reasoning behind the pull request
BuiltInFunctionOnMetaEnableEpoch,WaitingListFixEnableEpochsharding/nodesCoordinator/indexHashedNodesCoordinator.gowhen computingcomputeNodesConfigFromListfor previous nodes config. When having a leaving node,addValidatorToPreviousMapwould always consider that node was eligible.Proposed changes
BuiltInFunctionOnMetaEnableEpoch+WaitingListFixEnableEpochflagWaitingListFixinaddValidatorToPreviousMap, but fromflagStakingV4Startedand completely removed it fromhashValidatorShuffler. This is mandatory, since an auction unstaked node, would've been considered as previously elgibile otherwise. Therefore, in case offorced/actually remainingnodes, that auction node could've been directly sent to eligible nodes, without being selected with sufficient topUpTesting procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?