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

L1-L2-checks #11225

Merged
merged 8 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
27 changes: 12 additions & 15 deletions packages/protocol/contracts-0.8/governance/Validators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ contract Validators is
* @param uptime The Fixidity representation of the validator's uptime, between 0 and 1.
*/
function updateValidatorScoreFromSigner(address signer, uint256 uptime) external virtual onlyVm {
allowOnlyL1();
allowOnlyL1(); // no longer using signer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer using signers or uptime score

_updateValidatorScoreFromSigner(signer, uptime);
}

Expand All @@ -234,7 +234,7 @@ contract Validators is
address signer,
uint256 maxPayment
) external virtual onlyVm returns (uint256) {
allowOnlyL1();
allowOnlyL1(); // no longer using signer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer using signers

return _distributeEpochPaymentsFromSigner(signer, maxPayment);
}

Expand All @@ -255,7 +255,7 @@ contract Validators is
bytes calldata blsPublicKey,
bytes calldata blsPop
) external nonReentrant returns (bool) {
allowOnlyL1();
allowOnlyL1(); // no longer allow to register with a bls key
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer supporting BLS key.

address account = getAccounts().validatorSignerToAccount(msg.sender);
_isRegistrationAllowed(account);
require(!isValidator(account) && !isValidatorGroup(account), "Already registered");
Expand Down Expand Up @@ -385,7 +385,7 @@ contract Validators is
bytes calldata blsPublicKey,
bytes calldata blsPop
) external returns (bool) {
allowOnlyL1();
allowOnlyL1(); // no longer allow to register with a bls key
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer supporting BLS key.

address account = getAccounts().validatorSignerToAccount(msg.sender);
require(isValidator(account), "Not a validator");
Validator storage validator = validators[account];
Expand Down Expand Up @@ -461,7 +461,7 @@ contract Validators is
bytes calldata blsPublicKey,
bytes calldata blsPop
) external onlyRegisteredContract(ACCOUNTS_REGISTRY_ID) returns (bool) {
allowOnlyL1();
allowOnlyL1(); // no longer allow to register with a bls key
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer supporting BLS key.

require(isValidator(account), "Not a validator");
Validator storage validator = validators[account];
require(
Expand Down Expand Up @@ -611,6 +611,7 @@ contract Validators is
* @param validatorAccount The validator to deaffiliate from their affiliated validator group.
*/
function forceDeaffiliateIfValidator(address validatorAccount) external nonReentrant onlySlasher {
allowOnlyL1(); //Not used by governance slasher.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added since governance slasher does not use this function.

if (isValidator(validatorAccount)) {
Validator storage validator = validators[validatorAccount];
if (validator.affiliation != address(0)) {
Expand Down Expand Up @@ -639,7 +640,7 @@ contract Validators is
* @param account The group being slashed.
*/
function halveSlashingMultiplier(address account) external nonReentrant onlySlasher {
allowOnlyL1();
allowOnlyL1(); //Not used by governance slasher.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not used by governance slasher.

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 @@ -827,9 +828,8 @@ contract Validators is
uint256 epochNumber,
uint256 index
) external view returns (address) {
allowOnlyL1();
require(isValidator(account), "Not a validator");
require(epochNumber <= getEpochNumber(), "Epoch cannot be larger than current");
require(epochNumber <= _getEpochNumber(), "Epoch cannot be larger than current");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to support L2 epoch number

MembershipHistory storage history = validators[account].membershipHistory;
require(index < history.tail.add(history.numEntries), "index out of bounds");
require(index >= history.tail && history.numEntries > 0, "index out of bounds");
Expand Down Expand Up @@ -948,7 +948,6 @@ contract Validators is
* @param delay Number of blocks to delay the update
*/
function setCommissionUpdateDelay(uint256 delay) public onlyOwner {
allowOnlyL1();
require(delay != commissionUpdateDelay, "commission update delay not changed");
commissionUpdateDelay = delay;
emit CommissionUpdateDelaySet(delay);
Expand All @@ -960,7 +959,6 @@ contract Validators is
* @return True upon success.
*/
function setMaxGroupSize(uint256 size) public onlyOwner returns (bool) {
allowOnlyL1();
require(0 < size, "Max group size cannot be zero");
require(size != maxGroupSize, "Max group size not changed");
maxGroupSize = size;
Expand All @@ -974,7 +972,6 @@ contract Validators is
* @return True upon success.
*/
function setMembershipHistoryLength(uint256 length) public onlyOwner returns (bool) {
allowOnlyL1();
require(0 < length, "Membership history length cannot be zero");
require(length != membershipHistoryLength, "Membership history length not changed");
membershipHistoryLength = length;
Expand All @@ -992,7 +989,7 @@ contract Validators is
uint256 exponent,
uint256 adjustmentSpeed
) public onlyOwner returns (bool) {
allowOnlyL1();
allowOnlyL1(); // score handled by score manager now
require(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

score now handled by score manager. Leaving as is.

adjustmentSpeed <= FixidityLib.fixed1().unwrap(),
"Adjustment speed cannot be larger than 1"
Expand Down Expand Up @@ -1055,7 +1052,7 @@ contract Validators is
* @param value New reset period for slashing multiplier.
*/
function setSlashingMultiplierResetPeriod(uint256 value) public nonReentrant onlyOwner {
allowOnlyL1();
allowOnlyL1(); // no used by governance slasher.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not used by governance slasher.

slashingMultiplierResetPeriod = value;
}

Expand All @@ -1064,7 +1061,7 @@ contract Validators is
* @param value New downtime grace period for calculating epoch scores.
*/
function setDowntimeGracePeriod(uint256 value) public nonReentrant onlyOwner {
allowOnlyL1();
allowOnlyL1(); // downtime no longer supported
downtimeGracePeriod = value;
}

Expand Down Expand Up @@ -1119,7 +1116,7 @@ contract Validators is
* @return Fixidity representation of the epoch score between 0 and 1.
*/
function calculateEpochScore(uint256 uptime) public view returns (uint256) {
allowOnlyL1();
allowOnlyL1(); //score set by score manager.
require(uptime <= FixidityLib.fixed1().unwrap(), "Uptime cannot be larger than one");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not supported in L2. Score is now set by scoreManager.

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 @@ -567,7 +567,7 @@ contract Accounts is
* be greater than 1.
* @dev Use `deletePaymentDelegation` to unset the payment delegation.
*/
function setPaymentDelegation(address beneficiary, uint256 fraction) public onlyL1 {
function setPaymentDelegation(address beneficiary, uint256 fraction) public {
martinvol marked this conversation as resolved.
Show resolved Hide resolved
require(isAccount(msg.sender), "Must first register address with Account.createAccount");
require(beneficiary != address(0), "Beneficiary cannot be address 0x0");
FixidityLib.Fraction memory f = FixidityLib.wrap(fraction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ contract DoubleSigningSlasher is ICeloVersionedContract, SlasherUtil {
uint256 index,
bytes memory headerA,
bytes memory headerB
) public view returns (uint256) {
) public view onlyL1 returns (uint256) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uses precompile. Added modifier for a clean revert message.

require(hashHeader(headerA) != hashHeader(headerB), "Block hashes have to be different");
uint256 blockNumber = getBlockNumberFromHeader(headerA);
require(
Expand Down
6 changes: 3 additions & 3 deletions packages/protocol/contracts/governance/DowntimeSlasher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ contract DowntimeSlasher is ICeloVersionedContract, SlasherUtil {
function getBitmapForInterval(
uint256 startBlock,
uint256 endBlock
) public view returns (bytes32) {
) public view onlyL1 returns (bytes32) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uses precompile. Added modifier for a clean revert msg.

require(endBlock >= startBlock, "endBlock must be greater or equal than startBlock");
// The signature bitmap for block N is stored in block N+1.
// The latest block is `block.number - 1`, which stores the signature bitmap for
Expand Down Expand Up @@ -215,7 +215,7 @@ contract DowntimeSlasher is ICeloVersionedContract, SlasherUtil {
uint256 startBlock,
uint256 endBlock,
uint256 signerIndex
) public view returns (bool) {
) public view onlyL1 returns (bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

require(signerIndex < numberValidatorsInSet(startBlock), "bad validator index at start block");
require(
isBitmapSetForInterval(startBlock, endBlock),
Expand Down Expand Up @@ -250,7 +250,7 @@ contract DowntimeSlasher is ICeloVersionedContract, SlasherUtil {
uint256[] memory startBlocks,
uint256[] memory endBlocks,
uint256[] memory signerIndices
) public view returns (bool) {
) public view onlyL1 returns (bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

  • downtime slashing not supported

require(startBlocks.length > 0, "requires at least one interval");
require(
startBlocks.length == endBlocks.length,
Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/contracts/governance/test/MockElection.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ contract MockElection is IsL2Check {
isIneligible[account] = true;
}

function markGroupEligible(address account, address, address) external onlyL1 {
function markGroupEligible(address account, address, address) external {
isEligible[account] = true;
}

Expand All @@ -33,7 +33,7 @@ contract MockElection is IsL2Check {
electedValidators = _electedValidators;
}

function vote(address, uint256, address, address) external onlyL1 returns (bool) {
function vote(address, uint256, address, address) external returns (bool) {
return true;
}

Expand Down
33 changes: 15 additions & 18 deletions packages/protocol/contracts/governance/test/MockValidators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ contract MockValidators is IValidators, IsL2Check {
uint256 private numRegisteredValidators;

function updateEcdsaPublicKey(address, address, bytes calldata) external returns (bool) {
allowOnlyL1();
return true;
}

Expand All @@ -38,7 +37,7 @@ contract MockValidators is IValidators, IsL2Check {
bytes calldata,
bytes calldata
) external returns (bool) {
allowOnlyL1();
allowOnlyL1(); // BLS key no longer supported
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLS key no longer supported

return true;
}

Expand All @@ -50,12 +49,7 @@ contract MockValidators is IValidators, IsL2Check {
isValidatorGroup[group] = true;
}

function getValidatorsGroup(address validator) external view returns (address) {
return affiliations[validator];
}

function affiliate(address group) external returns (bool) {
allowOnlyL1();
affiliations[msg.sender] = group;
return true;
}
Expand Down Expand Up @@ -84,20 +78,15 @@ contract MockValidators is IValidators, IsL2Check {
}

function halveSlashingMultiplier(address) external {
allowOnlyL1(); // TODO remove
allowOnlyL1(); // not used by governance slasher
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not used by governance slasher.

}

function forceDeaffiliateIfValidator(address validator) external {
allowOnlyL1(); // TODO remove
allowOnlyL1(); // not used by governance slasher
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not used by governance slasher. L1 check stays.

}

function getTopGroupValidators(address group, uint256 n) public view returns (address[] memory) {
require(n <= members[group].length);
address[] memory validators = new address[](n);
for (uint256 i = 0; i < n; i = i.add(1)) {
validators[i] = members[group][i];
}
return validators;
function getValidatorsGroup(address validator) external view returns (address) {
return affiliations[validator];
}

function getTopGroupValidatorsAccounts(
Expand All @@ -119,7 +108,7 @@ contract MockValidators is IValidators, IsL2Check {
}

function getValidatorGroupSlashingMultiplier(address) external view returns (uint256) {
allowOnlyL1();
allowOnlyL1(); // not used by governance slasher
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is not used by governance slasher. L1 check stays

return FIXED1_UINT;
}

Expand Down Expand Up @@ -148,14 +137,22 @@ contract MockValidators is IValidators, IsL2Check {
}

function groupMembershipInEpoch(address addr, uint256, uint256) external view returns (address) {
allowOnlyL1();
return affiliations[addr];
}

function getGroupNumMembers(address group) public view returns (uint256) {
return members[group].length;
}

function getTopGroupValidators(address group, uint256 n) public view returns (address[] memory) {
require(n <= members[group].length);
address[] memory validators = new address[](n);
for (uint256 i = 0; i < n; i = i.add(1)) {
validators[i] = members[group][i];
}
return validators;
}

// Not implemented in mock, added here to support the interface
// without the interface, missing function erros get hard to debug

Expand Down
Loading