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 all 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
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");
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 @@ -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 {
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
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
Loading