From 090cfb8ec031dcbb25b6022e2fdfc785e5618f71 Mon Sep 17 00:00:00 2001 From: Sergey <2901744+evercoinx@users.noreply.github.com.> Date: Thu, 12 Sep 2024 20:19:26 +0200 Subject: [PATCH] Extend error handling on MaticX contract --- contracts/MaticX.sol | 29 ++-- test/MaticX.forking.spec.ts | 296 ++++++++++++++++++++++++++++++++++++ utils/account.ts | 6 + 3 files changed, 318 insertions(+), 13 deletions(-) create mode 100644 utils/account.ts diff --git a/contracts/MaticX.sol b/contracts/MaticX.sol index fc998e99..dae5415b 100644 --- a/contracts/MaticX.sol +++ b/contracts/MaticX.sol @@ -3,8 +3,9 @@ pragma solidity 0.8.7; import { ERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import { AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; -import { SafeERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; +import { SafeERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; +import { StringsUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/StringsUpgradeable.sol"; import { IERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import { IValidatorShare } from "./interfaces/IValidatorShare.sol"; import { IValidatorRegistry } from "./interfaces/IValidatorRegistry.sol"; @@ -24,6 +25,7 @@ contract MaticX is PausableUpgradeable { using SafeERC20Upgradeable for IERC20Upgradeable; + using StringsUpgradeable for string; bytes32 public constant PREDICATE_ROLE = keccak256("PREDICATE_ROLE"); bytes32 public constant BOT = keccak256("BOT"); @@ -647,10 +649,9 @@ contract MaticX is function setFeePercent( uint8 _feePercent ) external override onlyRole(DEFAULT_ADMIN_ROLE) { - require(_feePercent <= 100, "_feePercent must not exceed 100"); + require(_feePercent <= 100, "Fee percent must not exceed 100"); feePercent = _feePercent; - emit SetFeePercent(_feePercent); } @@ -661,20 +662,23 @@ contract MaticX is function setTreasury( address _address ) external override onlyRole(DEFAULT_ADMIN_ROLE) { - treasury = _address; + require(_address != address(0), "Zero treasury address"); + treasury = _address; emit SetTreasury(_address); } /** - * @dev Sets the address of the stake manager. Callable by the admin only. - * @param _address Address of the stake manager + * @dev Sets the address of the validator registry. Callable by the admin + * only. + * @param _address Address of the validator registry */ function setValidatorRegistry( address _address ) external override onlyRole(DEFAULT_ADMIN_ROLE) { - validatorRegistry = _address; + require(_address != address(0), "Zero validator registry address"); + validatorRegistry = _address; emit SetValidatorRegistry(_address); } @@ -685,8 +689,9 @@ contract MaticX is function setFxStateRootTunnel( address _address ) external override onlyRole(DEFAULT_ADMIN_ROLE) { - fxStateRootTunnel = _address; + require(_address != address(0), "Zero fx state root tunnel address"); + fxStateRootTunnel = _address; emit SetFxStateRootTunnel(_address); } @@ -697,8 +702,9 @@ contract MaticX is function setVersion( string calldata _version ) external override onlyRole(DEFAULT_ADMIN_ROLE) { - version = _version; + require(!version.equal(""), "Empty version"); + version = _version; emit SetVersion(_version); } @@ -714,10 +720,7 @@ contract MaticX is polToken = _address; emit SetPOLToken(_address); - IERC20Upgradeable(polToken).safeApprove( - stakeManager, - type(uint256).max - ); + IERC20Upgradeable(polToken).approve(stakeManager, type(uint256).max); } //////////////////////////////////////////////////////////// diff --git a/test/MaticX.forking.spec.ts b/test/MaticX.forking.spec.ts index e6e1c6e7..bcdae13d 100644 --- a/test/MaticX.forking.spec.ts +++ b/test/MaticX.forking.spec.ts @@ -16,6 +16,7 @@ import { ValidatorRegistry, } from "../typechain"; import { extractEnvironmentVariables } from "../utils/environment"; +import { generateRandomAddress } from "../utils/account"; const envVars = extractEnvironmentVariables(); @@ -100,6 +101,8 @@ describe("MaticX (Forking)", function () { .setFxStateRootTunnel(fxStateRootTunnel.address); await maticX.connect(manager).setPOLToken(pol.address); + const defaultAdminRole = await maticX.DEFAULT_ADMIN_ROLE(); + // ERC20 transfers for (const staker of stakers) { await matic @@ -134,6 +137,7 @@ describe("MaticX (Forking)", function () { stakerA, stakerB, stakers, + defaultAdminRole, validatorIds, preferredDepositValidatorId, preferredWithdrawalValidatorId, @@ -1263,4 +1267,296 @@ describe("MaticX (Forking)", function () { }); }); }); + + describe("Withdraw Matic validators rewards", function () { + describe("Negative", function () { + it("Should revert with the right error if having an insufficient rewards amount", async function () { + const { + maticX, + manager, + preferredDepositValidatorId, + preferredWithdrawalValidatorId, + } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .withdrawValidatorsReward([ + preferredDepositValidatorId, + preferredWithdrawalValidatorId, + ]); + await expect(promise).to.be.revertedWith( + "Too small rewards amount" + ); + }); + }); + + describe("Positive", function () { + // TODO Add tests + }); + }); + + describe("Withdraw POL validators rewards", function () { + describe("Negative", function () { + it("Should revert with the right error if having an insufficient rewards amount", async function () { + const { + maticX, + manager, + preferredDepositValidatorId, + preferredWithdrawalValidatorId, + } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .withdrawValidatorsRewardPOL([ + preferredDepositValidatorId, + preferredWithdrawalValidatorId, + ]); + await expect(promise).to.be.revertedWith( + "Too small rewards amount" + ); + }); + }); + + describe("Positive", function () { + // TODO Add tests + }); + }); + + describe("Set a fee percent", function () { + describe("Negative", function () { + it("Should revert with the right error if called by a non admin", async function () { + const { maticX, stakerA, defaultAdminRole } = + await loadFixture(deployFixture); + + const promise = maticX.connect(stakerA).setFeePercent(100); + await expect(promise).to.be.revertedWith( + `AccessControl: account ${stakerA.address.toLowerCase()} is missing role ${defaultAdminRole}` + ); + }); + + it("Should revert with the right error if passing a too high fee percent", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX.connect(manager).setFeePercent(101); + await expect(promise).to.be.revertedWith( + "Fee percent must not exceed 100" + ); + }); + }); + + describe("Positive", function () { + it("Should emit the SetFeePercent event", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const feePercent = 100; + const promise = maticX + .connect(manager) + .setFeePercent(feePercent); + await expect(promise) + .to.emit(maticX, "SetFeePercent") + .withArgs(feePercent); + }); + }); + }); + + describe("Set a treasury address", function () { + const treasuryAddress = generateRandomAddress(); + + describe("Negative", function () { + it("Should revert with the right error if called by a non admin", async function () { + const { maticX, stakerA, defaultAdminRole } = + await loadFixture(deployFixture); + + const promise = maticX + .connect(stakerA) + .setTreasury(treasuryAddress); + await expect(promise).to.be.revertedWith( + `AccessControl: account ${stakerA.address.toLowerCase()} is missing role ${defaultAdminRole}` + ); + }); + + it("Should revert with the right error if passing the zero treasury address", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setTreasury(ethers.constants.AddressZero); + await expect(promise).to.be.revertedWith( + "Zero treasury address" + ); + }); + }); + + describe("Positive", function () { + it("Should emit the SetTreasury event", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setTreasury(treasuryAddress); + await expect(promise) + .to.emit(maticX, "SetTreasury") + .withArgs(treasuryAddress); + }); + }); + }); + + describe("Set a validator registry address", function () { + const validatorRegistryAddress = generateRandomAddress(); + + describe("Negative", function () { + it("Should revert with the right error if called by a non admin", async function () { + const { maticX, stakerA, defaultAdminRole } = + await loadFixture(deployFixture); + + const promise = maticX + .connect(stakerA) + .setValidatorRegistry(validatorRegistryAddress); + await expect(promise).to.be.revertedWith( + `AccessControl: account ${stakerA.address.toLowerCase()} is missing role ${defaultAdminRole}` + ); + }); + + it("Should revert with the right error if passing the zero treasury address", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setValidatorRegistry(ethers.constants.AddressZero); + await expect(promise).to.be.revertedWith( + "Zero validator registry address" + ); + }); + }); + + describe("Positive", function () { + it("Should emit the SetValidatorRegistry event", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setValidatorRegistry(validatorRegistryAddress); + await expect(promise) + .to.emit(maticX, "SetValidatorRegistry") + .withArgs(validatorRegistryAddress); + }); + }); + }); + + describe("Set a fx state root tunnel address", function () { + const fxStateRootTunnelAddress = generateRandomAddress(); + + describe("Negative", function () { + it("Should revert with the right error if called by a non admin", async function () { + const { maticX, stakerA, defaultAdminRole } = + await loadFixture(deployFixture); + + const promise = maticX + .connect(stakerA) + .setFxStateRootTunnel(fxStateRootTunnelAddress); + await expect(promise).to.be.revertedWith( + `AccessControl: account ${stakerA.address.toLowerCase()} is missing role ${defaultAdminRole}` + ); + }); + + it("Should revert with the right error if passing the zero fx state root tunnel address", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setFxStateRootTunnel(ethers.constants.AddressZero); + await expect(promise).to.be.revertedWith( + "Zero fx state root tunnel address" + ); + }); + }); + + describe("Positive", function () { + it("Should emit the SetFxStateRootTunnel event", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setFxStateRootTunnel(fxStateRootTunnelAddress); + await expect(promise) + .to.emit(maticX, "SetFxStateRootTunnel") + .withArgs(fxStateRootTunnelAddress); + }); + }); + }); + + describe("Set a version", function () { + const version = "1"; + + describe("Negative", function () { + it("Should revert with the right error if called by a non admin", async function () { + const { maticX, stakerA, defaultAdminRole } = + await loadFixture(deployFixture); + + const promise = maticX.connect(stakerA).setVersion(version); + await expect(promise).to.be.revertedWith( + `AccessControl: account ${stakerA.address.toLowerCase()} is missing role ${defaultAdminRole}` + ); + }); + + it("Should revert with the right error if passing an empty version", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX.connect(manager).setVersion(""); + await expect(promise).to.be.revertedWith("Empty version"); + }); + }); + + describe("Positive", function () { + it("Should emit the SetVersion event", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX.connect(manager).setVersion(version); + await expect(promise) + .to.emit(maticX, "SetVersion") + .withArgs(version); + }); + }); + }); + + describe("Set a POL token address", function () { + describe("Negative", function () { + it("Should revert with the right error if called by a non admin", async function () { + const { maticX, pol, stakerA, defaultAdminRole } = + await loadFixture(deployFixture); + + const promise = maticX + .connect(stakerA) + .setPOLToken(pol.address); + await expect(promise).to.be.revertedWith( + `AccessControl: account ${stakerA.address.toLowerCase()} is missing role ${defaultAdminRole}` + ); + }); + + it("Should revert with the right error if passing the zero POL token address", async function () { + const { maticX, manager } = await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setPOLToken(ethers.constants.AddressZero); + await expect(promise).to.be.revertedWith( + "Zero POL token address" + ); + }); + }); + + describe("Positive", function () { + it("Should emit the SetPOLToken event", async function () { + const { maticX, pol, manager } = + await loadFixture(deployFixture); + + const promise = maticX + .connect(manager) + .setPOLToken(pol.address); + await expect(promise) + .to.emit(maticX, "SetPOLToken") + .withArgs(pol.address); + }); + }); + }); }); diff --git a/utils/account.ts b/utils/account.ts new file mode 100644 index 00000000..a8f1116b --- /dev/null +++ b/utils/account.ts @@ -0,0 +1,6 @@ +import { ethers } from "hardhat"; + +export function generateRandomAddress(): string { + const privateKey = `0x${Buffer.from(ethers.utils.randomBytes(32)).toString("hex")}`; + return new ethers.Wallet(privateKey).address; +}