rebase master (#220)

* index some event parameters

* update solidity compiler version

* systemcontract: governance unit tests (#210)

* update gitignore

* init hardhat project

* first ut

* systemcontract: add candidate management ut and getter check (#211)

* systemcontract: use customized error library (#209)

* systemcontract: use customized error library

* update existing ut

* systemcontract: add policy unit tests (#214)

* systemcontract: add ut about reward distribution and fix several ut (#215)

* systemcontract: optimize gas cost (#195)

* systemcontract: optimize gas cost

---------

Co-authored-by: belane <[email protected]>

Author: txhsl

.gitignore

@@ -53,4 +53,16 @@ logs/
 /privnet/**/*.log
 /privnet/**/geth/
 
-tests/spec-tests/
+tests/spec-tests/
+
+# contracts dev
+node_modules
+.env
+/contracts/cache
+/contracts/artifacts
+/contracts/typechain
+/contracts/typechain-types
+/contracts/coverage
+/contracts/coverage.json
+/contracts/ignition
+/contracts/package-lock.json

contracts/hardhat.config.ts

@@ -0,0 +1,21 @@
+import { HardhatUserConfig } from "hardhat/config";
+import "@nomicfoundation/hardhat-toolbox";
+
+const config: HardhatUserConfig = {
+  solidity: {
+    version: "0.8.25",
+    settings: {
+      evmVersion: "paris",
+      optimizer: {
+        enabled: true,
+        runs: 500,
+      },
+    },
+  },
+  paths: {
+    sources: "./solidity",
+    tests: "./test",
+  }
+};
+
+export default config;

contracts/package.json

@@ -0,0 +1,14 @@
+{
+  "name": "neox-governance",
+  "scripts": {
+    "compile": "npx hardhat compile",
+    "test": "npx hardhat test"
+  },
+  "devDependencies": {
+    "@nomicfoundation/hardhat-toolbox": "^5.0.0",
+    "hardhat": "^2.22.3"
+  },
+  "dependencies": {
+    "@openzeppelin/contracts-upgradeable": "^5.0.2"
+  }
+}

contracts/solidity/ERC1967Proxy.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.25;
+
+library Errors {
+    // Universal Errors
+    error NotAdmin();
+    error NotGovernance();
+
+    // GovReward Errors
+    error TransferFailed();
+
+    // GovernanceVote Errors
+    error NotMiner();
+
+    // Policy Errors
+    error BlacklistExists();
+    error BlacklistNotExists();
+    error InvalidMinGasTipCap();
+    error InvalidBaseFee();
+
+    // Governance Errors
+    error SideCallNotAllowed();
+    error OnlyEOA();
+    error InsufficientValue();
+    error InvalidShareRate();
+    error CandidateExists();
+    error CandidateNotExists();
+    error LeftNotClaimed();
+    error CandidateWithdrawNotAllowed();
+    error MultipleVoteNotAllowed();
+    error NoVote();
+}

contracts/solidity/Errors.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.20;
+pragma solidity ^0.8.25;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import "./GovernanceVote.sol";
@@ -27,8 +27,10 @@ contract GovProxyAdmin is GovernanceVote {
         payable
         virtual
         needVote(
-            keccak256("upgradeAndCall"),
-            keccak256(abi.encode(proxy, newImplementation, data))
+            bytes32(
+                0xe739b9109d83c1c6d0d640fe9ed476fc5862a6de5483b00678a3fffa7a2be2f6
+            ),
+            keccak256(abi.encodePacked(proxy, newImplementation, data))
         )
     {
         proxy.upgradeToAndCall{value: msg.value}(newImplementation, data);

contracts/solidity/GovProxyAdmin.sol

@@ -1,26 +1,9 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.20;
+pragma solidity ^0.8.25;
 
+import "./Errors.sol";
 import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 
-library TransferHelper {
-    function safeTransfer(address token, address to, uint256 value) internal {
-        // bytes4(keccak256(bytes('transfer(address,uint256)')));
-        (bool success, bytes memory data) = token.call(
-            abi.encodeWithSelector(0xa9059cbb, to, value)
-        );
-        require(
-            success && (data.length == 0 || abi.decode(data, (bool))),
-            "safeTransfer: transfer failed"
-        );
-    }
-
-    function safeTransferETH(address to, uint256 value) internal {
-        (bool success, ) = to.call{value: value}(new bytes(0));
-        require(success, "safeTransferETH: ETH transfer failed");
-    }
-}
-
 interface IGovernance {
     // get current consensus group
     function getCurrentConsensus() external view returns (address[] memory);
@@ -42,12 +25,12 @@ contract GovReward is IGovReward, UUPSUpgradeable {
     receive() external payable {}
 
     modifier onlyGov() {
-        require(msg.sender == GOV, "not governance");
+        if (msg.sender != GOV) revert Errors.NotGovernance();
         _;
     }
 
     modifier onlyAdmin() {
-        require(msg.sender == GOV_ADMIN, "not admin");
+        if (msg.sender != GOV_ADMIN) revert Errors.NotAdmin();
         _;
     }
 
@@ -81,7 +64,12 @@ contract GovReward is IGovReward, UUPSUpgradeable {
 
     function withdraw() external onlyGov {
         if (address(this).balance > 0) {
-            TransferHelper.safeTransferETH(GOV, address(this).balance);
+            _safeTransferETH(GOV, address(this).balance);
         }
     }
+
+    function _safeTransferETH(address to, uint value) internal {
+        (bool success, ) = to.call{value: value}(new bytes(0));
+        if (!success) revert Errors.TransferFailed();
+    }
 }

contracts/solidity/GovReward.sol

@@ -1,16 +1,17 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.20;
+pragma solidity ^0.8.25;
 
+import "./Errors.sol";
 import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
 import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 
 interface IGovernance {
     event Register(address candidate);
     event Exit(address candidate);
-    event Vote(address voter, address to, uint amount);
-    event Revoke(address voter, address from, uint amount);
-    event VoterClaim(address voter, uint reward);
+    event Vote(address indexed voter, address indexed to, uint amount);
+    event Revoke(address indexed voter, address indexed from, uint amount);
+    event VoterClaim(address indexed voter, uint reward);
     event CandidateWithdraw(address candidate, uint amount);
     event Persist(address[] validators);
 
@@ -106,7 +107,7 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     mapping(address => mapping(uint => uint)) public epochStartGasPerVote;
 
     modifier onlyAdmin() {
-        require(msg.sender == GOV_ADMIN, "not admin");
+        if (msg.sender != GOV_ADMIN) revert Errors.NotAdmin();
         _;
     }
 
@@ -135,7 +136,7 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     }
 
     receive() external payable nonReentrant {
-        require(msg.sender == GOV_REWARD, "side call not allowed");
+        if (msg.sender != GOV_REWARD) revert Errors.SideCallNotAllowed();
         address[] memory validators = currentConsensus;
         uint length = validators.length;
         for (uint i = 0; i < length; i++) {
@@ -160,12 +161,11 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     }
 
     function registerCandidate(uint shareRate) external payable {
-        require(tx.origin == msg.sender, "only allow EOA");
-        require(msg.value >= registerFee, "insufficient amount");
-        require(shareRate < 1000, "invalid rate");
-        require(!candidateList.contains(msg.sender), "candidate exists");
-        require(exitHeightOf[msg.sender] == 0, "left not claimed");
-        candidateList.add(msg.sender);
+        if (tx.origin != msg.sender) revert Errors.OnlyEOA();
+        if (msg.value < registerFee) revert Errors.InsufficientValue();
+        if (shareRate > 1000) revert Errors.InvalidShareRate();
+        if (exitHeightOf[msg.sender] > 0) revert Errors.LeftNotClaimed();
+        if (!candidateList.add(msg.sender)) revert Errors.CandidateExists();
         if (receivedVotes[msg.sender] > 0) {
             totalVotes += receivedVotes[msg.sender];
         }
@@ -177,9 +177,9 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     }
 
     function exitCandidate() external {
-        require(candidateList.contains(msg.sender), "candidate not exists");
+        if (!candidateList.remove(msg.sender))
+            revert Errors.CandidateNotExists();
         // remove candidate list, balance still locked
-        candidateList.remove(msg.sender);
         exitHeightOf[msg.sender] = block.number;
         if (receivedVotes[msg.sender] > 0) {
             totalVotes -= receivedVotes[msg.sender];
@@ -190,11 +190,10 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     function withdrawRegisterFee() external nonReentrant {
         // require 2 epochs to exit candidate list
         // NOTE: suppose epoch change always happens in time
-        require(
-            exitHeightOf[msg.sender] > 0 &&
-                block.number > exitHeightOf[msg.sender] + 2 * epochDuration,
-            "withdraw not allowed"
-        );
+        if (
+            exitHeightOf[msg.sender] <= 0 ||
+            block.number <= exitHeightOf[msg.sender] + 2 * epochDuration
+        ) revert Errors.CandidateWithdrawNotAllowed();
 
         // send back balance
         uint amount = candidateBalanceOf[msg.sender];
@@ -207,13 +206,12 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     }
 
     function vote(address candidateTo) external payable nonReentrant {
-        require(msg.value >= minVoteAmount, "insufficient amount");
-        require(candidateList.contains(candidateTo), "candidate not allowed");
+        if (msg.value < minVoteAmount) revert Errors.InsufficientValue();
+        if (!candidateList.contains(candidateTo))
+            revert Errors.CandidateNotExists();
         address votedCandidate = votedTo[msg.sender];
-        require(
-            votedCandidate == candidateTo || votedCandidate == address(0),
-            "only one choice is allowed"
-        );
+        if (votedCandidate != candidateTo && votedCandidate != address(0))
+            revert Errors.MultipleVoteNotAllowed();
 
         // settle reward here
         uint unclaimedReward = 0;
@@ -241,10 +239,7 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     function revokeVote() external nonReentrant {
         address candidateFrom = votedTo[msg.sender];
         uint amount = votedAmount[msg.sender];
-        require(
-            candidateFrom != address(0) && amount > 0,
-            "revoke not allowed"
-        );
+        if (candidateFrom == address(0) || amount <= 0) revert Errors.NoVote();
 
         // settle reward here
         uint unclaimedReward = _settleReward(msg.sender, candidateFrom);
@@ -290,7 +285,7 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
 
     function claimReward() external nonReentrant {
         address votedCandidate = votedTo[msg.sender];
-        require(votedCandidate != address(0), "claim not allowed");
+        if (votedCandidate == address(0)) revert Errors.NoVote();
         uint unclaimedReward = _settleReward(msg.sender, votedCandidate);
         if (unclaimedReward > 0) _safeTransferETH(msg.sender, unclaimedReward);
     }
@@ -303,7 +298,7 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
 
     function onPersist() external {
         // NOTE: suppose onPersist always happens at the beginning of every block
-        require(msg.sender == SYS_CALL, "side call not allowed");
+        if (msg.sender != SYS_CALL) revert Errors.SideCallNotAllowed();
         // only settle validator reward if there is no epoch change
         IGovReward(GOV_REWARD).withdraw();
         if (block.number < currentEpochStartHeight + epochDuration) return;
@@ -322,7 +317,7 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
         if (length < consensusSize || totalVotes < voteTargetAmount) {
             currentConsensus = standByValidators;
         } else {
-            currentConsensus = _computeConsensus();
+            currentConsensus = _computeConsensus(candidates);
         }
         emit Persist(currentConsensus);
     }
@@ -337,9 +332,9 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
     ) internal view returns (uint) {
         // NOTE: suppose onPersist always happens at the beginning of every block, then latestGasPerVote is always the latest
         uint height = voteHeight[voter];
+        if (currentEpochStartHeight <= height) return 0;
         uint lastGasPerVote = voterGasPerVote[voter];
         uint latestGasPerVote = candidateGasPerVote[candidate];
-        if (currentEpochStartHeight <= height) return 0;
 
         // NOTE: suppose epoch change always happens at the beginning of a block, then vote in that block should wait another epoch to farm reward
         uint voteEpochEndGasPerVote = epochStartGasPerVote[candidate][
@@ -366,12 +361,13 @@ contract Governance is IGovernance, ReentrancyGuard, UUPSUpgradeable {
 
     function _safeTransferETH(address to, uint value) internal {
         (bool success, ) = to.call{value: value}(new bytes(0));
-        require(success, "safeTransferETH: ETH transfer failed");
+        if (!success) revert Errors.TransferFailed();
     }
 
-    function _computeConsensus() internal view returns (address[] memory) {
+    function _computeConsensus(
+        address[] memory candidates
+    ) internal view returns (address[] memory) {
         // build up a votes array
-        address[] memory candidates = getCandidates();
         uint length = candidates.length;
         uint[] memory votes = new uint[](length);
         for (uint i = 0; i < length; i++) {