Re-work locked gold requirements for validators and groups #1474
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1474 +/- ##
=======================================
Coverage 73.61% 73.61%
=======================================
Files 277 277
Lines 7443 7443
Branches 956 956
=======================================
Hits 5479 5479
Misses 1852 1852
Partials 112 112
Continue to review full report at Codecov.
|
| Validator storage validator = validators[account]; | ||
| if (validator.affiliation != address(0)) { | ||
| _deaffiliate(validator, account); | ||
| require(!groups[validator.affiliation].members.contains(account)); |
There was a problem hiding this comment.
Would this assertion always fail, or will the validator always be removed from the group's affiliation list before deregisterValidator is called?
There was a problem hiding this comment.
The validator needs to be removed from the group's membership list before deregisterValidator is called. Furthermore, it needs to have been removed from this list more than validatorLockedGoldRequirements.duration seconds ago.
The reason we need to check to see if it's currently a member of the group with which it's affiliated is because the lastRemovedFromGroupTimestamp isn't relevant information if the validator is still a member of a group.
| @@ -596,12 +516,12 @@ contract Validators is | |||
| require(commission <= FixidityLib.fixed1().unwrap(), "Commission can't be greater than 100%"); | |||
| address account = getLockedGold().getAccountFromActiveValidator(msg.sender); | |||
| require(!isValidator(account) && !isValidatorGroup(account)); | |||
| require(meetsValidatorGroupBalanceRequirements(account)); | |||
|
|
|||
| uint256 lockedGoldBalance = getLockedGold().getAccountTotalLockedGold(account); | |||
There was a problem hiding this comment.
Should these two lines be changed to use meetsAccountLockedGoldRequirements instead?
There was a problem hiding this comment.
No, only because the group hasn't been registered yet, so the requirement is 0. If we did this check after registration (i.e. moved this line to the end of the function) then we could use meetsAccountLockedGoldRequirements
* master: (73 commits) Fix Ethstats Image reference (#1577) EU Cookies Behavior Change (#1447) [verifier] Upgrade to RN 61 (#1572) [Wallet] Update link styles and Implement VerificationEducationScreen (#1565) [wallet] Added native phone picker (#1310) [Wallet] Set up new verification screen skeletons (#1563) Bump e2e test migrate numbers where needed (#1567) [Wallet] Create new carousel component (#1555) [Wallet] Protect Backup Key and Safeguards with PIN (#1556) Increase ganache gas limit (#1569) Re-work locked gold requirements for validators and groups (#1474) Fix e2e on CI (#1537) Allow a specified address to disable/enable the Exchange (#1467) Avoid re-encrypting key files with yarn keys:encrypt command (#1560) Support protocol hotfixing (#613) Point e2e tests back (#1562) Refactor to Accounts.sol (#1392) Add selectIssuers Transaction (#1327) [Wallet] Get React Native Hot Reloading Working (#1551) Unify to prefix messages for signing (#1473) ...
Description
This PR re-works locked gold requirements to validators and groups. Specifically, validators and groups now must adhere to these requirements until de-registration, after which the requirements are lifted. Validators and groups are prevented from deregistering if they were in a group/had members in their group recently.
Tested
Unit tests
Other changes
UsingPrecompilescontractRelated issues