refactor(contracts): improve Consenus.sol naming and code structure (#…

…762) * Remove vote swapped * Update variables structure * Move functions * Remove updateValidator * Order functions * Use _prefixes * Rename * Renames * Rename * Rename * Use modifier errors * Validator errors * Vote errors * Range error * Layout * Remove todos * Remove block.prevrandao * Rename methods * UpdateValidator * Shuffle validators * Parameter naming * Rename blsPublicKey * Rename blsPublicKeyHash * Implement _verifyAndRegisterBlsPublicKey * Version contract * Update abi * Update crypto * Rename calculateActiveValidators * Rename getTopValidators * Update e2e * Update function sigs * Update function sigs
ArkEcosystem · Nov 15, 2024 · 348a089 · 348a089
1 parent c4f07e4
commit 348a089
Show file tree

Hide file tree

Showing 37 changed files with 2,694 additions and 2,457 deletions.
diff --git a/contracts/src/consensus/Consensus.sol b/contracts/src/consensus/Consensus.sol
diff --git a/contracts/src/consensus/ConsensusV1.sol b/contracts/src/consensus/ConsensusV1.sol
diff --git a/contracts/test/consensus/Base.sol b/contracts/test/consensus/Base.sol
@@ -2,6 +2,7 @@
 pragma solidity ^0.8.13;
 
 import {Test, console} from "@forge-std/Test.sol";
+import {Validator} from "@contracts/consensus/ConsensusV1.sol";
 
 contract Base is Test {
     function prepareBLSKey(address addr, uint8 lenght) public pure returns (bytes memory) {
@@ -16,4 +17,27 @@ contract Base is Test {
     function prepareBLSKey(address addr) public pure returns (bytes memory) {
         return prepareBLSKey(addr, 48);
     }
+
+    function sortValidators(Validator[] memory validators) public pure returns (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];
+                    validators[i] = validators[j];
+                    validators[j] = temp;
+                }
+            }
+        }
+        return validators;
+    }
+
+    function _isGreater(Validator memory validatorA, Validator memory validatorB) internal pure returns (bool) {
+        if (validatorA.data.voteBalance == validatorB.data.voteBalance) {
+            return validatorA.addr > validatorB.addr;
+        }
+
+        return validatorA.data.voteBalance > validatorB.data.voteBalance;
+    }
 }
diff --git a/contracts/test/consensus/Consensus-CalculateTop.sol b/contracts/test/consensus/Consensus-CalculateTop.sol
@@ -2,14 +2,14 @@
 pragma solidity ^0.8.13;
 
 import {Test, console} from "@forge-std/Test.sol";
-import {Consensus, ValidatorData, Validator} from "@contracts/consensus/Consensus.sol";
+import {ConsensusV1, ValidatorData, Validator, CallerIsNotOwner} from "@contracts/consensus/ConsensusV1.sol";
 import {Base} from "./Base.sol";
 
 contract ConsensusTest is Base {
-    Consensus public consensus;
+    ConsensusV1 public consensus;
 
     function setUp() public {
-        consensus = new Consensus();
+        consensus = new ConsensusV1();
     }
 
     function test_should_work_with_one_validator() public {
@@ -18,17 +18,17 @@ contract ConsensusTest is Base {
         consensus.registerValidator(prepareBLSKey(addr));
         vm.stopPrank();
 
-        consensus.calculateTopValidators(1);
-        Validator[] memory validators = consensus.getTopValidators();
+        consensus.calculateActiveValidators(1);
+        Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, 1);
         assertEq(validators[0].addr, addr);
     }
 
     function test_should_allow_only_caller() public {
         address addr = address(1);
         vm.startPrank(addr);
-        vm.expectRevert("Caller is not the contract owner");
-        consensus.calculateTopValidators(1);
+        vm.expectRevert(CallerIsNotOwner.selector);
+        consensus.calculateActiveValidators(1);
     }
 
     function test_should_ignore_resigned_validators() public {
@@ -39,8 +39,8 @@ contract ConsensusTest is Base {
         consensus.resignValidator();
         vm.stopPrank();
 
-        consensus.calculateTopValidators(1);
-        Validator[] memory validators = consensus.getTopValidators();
+        consensus.calculateActiveValidators(1);
+        Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, 0);
     }
 
@@ -79,14 +79,19 @@ contract ConsensusTest is Base {
 
         uint160 activeValidators = 53;
 
-        consensus.calculateTopValidators(uint8(activeValidators));
-        Validator[] memory validators = consensus.getTopValidators();
+        consensus.calculateActiveValidators(uint8(activeValidators));
+        Validator[] memory validators = consensus.getActiveValidators();
         assertEq(validators.length, activeValidators);
+
+        assertEq(validators[activeValidators - 1].addr, address(0xAE)); // Shuffled address
+        validators = sortValidators(validators);
         assertEq(validators[activeValidators - 1].addr, highest);
 
-        consensus.calculateTopValidators(uint8(activeValidators));
+        consensus.calculateActiveValidators(uint8(activeValidators));
 
-        validators = consensus.getTopValidators();
+        validators = consensus.getActiveValidators();
+        assertEq(validators[activeValidators - 1].addr, address(0xAE)); // Shuffled address
+        validators = sortValidators(validators);
         assertEq(validators.length, activeValidators);
         assertEq(validators[activeValidators - 1].addr, highest);
     }

diff --git a/contracts/test/consensus/Consensus-GetAllValidators.sol b/contracts/test/consensus/Consensus-GetAllValidators.sol
@@ -2,14 +2,14 @@
 pragma solidity ^0.8.13;
 
 import {Test, console} from "@forge-std/Test.sol";
-import {Consensus, ValidatorData, Validator} from "@contracts/consensus/Consensus.sol";
+import {ConsensusV1, ValidatorData, Validator} from "@contracts/consensus/ConsensusV1.sol";
 import {Base} from "./Base.sol";
 
 contract ConsensusTest is Base {
-    Consensus public consensus;
+    ConsensusV1 public consensus;
 
     function setUp() public {
-        consensus = new Consensus();
+        consensus = new ConsensusV1();
     }
 
     function test_200_validators() public {

diff --git a/contracts/test/consensus/Consensus-Rounds.sol b/contracts/test/consensus/Consensus-Rounds.sol
@@ -2,20 +2,20 @@
 pragma solidity ^0.8.13;
 
 import {Test, console} from "@forge-std/Test.sol";
-import {Consensus, Round} from "@contracts/consensus/Consensus.sol";
+import {ConsensusV1, Round, CallerIsNotOwner} from "@contracts/consensus/ConsensusV1.sol";
 import {Base} from "./Base.sol";
 
 contract ConsensusTest is Base {
-    Consensus public consensus;
+    ConsensusV1 public consensus;
 
     function setUp() public {
-        consensus = new Consensus();
+        consensus = new ConsensusV1();
     }
 
     function test_revert_if_caller_is_not_owner() public {
         vm.startPrank(address(1));
 
-        vm.expectRevert("Caller is not the contract owner");
+        vm.expectRevert(CallerIsNotOwner.selector);
         consensus.getRounds(0, 10);
     }
 
@@ -30,7 +30,7 @@ contract ConsensusTest is Base {
         consensus.registerValidator(prepareBLSKey(addr));
         vm.stopPrank();
 
-        consensus.calculateTopValidators(1);
+        consensus.calculateActiveValidators(1);
 
         assertEq(consensus.getRoundsCount(), 1);
         Round[] memory rounds = consensus.getRounds(0, 10);
@@ -47,7 +47,7 @@ contract ConsensusTest is Base {
         vm.stopPrank();
 
         // Round 1
-        consensus.calculateTopValidators(1);
+        consensus.calculateActiveValidators(1);
         assertEq(consensus.getRoundsCount(), 1);
         Round[] memory rounds = consensus.getRounds(0, 10);
         assertEq(rounds.length, 1);
@@ -64,7 +64,7 @@ contract ConsensusTest is Base {
         vm.stopPrank();
 
         // Round 2
-        consensus.calculateTopValidators(1);
+        consensus.calculateActiveValidators(1);
         assertEq(consensus.getRoundsCount(), 2);
         rounds = consensus.getRounds(0, 10);
         assertEq(rounds.length, 2);
@@ -85,7 +85,7 @@ contract ConsensusTest is Base {
         vm.stopPrank();
 
         // Round 3
-        consensus.calculateTopValidators(1);
+        consensus.calculateActiveValidators(1);
         assertEq(consensus.getRoundsCount(), 3);
         rounds = consensus.getRounds(0, 10);
         assertEq(rounds.length, 3);
@@ -110,9 +110,9 @@ contract ConsensusTest is Base {
         vm.stopPrank();
 
         // Create 3 rounds
-        consensus.calculateTopValidators(1);
-        consensus.calculateTopValidators(1);
-        consensus.calculateTopValidators(1);
+        consensus.calculateActiveValidators(1);
+        consensus.calculateActiveValidators(1);
+        consensus.calculateActiveValidators(1);
 
         // Assert rounds count
         assertEq(consensus.getRoundsCount(), 3);

diff --git a/contracts/test/consensus/Consensus-UpdateVoters.sol b/contracts/test/consensus/Consensus-UpdateVoters.sol
@@ -2,20 +2,27 @@
 pragma solidity ^0.8.13;
 
 import {Test, console} from "@forge-std/Test.sol";
-import {Consensus, ValidatorData, Validator, Unvoted, Voted} from "@contracts/consensus/Consensus.sol";
+import {
+    ConsensusV1,
+    ValidatorData,
+    Validator,
+    Unvoted,
+    Voted,
+    CallerIsNotOwner
+} from "@contracts/consensus/ConsensusV1.sol";
 
 contract ConsensusTest is Test {
-    Consensus public consensus;
+    ConsensusV1 public consensus;
 
     function setUp() public {
-        consensus = new Consensus();
+        consensus = new ConsensusV1();
     }
 
     function test_updateVoters_should_allow_only_caller() public {
         address addr = address(1);
         vm.startPrank(addr);
         address[] memory voters = new address[](0);
-        vm.expectRevert("Caller is not the contract owner");
+        vm.expectRevert(CallerIsNotOwner.selector);
         consensus.updateVoters(voters);
     }
 }
diff --git a/contracts/test/consensus/Consensus-ValidatoUpdate.sol b/contracts/test/consensus/Consensus-ValidatoUpdate.sol
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GNU GENERAL PUBLIC LICENSE
+pragma solidity ^0.8.13;
+
+import {
+    ConsensusV1,
+    ValidatorData,
+    Validator,
+    ValidatorUpdated,
+    CallerIsOwner,
+    ValidatorAlreadyRegistered,
+    BlsKeyAlreadyRegistered,
+    BlsKeyIsInvalid,
+    CallerIsNotValidator
+} from "@contracts/consensus/ConsensusV1.sol";
+import {Base} from "./Base.sol";
+
+contract ConsensusTest is Base {
+    ConsensusV1 public consensus;
+
+    function setUp() public {
+        consensus = new ConsensusV1();
+    }
+
+    function test_updateBlsPublicKey_revert_if_caller_is_not_validator() public {
+        vm.expectRevert(CallerIsNotValidator.selector);
+        consensus.resignValidator();
+    }
+
+    function test_updateBlsPublicKey_revert_if_bls_key_is_already_registered() public {
+        address addr = address(1);
+        vm.startPrank(addr);
+
+        consensus.registerValidator(prepareBLSKey(addr));
+
+        vm.expectRevert(BlsKeyAlreadyRegistered.selector);
+        consensus.updateValidator(prepareBLSKey(addr));
+    }
+
+    function test_updateBlsPublicKey_revert_if_bls_key_is_already_registered_by_different_vlidator() public {
+        address addr = address(1);
+        vm.startPrank(addr);
+        consensus.registerValidator(prepareBLSKey(addr));
+
+        address addr2 = address(2);
+        vm.startPrank(addr2);
+        consensus.registerValidator(prepareBLSKey(addr2));
+
+        vm.expectRevert(BlsKeyAlreadyRegistered.selector);
+        consensus.updateValidator(prepareBLSKey(addr));
+    }
+
+    function test_updateBlsPublicKey_revert_if_bls_key_is_invalid() public {
+        address addr = address(1);
+        vm.startPrank(addr);
+        consensus.registerValidator(prepareBLSKey(addr));
+
+        vm.expectRevert(BlsKeyIsInvalid.selector);
+        consensus.updateValidator(prepareBLSKey(addr, 46));
+        vm.expectRevert(BlsKeyIsInvalid.selector);
+        consensus.updateValidator(prepareBLSKey(addr, 47));
+        vm.expectRevert(BlsKeyIsInvalid.selector);
+        consensus.updateValidator(prepareBLSKey(addr, 49));
+        vm.expectRevert(BlsKeyIsInvalid.selector);
+        consensus.updateValidator(prepareBLSKey(addr, 50));
+    }
+
+    function test_updateBlsPublicKey_should_pass() public {
+        address addr = address(1);
+        vm.startPrank(addr);
+        consensus.registerValidator(prepareBLSKey(addr));
+
+        vm.expectEmit(address(consensus));
+        emit ValidatorUpdated(addr, prepareBLSKey(address(2)));
+        consensus.updateValidator(prepareBLSKey(address(2)));
+
+        Validator memory validator = consensus.getValidator(addr);
+        assertEq(validator.addr, addr);
+        assertEq(validator.data.blsPublicKey, prepareBLSKey(address(2)));
+    }
+
+    function test_updateBlsPublicKey_revert_on_second_update() public {
+        address addr = address(1);
+        vm.startPrank(addr);
+        consensus.registerValidator(prepareBLSKey(addr));
+
+        vm.expectEmit(address(consensus));
+        emit ValidatorUpdated(addr, prepareBLSKey(address(2)));
+        consensus.updateValidator(prepareBLSKey(address(2)));
+
+        Validator memory validator = consensus.getValidator(addr);
+        assertEq(validator.addr, addr);
+        assertEq(validator.data.blsPublicKey, prepareBLSKey(address(2)));
+
+        vm.expectRevert(BlsKeyAlreadyRegistered.selector);
+        consensus.updateValidator(prepareBLSKey(address(2)));
+    }
+}