Skip to content

Commit

Permalink
L1-L2-checks (#11225)
Browse files Browse the repository at this point in the history
  • Loading branch information
soloseng authored Sep 25, 2024
1 parent 916b71d commit b82b51d
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 99 deletions.
9 changes: 3 additions & 6 deletions packages/protocol/contracts-0.8/governance/Validators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ contract Validators is
bytes calldata blsPublicKey,
bytes calldata blsPop
) external nonReentrant returns (bool) {
allowOnlyL1();
allowOnlyL1(); // For L2, use registerValidatorNoBls
address account = getAccounts().validatorSignerToAccount(msg.sender);
_isRegistrationAllowed(account);
require(!isValidator(account) && !isValidatorGroup(account), "Already registered");
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();
if (isValidator(validatorAccount)) {
Validator storage validator = validators[validatorAccount];
if (validator.affiliation != address(0)) {
Expand Down Expand Up @@ -829,9 +830,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");
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 @@ -950,7 +950,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 @@ -962,7 +961,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 @@ -976,7 +974,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 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 {
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) {
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) {
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) {
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) {
require(startBlocks.length > 0, "requires at least one interval");
require(
startBlocks.length == endBlocks.length,
Expand Down
35 changes: 18 additions & 17 deletions packages/protocol/contracts/governance/Election.sol
Original file line number Diff line number Diff line change
Expand Up @@ -637,10 +637,7 @@ contract Election is
*/
function hasActivatablePendingVotes(address account, address group) external view returns (bool) {
PendingVote storage pendingVote = votes.pending.forGroup[group].byAccount[account];
if (isL2()) {
return pendingVote.epoch < getEpochManager().getCurrentEpochNumber() && pendingVote.value > 0;
}
return pendingVote.epoch < getEpochNumber() && pendingVote.value > 0;
return pendingVote.epoch < _getEpochNumber() && pendingVote.value > 0;
}

/**
Expand Down Expand Up @@ -1010,14 +1007,9 @@ contract Election is

function _activate(address group, address account) internal onlyWhenNotBlocked returns (bool) {
PendingVote storage pendingVote = votes.pending.forGroup[group].byAccount[account];
if (isL2()) {
require(
pendingVote.epoch < getEpochManager().getCurrentEpochNumber(),
"Pending vote epoch not passed"
);
} else {
require(pendingVote.epoch < getEpochNumber(), "Pending vote epoch not passed");
}

require(pendingVote.epoch < _getEpochNumber(), "Pending vote epoch not passed");

uint256 value = pendingVote.value;
require(value > 0, "Vote value cannot be zero");
decrementPendingVotes(group, account, value);
Expand Down Expand Up @@ -1165,11 +1157,7 @@ contract Election is

PendingVote storage pendingVote = groupPending.byAccount[account];
pendingVote.value = pendingVote.value.add(value);
if (isL2()) {
pendingVote.epoch = getEpochManager().getCurrentEpochNumber();
} else {
pendingVote.epoch = getEpochNumber();
}
pendingVote.epoch = _getEpochNumber();
}

/**
Expand Down Expand Up @@ -1290,4 +1278,17 @@ contract Election is
value.mul(votes.active.forGroup[group].total).div(votes.active.forGroup[group].totalUnits);
}
}

/**
* @notice Returns the epoch number.
* @return Current epoch number.
*/
function _getEpochNumber() private view returns (uint256) {
// TODO remove this after L2 is fully implemented
if (isL2()) {
return getEpochManager().getCurrentEpochNumber();
} else {
return getEpochNumber();
}
}
}
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
31 changes: 14 additions & 17 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 @@ -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();
}

function forceDeaffiliateIfValidator(address validator) external {
allowOnlyL1(); // TODO remove
allowOnlyL1();
}

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 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 Expand Up @@ -286,7 +283,7 @@ contract MockValidators is IValidators, IsL2Check {
revert("Method not implemented in mock");
}

function distributeEpochPaymentsFromSigner(address, uint256) external returns (uint256) {
function distributeEpochPaymentsFromSigner(address, uint256) external onlyL1 returns (uint256) {
revert("Method not implemented in mock");
}

Expand Down
6 changes: 4 additions & 2 deletions packages/protocol/test-sol/unit/common/Accounts.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -608,11 +608,13 @@ contract AccountsTest_setPaymentDelegation is AccountsTest {
assertEq(realFraction, fraction);
}

function test_Revert_SetPaymentDelegation_WhenL2() public {
function test_ShouldSetAnAddressAndAFraction_WhenL2() public {
_whenL2();
accounts.createAccount();
vm.expectRevert("This method is no longer supported in L2.");
accounts.setPaymentDelegation(beneficiary, fraction);
(address realBeneficiary, uint256 realFraction) = accounts.getPaymentDelegation(address(this));
assertEq(realBeneficiary, beneficiary);
assertEq(realFraction, fraction);
}

function test_ShouldNotAllowFractionGreaterThan1() public {
Expand Down
Loading

0 comments on commit b82b51d

Please sign in to comment.