From b10b77a5b3b1ac9e8750b246e786a649548a2556 Mon Sep 17 00:00:00 2001 From: Arthur Gousset <46296830+arthurgousset@users.noreply.github.com> Date: Fri, 7 Jun 2024 09:19:32 +0100 Subject: [PATCH 01/18] test: proof of concept 2e2 test using anvil devchain (#11020) * chore(test-sol/FeeCurrencyDirectory)): use `@celo-...` remapping not `../..` * test(test-sol/FeeCurrencyDirectory)): MVP e2e test MVP demo of an e2e test using the devchain. This is not the pattern I'll suggest, but good to see the test passes end-to-end. * chore(foundry.toml): adds `@test-sol` remapping * chore(test-sol/e2e): adds MVP `utils.sol` Idea is to have a Devchain class that can be inherited by Test contracts to have access to the deployed contracts on the devchain. * test(FeeCurrencyDirectory): MVP test using `Devchain` class * chore(migrations_sol): adds MVP script to run e2e tests * style(test-sol/e2e): linting * refactor(test-sol/FeeCurrencyDirectory): moves file to `.../e2e/` * chore(migrations_sol): use `test-sol/e2e/*` path * chore(test-sol/FeeCurrencyDirectory): match contract name in unit and e2e test * style(test-sol/FeeCurrencyDirectory): linting * chore(e2e/utils): removes unused imports and more Cleans up Adds `TODO` comments * test(test-sol/e2e): renames contract with "E2E..." I'm temporarily adding the "E2E..." prefix to the contract so I can exclude this test and the integration tests during the CI run. In a separate PR, I'll refactor the tests into a directory structure like: ``` test-sol/unit/ test-sol/e2e/ test-sol/integration/ ``` That way we could run tests with something like this: ``` # unit tests forge test --match-path "*test-sol/unit/*" # e2e tests forge test --match-path "*test-sol/e2e/*" # integration tests forge test --match-path "*test-sol/integration/*" ``` * chore(workflows/protocol_tests): excludes e2e test from workflow I'm temporarily adding the "E2E..." prefix to the contract so I can exclude this test and the integration tests during the CI run. In a separate PR, I'll refactor the tests into a directory structure like: ``` test-sol/unit/ test-sol/e2e/ test-sol/integration/ ``` That way we could run tests with something like this: ``` # unit tests forge test --match-path "*test-sol/unit/*" # e2e tests forge test --match-path "*test-sol/e2e/*" # integration tests forge test --match-path "*test-sol/integration/*" ``` * style(test-sol/e2e): linting * chore(e2e): temporarily renames contract with "E2E..." In subsequent PRs, I'll rename this more accurately. --- .github/workflows/protocol_tests.yml | 4 +-- packages/protocol/foundry.toml | 3 +- .../migrations_sol/run_e2e_tests_in_anvil.sh | 21 +++++++++++ .../common/FeeCurrencyDirectory.t.sol | 4 +-- .../e2e/common/FeeCurrencyDirectory.t.sol | 23 ++++++++++++ packages/protocol/test-sol/e2e/utils.sol | 35 +++++++++++++++++++ 6 files changed, 85 insertions(+), 5 deletions(-) create mode 100755 packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh create mode 100644 packages/protocol/test-sol/e2e/common/FeeCurrencyDirectory.t.sol create mode 100644 packages/protocol/test-sol/e2e/utils.sol diff --git a/.github/workflows/protocol_tests.yml b/.github/workflows/protocol_tests.yml index 10eff4b0881..b2f332ad8a0 100644 --- a/.github/workflows/protocol_tests.yml +++ b/.github/workflows/protocol_tests.yml @@ -94,10 +94,10 @@ jobs: exit 1 fi - - name: Run Everything just in case something was missed + - name: Run everything just in case something was missed (excl. integration and e2e tests) # can't use gas limit because some setUp function use more than the limit # Excludes integration tests, because they require an anvil devchain running on the localhost. - run: forge test -vvv --no-match-contract RegistryIntegrationTest + run: forge test -vvv --no-match-contract "RegistryIntegrationTest|E2EDemo" - name: Generate migrations if: success() || failure() diff --git a/packages/protocol/foundry.toml b/packages/protocol/foundry.toml index b6a500daa6e..af42e09c3a7 100644 --- a/packages/protocol/foundry.toml +++ b/packages/protocol/foundry.toml @@ -15,7 +15,8 @@ remappings = [ '@celo-contracts-8=contracts-0.8/', '@openzeppelin/contracts8/=lib/openzeppelin-contracts8/contracts/', '@celo-contracts/=contracts/', - '@celo-migrations/=migrations_sol/' + '@celo-migrations/=migrations_sol/', + '@test-sol/=test-sol/' ] no_match_test = "skip" diff --git a/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh b/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh new file mode 100755 index 00000000000..3e1289cc60b --- /dev/null +++ b/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Generate and run devchain +echo "Generating and running devchain before running e2e tests..." +source $PWD/migrations_sol/create_and_migrate_anvil_devchain.sh + +# Run e2e tests +echo "Running e2e tests..." +forge test \ +-vvv \ +--match-path "*test-sol/e2e/*" \ +--fork-url http://127.0.0.1:$ANVIL_PORT + +# helper kill anvil +# kill $(lsof -i tcp:$ANVIL_PORT | tail -n 1 | awk '{print $2}') + +echo "Killing Anvil" +if [[ -n $ANVIL_PID ]]; then + kill $ANVIL_PID +fi diff --git a/packages/protocol/test-sol/common/FeeCurrencyDirectory.t.sol b/packages/protocol/test-sol/common/FeeCurrencyDirectory.t.sol index 06d01e8106d..fd4161cfba5 100644 --- a/packages/protocol/test-sol/common/FeeCurrencyDirectory.t.sol +++ b/packages/protocol/test-sol/common/FeeCurrencyDirectory.t.sol @@ -2,8 +2,8 @@ pragma solidity >=0.8.7 <0.8.20; import "celo-foundry-8/Test.sol"; -import "../../contracts-0.8/common/FeeCurrencyDirectory.sol"; -import "../../contracts-0.8/common/mocks/MockOracle.sol"; +import "@celo-contracts-8/common/FeeCurrencyDirectory.sol"; +import "@celo-contracts-8/common/mocks/MockOracle.sol"; contract FeeCurrencyDirectoryTestBase is Test { FeeCurrencyDirectory directory; diff --git a/packages/protocol/test-sol/e2e/common/FeeCurrencyDirectory.t.sol b/packages/protocol/test-sol/e2e/common/FeeCurrencyDirectory.t.sol new file mode 100644 index 00000000000..26ecbb6e416 --- /dev/null +++ b/packages/protocol/test-sol/e2e/common/FeeCurrencyDirectory.t.sol @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.7 <0.8.20; + +import "celo-foundry-8/Test.sol"; +import { Devchain } from "@test-sol/e2e/utils.sol"; + +import "@celo-contracts-8/common/FeeCurrencyDirectory.sol"; + +contract E2EDemo is Test, Devchain { + function test_ShouldAllowOwnerSetCurrencyConfig() public { + address token = address(1); + uint256 intrinsicGas = 21000; + + vm.prank(feeCurrencyDirectory.owner()); + feeCurrencyDirectory.setCurrencyConfig(token, address(sortedOracles), intrinsicGas); + FeeCurrencyDirectory.CurrencyConfig memory config = feeCurrencyDirectory.getCurrencyConfig( + token + ); + + assertEq(config.oracle, address(sortedOracles)); + assertEq(config.intrinsicGas, intrinsicGas); + } +} diff --git a/packages/protocol/test-sol/e2e/utils.sol b/packages/protocol/test-sol/e2e/utils.sol new file mode 100644 index 00000000000..14d99940b06 --- /dev/null +++ b/packages/protocol/test-sol/e2e/utils.sol @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.7 <0.8.20; + +import "@celo-contracts-8/common/UsingRegistry.sol"; +import "@celo-contracts/common/interfaces/IRegistry.sol"; + +// All core contracts that are expected to be in the Registry on the devchain +import "@celo-contracts-8/common/FeeCurrencyDirectory.sol"; +import "@celo-contracts/stability/interfaces/ISortedOracles.sol"; + +contract Devchain is UsingRegistry { + address constant registryAddress = address(0x000000000000000000000000000000000000ce10); + + // Used in exceptional circumstances when a contract is not in UsingRegistry.sol + IRegistry devchainRegistry = IRegistry(registryAddress); + + // All core contracts that are expected to be in the Registry on the devchain + ISortedOracles sortedOracles; + FeeCurrencyDirectory feeCurrencyDirectory; + + constructor() { + // The following line is required by UsingRegistry.sol + setRegistry(registryAddress); + + // Fetch all core contracts that are expeceted to be in the Registry on the devchain + sortedOracles = getSortedOracles(); + feeCurrencyDirectory = FeeCurrencyDirectory( + devchainRegistry.getAddressForStringOrDie("FeeCurrencyDirectory") + ); // FeeCurrencyDirectory is not in UsingRegistry.sol + + // TODO: Add missing core contracts below (see list in migrations_sol/constants.sol) + // TODO: Consider asserting that all contracts we expect are available in the Devchain class + // (see list in migrations_sol/constants.sol) + } +} From 2993cc10f2c7414a6820c474795cb2342e07b1f1 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Fri, 7 Jun 2024 15:30:48 -0400 Subject: [PATCH 02/18] set EpochSize on L2 --- .../contracts/common/UsingPrecompiles.sol | 18 ++++++++++++------ .../governance/BlockchainParameters.sol | 4 +--- .../contracts/governance/EpochRewards.sol | 4 +--- .../contracts/governance/SlasherUtil.sol | 3 +-- .../contracts/governance/Validators.sol | 4 +--- .../protocol/contracts/identity/Random.sol | 4 +--- 6 files changed, 17 insertions(+), 20 deletions(-) diff --git a/packages/protocol/contracts/common/UsingPrecompiles.sol b/packages/protocol/contracts/common/UsingPrecompiles.sol index a09eea3410b..017ad69bd33 100644 --- a/packages/protocol/contracts/common/UsingPrecompiles.sol +++ b/packages/protocol/contracts/common/UsingPrecompiles.sol @@ -2,8 +2,9 @@ pragma solidity ^0.5.13; import "openzeppelin-solidity/contracts/math/SafeMath.sol"; import "../common/interfaces/ICeloVersionedContract.sol"; +import "../../contracts-0.8/common/IsL2Check.sol"; -contract UsingPrecompiles { +contract UsingPrecompiles is IsL2Check { using SafeMath for uint256; address constant TRANSFER = address(0xff - 2); @@ -16,6 +17,7 @@ contract UsingPrecompiles { address constant HASH_HEADER = address(0xff - 9); address constant GET_PARENT_SEAL_BITMAP = address(0xff - 10); address constant GET_VERIFIED_SEAL_BITMAP = address(0xff - 11); + uint256 constant DAY = 86400; /** * @notice calculate a * b^x for fractions a, b to `decimals` precision @@ -55,11 +57,15 @@ contract UsingPrecompiles { * @return The current epoch size in blocks. */ function getEpochSize() public view returns (uint256) { - bytes memory out; - bool success; - (success, out) = EPOCH_SIZE.staticcall(abi.encodePacked(true)); - require(success, "error calling getEpochSize precompile"); - return getUint256FromBytes(out, 0); + if (isL2()) { + return DAY.div(5); + } else { + bytes memory out; + bool success; + (success, out) = EPOCH_SIZE.staticcall(abi.encodePacked(true)); + require(success, "error calling getEpochSize precompile"); + return getUint256FromBytes(out, 0); + } } /** diff --git a/packages/protocol/contracts/governance/BlockchainParameters.sol b/packages/protocol/contracts/governance/BlockchainParameters.sol index 8d358602344..7162d7e5ab4 100644 --- a/packages/protocol/contracts/governance/BlockchainParameters.sol +++ b/packages/protocol/contracts/governance/BlockchainParameters.sol @@ -5,12 +5,10 @@ import "openzeppelin-solidity/contracts/ownership/Ownable.sol"; import "../common/Initializable.sol"; import "../common/UsingPrecompiles.sol"; -import "../../contracts-0.8/common/IsL2Check.sol"; - /** * @title Contract for storing blockchain parameters that can be set by governance. */ -contract BlockchainParameters is Ownable, Initializable, UsingPrecompiles, IsL2Check { +contract BlockchainParameters is Ownable, Initializable, UsingPrecompiles { using SafeMath for uint256; // obsolete diff --git a/packages/protocol/contracts/governance/EpochRewards.sol b/packages/protocol/contracts/governance/EpochRewards.sol index fc9135e3f39..b7e398d8ffa 100644 --- a/packages/protocol/contracts/governance/EpochRewards.sol +++ b/packages/protocol/contracts/governance/EpochRewards.sol @@ -10,7 +10,6 @@ import "../common/Initializable.sol"; import "../common/UsingRegistry.sol"; import "../common/UsingPrecompiles.sol"; import "../common/interfaces/ICeloVersionedContract.sol"; -import "../../contracts-0.8/common/IsL2Check.sol"; /** * @title Contract for calculating epoch rewards. @@ -22,8 +21,7 @@ contract EpochRewards is UsingPrecompiles, UsingRegistry, Freezable, - CalledByVm, - IsL2Check + CalledByVm { using FixidityLib for FixidityLib.Fraction; using SafeMath for uint256; diff --git a/packages/protocol/contracts/governance/SlasherUtil.sol b/packages/protocol/contracts/governance/SlasherUtil.sol index 066bed0dc6a..9884d15e082 100644 --- a/packages/protocol/contracts/governance/SlasherUtil.sol +++ b/packages/protocol/contracts/governance/SlasherUtil.sol @@ -7,9 +7,8 @@ import "../common/Initializable.sol"; import "../common/UsingRegistry.sol"; import "../common/UsingPrecompiles.sol"; import "../common/interfaces/ICeloVersionedContract.sol"; -import "../../contracts-0.8/common/IsL2Check.sol"; -contract SlasherUtil is Ownable, Initializable, UsingRegistry, UsingPrecompiles, IsL2Check { +contract SlasherUtil is Ownable, Initializable, UsingRegistry, UsingPrecompiles { using SafeMath for uint256; struct SlashingIncentives { diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 4bce9ac487e..6b3a54be150 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -15,7 +15,6 @@ import "../common/UsingRegistry.sol"; import "../common/UsingPrecompiles.sol"; import "../common/interfaces/ICeloVersionedContract.sol"; import "../common/libraries/ReentrancyGuard.sol"; -import "../../contracts-0.8/common/IsL2Check.sol"; /** * @title A contract for registering and electing Validator Groups and Validators. @@ -28,8 +27,7 @@ contract Validators is Initializable, UsingRegistry, UsingPrecompiles, - CalledByVm, - IsL2Check + CalledByVm { using FixidityLib for FixidityLib.Fraction; using AddressLinkedList for LinkedList.List; diff --git a/packages/protocol/contracts/identity/Random.sol b/packages/protocol/contracts/identity/Random.sol index 81249f17540..8e67853ed1d 100644 --- a/packages/protocol/contracts/identity/Random.sol +++ b/packages/protocol/contracts/identity/Random.sol @@ -8,7 +8,6 @@ import "../common/CalledByVm.sol"; import "../common/Initializable.sol"; import "../common/UsingPrecompiles.sol"; import "../common/interfaces/ICeloVersionedContract.sol"; -import "../../contracts-0.8/common/IsL2Check.sol"; /** * @title Provides randomness for verifier selection @@ -19,8 +18,7 @@ contract Random is Ownable, Initializable, UsingPrecompiles, - CalledByVm, - IsL2Check + CalledByVm { using SafeMath for uint256; From 7f54f1b21ea709d705788c5190173e4f06330728 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Fri, 7 Jun 2024 15:33:54 -0400 Subject: [PATCH 03/18] allow voting and activating --- .../contracts/governance/Election.sol | 32 +++++++------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/packages/protocol/contracts/governance/Election.sol b/packages/protocol/contracts/governance/Election.sol index 33c05c82ca6..75ca4dabab1 100644 --- a/packages/protocol/contracts/governance/Election.sol +++ b/packages/protocol/contracts/governance/Election.sol @@ -15,7 +15,6 @@ import "../common/UsingRegistry.sol"; import "../common/interfaces/ICeloVersionedContract.sol"; import "../common/libraries/Heap.sol"; import "../common/libraries/ReentrancyGuard.sol"; -import "../../contracts-0.8/common/IsL2Check.sol"; contract Election is IElection, @@ -25,8 +24,7 @@ contract Election is Initializable, UsingRegistry, UsingPrecompiles, - CalledByVm, - IsL2Check + CalledByVm { using AddressSortedLinkedList for SortedLinkedList.List; using FixidityLib for FixidityLib.Fraction; @@ -198,7 +196,7 @@ contract Election is uint256 value, address lesser, address greater - ) external nonReentrant onlyL1 returns (bool) { + ) external nonReentrant returns (bool) { require(votes.total.eligible.contains(group), "Group not eligible"); require(0 < value, "Vote value cannot be zero"); require(canReceiveVotes(group, value), "Group cannot receive votes"); @@ -231,7 +229,7 @@ contract Election is * @return True upon success. * @dev Pending votes cannot be activated until an election has been held. */ - function activate(address group) external nonReentrant onlyL1 returns (bool) { + function activate(address group) external nonReentrant returns (bool) { address account = getAccounts().voteSignerToAccount(msg.sender); return _activate(group, account); } @@ -243,10 +241,7 @@ contract Election is * @return True upon success. * @dev Pending votes cannot be activated until an election has been held. */ - function activateForAccount( - address group, - address account - ) external nonReentrant onlyL1 returns (bool) { + function activateForAccount(address group, address account) external nonReentrant returns (bool) { return _activate(group, account); } @@ -369,7 +364,7 @@ contract Election is address group, address lesser, address greater - ) external onlyL1 onlyRegisteredContract(VALIDATORS_REGISTRY_ID) { + ) external onlyRegisteredContract(VALIDATORS_REGISTRY_ID) { uint256 value = getTotalVotesForGroup(group); votes.total.eligible.insert(group, value, lesser, greater); emit ValidatorGroupMarkedEligible(group); @@ -577,10 +572,7 @@ contract Election is * @return Whether or not `account` has activatable votes for `group`. * @dev Pending votes cannot be activated until an election has been held. */ - function hasActivatablePendingVotes( - address account, - address group - ) external view onlyL1 returns (bool) { + function hasActivatablePendingVotes(address account, address group) external view returns (bool) { PendingVote storage pendingVote = votes.pending.forGroup[group].byAccount[account]; return pendingVote.epoch < getEpochNumber() && pendingVote.value > 0; } @@ -619,7 +611,7 @@ contract Election is * @param max The maximum number of validators that can be elected. * @return True upon success. */ - function setElectableValidators(uint256 min, uint256 max) public onlyOwner onlyL1 returns (bool) { + function setElectableValidators(uint256 min, uint256 max) public onlyOwner returns (bool) { require(0 < min, "Minimum electable validators cannot be zero"); require(min <= max, "Maximum electable validators cannot be smaller than minimum"); require( @@ -636,9 +628,7 @@ contract Election is * @param _maxNumGroupsVotedFor The maximum number of groups an account can vote for. * @return True upon success. */ - function setMaxNumGroupsVotedFor( - uint256 _maxNumGroupsVotedFor - ) public onlyOwner onlyL1 returns (bool) { + function setMaxNumGroupsVotedFor(uint256 _maxNumGroupsVotedFor) public onlyOwner returns (bool) { require(_maxNumGroupsVotedFor != maxNumGroupsVotedFor, "Max groups voted for not changed"); maxNumGroupsVotedFor = _maxNumGroupsVotedFor; emit MaxNumGroupsVotedForSet(_maxNumGroupsVotedFor); @@ -650,7 +640,7 @@ contract Election is * @param threshold Electability threshold as unwrapped Fraction. * @return True upon success. */ - function setElectabilityThreshold(uint256 threshold) public onlyOwner onlyL1 returns (bool) { + function setElectabilityThreshold(uint256 threshold) public onlyOwner returns (bool) { electabilityThreshold = FixidityLib.wrap(threshold); require( electabilityThreshold.lt(FixidityLib.fixed1()), @@ -681,7 +671,7 @@ contract Election is * If not run, voting power of account will not reflect rewards awarded. * @param flag The on/off flag. */ - function setAllowedToVoteOverMaxNumberOfGroups(bool flag) public onlyL1 { + function setAllowedToVoteOverMaxNumberOfGroups(bool flag) public { address account = getAccounts().voteSignerToAccount(msg.sender); IValidators validators = getValidators(); require( @@ -822,7 +812,7 @@ contract Election is * @notice Returns get current validator signers using the precompiles. * @return List of current validator signers. */ - function getCurrentValidatorSigners() public view returns (address[] memory) { + function getCurrentValidatorSigners() public view onlyL1 returns (address[] memory) { uint256 n = numberValidatorsInCurrentSet(); address[] memory res = new address[](n); for (uint256 i = 0; i < n; i = i.add(1)) { From cbc00285f618f5fe48da766dcc060e7965e9ac32 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Fri, 7 Jun 2024 15:35:42 -0400 Subject: [PATCH 04/18] move election test contract to vote dir --- .../protocol/test-sol/{ => governance}/voting/Election.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename packages/protocol/test-sol/{ => governance}/voting/Election.t.sol (99%) diff --git a/packages/protocol/test-sol/voting/Election.t.sol b/packages/protocol/test-sol/governance/voting/Election.t.sol similarity index 99% rename from packages/protocol/test-sol/voting/Election.t.sol rename to packages/protocol/test-sol/governance/voting/Election.t.sol index 5f9086b8c72..e79e49d401b 100644 --- a/packages/protocol/test-sol/voting/Election.t.sol +++ b/packages/protocol/test-sol/governance/voting/Election.t.sol @@ -11,8 +11,8 @@ import "@celo-contracts/common/Accounts.sol"; import "@celo-contracts/common/linkedlists/AddressSortedLinkedList.sol"; import "@celo-contracts/identity/test/MockRandom.sol"; import "@celo-contracts/common/Freezer.sol"; -import { Constants } from "../constants.sol"; -import "../utils.sol"; +import { Constants } from "../../constants.sol"; +import "../../utils.sol"; contract ElectionMock is Election(true) { function distributeEpochRewards( From a1ea90ca69c6eee7dd91335f0d4cbb37902141e2 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Fri, 7 Jun 2024 18:04:08 -0400 Subject: [PATCH 05/18] updated test to allow testing of L2 with no reward distribution --- .../test-sol/governance/voting/Election.t.sol | 948 +++++++++++++++--- 1 file changed, 797 insertions(+), 151 deletions(-) diff --git a/packages/protocol/test-sol/governance/voting/Election.t.sol b/packages/protocol/test-sol/governance/voting/Election.t.sol index e79e49d401b..274caf6c504 100644 --- a/packages/protocol/test-sol/governance/voting/Election.t.sol +++ b/packages/protocol/test-sol/governance/voting/Election.t.sol @@ -197,10 +197,19 @@ contract ElectionTest_SetElectabilityThreshold is ElectionTest { vm.expectRevert("Electability threshold must be lower than 100%"); election.setElectabilityThreshold(FixidityLib.fixed1().unwrap() + 1); } +} - function test_Revert_setElectabilityThreshold_WhenL2() public { +contract ElectionTest_SetElectabilityThreshold_L2 is ElectionTest { + function test_shouldSetElectabilityThreshold() public { _whenL2(); - vm.expectRevert("This method is no longer supported in L2."); + uint256 newElectabilityThreshold = FixidityLib.newFixedFraction(1, 200).unwrap(); + election.setElectabilityThreshold(newElectabilityThreshold); + assertEq(election.electabilityThreshold(), newElectabilityThreshold); + } + + function test_ShouldRevertWhenThresholdLargerThan100Percent() public { + _whenL2(); + vm.expectRevert("Electability threshold must be lower than 100%"); election.setElectabilityThreshold(FixidityLib.fixed1().unwrap() + 1); } } @@ -215,12 +224,48 @@ contract ElectionTest_SetElectableValidators is ElectionTest { assertEq(max, newElectableValidatorsMax); } - function test_Reverts_shouldSetElectableValidators_WhenL2() public { + function test_ShouldEmitTheElectableValidatorsSetEvent() public { + uint256 newElectableValidatorsMin = 2; + uint256 newElectableValidatorsMax = 4; + vm.expectEmit(true, false, false, false); + emit ElectableValidatorsSet(newElectableValidatorsMin, newElectableValidatorsMax); + election.setElectableValidators(newElectableValidatorsMin, newElectableValidatorsMax); + } + + function test_ShouldRevertWhenMinElectableValidatorsIsZero() public { + vm.expectRevert("Minimum electable validators cannot be zero"); + election.setElectableValidators(0, electableValidatorsMax); + } + + function test_ShouldRevertWhenTHeminIsGreaterThanMax() public { + vm.expectRevert("Maximum electable validators cannot be smaller than minimum"); + election.setElectableValidators(electableValidatorsMax, electableValidatorsMin); + } + + function test_ShouldRevertWhenValuesAreUnchanged() public { + vm.expectRevert("Electable validators not changed"); + election.setElectableValidators(electableValidatorsMin, electableValidatorsMax); + } + + function test_ShouldRevertWhenCalledByNonOwner() public { + vm.expectRevert("Ownable: caller is not the owner"); + vm.prank(nonOwner); + election.setElectableValidators(1, 2); + } +} + +contract ElectionTest_SetElectableValidators_L2 is ElectionTest { + function setUp() public { + super.setUp(); _whenL2(); + } + function test_shouldSetElectableValidators() public { uint256 newElectableValidatorsMin = 2; uint256 newElectableValidatorsMax = 4; - vm.expectRevert("This method is no longer supported in L2."); election.setElectableValidators(newElectableValidatorsMin, newElectableValidatorsMax); + (uint256 min, uint256 max) = election.getElectableValidators(); + assertEq(min, newElectableValidatorsMin); + assertEq(max, newElectableValidatorsMax); } function test_ShouldEmitTheElectableValidatorsSetEvent() public { @@ -260,11 +305,35 @@ contract ElectionTest_SetMaxNumGroupsVotedFor is ElectionTest { assertEq(election.maxNumGroupsVotedFor(), newMaxNumGroupsVotedFor); } - function test_Revert_SetMaxNumGroupsVotedFor_WhenL2() public { + function test_ShouldEmitMaxNumGroupsVotedForSetEvent() public { + uint256 newMaxNumGroupsVotedFor = 4; + vm.expectEmit(true, false, false, false); + emit MaxNumGroupsVotedForSet(newMaxNumGroupsVotedFor); + election.setMaxNumGroupsVotedFor(newMaxNumGroupsVotedFor); + } + + function test_ShouldRevertWhenCalledByNonOwner() public { + vm.expectRevert("Ownable: caller is not the owner"); + vm.prank(nonOwner); + election.setMaxNumGroupsVotedFor(1); + } + + function test_ShouldRevert_WhenMaxNumGroupsVotedForIsUnchanged() public { + vm.expectRevert("Max groups voted for not changed"); + election.setMaxNumGroupsVotedFor(maxNumGroupsVotedFor); + } +} + +contract ElectionTest_SetMaxNumGroupsVotedFor_L2 is ElectionTest { + function setUp() public { + super.setUp(); _whenL2(); + } + + function test_shouldSetMaxNumGroupsVotedFor() public { uint256 newMaxNumGroupsVotedFor = 4; - vm.expectRevert("This method is no longer supported in L2."); election.setMaxNumGroupsVotedFor(newMaxNumGroupsVotedFor); + assertEq(election.maxNumGroupsVotedFor(), newMaxNumGroupsVotedFor); } function test_ShouldEmitMaxNumGroupsVotedForSetEvent() public { @@ -292,10 +361,48 @@ contract ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups is ElectionTest { assertEq(election.allowedToVoteOverMaxNumberOfGroups(address(this)), true); } - function test_Revert_SetAllowedToVoteOverMaxNumberOfGroups_WhenL2() public { + function test_ShouldRevertWhenCalledByValidator() public { + validators.setValidator(address(this)); + vm.expectRevert("Validators cannot vote for more than max number of groups"); + election.setAllowedToVoteOverMaxNumberOfGroups(true); + } + + function test_ShouldRevertWhenCalledByValidatorGroup() public { + validators.setValidatorGroup(address(this)); + vm.expectRevert("Validator groups cannot vote for more than max number of groups"); + election.setAllowedToVoteOverMaxNumberOfGroups(true); + } + + function test_ShouldEmitAllowedToVoteOverMaxNumberOfGroupsEvent() public { + vm.expectEmit(true, false, false, false); + emit AllowedToVoteOverMaxNumberOfGroups(address(this), true); + election.setAllowedToVoteOverMaxNumberOfGroups(true); + } + + function test_ShouldSwitchAllowedToVoteOverMaxNumberOfGroupsOff_WhenTurnedOn() public { + election.setAllowedToVoteOverMaxNumberOfGroups(true); + assertEq(election.allowedToVoteOverMaxNumberOfGroups(address(this)), true); + election.setAllowedToVoteOverMaxNumberOfGroups(false); + assertEq(election.allowedToVoteOverMaxNumberOfGroups(address(this)), false); + } + + function test_ShouldEmitAllowedToVoteOverMaxNumberOfGroupsEvent_WhenTurnedOn() public { + election.setAllowedToVoteOverMaxNumberOfGroups(true); + vm.expectEmit(true, false, false, false); + emit AllowedToVoteOverMaxNumberOfGroups(address(this), false); + election.setAllowedToVoteOverMaxNumberOfGroups(false); + } +} + +contract ElectionTest_SetAllowedToVoteOverMaxNumberOfGroups_L2 is ElectionTest { + function setUp() public { + super.setUp(); _whenL2(); - vm.expectRevert("This method is no longer supported in L2."); + } + + function test_shouldSetAllowedToVoteOverMaxNumberOfGroups() public { election.setAllowedToVoteOverMaxNumberOfGroups(true); + assertEq(election.allowedToVoteOverMaxNumberOfGroups(address(this)), true); } function test_ShouldRevertWhenCalledByValidator() public { @@ -346,11 +453,40 @@ contract ElectionTest_MarkGroupEligible is ElectionTest { assertEq(eligibleGroups[0], group); } - function test_Revert_MarkGroupEligible_WhenL2() public { + function test_ShouldEmitValidatorGroupMarkedEligibleEvent() public { + address group = address(this); + vm.expectEmit(true, false, false, false); + emit ValidatorGroupMarkedEligible(group); + election.markGroupEligible(group, address(0), address(0)); + } + + function test_ShouldRevertWhenAlreadyMarkedEligible() public { + address group = address(this); + election.markGroupEligible(group, address(0), address(0)); + vm.expectRevert("invalid key"); + election.markGroupEligible(group, address(0), address(0)); + } + + function test_ShouldRevertWhenCalledByNonValidator() public { + vm.expectRevert("only registered contract"); + vm.prank(nonOwner); + election.markGroupEligible(address(this), address(0), address(0)); + } +} + +contract ElectionTest_MarkGroupEligible_L2 is ElectionTest { + function setUp() public { + super.setUp(); _whenL2(); + registry.setAddressFor("Validators", address(address(this))); + } + + function test_shouldMarkGroupEligible() public { address group = address(this); - vm.expectRevert("This method is no longer supported in L2."); election.markGroupEligible(group, address(0), address(0)); + address[] memory eligibleGroups = election.getEligibleValidatorGroups(); + assertEq(eligibleGroups.length, 1); + assertEq(eligibleGroups[0], group); } function test_ShouldEmitValidatorGroupMarkedEligibleEvent() public { @@ -409,6 +545,7 @@ contract ElectionTest_MarkGroupInEligible is ElectionTest { election.markGroupIneligible(address(this)); } } + contract ElectionTest_Vote_WhenGroupEligible is ElectionTest { address voter = address(this); address group = account1; @@ -466,13 +603,6 @@ contract ElectionTest_Vote_WhenGroupEligible is ElectionTest { assertEq(election.getPendingVotesForGroupByAccount(group, voter), value - maxNumGroupsVotedFor); } - function test_Revert_Vote_WhenL2() public { - _whenL2(); - - vm.expectRevert("This method is no longer supported in L2."); - election.vote(group, value - maxNumGroupsVotedFor, address(0), address(0)); - } - function test_ShouldSetTotalVotesByAccount_WhenMaxNumberOfGroupsWasNotReached() public { WhenVotedForMaxNumberOfGroups(); assertEq(election.getTotalVotesByAccount(voter), maxNumGroupsVotedFor); @@ -587,36 +717,559 @@ contract ElectionTest_Vote_WhenGroupEligible is ElectionTest { address newGroup = WhenAwardsAreDistributed(); manuallyUpdateTotalVotesForAllGroups(voter); - election.vote(newGroup, originallyNotVotedWithAmount, account4, group); + election.vote(newGroup, originallyNotVotedWithAmount, account4, group); + + assertEq(election.getTotalVotes(), value + rewardValue); + } + + function test_ShouldRevert_WhenTheGroupCannotReceiveVotes() public { + lockedGold.setTotalLockedGold(value / 2 - 1); + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + validators.setNumRegisteredValidators(1); + assertEq(election.getNumVotesReceivable(group), value - 2); + + vm.expectRevert("Group cannot receive votes"); + election.vote(group, value, address(0), address(0)); + } +} + +contract ElectionTest_Vote_WhenGroupEligible_L2 is ElectionTest { + address voter = address(this); + address group = account1; + uint256 value = 1000; + + uint256 originallyNotVotedWithAmount = 1; + uint256 voterFirstGroupVote = value - maxNumGroupsVotedFor - originallyNotVotedWithAmount; + uint256 rewardValue = 1000000; + + function setUp() public { + super.setUp(); + _whenL2(); + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + + registry.setAddressFor("Validators", address(this)); + election.markGroupEligible(group, address(0), address(0)); + registry.setAddressFor("Validators", address(validators)); + } + + function test_ShouldRevert_WhenTheVoterDoesNotHaveSufficientNonVotingBalance() public { + lockedGold.incrementNonvotingAccountBalance(voter, value - 1); + vm.expectRevert("SafeMath: subtraction overflow"); + election.vote(group, value, address(0), address(0)); + } + + function WhenVotedForMaxNumberOfGroups() public returns (address newGroup) { + lockedGold.incrementNonvotingAccountBalance(voter, value); + + for (uint256 i = 0; i < maxNumGroupsVotedFor; i++) { + address[] memory members = new address[](1); + members[0] = accountsArray[9]; + newGroup = accountsArray[i + 2]; + setupGroupAndVote(newGroup, group, members, true); + } + } + + function test_ShouldRevert_WhenTheVoterCannotVoteForAnAdditionalGroup() public { + address newGroup = WhenVotedForMaxNumberOfGroups(); + + vm.expectRevert("Voted for too many groups"); + election.vote(group, value - maxNumGroupsVotedFor, newGroup, address(0)); + } + + function test_ShouldAllowToVoteForAnotherGroup_WhenTheVoterIsOverMaxNumberGroupsVotedForButCanVoteForAdditionalGroup() + public + { + address newGroup = WhenVotedForMaxNumberOfGroups(); + election.setAllowedToVoteOverMaxNumberOfGroups(true); + + vm.expectEmit(true, true, true, true); + emit ValidatorGroupVoteCast(voter, group, value - maxNumGroupsVotedFor); + election.vote(group, value - maxNumGroupsVotedFor, newGroup, address(0)); + assertEq(election.getPendingVotesForGroupByAccount(group, voter), value - maxNumGroupsVotedFor); + } + + function test_ShouldSetTotalVotesByAccount_WhenMaxNumberOfGroupsWasNotReached() public { + WhenVotedForMaxNumberOfGroups(); + assertEq(election.getTotalVotesByAccount(voter), maxNumGroupsVotedFor); + } + + function WhenVotedForMoreThanMaxNumberOfGroups() public returns (address newGroup) { + newGroup = WhenVotedForMaxNumberOfGroups(); + election.setAllowedToVoteOverMaxNumberOfGroups(true); + election.vote(group, voterFirstGroupVote, newGroup, address(0)); + } + + function test_ShouldRevert_WhenTurningOffSetAllowedToVoteOverMaxNUmberOfGroups_WhenOverMaximumNumberOfGroupsVoted() + public + { + WhenVotedForMoreThanMaxNumberOfGroups(); + + vm.expectRevert("Too many groups voted for!"); + election.setAllowedToVoteOverMaxNumberOfGroups(false); + } + + function test_ShouldReturnOnlyLastVotedWith_WhenVotesWereNotManuallyCounted() public { + WhenVotedForMoreThanMaxNumberOfGroups(); + assertEq(election.getTotalVotesByAccount(voter), voterFirstGroupVote); + } + + function manuallyUpdateTotalVotesForAllGroups(address _voter) public { + for (uint256 i = 0; i < maxNumGroupsVotedFor; i++) { + election.updateTotalVotesByAccountForGroup(_voter, accountsArray[i + 2]); + } + election.updateTotalVotesByAccountForGroup(_voter, group); + } + + function WhenTotalVotesWereManuallyCounted() public { + WhenVotedForMoreThanMaxNumberOfGroups(); + manuallyUpdateTotalVotesForAllGroups(voter); + } + + function test_ShouldReturnTotalVotesByAccount_WhenTotalVotesAreManuallyCounted() public { + WhenTotalVotesWereManuallyCounted(); + assertEq(election.getTotalVotesByAccount(voter), value - originallyNotVotedWithAmount); + } + + function test_ShouldReturnLoweredTotalNumberOfVotes_WhenVotesRevoked_WhenTotalVotesWereManuallyCounted() + public + { + uint256 revokeDiff = 100; + uint256 revokeValue = voterFirstGroupVote - revokeDiff; + + WhenTotalVotesWereManuallyCounted(); + election.revokePending(group, revokeValue, accountsArray[4], address(0), 3); + assertEq(election.getTotalVotesByAccount(voter), maxNumGroupsVotedFor + revokeDiff); + } + + function WhenVotesAreBeingActivated() public returns (address newGroup) { + newGroup = WhenVotedForMoreThanMaxNumberOfGroups(); + blockTravel(ph.epochSize() + 1); + election.activateForAccount(group, voter); + } + + function test_ShouldIncrementTheAccountsActiveVotesForGroup_WhenVotesAreBeingActivated() public { + WhenVotesAreBeingActivated(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter), voterFirstGroupVote); + } + + function test_ShouldReturnCorrectValueWhenManuallyCounted_WhenVotesAreBeingActivated() public { + WhenVotesAreBeingActivated(); + manuallyUpdateTotalVotesForAllGroups(voter); + + assertEq(election.getTotalVotesByAccount(voter), value - originallyNotVotedWithAmount); + } + + function WhenAwardsAreDistributed() public returns (address newGroup) { + newGroup = WhenVotesAreBeingActivated(); + election.distributeEpochRewards(group, rewardValue, newGroup, address(0)); + } + + // TODO: Implement when validator L2 rewards mechanism is implemented. + function skip_test_ShouldRevokeActiveVotes_WhenAwardsAreDistributed() public { + // (more then original votes without rewards) + address newGroup = WhenAwardsAreDistributed(); + election.revokeActive(group, value, newGroup, address(0), 3); + assertEq( + election.getActiveVotesForGroupByAccount(group, voter), + rewardValue - maxNumGroupsVotedFor - originallyNotVotedWithAmount + ); + } + + // TODO: Implement when validator L2 rewards mechanism is implemented. + function skip_test_ShouldReturnCorrectValueWhenManuallyCounted_WhenMoreVotesThanActiveIsRevoked_WhenAwardsAreDistributed() + public + { + address newGroup = WhenAwardsAreDistributed(); + election.revokeActive(group, value, newGroup, address(0), 3); + manuallyUpdateTotalVotesForAllGroups(voter); + + assertEq(election.getTotalVotesByAccount(voter), rewardValue - originallyNotVotedWithAmount); + } + + // TODO: Implement when validator L2 rewards mechanism is implemented. + function skip_test_ShouldReturnTotalVotesByAccount_WhenTotalVotesAreManuallyCountedOnReward_WhenAwardsAreDistributed() + public + { + WhenAwardsAreDistributed(); + manuallyUpdateTotalVotesForAllGroups(voter); + + assertEq( + election.getTotalVotesByAccount(voter), + value + rewardValue - originallyNotVotedWithAmount + ); + } + + // TODO: Implement when validator L2 rewards mechanism is implemented. + function skip_test_ShouldIncreaseTotalVotesCountOnceVoted_WhenTotalVotesAreManuallyCountedOnReward_WhenAwardsAreDistributed() + public + { + address newGroup = WhenAwardsAreDistributed(); + manuallyUpdateTotalVotesForAllGroups(voter); + + election.vote(newGroup, originallyNotVotedWithAmount, account4, group); + + assertEq(election.getTotalVotes(), value + rewardValue); + } + + function test_ShouldRevert_WhenTheGroupCannotReceiveVotes() public { + lockedGold.setTotalLockedGold(value / 2 - 1); + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + validators.setNumRegisteredValidators(1); + assertEq(election.getNumVotesReceivable(group), value - 2); + + vm.expectRevert("Group cannot receive votes"); + election.vote(group, value, address(0), address(0)); + } +} + +contract ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes is ElectionTest { + address voter = address(this); + address group = account1; + uint256 value = 1000; + + uint256 originallyNotVotedWithAmount = 1; + uint256 voterFirstGroupVote = value - maxNumGroupsVotedFor - originallyNotVotedWithAmount; + uint256 rewardValue = 1000000; + + function setUp() public { + super.setUp(); + + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + + registry.setAddressFor("Validators", address(this)); + election.markGroupEligible(group, address(0), address(0)); + registry.setAddressFor("Validators", address(validators)); + + lockedGold.setTotalLockedGold(value); + validators.setNumRegisteredValidators(1); + } + + function WhenTheVoterCanVoteForAnAdditionalGroup() public { + lockedGold.incrementNonvotingAccountBalance(voter, value); + } + + function WhenTheVoterHasNotAlreadyVotedForThisGroup() public { + WhenTheVoterCanVoteForAnAdditionalGroup(); + election.vote(group, value, address(0), address(0)); + } + + function test_ShouldAddTheGroupToListOfGroupsTheAccountHasVotedFor_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + address[] memory groupsVotedFor = election.getGroupsVotedForByAccount(voter); + assertEq(groupsVotedFor.length, 1); + assertEq(groupsVotedFor[0], group); + } + + function test_ShouldIncrementTheAccountsPendingVotesForTheGroup_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + assertEq(election.getPendingVotesForGroupByAccount(group, voter), value); + } + + function test_ShouldIncrementTheAccountsTotalVotesForTheGroup_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter), value); + } + + function test_ShouldIncrementTheACcountsTotalVotes_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotesByAccount(voter), value); + } + + function test_ShouldIncrementTheTotalVotesForTheGroup_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotesForGroup(group), value); + } + + function test_ShouldIncrementTheTotalVotes_WhenTheVoterHasNotAlreadyVotedForThisGroup() public { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotes(), value); + } + + function test_ShouldDecrementTheAccountsNonVotingLockedGoldBalance_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + assertEq(lockedGold.nonvotingAccountBalance(voter), 0); + } + + function test_ShouldEmitTheValidatorGroupVoteCastEvent_WhenTheVoterHasNotAlreadyVotedForThisGroup() + public + { + WhenTheVoterCanVoteForAnAdditionalGroup(); + vm.expectEmit(true, false, false, false); + emit ValidatorGroupVoteCast(voter, group, value); + election.vote(group, value, address(0), address(0)); + } + + function WhenTheVoterHasAlreadyVotedForThisGroup() public { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + lockedGold.incrementNonvotingAccountBalance(voter, value); + election.vote(group, value, address(0), address(0)); + } + + function test_ShouldNotChangeTheListOfGroupsTheAccountVotedFor_WhenTheVoterHasAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasAlreadyVotedForThisGroup(); + address[] memory groupsVotedFor = election.getGroupsVotedForByAccount(voter); + assertEq(groupsVotedFor.length, 1); + assertEq(groupsVotedFor[0], group); + } + + function test_ShouldIncreaseAccountsPendingVotesForTheGroup_WhenTheVoterHasAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasAlreadyVotedForThisGroup(); + assertEq(election.getPendingVotesForGroupByAccount(group, voter), value * 2); + } + + function test_ShouldIncrementAccountTotalVotesForTheGroup_WhenTheVoterHasAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter), value * 2); + } + + function test_ShouldIncrementTheAccountsTotalVotes_WhenTheVoterHasAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotesByAccount(voter), value * 2); + } + + function test_ShouldIncrementTotalVotesForTheGroup_WhenTheVoterHasAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotesForGroup(group), value * 2); + } + + function test_ShouldIncrementTotalVotes_WhenTheVoterHasAlreadyVotedForThisGroup() public { + WhenTheVoterHasAlreadyVotedForThisGroup(); + assertEq(election.getTotalVotes(), value * 2); + } + + function test_ShouldDecrementAccountNonVotingBalance_WhenTheVoterHasAlreadyVotedForThisGroup() + public + { + WhenTheVoterHasAlreadyVotedForThisGroup(); + assertEq(lockedGold.nonvotingAccountBalance(voter), 0); + } + + function test_ShouldEmitValidatorGroupVoteCast_WhenTheVoterHasAlreadyVotedForThisGroup() public { + WhenTheVoterHasNotAlreadyVotedForThisGroup(); + lockedGold.incrementNonvotingAccountBalance(voter, value); + vm.expectEmit(true, true, false, false); + emit ValidatorGroupVoteCast(voter, group, value); + election.vote(group, value, address(0), address(0)); + } +} + +contract ElectionTest_Vote_GroupNotEligible is ElectionTest { + address voter = address(this); + address group = account1; + uint256 value = 1000; + + uint256 originallyNotVotedWithAmount = 1; + uint256 voterFirstGroupVote = value - maxNumGroupsVotedFor - originallyNotVotedWithAmount; + uint256 rewardValue = 1000000; + + function setUp() public { + super.setUp(); + + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + } + + function test_ShouldRevert_WhenTheGroupIsNotEligible() public { + vm.expectRevert("Group not eligible"); + election.vote(group, value, address(0), address(0)); + } +} + +contract ElectionTest_Activate is ElectionTest { + address voter = address(this); + address group = account1; + uint256 value = 1000; + + function setUp() public { + super.setUp(); + + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + + registry.setAddressFor("Validators", address(this)); + election.markGroupEligible(group, address(0), address(0)); + registry.setAddressFor("Validators", address(validators)); + + lockedGold.setTotalLockedGold(value); + validators.setMembers(group, members); + validators.setNumRegisteredValidators(1); + lockedGold.incrementNonvotingAccountBalance(voter, value); + } + + function WhenVoterHasPendingVotes() public { + election.vote(group, value, address(0), address(0)); + } + + function WhenEpochBoundaryHasPassed() public { + WhenVoterHasPendingVotes(); + blockTravel(ph.epochSize() + 1); + election.activate(group); + } + + function test_ShouldDecrementTheAccountsPendingVotesForTheGroup_WhenEpochBoundaryHasPassed() + public + { + WhenEpochBoundaryHasPassed(); + assertEq(election.getPendingVotesForGroupByAccount(group, voter), 0); + } + + function test_ShouldIncrementTheAccountsActiveVotesForTheGroup_WhenEpochBoundaryHasPassed() + public + { + WhenEpochBoundaryHasPassed(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter), value); + } + + function test_ShouldNotModifyTheAccountsTotalVotesForTheGroup_WhenEpochBoundaryHasPassed() + public + { + WhenEpochBoundaryHasPassed(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter), value); + } + + function test_ShouldNotModifyTheAccountsTotalVotes_WhenEpochBoundaryHasPassed() public { + WhenEpochBoundaryHasPassed(); + assertEq(election.getTotalVotesByAccount(voter), value); + } + + function test_ShouldNotModifyTotalVotesForGroup_WhenEpochBoundaryHasPassed() public { + WhenEpochBoundaryHasPassed(); + assertEq(election.getTotalVotesForGroup(group), value); + } + + function test_ShouldNotModifyTotalVotes_WhenEpochBoundaryHasPassed() public { + WhenEpochBoundaryHasPassed(); + assertEq(election.getTotalVotes(), value); + } + + function test_ShouldEmitValidatorGroupVoteActivatedEvent_WhenEpochBoundaryHasPassed() public { + WhenVoterHasPendingVotes(); + blockTravel(ph.epochSize() + 1); + vm.expectEmit(true, true, true, false); + emit ValidatorGroupVoteActivated(voter, group, value, value * 100000000000000000000); + election.activate(group); + } + + address voter2 = account2; + uint256 value2 = 573; + + function WhenAnotherVoterActivatesVotes() public { + WhenEpochBoundaryHasPassed(); + lockedGold.incrementNonvotingAccountBalance(voter2, value2); + vm.prank(voter2); + election.vote(group, value2, address(0), address(0)); + blockTravel(ph.epochSize() + 1); + vm.prank(voter2); + election.activate(group); + } + + function test_ShouldNotModifyTheFirstAccountActiveVotesForTheGroup_WhenAnotherVoterActivatesVotes() + public + { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter), value); + } + + function test_ShouldNotModifyTheFirstAccountTotalVotesForTheGroup_WhenAnotherVoterActivatesVotes() + public + { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter), value); + } + + function test_ShouldNotModifyTheFirstAccountTotalVotes_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesByAccount(voter), value); + } + + function test_ShouldDecrementTheSecondAccountsPendingVotesFOrTheGroup_WhenAnotherVoterActivatesVotes() + public + { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getPendingVotesForGroupByAccount(group, voter2), 0); + } + + function test_ShouldIncrementTheSecondAccountActiveVotesForTheGroup_WhenAnotherVoterActivatesVotes() + public + { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter2), value2); + } + + function test_ShouldNotModifyTheSecondsAccountTotalVotesForTheGroup_WhenAnotherVoterActivatesVotes() + public + { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter2), value2); + } + + function test_ShouldNotMOdifyTheSecondAccountTotalVotes_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesByAccount(voter2), value2); + } - assertEq(election.getTotalVotes(), value + rewardValue); + function test_ShouldNotModifyTotalVotesForGroup_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesForGroup(group), value + value2); } - function test_ShouldRevert_WhenTheGroupCannotReceiveVotes() public { - lockedGold.setTotalLockedGold(value / 2 - 1); - address[] memory members = new address[](1); - members[0] = account9; - validators.setMembers(group, members); - validators.setNumRegisteredValidators(1); - assertEq(election.getNumVotesReceivable(group), value - 2); + function test_ShouldNotModifyTotalVotes_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotes(), value + value2); + } - vm.expectRevert("Group cannot receive votes"); - election.vote(group, value, address(0), address(0)); + function test_ShouldRevert_WhenAnEpochBoundaryHadNotPassedSinceThePendingVotesWereMade() public { + WhenVoterHasPendingVotes(); + vm.expectRevert("Pending vote epoch not passed"); + election.activateForAccount(group, voter); + } + + function test_ShouldRevert_WhenTheVoterDoesNotHavePendingVotes() public { + vm.expectRevert("Vote value cannot be zero"); + election.activate(group); } } -contract ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes is ElectionTest { +contract ElectionTest_Activate_L2 is ElectionTest { address voter = address(this); address group = account1; uint256 value = 1000; - uint256 originallyNotVotedWithAmount = 1; - uint256 voterFirstGroupVote = value - maxNumGroupsVotedFor - originallyNotVotedWithAmount; - uint256 rewardValue = 1000000; - function setUp() public { super.setUp(); - + _whenL2(); address[] memory members = new address[](1); members[0] = account9; validators.setMembers(group, members); @@ -626,164 +1279,146 @@ contract ElectionTest_Vote_WhenGroupEligible_WhenGroupCanReceiveVotes is Electio registry.setAddressFor("Validators", address(validators)); lockedGold.setTotalLockedGold(value); + validators.setMembers(group, members); validators.setNumRegisteredValidators(1); - } - - function WhenTheVoterCanVoteForAnAdditionalGroup() public { lockedGold.incrementNonvotingAccountBalance(voter, value); } - function WhenTheVoterHasNotAlreadyVotedForThisGroup() public { - WhenTheVoterCanVoteForAnAdditionalGroup(); + function WhenVoterHasPendingVotes() public { election.vote(group, value, address(0), address(0)); } - function test_ShouldAddTheGroupToListOfGroupsTheAccountHasVotedFor_WhenTheVoterHasNotAlreadyVotedForThisGroup() + function WhenEpochBoundaryHasPassed() public { + WhenVoterHasPendingVotes(); + blockTravel(ph.epochSize() + 1); + election.activate(group); + } + + function test_ShouldDecrementTheAccountsPendingVotesForTheGroup_WhenEpochBoundaryHasPassed() public { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); - address[] memory groupsVotedFor = election.getGroupsVotedForByAccount(voter); - assertEq(groupsVotedFor.length, 1); - assertEq(groupsVotedFor[0], group); + WhenEpochBoundaryHasPassed(); + assertEq(election.getPendingVotesForGroupByAccount(group, voter), 0); } - function test_ShouldIncrementTheAccountsPendingVotesForTheGroup_WhenTheVoterHasNotAlreadyVotedForThisGroup() + function test_ShouldIncrementTheAccountsActiveVotesForTheGroup_WhenEpochBoundaryHasPassed() public { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); - assertEq(election.getPendingVotesForGroupByAccount(group, voter), value); + WhenEpochBoundaryHasPassed(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter), value); } - function test_ShouldIncrementTheAccountsTotalVotesForTheGroup_WhenTheVoterHasNotAlreadyVotedForThisGroup() + function test_ShouldNotModifyTheAccountsTotalVotesForTheGroup_WhenEpochBoundaryHasPassed() public { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); + WhenEpochBoundaryHasPassed(); assertEq(election.getTotalVotesForGroupByAccount(group, voter), value); } - function test_ShouldIncrementTheACcountsTotalVotes_WhenTheVoterHasNotAlreadyVotedForThisGroup() - public - { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); + function test_ShouldNotModifyTheAccountsTotalVotes_WhenEpochBoundaryHasPassed() public { + WhenEpochBoundaryHasPassed(); assertEq(election.getTotalVotesByAccount(voter), value); } - function test_ShouldIncrementTheTotalVotesForTheGroup_WhenTheVoterHasNotAlreadyVotedForThisGroup() - public - { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); + function test_ShouldNotModifyTotalVotesForGroup_WhenEpochBoundaryHasPassed() public { + WhenEpochBoundaryHasPassed(); assertEq(election.getTotalVotesForGroup(group), value); } - function test_ShouldIncrementTheTotalVotes_WhenTheVoterHasNotAlreadyVotedForThisGroup() public { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); + function test_ShouldNotModifyTotalVotes_WhenEpochBoundaryHasPassed() public { + WhenEpochBoundaryHasPassed(); assertEq(election.getTotalVotes(), value); } - function test_ShouldDecrementTheAccountsNonVotingLockedGoldBalance_WhenTheVoterHasNotAlreadyVotedForThisGroup() - public - { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); - assertEq(lockedGold.nonvotingAccountBalance(voter), 0); + function test_ShouldEmitValidatorGroupVoteActivatedEvent_WhenEpochBoundaryHasPassed() public { + WhenVoterHasPendingVotes(); + blockTravel(ph.epochSize() + 1); + vm.expectEmit(true, true, true, false); + emit ValidatorGroupVoteActivated(voter, group, value, value * 100000000000000000000); + election.activate(group); } - function test_ShouldEmitTheValidatorGroupVoteCastEvent_WhenTheVoterHasNotAlreadyVotedForThisGroup() - public - { - WhenTheVoterCanVoteForAnAdditionalGroup(); - vm.expectEmit(true, false, false, false); - emit ValidatorGroupVoteCast(voter, group, value); - election.vote(group, value, address(0), address(0)); - } + address voter2 = account2; + uint256 value2 = 573; - function WhenTheVoterHasAlreadyVotedForThisGroup() public { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); - lockedGold.incrementNonvotingAccountBalance(voter, value); - election.vote(group, value, address(0), address(0)); + function WhenAnotherVoterActivatesVotes() public { + WhenEpochBoundaryHasPassed(); + lockedGold.incrementNonvotingAccountBalance(voter2, value2); + vm.prank(voter2); + election.vote(group, value2, address(0), address(0)); + blockTravel(ph.epochSize() + 1); + vm.prank(voter2); + election.activate(group); } - function test_ShouldNotChangeTheListOfGroupsTheAccountVotedFor_WhenTheVoterHasAlreadyVotedForThisGroup() + function test_ShouldNotModifyTheFirstAccountActiveVotesForTheGroup_WhenAnotherVoterActivatesVotes() public { - WhenTheVoterHasAlreadyVotedForThisGroup(); - address[] memory groupsVotedFor = election.getGroupsVotedForByAccount(voter); - assertEq(groupsVotedFor.length, 1); - assertEq(groupsVotedFor[0], group); + WhenAnotherVoterActivatesVotes(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter), value); } - function test_ShouldIncreaseAccountsPendingVotesForTheGroup_WhenTheVoterHasAlreadyVotedForThisGroup() + function test_ShouldNotModifyTheFirstAccountTotalVotesForTheGroup_WhenAnotherVoterActivatesVotes() public { - WhenTheVoterHasAlreadyVotedForThisGroup(); - assertEq(election.getPendingVotesForGroupByAccount(group, voter), value * 2); + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter), value); } - function test_ShouldIncrementAccountTotalVotesForTheGroup_WhenTheVoterHasAlreadyVotedForThisGroup() - public - { - WhenTheVoterHasAlreadyVotedForThisGroup(); - assertEq(election.getTotalVotesForGroupByAccount(group, voter), value * 2); + function test_ShouldNotModifyTheFirstAccountTotalVotes_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesByAccount(voter), value); } - function test_ShouldIncrementTheAccountsTotalVotes_WhenTheVoterHasAlreadyVotedForThisGroup() + function test_ShouldDecrementTheSecondAccountsPendingVotesFOrTheGroup_WhenAnotherVoterActivatesVotes() public { - WhenTheVoterHasAlreadyVotedForThisGroup(); - assertEq(election.getTotalVotesByAccount(voter), value * 2); + WhenAnotherVoterActivatesVotes(); + assertEq(election.getPendingVotesForGroupByAccount(group, voter2), 0); } - function test_ShouldIncrementTotalVotesForTheGroup_WhenTheVoterHasAlreadyVotedForThisGroup() + function test_ShouldIncrementTheSecondAccountActiveVotesForTheGroup_WhenAnotherVoterActivatesVotes() public { - WhenTheVoterHasAlreadyVotedForThisGroup(); - assertEq(election.getTotalVotesForGroup(group), value * 2); - } - - function test_ShouldIncrementTotalVotes_WhenTheVoterHasAlreadyVotedForThisGroup() public { - WhenTheVoterHasAlreadyVotedForThisGroup(); - assertEq(election.getTotalVotes(), value * 2); + WhenAnotherVoterActivatesVotes(); + assertEq(election.getActiveVotesForGroupByAccount(group, voter2), value2); } - function test_ShouldDecrementAccountNonVotingBalance_WhenTheVoterHasAlreadyVotedForThisGroup() + function test_ShouldNotModifyTheSecondsAccountTotalVotesForTheGroup_WhenAnotherVoterActivatesVotes() public { - WhenTheVoterHasAlreadyVotedForThisGroup(); - assertEq(lockedGold.nonvotingAccountBalance(voter), 0); + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesForGroupByAccount(group, voter2), value2); } - function test_ShouldEmitValidatorGroupVoteCast_WhenTheVoterHasAlreadyVotedForThisGroup() public { - WhenTheVoterHasNotAlreadyVotedForThisGroup(); - lockedGold.incrementNonvotingAccountBalance(voter, value); - vm.expectEmit(true, true, false, false); - emit ValidatorGroupVoteCast(voter, group, value); - election.vote(group, value, address(0), address(0)); + function test_ShouldNotMOdifyTheSecondAccountTotalVotes_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesByAccount(voter2), value2); } -} - -contract ElectionTest_Vote_GroupNotEligible is ElectionTest { - address voter = address(this); - address group = account1; - uint256 value = 1000; - uint256 originallyNotVotedWithAmount = 1; - uint256 voterFirstGroupVote = value - maxNumGroupsVotedFor - originallyNotVotedWithAmount; - uint256 rewardValue = 1000000; + function test_ShouldNotModifyTotalVotesForGroup_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotesForGroup(group), value + value2); + } - function setUp() public { - super.setUp(); + function test_ShouldNotModifyTotalVotes_WhenAnotherVoterActivatesVotes() public { + WhenAnotherVoterActivatesVotes(); + assertEq(election.getTotalVotes(), value + value2); + } - address[] memory members = new address[](1); - members[0] = account9; - validators.setMembers(group, members); + function test_ShouldRevert_WhenAnEpochBoundaryHadNotPassedSinceThePendingVotesWereMade() public { + WhenVoterHasPendingVotes(); + vm.expectRevert("Pending vote epoch not passed"); + election.activateForAccount(group, voter); } - function test_ShouldRevert_WhenTheGroupIsNotEligible() public { - vm.expectRevert("Group not eligible"); - election.vote(group, value, address(0), address(0)); + function test_ShouldRevert_WhenTheVoterDoesNotHavePendingVotes() public { + vm.expectRevert("Vote value cannot be zero"); + election.activate(group); } } -contract ElectionTest_Activate is ElectionTest { +contract ElectionTest_ActivateForAccount is ElectionTest { address voter = address(this); address group = account1; uint256 value = 1000; @@ -812,7 +1447,7 @@ contract ElectionTest_Activate is ElectionTest { function WhenEpochBoundaryHasPassed() public { WhenVoterHasPendingVotes(); blockTravel(ph.epochSize() + 1); - election.activate(group); + election.activateForAccount(group, voter); } function test_ShouldDecrementTheAccountsPendingVotesForTheGroup_WhenEpochBoundaryHasPassed() @@ -859,13 +1494,6 @@ contract ElectionTest_Activate is ElectionTest { election.activate(group); } - function test_Revert_Activate_WhenL2() public { - WhenVoterHasPendingVotes(); - _whenL2(); - vm.expectRevert("This method is no longer supported in L2."); - election.activate(group); - } - address voter2 = account2; uint256 value2 = 573; @@ -875,8 +1503,7 @@ contract ElectionTest_Activate is ElectionTest { vm.prank(voter2); election.vote(group, value2, address(0), address(0)); blockTravel(ph.epochSize() + 1); - vm.prank(voter2); - election.activate(group); + election.activateForAccount(group, voter2); } function test_ShouldNotModifyTheFirstAccountActiveVotesForTheGroup_WhenAnotherVoterActivatesVotes() @@ -934,32 +1561,26 @@ contract ElectionTest_Activate is ElectionTest { assertEq(election.getTotalVotes(), value + value2); } - function test_ShouldRevert_WhenAnEpochBoundaryHadNotPassedSinceThePendingVotesWereMade() public { + function test_ShouldRevert_WhenEpochBoundaryHasNotPassedSinceThePendingVotesWereMade() public { WhenVoterHasPendingVotes(); vm.expectRevert("Pending vote epoch not passed"); election.activateForAccount(group, voter); } - function test_Revert_ActivateForAccount_WhenL2() public { - _whenL2(); - vm.expectRevert("This method is no longer supported in L2."); - election.activateForAccount(group, voter); - } - function test_ShouldRevert_WhenTheVoterDoesNotHavePendingVotes() public { vm.expectRevert("Vote value cannot be zero"); - election.activate(group); + election.activateForAccount(group, voter); } } -contract ElectionTest_ActivateForAccount is ElectionTest { +contract ElectionTest_ActivateForAccount_L2 is ElectionTest { address voter = address(this); address group = account1; uint256 value = 1000; function setUp() public { super.setUp(); - + _whenL2(); address[] memory members = new address[](1); members[0] = account9; validators.setMembers(group, members); @@ -2711,10 +3332,35 @@ contract ElectionTest_ConsistencyChecks is ElectionTest { } contract ElectionTest_HasActivatablePendingVotes is ElectionTest { - function test_Revert_hasActivatablePendingVotes_WhenL2() public { + address voter = address(this); + address group = account1; + uint256 value = 1000; + + function setUp() public { + super.setUp(); + address[] memory members = new address[](1); + members[0] = account9; + validators.setMembers(group, members); + + registry.setAddressFor("Validators", address(this)); + election.markGroupEligible(group, address(0), address(0)); + registry.setAddressFor("Validators", address(validators)); + + lockedGold.setTotalLockedGold(value); + validators.setMembers(group, members); + validators.setNumRegisteredValidators(1); + + lockedGold.incrementNonvotingAccountBalance(voter, value); + election.vote(group, value, address(0), address(0)); + blockTravel(ph.epochSize() + 1); + } + function test_ReturnsTrue_WhenUserHasVoted() public { + assertTrue(election.hasActivatablePendingVotes(voter, group)); + } + + function test_ReturnsTrue_WhenUserHasVotedOnL2() public { _whenL2(); - vm.expectRevert("This method is no longer supported in L2."); - vm.prank(address(0)); - election.hasActivatablePendingVotes(address(0), address(0)); + + assertTrue(election.hasActivatablePendingVotes(voter, group)); } } From adb51bdaac9d1d7912f9eed777994cd6a9e325d9 Mon Sep 17 00:00:00 2001 From: Arthur Gousset <46296830+arthurgousset@users.noreply.github.com> Date: Tue, 11 Jun 2024 09:55:26 +0100 Subject: [PATCH 06/18] feat: add ReserveSpenderMultiSig to anvil migrations (#11028) * feat(migrations_sol/migrationsConfig): adds `reserveSpenderMultiSig` configs * feat(migrations_sol): adds `migrateReserveSpenderMultiSig` Also adds contract calls in `migrateReserve` function. Not tested, and not sure this works yet. I might be missing some changes. * chore(test-sol/utils): adds `ReserveSpenderMultiSig.t.sol` From the code comment: The purpose of this file is not to provide test coverage for `ReserveSpenderMultiSig.sol`. This is an empty test to force foundry to compile `ReserveSpenderMultiSig.sol`, and include its artifacts in the `/out` directory. `ReserveSpenderMultiSig.sol` is needed in the migrations script, but because it's on Solidity 0.5 it can't be imported there directly. If there is a better way to force foundry to compile `ReserveSpenderMultiSig.sol` without this file, this file can confidently be deleted. * feat(migrations_sol/HelperInterFaces): adds `IReserveSpenderMultiSig` The helper interface is used as a workaround to allow the migrations script (`Migrations.s.sol`) to be on Solidity 0.8, while the ReserveSpenderMultiSig contract is on Solidity 0.5. The migration script only needs to initialize the contract, which is why the helper interface only defines an `initialize` function. * chore(migrations_sol): initialize `ReserveSpenderMultiSig` * chore(migrations_sol): adds code comment for readability * chore(migrations_sol): improves code comment, moves code block up * chore(migrations_sol): fix typo * style(migrations_sol): linting --- packages/protocol/foundry.toml | 3 +- .../migrations_sol/HelperInterFaces.sol | 14 +++++++ .../protocol/migrations_sol/Migration.s.sol | 40 ++++++++++++++++--- .../migrations_sol/migrationsConfig.json | 4 ++ .../utils/ReserveSpenderMultiSig.t.sol | 13 ++++++ 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 packages/protocol/test-sol/utils/ReserveSpenderMultiSig.t.sol diff --git a/packages/protocol/foundry.toml b/packages/protocol/foundry.toml index af42e09c3a7..3e360d306a1 100644 --- a/packages/protocol/foundry.toml +++ b/packages/protocol/foundry.toml @@ -16,7 +16,8 @@ remappings = [ '@openzeppelin/contracts8/=lib/openzeppelin-contracts8/contracts/', '@celo-contracts/=contracts/', '@celo-migrations/=migrations_sol/', - '@test-sol/=test-sol/' + '@test-sol/=test-sol/', + '@mento-core/=lib/mento-core/' ] no_match_test = "skip" diff --git a/packages/protocol/migrations_sol/HelperInterFaces.sol b/packages/protocol/migrations_sol/HelperInterFaces.sol index ace2cd69a0f..486aa976929 100644 --- a/packages/protocol/migrations_sol/HelperInterFaces.sol +++ b/packages/protocol/migrations_sol/HelperInterFaces.sol @@ -49,3 +49,17 @@ interface IExchangeInitializer { interface IExchange { function activateStable() external; } + +interface IReserveSpenderMultiSig { + /** + @dev Contract constructor sets initial owners and required number of confirmations. + @param _owners List of initial owners. + @param _required Number of required confirmations for external transactions. + @param _internalRequired Number of required confirmations for internal transactions. + */ + function initialize( + address[] calldata _owners, + uint256 _required, + uint256 _internalRequired + ) external; +} diff --git a/packages/protocol/migrations_sol/Migration.s.sol b/packages/protocol/migrations_sol/Migration.s.sol index 8913154eeb1..2cf777bc169 100644 --- a/packages/protocol/migrations_sol/Migration.s.sol +++ b/packages/protocol/migrations_sol/Migration.s.sol @@ -43,7 +43,7 @@ import "@celo-contracts/stability/interfaces/ISortedOracles.sol"; import "@celo-contracts-8/common/interfaces/IGasPriceMinimumInitializer.sol"; import "@celo-contracts-8/common/interfaces/IMintGoldScheduleInitializer.sol"; -import "./HelperInterFaces.sol"; +import "@celo-migrations/HelperInterFaces.sol"; import "@openzeppelin/contracts8/utils/math/Math.sol"; import "@celo-contracts-8/common/UsingRegistry.sol"; @@ -212,6 +212,7 @@ contract Migration is Script, UsingRegistry, Constants { migrateGoldToken(json); migrateSortedOracles(json); migrateGasPriceMinimum(json); + migrateReserveSpenderMultiSig(json); migrateReserve(json); migrateStableToken(json); migrateExchange(json); @@ -329,9 +330,31 @@ contract Migration is Script, UsingRegistry, Constants { ); } - function migrateReserve(string memory json) public { - // Reserve spend multisig not migrated + function migrateReserveSpenderMultiSig(string memory json) public { + address[] memory owners = new address[](1); + owners[0] = deployerAccount; + + uint256 required = abi.decode(json.parseRaw(".reserveSpenderMultiSig.required"), (uint256)); + uint256 internalRequired = abi.decode( + json.parseRaw(".reserveSpenderMultiSig.internalRequired"), + (uint256) + ); + // Deploys and adds the ReserveSpenderMultiSig to the Registry for ease of reference. + // The ReserveSpenderMultiSig is not in the Registry on Mainnet, but it's useful to keep a + // reference of the deployed contract, so it's in the Registry on the devchain. + deployProxiedContract( + "ReserveSpenderMultiSig", + abi.encodeWithSelector( + IReserveSpenderMultiSig.initialize.selector, + owners, + required, + internalRequired + ) + ); + } + + function migrateReserve(string memory json) public { uint256 tobinTaxStalenessThreshold = abi.decode( json.parseRaw(".reserve.tobinTaxStalenessThreshold"), (uint256) @@ -374,9 +397,14 @@ contract Migration is Script, UsingRegistry, Constants { // TODO this should be a transfer from the deployer rather than a deal vm.deal(reserveProxyAddress, initialBalance); - address reserveSpenderMultiSig = deployerAccount; - IReserve(reserveProxyAddress).addSpender(reserveSpenderMultiSig); - console.log("reserveSpenderMultiSig set to:", reserveSpenderMultiSig); + // Adds ReserveSpenderMultiSig to Reserve + bool useSpender = abi.decode(json.parseRaw(".reserveSpenderMultiSig.required"), (bool)); + address spender = useSpender + ? registry.getAddressForString("ReserveSpenderMultiSig") + : deployerAccount; + + IReserve(reserveProxyAddress).addSpender(spender); + console.log("reserveSpenderMultiSig added as Reserve spender"); } function deployStable( diff --git a/packages/protocol/migrations_sol/migrationsConfig.json b/packages/protocol/migrations_sol/migrationsConfig.json index 94ad15ddc2f..54c5e208267 100644 --- a/packages/protocol/migrations_sol/migrationsConfig.json +++ b/packages/protocol/migrations_sol/migrationsConfig.json @@ -134,6 +134,10 @@ "internalRequired": 1, "governanceApproverMultiSig": true }, + "reserveSpenderMultiSig": { + "required": 1, + "internalRequired": 1 + }, "feeHandler": { "beneficiary": "0x2A486910DBC72cACcbb8d0e1439C96b03B2A4699", "burnFraction": 800000000000000000000000 diff --git a/packages/protocol/test-sol/utils/ReserveSpenderMultiSig.t.sol b/packages/protocol/test-sol/utils/ReserveSpenderMultiSig.t.sol new file mode 100644 index 00000000000..409cf9c0f45 --- /dev/null +++ b/packages/protocol/test-sol/utils/ReserveSpenderMultiSig.t.sol @@ -0,0 +1,13 @@ +pragma solidity ^0.5.13; + +import "@mento-core/contracts/ReserveSpenderMultiSig.sol"; + +/** + The purpose of this file is not to provide test coverage for `ReserveSpenderMultiSig.sol`. + This is an empty test to force foundry to compile `ReserveSpenderMultiSig.sol`, and include its + artifacts in the `/out` directory. `ReserveSpenderMultiSig.sol` is needed in the migrations + script, but because it's on Solidity 0.5 it can't be imported there directly. + If there is a better way to force foundry to compile `ReserveSpenderMultiSig.sol` without + this file, this file can confidently be deleted. + */ +contract ReserveSpenderMultiSigTest {} From 08222097e221883f9c2bee30317cfd689dbba0a9 Mon Sep 17 00:00:00 2001 From: Arthur Gousset <46296830+arthurgousset@users.noreply.github.com> Date: Tue, 11 Jun 2024 13:46:30 +0100 Subject: [PATCH 07/18] test: refactor foundry test directory into unit, e2e, and integration tests (#11023) * chore(foundry.toml): adds `@mento-core/...` remapping Also moves existing mappings into groups for better readability * refactor(test-sol/common): moves files to `unit/common/` Also updates imports respectively using remappings. * refactor(test-sol/identity): moves files to `unit/identity/` Also updates imports using remappings where necessary. * refactor(test-sol/stability): moves files to `unit/stability/` Also updates imports using remappings where necessary. * refactor(test-sol/voting): moves files to `unit/voting/` Also updates imports using remappings where necessary. * refactor(test-sol/governance): moves files to `unit/governance/` Also updates imports using remappings where necessary. * chore(test-sol): update missing remappings With these changes all contracts compile and resolve correctly. * chore(workflows/protocol_tests): updates paths to `test-sol/unit` * chore(workflows/protocol_tests): adds integration and e2e tests * style(workflows/protocol_tests): refactors `forge test` for better readability * chore(migrations_sol): moves scripts to `scripts/foundry/ Moves all bash scripts from `migrations_sol` to `scripts/foundry/`, and updates paths where necessary. We already have a directory for `scripts/truffle/` and `scripts/bash/`, so this makes it easier to find foundry-related bash scripts. * test(governance/mock): move `MockGovernance` to `unit/` folder * refactor(foundry.toml): rename `migrations-sol/` remapping * refactor(workflows): refactor "run everything" step Runs all tests in the `unit/` directory instead of all test files in the repo. This makes sense from my perspective, because e2e tests (in the `e2e/` directory) and integration tests (in the `integration/` directory) require a connection to an anvil devchain serving at localhost. The intent of this command is to ensure that no unit tests are forgotten, since unit tests are run explicitly by going through the directories above. But, the intention is not to run all tests in the repo generally. * style(workflows/protocol-devchain-anvil): splits migration into 2 steps This helps the script becoming more readable, and ensure error logs are clearly associated with a workflow step rather than a single bash script. * chore(workflows): defines `ANVIL_PORT` env variable in workflow Previously, the `$ANVIL_PORT` variable was exported and passed around as an env variable in a bash script. But that required generating anvil migrations and running a devchain repeatedly. Instead, the workflow does that once and different steps can access the devchain. But, in that case the env variable is not passed around, so it has to be defined at the workflow level. Source: https://docs.github.com/en/actions/learn-github-actions/variables * chore(workflows/protocol_tests): removes code comment * feat(scripts/foundry): adds `stop_anvil.sh` * refactor(scripts/foundry): use `stop_anvil.sh` to stop anvil * feat(protocol/package.json): adds `anvil-devchain:...` (`start` and `stop`) * refactor(scripts/foundry): use `stop_anvil.sh` to stop anvil * refactor(migrations_sol): update `@migrations-sol` remapping Previous the remapping was called `@celo-migrations`. * docs(migrations_sol/README): renames README file Also changes title to match NPM package name. This is a matter of personal preference. IMO this is a small improvement in readability for 3rd party users. * docs(scripts/foundry): adds more links to `package.json` Adds `repository` and `directory` to `package.json`. This ensures npmjs.org displays hyperlinks to the github repository. * docs(migrations_sol/CONTRIBUTING): adds MVP dev docs We can complete this doc with additional information regarding the anvil devchain and migrations going forward. * docs(migrations_sol/README): adds "how we work" section This is helpful for 3rd party developers that found this package on npmjs.org. The links help developers take action and help improve the anvil devchain. --- .github/workflows/protocol-devchain-anvil.yml | 13 +++- .github/workflows/protocol_tests.yml | 69 ++++++++++++++----- packages/protocol/foundry.toml | 20 +++--- .../protocol/migrations_sol/CONTRIBUTING.md | 57 +++++++++++++++ .../protocol/migrations_sol/Migration.s.sol | 4 +- packages/protocol/migrations_sol/README.md | 30 ++++++++ .../migrations_sol/celo-anvil-README.md | 10 --- .../migrations_sol/run_e2e_tests_in_anvil.sh | 10 +-- packages/protocol/package.json | 2 + .../create_and_migrate_anvil_devchain.sh | 6 +- .../foundry}/deploy_libraries.sh | 0 .../foundry}/deploy_precompiles.sh | 0 .../run_integration_tests_in_anvil.sh | 12 ++-- .../foundry}/start_anvil.sh | 4 +- .../protocol/scripts/foundry/stop_anvil.sh | 11 +++ .../protocol/test-sol/common/Import05.t.sol | 8 --- .../test-sol/integration/Integration.t.sol | 2 +- .../RevokeCeloAfterL2Transition.sol | 4 +- .../test-sol/{ => unit}/common/Accounts.t.sol | 0 .../AddressSortedLinkedListWithMedian.t.sol | 0 .../common/ExtractFunctionSignature.t.sol | 0 .../common/FeeCurrencyDirectory.t.sol | 0 .../common/FeeCurrencyWhitelist.t.sol | 2 +- .../{ => unit}/common/FeeHandler.t.sol | 10 +-- .../test-sol/{ => unit}/common/Fixidity.t.sol | 0 .../{ => unit}/common/GasPriceMinimum.t.sol | 0 .../{ => unit}/common/GoldToken.t.sol | 2 +- .../{ => unit}/common/GoldTokenMock.sol | 0 .../test-sol/{ => unit}/common/Heap.t.sol | 0 .../test-sol/unit/common/Import05.t.sol | 8 +++ .../{ => unit}/common/ImportPrecompiles.t.sol | 0 .../{ => unit}/common/IsL2Check.t.sol | 0 .../{ => unit}/common/LinkedList.t.sol | 0 .../common/MentoFeeCurrencyAdapter.t.sol | 4 +- .../{ => unit}/common/MintGoldSchedule.t.sol | 2 +- .../test-sol/{ => unit}/common/Multisig.t.sol | 0 .../test-sol/{ => unit}/common/Proxy.t.sol | 0 .../test-sol/{ => unit}/common/Registry.t.sol | 0 .../governance/mock/MockGovernance.sol | 0 .../network/BlockchainParameters.t.sol | 0 .../governance/network/EpochRewards.t.sol | 2 +- .../governance/network/Governance.t.sol | 0 .../network/GovernanceSlasher.t.sol | 0 .../governance/network/Proposal.t.sol | 0 .../validators/DoubleSigningSlasher.t.sol | 0 .../validators/DowntimeSlasher.t.sol | 0 .../IntegerSortedLinkedListMock-8.sol | 0 .../IntergerSortedLinkedList-8.t.sol | 2 +- .../validators/IntergerSortedLinkedList.t.sol | 0 .../governance/validators/Validators.t.sol | 2 +- .../validators/mocks/ValidatorsMockTunnel.sol | 0 .../governance/voting/LockedGold.t.sol | 2 +- .../governance/voting/ReleaseGold.t.sol | 32 ++++----- .../voting/mocks/ReleaseGoldMockTunnel.sol | 2 +- .../{ => unit}/identity/Attestations.t.sol | 0 .../test-sol/{ => unit}/identity/Escrow.t.sol | 0 .../identity/FederatedAttestations.t.sol | 0 .../{ => unit}/identity/IdentityProxy.t.sol | 0 .../identity/IdentityProxyHub.t.sol | 0 .../{ => unit}/identity/OdisPayments.t.sol | 2 +- .../test-sol/{ => unit}/identity/Random.t.sol | 0 .../stability/FeeCurrencyAdapter.t.sol | 4 +- .../stability/SortedOracles.mento.t.sol | 4 +- .../{ => unit}/stability/SortedOracles.t.sol | 4 +- .../test-sol/{ => unit}/voting/Election.t.sol | 4 +- 65 files changed, 238 insertions(+), 112 deletions(-) create mode 100644 packages/protocol/migrations_sol/CONTRIBUTING.md create mode 100644 packages/protocol/migrations_sol/README.md delete mode 100644 packages/protocol/migrations_sol/celo-anvil-README.md rename packages/protocol/{migrations_sol => scripts/foundry}/create_and_migrate_anvil_devchain.sh (94%) rename packages/protocol/{migrations_sol => scripts/foundry}/deploy_libraries.sh (100%) rename packages/protocol/{migrations_sol => scripts/foundry}/deploy_precompiles.sh (100%) rename packages/protocol/{migrations_sol => scripts/foundry}/run_integration_tests_in_anvil.sh (58%) rename packages/protocol/{migrations_sol => scripts/foundry}/start_anvil.sh (68%) create mode 100755 packages/protocol/scripts/foundry/stop_anvil.sh delete mode 100644 packages/protocol/test-sol/common/Import05.t.sol rename packages/protocol/test-sol/{ => unit}/common/Accounts.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/AddressSortedLinkedListWithMedian.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/ExtractFunctionSignature.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/FeeCurrencyDirectory.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/FeeCurrencyWhitelist.t.sol (97%) rename packages/protocol/test-sol/{ => unit}/common/FeeHandler.t.sol (98%) rename packages/protocol/test-sol/{ => unit}/common/Fixidity.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/GasPriceMinimum.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/GoldToken.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/common/GoldTokenMock.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/Heap.t.sol (100%) create mode 100644 packages/protocol/test-sol/unit/common/Import05.t.sol rename packages/protocol/test-sol/{ => unit}/common/ImportPrecompiles.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/IsL2Check.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/LinkedList.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/MentoFeeCurrencyAdapter.t.sol (97%) rename packages/protocol/test-sol/{ => unit}/common/MintGoldSchedule.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/common/Multisig.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/Proxy.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/common/Registry.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/mock/MockGovernance.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/network/BlockchainParameters.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/network/EpochRewards.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/governance/network/Governance.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/network/GovernanceSlasher.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/network/Proposal.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/validators/DoubleSigningSlasher.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/validators/DowntimeSlasher.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/validators/IntegerSortedLinkedListMock-8.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/validators/IntergerSortedLinkedList-8.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/governance/validators/IntergerSortedLinkedList.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/validators/Validators.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/governance/validators/mocks/ValidatorsMockTunnel.sol (100%) rename packages/protocol/test-sol/{ => unit}/governance/voting/LockedGold.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/governance/voting/ReleaseGold.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/governance/voting/mocks/ReleaseGoldMockTunnel.sol (97%) rename packages/protocol/test-sol/{ => unit}/identity/Attestations.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/identity/Escrow.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/identity/FederatedAttestations.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/identity/IdentityProxy.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/identity/IdentityProxyHub.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/identity/OdisPayments.t.sol (98%) rename packages/protocol/test-sol/{ => unit}/identity/Random.t.sol (100%) rename packages/protocol/test-sol/{ => unit}/stability/FeeCurrencyAdapter.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/stability/SortedOracles.mento.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/stability/SortedOracles.t.sol (99%) rename packages/protocol/test-sol/{ => unit}/voting/Election.t.sol (99%) diff --git a/.github/workflows/protocol-devchain-anvil.yml b/.github/workflows/protocol-devchain-anvil.yml index de0608208c5..14af7bfa69b 100644 --- a/.github/workflows/protocol-devchain-anvil.yml +++ b/.github/workflows/protocol-devchain-anvil.yml @@ -7,6 +7,7 @@ on: env: # Increment these to force cache rebuilding FOUNDRY_CACHE_KEY: 1 + ANVIL_PORT: 8546 jobs: build: @@ -86,8 +87,16 @@ jobs: id: date run: echo "::set-output name=date::$(date +'%Y-%m-%d')" - - name: Generate migrations - run: ./migrations_sol/run_integration_tests_in_anvil.sh + - name: Generate migrations and run devchain + if: success() || failure() + run: ./scripts/foundry/create_and_migrate_anvil_devchain.sh + + - name: Run integration tests against local anvil devchain + if: success() || failure() + run: | + forge test -vvv \ + --match-path "test-sol/integration/*" \ + --fork-url http://127.0.0.1:${{ env.ANVIL_PORT }} - name: Sanitize ref name id: sanitize-ref-name diff --git a/.github/workflows/protocol_tests.yml b/.github/workflows/protocol_tests.yml index b2f332ad8a0..d7bb9d5b9e5 100644 --- a/.github/workflows/protocol_tests.yml +++ b/.github/workflows/protocol_tests.yml @@ -11,6 +11,7 @@ on: env: # Increment these to force cache rebuilding FOUNDRY_CACHE_KEY: 2 + ANVIL_PORT: 8546 jobs: check: @@ -61,30 +62,47 @@ jobs: - name: Compile Contracts run: forge --version && forge compile - - name: Run tests common + - name: Run unit tests common # can't use gas limit because some setUp function use more than the limit - run: forge test -vvv --match-path "test-sol/common/*" # --block-gas-limit 50000000 + run: | + forge test -vvv \ + --match-path "test-sol/unit/common/*" - - name: Run tests governance/network + - name: Run unit tests governance/network if: success() || failure() - run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/governance/network/*" + run: | + forge test -vvv \ + --match-path "test-sol/unit/governance/network/*" \ + --block-gas-limit 50000000 - - name: Run tests governance/validators + - name: Run unit tests governance/validators if: success() || failure() - run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/governance/validators/*" + run: | + forge test -vvv \ + --match-path "test-sol/unit/governance/validators/*" \ + --block-gas-limit 50000000 - - name: Run tests governance/voting + - name: Run unit tests governance/voting # can't use gas limit because some setUp function use more than the limit if: success() || failure() - run: forge test -vvv --match-path "test-sol/governance/voting/*" --block-gas-limit 50000000 + run: | + forge test -vvv \ + --match-path "test-sol/unit/governance/voting/*" \ + --block-gas-limit 50000000 - - name: Run tests stability + - name: Run unit tests stability if: success() || failure() - run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/stability/*" + run: | + forge test -vvv \ + --match-path "test-sol/unit/stability/*" \ + --block-gas-limit 50000000 - - name: Run tests identity + - name: Run unit tests identity if: success() || failure() - run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/identity/*" + run: | + forge test -vvv \ + --match-path "test-sol/unit/identity/*" \ + --block-gas-limit 50000000 - name: Fail if there are tests without folder if: success() || failure() @@ -94,11 +112,28 @@ jobs: exit 1 fi - - name: Run everything just in case something was missed (excl. integration and e2e tests) + - name: Run all unit tests in case some were missed (excl. integration and e2e tests) # can't use gas limit because some setUp function use more than the limit - # Excludes integration tests, because they require an anvil devchain running on the localhost. - run: forge test -vvv --no-match-contract "RegistryIntegrationTest|E2EDemo" + # Excludes e2e and integration tests, because they require a connection to an anvil devchain + # serving at localhost. + run: | + forge test -vvv \ + --match-path "test-sol/unit/*" - - name: Generate migrations + - name: Generate migrations and run devchain if: success() || failure() - run: ./migrations_sol/run_integration_tests_in_anvil.sh + run: ./scripts/foundry/create_and_migrate_anvil_devchain.sh + + - name: Run integration tests against local anvil devchain + if: success() || failure() + run: | + forge test -vvv \ + --match-path "test-sol/integration/*" \ + --fork-url http://127.0.0.1:${{ env.ANVIL_PORT }} + + - name: Run e2e tests against local anvil devchain + if: success() || failure() + run: | + forge test -vvv \ + --match-path "test-sol/e2e/*" \ + --fork-url http://127.0.0.1:${{ env.ANVIL_PORT }} \ No newline at end of file diff --git a/packages/protocol/foundry.toml b/packages/protocol/foundry.toml index 3e360d306a1..cf4c98d644b 100644 --- a/packages/protocol/foundry.toml +++ b/packages/protocol/foundry.toml @@ -4,20 +4,20 @@ out = 'out' test = 'test-sol' libs = ['lib', 'node_modules'] remappings = [ + '@celo-contracts/=contracts/', + '@celo-contracts-8=contracts-0.8/', + '@mento-core/=lib/mento-core', 'openzeppelin-solidity/=lib/openzeppelin-contracts/', - 'solidity-bytes-utils/=lib/solidity-bytes-utils/', - 'forge-std/=lib/celo-foundry/lib/forge-std/src/', - 'ds-test/=lib/celo-foundry/lib/forge-std/lib/ds-test/src/', + '@openzeppelin/contracts8/=lib/openzeppelin-contracts8/contracts/', 'celo-foundry/=lib/celo-foundry/src/', - '@summa-tx/memview.sol/=lib/memview.sol', 'celo-foundry-8/=lib/celo-foundry-8/src/', - 'forge-std-8/=lib/celo-foundry-8/lib/forge-std/src/', - '@celo-contracts-8=contracts-0.8/', - '@openzeppelin/contracts8/=lib/openzeppelin-contracts8/contracts/', - '@celo-contracts/=contracts/', - '@celo-migrations/=migrations_sol/', '@test-sol/=test-sol/', - '@mento-core/=lib/mento-core/' + '@migrations-sol/=migrations_sol/', + 'forge-std/=lib/celo-foundry/lib/forge-std/src/', + 'forge-std-8/=lib/celo-foundry-8/lib/forge-std/src/', + '@summa-tx/memview.sol/=lib/memview.sol', + 'solidity-bytes-utils/=lib/solidity-bytes-utils/', + 'ds-test/=lib/celo-foundry/lib/forge-std/lib/ds-test/src/', ] no_match_test = "skip" diff --git a/packages/protocol/migrations_sol/CONTRIBUTING.md b/packages/protocol/migrations_sol/CONTRIBUTING.md new file mode 100644 index 00000000000..54e32e6bff4 --- /dev/null +++ b/packages/protocol/migrations_sol/CONTRIBUTING.md @@ -0,0 +1,57 @@ +# Anvil migrations + +The "Anvil migrations" are a set of scripts that generate a local anvil-based devchain. +This devchain is useful for testing and development of the Celo protocol. + +## Usage + +### Start devchain + +```sh +$ yarn anvil-devchain:start +``` + +Starts a new anvil devchain serving at localhost (default port 8546). + +For example: + +```sh +$ yarn anvil-devchain:start + +yarn run v1.22.22 +$ ./scripts/foundry/create_and_migrate_anvil_devchain.sh +# ... +Total elapsed time: 193 seconds +✨ Done in 193.09s. +``` + +You can now run commands against the local devchain. + +For example: + +```sh +cast block-number \ +--rpc-url http://127.0.0.1:8546 +266 +``` + +### Stop devchain + +```sh +$ yarn anvil-devchain:stop +``` + +Terminates any anvil nodes serving at localhost. + +For example: + +```sh +# in packages/protocol/ directory +$ yarn anvil-devchain:stop + +yarn run v1.22.22 +$ ./scripts/foundry/stop_anvil.sh +Connection to localhost port 8546 [tcp/*] succeeded! +Killed Anvil +✨ Done in 0.11s. +``` \ No newline at end of file diff --git a/packages/protocol/migrations_sol/Migration.s.sol b/packages/protocol/migrations_sol/Migration.s.sol index 2cf777bc169..d7c4ab31209 100644 --- a/packages/protocol/migrations_sol/Migration.s.sol +++ b/packages/protocol/migrations_sol/Migration.s.sol @@ -43,12 +43,12 @@ import "@celo-contracts/stability/interfaces/ISortedOracles.sol"; import "@celo-contracts-8/common/interfaces/IGasPriceMinimumInitializer.sol"; import "@celo-contracts-8/common/interfaces/IMintGoldScheduleInitializer.sol"; -import "@celo-migrations/HelperInterFaces.sol"; +import "@migrations-sol/HelperInterFaces.sol"; import "@openzeppelin/contracts8/utils/math/Math.sol"; import "@celo-contracts-8/common/UsingRegistry.sol"; -import { Constants } from "@celo-migrations/constants.sol"; +import { Constants } from "@migrations-sol/constants.sol"; contract ForceTx { // event to trigger so a tx can be processed diff --git a/packages/protocol/migrations_sol/README.md b/packages/protocol/migrations_sol/README.md new file mode 100644 index 00000000000..68526eb67df --- /dev/null +++ b/packages/protocol/migrations_sol/README.md @@ -0,0 +1,30 @@ +# @celo/devchain-anvil + +Anvil state with Celo core contracts for local testing and development. + +# Usage + +```bash +npm install --save-dev @celo/devchain-anvil +anvil --state +``` + +## How we work + +We are a GitHub-first team, which means we have a strong preference for communicating via GitHub. +Please use GitHub to: + +🐞 [File a bug report](https://github.com/celo-org/celo-monorepo/issues/new/choose) + +💬 [Ask a question](https://github.com/celo-org/celo-monorepo/discussions) + +✨ [Suggest a feature](https://github.com/celo-org/celo-monorepo/issues/new/choose) + +🧑‍💻 [Contribute!](https://github.com/celo-org/celo-monorepo/tree/master/packages/protocol/migrations_sol/CONTRIBUTING.md) + +🚔 [Report a security vulnerability](https://github.com/celo-org/celo-monorepo/issues/new/choose) + +> [!TIP] +> +> Please avoid messaging us via Slack, Telegram, or email. We are more likely to respond to you on +> GitHub than if you message us anywhere else. We actively monitor GitHub, and will get back to you shortly 🌟 diff --git a/packages/protocol/migrations_sol/celo-anvil-README.md b/packages/protocol/migrations_sol/celo-anvil-README.md deleted file mode 100644 index 46a9b59be67..00000000000 --- a/packages/protocol/migrations_sol/celo-anvil-README.md +++ /dev/null @@ -1,10 +0,0 @@ -# celo devchain Anvil - -Anvil state with Celo core contracts for local testing and development. - -# Usage - -```bash -npm install --save-dev @celo/devchain-anvil -anvil --state -``` diff --git a/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh b/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh index 3e1289cc60b..dc1d1483c49 100755 --- a/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh +++ b/packages/protocol/migrations_sol/run_e2e_tests_in_anvil.sh @@ -12,10 +12,6 @@ forge test \ --match-path "*test-sol/e2e/*" \ --fork-url http://127.0.0.1:$ANVIL_PORT -# helper kill anvil -# kill $(lsof -i tcp:$ANVIL_PORT | tail -n 1 | awk '{print $2}') - -echo "Killing Anvil" -if [[ -n $ANVIL_PID ]]; then - kill $ANVIL_PID -fi +# Stop devchain +echo "Stopping devchain..." +source $PWD/scripts/foundry/stop_anvil.sh \ No newline at end of file diff --git a/packages/protocol/package.json b/packages/protocol/package.json index 330d769084d..87f65edd23d 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -50,6 +50,8 @@ "truffle:migrate": "truffle migrate", "devchain": "yarn ts-node scripts/devchain.ts", "devchain:reset": "yarn devchain generate-tar .tmp/devchain.tar.gz --upto 29", + "anvil-devchain:start": "./scripts/foundry/create_and_migrate_anvil_devchain.sh", + "anvil-devchain:stop": "./scripts/foundry/stop_anvil.sh", "view-tags": "git for-each-ref 'refs/tags/core-contracts.*' --sort=-committerdate --format='%(color:magenta)%(committerdate:short) %(color:blue)%(tree) %(color:green)github.com/celo-org/celo-monorepo/releases/tag/%(color:yellow)%(refname:short)'", "generate-stabletoken-files": "yarn ts-node ./scripts/generate-stabletoken-files.ts", "truffle-verify": "yarn truffle run verify" diff --git a/packages/protocol/migrations_sol/create_and_migrate_anvil_devchain.sh b/packages/protocol/scripts/foundry/create_and_migrate_anvil_devchain.sh similarity index 94% rename from packages/protocol/migrations_sol/create_and_migrate_anvil_devchain.sh rename to packages/protocol/scripts/foundry/create_and_migrate_anvil_devchain.sh index 8050b93c302..b4688d4cfda 100755 --- a/packages/protocol/migrations_sol/create_and_migrate_anvil_devchain.sh +++ b/packages/protocol/scripts/foundry/create_and_migrate_anvil_devchain.sh @@ -22,10 +22,10 @@ fi mkdir -p $TEMP_FOLDER # Start a local anvil instance -source $PWD/migrations_sol/start_anvil.sh +source $PWD/scripts/foundry/start_anvil.sh # Deploy libraries to the anvil instance -source $PWD/migrations_sol/deploy_libraries.sh +source $PWD/scripts/foundry/deploy_libraries.sh echo "Library flags are: $LIBRARY_FLAGS" # Build all contracts with deployed libraries @@ -35,7 +35,7 @@ echo "Compiling with libraries... " time forge build $LIBRARY_FLAGS # Deploy precompile contracts -source $PWD/migrations_sol/deploy_precompiles.sh +source $PWD/scripts/foundry/deploy_precompiles.sh echo "Setting Registry Proxy" REGISTRY_ADDRESS="0x000000000000000000000000000000000000ce10" diff --git a/packages/protocol/migrations_sol/deploy_libraries.sh b/packages/protocol/scripts/foundry/deploy_libraries.sh similarity index 100% rename from packages/protocol/migrations_sol/deploy_libraries.sh rename to packages/protocol/scripts/foundry/deploy_libraries.sh diff --git a/packages/protocol/migrations_sol/deploy_precompiles.sh b/packages/protocol/scripts/foundry/deploy_precompiles.sh similarity index 100% rename from packages/protocol/migrations_sol/deploy_precompiles.sh rename to packages/protocol/scripts/foundry/deploy_precompiles.sh diff --git a/packages/protocol/migrations_sol/run_integration_tests_in_anvil.sh b/packages/protocol/scripts/foundry/run_integration_tests_in_anvil.sh similarity index 58% rename from packages/protocol/migrations_sol/run_integration_tests_in_anvil.sh rename to packages/protocol/scripts/foundry/run_integration_tests_in_anvil.sh index af25e4b069a..8b45f6879b1 100755 --- a/packages/protocol/migrations_sol/run_integration_tests_in_anvil.sh +++ b/packages/protocol/scripts/foundry/run_integration_tests_in_anvil.sh @@ -3,7 +3,7 @@ set -euo pipefail # Generate and run devchain echo "Generating and running devchain before running integration tests..." -source $PWD/migrations_sol/create_and_migrate_anvil_devchain.sh +source $PWD/scripts/foundry/create_and_migrate_anvil_devchain.sh # Run integration tests echo "Running integration tests..." @@ -12,10 +12,6 @@ forge test \ --match-contract RegistryIntegrationTest \ --fork-url http://127.0.0.1:$ANVIL_PORT -# helper kill anvil -# kill $(lsof -i tcp:$ANVIL_PORT | tail -n 1 | awk '{print $2}') - -echo "Killing Anvil" -if [[ -n $ANVIL_PID ]]; then - kill $ANVIL_PID -fi +# Stop devchain +echo "Stopping devchain..." +source $PWD/scripts/foundry/stop_anvil.sh \ No newline at end of file diff --git a/packages/protocol/migrations_sol/start_anvil.sh b/packages/protocol/scripts/foundry/start_anvil.sh similarity index 68% rename from packages/protocol/migrations_sol/start_anvil.sh rename to packages/protocol/scripts/foundry/start_anvil.sh index e75a82162b1..8387077b196 100755 --- a/packages/protocol/migrations_sol/start_anvil.sh +++ b/packages/protocol/scripts/foundry/start_anvil.sh @@ -8,9 +8,9 @@ mkdir -p $ANVIL_FOLDER echo "Anvil state will be saved to $ANVIL_FOLDER" # create package.json -echo "{\"name\": \"@celo/devchain-anvil\",\"version\": \"1.0.0\",\"description\": \"Anvil based devchain that contains core smart contracts of celo\",\"author\":\"Celo\",\"license\": \"LGPL-3.0\"}" > $TMP_FOLDER/package.json +echo "{\"name\": \"@celo/devchain-anvil\",\"version\": \"1.0.0\",\"repository\": { \"url\": \"https://github.com/celo-org/celo-monorepo\", \"directory\": \"packages/protocol/migrations_sol\" },\"homepage\": \"https://github.com/celo-org/celo-monorepo/blob/master/packages/protocol/migrations_sol/README.md\",\"description\": \"Anvil based devchain that contains core smart contracts of celo\",\"author\":\"Celo\",\"license\": \"LGPL-3.0\"}" > $TMP_FOLDER/package.json -cp $PWD/migrations_sol/celo-anvil-README.md $TMP_FOLDER/README.md +cp $PWD/migrations_sol/README.md $TMP_FOLDER/README.md if nc -z localhost $ANVIL_PORT; then echo "Port already used" diff --git a/packages/protocol/scripts/foundry/stop_anvil.sh b/packages/protocol/scripts/foundry/stop_anvil.sh new file mode 100755 index 00000000000..c1b8c0456bb --- /dev/null +++ b/packages/protocol/scripts/foundry/stop_anvil.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +set -euo pipefail + +# A small script to terminate any instance of anvil currently serving at localhost. + +ANVIL_PORT=8546 + +if nc -z localhost $ANVIL_PORT; then + kill $(lsof -i tcp:$ANVIL_PORT | tail -n 1 | awk '{print $2}') + echo "Killed Anvil" +fi \ No newline at end of file diff --git a/packages/protocol/test-sol/common/Import05.t.sol b/packages/protocol/test-sol/common/Import05.t.sol deleted file mode 100644 index 30d1f1760cc..00000000000 --- a/packages/protocol/test-sol/common/Import05.t.sol +++ /dev/null @@ -1,8 +0,0 @@ -pragma solidity ^0.5.13; - -// this file only exists so that foundry compiles this contracts -import "../../lib/mento-core/contracts/StableToken.sol"; -import "../../lib/mento-core/contracts/StableTokenBRL.sol"; -import "../../lib/mento-core/contracts/StableTokenEUR.sol"; - -contract Import05 {} diff --git a/packages/protocol/test-sol/integration/Integration.t.sol b/packages/protocol/test-sol/integration/Integration.t.sol index e601de7ff46..392a3427fdd 100644 --- a/packages/protocol/test-sol/integration/Integration.t.sol +++ b/packages/protocol/test-sol/integration/Integration.t.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.7 <0.8.20; import { Test } from "forge-std-8/Test.sol"; import "forge-std-8/console2.sol"; -import { Constants } from "@celo-migrations/constants.sol"; +import { Constants } from "@migrations-sol/constants.sol"; import "@celo-contracts/common/interfaces/IRegistry.sol"; import "@celo-contracts/common/interfaces/IProxy.sol"; diff --git a/packages/protocol/test-sol/integration/RevokeCeloAfterL2Transition.sol b/packages/protocol/test-sol/integration/RevokeCeloAfterL2Transition.sol index aac1d94d517..f12e22642e1 100644 --- a/packages/protocol/test-sol/integration/RevokeCeloAfterL2Transition.sol +++ b/packages/protocol/test-sol/integration/RevokeCeloAfterL2Transition.sol @@ -23,8 +23,8 @@ import "@test-sol/constants.sol"; import "@test-sol/utils/ECDSAHelper.sol"; import { Utils } from "@test-sol/utils.sol"; import { Test as ForgeTest } from "forge-std/Test.sol"; -import "../governance/validators/mocks/ValidatorsMockTunnel.sol"; -import "../governance/voting/mocks/ReleaseGoldMockTunnel.sol"; +import "@test-sol/unit/governance/validators/mocks/ValidatorsMockTunnel.sol"; +import "@test-sol/unit/governance/voting/mocks/ReleaseGoldMockTunnel.sol"; contract RevokeCeloAfterL2Transition is Test, Constants, ECDSAHelper, Utils { using FixidityLib for FixidityLib.Fraction; diff --git a/packages/protocol/test-sol/common/Accounts.t.sol b/packages/protocol/test-sol/unit/common/Accounts.t.sol similarity index 100% rename from packages/protocol/test-sol/common/Accounts.t.sol rename to packages/protocol/test-sol/unit/common/Accounts.t.sol diff --git a/packages/protocol/test-sol/common/AddressSortedLinkedListWithMedian.t.sol b/packages/protocol/test-sol/unit/common/AddressSortedLinkedListWithMedian.t.sol similarity index 100% rename from packages/protocol/test-sol/common/AddressSortedLinkedListWithMedian.t.sol rename to packages/protocol/test-sol/unit/common/AddressSortedLinkedListWithMedian.t.sol diff --git a/packages/protocol/test-sol/common/ExtractFunctionSignature.t.sol b/packages/protocol/test-sol/unit/common/ExtractFunctionSignature.t.sol similarity index 100% rename from packages/protocol/test-sol/common/ExtractFunctionSignature.t.sol rename to packages/protocol/test-sol/unit/common/ExtractFunctionSignature.t.sol diff --git a/packages/protocol/test-sol/common/FeeCurrencyDirectory.t.sol b/packages/protocol/test-sol/unit/common/FeeCurrencyDirectory.t.sol similarity index 100% rename from packages/protocol/test-sol/common/FeeCurrencyDirectory.t.sol rename to packages/protocol/test-sol/unit/common/FeeCurrencyDirectory.t.sol diff --git a/packages/protocol/test-sol/common/FeeCurrencyWhitelist.t.sol b/packages/protocol/test-sol/unit/common/FeeCurrencyWhitelist.t.sol similarity index 97% rename from packages/protocol/test-sol/common/FeeCurrencyWhitelist.t.sol rename to packages/protocol/test-sol/unit/common/FeeCurrencyWhitelist.t.sol index 152873d76d4..8f5d6a787a0 100644 --- a/packages/protocol/test-sol/common/FeeCurrencyWhitelist.t.sol +++ b/packages/protocol/test-sol/unit/common/FeeCurrencyWhitelist.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.5.13; import "celo-foundry/Test.sol"; -import "../../contracts/common/FeeCurrencyWhitelist.sol"; +import "@celo-contracts/common/FeeCurrencyWhitelist.sol"; contract FeeCurrencyWhitelistTest is Test { FeeCurrencyWhitelist feeCurrencyWhitelist; diff --git a/packages/protocol/test-sol/common/FeeHandler.t.sol b/packages/protocol/test-sol/unit/common/FeeHandler.t.sol similarity index 98% rename from packages/protocol/test-sol/common/FeeHandler.t.sol rename to packages/protocol/test-sol/unit/common/FeeHandler.t.sol index fc65e2df59c..6addc95f452 100644 --- a/packages/protocol/test-sol/common/FeeHandler.t.sol +++ b/packages/protocol/test-sol/unit/common/FeeHandler.t.sol @@ -4,10 +4,10 @@ pragma experimental ABIEncoderV2; import "celo-foundry/Test.sol"; import "@celo-contracts/common/FeeHandler.sol"; -import "../constants.sol"; +import { Constants } from "@test-sol/constants.sol"; -import { Exchange } from "../../lib/mento-core/contracts/Exchange.sol"; -import { StableToken } from "../../lib/mento-core/contracts/StableToken.sol"; +import { Exchange } from "@mento-core/contracts/Exchange.sol"; +import { StableToken } from "@mento-core/contracts/StableToken.sol"; import "@celo-contracts/common/FixidityLib.sol"; import "@celo-contracts/common/Freezer.sol"; import "@celo-contracts/common/GoldToken.sol"; @@ -17,8 +17,8 @@ import "@celo-contracts/common/UniswapFeeHandlerSeller.sol"; import "@celo-contracts/uniswap/test/MockUniswapV2Router02.sol"; import "@celo-contracts/uniswap/test/MockUniswapV2Factory.sol"; import "@celo-contracts/uniswap/test/MockERC20.sol"; -import "../../lib/mento-core/test/mocks/MockSortedOracles.sol"; -import "../../lib/mento-core/test/mocks/MockReserve.sol"; +import "@mento-core/test/mocks/MockSortedOracles.sol"; +import "@mento-core/test/mocks/MockReserve.sol"; contract FeeHandlerTest is Test, Constants { using FixidityLib for FixidityLib.Fraction; diff --git a/packages/protocol/test-sol/common/Fixidity.t.sol b/packages/protocol/test-sol/unit/common/Fixidity.t.sol similarity index 100% rename from packages/protocol/test-sol/common/Fixidity.t.sol rename to packages/protocol/test-sol/unit/common/Fixidity.t.sol diff --git a/packages/protocol/test-sol/common/GasPriceMinimum.t.sol b/packages/protocol/test-sol/unit/common/GasPriceMinimum.t.sol similarity index 100% rename from packages/protocol/test-sol/common/GasPriceMinimum.t.sol rename to packages/protocol/test-sol/unit/common/GasPriceMinimum.t.sol diff --git a/packages/protocol/test-sol/common/GoldToken.t.sol b/packages/protocol/test-sol/unit/common/GoldToken.t.sol similarity index 99% rename from packages/protocol/test-sol/common/GoldToken.t.sol rename to packages/protocol/test-sol/unit/common/GoldToken.t.sol index f5aac2c5f58..c422eef35c9 100644 --- a/packages/protocol/test-sol/common/GoldToken.t.sol +++ b/packages/protocol/test-sol/unit/common/GoldToken.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.5.13; import "celo-foundry/Test.sol"; import "@celo-contracts/common/GoldToken.sol"; -import "./GoldTokenMock.sol"; +import "@test-sol/unit/common/GoldTokenMock.sol"; contract GoldTokenTest is Test, IsL2Check { GoldToken goldToken; diff --git a/packages/protocol/test-sol/common/GoldTokenMock.sol b/packages/protocol/test-sol/unit/common/GoldTokenMock.sol similarity index 100% rename from packages/protocol/test-sol/common/GoldTokenMock.sol rename to packages/protocol/test-sol/unit/common/GoldTokenMock.sol diff --git a/packages/protocol/test-sol/common/Heap.t.sol b/packages/protocol/test-sol/unit/common/Heap.t.sol similarity index 100% rename from packages/protocol/test-sol/common/Heap.t.sol rename to packages/protocol/test-sol/unit/common/Heap.t.sol diff --git a/packages/protocol/test-sol/unit/common/Import05.t.sol b/packages/protocol/test-sol/unit/common/Import05.t.sol new file mode 100644 index 00000000000..48be42e45fb --- /dev/null +++ b/packages/protocol/test-sol/unit/common/Import05.t.sol @@ -0,0 +1,8 @@ +pragma solidity ^0.5.13; + +// this file only exists so that foundry compiles this contracts +import "@mento-core/contracts/StableToken.sol"; +import "@mento-core/contracts/StableTokenBRL.sol"; +import "@mento-core/contracts/StableTokenEUR.sol"; + +contract Import05 {} diff --git a/packages/protocol/test-sol/common/ImportPrecompiles.t.sol b/packages/protocol/test-sol/unit/common/ImportPrecompiles.t.sol similarity index 100% rename from packages/protocol/test-sol/common/ImportPrecompiles.t.sol rename to packages/protocol/test-sol/unit/common/ImportPrecompiles.t.sol diff --git a/packages/protocol/test-sol/common/IsL2Check.t.sol b/packages/protocol/test-sol/unit/common/IsL2Check.t.sol similarity index 100% rename from packages/protocol/test-sol/common/IsL2Check.t.sol rename to packages/protocol/test-sol/unit/common/IsL2Check.t.sol diff --git a/packages/protocol/test-sol/common/LinkedList.t.sol b/packages/protocol/test-sol/unit/common/LinkedList.t.sol similarity index 100% rename from packages/protocol/test-sol/common/LinkedList.t.sol rename to packages/protocol/test-sol/unit/common/LinkedList.t.sol diff --git a/packages/protocol/test-sol/common/MentoFeeCurrencyAdapter.t.sol b/packages/protocol/test-sol/unit/common/MentoFeeCurrencyAdapter.t.sol similarity index 97% rename from packages/protocol/test-sol/common/MentoFeeCurrencyAdapter.t.sol rename to packages/protocol/test-sol/unit/common/MentoFeeCurrencyAdapter.t.sol index d4bab08d2b0..8c5657cc0c9 100644 --- a/packages/protocol/test-sol/common/MentoFeeCurrencyAdapter.t.sol +++ b/packages/protocol/test-sol/unit/common/MentoFeeCurrencyAdapter.t.sol @@ -2,8 +2,8 @@ pragma solidity >=0.8.7 <0.8.20; import "celo-foundry-8/Test.sol"; -import "../../contracts-0.8/common/MentoFeeCurrencyAdapter.sol"; -import "../../contracts-0.8/common/mocks/MockOracle.sol"; +import "@celo-contracts-8/common/MentoFeeCurrencyAdapter.sol"; +import "@celo-contracts-8/common/mocks/MockOracle.sol"; contract MentoFeeCurrencyAdapterBase is Test { MentoFeeCurrencyAdapter mentoAdapter; diff --git a/packages/protocol/test-sol/common/MintGoldSchedule.t.sol b/packages/protocol/test-sol/unit/common/MintGoldSchedule.t.sol similarity index 99% rename from packages/protocol/test-sol/common/MintGoldSchedule.t.sol rename to packages/protocol/test-sol/unit/common/MintGoldSchedule.t.sol index 40e3f984578..53fe8155d6e 100644 --- a/packages/protocol/test-sol/common/MintGoldSchedule.t.sol +++ b/packages/protocol/test-sol/unit/common/MintGoldSchedule.t.sol @@ -11,7 +11,7 @@ import "@celo-contracts-8/common/MintGoldSchedule.sol"; import "@celo-contracts-8/common/IsL2Check.sol"; import { Constants } from "@test-sol/constants.sol"; -import "../governance/mock/MockGovernance.sol"; +import "@test-sol/unit/governance/mock/MockGovernance.sol"; contract MintGoldScheduleTest is Test, Constants, IsL2Check { using FixidityLib for FixidityLib.Fraction; diff --git a/packages/protocol/test-sol/common/Multisig.t.sol b/packages/protocol/test-sol/unit/common/Multisig.t.sol similarity index 100% rename from packages/protocol/test-sol/common/Multisig.t.sol rename to packages/protocol/test-sol/unit/common/Multisig.t.sol diff --git a/packages/protocol/test-sol/common/Proxy.t.sol b/packages/protocol/test-sol/unit/common/Proxy.t.sol similarity index 100% rename from packages/protocol/test-sol/common/Proxy.t.sol rename to packages/protocol/test-sol/unit/common/Proxy.t.sol diff --git a/packages/protocol/test-sol/common/Registry.t.sol b/packages/protocol/test-sol/unit/common/Registry.t.sol similarity index 100% rename from packages/protocol/test-sol/common/Registry.t.sol rename to packages/protocol/test-sol/unit/common/Registry.t.sol diff --git a/packages/protocol/test-sol/governance/mock/MockGovernance.sol b/packages/protocol/test-sol/unit/governance/mock/MockGovernance.sol similarity index 100% rename from packages/protocol/test-sol/governance/mock/MockGovernance.sol rename to packages/protocol/test-sol/unit/governance/mock/MockGovernance.sol diff --git a/packages/protocol/test-sol/governance/network/BlockchainParameters.t.sol b/packages/protocol/test-sol/unit/governance/network/BlockchainParameters.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/network/BlockchainParameters.t.sol rename to packages/protocol/test-sol/unit/governance/network/BlockchainParameters.t.sol diff --git a/packages/protocol/test-sol/governance/network/EpochRewards.t.sol b/packages/protocol/test-sol/unit/governance/network/EpochRewards.t.sol similarity index 99% rename from packages/protocol/test-sol/governance/network/EpochRewards.t.sol rename to packages/protocol/test-sol/unit/governance/network/EpochRewards.t.sol index bac0bc3888a..55ca1b3f58d 100644 --- a/packages/protocol/test-sol/governance/network/EpochRewards.t.sol +++ b/packages/protocol/test-sol/unit/governance/network/EpochRewards.t.sol @@ -11,7 +11,7 @@ import { Reserve } from "@lib/mento-core/contracts/Reserve.sol"; import { MockSortedOracles } from "@celo-contracts/stability/test/MockSortedOracles.sol"; import { MockStableToken } from "@celo-contracts/stability/test/MockStableToken.sol"; -import { GoldTokenMock } from "@test-sol/common/GoldTokenMock.sol"; +import { GoldTokenMock } from "@test-sol/unit/common/GoldTokenMock.sol"; import { Constants } from "@test-sol/constants.sol"; import { Utils } from "@test-sol/utils.sol"; diff --git a/packages/protocol/test-sol/governance/network/Governance.t.sol b/packages/protocol/test-sol/unit/governance/network/Governance.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/network/Governance.t.sol rename to packages/protocol/test-sol/unit/governance/network/Governance.t.sol diff --git a/packages/protocol/test-sol/governance/network/GovernanceSlasher.t.sol b/packages/protocol/test-sol/unit/governance/network/GovernanceSlasher.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/network/GovernanceSlasher.t.sol rename to packages/protocol/test-sol/unit/governance/network/GovernanceSlasher.t.sol diff --git a/packages/protocol/test-sol/governance/network/Proposal.t.sol b/packages/protocol/test-sol/unit/governance/network/Proposal.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/network/Proposal.t.sol rename to packages/protocol/test-sol/unit/governance/network/Proposal.t.sol diff --git a/packages/protocol/test-sol/governance/validators/DoubleSigningSlasher.t.sol b/packages/protocol/test-sol/unit/governance/validators/DoubleSigningSlasher.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/validators/DoubleSigningSlasher.t.sol rename to packages/protocol/test-sol/unit/governance/validators/DoubleSigningSlasher.t.sol diff --git a/packages/protocol/test-sol/governance/validators/DowntimeSlasher.t.sol b/packages/protocol/test-sol/unit/governance/validators/DowntimeSlasher.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/validators/DowntimeSlasher.t.sol rename to packages/protocol/test-sol/unit/governance/validators/DowntimeSlasher.t.sol diff --git a/packages/protocol/test-sol/governance/validators/IntegerSortedLinkedListMock-8.sol b/packages/protocol/test-sol/unit/governance/validators/IntegerSortedLinkedListMock-8.sol similarity index 100% rename from packages/protocol/test-sol/governance/validators/IntegerSortedLinkedListMock-8.sol rename to packages/protocol/test-sol/unit/governance/validators/IntegerSortedLinkedListMock-8.sol diff --git a/packages/protocol/test-sol/governance/validators/IntergerSortedLinkedList-8.t.sol b/packages/protocol/test-sol/unit/governance/validators/IntergerSortedLinkedList-8.t.sol similarity index 99% rename from packages/protocol/test-sol/governance/validators/IntergerSortedLinkedList-8.t.sol rename to packages/protocol/test-sol/unit/governance/validators/IntergerSortedLinkedList-8.t.sol index f4280ce73af..e5b482663c9 100644 --- a/packages/protocol/test-sol/governance/validators/IntergerSortedLinkedList-8.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/IntergerSortedLinkedList-8.t.sol @@ -7,7 +7,7 @@ import { CommonBase } from "forge-std-8/Base.sol"; import { StdCheats } from "forge-std-8/StdCheats.sol"; import { StdUtils } from "forge-std-8/StdUtils.sol"; -import "@test-sol/governance/validators/IntegerSortedLinkedListMock-8.sol"; +import "@test-sol/unit/governance/validators/IntegerSortedLinkedListMock-8.sol"; contract IntegerSortedLinkedListTest8 is Test { IntegerSortedLinkedListMock public integerSortedLinkedListMock; diff --git a/packages/protocol/test-sol/governance/validators/IntergerSortedLinkedList.t.sol b/packages/protocol/test-sol/unit/governance/validators/IntergerSortedLinkedList.t.sol similarity index 100% rename from packages/protocol/test-sol/governance/validators/IntergerSortedLinkedList.t.sol rename to packages/protocol/test-sol/unit/governance/validators/IntergerSortedLinkedList.t.sol diff --git a/packages/protocol/test-sol/governance/validators/Validators.t.sol b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol similarity index 99% rename from packages/protocol/test-sol/governance/validators/Validators.t.sol rename to packages/protocol/test-sol/unit/governance/validators/Validators.t.sol index 5b8ff4d2524..737cc83eef1 100644 --- a/packages/protocol/test-sol/governance/validators/Validators.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol @@ -15,7 +15,7 @@ import "@celo-contracts/governance/LockedGold.sol"; import "@celo-contracts/stability/test/MockStableToken.sol"; import "@celo-contracts/governance/test/MockElection.sol"; import "@celo-contracts/governance/test/MockLockedGold.sol"; -import "./mocks/ValidatorsMockTunnel.sol"; +import "@test-sol/unit/governance/validators/mocks/ValidatorsMockTunnel.sol"; import "@celo-contracts/governance/test/ValidatorsMock.sol"; import "@test-sol/constants.sol"; diff --git a/packages/protocol/test-sol/governance/validators/mocks/ValidatorsMockTunnel.sol b/packages/protocol/test-sol/unit/governance/validators/mocks/ValidatorsMockTunnel.sol similarity index 100% rename from packages/protocol/test-sol/governance/validators/mocks/ValidatorsMockTunnel.sol rename to packages/protocol/test-sol/unit/governance/validators/mocks/ValidatorsMockTunnel.sol diff --git a/packages/protocol/test-sol/governance/voting/LockedGold.t.sol b/packages/protocol/test-sol/unit/governance/voting/LockedGold.t.sol similarity index 99% rename from packages/protocol/test-sol/governance/voting/LockedGold.t.sol rename to packages/protocol/test-sol/unit/governance/voting/LockedGold.t.sol index 6c6d27525f9..6d7347ec6c5 100644 --- a/packages/protocol/test-sol/governance/voting/LockedGold.t.sol +++ b/packages/protocol/test-sol/unit/governance/voting/LockedGold.t.sol @@ -7,7 +7,7 @@ import "celo-foundry/Test.sol"; import "@celo-contracts/common/FixidityLib.sol"; import "@celo-contracts/common/Registry.sol"; import "@celo-contracts/common/Accounts.sol"; -import "@test-sol/common/GoldTokenMock.sol"; +import "@test-sol/unit/common/GoldTokenMock.sol"; import "@celo-contracts/governance/LockedGold.sol"; import "@celo-contracts/governance/ReleaseGold.sol"; import "@celo-contracts/governance/Election.sol"; diff --git a/packages/protocol/test-sol/governance/voting/ReleaseGold.t.sol b/packages/protocol/test-sol/unit/governance/voting/ReleaseGold.t.sol similarity index 99% rename from packages/protocol/test-sol/governance/voting/ReleaseGold.t.sol rename to packages/protocol/test-sol/unit/governance/voting/ReleaseGold.t.sol index bd76502b832..641afdd0124 100644 --- a/packages/protocol/test-sol/governance/voting/ReleaseGold.t.sol +++ b/packages/protocol/test-sol/unit/governance/voting/ReleaseGold.t.sol @@ -3,23 +3,23 @@ pragma solidity ^0.5.13; pragma experimental ABIEncoderV2; import "celo-foundry/Test.sol"; -import "../../../contracts/identity/Escrow.sol"; -import "../../../contracts/identity/FederatedAttestations.sol"; -import "../../../contracts/identity/test/MockAttestations.sol"; -import "../../../contracts/identity/test/MockERC20Token.sol"; -import "../../../contracts/common/FixidityLib.sol"; - -import "../../../contracts/common/Registry.sol"; -import "../../../contracts/common/Accounts.sol"; -import "../../../contracts/common/Freezer.sol"; -import "../../../contracts/common/GoldToken.sol"; -import "../../../contracts/governance/LockedGold.sol"; -import "../../../contracts/governance/ReleaseGold.sol"; +import "@celo-contracts/identity/Escrow.sol"; +import "@celo-contracts/identity/FederatedAttestations.sol"; +import "@celo-contracts/identity/test/MockAttestations.sol"; +import "@celo-contracts/identity/test/MockERC20Token.sol"; +import "@celo-contracts/common/FixidityLib.sol"; + +import "@celo-contracts/common/Registry.sol"; +import "@celo-contracts/common/Accounts.sol"; +import "@celo-contracts/common/Freezer.sol"; +import "@celo-contracts/common/GoldToken.sol"; +import "@celo-contracts/governance/LockedGold.sol"; +import "@celo-contracts/governance/ReleaseGold.sol"; import "./mocks/ReleaseGoldMockTunnel.sol"; -import "../../../contracts/stability/test/MockStableToken.sol"; -import "../../../contracts/governance/test/MockElection.sol"; -import "../../../contracts/governance/test/MockGovernance.sol"; -import "../../../contracts/governance/test/MockValidators.sol"; +import "@celo-contracts/stability/test/MockStableToken.sol"; +import "@celo-contracts/governance/test/MockElection.sol"; +import "@celo-contracts/governance/test/MockGovernance.sol"; +import "@celo-contracts/governance/test/MockValidators.sol"; import "@test-sol/utils/ECDSAHelper.sol"; contract ReleaseGoldTest is Test, ECDSAHelper { diff --git a/packages/protocol/test-sol/governance/voting/mocks/ReleaseGoldMockTunnel.sol b/packages/protocol/test-sol/unit/governance/voting/mocks/ReleaseGoldMockTunnel.sol similarity index 97% rename from packages/protocol/test-sol/governance/voting/mocks/ReleaseGoldMockTunnel.sol rename to packages/protocol/test-sol/unit/governance/voting/mocks/ReleaseGoldMockTunnel.sol index 161b377fcb0..c1f4c358bfb 100644 --- a/packages/protocol/test-sol/governance/voting/mocks/ReleaseGoldMockTunnel.sol +++ b/packages/protocol/test-sol/unit/governance/voting/mocks/ReleaseGoldMockTunnel.sol @@ -2,7 +2,7 @@ pragma solidity ^0.5.13; pragma experimental ABIEncoderV2; -import "../../../../contracts/governance/ReleaseGold.sol"; +import "@celo-contracts/governance/ReleaseGold.sol"; import { Test as ForgeTest } from "forge-std/Test.sol"; contract ReleaseGoldMockTunnel is ForgeTest { diff --git a/packages/protocol/test-sol/identity/Attestations.t.sol b/packages/protocol/test-sol/unit/identity/Attestations.t.sol similarity index 100% rename from packages/protocol/test-sol/identity/Attestations.t.sol rename to packages/protocol/test-sol/unit/identity/Attestations.t.sol diff --git a/packages/protocol/test-sol/identity/Escrow.t.sol b/packages/protocol/test-sol/unit/identity/Escrow.t.sol similarity index 100% rename from packages/protocol/test-sol/identity/Escrow.t.sol rename to packages/protocol/test-sol/unit/identity/Escrow.t.sol diff --git a/packages/protocol/test-sol/identity/FederatedAttestations.t.sol b/packages/protocol/test-sol/unit/identity/FederatedAttestations.t.sol similarity index 100% rename from packages/protocol/test-sol/identity/FederatedAttestations.t.sol rename to packages/protocol/test-sol/unit/identity/FederatedAttestations.t.sol diff --git a/packages/protocol/test-sol/identity/IdentityProxy.t.sol b/packages/protocol/test-sol/unit/identity/IdentityProxy.t.sol similarity index 100% rename from packages/protocol/test-sol/identity/IdentityProxy.t.sol rename to packages/protocol/test-sol/unit/identity/IdentityProxy.t.sol diff --git a/packages/protocol/test-sol/identity/IdentityProxyHub.t.sol b/packages/protocol/test-sol/unit/identity/IdentityProxyHub.t.sol similarity index 100% rename from packages/protocol/test-sol/identity/IdentityProxyHub.t.sol rename to packages/protocol/test-sol/unit/identity/IdentityProxyHub.t.sol diff --git a/packages/protocol/test-sol/identity/OdisPayments.t.sol b/packages/protocol/test-sol/unit/identity/OdisPayments.t.sol similarity index 98% rename from packages/protocol/test-sol/identity/OdisPayments.t.sol rename to packages/protocol/test-sol/unit/identity/OdisPayments.t.sol index dbb730802ad..f69c044187e 100644 --- a/packages/protocol/test-sol/identity/OdisPayments.t.sol +++ b/packages/protocol/test-sol/unit/identity/OdisPayments.t.sol @@ -4,7 +4,7 @@ pragma experimental ABIEncoderV2; import "celo-foundry/Test.sol"; import "@celo-contracts/identity/OdisPayments.sol"; -import { StableToken } from "../../lib/mento-core/contracts/StableToken.sol"; +import { StableToken } from "@mento-core/contracts/StableToken.sol"; import "@celo-contracts/common/Registry.sol"; import "@celo-contracts/common/Freezer.sol"; diff --git a/packages/protocol/test-sol/identity/Random.t.sol b/packages/protocol/test-sol/unit/identity/Random.t.sol similarity index 100% rename from packages/protocol/test-sol/identity/Random.t.sol rename to packages/protocol/test-sol/unit/identity/Random.t.sol diff --git a/packages/protocol/test-sol/stability/FeeCurrencyAdapter.t.sol b/packages/protocol/test-sol/unit/stability/FeeCurrencyAdapter.t.sol similarity index 99% rename from packages/protocol/test-sol/stability/FeeCurrencyAdapter.t.sol rename to packages/protocol/test-sol/unit/stability/FeeCurrencyAdapter.t.sol index e0d6cb69ff6..13f13bd6d97 100644 --- a/packages/protocol/test-sol/stability/FeeCurrencyAdapter.t.sol +++ b/packages/protocol/test-sol/unit/stability/FeeCurrencyAdapter.t.sol @@ -3,8 +3,8 @@ pragma solidity >=0.8.7 <=0.8.20; import "celo-foundry-8/Test.sol"; -import "../../contracts/common/FixidityLib.sol"; -import "../../contracts/common/interfaces/IRegistry.sol"; +import "@celo-contracts/common/FixidityLib.sol"; +import "@celo-contracts/common/interfaces/IRegistry.sol"; // Contract to test import "@celo-contracts-8/stability/CeloFeeCurrencyAdapterOwnable.sol"; diff --git a/packages/protocol/test-sol/stability/SortedOracles.mento.t.sol b/packages/protocol/test-sol/unit/stability/SortedOracles.mento.t.sol similarity index 99% rename from packages/protocol/test-sol/stability/SortedOracles.mento.t.sol rename to packages/protocol/test-sol/unit/stability/SortedOracles.mento.t.sol index 25e26859b4e..9f6ad19e9c8 100644 --- a/packages/protocol/test-sol/stability/SortedOracles.mento.t.sol +++ b/packages/protocol/test-sol/unit/stability/SortedOracles.mento.t.sol @@ -8,8 +8,8 @@ import { Test, console2 as console } from "celo-foundry/Test.sol"; import { SortedLinkedListWithMedian } from "contracts/common/linkedlists/SortedLinkedListWithMedian.sol"; import { FixidityLib } from "contracts/common/FixidityLib.sol"; -import { IBreakerBox } from "../../contracts/stability/interfaces/IBreakerBox.sol"; -import { SortedOracles } from "../../contracts/stability/SortedOracles.sol"; +import { IBreakerBox } from "@celo-contracts/stability/interfaces/IBreakerBox.sol"; +import { SortedOracles } from "@celo-contracts/stability/SortedOracles.sol"; contract MockBreakerBox is IBreakerBox { uint256 public tradingMode; diff --git a/packages/protocol/test-sol/stability/SortedOracles.t.sol b/packages/protocol/test-sol/unit/stability/SortedOracles.t.sol similarity index 99% rename from packages/protocol/test-sol/stability/SortedOracles.t.sol rename to packages/protocol/test-sol/unit/stability/SortedOracles.t.sol index 003cb5e3b30..8a5e6b48def 100644 --- a/packages/protocol/test-sol/stability/SortedOracles.t.sol +++ b/packages/protocol/test-sol/unit/stability/SortedOracles.t.sol @@ -3,11 +3,11 @@ pragma solidity ^0.5.13; pragma experimental ABIEncoderV2; import { Test } from "celo-foundry/Test.sol"; -import { SortedOracles } from "../../contracts/stability/SortedOracles.sol"; +import { SortedOracles } from "@celo-contracts/stability/SortedOracles.sol"; import "@celo-contracts/common/FixidityLib.sol"; import "@celo-contracts/common/linkedlists/AddressSortedLinkedListWithMedian.sol"; import "@celo-contracts/common/linkedlists/SortedLinkedListWithMedian.sol"; -import { Constants } from "../constants.sol"; +import { Constants } from "@test-sol/constants.sol"; import "forge-std/console.sol"; contract SortedOraclesTest is Test, Constants { diff --git a/packages/protocol/test-sol/voting/Election.t.sol b/packages/protocol/test-sol/unit/voting/Election.t.sol similarity index 99% rename from packages/protocol/test-sol/voting/Election.t.sol rename to packages/protocol/test-sol/unit/voting/Election.t.sol index 5f9086b8c72..86fe2a80076 100644 --- a/packages/protocol/test-sol/voting/Election.t.sol +++ b/packages/protocol/test-sol/unit/voting/Election.t.sol @@ -11,8 +11,8 @@ import "@celo-contracts/common/Accounts.sol"; import "@celo-contracts/common/linkedlists/AddressSortedLinkedList.sol"; import "@celo-contracts/identity/test/MockRandom.sol"; import "@celo-contracts/common/Freezer.sol"; -import { Constants } from "../constants.sol"; -import "../utils.sol"; +import { Constants } from "@test-sol/constants.sol"; +import "@test-sol/utils.sol"; contract ElectionMock is Election(true) { function distributeEpochRewards( From 714c966adb5cc6938bd493eaca948f0744e38c6d Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 11:18:30 -0400 Subject: [PATCH 08/18] lint: function order --- .../test-sol/governance/voting/Election.t.sol | 296 +++++++++--------- 1 file changed, 148 insertions(+), 148 deletions(-) diff --git a/packages/protocol/test-sol/governance/voting/Election.t.sol b/packages/protocol/test-sol/governance/voting/Election.t.sol index 274caf6c504..ff958f5a5a9 100644 --- a/packages/protocol/test-sol/governance/voting/Election.t.sol +++ b/packages/protocol/test-sol/governance/voting/Election.t.sol @@ -30,32 +30,6 @@ contract ElectionTest is Utils, Constants { address constant proxyAdminAddress = 0x4200000000000000000000000000000000000018; - event ElectableValidatorsSet(uint256 min, uint256 max); - event MaxNumGroupsVotedForSet(uint256 maxNumGroupsVotedFor); - event ElectabilityThresholdSet(uint256 electabilityThreshold); - event AllowedToVoteOverMaxNumberOfGroups(address indexed account, bool flag); - event ValidatorGroupMarkedEligible(address indexed group); - event ValidatorGroupMarkedIneligible(address indexed group); - event ValidatorGroupVoteCast(address indexed account, address indexed group, uint256 value); - event ValidatorGroupVoteActivated( - address indexed account, - address indexed group, - uint256 value, - uint256 units - ); - event ValidatorGroupPendingVoteRevoked( - address indexed account, - address indexed group, - uint256 value - ); - event ValidatorGroupActiveVoteRevoked( - address indexed account, - address indexed group, - uint256 value, - uint256 units - ); - event EpochRewardsDistributedToVoters(address indexed group, uint256 value); - Accounts accounts; ElectionMock election; Freezer freezer; @@ -85,6 +59,32 @@ contract ElectionTest is Utils, Constants { address[] accountsArray; + event ElectableValidatorsSet(uint256 min, uint256 max); + event MaxNumGroupsVotedForSet(uint256 maxNumGroupsVotedFor); + event ElectabilityThresholdSet(uint256 electabilityThreshold); + event AllowedToVoteOverMaxNumberOfGroups(address indexed account, bool flag); + event ValidatorGroupMarkedEligible(address indexed group); + event ValidatorGroupMarkedIneligible(address indexed group); + event ValidatorGroupVoteCast(address indexed account, address indexed group, uint256 value); + event ValidatorGroupVoteActivated( + address indexed account, + address indexed group, + uint256 value, + uint256 units + ); + event ValidatorGroupPendingVoteRevoked( + address indexed account, + address indexed group, + uint256 value + ); + event ValidatorGroupActiveVoteRevoked( + address indexed account, + address indexed group, + uint256 value, + uint256 units + ); + event EpochRewardsDistributedToVoters(address indexed group, uint256 value); + function createAccount(address account) public { vm.prank(account); accounts.createAccount(); @@ -1111,6 +1111,9 @@ contract ElectionTest_Activate is ElectionTest { address group = account1; uint256 value = 1000; + address voter2 = account2; + uint256 value2 = 573; + function setUp() public { super.setUp(); @@ -1182,9 +1185,6 @@ contract ElectionTest_Activate is ElectionTest { election.activate(group); } - address voter2 = account2; - uint256 value2 = 573; - function WhenAnotherVoterActivatesVotes() public { WhenEpochBoundaryHasPassed(); lockedGold.incrementNonvotingAccountBalance(voter2, value2); @@ -1267,6 +1267,9 @@ contract ElectionTest_Activate_L2 is ElectionTest { address group = account1; uint256 value = 1000; + address voter2 = account2; + uint256 value2 = 573; + function setUp() public { super.setUp(); _whenL2(); @@ -1338,9 +1341,6 @@ contract ElectionTest_Activate_L2 is ElectionTest { election.activate(group); } - address voter2 = account2; - uint256 value2 = 573; - function WhenAnotherVoterActivatesVotes() public { WhenEpochBoundaryHasPassed(); lockedGold.incrementNonvotingAccountBalance(voter2, value2); @@ -1423,6 +1423,9 @@ contract ElectionTest_ActivateForAccount is ElectionTest { address group = account1; uint256 value = 1000; + address voter2 = account2; + uint256 value2 = 573; + function setUp() public { super.setUp(); @@ -1494,9 +1497,6 @@ contract ElectionTest_ActivateForAccount is ElectionTest { election.activate(group); } - address voter2 = account2; - uint256 value2 = 573; - function WhenAnotherVoterActivatesVotes() public { WhenEpochBoundaryHasPassed(); lockedGold.incrementNonvotingAccountBalance(voter2, value2); @@ -1578,6 +1578,9 @@ contract ElectionTest_ActivateForAccount_L2 is ElectionTest { address group = account1; uint256 value = 1000; + address voter2 = account2; + uint256 value2 = 573; + function setUp() public { super.setUp(); _whenL2(); @@ -1649,9 +1652,6 @@ contract ElectionTest_ActivateForAccount_L2 is ElectionTest { election.activate(group); } - address voter2 = account2; - uint256 value2 = 573; - function WhenAnotherVoterActivatesVotes() public { WhenEpochBoundaryHasPassed(); lockedGold.incrementNonvotingAccountBalance(voter2, value2); @@ -2159,6 +2159,11 @@ contract ElectionTest_RevokeActive is ElectionTest { } contract ElectionTest_ElectionValidatorSigners is ElectionTest { + struct MemberWithVotes { + address member; + uint256 votes; + } + address group1 = address(this); address group2 = account1; address group3 = account2; @@ -2192,11 +2197,6 @@ contract ElectionTest_ElectionValidatorSigners is ElectionTest { uint256 totalLockedGold = voter1Weight + voter2Weight + voter3Weight; - struct MemberWithVotes { - address member; - uint256 votes; - } - mapping(address => uint256) votesConsideredForElection; MemberWithVotes[] membersWithVotes; @@ -2219,37 +2219,6 @@ contract ElectionTest_ElectionValidatorSigners is ElectionTest { random.addTestRandomness(block.number + 1, hash); } - // Helper function to sort an array of uint256 - function sort(uint256[] memory data) internal pure returns (uint256[] memory) { - uint256 length = data.length; - for (uint256 i = 0; i < length; i++) { - for (uint256 j = i + 1; j < length; j++) { - if (data[i] > data[j]) { - uint256 temp = data[i]; - data[i] = data[j]; - data[j] = temp; - } - } - } - return data; - } - - function sortMembersWithVotesDesc( - MemberWithVotes[] memory data - ) internal pure returns (MemberWithVotes[] memory) { - uint256 length = data.length; - for (uint256 i = 0; i < length; i++) { - for (uint256 j = i + 1; j < length; j++) { - if (data[i].votes < data[j].votes) { - MemberWithVotes memory temp = data[i]; - data[i] = data[j]; - data[j] = temp; - } - } - } - return data; - } - function WhenThereIsALargeNumberOfGroups() public { lockedGold.setTotalLockedGold(1e25); validators.setNumRegisteredValidators(400); @@ -2412,6 +2381,37 @@ contract ElectionTest_ElectionValidatorSigners is ElectionTest { vm.expectRevert("Not enough elected validators"); election.electValidatorSigners(); } + + // Helper function to sort an array of uint256 + function sort(uint256[] memory data) internal pure returns (uint256[] memory) { + uint256 length = data.length; + for (uint256 i = 0; i < length; i++) { + for (uint256 j = i + 1; j < length; j++) { + if (data[i] > data[j]) { + uint256 temp = data[i]; + data[i] = data[j]; + data[j] = temp; + } + } + } + return data; + } + + function sortMembersWithVotesDesc( + MemberWithVotes[] memory data + ) internal pure returns (MemberWithVotes[] memory) { + uint256 length = data.length; + for (uint256 i = 0; i < length; i++) { + for (uint256 j = i + 1; j < length; j++) { + if (data[i].votes < data[j].votes) { + MemberWithVotes memory temp = data[i]; + data[i] = data[j]; + data[j] = temp; + } + } + } + return data; + } } contract ElectionTest_GetGroupEpochRewards is ElectionTest { @@ -2422,6 +2422,12 @@ contract ElectionTest_GetGroupEpochRewards is ElectionTest { uint256 voteValue2 = 1000000000; uint256 totalRewardValue = 3000000000; + uint256 expectedGroup1EpochRewards = + FixidityLib + .newFixedFraction(voteValue1, voteValue1 + voteValue2) + .multiply(FixidityLib.newFixed(totalRewardValue)) + .fromFixed(); + function setUp() public { super.setUp(); @@ -2501,12 +2507,6 @@ contract ElectionTest_GetGroupEpochRewards is ElectionTest { assertEq(election.getGroupEpochRewards(group2, totalRewardValue, uptimes), 0); } - uint256 expectedGroup1EpochRewards = - FixidityLib - .newFixedFraction(voteValue1, voteValue1 + voteValue2) - .multiply(FixidityLib.newFixed(totalRewardValue)) - .fromFixed(); - function test_ShouldReturnProportionalRewardValueForOtherGroup_WhenOneGroupDoesNotMeetLockedGoldRequirements_WhenTwoGroupsHaveActiveVotes() public { @@ -2541,6 +2541,13 @@ contract ElectionTest_DistributeEpochRewards is ElectionTest { uint256 rewardValue = 1000000; uint256 rewardValue2 = 10000000; + uint256 expectedGroupTotalActiveVotes = voteValue + voteValue2 / 2 + rewardValue; + uint256 expectedVoterActiveVotesForGroup = + FixidityLib.newFixedFraction(expectedGroupTotalActiveVotes * 2, 3).fromFixed(); + uint256 expectedVoter2ActiveVotesForGroup = + FixidityLib.newFixedFraction(expectedGroupTotalActiveVotes, 3).fromFixed(); + uint256 expectedVoter2ActiveVotesForGroup2 = voteValue / 2 + rewardValue2; + function setUp() public { super.setUp(); @@ -2598,13 +2605,6 @@ contract ElectionTest_DistributeEpochRewards is ElectionTest { assertEq(election.getTotalVotes(), voteValue + rewardValue); } - uint256 expectedGroupTotalActiveVotes = voteValue + voteValue2 / 2 + rewardValue; - uint256 expectedVoterActiveVotesForGroup = - FixidityLib.newFixedFraction(expectedGroupTotalActiveVotes * 2, 3).fromFixed(); - uint256 expectedVoter2ActiveVotesForGroup = - FixidityLib.newFixedFraction(expectedGroupTotalActiveVotes, 3).fromFixed(); - uint256 expectedVoter2ActiveVotesForGroup2 = voteValue / 2 + rewardValue2; - function WhenThereAreTwoGroupsWithActiveVotes() public { registry.setAddressFor("Validators", address(this)); election.markGroupEligible(group2, address(0), group); @@ -2708,6 +2708,15 @@ contract ElectionTest_ForceDecrementVotes is ElectionTest { uint256 slashedValue = value; uint256 remaining = value - slashedValue; + uint256 totalRemaining; + uint256 group1Remaining; + uint256 group2TotalRemaining; + uint256 group2PendingRemaining; + uint256 group2ActiveRemaining; + + uint256 group1RemainingActiveVotes; + address[] initialOrdering; + function setUp() public { super.setUp(); } @@ -2952,12 +2961,6 @@ contract ElectionTest_ForceDecrementVotes is ElectionTest { assertEq(election.getActiveVotesForGroupByAccount(group, voter), remaining); } - uint256 totalRemaining; - uint256 group1Remaining; - uint256 group2TotalRemaining; - uint256 group2PendingRemaining; - uint256 group2ActiveRemaining; - function WhenWeSlashAllOfGroup1VotesAndSomeOfGroup2__WhenWeSlash1MoreVoteThanGroup1PendingVoteTotal_WhenAccountHasVotedForMoreThanOneGroupInequally() public { @@ -2994,9 +2997,6 @@ contract ElectionTest_ForceDecrementVotes is ElectionTest { assertEq(election.getActiveVotesForGroupByAccount(group2, voter), group2ActiveRemaining); } - uint256 group1RemainingActiveVotes; - address[] initialOrdering; - function WhenSlashAffectsElectionOrder() public { WhenAccountHasVotedForMoreThanOneGroupInequally(); @@ -3112,12 +3112,6 @@ contract ElectionTest_ForceDecrementVotes is ElectionTest { } contract ElectionTest_ConsistencyChecks is ElectionTest { - address voter = address(this); - address group = account2; - uint256 rewardValue2 = 10000000; - - AccountStruct[] _accounts; - struct AccountStruct { address account; uint256 active; @@ -3132,6 +3126,12 @@ contract ElectionTest_ConsistencyChecks is ElectionTest { RevokeActive } + address voter = address(this); + address group = account2; + uint256 rewardValue2 = 10000000; + + AccountStruct[] _accounts; + function setUp() public { super.setUp(); @@ -3159,52 +3159,6 @@ contract ElectionTest_ConsistencyChecks is ElectionTest { } } - function makeRandomAction(AccountStruct storage account, uint256 salt) internal { - VoteActionType[] memory actions = new VoteActionType[](4); - uint256 actionCount = 0; - - if (account.nonVoting > 0) { - actions[actionCount++] = VoteActionType.Vote; - } - if (election.hasActivatablePendingVotes(account.account, group)) { - // Assuming this is a view function - actions[actionCount++] = VoteActionType.Activate; - } - if (account.pending > 0) { - actions[actionCount++] = VoteActionType.RevokePending; - } - if (account.active > 0) { - actions[actionCount++] = VoteActionType.RevokeActive; - } - - VoteActionType action = actions[generatePRN(0, actionCount - 1, uint256(account.account))]; - uint256 value; - - vm.startPrank(account.account); - if (action == VoteActionType.Vote) { - value = generatePRN(0, account.nonVoting, uint256(account.account) + salt); - election.vote(group, value, address(0), address(0)); - account.nonVoting -= value; - account.pending += value; - } else if (action == VoteActionType.Activate) { - value = account.pending; - election.activate(group); - account.pending -= value; - account.active += value; - } else if (action == VoteActionType.RevokePending) { - value = generatePRN(0, account.pending, uint256(account.account) + salt); - election.revokePending(group, value, address(0), address(0), 0); - account.pending -= value; - account.nonVoting += value; - } else if (action == VoteActionType.RevokeActive) { - value = generatePRN(0, account.active, uint256(account.account) + salt); - election.revokeActive(group, value, address(0), address(0), 0); - account.active -= value; - account.nonVoting += value; - } - vm.stopPrank(); - } - function checkVoterInvariants(AccountStruct memory account, uint256 delta) public { assertAlmostEqual( election.getPendingVotesForGroupByAccount(group, account.account), @@ -3329,6 +3283,52 @@ contract ElectionTest_ConsistencyChecks is ElectionTest { } revokeAllAndCheckInvariants(100); } + + function makeRandomAction(AccountStruct storage account, uint256 salt) internal { + VoteActionType[] memory actions = new VoteActionType[](4); + uint256 actionCount = 0; + + if (account.nonVoting > 0) { + actions[actionCount++] = VoteActionType.Vote; + } + if (election.hasActivatablePendingVotes(account.account, group)) { + // Assuming this is a view function + actions[actionCount++] = VoteActionType.Activate; + } + if (account.pending > 0) { + actions[actionCount++] = VoteActionType.RevokePending; + } + if (account.active > 0) { + actions[actionCount++] = VoteActionType.RevokeActive; + } + + VoteActionType action = actions[generatePRN(0, actionCount - 1, uint256(account.account))]; + uint256 value; + + vm.startPrank(account.account); + if (action == VoteActionType.Vote) { + value = generatePRN(0, account.nonVoting, uint256(account.account) + salt); + election.vote(group, value, address(0), address(0)); + account.nonVoting -= value; + account.pending += value; + } else if (action == VoteActionType.Activate) { + value = account.pending; + election.activate(group); + account.pending -= value; + account.active += value; + } else if (action == VoteActionType.RevokePending) { + value = generatePRN(0, account.pending, uint256(account.account) + salt); + election.revokePending(group, value, address(0), address(0), 0); + account.pending -= value; + account.nonVoting += value; + } else if (action == VoteActionType.RevokeActive) { + value = generatePRN(0, account.active, uint256(account.account) + salt); + election.revokeActive(group, value, address(0), address(0), 0); + account.active -= value; + account.nonVoting += value; + } + vm.stopPrank(); + } } contract ElectionTest_HasActivatablePendingVotes is ElectionTest { From 4a167fb4c3bdd2b240c63304b0fe979f70616f25 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 11:31:27 -0400 Subject: [PATCH 09/18] Soloseng/CHORE-update-workflow (#11029) * update workflow to run on push to release branches * ++ mintgoldschedule to migration test constants --- .github/workflows/protocol-devchain-anvil.yml | 1 + packages/protocol/migrations_sol/constants.sol | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/protocol-devchain-anvil.yml b/.github/workflows/protocol-devchain-anvil.yml index 14af7bfa69b..891639cc365 100644 --- a/.github/workflows/protocol-devchain-anvil.yml +++ b/.github/workflows/protocol-devchain-anvil.yml @@ -3,6 +3,7 @@ on: push: branches: - master + - 'release/**' env: # Increment these to force cache rebuilding diff --git a/packages/protocol/migrations_sol/constants.sol b/packages/protocol/migrations_sol/constants.sol index 81bee276509..dcdec2c2d3a 100644 --- a/packages/protocol/migrations_sol/constants.sol +++ b/packages/protocol/migrations_sol/constants.sol @@ -2,7 +2,7 @@ pragma solidity >=0.8.7 <0.8.20; contract Constants { // List of contracts that are expected to be in Registry.sol - string[23] contractsInRegistry = [ + string[24] contractsInRegistry = [ "Accounts", "BlockchainParameters", "DoubleSigningSlasher", @@ -19,6 +19,7 @@ contract Constants { "Governance", "GovernanceSlasher", "LockedGold", + "MintGoldSchedule", "OdisPayments", "Random", "Registry", From 8fdcad325ccff465fa6fc95292abe2986e03d812 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 15:48:49 -0400 Subject: [PATCH 10/18] initial validators contract review --- .../contracts/governance/Validators.sol | 65 +++++++++++++++---- .../governance/validators/Validators.t.sol | 64 ++++++++++++++++-- 2 files changed, 111 insertions(+), 18 deletions(-) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 6b3a54be150..1811e39717a 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -205,6 +205,8 @@ contract Validators is setDowntimeGracePeriod(_downtimeGracePeriod); } + // XXX: Unchanged, deals with rewards distribution. + // XXX(soloseng): should this set uptime to 1 by default on L2? /** * @notice Updates a validator's score based on its uptime for the epoch. * @param signer The validator signer of the validator account whose score needs updating. @@ -216,6 +218,7 @@ contract Validators is _updateValidatorScoreFromSigner(signer, uptime); } + // XXX: Unchanged, deals with rewards distribution. /** * @notice Distributes epoch payments to the account associated with `signer` and its group. * @param signer The validator signer of the account to distribute the epoch payment to. @@ -231,6 +234,7 @@ contract Validators is return _distributeEpochPaymentsFromSigner(signer, maxPayment); } + // XXX: Unchanged, new registrations require PoP. /** * @notice Registers a validator unaffiliated with any validator group. * @param ecdsaPublicKey The ECDSA public key that the validator is using for consensus, should @@ -303,6 +307,7 @@ contract Validators is return true; } + // XXX: Unchanged, deals with Validator/group registration. /** * @notice Affiliates a validator with a group, allowing it to be added as a member. * @param group The validator group with which to affiliate. @@ -339,6 +344,7 @@ contract Validators is return true; } + // XXX: Unchanged, new registrations require PoP. /** * @notice Updates a validator's BLS key. * @param blsPublicKey The BLS public key that the validator is using for consensus, should pass @@ -362,6 +368,7 @@ contract Validators is return true; } + // XXX: Unchanged, used only during registration. /** * @notice Updates a validator's ECDSA key. * @param account The address under which the validator is registered. @@ -410,6 +417,7 @@ contract Validators is return true; } + // XXX: Unchanged, require PoP precompile. /** * @notice Updates a validator's ECDSA and BLS keys. * @param account The address under which the validator is registered. @@ -442,6 +450,7 @@ contract Validators is return true; } + // XXX: Unchanged, deals with Validator/group registration. /** * @notice Registers a validator group with no member validators. * @param commission Fixidity representation of the commission this group receives on epoch @@ -468,6 +477,7 @@ contract Validators is return true; } + // XXX: Unchanged, deals with Validator/group registration. /** * @notice Adds a member to the end of a validator group's list of members. * @param validator The validator to add to the group @@ -482,6 +492,7 @@ contract Validators is return _addMember(account, validator, address(0), address(0)); } + // XXX: Unchanged, deals with Validator/group registration. /** * @notice Adds the first member to a group's list of members and marks it eligible for election. * @param validator The validator to add to the group @@ -529,7 +540,6 @@ contract Validators is address lesserMember, address greaterMember ) external nonReentrant returns (bool) { - allowOnlyL1(); address account = getAccounts().validatorSignerToAccount(msg.sender); require(isValidatorGroup(account), "Not a group"); require(isValidator(validator), "Not a validator"); @@ -540,6 +550,7 @@ contract Validators is return true; } + //XXX unchanged as rewards are not implemented. /** * @notice Queues an update to a validator group's commission. * If there was a previously scheduled update, that is overwritten. @@ -558,6 +569,8 @@ contract Validators is group.nextCommissionBlock = block.number.add(commissionUpdateDelay); emit ValidatorGroupCommissionUpdateQueued(account, commission, group.nextCommissionBlock); } + + //XXX unchanged as rewards are not implemented. /** * @notice Updates a validator group's commission based on the previously queued update */ @@ -581,7 +594,6 @@ 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)) { @@ -590,6 +602,7 @@ contract Validators is } } + // XXX: rewards and Slashing not supported on L2 /** * @notice Resets a group's slashing multiplier if it has been >= the reset period since * the last time the group was slashed. @@ -606,6 +619,7 @@ contract Validators is group.slashInfo.multiplier = FixidityLib.fixed1(); } +// XXX: rewards and Slashing not supported on L2 /** * @notice Halves the group's slashing multiplier. * @param account The group being slashed. @@ -651,15 +665,27 @@ contract Validators is { require(isValidatorGroup(account), "Not a validator group"); ValidatorGroup storage group = groups[account]; - return ( - group.members.getKeys(), - group.commission.unwrap(), - group.nextCommission.unwrap(), - group.nextCommissionBlock, - group.sizeHistory, - group.slashInfo.multiplier.unwrap(), - group.slashInfo.lastSlashed - ); + if (isL2()) { + return ( + group.members.getKeys(), + group.commission.unwrap(), + group.nextCommission.unwrap(), + group.nextCommissionBlock, + group.sizeHistory, + 1, + group.slashInfo.lastSlashed + ); + } else { + return ( + group.members.getKeys(), + group.commission.unwrap(), + group.nextCommission.unwrap(), + group.nextCommissionBlock, + group.sizeHistory, + group.slashInfo.multiplier.unwrap(), + group.slashInfo.lastSlashed + ); + } } /** @@ -748,15 +774,20 @@ contract Validators is return getMembershipInLastEpoch(account); } + //XXX: should this always return 1? /** * @notice Getter for a group's slashing multiplier. * @param account The group to fetch slashing multiplier for. */ function getValidatorGroupSlashingMultiplier(address account) external view returns (uint256) { - allowOnlyL1(); require(isValidatorGroup(account), "Not a validator group"); - ValidatorGroup storage group = groups[account]; - return group.slashInfo.multiplier.unwrap(); + + if (isL2()) { + return 1; + } else { + ValidatorGroup storage group = groups[account]; + return group.slashInfo.multiplier.unwrap(); + } } /** @@ -865,6 +896,7 @@ contract Validators is emit CommissionUpdateDelaySet(delay); } + //XXX: unchanged, as the validator group constitution should not change on L2 /** * @notice Updates the maximum number of members a group can have. * @param size The maximum group size. @@ -879,6 +911,7 @@ contract Validators is return true; } + //XXX: unchanged, as the validator group constitution should not change on L2 /** * @notice Updates the number of validator group membership entries to store. * @param length The number of validator group membership entries to store. @@ -893,6 +926,7 @@ contract Validators is return true; } + //XXX: unchanged, as the validator group constitution should not change on L2 /** * @notice Updates the validator score parameters. * @param exponent The exponent used in calculating the score. @@ -961,6 +995,7 @@ contract Validators is return true; } + //XXX: unchanged, as rewards are not yet implemented. /** * @notice Sets the slashingMultiplierRestPeriod property if called by owner. * @param value New reset period for slashing multiplier. @@ -970,6 +1005,7 @@ contract Validators is slashingMultiplierResetPeriod = value; } + //XXX: unchanged, as rewards are not yet implemented. /** * @notice Sets the downtimeGracePeriod property if called by owner. * @param value New downtime grace period for calculating epoch scores. @@ -1003,6 +1039,7 @@ contract Validators is return 0; } + //XXX: unchanged, as elections are not yet implemented. /** * @notice Returns the group that `account` was a member of at the end of the last epoch. * @param account The account whose group membership should be returned. 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 737cc83eef1..5c47108fcd2 100644 --- a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol @@ -2148,15 +2148,53 @@ contract ValidatorsTest_ReorderMember is ValidatorsTest { assertEq(expectedMembersList.length, members.length); } - function test_Reverts_ReorderGroupMemberList_WhenL2() public { + function test_Emits_ValidatorGroupMemberReorderedEvent() public { + vm.expectEmit(true, true, true, true); + emit ValidatorGroupMemberReordered(group, vm.addr(1)); + + vm.prank(group); + validators.reorderMember(vm.addr(1), validator, address(0)); + } + + function test_Reverts_WhenAccountIsNotRegisteredValidatorGroup() public { + vm.expectRevert("Not a group"); + vm.prank(vm.addr(1)); + validators.reorderMember(vm.addr(1), validator, address(0)); + } + + function test_Reverts_WhenMemberNotRegisteredValidator() public { + vm.expectRevert("Not a validator"); + vm.prank(group); + validators.reorderMember(nonValidator, validator, address(0)); + } + + function test_Reverts_WhenValidatorNotMemberOfValidatorGroup() public { + vm.prank(vm.addr(1)); + validators.deaffiliate(); + + vm.expectRevert("Not a member of the group"); + vm.prank(group); + validators.reorderMember(vm.addr(1), validator, address(0)); + } +} +contract ValidatorsTest_ReorderMember_L2 is ValidatorsTest { + function setUp() public { + super.setUp(); + _registerValidatorGroupWithMembers(group, 2); _whenL2(); + } + + function test_ShouldReorderGroupMemberList() public { address[] memory expectedMembersList = new address[](2); expectedMembersList[0] = vm.addr(1); expectedMembersList[1] = validator; vm.prank(group); - vm.expectRevert("This method is no longer supported in L2."); validators.reorderMember(vm.addr(1), validator, address(0)); + (address[] memory members, , , , , , ) = validators.getValidatorGroup(group); + + assertEq(expectedMembersList, members); + assertEq(expectedMembersList.length, members.length); } function test_Emits_ValidatorGroupMemberReorderedEvent() public { @@ -3159,11 +3197,29 @@ contract ValidatorsTest_ForceDeaffiliateIfValidator is ValidatorsTest { assertEq(affiliation, address(0)); } - function test_Reverts_WhenSenderIsWhitelistedSlashingAddress_WhenL2() public { + function test_Reverts_WhenSenderNotApprovedAddress() public { + vm.expectRevert("Only registered slasher can call"); + validators.forceDeaffiliateIfValidator(validator); + } +} +contract ValidatorsTest_ForceDeaffiliateIfValidator_L2 is ValidatorsTest { + function setUp() public { + super.setUp(); + + _registerValidatorHelper(validator, validatorPk); + _registerValidatorGroupHelper(group, 1); + + vm.prank(validator); + validators.affiliate(group); _whenL2(); + lockedGold.addSlasherTest(paymentDelegatee); + } + + function test_ShouldSucceed_WhenSenderIsWhitelistedSlashingAddress() public { vm.prank(paymentDelegatee); - vm.expectRevert("This method is no longer supported in L2."); validators.forceDeaffiliateIfValidator(validator); + (, , address affiliation, , ) = validators.getValidator(validator); + assertEq(affiliation, address(0)); } function test_Reverts_WhenSenderNotApprovedAddress() public { From 743ea3d0c96da4541c0169aae7b5a4dab4a752fd Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 16:39:22 -0400 Subject: [PATCH 11/18] Remove block gas limit flag for `governance/voting` CI tests --- .github/workflows/protocol_tests.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/protocol_tests.yml b/.github/workflows/protocol_tests.yml index d7bb9d5b9e5..339aae29af8 100644 --- a/.github/workflows/protocol_tests.yml +++ b/.github/workflows/protocol_tests.yml @@ -87,8 +87,7 @@ jobs: if: success() || failure() run: | forge test -vvv \ - --match-path "test-sol/unit/governance/voting/*" \ - --block-gas-limit 50000000 + --match-path "test-sol/unit/governance/voting/*" - name: Run unit tests stability if: success() || failure() From 7f853ea68cbbd970f400249ffcd7204a6a5bbe39 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 17:08:02 -0400 Subject: [PATCH 12/18] bump version --- packages/protocol/contracts/governance/Governance.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/contracts/governance/Governance.sol b/packages/protocol/contracts/governance/Governance.sol index 7d3a50e81e8..e4860e60a7a 100644 --- a/packages/protocol/contracts/governance/Governance.sol +++ b/packages/protocol/contracts/governance/Governance.sol @@ -941,7 +941,7 @@ contract Governance is * @return Patch version of the contract. */ function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { - return (1, 4, 1, 1); + return (1, 4, 2, 0); } /** From ca9d29c8115576ff7f99839f1ddba7e80453cdce Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 17:10:13 -0400 Subject: [PATCH 13/18] ++ tests && format --- .../contracts/governance/Election.sol | 2 +- .../contracts/governance/Validators.sol | 3 +- .../governance/validators/Validators.t.sol | 194 ++++++++++-------- 3 files changed, 109 insertions(+), 90 deletions(-) diff --git a/packages/protocol/contracts/governance/Election.sol b/packages/protocol/contracts/governance/Election.sol index 75ca4dabab1..8a7633599cd 100644 --- a/packages/protocol/contracts/governance/Election.sol +++ b/packages/protocol/contracts/governance/Election.sol @@ -539,7 +539,7 @@ contract Election is address group, uint256 totalEpochRewards, uint256[] calldata uptimes - ) external view returns (uint256) { + ) external view onlyL1 returns (uint256) { IValidators validators = getValidators(); // The group must meet the balance requirements for their voters to receive epoch rewards. if (!validators.meetsAccountLockedGoldRequirements(group) || votes.active.total <= 0) { diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 1811e39717a..b8f5d8708da 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -619,7 +619,7 @@ contract Validators is group.slashInfo.multiplier = FixidityLib.fixed1(); } -// XXX: rewards and Slashing not supported on L2 + // XXX: rewards and Slashing not supported on L2 /** * @notice Halves the group's slashing multiplier. * @param account The group being slashed. @@ -1067,6 +1067,7 @@ contract Validators is * @return Fixidity representation of the epoch score between 0 and 1. */ function calculateEpochScore(uint256 uptime) public view returns (uint256) { + allowOnlyL1(); require(uptime <= FixidityLib.fixed1().unwrap(), "Uptime cannot be larger than one"); uint256 numerator; uint256 denominator; 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 5c47108fcd2..a82220093c3 100644 --- a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol @@ -27,6 +27,21 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { using FixidityLib for FixidityLib.Fraction; using SafeMath for uint256; + struct ValidatorLockedGoldRequirements { + uint256 value; + uint256 duration; + } + + struct GroupLockedGoldRequirements { + uint256 value; + uint256 duration; + } + + struct ValidatorScoreParameters { + uint256 exponent; + FixidityLib.Fraction adjustmentSpeed; + } + address constant proxyAdminAddress = 0x4200000000000000000000000000000000000018; Registry registry; @@ -68,28 +83,6 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { FixidityLib.Fraction public commission = FixidityLib.newFixedFraction(1, 100); - event AccountSlashed( - address indexed slashed, - uint256 penalty, - address indexed reporter, - uint256 reward - ); - - struct ValidatorLockedGoldRequirements { - uint256 value; - uint256 duration; - } - - struct GroupLockedGoldRequirements { - uint256 value; - uint256 duration; - } - - struct ValidatorScoreParameters { - uint256 exponent; - FixidityLib.Fraction adjustmentSpeed; - } - ValidatorLockedGoldRequirements public originalValidatorLockedGoldRequirements; GroupLockedGoldRequirements public originalGroupLockedGoldRequirements; ValidatorScoreParameters public originalValidatorScoreParameters; @@ -103,6 +96,12 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { ValidatorsMockTunnel.InitParams public initParams; ValidatorsMockTunnel.InitParams2 public initParams2; + event AccountSlashed( + address indexed slashed, + uint256 penalty, + address indexed reporter, + uint256 reward + ); event MaxGroupSizeSet(uint256 size); event CommissionUpdateDelaySet(uint256 delay); event ValidatorScoreParametersSet(uint256 exponent, uint256 adjustmentSpeed); @@ -212,6 +211,38 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { accounts.createAccount(); } + function _whenL2() public { + deployCodeTo("Registry.sol", abi.encode(false), proxyAdminAddress); + } + + function _registerValidatorGroupWithMembers(address _group, uint256 _numMembers) public { + _registerValidatorGroupHelper(_group, _numMembers); + + for (uint256 i = 0; i < _numMembers; i++) { + if (i == 0) { + _registerValidatorHelper(validator, validatorPk); + + vm.prank(validator); + validators.affiliate(group); + + vm.prank(group); + validators.addFirstMember(validator, address(0), address(0)); + } else { + uint256 _validator1Pk = i; + address _validator1 = vm.addr(_validator1Pk); + + vm.prank(_validator1); + accounts.createAccount(); + _registerValidatorHelper(_validator1, _validator1Pk); + vm.prank(_validator1); + validators.affiliate(group); + + vm.prank(group); + validators.addMember(_validator1); + } + } + } + function getParsedSignatureOfAddress( address _address, uint256 privateKey @@ -296,34 +327,6 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { validators.registerValidatorGroup(commission.unwrap()); } - function _registerValidatorGroupWithMembers(address _group, uint256 _numMembers) public { - _registerValidatorGroupHelper(_group, _numMembers); - - for (uint256 i = 0; i < _numMembers; i++) { - if (i == 0) { - _registerValidatorHelper(validator, validatorPk); - - vm.prank(validator); - validators.affiliate(group); - - vm.prank(group); - validators.addFirstMember(validator, address(0), address(0)); - } else { - uint256 _validator1Pk = i; - address _validator1 = vm.addr(_validator1Pk); - - vm.prank(_validator1); - accounts.createAccount(); - _registerValidatorHelper(_validator1, _validator1Pk); - vm.prank(_validator1); - validators.affiliate(group); - - vm.prank(group); - validators.addMember(_validator1); - } - } - } - function _removeMemberAndTimeTravel( address _group, address _validator, @@ -334,6 +337,14 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { timeTravel(_duration); } + function _calculateScore(uint256 _uptime, uint256 _gracePeriod) internal view returns (uint256) { + return + _safeExponent( + _max1(_uptime.add(_gracePeriod)), + FixidityLib.wrap(originalValidatorScoreParameters.exponent) + ); + } + function _max1(uint256 num) internal pure returns (FixidityLib.Fraction memory) { return num > FixidityLib.fixed1().unwrap() ? FixidityLib.fixed1() : FixidityLib.wrap(num); } @@ -354,18 +365,6 @@ contract ValidatorsTest is Test, Constants, Utils, ECDSAHelper { } return result.unwrap(); } - - function _calculateScore(uint256 _uptime, uint256 _gracePeriod) internal view returns (uint256) { - return - _safeExponent( - _max1(_uptime.add(_gracePeriod)), - FixidityLib.wrap(originalValidatorScoreParameters.exponent) - ); - } - - function _whenL2() public { - deployCodeTo("Registry.sol", abi.encode(false), proxyAdminAddress); - } } contract ValidatorsTest_Initialize is ValidatorsTest { @@ -822,11 +821,6 @@ contract ValidatorsTest_DeregisterValidator_WhenAccountHasNeverBeenMemberOfValid timeTravel(originalValidatorLockedGoldRequirements.duration); } - function _deregisterValidator(address _validator) internal { - vm.prank(_validator); - validators.deregisterValidator(INDEX); - } - function test_ShouldMarkAccountAsNotValidator_WhenAccountHasNeverBeenMemberOfValidatorGroup() public { @@ -875,6 +869,10 @@ contract ValidatorsTest_DeregisterValidator_WhenAccountHasNeverBeenMemberOfValid vm.prank(validator); validators.deregisterValidator(INDEX + 1); } + function _deregisterValidator(address _validator) internal { + vm.prank(_validator); + validators.deregisterValidator(INDEX); + } } contract ValidatorsTest_DeregisterValidator_WhenAccountHasBeenMemberOfValidatorGroup is @@ -896,11 +894,6 @@ contract ValidatorsTest_DeregisterValidator_WhenAccountHasBeenMemberOfValidatorG validators.addFirstMember(validator, address(0), address(0)); } - function _deregisterValidator(address _validator) internal { - vm.prank(_validator); - validators.deregisterValidator(INDEX); - } - function test_ShouldMarkAccountAsNotValidator_WhenValidatorNoLongerMemberOfValidatorGroup() public { @@ -970,6 +963,11 @@ contract ValidatorsTest_DeregisterValidator_WhenAccountHasBeenMemberOfValidatorG vm.expectRevert("Has been group member recently"); _deregisterValidator(validator); } + + function _deregisterValidator(address _validator) internal { + vm.prank(_validator); + validators.deregisterValidator(INDEX); + } } contract ValidatorsTest_Affiliate_WhenGroupAndValidatorMeetLockedGoldRequirements is @@ -1828,6 +1826,8 @@ contract ValidatorsTest_AddMember is ValidatorsTest { uint256 _registrationEpoch; uint256 _additionEpoch; + uint256[] expectedSizeHistory; + function setUp() public { super.setUp(); @@ -1935,8 +1935,6 @@ contract ValidatorsTest_AddMember is ValidatorsTest { validators.addMember(otherValidator); } - uint256[] expectedSizeHistory; - function test_ShouldUpdateGroupsSizeHistoryAndBalanceRequirements_WhenAddingManyValidatorsAffiliatedWithGroup() public { @@ -2041,6 +2039,9 @@ contract ValidatorsTest_AddMember is ValidatorsTest { } contract ValidatorsTest_RemoveMember is ValidatorsTest { + uint256 _registrationEpoch; + uint256 _additionEpoch; + function setUp() public { super.setUp(); _registerValidatorGroupWithMembers(group, 1); @@ -2058,9 +2059,6 @@ contract ValidatorsTest_RemoveMember is ValidatorsTest { assertEq(members.length, expectedMembersList.length); } - uint256 _registrationEpoch; - uint256 _additionEpoch; - function test_ShouldUpdateMemberMembershipHistory() public { vm.prank(group); validators.removeMember(validator); @@ -2448,6 +2446,13 @@ contract ValidatorsTest_CalculateEpochScore is ValidatorsTest { vm.expectRevert("Uptime cannot be larger than one"); validators.calculateEpochScore(uptime.unwrap()); } + + function test_Reverts_WhenL2() public { + _whenL2(); + + vm.expectRevert("This method is no longer supported in L2."); + validators.calculateEpochScore(1); + } } contract ValidatorsTest_CalculateGroupEpochScore is ValidatorsTest { @@ -2593,6 +2598,19 @@ contract ValidatorsTest_CalculateGroupEpochScore is ValidatorsTest { vm.expectRevert("Uptime cannot be larger than one"); validators.calculateGroupEpochScore(unwrapedUptimes); } + + function test_Reverts_WhenL2() public { + _whenL2(); + FixidityLib.Fraction[] memory uptimes = new FixidityLib.Fraction[](5); + uptimes[0] = FixidityLib.newFixedFraction(9, 10); + uptimes[1] = FixidityLib.newFixedFraction(9, 10); + uptimes[3] = FixidityLib.newFixedFraction(9, 10); + uptimes[4] = FixidityLib.newFixedFraction(9, 10); + + (uint256[] memory unwrapedUptimes, ) = _computeGroupUptimeCalculation(uptimes); + vm.expectRevert("This method is no longer supported in L2."); + validators.calculateGroupEpochScore(unwrapedUptimes); + } } contract ValidatorsTest_UpdateValidatorScoreFromSigner is ValidatorsTest { @@ -2676,6 +2694,12 @@ contract ValidatorsTest_UpdateValidatorScoreFromSigner is ValidatorsTest { } contract ValidatorsTest_UpdateMembershipHistory is ValidatorsTest { + address[] public expectedMembershipHistoryGroups; + uint256[] public expectedMembershipHistoryEpochs; + + address[] public actualMembershipHistoryGroups; + uint256[] public actualMembershipHistoryEpochs; + function setUp() public { super.setUp(); _registerValidatorHelper(validator, validatorPk); @@ -2686,12 +2710,6 @@ contract ValidatorsTest_UpdateMembershipHistory is ValidatorsTest { } } - address[] public expectedMembershipHistoryGroups; - uint256[] public expectedMembershipHistoryEpochs; - - address[] public actualMembershipHistoryGroups; - uint256[] public actualMembershipHistoryEpochs; - function test_ShouldOverwritePreviousEntry_WhenChangingGroupsInSameEpoch() public { uint256 numTest = 10; @@ -3229,17 +3247,17 @@ contract ValidatorsTest_ForceDeaffiliateIfValidator_L2 is ValidatorsTest { } contract ValidatorsTest_GroupMembershipInEpoch is ValidatorsTest { + struct EpochInfo { + uint256 epochNumber; + address groupy; + } + uint256 totalEpochs = 24; uint256 gapSize = 3; uint256 contractIndex; EpochInfo[] public epochInfoList; - struct EpochInfo { - uint256 epochNumber; - address groupy; - } - function setUp() public { super.setUp(); From 9b1ca786b1628c17e716e1067b76a77088075bc2 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 18:40:42 -0400 Subject: [PATCH 14/18] removed comments --- .../contracts/governance/Validators.sol | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index b8f5d8708da..5bb0d539f3b 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -205,8 +205,6 @@ contract Validators is setDowntimeGracePeriod(_downtimeGracePeriod); } - // XXX: Unchanged, deals with rewards distribution. - // XXX(soloseng): should this set uptime to 1 by default on L2? /** * @notice Updates a validator's score based on its uptime for the epoch. * @param signer The validator signer of the validator account whose score needs updating. @@ -218,7 +216,6 @@ contract Validators is _updateValidatorScoreFromSigner(signer, uptime); } - // XXX: Unchanged, deals with rewards distribution. /** * @notice Distributes epoch payments to the account associated with `signer` and its group. * @param signer The validator signer of the account to distribute the epoch payment to. @@ -234,7 +231,6 @@ contract Validators is return _distributeEpochPaymentsFromSigner(signer, maxPayment); } - // XXX: Unchanged, new registrations require PoP. /** * @notice Registers a validator unaffiliated with any validator group. * @param ecdsaPublicKey The ECDSA public key that the validator is using for consensus, should @@ -307,7 +303,6 @@ contract Validators is return true; } - // XXX: Unchanged, deals with Validator/group registration. /** * @notice Affiliates a validator with a group, allowing it to be added as a member. * @param group The validator group with which to affiliate. @@ -344,7 +339,6 @@ contract Validators is return true; } - // XXX: Unchanged, new registrations require PoP. /** * @notice Updates a validator's BLS key. * @param blsPublicKey The BLS public key that the validator is using for consensus, should pass @@ -368,7 +362,6 @@ contract Validators is return true; } - // XXX: Unchanged, used only during registration. /** * @notice Updates a validator's ECDSA key. * @param account The address under which the validator is registered. @@ -417,7 +410,6 @@ contract Validators is return true; } - // XXX: Unchanged, require PoP precompile. /** * @notice Updates a validator's ECDSA and BLS keys. * @param account The address under which the validator is registered. @@ -450,7 +442,6 @@ contract Validators is return true; } - // XXX: Unchanged, deals with Validator/group registration. /** * @notice Registers a validator group with no member validators. * @param commission Fixidity representation of the commission this group receives on epoch @@ -477,7 +468,6 @@ contract Validators is return true; } - // XXX: Unchanged, deals with Validator/group registration. /** * @notice Adds a member to the end of a validator group's list of members. * @param validator The validator to add to the group @@ -492,7 +482,6 @@ contract Validators is return _addMember(account, validator, address(0), address(0)); } - // XXX: Unchanged, deals with Validator/group registration. /** * @notice Adds the first member to a group's list of members and marks it eligible for election. * @param validator The validator to add to the group @@ -550,7 +539,6 @@ contract Validators is return true; } - //XXX unchanged as rewards are not implemented. /** * @notice Queues an update to a validator group's commission. * If there was a previously scheduled update, that is overwritten. @@ -570,7 +558,6 @@ contract Validators is emit ValidatorGroupCommissionUpdateQueued(account, commission, group.nextCommissionBlock); } - //XXX unchanged as rewards are not implemented. /** * @notice Updates a validator group's commission based on the previously queued update */ @@ -602,7 +589,6 @@ contract Validators is } } - // XXX: rewards and Slashing not supported on L2 /** * @notice Resets a group's slashing multiplier if it has been >= the reset period since * the last time the group was slashed. @@ -619,7 +605,6 @@ contract Validators is group.slashInfo.multiplier = FixidityLib.fixed1(); } - // XXX: rewards and Slashing not supported on L2 /** * @notice Halves the group's slashing multiplier. * @param account The group being slashed. @@ -774,7 +759,6 @@ contract Validators is return getMembershipInLastEpoch(account); } - //XXX: should this always return 1? /** * @notice Getter for a group's slashing multiplier. * @param account The group to fetch slashing multiplier for. @@ -896,7 +880,6 @@ contract Validators is emit CommissionUpdateDelaySet(delay); } - //XXX: unchanged, as the validator group constitution should not change on L2 /** * @notice Updates the maximum number of members a group can have. * @param size The maximum group size. @@ -911,7 +894,6 @@ contract Validators is return true; } - //XXX: unchanged, as the validator group constitution should not change on L2 /** * @notice Updates the number of validator group membership entries to store. * @param length The number of validator group membership entries to store. @@ -926,7 +908,6 @@ contract Validators is return true; } - //XXX: unchanged, as the validator group constitution should not change on L2 /** * @notice Updates the validator score parameters. * @param exponent The exponent used in calculating the score. @@ -995,7 +976,6 @@ contract Validators is return true; } - //XXX: unchanged, as rewards are not yet implemented. /** * @notice Sets the slashingMultiplierRestPeriod property if called by owner. * @param value New reset period for slashing multiplier. @@ -1005,7 +985,6 @@ contract Validators is slashingMultiplierResetPeriod = value; } - //XXX: unchanged, as rewards are not yet implemented. /** * @notice Sets the downtimeGracePeriod property if called by owner. * @param value New downtime grace period for calculating epoch scores. @@ -1039,7 +1018,6 @@ contract Validators is return 0; } - //XXX: unchanged, as elections are not yet implemented. /** * @notice Returns the group that `account` was a member of at the end of the last epoch. * @param account The account whose group membership should be returned. From e027260235b04645986d88468c6a94de45ad6bc9 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Tue, 11 Jun 2024 19:14:04 -0400 Subject: [PATCH 15/18] getValidatorGroupSlashingMultiplier allowed only on L1 --- packages/protocol/contracts/governance/Validators.sol | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 5bb0d539f3b..9c48988fec1 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -764,14 +764,10 @@ contract Validators is * @param account The group to fetch slashing multiplier for. */ function getValidatorGroupSlashingMultiplier(address account) external view returns (uint256) { + allowOnlyL1(); require(isValidatorGroup(account), "Not a validator group"); - - if (isL2()) { - return 1; - } else { - ValidatorGroup storage group = groups[account]; - return group.slashInfo.multiplier.unwrap(); - } + ValidatorGroup storage group = groups[account]; + return group.slashInfo.multiplier.unwrap(); } /** From 7f289e6a8a7e5a80780e045614319244139aa423 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 12 Jun 2024 10:15:10 -0400 Subject: [PATCH 16/18] reverting changes to test contract size issue --- .../contracts/governance/Validators.sol | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 9c48988fec1..6b3a54be150 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -529,6 +529,7 @@ contract Validators is address lesserMember, address greaterMember ) external nonReentrant returns (bool) { + allowOnlyL1(); address account = getAccounts().validatorSignerToAccount(msg.sender); require(isValidatorGroup(account), "Not a group"); require(isValidator(validator), "Not a validator"); @@ -557,7 +558,6 @@ contract Validators is group.nextCommissionBlock = block.number.add(commissionUpdateDelay); emit ValidatorGroupCommissionUpdateQueued(account, commission, group.nextCommissionBlock); } - /** * @notice Updates a validator group's commission based on the previously queued update */ @@ -581,6 +581,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)) { @@ -650,27 +651,15 @@ contract Validators is { require(isValidatorGroup(account), "Not a validator group"); ValidatorGroup storage group = groups[account]; - if (isL2()) { - return ( - group.members.getKeys(), - group.commission.unwrap(), - group.nextCommission.unwrap(), - group.nextCommissionBlock, - group.sizeHistory, - 1, - group.slashInfo.lastSlashed - ); - } else { - return ( - group.members.getKeys(), - group.commission.unwrap(), - group.nextCommission.unwrap(), - group.nextCommissionBlock, - group.sizeHistory, - group.slashInfo.multiplier.unwrap(), - group.slashInfo.lastSlashed - ); - } + return ( + group.members.getKeys(), + group.commission.unwrap(), + group.nextCommission.unwrap(), + group.nextCommissionBlock, + group.sizeHistory, + group.slashInfo.multiplier.unwrap(), + group.slashInfo.lastSlashed + ); } /** @@ -1041,7 +1030,6 @@ contract Validators is * @return Fixidity representation of the epoch score between 0 and 1. */ function calculateEpochScore(uint256 uptime) public view returns (uint256) { - allowOnlyL1(); require(uptime <= FixidityLib.fixed1().unwrap(), "Uptime cannot be larger than one"); uint256 numerator; uint256 denominator; From be3c2cfd22354d31d4e0ebccddbda0ade5b124dc Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 12 Jun 2024 13:23:29 -0400 Subject: [PATCH 17/18] minimal changes --- packages/protocol/contracts/governance/Validators.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 6b3a54be150..21d705ce6fd 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -529,7 +529,6 @@ contract Validators is address lesserMember, address greaterMember ) external nonReentrant returns (bool) { - allowOnlyL1(); address account = getAccounts().validatorSignerToAccount(msg.sender); require(isValidatorGroup(account), "Not a group"); require(isValidator(validator), "Not a validator"); @@ -558,6 +557,7 @@ contract Validators is group.nextCommissionBlock = block.number.add(commissionUpdateDelay); emit ValidatorGroupCommissionUpdateQueued(account, commission, group.nextCommissionBlock); } + /** * @notice Updates a validator group's commission based on the previously queued update */ @@ -581,7 +581,6 @@ 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)) { @@ -1030,6 +1029,7 @@ contract Validators is * @return Fixidity representation of the epoch score between 0 and 1. */ function calculateEpochScore(uint256 uptime) public view returns (uint256) { + allowOnlyL1(); require(uptime <= FixidityLib.fixed1().unwrap(), "Uptime cannot be larger than one"); uint256 numerator; uint256 denominator; From e3b4ccb68b72b2b96bf2a738049389887bef03a7 Mon Sep 17 00:00:00 2001 From: soloseng <102702451+soloseng@users.noreply.github.com> Date: Wed, 12 Jun 2024 16:17:59 -0400 Subject: [PATCH 18/18] allow slashing multiplier reset on L2 --- packages/protocol/contracts/governance/Validators.sol | 2 -- .../test-sol/unit/governance/validators/Validators.t.sol | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/protocol/contracts/governance/Validators.sol b/packages/protocol/contracts/governance/Validators.sol index 21d705ce6fd..2f5ce7b2af5 100644 --- a/packages/protocol/contracts/governance/Validators.sol +++ b/packages/protocol/contracts/governance/Validators.sol @@ -594,7 +594,6 @@ contract Validators is * the last time the group was slashed. */ function resetSlashingMultiplier() external nonReentrant { - allowOnlyL1(); address account = getAccounts().validatorSignerToAccount(msg.sender); require(isValidatorGroup(account), "Not a validator group"); ValidatorGroup storage group = groups[account]; @@ -752,7 +751,6 @@ contract Validators is * @param account The group to fetch slashing multiplier for. */ function getValidatorGroupSlashingMultiplier(address account) external view returns (uint256) { - allowOnlyL1(); require(isValidatorGroup(account), "Not a validator group"); ValidatorGroup storage group = groups[account]; return group.slashInfo.multiplier.unwrap(); 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 a82220093c3..2978ec2f183 100644 --- a/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol +++ b/packages/protocol/test-sol/unit/governance/validators/Validators.t.sol @@ -3459,13 +3459,16 @@ contract ValidatorsTest_ResetSlashingMultiplier is ValidatorsTest { assertEq(actualMultiplier, FixidityLib.fixed1().unwrap()); } - function test_Reverts_WhenSlashingMultiplierIsResetAfterResetPeriod_WhenL2() public { + function test_ShouldReturnToDefault_WhenSlashingMultiplierIsResetAfterResetPeriod_WhenL2() + public + { _whenL2(); timeTravel(slashingMultiplierResetPeriod); vm.prank(group); - vm.expectRevert("This method is no longer supported in L2."); validators.resetSlashingMultiplier(); + (, , , , , uint256 actualMultiplier, ) = validators.getValidatorGroup(group); + assertEq(actualMultiplier, FixidityLib.fixed1().unwrap()); } function test_Reverts_WhenSlashingMultiplierIsResetBeforeResetPeriod() public {