From f59e0671a195f4e87f51c70097ef6c22cf7f4240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Volpe?= Date: Fri, 20 Sep 2024 19:11:24 +0200 Subject: [PATCH] Added little optimizations --- .../contracts-0.8/governance/Validators.sol | 19 ++++------- .../contracts/governance/Election.sol | 33 ++++++++----------- .../governance/validators/Validators.t.sol | 3 +- .../unit/governance/voting/Election.t.sol | 12 +++---- 4 files changed, 27 insertions(+), 40 deletions(-) diff --git a/packages/protocol/contracts-0.8/governance/Validators.sol b/packages/protocol/contracts-0.8/governance/Validators.sol index 1063877a53..b670184250 100644 --- a/packages/protocol/contracts-0.8/governance/Validators.sol +++ b/packages/protocol/contracts-0.8/governance/Validators.sol @@ -18,7 +18,7 @@ import "../../contracts/common/interfaces/ICeloVersionedContract.sol"; import "../../contracts/common/libraries/ReentrancyGuard.sol"; import "../common/interfaces/IStableToken.sol"; -import { console } from "forge-std/console.sol"; +import "../../contracts/common/interfaces/IAccounts.sol"; /** * @title A contract for registering and electing Validator Groups and Validators. @@ -714,7 +714,7 @@ contract Validators is * @notice Returns the top n group members for a particular group. * @param account The address of the validator group. * @param n The number of members to return. - * @return The top n group members for a particular group. + * @return The signers of the top n group members for a particular group. * @dev Returns the account instead of signer on L2. */ function getTopGroupValidators( @@ -724,23 +724,18 @@ contract Validators is address[] memory topAccounts = groups[account].members.headN(n); address[] memory topValidators = new address[](n); - // TODO remove this if - if (isL2()) { - return topAccounts; - } else { - for (uint256 i = 0; i < n; i = i.add(1)) { - // todo move out of loop - topValidators[i] = getAccounts().getValidatorSigner(topAccounts[i]); - } - return topValidators; + IAccounts accounts = getAccounts(); + + for (uint256 i = 0; i < n; i = i.add(1)) { + topValidators[i] = accounts.getValidatorSigner(topAccounts[i]); } + return topValidators; } function getTopGroupValidatorsAccounts( address account, uint256 n ) external view returns (address[] memory) { - console.log("Hello from getTopGroupValidatorsAccounts"); address[] memory topAccounts = groups[account].members.headN(n); return topAccounts; } diff --git a/packages/protocol/contracts/governance/Election.sol b/packages/protocol/contracts/governance/Election.sol index 75cd06c5a2..c6786510ea 100644 --- a/packages/protocol/contracts/governance/Election.sol +++ b/packages/protocol/contracts/governance/Election.sol @@ -17,8 +17,6 @@ import "../common/libraries/Heap.sol"; import "../common/libraries/ReentrancyGuard.sol"; import "../common/Blockable.sol"; -import { console } from "forge-std/console.sol"; - contract Election is IElection, ICeloVersionedContract, @@ -805,13 +803,14 @@ contract Election is ) public view - // TODO REMOVE THIS onlyL1 - onlyL1 - returns (address[] memory) + returns ( + // TODO add tests this only works on L1 and L2 + address[] memory + ) { bool accounts = false; - console.log("electNValidatorSigners.maxElectableValidators", maxElectableValidators); - return _electNValidator(minElectableValidators, maxElectableValidators, accounts); + return + _electNValidatorSignerOrAccount(minElectableValidators, maxElectableValidators, accounts); } function electNValidator( @@ -819,7 +818,8 @@ contract Election is uint256 maxElectableValidators ) public view returns (address[] memory) { bool accounts = true; - return _electNValidator(minElectableValidators, maxElectableValidators, accounts); + return + _electNValidatorSignerOrAccount(minElectableValidators, maxElectableValidators, accounts); } /** @@ -828,7 +828,7 @@ contract Election is * @return The list of elected validator signers. * @dev See https://en.wikipedia.org/wiki/D%27Hondt_method#Allocation for more information. */ - function _electNValidator( + function _electNValidatorSignerOrAccount( uint256 minElectableValidators, uint256 maxElectableValidators, bool accounts // accounts or signers @@ -844,7 +844,7 @@ contract Election is requiredVotes, maxElectableValidators ); - console.log("maxElectableValidators", maxElectableValidators); + address[] memory electionGroups = votes.total.eligible.headN(numElectionGroups); uint256[] memory numMembers = getValidators().getGroupsNumMembers(electionGroups); // Holds the number of members elected for each of the eligible validator groups. @@ -862,7 +862,6 @@ contract Election is ); } - console.log("electionGroups", electionGroups.length); // Assign a number of seats to each validator group. while (totalNumMembersElected < maxElectableValidators && electionGroups.length > 0) { uint256 groupIndex = keys[0]; @@ -888,32 +887,28 @@ contract Election is address[] memory electedValidators = new address[](totalNumMembersElected); totalNumMembersElected = 0; - console.log("- 0", electionGroups.length); + IValidators validators = getValidators(); + for (uint256 i = 0; i < electionGroups.length; i = i.add(1)) { // We use the validating delegate if one is set. address[] memory electedGroupValidators; if (accounts) { // TODO move get validators outside of loop - electedGroupValidators = getValidators().getTopGroupValidatorsAccounts( + electedGroupValidators = validators.getTopGroupValidatorsAccounts( electionGroups[i], numMembersElected[i] ); } else { - electedGroupValidators = getValidators().getTopGroupValidators( + electedGroupValidators = validators.getTopGroupValidators( electionGroups[i], numMembersElected[i] ); } - console.log("1", electedGroupValidators.length); for (uint256 j = 0; j < electedGroupValidators.length; j = j.add(1)) { - // console.log("2", j); electedValidators[totalNumMembersElected] = electedGroupValidators[j]; - // console.log("3"); totalNumMembersElected = totalNumMembersElected.add(1); - // console.log("4"); } } - // console.log("5"); return electedValidators; } diff --git a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol index 76ccecfc06..c158518347 100644 --- a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol @@ -3142,9 +3142,10 @@ contract ValidatorsTest_GetTopGroupValidators is ValidatorsTest { assertFalse(_validatorSigner[0] == validator); } + // TODO move to other contract function test_ShouldReturnTheAccount_WhenL2() public { _whenL2(); - address[] memory validatorAccount = validators.getTopGroupValidators(group, 3); + address[] memory validatorAccount = validators.getTopGroupValidatorsAccounts(group, 3); assertEq(validatorAccount[0], validator); assertEq(validatorAccount[1], vm.addr(1)); diff --git a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol index ca133841d6..9782864281 100644 --- a/packages/protocol/test-sol/unit/governance/voting/Election.t.sol +++ b/packages/protocol/test-sol/unit/governance/voting/Election.t.sol @@ -2269,9 +2269,6 @@ contract ElectionTest_ElectValidatorSigners is ElectionTest { election.setElectabilityThreshold(0); election.setElectableValidators(10, 100); - // (uint256 min, uint256 max) = election.getElectableValidators(); - // console.log("getElectableValidators", min, max); - election.setMaxNumGroupsVotedFor(200); address prev = address(0); @@ -2302,18 +2299,17 @@ contract ElectionTest_ElectValidatorSigners is ElectionTest { function test_ShouldElectCorrectValidators_WhenThereIsALargeNumberOfGroupsSig() public { WhenThereIsALargeNumberOfGroups(); address[] memory elected = election.electValidatorSigners(); - console.log("hola"); + MemberWithVotes[] memory sortedMembersWithVotes = sortMembersWithVotesDesc(membersWithVotes); - console.log("hola2"); + MemberWithVotes[] memory electedUnsorted = new MemberWithVotes[](100); - console.log("hola3", elected.length); for (uint256 i = 0; i < 100; i++) { electedUnsorted[i] = MemberWithVotes(elected[i], votesConsideredForElection[elected[i]]); } - console.log("hola5"); + MemberWithVotes[] memory electedSorted = sortMembersWithVotesDesc(electedUnsorted); - console.log("hola6"); + for (uint256 i = 0; i < 100; i++) { assertEq(electedSorted[i].member, sortedMembersWithVotes[i].member); assertEq(electedSorted[i].votes, sortedMembersWithVotes[i].votes);