Skip to content
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

Make Governance slasher able to remove validator and decrease slash m… #11245

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ contract GasPriceMinimum is
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (1, 2, 1, 0);
return (1, 2, 0, 2);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts-0.8/common/IsL2Check.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract IsL2Check {
_;
}

function isL2() public view returns (bool) {
function isL2() internal view returns (bool) {
uint32 size;
address _addr = proxyAdminAddress;
assembly {
Expand Down
35 changes: 16 additions & 19 deletions packages/protocol/contracts-0.8/governance/Validators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,10 @@ contract Validators is
* @param signer The validator signer of the validator account whose score needs updating.
* @param uptime The Fixidity representation of the validator's uptime, between 0 and 1.
*/
function updateValidatorScoreFromSigner(address signer, uint256 uptime) external virtual onlyVm {
allowOnlyL1();
function updateValidatorScoreFromSigner(
address signer,
uint256 uptime
) external virtual onlyVm onlyL1 {
_updateValidatorScoreFromSigner(signer, uptime);
}

Expand All @@ -233,8 +235,7 @@ contract Validators is
function distributeEpochPaymentsFromSigner(
address signer,
uint256 maxPayment
) external virtual onlyVm returns (uint256) {
allowOnlyL1();
) external virtual onlyVm onlyL1 returns (uint256) {
return _distributeEpochPaymentsFromSigner(signer, maxPayment);
}

Expand All @@ -254,8 +255,12 @@ contract Validators is
bytes calldata ecdsaPublicKey,
bytes calldata blsPublicKey,
bytes calldata blsPop
) external nonReentrant returns (bool) {
allowOnlyL1(); // For L2, use registerValidatorNoBls
)
external
nonReentrant
onlyL1 // For L2, use registerValidatorNoBls
returns (bool)
{
address account = getAccounts().validatorSignerToAccount(msg.sender);
_isRegistrationAllowed(account);
require(!isValidator(account) && !isValidatorGroup(account), "Already registered");
Expand Down Expand Up @@ -384,8 +389,7 @@ contract Validators is
function updateBlsPublicKey(
bytes calldata blsPublicKey,
bytes calldata blsPop
) external returns (bool) {
allowOnlyL1();
) external onlyL1 returns (bool) {
address account = getAccounts().validatorSignerToAccount(msg.sender);
require(isValidator(account), "Not a validator");
Validator storage validator = validators[account];
Expand Down Expand Up @@ -460,8 +464,7 @@ contract Validators is
bytes calldata ecdsaPublicKey,
bytes calldata blsPublicKey,
bytes calldata blsPop
) external onlyRegisteredContract(ACCOUNTS_REGISTRY_ID) returns (bool) {
allowOnlyL1();
) external onlyL1 onlyRegisteredContract(ACCOUNTS_REGISTRY_ID) returns (bool) {
require(isValidator(account), "Not a validator");
Validator storage validator = validators[account];
require(
Expand Down Expand Up @@ -611,7 +614,6 @@ contract Validators is
* @param validatorAccount The validator to deaffiliate from their affiliated validator group.
*/
function forceDeaffiliateIfValidator(address validatorAccount) external nonReentrant onlySlasher {
allowOnlyL1();
if (isValidator(validatorAccount)) {
Validator storage validator = validators[validatorAccount];
if (validator.affiliation != address(0)) {
Expand Down Expand Up @@ -640,7 +642,6 @@ contract Validators is
* @param account The group being slashed.
*/
function halveSlashingMultiplier(address account) external nonReentrant onlySlasher {
allowOnlyL1();
require(isValidatorGroup(account), "Not a validator group");
ValidatorGroup storage group = groups[account];
group.slashInfo.multiplier = FixidityLib.wrap(group.slashInfo.multiplier.unwrap().div(2));
Expand Down Expand Up @@ -1011,8 +1012,7 @@ contract Validators is
function setValidatorScoreParameters(
uint256 exponent,
uint256 adjustmentSpeed
) public onlyOwner returns (bool) {
allowOnlyL1();
) public onlyOwner onlyL1 returns (bool) {
require(
adjustmentSpeed <= FixidityLib.fixed1().unwrap(),
"Adjustment speed cannot be larger than 1"
Expand Down Expand Up @@ -1075,16 +1075,14 @@ contract Validators is
* @param value New reset period for slashing multiplier.
*/
function setSlashingMultiplierResetPeriod(uint256 value) public nonReentrant onlyOwner {
allowOnlyL1();
slashingMultiplierResetPeriod = value;
}

/**
* @notice Sets the downtimeGracePeriod property if called by owner.
* @param value New downtime grace period for calculating epoch scores.
*/
function setDowntimeGracePeriod(uint256 value) public nonReentrant onlyOwner {
allowOnlyL1();
function setDowntimeGracePeriod(uint256 value) public nonReentrant onlyOwner onlyL1 {
downtimeGracePeriod = value;
}

Expand Down Expand Up @@ -1138,8 +1136,7 @@ contract Validators is
* @dev epoch_score = uptime ** exponent
* @return Fixidity representation of the epoch score between 0 and 1.
*/
function calculateEpochScore(uint256 uptime) public view returns (uint256) {
allowOnlyL1();
function calculateEpochScore(uint256 uptime) public view onlyL1 returns (uint256) {
require(uptime <= FixidityLib.fixed1().unwrap(), "Uptime cannot be larger than one");
uint256 numerator;
uint256 denominator;
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/common/Accounts.sol
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ contract Accounts is
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (1, 1, 5, 0);
return (1, 1, 4, 2);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contract BlockchainParameters is Ownable, Initializable, UsingPrecompiles {
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (1, 3, 1, 0);
return (1, 3, 0, 1);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ contract DoubleSigningSlasher is ICeloVersionedContract, SlasherUtil {
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (1, 1, 2, 0);
return (1, 1, 1, 1);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract DowntimeSlasher is ICeloVersionedContract, SlasherUtil {
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (2, 0, 1, 0);
return (2, 0, 0, 1);
}

/**
Expand Down
95 changes: 92 additions & 3 deletions packages/protocol/contracts/governance/GovernanceSlasher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,35 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol";

import "../common/Initializable.sol";
import "../common/UsingRegistry.sol";
import "./interfaces/IValidators.sol";
import "../../contracts-0.8/common/IsL2Check.sol";
import "../common/interfaces/ICeloVersionedContract.sol";

contract GovernanceSlasher is Ownable, Initializable, UsingRegistry {
contract GovernanceSlasher is
Ownable,
Initializable,
UsingRegistry,
ICeloVersionedContract,
IsL2Check
{
using SafeMath for uint256;
// Maps a slashed address to the amount to be slashed.
// Note that there is no reward paid when slashing via governance.
mapping(address => uint256) slashed;
address internal slasherExecuter;

event SlashingApproved(address indexed account, uint256 amount);
event GovernanceSlashPerformed(address indexed account, uint256 amount);
event GovernanceSlashL2Performed(address indexed account, address indexed group, uint256 amount);
event SlasherExecuterSet(address slasherExecuter);

modifier onlyAuthorizedToSlash() {
require(
msg.sender == owner() || slasherExecuter == msg.sender,
"Sender not authorized to slash"
);
_;
}

/**
* @notice Sets initialized == true on implementation contracts
Expand All @@ -30,13 +50,29 @@ contract GovernanceSlasher is Ownable, Initializable, UsingRegistry {
setRegistry(registryAddress);
}

/**
* @notice Returns the storage, major, minor, and patch version of the contract.
* @return Storage version of the contract.
* @return Major version of the contract.
* @return Minor version of the contract.
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (1, 1, 1, 0);
}

function setSlasherExecuter(address _slasherExecuter) external onlyOwner {
slasherExecuter = _slasherExecuter;
emit SlasherExecuterSet(_slasherExecuter);
}

/**
* @notice Sets account penalty.
* @param account Address that is punished.
* @param penalty Amount of penalty in wei.
* @dev Only callable by governance.
*/
function approveSlashing(address account, uint256 penalty) external onlyOwner {
function approveSlashing(address account, uint256 penalty) external onlyAuthorizedToSlash {
slashed[account] = slashed[account].add(penalty);
emit SlashingApproved(account, penalty);
}
Expand All @@ -53,7 +89,7 @@ contract GovernanceSlasher is Ownable, Initializable, UsingRegistry {
address[] calldata electionLessers,
address[] calldata electionGreaters,
uint256[] calldata electionIndices
) external returns (bool) {
) external onlyL1 returns (bool) {
uint256 penalty = slashed[account];
require(penalty > 0, "No penalty given by governance");
slashed[account] = 0;
Expand All @@ -70,6 +106,55 @@ contract GovernanceSlasher is Ownable, Initializable, UsingRegistry {
return true;
}

/**
* @notice Calls `LockedGold.slash` on `account` if `account` has an entry in `slashed`.
* @param account Account to slash
* @param electionLessers Lesser pointers for slashing locked election gold.
* @param electionGreaters Greater pointers for slashing locked election gold.
* @param electionIndices Indices of groups voted by slashed account.
*/
function slashL2(
address account,
address group,
address[] calldata electionLessers,
address[] calldata electionGreaters,
uint256[] calldata electionIndices
) external onlyL2 onlyAuthorizedToSlash returns (bool) {
uint256 penalty = slashed[account];
require(penalty > 0, "No penalty given by governance");
slashed[account] = 0;

ILockedGold lockedGold = getLockedGold();

lockedGold.slash(
account,
penalty,
address(0),
0,
electionLessers,
electionGreaters,
electionIndices
);

if (group != address(0)) {
lockedGold.slash(
group,
penalty,
address(0),
pahor167 marked this conversation as resolved.
Show resolved Hide resolved
0,
electionLessers,
electionGreaters,
electionIndices
);
IValidators validators = getValidators();
validators.forceDeaffiliateIfValidator(account);
validators.halveSlashingMultiplier(group);
}

emit GovernanceSlashL2Performed(account, group, penalty);
return true;
}

/**
* @notice Gets account penalty.
* @param account Address that is punished.
Expand All @@ -78,4 +163,8 @@ contract GovernanceSlasher is Ownable, Initializable, UsingRegistry {
function getApprovedSlashing(address account) external view returns (uint256) {
return slashed[account];
}

function getSlasherExecuter() external view returns (address) {
return slasherExecuter;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import "../../../contracts-0.8/common/IsL2Check.sol";
contract MockValidators is IValidators, IsL2Check {
using SafeMath for uint256;

event HavelSlashingMultiplierHalved(address validator);
event ValidatorDeaffiliatedCalled(address validator);

uint256 private constant FIXED1_UINT = 1000000000000000000000000;

mapping(address => bool) public isValidator;
Expand Down Expand Up @@ -79,12 +82,12 @@ contract MockValidators is IValidators, IsL2Check {
lockedGoldRequirements[account] = value;
}

function halveSlashingMultiplier(address) external {
allowOnlyL1();
function halveSlashingMultiplier(address validator) external {
emit HavelSlashingMultiplierHalved(validator);
}

function forceDeaffiliateIfValidator(address validator) external {
allowOnlyL1();
emit ValidatorDeaffiliatedCalled(validator);
}

function getValidatorsGroup(address validator) external view returns (address) {
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/identity/Random.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ contract Random is
* @return Patch version of the contract.
*/
function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
return (1, 1, 2, 0);
return (1, 1, 1, 1);
}

/**
Expand Down
12 changes: 9 additions & 3 deletions packages/protocol/test-sol/unit/common/IsL2Check.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import { TestConstants } from "@test-sol/constants.sol";
import "@celo-contracts-8/common/IsL2Check.sol";

contract IsL2Test is IsL2Check {
// example function
function onlyL1Function() public view onlyL1 returns (bool) {
return true;
}

// expose the function for testing
function isL2_public() public view returns (bool) {
return super.isL2();
}
}

contract IsL2CheckBase is Test, TestConstants {
Expand All @@ -27,17 +33,17 @@ contract IsL2CheckBase is Test, TestConstants {

contract IsL2Check_IsL2Test is IsL2CheckBase {
function test_IsL2() public {
assertFalse(isL2Check.isL2());
assertFalse(isL2Check.isL2_public());
}

function test_IsL2_WhenProxyAdminSet() public {
helper_WhenProxyAdminAddressIsSet();
assertTrue(isL2Check.isL2());
assertTrue(isL2Check.isL2_public());
}

function test_IsL1_WhenProxyAdminSet() public {
helper_WhenProxyAdminAddressIsSet();
assertFalse(!isL2Check.isL2());
assertFalse(!isL2Check.isL2_public());
}
}

Expand Down
Loading
Loading