fix: revert early and added lookup for validators

icon-project · Oct 8, 2024 · b5efc77 · b5efc77
1 parent 70ad755
commit b5efc77
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 29 deletions.
diff --git a/...s/javascore/cluster-connection/src/main/java/xcall/adapter/cluster/ClusterConnection.java b/...s/javascore/cluster-connection/src/main/java/xcall/adapter/cluster/ClusterConnection.java
@@ -44,13 +44,15 @@ public class ClusterConnection {
     protected final DictDB<String, BigInteger> responseFees = Context.newDictDB("responseFees", BigInteger.class);
     protected final BranchDB<String, DictDB<BigInteger, Boolean>> receipts = Context.newBranchDB("receipts",
             Boolean.class);
+    private final DictDB<Address, Boolean> validatorsLookup = Context.newDictDB("validatorsLookup", Boolean.class);
 
     public ClusterConnection(Address _relayer, Address _xCall) {
         if (xCall.get() == null) {
             xCall.set(_xCall);
             adminAddress.set(_relayer);
             connSn.set(BigInteger.ZERO);
             validators.add(_relayer);
+            validatorsLookup.set(_relayer,true);
             ValidatorAdded(_relayer);
         }
     }
@@ -72,28 +74,30 @@ public Address[] listValidators() {
     @External
     public void addValidator(Address _validator) {
         OnlyAdmin();
-        if (!validatorExists(_validator)){
-            validators.add(_validator);
-            ValidatorAdded(_validator);
-        }
+        Context.require(validatorsLookup.get(_validator)==null,"Validator already exists");
+        validators.add(_validator);
+        validatorsLookup.set(_validator,true);
+        ValidatorAdded(_validator);
     }
 
     @External
     public void removeValidator(Address _validator) {
         OnlyAdmin();
         Context.require(_validator != adminAddress.get(),"cannot remove admin");
-        if (validatorExists(_validator)){
-            Address top = this.validators.pop();
-            if (!top.equals(_validator)) {
-                for (int i = 0; i < this.validators.size(); i++) {
-                    if (_validator.equals(this.validators.get(i))) {
-                        this.validators.set(i, top);
-                        break;
-                    }
+        Context.require(validatorsLookup.get(_validator)!=null,"Validator doesn't exists");
+        Context.require((this.validators.size() - 1) >= reqValidatorCnt.get().intValue(),"Validator size less than required count after removal");
+        Address top = this.validators.pop();
+        if (!top.equals(_validator)) {
+            for (int i = 0; i < this.validators.size(); i++) {
+                if (_validator.equals(this.validators.get(i))) {
+                    this.validators.set(i, top);
+                    break;
                 }
             }
+            validatorsLookup.set(_validator,null);
             ValidatorRemoved(_validator);
         }
+
     }
 
     @EventLog(indexed = 2)
@@ -224,7 +228,7 @@ public void recvMessageWithSignatures(String srcNetwork, BigInteger _connSn, byt
          List<Address> uniqueValidators = new ArrayList<>();
          for (byte[] signature : signatures) {
              Address validator = getValidator(msg, signature);
-             Context.require(validatorExists(validator), "Invalid signature provided");
+             Context.require(validatorsLookup.get(validator)!=null, "Invalid signature provided");
              if (!uniqueValidators.contains(validator)) {
                  uniqueValidators.add(validator);
              }
@@ -233,15 +237,6 @@ public void recvMessageWithSignatures(String srcNetwork, BigInteger _connSn, byt
          recvMessage(srcNetwork, _connSn, msg);
      }
 
-    private boolean validatorExists(Address _validator) {
-        for (int i = 0; i < validators.size(); i++) {
-            if (validators.get(i).equals(_validator)) {
-                return true; 
-            }
-        }
-        return false;
-    }
-
     /**
      * Receives a message from a source network.
      *

diff --git a/...vascore/cluster-connection/src/test/java/xcall/adapter/cluster/ClusterConnectionTest.java b/...vascore/cluster-connection/src/test/java/xcall/adapter/cluster/ClusterConnectionTest.java
@@ -284,18 +284,19 @@ public void testAddSigners() throws Exception{
     @Test
     public void testAddNRemoveSigners() throws Exception{
         KeyWallet wallet = KeyWallet.create();
-        KeyWallet wallet2 = KeyWallet.create();
         KeyWallet wallet3 = KeyWallet.create();
         connection.invoke(source_relayer, "addValidator", Address.fromString(wallet.getAddress().toString()));
-        connection.invoke(source_relayer, "addValidator", Address.fromString(wallet2.getAddress().toString()));
+        connection.invoke(source_relayer, "setRequiredValidatorCount", BigInteger.TWO);
         Address[] signers = connection.call(Address[].class,"listValidators");
-        assertEquals(signers.length, 3);
+        assertEquals(signers.length, 2);
 
-        connection.invoke(source_relayer, "removeValidator", Address.fromString(wallet3.getAddress().toString()));
-        signers = connection.call(Address[].class,"listValidators");
-        assertEquals(signers.length, 3);
+        UserRevertedException e = assertThrows(UserRevertedException.class,
+                ()-> connection.invoke(source_relayer, "removeValidator", Address.fromString(wallet3.getAddress().toString())));
+        assertEquals("Reverted(0): Validator doesn't exists", e.getMessage());
 
-        connection.invoke(source_relayer, "removeValidator", Address.fromString(wallet2.getAddress().toString()));
+        UserRevertedException ex = assertThrows(UserRevertedException.class,
+                ()-> connection.invoke(source_relayer, "removeValidator", Address.fromString(wallet.getAddress().toString())));
+        assertEquals("Reverted(0): Validator size less than required count after removal", ex.getMessage());
         signers = connection.call(Address[].class,"listValidators");
         assertEquals(signers.length, 2);
     }