Skip to content

Commit

Permalink
L2 Staking and Voting (#11034)
Browse files Browse the repository at this point in the history
* 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.

* set EpochSize on L2

* allow voting and activating

* move election test contract to vote dir

* updated test to allow testing of L2 with no reward distribution

* 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

* 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.

* lint: function order

* Soloseng/CHORE-update-workflow (#11029)

* update workflow to run on push to release branches

* ++ mintgoldschedule to migration test constants

* initial validators contract review

* Remove block gas limit flag for `governance/voting` CI tests

* bump version

* ++ tests && format

* removed comments

* getValidatorGroupSlashingMultiplier allowed only on L1

* reverting changes to test contract size issue

* minimal changes

* allow slashing multiplier reset on L2

---------

Co-authored-by: Arthur Gousset <[email protected]>
  • Loading branch information
soloseng and arthurgousset authored Jun 18, 2024
1 parent 1adfa6c commit 26ddf3c
Show file tree
Hide file tree
Showing 11 changed files with 1,046 additions and 339 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/protocol_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,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()
Expand Down
18 changes: 12 additions & 6 deletions packages/protocol/contracts/common/UsingPrecompiles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 12 additions & 22 deletions packages/protocol/contracts/governance/Election.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -25,8 +24,7 @@ contract Election is
Initializable,
UsingRegistry,
UsingPrecompiles,
CalledByVm,
IsL2Check
CalledByVm
{
using AddressSortedLinkedList for SortedLinkedList.List;
using FixidityLib for FixidityLib.Fraction;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -544,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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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(
Expand All @@ -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);
Expand All @@ -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()),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)) {
Expand Down
4 changes: 1 addition & 3 deletions packages/protocol/contracts/governance/EpochRewards.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -22,8 +21,7 @@ contract EpochRewards is
UsingPrecompiles,
UsingRegistry,
Freezable,
CalledByVm,
IsL2Check
CalledByVm
{
using FixidityLib for FixidityLib.Fraction;
using SafeMath for uint256;
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/governance/Governance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
3 changes: 1 addition & 2 deletions packages/protocol/contracts/governance/SlasherUtil.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 3 additions & 7 deletions packages/protocol/contracts/governance/Validators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -28,8 +27,7 @@ contract Validators is
Initializable,
UsingRegistry,
UsingPrecompiles,
CalledByVm,
IsL2Check
CalledByVm
{
using FixidityLib for FixidityLib.Fraction;
using AddressLinkedList for LinkedList.List;
Expand Down Expand Up @@ -531,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");
Expand Down Expand Up @@ -560,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
*/
Expand All @@ -583,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)) {
Expand All @@ -597,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];
Expand Down Expand Up @@ -755,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();
Expand Down Expand Up @@ -1032,6 +1027,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;
Expand Down
4 changes: 1 addition & 3 deletions packages/protocol/contracts/identity/Random.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -19,8 +18,7 @@ contract Random is
Ownable,
Initializable,
UsingPrecompiles,
CalledByVm,
IsL2Check
CalledByVm
{
using SafeMath for uint256;

Expand Down
Loading

0 comments on commit 26ddf3c

Please sign in to comment.