Merge pull request #421 from icon-project/fix/aggregator_audit

fix: aggregator audit changes

Author: sherpalden

contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java

@@ -2,6 +2,7 @@
 
 import java.math.BigInteger;
 
+import score.ByteArrayObjectWriter;
 import score.Context;
 import score.ObjectReader;
 import score.ObjectWriter;
@@ -66,8 +67,6 @@ public Packet(String srcNetwork, String srcContractAddress, BigInteger srcSn, Bi
                 || srcHeight == null || dstNetwork == null || dstContractAddress == null || data == null;
         Context.require(!isIllegalArg,
                 "srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, and data cannot be null");
-        if (isIllegalArg) {
-        }
         this.srcNetwork = srcNetwork;
         this.srcContractAddress = srcContractAddress;
         this.srcSn = srcSn;
@@ -77,12 +76,8 @@ public Packet(String srcNetwork, String srcContractAddress, BigInteger srcSn, Bi
         this.data = data;
     }
 
-    public String getId() {
-        return createId(this.srcNetwork, this.srcContractAddress, this.srcSn);
-    }
-
-    public static String createId(String srcNetwork, String contractAddress, BigInteger srcSn) {
-        return srcNetwork + "/" + contractAddress + "/" + srcSn.toString();
+    public byte[] getId() {
+        return Context.hash("sha-256", this.toBytes());
     }
 
     /**
@@ -173,4 +168,10 @@ public static Packet readObject(ObjectReader r) {
         r.end();
         return p;
     }
+
+    public byte[] toBytes() {
+        ByteArrayObjectWriter writer = Context.newByteArrayObjectWriter("RLPn");
+        Packet.writeObject(writer, this);
+        return writer.toByteArray();
+    }
 }

contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java

@@ -41,16 +41,15 @@ public class RelayAggregator {
     private final ArrayDB<Address> relayers = Context.newArrayDB("relayers", Address.class);
     private final DictDB<Address, Boolean> relayersLookup = Context.newDictDB("relayersLookup", Boolean.class);
 
-    private final DictDB<String, Packet> packets = Context.newDictDB("packets", Packet.class);
+    private final DictDB<byte[], Packet> packets = Context.newDictDB("packets", Packet.class);
 
-    private final BranchDB<String, DictDB<Address, byte[]>> signatures = Context.newBranchDB("signatures",
+    private final BranchDB<byte[], DictDB<Address, byte[]>> signatures = Context.newBranchDB("signatures",
             byte[].class);
 
     public RelayAggregator(Address _admin) {
         if (admin.get() == null) {
             admin.set(_admin);
             signatureThreshold.set(DEFAULT_SIGNATURE_THRESHOLD);
-            addRelayer(_admin);
         }
     }
 
@@ -60,12 +59,6 @@ public void setAdmin(Address _admin) {
 
         Context.require(admin.get() != _admin, "admin already set");
 
-        // add new admin as relayer
-        addRelayer(_admin);
-
-        // remove old admin from relayer list
-        removeRelayer(admin.get());
-
         admin.set(_admin);
     }
 
@@ -107,10 +100,9 @@ public void setRelayers(Address[] newRelayers, int threshold) {
                 addRelayer(newRelayer);
             }
 
-            Address adminAdrr = admin.get();
             for (int i = 0; i < relayers.size(); i++) {
                 Address oldRelayer = relayers.get(i);
-                if (!oldRelayer.equals(adminAdrr) && !newRelayersMap.containsKey(oldRelayer)) {
+                if (!newRelayersMap.containsKey(oldRelayer)) {
                     removeRelayer(oldRelayer);
                 }
             }
@@ -127,8 +119,13 @@ public boolean packetSubmitted(
             Address relayer,
             String srcNetwork,
             String srcContractAddress,
-            BigInteger srcSn) {
-        String pktID = Packet.createId(srcNetwork, srcContractAddress, srcSn);
+            BigInteger srcSn,
+            BigInteger srcHeight,
+            String dstNetwork,
+            String dstContractAddress,
+            byte[] data) {
+        Packet pkt = new Packet(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, dstContractAddress, data);
+        byte[] pktID = pkt.getId();
         byte[] existingSign = signatures.at(pktID).get(relayer);
         return existingSign != null;
     }
@@ -147,7 +144,7 @@ public void submitPacket(
         relayersOnly();
 
         Packet pkt = new Packet(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, dstContractAddress, data);
-        String pktID = pkt.getId();
+        byte[] pktID = pkt.getId();
 
         if (packets.get(pktID) == null) {
             packets.set(pktID, pkt);
@@ -170,7 +167,7 @@ public void submitPacket(
         setSignature(pktID, Context.getCaller(), signature);
 
         if (signatureThresholdReached(pktID)) {
-            byte[][] sigs = getSignatures(srcNetwork, srcContractAddress, srcSn);
+            byte[][] sigs = getSignatures(pktID);
             byte[] encodedSigs = serializeSignatures(sigs);
             PacketAcknowledged(
                     pkt.getSrcNetwork(),
@@ -185,8 +182,7 @@ public void submitPacket(
         }
     }
 
-    private byte[][] getSignatures(String srcNetwork, String srcContractAddress, BigInteger srcSn) {
-        String pktID = Packet.createId(srcNetwork, srcContractAddress, srcSn);
+    private byte[][] getSignatures(byte[] pktID) {
         DictDB<Address, byte[]> signDict = signatures.at(pktID);
         ArrayList<byte[]> signatureList = new ArrayList<byte[]>();
 
@@ -205,7 +201,7 @@ private byte[][] getSignatures(String srcNetwork, String srcContractAddress, Big
         return sigs;
     }
 
-    protected void setSignature(String pktID, Address addr, byte[] sign) {
+    protected void setSignature(byte[] pktID, Address addr, byte[] sign) {
         signatures.at(pktID).set(addr, sign);
     }
 
@@ -241,7 +237,7 @@ protected static byte[][] deserializeSignatures(byte[] encodedSigs) {
     }
 
     private void adminOnly() {
-        Context.require(Context.getCaller().equals(admin.get()), "Unauthorized: caller is not the leader relayer");
+        Context.require(Context.getCaller().equals(admin.get()), "Unauthorized: caller is not the admin");
     }
 
     private void relayersOnly() {
@@ -270,7 +266,7 @@ private void removeRelayer(Address oldRelayer) {
         }
     }
 
-    private Boolean signatureThresholdReached(String pktID) {
+    private Boolean signatureThresholdReached(byte[] pktID) {
         int noOfSignatures = 0;
         for (int i = 0; i < relayers.size(); i++) {
             Address relayer = relayers.get(i);
@@ -282,7 +278,7 @@ private Boolean signatureThresholdReached(String pktID) {
         return noOfSignatures >= signatureThreshold.get();
     }
 
-    private void removePacket(String pktID) {
+    private void removePacket(byte[] pktID) {
         packets.set(pktID, null);
         DictDB<Address, byte[]> signDict = signatures.at(pktID);