Skip to content

Commit

Permalink
fix: add setRelayers method
Browse files Browse the repository at this point in the history
  • Loading branch information
@sherpalden
sherpalden committed Oct 25, 2024
1 parent 08508f5 commit 7034223
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 71 deletions.
59 changes: 33 additions & 26 deletions contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import score.annotation.External;
import score.ObjectReader;
import scorex.util.ArrayList;
import scorex.util.HashMap;

public class RelayAggregator {
private final Integer DEFAULT_SIGNATURE_THRESHOLD = 1;
Expand Down Expand Up @@ -57,6 +58,8 @@ public RelayAggregator(Address _admin) {
public void setAdmin(Address _admin) {
adminOnly();

Context.require(admin.get() != _admin, "admin already set");

// add new admin as relayer
addRelayer(_admin);

Expand All @@ -74,7 +77,8 @@ public Address getAdmin() {
@External
public void setSignatureThreshold(int threshold) {
adminOnly();
Context.require(threshold >= 1, "invalid threshold value: should be >= 1");
Context.require(threshold > 0 && threshold <= relayers.size(),
"threshold value should be at least 1 and not greater than relayers size");
signatureThreshold.set(threshold);
}

Expand All @@ -93,30 +97,29 @@ public Address[] getRelayers() {
}

@External
public void addRelayers(Address[] newRelayers) {
public void setRelayers(Address[] newRelayers, int threshold) {
adminOnly();

Context.require(newRelayers != null && newRelayers.length != 0, "new relayers cannot be empty");

for (Address newRelayer : newRelayers) {
Boolean exits = relayersLookup.get(newRelayer);
if (exits == null) {
if (newRelayers.length > 0) {
HashMap<Address, Boolean> newRelayersMap = new HashMap<Address, Boolean>();
for (Address newRelayer : newRelayers) {
newRelayersMap.put(newRelayer, true);
addRelayer(newRelayer);
}
}
}

@External
public void removeRelayers(Address[] relayersToBeRemoved) {
adminOnly();
Address adminAdrr = admin.get();
for (int i = 0; i < relayers.size(); i++) {
Address oldRelayer = relayers.get(i);
if (!oldRelayer.equals(adminAdrr) && !newRelayersMap.containsKey(oldRelayer)) {
removeRelayer(oldRelayer);
}
}
}

Context.require(relayersToBeRemoved != null && relayersToBeRemoved.length != 0,
"relayers to be removed cannot be empty");
Context.require(threshold > 0 && threshold <= relayers.size(),
"threshold value should be at least 1 and not greater than relayers size");

for (Address relayerToBeRemoved : relayersToBeRemoved) {
Context.require(relayerToBeRemoved != admin.get(), "admin cannot be removed from relayers list");
removeRelayer(relayerToBeRemoved);
}
signatureThreshold.set(threshold);
}

@External(readonly = true)
Expand Down Expand Up @@ -248,17 +251,21 @@ private void relayersOnly() {
}

private void addRelayer(Address newRelayer) {
relayers.add(newRelayer);
relayersLookup.set(newRelayer, true);
if (relayersLookup.get(newRelayer) == null) {
relayers.add(newRelayer);
relayersLookup.set(newRelayer, true);
}
}

private void removeRelayer(Address oldRelayer) {
relayersLookup.set(oldRelayer, null);
Address top = relayers.pop();
for (int i = 0; i < relayers.size(); i++) {
if (oldRelayer.equals(relayers.get(i))) {
relayers.set(i, top);
break;
if (relayersLookup.get(oldRelayer)) {
relayersLookup.set(oldRelayer, null);
Address top = relayers.pop();
for (int i = 0; i < relayers.size(); i++) {
if (oldRelayer.equals(relayers.get(i))) {
relayers.set(i, top);
break;
}
}
}
}
Expand Down
125 changes: 80 additions & 45 deletions contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import score.Address;
import score.Context;
import score.UserRevertedException;
import scorex.util.HashSet;

import com.iconloop.score.test.Account;
import com.iconloop.score.test.Score;
Expand Down Expand Up @@ -69,7 +70,7 @@ void setup() throws Exception {
Address[] relayers = new Address[] { adminAc.getAddress(), relayerOneAc.getAddress(), relayerTwoAc.getAddress(),
relayerThreeAc.getAddress() };

aggregator.invoke(adminAc, "addRelayers", (Object) relayers);
aggregator.invoke(adminAc, "setRelayers", (Object) relayers, 2);

aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance());
aggregator.setInstance(aggregatorSpy);
Expand Down Expand Up @@ -118,68 +119,91 @@ public void testSetSignatureThreshold() {
public void testSetSignatureThreshold_unauthorised() {
int threshold = 3;

Executable action = () -> aggregator.invoke(relayerOneAc, "setSignatureThreshold", threshold);
Executable action = () -> aggregator.invoke(relayerOneAc,
"setSignatureThreshold", threshold);
UserRevertedException e = assertThrows(UserRevertedException.class, action);

assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage());
assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer",
e.getMessage());
}

@Test
public void testAddRelayers() {
public void testSetRelayers() {
Account relayerFiveAc = sm.createAccount();
Address[] newRelayers = new Address[] { relayerFourAc.getAddress(), relayerFiveAc.getAddress() };
Address[] newRelayers = new Address[] { relayerThreeAc.getAddress(), relayerFourAc.getAddress(),
relayerFiveAc.getAddress() };

aggregator.invoke(adminAc, "addRelayers", (Object) newRelayers);
Integer threshold = 3;
aggregator.invoke(adminAc, "setRelayers", (Object) newRelayers, threshold);

Address[] updatedRelayers = (Address[]) aggregator.call("getRelayers");

assertTrue(updatedRelayers[updatedRelayers.length - 1].equals(relayerFiveAc.getAddress()));
assertTrue(updatedRelayers[updatedRelayers.length - 2].equals(relayerFourAc.getAddress()));
Address[] expectedRelayers = new Address[] { adminAc.getAddress(), relayerThreeAc.getAddress(),
relayerFourAc.getAddress(),
relayerFiveAc.getAddress() };

HashSet<Address> updatedRelayersSet = new HashSet<Address>();
for (Address rlr : updatedRelayers) {
updatedRelayersSet.add(rlr);
}

HashSet<Address> expectedRelayersSet = new HashSet<Address>();
for (Address rlr : expectedRelayers) {
expectedRelayersSet.add(rlr);
}

assertEquals(expectedRelayersSet, updatedRelayersSet);

Integer result = (Integer) aggregator.call("getSignatureThreshold");
assertEquals(threshold, result);
}

@Test
public void testAddRelayers_unauthorised() {
public void testSetRelayers_unauthorized() {
Account relayerFiveAc = sm.createAccount();
Address[] newRelayers = new Address[] { relayerFourAc.getAddress(), relayerFiveAc.getAddress() };
Address[] newRelayers = new Address[] { relayerThreeAc.getAddress(), relayerFourAc.getAddress(),
relayerFiveAc.getAddress() };

Executable action = () -> aggregator.invoke(relayerOneAc, "addRelayers", (Object) newRelayers);
Integer threshold = 3;
Executable action = () -> aggregator.invoke(relayerOneAc, "setRelayers",
(Object) newRelayers, threshold);
UserRevertedException e = assertThrows(UserRevertedException.class, action);

assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage());
assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer",
e.getMessage());

}

@Test
public void testRemoveRelayers() {
Address[] relayerToBeRemoved = new Address[] { relayerOneAc.getAddress(),
relayerTwoAc.getAddress() };

aggregator.invoke(adminAc, "removeRelayers", (Object) relayerToBeRemoved);
public void testSetRelayers_invalidThreshold() {
Account relayerFiveAc = sm.createAccount();
Address[] newRelayers = new Address[] { relayerThreeAc.getAddress(), relayerFourAc.getAddress(),
relayerFiveAc.getAddress() };

Address[] updatedRelayers = (Address[]) aggregator.call("getRelayers");
Integer threshold = 5;
Executable action = () -> aggregator.invoke(adminAc, "setRelayers",
(Object) newRelayers, threshold);
UserRevertedException e = assertThrows(UserRevertedException.class, action);

Boolean removed = true;
for (Address rlr : updatedRelayers) {
if (rlr.equals(relayerOneAc.getAddress()) || rlr.equals(relayerTwoAc.getAddress())) {
removed = false;
break;
}
}
assertEquals("Reverted(0): threshold value should be at least 1 and not greater than relayers size",
e.getMessage());

assertTrue(removed);
assertEquals(updatedRelayers[1], relayerThreeAc.getAddress());
}

@Test
public void testRemoveRelayers_unauthorised() {
Address[] relayerToBeRemoved = new Address[] { relayerOneAc.getAddress(),
relayerTwoAc.getAddress() };

aggregator.invoke(adminAc, "removeRelayers", (Object) relayerToBeRemoved);
public void testSetRelayers_invalidThresholdZero() {
Account relayerFiveAc = sm.createAccount();
Address[] newRelayers = new Address[] { relayerThreeAc.getAddress(), relayerFourAc.getAddress(),
relayerFiveAc.getAddress() };

Executable action = () -> aggregator.invoke(relayerFourAc, "removeRelayers", (Object) relayerToBeRemoved);
Integer threshold = 0;
Executable action = () -> aggregator.invoke(adminAc, "setRelayers",
(Object) newRelayers, threshold);
UserRevertedException e = assertThrows(UserRevertedException.class, action);

assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage());
assertEquals("Reverted(0): threshold value should be at least 1 and not greater than relayers size",
e.getMessage());

}

@Test
Expand All @@ -197,11 +221,13 @@ public void testPacketSubmitted_true() throws Exception {
byte[] dataHash = Context.hash("sha-256", data);
byte[] sign = relayerOne.sign(dataHash);

aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork,
srcContractAddress, srcSn, srcHeight, dstNetwork,
dstContractAddress, data,
sign);

boolean submitted = (boolean) aggregator.call("packetSubmitted", relayerOneAc.getAddress(), srcNetwork,
boolean submitted = (boolean) aggregator.call("packetSubmitted",
relayerOneAc.getAddress(), srcNetwork,
srcContractAddress, srcSn);
assertEquals(submitted, true);
}
Expand All @@ -212,7 +238,8 @@ public void testPacketSubmitted_false() throws Exception {
BigInteger srcSn = BigInteger.ONE;
String srcContractAddress = "hxjuiod";

boolean submitted = (boolean) aggregator.call("packetSubmitted", relayerOneAc.getAddress(), srcNetwork,
boolean submitted = (boolean) aggregator.call("packetSubmitted",
relayerOneAc.getAddress(), srcNetwork,
srcContractAddress, srcSn);
assertEquals(submitted, false);
}
Expand All @@ -232,12 +259,14 @@ public void testSubmitPacket() throws Exception {
byte[] dataHash = Context.hash("sha-256", data);
byte[] sign = relayerOne.sign(dataHash);

aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork,
srcContractAddress, srcSn, srcHeight, dstNetwork,
dstContractAddress, data,
sign);

String pktID = Packet.createId(srcNetwork, srcContractAddress, srcSn);
verify(aggregatorSpy).PacketRegistered(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
verify(aggregatorSpy).PacketRegistered(srcNetwork, srcContractAddress, srcSn,
srcHeight, dstNetwork,
dstContractAddress, data);
verify(aggregatorSpy).setSignature(pktID, relayerOneAc.getAddress(), sign);
}
Expand All @@ -257,12 +286,14 @@ public void testSubmitPacket_thresholdReached() throws Exception {
byte[] dataHash = Context.hash("sha-256", data);

byte[] signAdmin = admin.sign(dataHash);
aggregator.invoke(adminAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
aggregator.invoke(adminAc, "submitPacket", srcNetwork, srcContractAddress,
srcSn, srcHeight, dstNetwork,
dstContractAddress, data,
signAdmin);

byte[] signOne = relayerOne.sign(dataHash);
aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork,
srcContractAddress, srcSn, srcHeight, dstNetwork,
dstContractAddress,
data,
signOne);
Expand All @@ -277,7 +308,8 @@ public void testSubmitPacket_thresholdReached() throws Exception {
assertArrayEquals(signAdmin, decodedSigs[0]);
assertArrayEquals(signOne, decodedSigs[1]);

verify(aggregatorSpy).PacketAcknowledged(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
verify(aggregatorSpy).PacketAcknowledged(srcNetwork, srcContractAddress,
srcSn, srcHeight, dstNetwork,
dstContractAddress, data,
encodedSigs);
}
Expand All @@ -295,7 +327,8 @@ public void testSubmitPacket_unauthorized() throws Exception {
byte[] dataHash = Context.hash("sha-256", data);
byte[] sign = relayerFour.sign(dataHash);

Executable action = () -> aggregator.invoke(relayerFourAc, "submitPacket", srcNetwork, srcContractAddress,
Executable action = () -> aggregator.invoke(relayerFourAc, "submitPacket",
srcNetwork, srcContractAddress,
srcSn,
srcHeight, dstNetwork, dstContractAddress, data, sign);

Expand All @@ -319,10 +352,12 @@ public void testSubmitPacket_duplicate() throws Exception {
byte[] dataHash = Context.hash("sha-256", data);
byte[] sign = relayerOne.sign(dataHash);

aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork,
aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork,
srcContractAddress, srcSn, srcHeight, dstNetwork,
dstContractAddress, data, sign);

Executable action = () -> aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn,
Executable action = () -> aggregator.invoke(relayerOneAc, "submitPacket",
srcNetwork, srcContractAddress, srcSn,
srcHeight, dstNetwork, dstContractAddress,
data, sign);
;
Expand Down

0 comments on commit 7034223

Please sign in to comment.