refactor(contracts): move events, structs and errors inside contracts (…

…#776) * Refactor consensus * Refactor MultiPayment * Refactor usernames * Update abi
ArkEcosystem · Nov 26, 2024 · 7081777 · 7081777
1 parent 1aabb0f
commit 7081777
Show file tree

Hide file tree

Showing 21 changed files with 290 additions and 367 deletions.
diff --git a/contracts/src/consensus/ConsensusV1.sol b/contracts/src/consensus/ConsensusV1.sol
@@ -4,70 +4,6 @@ pragma solidity ^0.8.27;
 import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 
-struct ValidatorData {
-    uint256 votersCount;
-    uint256 voteBalance;
-    bool isResigned;
-    bytes blsPublicKey; // 96 bits
-}
-
-struct Validator {
-    address addr;
-    ValidatorData data;
-}
-
-struct RoundValidator {
-    address addr;
-    uint256 voteBalance;
-}
-
-struct Round {
-    uint256 round;
-    RoundValidator[] validators;
-}
-
-struct Vote {
-    address validator;
-    uint256 balance;
-    address prev;
-    address next;
-}
-
-struct VoteResult {
-    address voter;
-    address validator;
-}
-
-event ValidatorRegistered(address addr, bytes blsPublicKey);
-
-event ValidatorUpdated(address addr, bytes blsPublicKey);
-
-event ValidatorResigned(address addr);
-
-event Voted(address voter, address validator);
-
-event Unvoted(address voter, address validator);
-
-error CallerIsNotValidator();
-error ValidatorNotRegistered();
-error ValidatorAlreadyRegistered();
-error ValidatorAlreadyResigned();
-error BellowMinValidators();
-error NoActiveValidators();
-
-error BlsKeyAlreadyRegistered();
-error BlsKeyIsInvalid();
-
-error VoteResignedValidator();
-error VoteSameValidator();
-error VoteValidatorWithoutBlsPublicKey();
-error AlreadyVoted();
-error MissingVote();
-
-error InvalidRange(uint256 min, uint256 max);
-error InvalidParameters();
-error ImportIsNotAllowed();
-
 // Validators:
 // - Registered -> All validators that are registered including resigned validators
 // - Active -> Top N validators with the highest vote balance, that participate in the consensus
@@ -87,6 +23,70 @@ error ImportIsNotAllowed();
 // This process will only work fine if we pass the new wallet balance (88) and keep track of voteBalances in EVM contract.
 
 contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {
+    struct ValidatorData {
+        uint256 votersCount;
+        uint256 voteBalance;
+        bool isResigned;
+        bytes blsPublicKey; // 96 bits
+    }
+
+    struct Validator {
+        address addr;
+        ValidatorData data;
+    }
+
+    struct RoundValidator {
+        address addr;
+        uint256 voteBalance;
+    }
+
+    struct Round {
+        uint256 round;
+        RoundValidator[] validators;
+    }
+
+    struct Vote {
+        address validator;
+        uint256 balance;
+        address prev;
+        address next;
+    }
+
+    struct VoteResult {
+        address voter;
+        address validator;
+    }
+
+    event ValidatorRegistered(address addr, bytes blsPublicKey);
+
+    event ValidatorUpdated(address addr, bytes blsPublicKey);
+
+    event ValidatorResigned(address addr);
+
+    event Voted(address voter, address validator);
+
+    event Unvoted(address voter, address validator);
+
+    error CallerIsNotValidator();
+    error ValidatorNotRegistered();
+    error ValidatorAlreadyRegistered();
+    error ValidatorAlreadyResigned();
+    error BellowMinValidators();
+    error NoActiveValidators();
+
+    error BlsKeyAlreadyRegistered();
+    error BlsKeyIsInvalid();
+
+    error VoteResignedValidator();
+    error VoteSameValidator();
+    error VoteValidatorWithoutBlsPublicKey();
+    error AlreadyVoted();
+    error MissingVote();
+
+    error InvalidRange(uint256 min, uint256 max);
+    error InvalidParameters();
+    error ImportIsNotAllowed();
+
     mapping(address => ValidatorData) private _validatorsData;
     mapping(address => bool) private _hasValidator;
     mapping(bytes32 => bool) private _blsPublicKeys;

diff --git a/contracts/src/multi-payment/MultiPayment.sol b/contracts/src/multi-payment/MultiPayment.sol
@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: GNU GENERAL PUBLIC LICENSE
 pragma solidity ^0.8.27;
 
-error RecipientsAndAmountsMismatch();
-error InvalidValue();
-error FailedToSendEther();
-
 contract MultiPayment {
+    error RecipientsAndAmountsMismatch();
+    error InvalidValue();
+    error FailedToSendEther();
+
     function pay(address payable[] calldata recipients, uint256[] calldata amounts) public payable {
         if (recipients.length != amounts.length) {
             revert RecipientsAndAmountsMismatch();

diff --git a/contracts/src/usernames/UsernamesV1.sol b/contracts/src/usernames/UsernamesV1.sol
@@ -4,20 +4,20 @@ pragma solidity ^0.8.27;
 import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 
-error InvalidUsername();
-error TakenUsername();
-error UsernameNotRegistered();
+contract UsernamesV1 is UUPSUpgradeable, OwnableUpgradeable {
+    error InvalidUsername();
+    error TakenUsername();
+    error UsernameNotRegistered();
 
-event UsernameRegistered(address addr, string username, string previousUsername);
+    event UsernameRegistered(address addr, string username, string previousUsername);
 
-event UsernameResigned(address addr, string username);
+    event UsernameResigned(address addr, string username);
 
-struct User {
-    address addr;
-    string username;
-}
+    struct User {
+        address addr;
+        string username;
+    }
 
-contract UsernamesV1 is UUPSUpgradeable, OwnableUpgradeable {
     mapping(address => string) private _usernames;
     mapping(bytes32 => bool) private _usernameExists;
 

diff --git a/contracts/test/consensus/Base.sol b/contracts/test/consensus/Base.sol
@@ -2,7 +2,7 @@
 pragma solidity ^0.8.13;
 
 import {Test, console} from "@forge-std/Test.sol";
-import {ConsensusV1, Validator} from "@contracts/consensus/ConsensusV1.sol";
+import {ConsensusV1} from "@contracts/consensus/ConsensusV1.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
 contract Base is Test {
@@ -27,13 +27,17 @@ contract Base is Test {
         return prepareBLSKey(addr, 48);
     }
 
-    function sortValidators(Validator[] memory validators) public pure returns (Validator[] memory) {
+    function sortValidators(ConsensusV1.Validator[] memory validators)
+        public
+        pure
+        returns (ConsensusV1.Validator[] memory)
+    {
         uint256 length = validators.length;
         for (uint256 i = 0; i < length; i++) {
             for (uint256 j = i + 1; j < length; j++) {
                 // Sort in descending order by votersCount
                 if (_isGreater(validators[i], validators[j])) {
-                    Validator memory temp = validators[i];
+                    ConsensusV1.Validator memory temp = validators[i];
                     validators[i] = validators[j];
                     validators[j] = temp;
                 }
@@ -42,7 +46,11 @@ contract Base is Test {
         return validators;
     }
 
-    function _isGreater(Validator memory validatorA, Validator memory validatorB) internal pure returns (bool) {
+    function _isGreater(ConsensusV1.Validator memory validatorA, ConsensusV1.Validator memory validatorB)
+        internal
+        pure
+        returns (bool)
+    {
         if (validatorA.data.voteBalance == validatorB.data.voteBalance) {
             return validatorA.addr > validatorB.addr;
         }
@@ -61,7 +69,7 @@ contract Base is Test {
         consensus.registerValidator(validatorKey);
         vm.stopPrank();
 
-        Validator memory validator = consensus.getValidator(addr);
+        ConsensusV1.Validator memory validator = consensus.getValidator(addr);
         assertEq(validator.addr, addr);
         assertEq(validator.data.voteBalance, 0 ether);
         assertEq(validator.data.votersCount, 0);

diff --git a/contracts/test/consensus/Consensus-CalculateTop.sol b/contracts/test/consensus/Consensus-CalculateTop.sol
@@ -1,13 +1,7 @@
 // SPDX-License-Identifier: GNU GENERAL PUBLIC LICENSE
 pragma solidity ^0.8.13;
 
-import {
-    ConsensusV1,
-    ValidatorData,
-    Validator,
-    InvalidParameters,
-    NoActiveValidators
-} from "@contracts/consensus/ConsensusV1.sol";
+import {ConsensusV1} from "@contracts/consensus/ConsensusV1.sol";
 import {Base} from "./Base.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
@@ -18,7 +12,7 @@ contract ConsensusTest is Base {
         registerValidator(addr);
 
         consensus.calculateActiveValidators(1);
-        Validator[] memory validators = consensus.getActiveValidators();
+        ConsensusV1.Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, 1);
         assertEq(validators[0].addr, addr);
     }
@@ -33,26 +27,26 @@ contract ConsensusTest is Base {
     function test_should_revert_with_0_parameter() public {
         registerValidator(address(1));
 
-        vm.expectRevert(InvalidParameters.selector);
+        vm.expectRevert(ConsensusV1.InvalidParameters.selector);
         consensus.calculateActiveValidators(0);
     }
 
     function test_should_revert_without_validators() public {
-        vm.expectRevert(NoActiveValidators.selector);
+        vm.expectRevert(ConsensusV1.NoActiveValidators.selector);
         consensus.calculateActiveValidators(1);
     }
 
     function test_should_revert_with_only_resigned_validators() public {
         consensus.addValidator(address(2), prepareBLSKey(address(2)), true);
 
-        vm.expectRevert(NoActiveValidators.selector);
+        vm.expectRevert(ConsensusV1.NoActiveValidators.selector);
         consensus.calculateActiveValidators(1);
     }
 
     function test_should_revert_with_only_validators_without_public_key() public {
         consensus.addValidator(address(1), new bytes(0), false);
 
-        vm.expectRevert(NoActiveValidators.selector);
+        vm.expectRevert(ConsensusV1.NoActiveValidators.selector);
         consensus.calculateActiveValidators(1);
     }
 
@@ -64,7 +58,7 @@ contract ConsensusTest is Base {
         resignValidator(addr);
 
         consensus.calculateActiveValidators(2);
-        Validator[] memory validators = consensus.getActiveValidators();
+        ConsensusV1.Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, 2);
         assertEq(validators[0].addr, address(2));
         assertEq(validators[1].addr, address(2)); // Second validator is duplicated
@@ -79,7 +73,7 @@ contract ConsensusTest is Base {
         resignValidator(address(2));
 
         consensus.calculateActiveValidators(2);
-        Validator[] memory validators = consensus.getActiveValidators();
+        ConsensusV1.Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, 2);
         assertEq(validators[0].addr, addr);
         assertEq(validators[1].addr, addr); // Second validator is duplicated
@@ -92,7 +86,7 @@ contract ConsensusTest is Base {
         consensus.addValidator(address(2), new bytes(0), false);
 
         consensus.calculateActiveValidators(2);
-        Validator[] memory validators = consensus.getActiveValidators();
+        ConsensusV1.Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, 2);
         assertEq(validators[0].addr, addr);
         assertEq(validators[1].addr, addr); // Second validator is duplicated
@@ -134,7 +128,7 @@ contract ConsensusTest is Base {
         uint160 activeValidators = 53;
 
         consensus.calculateActiveValidators(uint8(activeValidators));
-        Validator[] memory validators = consensus.getActiveValidators();
+        ConsensusV1.Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, activeValidators);
 
         assertEq(validators[activeValidators - 1].addr, address(0xAE)); // Shuffled address

diff --git a/contracts/test/consensus/Consensus-GetAllValidators.sol b/contracts/test/consensus/Consensus-GetAllValidators.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GNU GENERAL PUBLIC LICENSE
 pragma solidity ^0.8.13;
 
-import {ConsensusV1, ValidatorData, Validator} from "@contracts/consensus/ConsensusV1.sol";
+import {ConsensusV1} from "@contracts/consensus/ConsensusV1.sol";
 import {Base} from "./Base.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
@@ -22,7 +22,7 @@ contract ConsensusTest is Base {
 
         vm.resumeGasMetering();
 
-        Validator[] memory validators = consensus.getAllValidators();
+        ConsensusV1.Validator[] memory validators = consensus.getAllValidators();
         assertEq(validators.length, n);
     }
 }
diff --git a/contracts/test/consensus/Consensus-Proxy.sol b/contracts/test/consensus/Consensus-Proxy.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GNU GENERAL PUBLIC LICENSE
 pragma solidity ^0.8.13;
 
-import {ConsensusV1, ValidatorData, Validator} from "@contracts/consensus/ConsensusV1.sol";
+import {ConsensusV1} from "@contracts/consensus/ConsensusV1.sol";
 import {Base} from "./Base.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
@@ -65,7 +65,7 @@ contract ConsensusTest is Base {
         assertEq(consensus.activeValidatorsCount(), 2);
         assertEq(consensus.getVotesCount(), 1);
         assertEq(consensus.getActiveValidators().length, 2);
-        Validator[] memory validatorsBefore = consensus.getAllValidators();
+        ConsensusV1.Validator[] memory validatorsBefore = consensus.getAllValidators();
         assertEq(validatorsBefore.length, 3);
 
         // Upgrade
@@ -80,7 +80,7 @@ contract ConsensusTest is Base {
         assertEq(consensusNew.activeValidatorsCount(), 2);
         assertEq(consensusNew.getVotesCount(), 1);
         assertEq(consensus.getActiveValidators().length, 2);
-        Validator[] memory validatorsAfter = consensusNew.getAllValidators();
+        ConsensusV1.Validator[] memory validatorsAfter = consensusNew.getAllValidators();
         assertEq(validatorsAfter.length, 3);
 
         // Compare valdiators