Re-work locked gold requirements for validators and groups (#1474)

packages/celotool/src/e2e-tests/validator_order_tests.ts

@@ -10,7 +10,7 @@ const BLOCK_COUNT = EPOCH * EPOCHS_TO_WAIT
 
 describe('governance tests', () => {
   const gethConfig: GethTestConfig = {
-    migrateTo: 13,
+    migrateTo: 14,
     instances: _.range(VALIDATORS).map((i) => ({
       name: `validator${i}`,
       validating: true,

packages/contractkit/src/wrappers/Validators.ts

@@ -24,19 +24,14 @@ export interface ValidatorGroup {
   commission: BigNumber
 }
 
-export interface BalanceRequirements {
-  group: BigNumber
-  validator: BigNumber
-}
-
-export interface DeregistrationLockups {
-  group: BigNumber
-  validator: BigNumber
+export interface LockedGoldRequirements {
+  value: BigNumber
+  duration: BigNumber
 }
 
 export interface ValidatorsConfig {
-  balanceRequirements: BalanceRequirements
-  deregistrationLockups: DeregistrationLockups
+  groupLockedGoldRequirements: LockedGoldRequirements
+  validatorLockedGoldRequirements: LockedGoldRequirements
   maxGroupSize: BigNumber
 }
 
@@ -72,26 +67,26 @@ export class ValidatorsWrapper extends BaseWrapper<Validators> {
     }
   }
   /**
-   * Returns the current registration requirements.
-   * @returns Group and validator registration requirements.
+   * Returns the Locked Gold requirements for validators.
+   * @returns The Locked Gold requirements for validators.
    */
-  async getBalanceRequirements(): Promise<BalanceRequirements> {
-    const res = await this.contract.methods.getBalanceRequirements().call()
+  async getValidatorLockedGoldRequirements(): Promise<LockedGoldRequirements> {
+    const res = await this.contract.methods.getValidatorLockedGoldRequirements().call()
     return {
-      group: toBigNumber(res[0]),
-      validator: toBigNumber(res[1]),
+      value: toBigNumber(res[0]),
+      duration: toBigNumber(res[1]),
     }
   }
 
   /**
-   * Returns the lockup periods after deregistering groups and validators.
-   * @return The lockup periods after deregistering groups and validators.
+   * Returns the Locked Gold requirements for validator groups.
+   * @returns The Locked Gold requirements for validator groups.
    */
-  async getDeregistrationLockups(): Promise<DeregistrationLockups> {
-    const res = await this.contract.methods.getDeregistrationLockups().call()
+  async getGroupLockedGoldRequirements(): Promise<LockedGoldRequirements> {
+    const res = await this.contract.methods.getGroupLockedGoldRequirements().call()
     return {
-      group: toBigNumber(res[0]),
-      validator: toBigNumber(res[1]),
+      value: toBigNumber(res[0]),
+      duration: toBigNumber(res[1]),
     }
   }
 
@@ -100,13 +95,13 @@ export class ValidatorsWrapper extends BaseWrapper<Validators> {
    */
   async getConfig(): Promise<ValidatorsConfig> {
     const res = await Promise.all([
-      this.getBalanceRequirements(),
-      this.getDeregistrationLockups(),
+      this.getValidatorLockedGoldRequirements(),
+      this.getGroupLockedGoldRequirements(),
       this.contract.methods.maxGroupSize().call(),
     ])
     return {
-      balanceRequirements: res[0],
-      deregistrationLockups: res[1],
+      validatorLockedGoldRequirements: res[0],
+      groupLockedGoldRequirements: res[1],
       maxGroupSize: toBigNumber(res[2]),
     }
   }

packages/protocol/contracts/common/UsingPrecompiles.sol

@@ -1,6 +1,8 @@
 pragma solidity ^0.5.3;
 
+// TODO(asa): Limit assembly usage by using X.staticcall instead.
 contract UsingPrecompiles {
+  address constant PROOF_OF_POSSESSION = address(0xff - 4);
 
   /**
    * @notice calculate a * b^x for fractions a, b to `decimals` precision
@@ -122,4 +124,24 @@ contract UsingPrecompiles {
 
     return numberValidators;
   }
+
+  /**
+   * @notice Checks a BLS proof of possession.
+   * @param proofOfPossessionBytes The public key and signature of the proof of possession.
+   * @return True upon success.
+   */
+  function checkProofOfPossession(
+    address sender,
+    bytes memory proofOfPossessionBytes
+  )
+    public
+    returns (bool)
+  {
+    bool success;
+    (success, ) = PROOF_OF_POSSESSION
+      .call
+      .value(0)
+      .gas(gasleft())(abi.encodePacked(sender, proofOfPossessionBytes));
+    return success;
+  }
 }

packages/protocol/contracts/common/UsingRegistry.sol

@@ -8,6 +8,7 @@ import "./interfaces/IAccounts.sol";
 
 
 import "../governance/interfaces/IElection.sol";
+import "../governance/interfaces/IGovernance.sol";
 import "../governance/interfaces/ILockedGold.sol";
 import "../governance/interfaces/IValidators.sol";
 
@@ -70,6 +71,10 @@ contract UsingRegistry is Ownable {
     return IERC20Token(registry.getAddressForOrDie(GOLD_TOKEN_REGISTRY_ID));
   }
 
+  function getGovernance() internal view returns (IGovernance) {
+    return IGovernance(registry.getAddressForOrDie(GOVERNANCE_REGISTRY_ID));
+  }
+
   function getLockedGold() internal view returns (ILockedGold) {
     return ILockedGold(registry.getAddressForOrDie(LOCKED_GOLD_REGISTRY_ID));
   }

packages/protocol/contracts/governance/Election.sol

@@ -88,7 +88,6 @@ contract Election is
   uint256 public maxNumGroupsVotedFor;
   // Groups must receive at least this fraction of the total votes in order to be considered in
   // elections.
-  // TODO(asa): Implement this constraint.
   FixidityLib.Fraction public electabilityThreshold;
 
   event ElectableValidatorsSet(
@@ -151,9 +150,9 @@ contract Election is
   {
     _transferOwnership(msg.sender);
     setRegistry(registryAddress);
-    _setElectableValidators(minElectableValidators, maxElectableValidators);
-    _setMaxNumGroupsVotedFor(_maxNumGroupsVotedFor);
-    _setElectabilityThreshold(_electabilityThreshold);
+    setElectableValidators(minElectableValidators, maxElectableValidators);
+    setMaxNumGroupsVotedFor(_maxNumGroupsVotedFor);
+    setElectabilityThreshold(_electabilityThreshold);
   }
 
   /**
@@ -162,8 +161,12 @@ contract Election is
    * @param max The maximum number of validators that can be elected.
    * @return True upon success.
    */
-  function setElectableValidators(uint256 min, uint256 max) external onlyOwner returns (bool) {
-    return _setElectableValidators(min, max);
+  function setElectableValidators(uint256 min, uint256 max) public onlyOwner returns (bool) {
+    require(0 < min && min <= max);
+    require(min != electableValidators.min || max != electableValidators.max);
+    electableValidators = ElectableValidators(min, max);
+    emit ElectableValidatorsSet(min, max);
+    return true;
   }
 
   /**
@@ -174,20 +177,6 @@ contract Election is
     return (electableValidators.min, electableValidators.max);
   }
 
-  /**
-   * @notice Updates the minimum and maximum number of validators that can be elected.
-   * @param min The minimum number of validators that can be elected.
-   * @param max The maximum number of validators that can be elected.
-   * @return True upon success.
-   */
-  function _setElectableValidators(uint256 min, uint256 max) private returns (bool) {
-    require(0 < min && min <= max);
-    require(min != electableValidators.min || max != electableValidators.max);
-    electableValidators = ElectableValidators(min, max);
-    emit ElectableValidatorsSet(min, max);
-    return true;
-  }
-
   /**
    * @notice Updates the maximum number of groups an account can be voting for at once.
    * @param _maxNumGroupsVotedFor The maximum number of groups an account can vote for.
@@ -196,19 +185,10 @@ contract Election is
   function setMaxNumGroupsVotedFor(
     uint256 _maxNumGroupsVotedFor
   )
-    external
+    public
     onlyOwner
     returns (bool)
   {
-    return _setMaxNumGroupsVotedFor(_maxNumGroupsVotedFor);
-  }
-
-  /**
-   * @notice Updates the maximum number of groups an account can be voting for at once.
-   * @param _maxNumGroupsVotedFor The maximum number of groups an account can vote for.
-   * @return True upon success.
-   */
-  function _setMaxNumGroupsVotedFor(uint256 _maxNumGroupsVotedFor) private returns (bool) {
     require(_maxNumGroupsVotedFor != maxNumGroupsVotedFor);
     maxNumGroupsVotedFor = _maxNumGroupsVotedFor;
     emit MaxNumGroupsVotedForSet(_maxNumGroupsVotedFor);
@@ -221,15 +201,6 @@ contract Election is
    * @return True upon success.
    */
   function setElectabilityThreshold(uint256 threshold) public onlyOwner returns (bool) {
-    return _setElectabilityThreshold(threshold);
-  }
-
-  /**
-   * @notice Sets the electability threshold.
-   * @param threshold Electability threshold as unwrapped Fraction.
-   * @return True upon success.
-   */
-  function _setElectabilityThreshold(uint256 threshold) private returns (bool) {
     electabilityThreshold = FixidityLib.wrap(threshold);
     require(
       electabilityThreshold.lt(FixidityLib.fixed1()),
@@ -486,12 +457,7 @@ contract Election is
   {
     // The group must meet the balance requirements in order for their voters to receive epoch
     // rewards.
-    bool meetsBalanceRequirements = (
-      getLockedGold().getAccountTotalLockedGold(group) >=
-      getValidators().getAccountBalanceRequirement(group)
-    );
-
-    if (meetsBalanceRequirements && votes.active.total > 0) {
+    if (getValidators().meetsAccountLockedGoldRequirements(group) && votes.active.total > 0) {
       return totalEpochRewards.mul(votes.active.forGroup[group].total).div(votes.active.total);
     } else {
       return 0;

packages/protocol/contracts/governance/Governance.sol

@@ -18,7 +18,7 @@ import "../common/UsingPrecompiles.sol";
 /**
  * @title A contract for making, passing, and executing on-chain governance proposals.
  */
-contract Governance is 
+contract Governance is
   IGovernance,
   Ownable,
   Initializable,
@@ -758,7 +758,7 @@ contract Governance is
       msg.sender,
       0
     ).executeMem();
-    
+
     hotfixes[hash].executed = true;
     emit HotfixExecuted(hash);
   }
@@ -775,6 +775,22 @@ contract Governance is
     return true;
   }
 
+  /**
+   * @notice Returns whether or not a particular account is voting on proposals.
+   * @param account The address of the account.
+   * @return Whether or not the account is voting on proposals.
+   */
+  function isVoting(address account) external view returns (bool) {
+    Voter storage voter = voters[account];
+    uint256 upvotedProposal = voter.upvote.proposalId;
+    bool isVotingQueue = upvotedProposal != 0 && isQueued(upvotedProposal);
+    Proposals.Proposal storage proposal = proposals[voter.mostRecentReferendumProposal];
+    bool isVotingReferendum = (
+      proposal.getDequeuedStage(stageDurations) == Proposals.Stage.Referendum
+    );
+    return isVotingQueue || isVotingReferendum;
+  }
+
   /**
    * @notice Returns the number of seconds proposals stay in approval stage.
    * @return The number of seconds proposals stay in approval stage.

packages/protocol/contracts/governance/LockedGold.sol

@@ -148,7 +148,10 @@ contract LockedGold is ILockedGold, ReentrancyGuard, Initializable, UsingRegistr
   function unlock(uint256 value) external nonReentrant {
     require(getAccounts().isAccount(msg.sender));
     Account storage account = accounts[msg.sender];
-    uint256 balanceRequirement = getValidators().getAccountBalanceRequirement(msg.sender);
+    // Prevent unlocking gold when voting on governance proposals so that the gold cannot be
+    // used to vote more than once.
+    require(!getGovernance().isVoting(msg.sender));
+    uint256 balanceRequirement = getValidators().getAccountLockedGoldRequirement(msg.sender);
     require(
       balanceRequirement == 0 ||
       balanceRequirement <= getAccountTotalLockedGold(msg.sender).sub(value)