Merge branch 'dev' into governance/add-new-parameter

.github/workflows/ci.yml

@@ -7,18 +7,6 @@ on:
     branches: [main, dev]
 
 jobs:
-  maintenance:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - name: Install Node modules
-        uses: actions/setup-node@v3
-        with:
-          node-version: "18.x"
-          cache: "npm"
-      - run: npm ci
-      - name: Check modified OpenZeppelin contracts
-        run: node scripts/maintenance/checkModifiedOpenZeppelinContracts.js
   tests:
     runs-on: ubuntu-latest
     steps:
@@ -82,7 +70,6 @@ jobs:
         with:
           node-version: "18.x"
           cache: "npm"
-      - run: rm foundry.toml
       - uses: crytic/[email protected]
         with:
           node-version: 18

README.md

@@ -162,9 +162,7 @@ forge install
 
 Deploying these contracts in the context of a production blockchain is out of the scope of this repo, as it requires a client that has support of the Edge specification integrated. At current, Edge maintains its own client [here](https://github.com/0xPolygon/polygon-edge), which can be consulted.
 
-One point that is worth emphasizing in this context is that from the perspective of launching a Supernet is understanding genesis contracts. Another is that for at least the time being, the decision has been made to proxify all genesis contracts in order to facilitate upgrades/updates without necessitating a hardfork or regenesis. A basic proxy for this task has been provided in [`contracts/lib/BasicGenesisProxy.sol`](/contracts/lib/BasicGenesisProxy.sol).
-
-**Important:** If you have to hardfork, the `RewardPool` and `ValidatorSet` contracts have their own custom proxies. This stems from an incident in which Supernets were deployed without proxifying the genesis contracts. These contracts already had historical state, which complicated simply hardforking their bytecode into being a proxy. The proxy contracts supplied for them (in [`contracts/child/validator/proxy/hardfork`](/contracts/child/validator/proxy/hardfork)) have been tailored to facilitate storage compatability between the legacy and current versions of these contracts.
+One point that is worth emphasizing in this context is that from the perspective of launching a Supernet is understanding genesis contracts. Another is that for at least the time being, the decision has been made to proxify all genesis contracts in order to facilitate upgrades/updates without necessitating a hardfork or regenesis. All deployment scripts in `script/deployment` use OpenZeppelin's `TransparentUpgradeableProxy`.
 
 ### Environment Setup
 

contracts/child/EIP1559Burn.sol

@@ -13,7 +13,7 @@ import "../interfaces/child/IChildERC20Predicate.sol";
 contract EIP1559Burn is Initializable {
     IChildERC20Predicate public childERC20Predicate;
     address public burnDestination;
-    IChildERC20 private constant NATIVE_TOKEN = IChildERC20(0x0000000000000000000000000000000000001010);
+    IChildERC20 private constant _NATIVE_TOKEN = IChildERC20(0x0000000000000000000000000000000000001010);
 
     event NativeTokenBurnt(address indexed burner, uint256 amount);
 
@@ -42,7 +42,7 @@ contract EIP1559Burn is Initializable {
 
         uint256 balance = address(this).balance;
 
-        childERC20Predicate.withdrawTo(NATIVE_TOKEN, burnDestination, balance);
+        childERC20Predicate.withdrawTo(_NATIVE_TOKEN, burnDestination, balance);
         // slither-disable-next-line reentrancy-events
         emit NativeTokenBurnt(msg.sender, balance);
     }

contracts/child/governance/ChildGovernor.sol

@@ -16,36 +16,36 @@ contract ChildGovernor is
     GovernorVotesQuorumFractionUpgradeable,
     GovernorTimelockControlUpgradeable
 {
-    NetworkParams internal networkParams;
+    NetworkParams private _networkParams;
 
     function initialize(
         IVotesUpgradeable token_,
         TimelockControllerUpgradeable timelock_,
         uint256 quorumNumerator_,
-        address networkParams_
+        address networkParams
     ) public initializer {
         __Governor_init("ChildGovernor");
         __GovernorTimelockControl_init(timelock_);
         __GovernorVotes_init(token_);
         __GovernorVotesQuorumFraction_init(quorumNumerator_);
 
-        networkParams = NetworkParams(networkParams_);
+        _networkParams = NetworkParams(networkParams);
     }
 
     // TODO: adjust values for block time of child chain
 
     function votingDelay() public view override returns (uint256) {
-        return networkParams.votingDelay();
+        return _networkParams.votingDelay();
     }
 
     function votingPeriod() public view override returns (uint256) {
-        return networkParams.votingPeriod();
+        return _networkParams.votingPeriod();
     }
 
     // END TODO
 
     function proposalThreshold() public view override returns (uint256) {
-        return networkParams.proposalThreshold();
+        return _networkParams.proposalThreshold();
     }
 
     // The functions below are overrides required by Solidity.

contracts/child/validator/README.md

@@ -11,9 +11,3 @@ This contract manages the distribution of assets as rewards for validators valid
 ## ValidatorSet
 
 Manages most direct validator tasks, such as committing epochs or announcing intent to unstake.
-
-## `proxy/hardfork/` and `legacy-compat/`
-
-`RewardPool` and `ValidatorSet` are both intended to be genesis contracts, as in they are intended to be instantiated at the genesis of the chain, as opposed to traditionally deployed. In Edge's earlier stages, the decision has been made to proxify all genesis contracts, as to say that a proxy will be deployed for each at genesis, pointing towards its implementation. This should allow updates to genesis contracts without needing a hardfork or regenesis. The `proxy/hardfork/` directory contains proxies for `RewardPool` and `ValidatorSet`, in case you have to hardfork. A specific proxy was needed for each due to a situation that arose from production Supernets which had not proxified, but then needed to update to a newer version. These custom proxies were created so that they would be able to continue using the historical state of `RewardPool` and `ValidatorSet`. A more generic proxy for the rest of the genesis contracts (which do not create state) can be found in [`contracts/lib/BasicGenesisProxy.sol`](../../lib/BasicGenesisProxy.sol).
-
-`legacy-compat/` serves a similar purpose. In the migrated contracts there were some variables and functions that changed. This directory contains contracts from OpenZeppelin which we have tailored in order to synchronize the storage slots of the older and newer versions of `RewardPool` and `ValidatorSet`.

contracts/child/validator/RewardPool.sol

@@ -15,13 +15,11 @@ contract RewardPool is IRewardPool, System, Initializable {
     IERC20Upgradeable public rewardToken;
     address public rewardWallet;
     IValidatorSet public validatorSet;
-    // slither-disable-next-line unused-state,naming-convention
-    uint256[1] private __legacy_compat_gap;
 
     mapping(uint256 => uint256) public paidRewardPerEpoch;
     mapping(address => uint256) public pendingRewards;
 
-    NetworkParams internal networkParams;
+    NetworkParams private _networkParams;
 
     function initialize(
         address newRewardToken,
@@ -38,7 +36,7 @@ contract RewardPool is IRewardPool, System, Initializable {
         rewardWallet = newRewardWallet;
         validatorSet = IValidatorSet(newValidatorSet);
 
-        networkParams = NetworkParams(networkParamsAddr);
+        _networkParams = NetworkParams(networkParamsAddr);
     }
 
     /**
@@ -49,7 +47,7 @@ contract RewardPool is IRewardPool, System, Initializable {
         uint256 totalBlocks = validatorSet.totalBlocks(epochId);
         require(totalBlocks != 0, "EPOCH_NOT_COMMITTED");
         // slither-disable-next-line divide-before-multiply
-        uint256 reward = (networkParams.epochReward() * totalBlocks) / epochSize;
+        uint256 reward = (_networkParams.epochReward() * totalBlocks) / epochSize;
         // TODO disincentivize long epoch times
 
         uint256 totalSupply = validatorSet.totalSupplyAt(epochId);

contracts/child/validator/ValidatorSet.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.19;
 
-import "./legacy-compat/@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20VotesUpgradeable.sol";
+import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20VotesUpgradeable.sol";
 import "../../lib/WithdrawalQueue.sol";
 import "../../interfaces/child/validator/IValidatorSet.sol";
 import "../../interfaces/IStateSender.sol";
@@ -12,25 +12,23 @@ import "../System.sol";
 contract ValidatorSet is IValidatorSet, ERC20VotesUpgradeable, System {
     using WithdrawalQueueLib for WithdrawalQueue;
 
-    bytes32 private constant STAKE_SIG = keccak256("STAKE");
-    bytes32 private constant UNSTAKE_SIG = keccak256("UNSTAKE");
-    bytes32 private constant SLASH_SIG = keccak256("SLASH");
+    bytes32 private constant _STAKE_SIG = keccak256("STAKE");
+    bytes32 private constant _UNSTAKE_SIG = keccak256("UNSTAKE");
+    bytes32 private constant _SLASH_SIG = keccak256("SLASH");
     uint256 public constant SLASHING_PERCENTAGE = 50; // to be read through NetworkParams later
     uint256 public constant SLASH_INCENTIVE_PERCENTAGE = 30; // exitor reward, to be read through NetworkParams later
 
-    IStateSender private stateSender;
-    address private stateReceiver;
-    address private rootChainManager;
-    // slither-disable-next-line unused-state,naming-convention
-    uint256[1] private __legacy_compat_gap;
+    IStateSender private _stateSender;
+    address private _stateReceiver;
+    address private _rootChainManager;
 
     uint256 public currentEpochId;
 
     mapping(uint256 => Epoch) public epochs;
     uint256[] public epochEndBlocks;
-    mapping(address => WithdrawalQueue) private withdrawals;
+    mapping(address => WithdrawalQueue) private _withdrawals;
 
-    NetworkParams private networkParams;
+    NetworkParams private _networkParams;
     mapping(uint256 => uint256) private _commitBlockNumbers;
 
     mapping(uint256 => bool) public slashProcessed;
@@ -47,10 +45,10 @@ contract ValidatorSet is IValidatorSet, ERC20VotesUpgradeable, System {
             "INVALID_INPUT"
         );
         __ERC20_init("ValidatorSet", "VSET");
-        stateSender = IStateSender(newStateSender);
-        stateReceiver = newStateReceiver;
-        rootChainManager = newRootChainManager;
-        networkParams = NetworkParams(newNetworkParams);
+        _stateSender = IStateSender(newStateSender);
+        _stateReceiver = newStateReceiver;
+        _rootChainManager = newRootChainManager;
+        _networkParams = NetworkParams(newNetworkParams);
         for (uint256 i = 0; i < initialValidators.length; ) {
             _stake(initialValidators[i].addr, initialValidators[i].stake);
             unchecked {
@@ -80,18 +78,18 @@ contract ValidatorSet is IValidatorSet, ERC20VotesUpgradeable, System {
      * @inheritdoc IValidatorSet
      */
     function slash(address[] calldata validators) external onlySystemCall {
-        stateSender.syncState(
-            rootChainManager,
-            abi.encode(SLASH_SIG, validators, SLASHING_PERCENTAGE, SLASH_INCENTIVE_PERCENTAGE)
+        _stateSender.syncState(
+            _rootChainManager,
+            abi.encode(_SLASH_SIG, validators, SLASHING_PERCENTAGE, SLASH_INCENTIVE_PERCENTAGE)
         );
     }
 
     function onStateReceive(uint256 /*counter*/, address sender, bytes calldata data) external override {
-        require(msg.sender == stateReceiver && sender == rootChainManager, "INVALID_SENDER");
-        if (bytes32(data[:32]) == STAKE_SIG) {
+        require(msg.sender == _stateReceiver && sender == _rootChainManager, "INVALID_SENDER");
+        if (bytes32(data[:32]) == _STAKE_SIG) {
             (address validator, uint256 amount) = abi.decode(data[32:], (address, uint256));
             _stake(validator, amount);
-        } else if (bytes32(data[:32]) == SLASH_SIG) {
+        } else if (bytes32(data[:32]) == _SLASH_SIG) {
             (, uint256 exitEventId, address[] memory validatorsToSlash, ) = abi.decode(
                 data,
                 (bytes32, uint256, address[], uint256)
@@ -112,26 +110,26 @@ contract ValidatorSet is IValidatorSet, ERC20VotesUpgradeable, System {
      * @inheritdoc IValidatorSet
      */
     function withdraw() external {
-        WithdrawalQueue storage queue = withdrawals[msg.sender];
+        WithdrawalQueue storage queue = _withdrawals[msg.sender];
         (uint256 amount, uint256 newHead) = queue.withdrawable(currentEpochId);
         queue.head = newHead;
         emit Withdrawal(msg.sender, amount);
-        stateSender.syncState(rootChainManager, abi.encode(UNSTAKE_SIG, msg.sender, amount));
+        _stateSender.syncState(_rootChainManager, abi.encode(_UNSTAKE_SIG, msg.sender, amount));
     }
 
     /**
      * @inheritdoc IValidatorSet
      */
     // slither-disable-next-line unused-return
     function withdrawable(address account) external view returns (uint256 amount) {
-        (amount, ) = withdrawals[account].withdrawable(currentEpochId);
+        (amount, ) = _withdrawals[account].withdrawable(currentEpochId);
     }
 
     /**
      * @inheritdoc IValidatorSet
      */
     function pendingWithdrawals(address account) external view returns (uint256) {
-        return withdrawals[account].pending(currentEpochId);
+        return _withdrawals[account].pending(currentEpochId);
     }
 
     /**
@@ -151,7 +149,7 @@ contract ValidatorSet is IValidatorSet, ERC20VotesUpgradeable, System {
     }
 
     function _registerWithdrawal(address account, uint256 amount) internal {
-        withdrawals[account].append(amount, currentEpochId + networkParams.withdrawalWaitPeriod());
+        _withdrawals[account].append(amount, currentEpochId + _networkParams.withdrawalWaitPeriod());
         emit WithdrawalRegistered(account, amount);
     }
 
@@ -162,7 +160,7 @@ contract ValidatorSet is IValidatorSet, ERC20VotesUpgradeable, System {
         for (uint256 i = 0; i < length; ) {
             _burn(validatorsToSlash[i], balanceOf(validatorsToSlash[i])); // unstake validator
             // slither-disable-next-line mapping-deletion
-            delete withdrawals[validatorsToSlash[i]]; // remove pending withdrawals
+            delete _withdrawals[validatorsToSlash[i]]; // remove pending withdrawals
             unchecked {
                 ++i;
             }