Skip to content

Commit

Permalink
Send validator payment on actions that would modify reward distributi…
Browse files Browse the repository at this point in the history
…on (#11211)
  • Loading branch information
m-chrzan authored Sep 23, 2024
1 parent a35fdba commit 02454f4
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 36 deletions.
13 changes: 8 additions & 5 deletions packages/protocol/contracts-0.8/common/EpochManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ contract EpochManager is
*/
constructor(bool test) public Initializable(test) {}

modifier onlySystemAlreadyInitialized() {
require(systemAlreadyInitialized(), "Epoch system not initialized");
_;
}

/**
* @notice Used in place of the constructor to allow the contract to be upgradable via proxy.
* @param registryAddress The address of the registry core smart contract.
Expand Down Expand Up @@ -154,8 +159,7 @@ contract EpochManager is
/// start next epoch process.
/// it freezes the epochrewards at the time of execution,
/// and starts the distribution of the rewards.
function startNextEpochProcess() external nonReentrant {
require(systemAlreadyInitialized(), "Epoch system not initialized");
function startNextEpochProcess() external nonReentrant onlySystemAlreadyInitialized {
require(isTimeForNextEpoch(), "Epoch is not ready to start");
require(!isOnEpochProcess(), "Epoch process is already started");
epochProcessing.status = EpochProcessStatus.Started;
Expand Down Expand Up @@ -247,7 +251,7 @@ contract EpochManager is
* delegation beneficiary.
* @param validator Account of the validator.
*/
function sendValidatorPayment(address validator) external {
function sendValidatorPayment(address validator) external onlySystemAlreadyInitialized {
FixidityLib.Fraction memory totalPayment = FixidityLib.newFixed(
validatorPendingPayments[validator]
);
Expand Down Expand Up @@ -300,8 +304,7 @@ contract EpochManager is
}

/// returns the current epoch number.
function getCurrentEpochNumber() external view returns (uint256) {
require(systemAlreadyInitialized(), "EpochManager system not yet initialized.");
function getCurrentEpochNumber() external view onlySystemAlreadyInitialized returns (uint256) {
return currentEpochNumber;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ contract MockAccounts {
) public {
delegations[validator] = PaymentDelegation(beneficiary, FixidityLib.wrap(fraction));
}

function deletePaymentDelegationFor(address validator) public {
delete delegations[validator];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ contract MockEpochManager is IEpochManager {
bool initialized;
mapping(uint256 => Epoch) private epochs;

event SendValidatorPaymentCalled(address validator);

function setCurrentEpochNumber(uint256 _newEpochNumber) external {
currentEpochNumber = _newEpochNumber;
}
Expand Down Expand Up @@ -75,4 +77,8 @@ contract MockEpochManager is IEpochManager {
{
return (0, 0, 0, 0, 0);
}

function sendValidatorPayment(address validator) public {
emit SendValidatorPaymentCalled(validator);
}
}
18 changes: 16 additions & 2 deletions packages/protocol/contracts-0.8/governance/Validators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,6 @@ contract Validators is
* payments made to its members. Must be in the range [0, 1.0].
*/
function setNextCommissionUpdate(uint256 commission) external {
allowOnlyL1();
address account = getAccounts().validatorSignerToAccount(msg.sender);
require(isValidatorGroup(account), "Not a validator group");
ValidatorGroup storage group = groups[account];
Expand All @@ -592,11 +591,12 @@ contract Validators is
* @notice Updates a validator group's commission based on the previously queued update
*/
function updateCommission() external {
allowOnlyL1();
address account = getAccounts().validatorSignerToAccount(msg.sender);
require(isValidatorGroup(account), "Not a validator group");
ValidatorGroup storage group = groups[account];

_sendValidatorGroupPaymentsIfNecessary(group);

require(group.nextCommissionBlock != 0, "No commission update queued");
require(group.nextCommissionBlock <= block.number, "Can't apply commission update yet");

Expand Down Expand Up @@ -1498,6 +1498,7 @@ contract Validators is
Validator storage validator,
address validatorAccount
) private returns (bool) {
_sendValidatorPaymentIfNecessary(validatorAccount);
address affiliation = validator.affiliation;
ValidatorGroup storage group = groups[affiliation];
if (group.members.contains(validatorAccount)) {
Expand All @@ -1508,6 +1509,19 @@ contract Validators is
return true;
}

function _sendValidatorPaymentIfNecessary(address validator) private {
if (isL2()) {
getEpochManager().sendValidatorPayment(validator);
}
}

function _sendValidatorGroupPaymentsIfNecessary(ValidatorGroup storage group) private {
address[] memory members = group.members.getKeys();
for (uint256 i = 0; i < members.length; i++) {
_sendValidatorPaymentIfNecessary(members[i]);
}
}

/**
* @notice Returns the epoch number.
* @return Current epoch number.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface IEpochManager {
address[] calldata lessers,
address[] calldata greaters
) external;
function sendValidatorPayment(address) external;
function getCurrentEpoch() external view returns (uint256, uint256, uint256, uint256);
function getCurrentEpochNumber() external view returns (uint256);
function getElected() external view returns (address[] memory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ contract MockValidators is IValidators, IsL2Check {
}

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

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

function getTopGroupValidators(address group, uint256 n) public view returns (address[] memory) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ source $PWD/scripts/foundry/create_and_migrate_anvil_devchain.sh

# Run integration tests
echo "Running integration tests..."
forge test \
time FOUNDRY_PROFILE=devchain forge test \
-vvv \
--match-contract RegistryIntegrationTest \
--match-path "test-sol/devchain/migration/*" \
--fork-url $ANVIL_RPC_URL

# Stop devchain
Expand Down
20 changes: 10 additions & 10 deletions packages/protocol/test-sol/devchain/e2e/common/EpochManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ import "@test-sol/utils/ECDSAHelper08.sol";
import "@openzeppelin/contracts8/utils/structs/EnumerableSet.sol";

contract E2E_EpochManager is Test, Devchain, Utils08, ECDSAHelper08 {
struct VoterWithPK {
address voter;
uint256 privateKey;
}

struct GroupWithVotes {
address group;
uint256 votes;
}

address epochManagerOwner;
address epochManagerEnabler;
address[] firstElected;
Expand All @@ -24,16 +34,6 @@ contract E2E_EpochManager is Test, Devchain, Utils08, ECDSAHelper08 {
uint256[] groupScore = [5e23, 7e23, 1e24, 4e23];
uint256[] validatorScore = [1e23, 1e23, 1e23, 1e23, 1e23, 1e23, 1e23];

struct VoterWithPK {
address voter;
uint256 privateKey;
}

struct GroupWithVotes {
address group;
uint256 votes;
}

mapping(address => uint256) addressToPrivateKeys;
mapping(address => VoterWithPK) validatorToVoter;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.7 <0.8.20;

import "celo-foundry-8/Test.sol";
import { Devchain } from "@test-sol/devchain/e2e/utils.sol";

contract IntegrationsValidators is Test, Devchain {
function test_deaffiliateWorskWithEpochManager() public {
vm.prank(election.electValidatorAccounts()[0]);
validators.deaffiliate();
}
}
3 changes: 3 additions & 0 deletions packages/protocol/test-sol/unit/common/EpochManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ contract EpochManagerTest_sendValidatorPayment is EpochManagerTest {
members[1] = validator2;
mockValidators.setMembers(group, members);

vm.prank(epochManagerEnabler);
epochManager.initializeSystem(firstEpochNumber, firstEpochBlock, firstElected);

stableToken.mint(address(epochManager), paymentAmount * 2);
epochManagerBalanceBefore = stableToken.balanceOf(address(epochManager));
epochManager._setPaymentAllocation(validator1, paymentAmount);
Expand Down
102 changes: 87 additions & 15 deletions packages/protocol/test-sol/unit/governance/validators/Validators.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ contract ValidatorsTest is Test, TestConstants, Utils, ECDSAHelper {
uint256 groupPayment
);

event SendValidatorPaymentCalled(address validator);

function setUp() public {
owner = address(this);
group = actor("group");
Expand Down Expand Up @@ -1412,6 +1414,21 @@ contract ValidatorsTest_Affiliate_WhenValidatorIsAlreadyAffiliatedWithValidatorG

assertTrue(election.isIneligible(group));
}

function test_ShouldNotTryToSendValidatorPayment_WhenL1() public {
vm.prank(validator);
validators.affiliate(group);
Vm.Log[] memory entries = vm.getRecordedLogs();
assertEq(entries.length, 0);
}

function test_ShouldSendValidatorPayment_WhenL2() public {
_whenL2();
vm.expectEmit(true, true, true, true);
emit SendValidatorPaymentCalled(validator);
vm.prank(validator);
validators.affiliate(group);
}
}

contract ValidatorsTest_Deaffiliate is ValidatorsTest {
Expand Down Expand Up @@ -1537,6 +1554,21 @@ contract ValidatorsTest_Deaffiliate is ValidatorsTest {
validators.deaffiliate();
assertTrue(election.isIneligible(group));
}

function test_ShouldNotTryToSendValidatorPayment_WhenL1() public {
vm.prank(validator);
validators.affiliate(group);
Vm.Log[] memory entries = vm.getRecordedLogs();
assertEq(entries.length, 0);
}

function test_ShouldSendValidatorPayment_WhenL2() public {
_whenL2();
vm.expectEmit(true, true, true, true);
emit SendValidatorPaymentCalled(validator);
vm.prank(validator);
validators.deaffiliate();
}
}

contract ValidatorsTest_UpdateEcdsaPublicKey is ValidatorsTest {
Expand Down Expand Up @@ -2487,13 +2519,6 @@ contract ValidatorsTest_SetNextCommissionUpdate is ValidatorsTest {
assertEq(_commission, commission.unwrap());
}

function test_Reverts_SetValidatorGroupCommission_WhenL2() public {
_whenL2();
vm.prank(group);
vm.expectRevert("This method is no longer supported in L2.");
validators.setNextCommissionUpdate(newCommission);
}

function test_ShouldSetValidatorGroupNextCommission() public {
vm.prank(group);
validators.setNextCommissionUpdate(newCommission);
Expand Down Expand Up @@ -2534,7 +2559,21 @@ contract ValidatorsTest_UpdateCommission is ValidatorsTest {
function setUp() public {
super.setUp();

_registerValidatorGroupHelper(group, 1);
_registerValidatorGroupHelper(group, 2);

_registerValidatorHelper(validator, validatorPk);
_registerValidatorHelper(otherValidator, otherValidatorPk);

vm.prank(validator);
validators.affiliate(group);
(, , address _affiliation1, , ) = validators.getValidator(validator);

vm.prank(otherValidator);
validators.affiliate(group);
(, , address _affiliation2, , ) = validators.getValidator(otherValidator);

require(_affiliation1 == group, "Affiliation failed.");
require(_affiliation2 == group, "Affiliation failed.");
}

function test_ShouldSetValidatorGroupCommission() public {
Expand All @@ -2551,13 +2590,6 @@ contract ValidatorsTest_UpdateCommission is ValidatorsTest {
assertEq(_commission, newCommission);
}

function test_Reverts_SetValidatorGroupCommission_WhenL2() public {
_whenL2();
vm.prank(group);
vm.expectRevert("This method is no longer supported in L2.");
validators.setNextCommissionUpdate(newCommission);
}

function test_Emits_ValidatorGroupCommissionUpdated() public {
vm.prank(group);
validators.setNextCommissionUpdate(newCommission);
Expand Down Expand Up @@ -2600,6 +2632,31 @@ contract ValidatorsTest_UpdateCommission is ValidatorsTest {
vm.prank(group);
validators.updateCommission();
}

function test_ShouldNotTryTodSendMultipleValidatorPayments_WhenL1() public {
vm.prank(validator);
validators.affiliate(group);
Vm.Log[] memory entries = vm.getRecordedLogs();
assertEq(entries.length, 0);
}

function test_ShouldSendMultipleValidatorPayments_WhenL2() public {
vm.prank(group);
validators.addFirstMember(validator, address(0), address(0));
vm.prank(group);
validators.addMember(otherValidator);
vm.prank(group);
validators.setNextCommissionUpdate(newCommission);
blockTravel(commissionUpdateDelay);

_whenL2();
vm.expectEmit(true, true, true, true);
emit SendValidatorPaymentCalled(validator);
vm.expectEmit(true, true, true, true);
emit SendValidatorPaymentCalled(otherValidator);
vm.prank(group);
validators.updateCommission();
}
}

contract ValidatorsTest_CalculateEpochScore is ValidatorsTest {
Expand Down Expand Up @@ -3568,6 +3625,21 @@ contract ValidatorsTest_ForceDeaffiliateIfValidator is ValidatorsTest {
vm.expectRevert("Only registered slasher can call");
validators.forceDeaffiliateIfValidator(validator);
}

function test_ShouldNotTryToSendValidatorPayment_WhenL1() public {
vm.prank(validator);
validators.affiliate(group);
Vm.Log[] memory entries = vm.getRecordedLogs();
assertEq(entries.length, 0);
}

function test_ShouldSendValidatorPayment_WhenL2() public {
_whenL2();
vm.expectEmit(true, true, true, true);
emit SendValidatorPaymentCalled(validator);
vm.prank(paymentDelegatee);
validators.forceDeaffiliateIfValidator(validator);
}
}
contract ValidatorsTest_ForceDeaffiliateIfValidator_L2 is ValidatorsTest {
function setUp() public {
Expand Down

0 comments on commit 02454f4

Please sign in to comment.