From d4b172bac75f7984cb7f152006049cb98085fc71 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Tue, 17 Sep 2024 18:56:49 +0545 Subject: [PATCH 01/18] feat: register packet in aggregator --- contracts/javascore/aggregator/build.gradle | 43 ++++++++++ .../relay/aggregator/RelayAggregator.java | 86 +++++++++++++++++++ contracts/javascore/settings.gradle | 3 +- 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 contracts/javascore/aggregator/build.gradle create mode 100644 contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java diff --git a/contracts/javascore/aggregator/build.gradle b/contracts/javascore/aggregator/build.gradle new file mode 100644 index 00000000..f8881dcc --- /dev/null +++ b/contracts/javascore/aggregator/build.gradle @@ -0,0 +1,43 @@ +version = '0.1.0' + +dependencies { + testImplementation 'foundation.icon:javaee-unittest:0.11.1' + testImplementation project(':test-lib') +} + +test { + useJUnitPlatform() + finalizedBy jacocoTestReport +} + +optimizedJar { + mainClassName = 'relay.aggregator.RelayAggregator' + duplicatesStrategy = DuplicatesStrategy.EXCLUDE + from { + configurations.runtimeClasspath.collect { it.isDirectory() ? it : zipTree(it) } + } +} + +deployJar { + endpoints { + lisbon { + uri = 'https://lisbon.net.solidwallet.io/api/v3' + nid = 0x2 + } + local { + uri = 'http://localhost:9082/api/v3' + nid = 0x3 + } + mainnet { + uri = 'https://ctz.solidwallet.io/api/v3' + nid = 0x1 + } + } + keystore = rootProject.hasProperty('keystoreName') ? "$keystoreName" : '' + password = rootProject.hasProperty('keystorePass') ? "$keystorePass" : '' + parameters { + arg('_admin', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fd") + arg('_relayers', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fd") + arg('_relayers', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fe") + } +} \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java new file mode 100644 index 00000000..1364a573 --- /dev/null +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -0,0 +1,86 @@ +/* + * Copyright 2022 ICON Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package relay.aggregator; + +import score.Context; + +import java.math.BigInteger; + +import score.Address; +import score.VarDB; +import score.DictDB; +import score.BranchDB; + +import score.annotation.External; + +public class RelayAggregator { + protected final VarDB
admin = Context.newVarDB("admin", Address.class); + + protected final DictDB relayers = Context.newDictDB("relayers", Boolean.class); + + protected final BranchDB> packets = Context.newBranchDB("packets", byte[].class); + + // protected final BranchDB> signatures = Context.newBranchDB("signatures", String[].class); + + public RelayAggregator(Address _admin, Address[] _relayers) { + if (admin.get() == null) { + admin.set(_admin); + for (Address relayer : _relayers) { + relayers.set(relayer, true); + } + } + } + + /** + * Registers a new packet. + * + * @param nid the network ID + * @param sn the sequence number + * @param data the packet data + */ + @External + public void registerPacket(String nid, BigInteger sn, byte[] data) { + adminOnly(); + DictDB packetDict = packets.at(nid); + byte[] pkt = packetDict.get(sn); + Context.require(pkt == null, "Packet already exists."); + + packetDict.set(sn, data); + } + + + /** + * Sets the admin address. + * + * @param _admin the new admin address + */ + @External + public void setAdmin(Address _admin) { + adminOnly(); + admin.set(_admin); + } + + + /** + * Checks if the caller of the function is the admin. + * + * @return true if the caller is the admin, false otherwise + */ + private void adminOnly() { + Context.require(Context.getCaller().equals(admin.get()), "Unauthorized to call this method"); + } +} \ No newline at end of file diff --git a/contracts/javascore/settings.gradle b/contracts/javascore/settings.gradle index 6e6fdcdf..243072ad 100644 --- a/contracts/javascore/settings.gradle +++ b/contracts/javascore/settings.gradle @@ -3,7 +3,8 @@ include( 'test-lib', 'xcall', 'xcall-lib', - 'centralized-connection' + 'centralized-connection', + 'aggregator' ) include(':dapp-simple') From 46c202e51980b9f244b2ec88f171c0c112c27b04 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Tue, 17 Sep 2024 23:25:00 +0545 Subject: [PATCH 02/18] feat: add unit tests for aggregator --- .../relay/aggregator/RelayAggregator.java | 9 ++++ .../relay/aggregator/RelayAggregatorTest.java | 52 +++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 1364a573..81cfe28d 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -74,6 +74,15 @@ public void setAdmin(Address _admin) { admin.set(_admin); } + /** + * Retrieves the admin address. + * + * @return The admin address. + */ + @External(readonly = true) + public Address getAdmin() { + return admin.get(); + } /** * Checks if the caller of the function is the admin. diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java new file mode 100644 index 00000000..50dcb21f --- /dev/null +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -0,0 +1,52 @@ +package relay.aggregator; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +import score.Address; +import score.UserRevertedException; + +import com.iconloop.score.test.Account; +import com.iconloop.score.test.Score; +import com.iconloop.score.test.ServiceManager; +import com.iconloop.score.test.TestBase; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class RelayAggregatorTest extends TestBase { + protected final ServiceManager sm = getServiceManager(); + + protected final Account adminAc = sm.createAccount(); + protected final Account relayerAcOne = sm.createAccount(); + protected final Account relayerAcTwo = sm.createAccount(); + + protected Score aggregator; + + @BeforeEach + void setup() throws Exception { + Address[] relayers = new Address[]{relayerAcOne.getAddress(), relayerAcTwo.getAddress()}; + aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); + } + + @Test + public void testSetAdmin() { + Account newAdminAc = sm.createAccount(); + aggregator.invoke(adminAc, "setAdmin", newAdminAc.getAddress()); + + Address result = (Address) aggregator.call("getAdmin"); + assertEquals(newAdminAc.getAddress(), result); + } + + @Test + public void testSetAdmin_unauthorized() { + Account normalAc = sm.createAccount(); + Account newAdminAc = sm.createAccount(); + + Executable action = () -> aggregator.invoke(normalAc, "setAdmin", newAdminAc.getAddress()); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + + assertEquals("Reverted(0): Unauthorized to call this method", e.getMessage()); + } +} From 395acc78f01174a0f02d552dfd534f4e13b64d1b Mon Sep 17 00:00:00 2001 From: sherpalden Date: Mon, 23 Sep 2024 18:04:11 +0545 Subject: [PATCH 03/18] test: submitSignature --- contracts/javascore/aggregator/build.gradle | 5 + .../main/java/relay/aggregator/HexString.java | 26 ++++ .../relay/aggregator/RelayAggregator.java | 85 +++++++++-- .../relay/aggregator/RelayAggregatorTest.java | 140 +++++++++++++++++- contracts/javascore/keystore.json | 1 + 5 files changed, 239 insertions(+), 18 deletions(-) create mode 100644 contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java create mode 100644 contracts/javascore/keystore.json diff --git a/contracts/javascore/aggregator/build.gradle b/contracts/javascore/aggregator/build.gradle index f8881dcc..0704d410 100644 --- a/contracts/javascore/aggregator/build.gradle +++ b/contracts/javascore/aggregator/build.gradle @@ -1,6 +1,11 @@ version = '0.1.0' +repositories { + mavenCentral() +} + dependencies { + implementation 'org.bouncycastle:bcprov-jdk15on:1.70' testImplementation 'foundation.icon:javaee-unittest:0.11.1' testImplementation project(':test-lib') } diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java new file mode 100644 index 00000000..6c12e466 --- /dev/null +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java @@ -0,0 +1,26 @@ +package relay.aggregator; + +public class HexString { + public static byte[] toBytes(String hex) { + int len = hex.length(); + byte[] data = new byte[len / 2]; + + for (int i = 0; i < len; i += 2) { + data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + + Character.digit(hex.charAt(i + 1), 16)); + } + return data; + } + + public static String fromBytes(byte[] bytes) { + StringBuilder hexString = new StringBuilder(); + for (byte b : bytes) { + String hex = Integer.toHexString(0xff & b); + if (hex.length() == 1) { + hexString.append('0'); // Append leading zero if needed + } + hexString.append(hex); + } + return hexString.toString(); + } +} diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 81cfe28d..a7a5e979 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -16,10 +16,10 @@ package relay.aggregator; -import score.Context; import java.math.BigInteger; +import score.Context; import score.Address; import score.VarDB; import score.DictDB; @@ -28,13 +28,13 @@ import score.annotation.External; public class RelayAggregator { - protected final VarDB
admin = Context.newVarDB("admin", Address.class); + private final VarDB
admin = Context.newVarDB("admin", Address.class); - protected final DictDB relayers = Context.newDictDB("relayers", Boolean.class); + private final DictDB relayers = Context.newDictDB("relayers", Boolean.class); - protected final BranchDB> packets = Context.newBranchDB("packets", byte[].class); + private final BranchDB> packets = Context.newBranchDB("packets", byte[].class); - // protected final BranchDB> signatures = Context.newBranchDB("signatures", String[].class); + private final BranchDB>> signatures = Context.newBranchDB("signatures", DictDB.class); public RelayAggregator(Address _admin, Address[] _relayers) { if (admin.get() == null) { @@ -48,20 +48,60 @@ public RelayAggregator(Address _admin, Address[] _relayers) { /** * Registers a new packet. * - * @param nid the network ID - * @param sn the sequence number - * @param data the packet data + * @param nid network ID + * @param sn sequence number + * @param data packet data */ @External public void registerPacket(String nid, BigInteger sn, byte[] data) { adminOnly(); - DictDB packetDict = packets.at(nid); + DictDB packetDict = getPackets(nid); byte[] pkt = packetDict.get(sn); - Context.require(pkt == null, "Packet already exists."); + Context.require(pkt == null, "Packet already exists"); packetDict.set(sn, data); } + /** + * Submits a signature for the registered packet. + * + * @param nid network ID + * @param sn sequence number + * @param signature packet signature + */ + @External + public void submitSignature(String nid, BigInteger sn, String signature) { + relayersOnly(); + + DictDB packetDict = getPackets(nid); + byte[] packetData = packetDict.get(sn); + Context.require(packetData != null, "Packet does not exist"); + + + DictDB addressSigns = getSignatures(nid, sn); + + Address caller = Context.getCaller(); + String sign = addressSigns.get(caller); + Context.require(sign == null, "Signature already exists."); + + addressSigns.set(caller, signature); + } + + /** + * Retrieves the signatures dictionary for the packet. + * + * @param nid network ID of the source chain + * @param sn sequence number of the source chain message + * @return map of relayer addresses to signatures + */ + protected DictDB getSignatures(String nid, BigInteger sn) { + BranchDB> snSigns = signatures.at(nid); + if (snSigns == null) { + snSigns = Context.newBranchDB(nid+"-signs", DictDB.class); + } + return snSigns.at(sn); + } + /** * Sets the admin address. @@ -77,19 +117,40 @@ public void setAdmin(Address _admin) { /** * Retrieves the admin address. * - * @return The admin address. + * @return admin address. */ @External(readonly = true) public Address getAdmin() { return admin.get(); } + /** + * Retrieves the packets dictionary. + * * + * @param nid network id of the source chain + * @return list of mapping from sn to packet data. + */ + protected DictDB getPackets(String nid) { + return packets.at(nid); + } + /** * Checks if the caller of the function is the admin. * * @return true if the caller is the admin, false otherwise */ private void adminOnly() { - Context.require(Context.getCaller().equals(admin.get()), "Unauthorized to call this method"); + Context.require(Context.getCaller().equals(admin.get()), "Unauthorized: caller is not the leader relayer"); + } + + /** + * Checks if the caller of the function is among the relayers. + * + * @return true if the caller is among the relayers, otherwise false + */ + private void relayersOnly() { + Address caller = Context.getCaller(); + boolean isRelayer = relayers.get(caller); + Context.require(isRelayer, "Unauthorized: caller is not a registered relayer"); } } \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index 50dcb21f..cb2784ce 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -1,11 +1,25 @@ package relay.aggregator; +import java.math.BigInteger; +import java.security.MessageDigest; +import java.security.PrivateKey; +import java.security.Security; +import java.security.AlgorithmParameters; +import java.security.KeyFactory; +import java.security.Signature; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import java.security.spec.ECPrivateKeySpec; +import java.security.spec.ECGenParameterSpec; +import java.security.spec.ECParameterSpec; + import score.Address; import score.UserRevertedException; +import score.DictDB; import com.iconloop.score.test.Account; import com.iconloop.score.test.Score; @@ -14,20 +28,34 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class RelayAggregatorTest extends TestBase { - protected final ServiceManager sm = getServiceManager(); + private final ServiceManager sm = getServiceManager(); + + private final Account adminAc = sm.createAccount(); - protected final Account adminAc = sm.createAccount(); - protected final Account relayerAcOne = sm.createAccount(); - protected final Account relayerAcTwo = sm.createAccount(); + private final Account relayerAcOne = sm.getAccount(new Address(HexString.toBytes("hxe794bcf6a92efb5e0b58cfc4728236c650d86dce"))); + private final String relayerAcOnePkey = "40b90166d6ace4acbdc59596fd483e487bf58ec4ae5ff31e4f97f039af5b23f7"; + + private final Account relayerAcTwo = sm.getAccount(new Address(HexString.toBytes("hx3445c3d4341a9b1fc2ae6fd578a4453ab0072c07"))); + private final String relayerAcTwoPkey = "0xb533387c502c39eea62f4c2a5be31e388fa87313438236656b4c71be92fed066"; - protected Score aggregator; + private Score aggregator; + private RelayAggregator aggregatorSpy; @BeforeEach void setup() throws Exception { Address[] relayers = new Address[]{relayerAcOne.getAddress(), relayerAcTwo.getAddress()}; aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); + + aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance()); + aggregator.setInstance(aggregatorSpy); + + Security.addProvider(new BouncyCastleProvider()); } @Test @@ -47,6 +75,106 @@ public void testSetAdmin_unauthorized() { Executable action = () -> aggregator.invoke(normalAc, "setAdmin", newAdminAc.getAddress()); UserRevertedException e = assertThrows(UserRevertedException.class, action); - assertEquals("Reverted(0): Unauthorized to call this method", e.getMessage()); + assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage()); + } + + @Test + @SuppressWarnings("unchecked") + public void testRegisterPacket() { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; + + DictDB mockDictDB = mock(DictDB.class); + + when(aggregatorSpy.getPackets(nid)).thenReturn(mockDictDB); + + aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + + verify(mockDictDB).set(sn, data); + } + + @Test + public void testRegisterPacket_duplicate() { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; + + aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + + Executable action = () -> aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + + assertEquals("Reverted(0): Packet already exists", e.getMessage()); + } + + @Test + @SuppressWarnings("unchecked") + public void testSubmitSignature() throws Exception { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; + + aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + + DictDB mockSignatures = mock(DictDB.class); + + + when(aggregatorSpy.getSignatures(nid, sn)).thenReturn(mockSignatures); + + byte[] dataHash = hashData(data); + String sign = signData(dataHash,relayerAcOnePkey); + + aggregator.invoke(relayerAcOne, "submitSignature", nid, sn, sign); + + verify(mockSignatures).set(relayerAcOne.getAddress(), sign); + } + + private static byte[] hashData(byte[] data) throws Exception { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return digest.digest(data); + } + + private static String signData(byte[] dataHash, String pKeyStr) throws Exception { + byte[] pKeyBytes = HexString.toBytes(pKeyStr); + + PrivateKey pKey = parsePrivateKey(pKeyBytes); + + Signature signature = Signature.getInstance("SHA256withECDSA", "BC"); + signature.initSign(pKey); + signature.update(dataHash); + + byte[] sign = signature.sign(); + return HexString.fromBytes(sign); + } + + public static PrivateKey parsePrivateKey(byte[] pKeyBytes) throws Exception { + // Ensure the private key is 32 bytes + if (pKeyBytes.length != 32) { + throw new IllegalArgumentException("Invalid private key length: must be 32 bytes."); + } + + // Create a BigInteger from the private key bytes (unsigned) + BigInteger privateKeyInt = new BigInteger(1, pKeyBytes); + + // Create EC PrivateKeySpec using the secp256k1 curve parameters + ECParameterSpec ecSpec = getSecp256k1Curve(); + ECPrivateKeySpec privateKeySpec = new ECPrivateKeySpec(privateKeyInt, ecSpec); + + // Create the KeyFactory for generating EC keys + KeyFactory keyFactory = KeyFactory.getInstance("ECDSA", "BC"); + + // Generate the private key from the spec + return keyFactory.generatePrivate(privateKeySpec); + } + + private static ECParameterSpec getSecp256k1Curve() { + try { + AlgorithmParameters parameters = AlgorithmParameters.getInstance("EC", "BC"); + parameters.init(new ECGenParameterSpec("secp256k1")); + return parameters.getParameterSpec(ECParameterSpec.class); + } catch (Exception e) { + throw new RuntimeException("Failed to get secp256k1 curve parameters", e); + } } } diff --git a/contracts/javascore/keystore.json b/contracts/javascore/keystore.json new file mode 100644 index 00000000..4632fa28 --- /dev/null +++ b/contracts/javascore/keystore.json @@ -0,0 +1 @@ +{"address":"hx5f7afdb96154bbe9dbeb2dde3a58afcb1efbfaff","id":"7a8237b8-a61f-4501-9896-b96a39e9c72b","version":3,"coinType":"icx","crypto":{"cipher":"aes-128-ctr","cipherparams":{"iv":"c2e5a31a638992a36e3afe96d38ccfda"},"ciphertext":"68d61f1325e1198ca7fd0ee961997e31541ead36ff24b29fc04e6f39cb05eeac","kdf":"scrypt","kdfparams":{"dklen":32,"n":65536,"r":8,"p":1,"salt":"53663f4c4320aff5"},"mac":"388b1466f2a75736fee1f576e2307da2fcaa0cf3d00b968d3c45a98eb9c06f84"}} \ No newline at end of file From 4bfc41ec9bdb92b5570c789acfa4132312c7b276 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Tue, 24 Sep 2024 14:43:32 +0545 Subject: [PATCH 04/18] fix: use icx wallet --- .../relay/aggregator/RelayAggregator.java | 33 ++--- .../relay/aggregator/RelayAggregatorTest.java | 131 +++++++++--------- 2 files changed, 80 insertions(+), 84 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index a7a5e979..f50b80ce 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -34,7 +34,7 @@ public class RelayAggregator { private final BranchDB> packets = Context.newBranchDB("packets", byte[].class); - private final BranchDB>> signatures = Context.newBranchDB("signatures", DictDB.class); + private final BranchDB>> signatures = Context.newBranchDB("signatures", byte[].class); public RelayAggregator(Address _admin, Address[] _relayers) { if (admin.get() == null) { @@ -70,36 +70,37 @@ public void registerPacket(String nid, BigInteger sn, byte[] data) { * @param signature packet signature */ @External - public void submitSignature(String nid, BigInteger sn, String signature) { + public void submitSignature(String nid, BigInteger sn, byte[] signature) { relayersOnly(); DictDB packetDict = getPackets(nid); byte[] packetData = packetDict.get(sn); - Context.require(packetData != null, "Packet does not exist"); + Context.require(packetData != null, "Packet not registered"); + byte[] dataHash = Context.hash("sha-256", packetData); - DictDB addressSigns = getSignatures(nid, sn); - + byte[] key = Context.recoverKey("ecdsa-secp256k1", dataHash, signature, true); + Address address = Context.getAddressFromKey(key); Address caller = Context.getCaller(); - String sign = addressSigns.get(caller); - Context.require(sign == null, "Signature already exists."); - addressSigns.set(caller, signature); + Context.require(address.equals(caller), "Invalid signature"); + + byte[] sign = signatures.at(nid).at(sn).get(caller); + Context.require(sign == null, "Signature already exists"); + + setSignature(nid, sn, caller, signature); } /** - * Retrieves the signatures dictionary for the packet. + * Sets the signature for that packet at particular nid, sn and address * * @param nid network ID of the source chain * @param sn sequence number of the source chain message - * @return map of relayer addresses to signatures + * @param addr address of signature setter + * @param sign signature of packet */ - protected DictDB getSignatures(String nid, BigInteger sn) { - BranchDB> snSigns = signatures.at(nid); - if (snSigns == null) { - snSigns = Context.newBranchDB(nid+"-signs", DictDB.class); - } - return snSigns.at(sn); + protected void setSignature(String nid, BigInteger sn, Address addr, byte[] sign) { + signatures.at(nid).at(sn).set(addr, sign); } diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index cb2784ce..6d1c8e4c 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -1,23 +1,13 @@ package relay.aggregator; import java.math.BigInteger; -import java.security.MessageDigest; -import java.security.PrivateKey; -import java.security.Security; -import java.security.AlgorithmParameters; -import java.security.KeyFactory; -import java.security.Signature; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; -import org.bouncycastle.jce.provider.BouncyCastleProvider; -import java.security.spec.ECPrivateKeySpec; -import java.security.spec.ECGenParameterSpec; -import java.security.spec.ECParameterSpec; - import score.Address; +import score.Context; import score.UserRevertedException; import score.DictDB; @@ -25,6 +15,7 @@ import com.iconloop.score.test.Score; import com.iconloop.score.test.ServiceManager; import com.iconloop.score.test.TestBase; +import foundation.icon.icx.KeyWallet; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -36,26 +27,34 @@ class RelayAggregatorTest extends TestBase { private final ServiceManager sm = getServiceManager(); - private final Account adminAc = sm.createAccount(); - - private final Account relayerAcOne = sm.getAccount(new Address(HexString.toBytes("hxe794bcf6a92efb5e0b58cfc4728236c650d86dce"))); - private final String relayerAcOnePkey = "40b90166d6ace4acbdc59596fd483e487bf58ec4ae5ff31e4f97f039af5b23f7"; + private KeyWallet admin; + private Account adminAc; - private final Account relayerAcTwo = sm.getAccount(new Address(HexString.toBytes("hx3445c3d4341a9b1fc2ae6fd578a4453ab0072c07"))); - private final String relayerAcTwoPkey = "0xb533387c502c39eea62f4c2a5be31e388fa87313438236656b4c71be92fed066"; + private KeyWallet relayerOne; + private Account relayerOneAc; + + private KeyWallet relayerTwo; + private Account relayerTwoAc; private Score aggregator; private RelayAggregator aggregatorSpy; @BeforeEach void setup() throws Exception { - Address[] relayers = new Address[]{relayerAcOne.getAddress(), relayerAcTwo.getAddress()}; + admin = KeyWallet.create(); + adminAc = sm.getAccount(Address.fromString(admin.getAddress().toString())); + + relayerOne = KeyWallet.create(); + relayerOneAc = sm.getAccount(Address.fromString(relayerOne.getAddress().toString())); + + relayerTwo = KeyWallet.create(); + relayerTwoAc = sm.getAccount(Address.fromString(relayerTwo.getAddress().toString())); + + Address[] relayers = new Address[]{relayerOneAc.getAddress(), relayerTwoAc.getAddress()}; aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance()); aggregator.setInstance(aggregatorSpy); - - Security.addProvider(new BouncyCastleProvider()); } @Test @@ -109,72 +108,68 @@ public void testRegisterPacket_duplicate() { } @Test - @SuppressWarnings("unchecked") public void testSubmitSignature() throws Exception { String nid = "0x2.icon"; BigInteger sn = BigInteger.ONE; byte[] data = new byte[]{0x01, 0x02}; aggregator.invoke(adminAc, "registerPacket", nid, sn, data); - - DictDB mockSignatures = mock(DictDB.class); - - when(aggregatorSpy.getSignatures(nid, sn)).thenReturn(mockSignatures); + byte[] dataHash = Context.hash("sha-256", data); + byte[] sign = relayerOne.sign(dataHash);; - byte[] dataHash = hashData(data); - String sign = signData(dataHash,relayerAcOnePkey); + aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); - aggregator.invoke(relayerAcOne, "submitSignature", nid, sn, sign); - - verify(mockSignatures).set(relayerAcOne.getAddress(), sign); + verify(aggregatorSpy).setSignature(nid, sn, relayerOneAc.getAddress(), sign); } - private static byte[] hashData(byte[] data) throws Exception { - MessageDigest digest = MessageDigest.getInstance("SHA-256"); - return digest.digest(data); - } + @Test + public void testSubmitSignature_duplicate() throws Exception { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; - private static String signData(byte[] dataHash, String pKeyStr) throws Exception { - byte[] pKeyBytes = HexString.toBytes(pKeyStr); - - PrivateKey pKey = parsePrivateKey(pKeyBytes); - - Signature signature = Signature.getInstance("SHA256withECDSA", "BC"); - signature.initSign(pKey); - signature.update(dataHash); - - byte[] sign = signature.sign(); - return HexString.fromBytes(sign); - } + aggregator.invoke(adminAc, "registerPacket", nid, sn, data); - public static PrivateKey parsePrivateKey(byte[] pKeyBytes) throws Exception { - // Ensure the private key is 32 bytes - if (pKeyBytes.length != 32) { - throw new IllegalArgumentException("Invalid private key length: must be 32 bytes."); - } + byte[] dataHash = Context.hash("sha-256", data); + byte[] sign = relayerOne.sign(dataHash);; - // Create a BigInteger from the private key bytes (unsigned) - BigInteger privateKeyInt = new BigInteger(1, pKeyBytes); + aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); - // Create EC PrivateKeySpec using the secp256k1 curve parameters - ECParameterSpec ecSpec = getSecp256k1Curve(); - ECPrivateKeySpec privateKeySpec = new ECPrivateKeySpec(privateKeyInt, ecSpec); + Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + assertEquals("Reverted(0): Signature already exists", e.getMessage()); + } - // Create the KeyFactory for generating EC keys - KeyFactory keyFactory = KeyFactory.getInstance("ECDSA", "BC"); + @Test + public void testSubmitSignature_packetUnregistered() throws Exception { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; - // Generate the private key from the spec - return keyFactory.generatePrivate(privateKeySpec); + byte[] dataHash = Context.hash("sha-256", data); + byte[] sign = relayerOne.sign(dataHash);; + + Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + assertEquals("Reverted(0): Packet not registered", e.getMessage()); } - private static ECParameterSpec getSecp256k1Curve() { - try { - AlgorithmParameters parameters = AlgorithmParameters.getInstance("EC", "BC"); - parameters.init(new ECGenParameterSpec("secp256k1")); - return parameters.getParameterSpec(ECParameterSpec.class); - } catch (Exception e) { - throw new RuntimeException("Failed to get secp256k1 curve parameters", e); - } + @Test + public void testSubmitSignature_invalidSignatureWithWrongData() throws Exception { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; + + aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + + byte[] wrongData = new byte[]{0x01, 0x02, 0x03}; + byte[] dataHash = Context.hash("sha-256", wrongData); + byte[] sign = relayerOne.sign(dataHash);; + + Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + + assertEquals("Reverted(0): Invalid signature", e.getMessage()); } } From d9f7eda452689764b1bbc352058e35fe84ab0e8b Mon Sep 17 00:00:00 2001 From: sherpalden Date: Tue, 24 Sep 2024 22:57:01 +0545 Subject: [PATCH 05/18] fix: emit required events --- .../relay/aggregator/RelayAggregator.java | 134 +++++++++++++----- .../relay/aggregator/RelayAggregatorTest.java | 25 +++- 2 files changed, 124 insertions(+), 35 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index f50b80ce..576a17eb 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -19,18 +19,21 @@ import java.math.BigInteger; +import s.java.lang.Integer; import score.Context; import score.Address; +import score.ArrayDB; import score.VarDB; import score.DictDB; import score.BranchDB; - +import score.annotation.EventLog; import score.annotation.External; +import scorex.util.ArrayList; public class RelayAggregator { private final VarDB
admin = Context.newVarDB("admin", Address.class); - private final DictDB relayers = Context.newDictDB("relayers", Boolean.class); + private final ArrayDB
relayers = Context.newArrayDB("relayers", Address.class); private final BranchDB> packets = Context.newBranchDB("packets", byte[].class); @@ -40,11 +43,32 @@ public RelayAggregator(Address _admin, Address[] _relayers) { if (admin.get() == null) { admin.set(_admin); for (Address relayer : _relayers) { - relayers.set(relayer, true); + relayers.add(relayer); } } } + /** + * Sets the admin address. + * + * @param _admin the new admin address + */ + @External + public void setAdmin(Address _admin) { + adminOnly(); + admin.set(_admin); + } + + /** + * Retrieves the admin address. + * + * @return admin address. + */ + @External(readonly = true) + public Address getAdmin() { + return admin.get(); + } + /** * Registers a new packet. * @@ -60,6 +84,8 @@ public void registerPacket(String nid, BigInteger sn, byte[] data) { Context.require(pkt == null, "Packet already exists"); packetDict.set(sn, data); + + PacketRegistered(nid, sn, data); } /** @@ -89,43 +115,35 @@ public void submitSignature(String nid, BigInteger sn, byte[] signature) { Context.require(sign == null, "Signature already exists"); setSignature(nid, sn, caller, signature); - } - /** - * Sets the signature for that packet at particular nid, sn and address - * - * @param nid network ID of the source chain - * @param sn sequence number of the source chain message - * @param addr address of signature setter - * @param sign signature of packet - */ - protected void setSignature(String nid, BigInteger sn, Address addr, byte[] sign) { - signatures.at(nid).at(sn).set(addr, sign); - } - - - /** - * Sets the admin address. - * - * @param _admin the new admin address - */ - @External - public void setAdmin(Address _admin) { - adminOnly(); - admin.set(_admin); - } + if (signatureThresholdReached(nid, sn)) { + PacketConfirmed(nid, sn, packetData); + } + } /** - * Retrieves the admin address. - * - * @return admin address. + * Retrieves the list of signatures for the particular packet. + * * + * @param nid network id of the source chain + * @param sn sequence number of packet on source chain + * @return list of signatures */ @External(readonly = true) - public Address getAdmin() { - return admin.get(); + public ArrayList getSignatures(String nid, BigInteger sn) { + DictDB signDict = signatures.at(nid).at(sn); + ArrayList signatureList = new ArrayList(); + + for (int i = 0; i < relayers.size(); i++) { + Address relayer = relayers.get(i); + byte[] sign = signDict.get(relayer); + if (sign != null) { + signatureList.add(sign); + } + } + return signatureList; } - /** + /** * Retrieves the packets dictionary. * * * @param nid network id of the source chain @@ -135,6 +153,18 @@ protected DictDB getPackets(String nid) { return packets.at(nid); } + /** + * Sets the signature for that packet at particular nid, sn and address + * + * @param nid network ID of the source chain + * @param sn sequence number of the source chain message + * @param addr address of signature setter + * @param sign signature of packet + */ + protected void setSignature(String nid, BigInteger sn, Address addr, byte[] sign) { + signatures.at(nid).at(sn).set(addr, sign); + } + /** * Checks if the caller of the function is the admin. * @@ -151,7 +181,43 @@ private void adminOnly() { */ private void relayersOnly() { Address caller = Context.getCaller(); - boolean isRelayer = relayers.get(caller); + Boolean isRelayer = false; + for (int i = 0; i < relayers.size(); i++) { + Address relayer = relayers.get(i); + if (relayer.equals(caller)){ + isRelayer = true; + break; + } + } Context.require(isRelayer, "Unauthorized: caller is not a registered relayer"); } + + /** + * Checks if the number of signatures reached the threshold for the packet. + * + * @param nid network ID of the source chain + * @param sn sequence number of the source chain message + * + * @return true if the number of signatures reached the threshold. + */ + private Boolean signatureThresholdReached(String nid, BigInteger sn) { + int noOfSignatures = 0; + for (int i = 0; i < relayers.size(); i++) { + Address relayer = relayers.get(i); + byte[] relayerSign = signatures.at(nid).at(sn).get(relayer); + if (relayerSign != null) { + noOfSignatures++; + } + } + int threshold = (relayers.size() * 66) / 100; + return noOfSignatures >= threshold; + } + + @EventLog(indexed = 2) + public void PacketRegistered(String srcNetwork, BigInteger srcSn, byte[] data) { + } + + @EventLog(indexed = 1) + public void PacketConfirmed(String srcNetwork, BigInteger srcSn, byte[] data) { + } } \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index 6d1c8e4c..9261a3a5 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -36,6 +36,9 @@ class RelayAggregatorTest extends TestBase { private KeyWallet relayerTwo; private Account relayerTwoAc; + private KeyWallet relayerThree; + private Account relayerThreeAc; + private Score aggregator; private RelayAggregator aggregatorSpy; @@ -50,6 +53,9 @@ void setup() throws Exception { relayerTwo = KeyWallet.create(); relayerTwoAc = sm.getAccount(Address.fromString(relayerTwo.getAddress().toString())); + relayerThree = KeyWallet.create(); + relayerThreeAc = sm.getAccount(Address.fromString(relayerThree.getAddress().toString())); + Address[] relayers = new Address[]{relayerOneAc.getAddress(), relayerTwoAc.getAddress()}; aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); @@ -116,13 +122,29 @@ public void testSubmitSignature() throws Exception { aggregator.invoke(adminAc, "registerPacket", nid, sn, data); byte[] dataHash = Context.hash("sha-256", data); - byte[] sign = relayerOne.sign(dataHash);; + byte[] sign = relayerOne.sign(dataHash); aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); verify(aggregatorSpy).setSignature(nid, sn, relayerOneAc.getAddress(), sign); } + @Test + public void testSubmitSignature_unauthorized() throws Exception { + String nid = "0x2.icon"; + BigInteger sn = BigInteger.ONE; + byte[] data = new byte[]{0x01, 0x02}; + + aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + + byte[] dataHash = Context.hash("sha-256", data); + byte[] sign = relayerThree.sign(dataHash); + + Executable action = () -> aggregator.invoke(relayerThreeAc, "submitSignature", nid, sn, sign); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + assertEquals("Reverted(0): Unauthorized: caller is not a registered relayer", e.getMessage()); + } + @Test public void testSubmitSignature_duplicate() throws Exception { String nid = "0x2.icon"; @@ -172,4 +194,5 @@ public void testSubmitSignature_invalidSignatureWithWrongData() throws Exception assertEquals("Reverted(0): Invalid signature", e.getMessage()); } + } From 7f383dab0f27b0132bc73b1eb8acfa71301f965c Mon Sep 17 00:00:00 2001 From: sherpalden Date: Tue, 24 Sep 2024 22:58:01 +0545 Subject: [PATCH 06/18] fix: remove unused import --- .../src/main/java/relay/aggregator/RelayAggregator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 576a17eb..a3e159ca 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -19,7 +19,6 @@ import java.math.BigInteger; -import s.java.lang.Integer; import score.Context; import score.Address; import score.ArrayDB; From c3bf1f39217cc058c46ad56937f8693d5f289a78 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Wed, 25 Sep 2024 20:24:12 +0545 Subject: [PATCH 07/18] refactor: improve data structures --- .../main/java/relay/aggregator/Packet.java | 135 +++++++++++++++ .../relay/aggregator/RelayAggregator.java | 162 ++++++------------ .../relay/aggregator/RelayAggregatorTest.java | 138 +++++++-------- 3 files changed, 257 insertions(+), 178 deletions(-) create mode 100644 contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java new file mode 100644 index 00000000..64c6b20a --- /dev/null +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java @@ -0,0 +1,135 @@ +package relay.aggregator; + +import java.math.BigInteger; + +import score.ObjectReader; +import score.ObjectWriter; + +public class Packet { + + /** + * The ID of the source network (chain) from where the packet originated. + */ + private final String srcNetwork; + + /** + * The contract address on the source network (chain). + */ + private final String contractAddress; + + /** + * The sequence number of the packet in the source network (chain). + */ + private final BigInteger srcSn; + + /** + * The ID of the destination network (chain) where the packet is being sent. + */ + private final String dstNetwork; + + /** + * The payload data associated with this packet. + */ + private final byte[] data; + + /** + * Constructs a new {@code Packet} object with the specified {@code PacketID}, + * destination network, and data. + * All parameters must be non-null. + * + * @param id the unique identifier for the packet. + * @param dstNetwork the ID of the destination network (chain). + * @param data the payload data for this packet. + * @throws IllegalArgumentException if {@code srcNetwork}, + * {@code contractAddress}, {@code srcSn}, + * {@code dstNetwork}, or {@code data} is + * {@code null}. + */ + public Packet(String srcNetwork, String contractAddress, BigInteger srcSn, String dstNetwork, byte[] data) { + if (srcNetwork == null || contractAddress == null || contractAddress == null || srcSn == null + || dstNetwork == null || data == null) { + throw new IllegalArgumentException( + "srcNetwork, contractAddress, srcSn, dstNetwork, and data cannot be null"); + } + this.srcNetwork = srcNetwork; + this.contractAddress = contractAddress; + this.srcSn = srcSn; + this.dstNetwork = dstNetwork; + this.data = data.clone(); // clone for immutability + } + + public String getId() { + return createId(this.srcNetwork, this.contractAddress, this.srcSn); + } + + public static String createId(String srcNetwork, String contractAddress, BigInteger srcSn) { + return srcNetwork + "/" + contractAddress + "/" + srcSn.toString(); + } + + /** + * Returns the source network (chain) from where the packet originated. + * + * @return the source network ID. + */ + public String getSrcNetwork() { + return srcNetwork; + } + + /** + * Returns the contract address on the source network (chain). + * + * @return the contract address. + */ + public String getContractAddress() { + return contractAddress; + } + + /** + * Returns the sequence number of the packet in the source network (chain). + * + * @return the sequence number. + */ + public BigInteger getSrcSn() { + return srcSn; + } + + /** + * Returns the destination network (chain) where the packet is being sent. + * + * @return the destination network ID. + */ + public String getDstNetwork() { + return dstNetwork; + } + + /** + * Returns a copy of the data associated with this packet. + * + * @return a byte array containing the packet data. + */ + public byte[] getData() { + return data.clone(); // return a clone to preserve immutability + } + + public static void writeObject(ObjectWriter w, Packet p) { + w.beginList(5); + w.write(p.srcNetwork); + w.write(p.contractAddress); + w.write(p.srcSn); + w.write(p.dstNetwork); + w.writeNullable(p.data); + w.end(); + } + + public static Packet readObject(ObjectReader r) { + r.beginList(); + Packet p = new Packet( + r.readString(), + r.readString(), + r.readBigInteger(), + r.readString(), + r.readNullable(byte[].class)); + r.end(); + return p; + } +} diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index a3e159ca..bacf3b0f 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -16,7 +16,6 @@ package relay.aggregator; - import java.math.BigInteger; import score.Context; @@ -34,9 +33,10 @@ public class RelayAggregator { private final ArrayDB
relayers = Context.newArrayDB("relayers", Address.class); - private final BranchDB> packets = Context.newBranchDB("packets", byte[].class); + private final DictDB packets = Context.newDictDB("packets", Packet.class); - private final BranchDB>> signatures = Context.newBranchDB("signatures", byte[].class); + private final BranchDB> signatures = Context.newBranchDB("signatures", + byte[].class); public RelayAggregator(Address _admin, Address[] _relayers) { if (admin.get() == null) { @@ -47,89 +47,69 @@ public RelayAggregator(Address _admin, Address[] _relayers) { } } - /** - * Sets the admin address. - * - * @param _admin the new admin address - */ @External public void setAdmin(Address _admin) { adminOnly(); admin.set(_admin); } - /** - * Retrieves the admin address. - * - * @return admin address. - */ @External(readonly = true) public Address getAdmin() { return admin.get(); } - /** - * Registers a new packet. - * - * @param nid network ID - * @param sn sequence number - * @param data packet data - */ @External - public void registerPacket(String nid, BigInteger sn, byte[] data) { + public void registerPacket( + String srcNetwork, + String contractAddress, + BigInteger srcSn, + String dstNetwork, + byte[] data) { + adminOnly(); - DictDB packetDict = getPackets(nid); - byte[] pkt = packetDict.get(sn); - Context.require(pkt == null, "Packet already exists"); - packetDict.set(sn, data); + Packet pkt = new Packet(srcNetwork, contractAddress, srcSn, dstNetwork, data); + String id = pkt.getId(); - PacketRegistered(nid, sn, data); - } + Context.require(packets.get(id) == null, "Packet already exists"); - /** - * Submits a signature for the registered packet. - * - * @param nid network ID - * @param sn sequence number - * @param signature packet signature - */ - @External - public void submitSignature(String nid, BigInteger sn, byte[] signature) { - relayersOnly(); + packets.set(id, pkt); - DictDB packetDict = getPackets(nid); - byte[] packetData = packetDict.get(sn); - Context.require(packetData != null, "Packet not registered"); + PacketRegistered(pkt.getSrcNetwork(), pkt.getContractAddress(), pkt.getSrcSn()); + } - byte[] dataHash = Context.hash("sha-256", packetData); + @External + public void submitSignature( + String srcNetwork, + String contractAddress, + BigInteger srcSn, + byte[] signature) { - byte[] key = Context.recoverKey("ecdsa-secp256k1", dataHash, signature, true); - Address address = Context.getAddressFromKey(key); - Address caller = Context.getCaller(); + relayersOnly(); - Context.require(address.equals(caller), "Invalid signature"); + String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + Packet pkt = packets.get(pktID); + Context.require(pkt != null, "Packet not registered"); - byte[] sign = signatures.at(nid).at(sn).get(caller); - Context.require(sign == null, "Signature already exists"); + byte[] existingSign = signatures.at(pktID).get(Context.getCaller()); + Context.require(existingSign == null, "Signature already exists"); - setSignature(nid, sn, caller, signature); + setSignature(pktID, Context.getCaller(), signature); - if (signatureThresholdReached(nid, sn)) { - PacketConfirmed(nid, sn, packetData); + if (signatureThresholdReached(pktID)) { + PacketConfirmed( + pkt.getSrcNetwork(), + pkt.getContractAddress(), + pkt.getSrcSn(), + pkt.getDstNetwork(), + pkt.getData()); } - } - - /** - * Retrieves the list of signatures for the particular packet. - * * - * @param nid network id of the source chain - * @param sn sequence number of packet on source chain - * @return list of signatures - */ + } + @External(readonly = true) - public ArrayList getSignatures(String nid, BigInteger sn) { - DictDB signDict = signatures.at(nid).at(sn); + public ArrayList getSignatures(String srcNetwork, String contractAddress, BigInteger srcSn) { + String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + DictDB signDict = signatures.at(pktID); ArrayList signatureList = new ArrayList(); for (int i = 0; i < relayers.size(); i++) { @@ -142,48 +122,20 @@ public ArrayList getSignatures(String nid, BigInteger sn) { return signatureList; } - /** - * Retrieves the packets dictionary. - * * - * @param nid network id of the source chain - * @return list of mapping from sn to packet data. - */ - protected DictDB getPackets(String nid) { - return packets.at(nid); + protected void setSignature(String pktID, Address addr, byte[] sign) { + signatures.at(pktID).set(addr, sign); } - /** - * Sets the signature for that packet at particular nid, sn and address - * - * @param nid network ID of the source chain - * @param sn sequence number of the source chain message - * @param addr address of signature setter - * @param sign signature of packet - */ - protected void setSignature(String nid, BigInteger sn, Address addr, byte[] sign) { - signatures.at(nid).at(sn).set(addr, sign); - } - - /** - * Checks if the caller of the function is the admin. - * - * @return true if the caller is the admin, false otherwise - */ private void adminOnly() { Context.require(Context.getCaller().equals(admin.get()), "Unauthorized: caller is not the leader relayer"); } - /** - * Checks if the caller of the function is among the relayers. - * - * @return true if the caller is among the relayers, otherwise false - */ private void relayersOnly() { Address caller = Context.getCaller(); Boolean isRelayer = false; for (int i = 0; i < relayers.size(); i++) { Address relayer = relayers.get(i); - if (relayer.equals(caller)){ + if (relayer.equals(caller)) { isRelayer = true; break; } @@ -191,19 +143,11 @@ private void relayersOnly() { Context.require(isRelayer, "Unauthorized: caller is not a registered relayer"); } - /** - * Checks if the number of signatures reached the threshold for the packet. - * - * @param nid network ID of the source chain - * @param sn sequence number of the source chain message - * - * @return true if the number of signatures reached the threshold. - */ - private Boolean signatureThresholdReached(String nid, BigInteger sn) { + private Boolean signatureThresholdReached(String pktID) { int noOfSignatures = 0; for (int i = 0; i < relayers.size(); i++) { Address relayer = relayers.get(i); - byte[] relayerSign = signatures.at(nid).at(sn).get(relayer); + byte[] relayerSign = signatures.at(pktID).get(relayer); if (relayerSign != null) { noOfSignatures++; } @@ -213,10 +157,18 @@ private Boolean signatureThresholdReached(String nid, BigInteger sn) { } @EventLog(indexed = 2) - public void PacketRegistered(String srcNetwork, BigInteger srcSn, byte[] data) { + public void PacketRegistered( + String srcNetwork, + String contractAddress, + BigInteger srcSn) { } - @EventLog(indexed = 1) - public void PacketConfirmed(String srcNetwork, BigInteger srcSn, byte[] data) { + @EventLog(indexed = 2) + public void PacketConfirmed( + String srcNetwork, + String contractAddress, + BigInteger srcSn, + String dstNetwork, + byte[] data) { } } \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index 9261a3a5..d3369644 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -9,7 +9,6 @@ import score.Address; import score.Context; import score.UserRevertedException; -import score.DictDB; import com.iconloop.score.test.Account; import com.iconloop.score.test.Score; @@ -19,25 +18,23 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; class RelayAggregatorTest extends TestBase { private final ServiceManager sm = getServiceManager(); - private KeyWallet admin; - private Account adminAc; - - private KeyWallet relayerOne; - private Account relayerOneAc; + private KeyWallet admin; + private Account adminAc; - private KeyWallet relayerTwo; - private Account relayerTwoAc; + private KeyWallet relayerOne; + private Account relayerOneAc; - private KeyWallet relayerThree; - private Account relayerThreeAc; + private KeyWallet relayerTwo; + private Account relayerTwoAc; + + private KeyWallet relayerThree; + private Account relayerThreeAc; private Score aggregator; private RelayAggregator aggregatorSpy; @@ -56,7 +53,7 @@ void setup() throws Exception { relayerThree = KeyWallet.create(); relayerThreeAc = sm.getAccount(Address.fromString(relayerThree.getAddress().toString())); - Address[] relayers = new Address[]{relayerOneAc.getAddress(), relayerTwoAc.getAddress()}; + Address[] relayers = new Address[] { relayerOneAc.getAddress(), relayerTwoAc.getAddress() }; aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance()); @@ -76,123 +73,118 @@ public void testSetAdmin() { public void testSetAdmin_unauthorized() { Account normalAc = sm.createAccount(); Account newAdminAc = sm.createAccount(); - + Executable action = () -> aggregator.invoke(normalAc, "setAdmin", newAdminAc.getAddress()); UserRevertedException e = assertThrows(UserRevertedException.class, action); - + assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage()); } @Test - @SuppressWarnings("unchecked") public void testRegisterPacket() { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; - - DictDB mockDictDB = mock(DictDB.class); - - when(aggregatorSpy.getPackets(nid)).thenReturn(mockDictDB); + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); - verify(mockDictDB).set(sn, data); + verify(aggregatorSpy).PacketRegistered(srcNetwork, contractAddress, srcSn); } @Test public void testRegisterPacket_duplicate() { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); - Executable action = () -> aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + Executable action = () -> aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, + dstNetwork, data); UserRevertedException e = assertThrows(UserRevertedException.class, action); - + assertEquals("Reverted(0): Packet already exists", e.getMessage()); } @Test public void testSubmitSignature() throws Exception { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, sign); - verify(aggregatorSpy).setSignature(nid, sn, relayerOneAc.getAddress(), sign); + String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + verify(aggregatorSpy).setSignature(pktID, relayerOneAc.getAddress(), sign); } @Test public void testSubmitSignature_unauthorized() throws Exception { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerThree.sign(dataHash); - Executable action = () -> aggregator.invoke(relayerThreeAc, "submitSignature", nid, sn, sign); + Executable action = () -> aggregator.invoke(relayerThreeAc, "submitSignature", srcNetwork, contractAddress, + srcSn, + sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); - assertEquals("Reverted(0): Unauthorized: caller is not a registered relayer", e.getMessage()); + assertEquals("Reverted(0): Unauthorized: caller is not a registered relayer", + e.getMessage()); } @Test public void testSubmitSignature_duplicate() throws Exception { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", nid, sn, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); byte[] dataHash = Context.hash("sha-256", data); - byte[] sign = relayerOne.sign(dataHash);; + byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, sign); - Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, + sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Signature already exists", e.getMessage()); } @Test public void testSubmitSignature_packetUnregistered() throws Exception { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; + String srcNetwork = "0x2.icon"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; byte[] dataHash = Context.hash("sha-256", data); - byte[] sign = relayerOne.sign(dataHash);; + byte[] sign = relayerOne.sign(dataHash); - Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); + Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, + sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Packet not registered", e.getMessage()); } - @Test - public void testSubmitSignature_invalidSignatureWithWrongData() throws Exception { - String nid = "0x2.icon"; - BigInteger sn = BigInteger.ONE; - byte[] data = new byte[]{0x01, 0x02}; - - aggregator.invoke(adminAc, "registerPacket", nid, sn, data); - - byte[] wrongData = new byte[]{0x01, 0x02, 0x03}; - byte[] dataHash = Context.hash("sha-256", wrongData); - byte[] sign = relayerOne.sign(dataHash);; - - Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", nid, sn, sign); - UserRevertedException e = assertThrows(UserRevertedException.class, action); - - assertEquals("Reverted(0): Invalid signature", e.getMessage()); - } - } From 7edb48b1a50029ea44a34220b9e1f875a21e9986 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Fri, 27 Sep 2024 14:22:07 +0545 Subject: [PATCH 08/18] feat: add relayers add and remove functionaility --- .../main/java/relay/aggregator/HexString.java | 26 --- .../main/java/relay/aggregator/Packet.java | 37 +++- .../relay/aggregator/RelayAggregator.java | 160 +++++++++++++-- .../relay/aggregator/RelayAggregatorTest.java | 184 ++++++++++++++++-- 4 files changed, 341 insertions(+), 66 deletions(-) delete mode 100644 contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java deleted file mode 100644 index 6c12e466..00000000 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/HexString.java +++ /dev/null @@ -1,26 +0,0 @@ -package relay.aggregator; - -public class HexString { - public static byte[] toBytes(String hex) { - int len = hex.length(); - byte[] data = new byte[len / 2]; - - for (int i = 0; i < len; i += 2) { - data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) - + Character.digit(hex.charAt(i + 1), 16)); - } - return data; - } - - public static String fromBytes(byte[] bytes) { - StringBuilder hexString = new StringBuilder(); - for (byte b : bytes) { - String hex = Integer.toHexString(0xff & b); - if (hex.length() == 1) { - hexString.append('0'); // Append leading zero if needed - } - hexString.append(hex); - } - return hexString.toString(); - } -} diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java index 64c6b20a..e481a235 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java @@ -2,6 +2,7 @@ import java.math.BigInteger; +import score.Context; import score.ObjectReader; import score.ObjectWriter; @@ -22,6 +23,11 @@ public class Packet { */ private final BigInteger srcSn; + /** + * The source height of the packet in the source network (chain). + */ + private final BigInteger srcHeight; + /** * The ID of the destination network (chain) where the packet is being sent. */ @@ -42,20 +48,24 @@ public class Packet { * @param data the payload data for this packet. * @throws IllegalArgumentException if {@code srcNetwork}, * {@code contractAddress}, {@code srcSn}, + * {@code srcHeight}, * {@code dstNetwork}, or {@code data} is * {@code null}. */ - public Packet(String srcNetwork, String contractAddress, BigInteger srcSn, String dstNetwork, byte[] data) { - if (srcNetwork == null || contractAddress == null || contractAddress == null || srcSn == null - || dstNetwork == null || data == null) { - throw new IllegalArgumentException( - "srcNetwork, contractAddress, srcSn, dstNetwork, and data cannot be null"); + public Packet(String srcNetwork, String contractAddress, BigInteger srcSn, BigInteger srcHeight, String dstNetwork, + byte[] data) { + Boolean isIllegalArg = srcNetwork == null || contractAddress == null || contractAddress == null || srcSn == null + || srcHeight == null || dstNetwork == null || data == null; + Context.require(!isIllegalArg, + "srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, and data cannot be null"); + if (isIllegalArg) { } this.srcNetwork = srcNetwork; this.contractAddress = contractAddress; this.srcSn = srcSn; + this.srcHeight = srcHeight; this.dstNetwork = dstNetwork; - this.data = data.clone(); // clone for immutability + this.data = data; } public String getId() { @@ -93,6 +103,15 @@ public BigInteger getSrcSn() { return srcSn; } + /** + * Returns the height of the packet in the source network (chain). + * + * @return the source height. + */ + public BigInteger getSrcHeight() { + return srcHeight; + } + /** * Returns the destination network (chain) where the packet is being sent. * @@ -108,14 +127,15 @@ public String getDstNetwork() { * @return a byte array containing the packet data. */ public byte[] getData() { - return data.clone(); // return a clone to preserve immutability + return data; } public static void writeObject(ObjectWriter w, Packet p) { - w.beginList(5); + w.beginList(6); w.write(p.srcNetwork); w.write(p.contractAddress); w.write(p.srcSn); + w.write(p.srcHeight); w.write(p.dstNetwork); w.writeNullable(p.data); w.end(); @@ -127,6 +147,7 @@ public static Packet readObject(ObjectReader r) { r.readString(), r.readString(), r.readBigInteger(), + r.readBigInteger(), r.readString(), r.readNullable(byte[].class)); r.end(); diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index bacf3b0f..a4e5ed15 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -24,11 +24,18 @@ import score.VarDB; import score.DictDB; import score.BranchDB; +import score.ByteArrayObjectWriter; import score.annotation.EventLog; import score.annotation.External; +import score.ObjectReader; import scorex.util.ArrayList; +import scorex.util.HashMap; public class RelayAggregator { + private final Integer DEFAULT_SIGNATURE_THRESHOLD = 2; + + private final VarDB signatureThreshold = Context.newVarDB("signatureThreshold", Integer.class); + private final VarDB
admin = Context.newVarDB("admin", Address.class); private final ArrayDB
relayers = Context.newArrayDB("relayers", Address.class); @@ -44,6 +51,7 @@ public RelayAggregator(Address _admin, Address[] _relayers) { for (Address relayer : _relayers) { relayers.add(relayer); } + signatureThreshold.set(DEFAULT_SIGNATURE_THRESHOLD); } } @@ -58,28 +66,100 @@ public Address getAdmin() { return admin.get(); } + @External + public void setSignatureThreshold(int threshold) { + adminOnly(); + signatureThreshold.set(threshold); + } + + @External(readonly = true) + public Integer getSignatureThreshold() { + return signatureThreshold.get(); + } + + @External(readonly = true) + public Address[] getRelayers() { + Address[] rlrs = new Address[relayers.size()]; + for (int i = 0; i < relayers.size(); i++) { + rlrs[i] = relayers.get(i); + } + return rlrs; + } + + @External + public void addRelayers(Address[] newRelayers) { + adminOnly(); + + Context.require(newRelayers != null && newRelayers.length != 0, "new relayers cannot be empty"); + + HashMap existingRelayers = new HashMap(); + for (int i = 0; i < relayers.size(); i++) { + Address relayer = relayers.get(i); + existingRelayers.put(relayer, true); + } + + for (Address newRelayer : newRelayers) { + if (!existingRelayers.containsKey(newRelayer)) { + relayers.add(newRelayer); + existingRelayers.put(newRelayer, true); + } + } + } + + @External + public void removeRelayers(Address[] relayersToBeRemoved) { + adminOnly(); + + Context.require(relayersToBeRemoved != null && relayersToBeRemoved.length != 0, + "relayers to be removed cannot be empty"); + + HashMap existingRelayers = new HashMap(); + for (int i = 0; i < relayers.size(); i++) { + Address relayer = relayers.get(i); + existingRelayers.put(relayer, i); + } + + for (Address relayerToBeRemoved : relayersToBeRemoved) { + if (existingRelayers.containsKey(relayerToBeRemoved)) { + Address top = relayers.pop(); + if (!top.equals(relayerToBeRemoved)) { + Integer pos = existingRelayers.get(relayerToBeRemoved); + relayers.set(pos, top); + } + existingRelayers.remove(relayerToBeRemoved); + } + } + } + @External public void registerPacket( String srcNetwork, String contractAddress, BigInteger srcSn, + BigInteger srcHeight, String dstNetwork, byte[] data) { adminOnly(); - Packet pkt = new Packet(srcNetwork, contractAddress, srcSn, dstNetwork, data); + Packet pkt = new Packet(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); String id = pkt.getId(); Context.require(packets.get(id) == null, "Packet already exists"); packets.set(id, pkt); - PacketRegistered(pkt.getSrcNetwork(), pkt.getContractAddress(), pkt.getSrcSn()); + PacketRegistered( + pkt.getSrcNetwork(), + pkt.getContractAddress(), + pkt.getSrcSn(), + pkt.getSrcHeight(), + pkt.getDstNetwork(), + pkt.getData()); } @External - public void submitSignature( + public void acknowledgePacket( String srcNetwork, String contractAddress, BigInteger srcSn, @@ -97,17 +177,21 @@ public void submitSignature( setSignature(pktID, Context.getCaller(), signature); if (signatureThresholdReached(pktID)) { - PacketConfirmed( + byte[][] sigs = getSignatures(srcNetwork, contractAddress, srcSn); + byte[] encodedSigs = serializeSignatures(sigs); + PacketAcknowledged( pkt.getSrcNetwork(), pkt.getContractAddress(), pkt.getSrcSn(), + pkt.getSrcHeight(), pkt.getDstNetwork(), - pkt.getData()); + pkt.getData(), + encodedSigs); + removePacket(pktID); } } - @External(readonly = true) - public ArrayList getSignatures(String srcNetwork, String contractAddress, BigInteger srcSn) { + private byte[][] getSignatures(String srcNetwork, String contractAddress, BigInteger srcSn) { String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); DictDB signDict = signatures.at(pktID); ArrayList signatureList = new ArrayList(); @@ -119,13 +203,49 @@ public ArrayList getSignatures(String srcNetwork, String contractAddress signatureList.add(sign); } } - return signatureList; + + byte[][] sigs = new byte[signatureList.size()][]; + for (int i = 0; i < signatureList.size(); i++) { + sigs[i] = signatureList.get(i); + } + return sigs; } protected void setSignature(String pktID, Address addr, byte[] sign) { signatures.at(pktID).set(addr, sign); } + protected static byte[] serializeSignatures(byte[][] sigs) { + ByteArrayObjectWriter w = Context.newByteArrayObjectWriter("RLPn"); + w.beginList(sigs.length); + + for (byte[] sig : sigs) { + w.write(sig); + } + + w.end(); + return w.toByteArray(); + } + + protected static byte[][] deserializeSignatures(byte[] encodedSigs) { + ObjectReader r = Context.newByteArrayObjectReader("RLPn", encodedSigs); + + ArrayList sigList = new ArrayList<>(); + + r.beginList(); + while (r.hasNext()) { + sigList.add(r.readByteArray()); + } + r.end(); + + byte[][] sigs = new byte[sigList.size()][]; + for (int i = 0; i < sigList.size(); i++) { + sigs[i] = sigList.get(i); + } + + return sigs; + } + private void adminOnly() { Context.require(Context.getCaller().equals(admin.get()), "Unauthorized: caller is not the leader relayer"); } @@ -152,23 +272,37 @@ private Boolean signatureThresholdReached(String pktID) { noOfSignatures++; } } - int threshold = (relayers.size() * 66) / 100; - return noOfSignatures >= threshold; + return noOfSignatures >= 2; + } + + private void removePacket(String pktID) { + packets.set(pktID, null); + DictDB signDict = signatures.at(pktID); + + for (int i = 0; i < relayers.size(); i++) { + Address relayer = relayers.get(i); + signDict.set(relayer, null); + } } @EventLog(indexed = 2) public void PacketRegistered( String srcNetwork, String contractAddress, - BigInteger srcSn) { + BigInteger srcSn, + BigInteger srcHeight, + String dstNetwork, + byte[] data) { } @EventLog(indexed = 2) - public void PacketConfirmed( + public void PacketAcknowledged( String srcNetwork, String contractAddress, BigInteger srcSn, + BigInteger srcHeight, String dstNetwork, - byte[] data) { + byte[] data, + byte[] signatures) { } } \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index d3369644..b01113d0 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -2,6 +2,7 @@ import java.math.BigInteger; +import org.bouncycastle.util.Arrays; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; @@ -14,10 +15,12 @@ import com.iconloop.score.test.Score; import com.iconloop.score.test.ServiceManager; import com.iconloop.score.test.TestBase; + import foundation.icon.icx.KeyWallet; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -36,6 +39,9 @@ class RelayAggregatorTest extends TestBase { private KeyWallet relayerThree; private Account relayerThreeAc; + private KeyWallet relayerFour; + private Account relayerFourAc; + private Score aggregator; private RelayAggregator aggregatorSpy; @@ -53,7 +59,11 @@ void setup() throws Exception { relayerThree = KeyWallet.create(); relayerThreeAc = sm.getAccount(Address.fromString(relayerThree.getAddress().toString())); - Address[] relayers = new Address[] { relayerOneAc.getAddress(), relayerTwoAc.getAddress() }; + relayerFour = KeyWallet.create(); + relayerFourAc = sm.getAccount(Address.fromString(relayerFour.getAddress().toString())); + + Address[] relayers = new Address[] { relayerOneAc.getAddress(), relayerTwoAc.getAddress(), + relayerThreeAc.getAddress() }; aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance()); @@ -80,17 +90,113 @@ public void testSetAdmin_unauthorized() { assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage()); } + @Test + public void testSetSignatureThreshold() { + int threshold = 3; + aggregator.invoke(adminAc, "setSignatureThreshold", threshold); + + Integer result = (Integer) aggregator.call("getSignatureThreshold"); + assertEquals(threshold, result); + } + + @Test + public void testSetSignatureThreshold_unauthorised() { + int threshold = 3; + + 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()); + } + + @Test + public void testAddRelayers() { + Account relayerFiveAc = sm.createAccount(); + Address[] newRelayers = new Address[] { relayerFourAc.getAddress(), relayerFiveAc.getAddress() }; + + aggregator.invoke(adminAc, "addRelayers", (Object) newRelayers); + + Address[] updatedRelayers = (Address[]) aggregator.call("getRelayers"); + + assertTrue(updatedRelayers[updatedRelayers.length - 1].equals(relayerFiveAc.getAddress())); + assertTrue(updatedRelayers[updatedRelayers.length - 2].equals(relayerFourAc.getAddress())); + } + + @Test + public void testAddRelayers_unauthorised() { + Account relayerFiveAc = sm.createAccount(); + Address[] newRelayers = new Address[] { relayerFourAc.getAddress(), relayerFiveAc.getAddress() }; + + Executable action = () -> aggregator.invoke(relayerOneAc, "addRelayers", (Object) newRelayers); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + + 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); + + Address[] updatedRelayers = (Address[]) aggregator.call("getRelayers"); + + Boolean removed = true; + for (Address rlr : updatedRelayers) { + if (rlr.equals(relayerOneAc.getAddress()) || rlr.equals(relayerTwoAc.getAddress())) { + removed = false; + break; + } + } + + assertTrue(removed); + assertEquals(updatedRelayers[0], relayerThreeAc.getAddress()); + } + + @Test + public void testRemoveRelayers_unauthorised() { + Address[] relayerToBeRemoved = new Address[] { relayerOneAc.getAddress(), + relayerTwoAc.getAddress() }; + + aggregator.invoke(adminAc, "removeRelayers", (Object) relayerToBeRemoved); + + Executable action = () -> aggregator.invoke(relayerFourAc, "removeRelayers", (Object) relayerToBeRemoved); + UserRevertedException e = assertThrows(UserRevertedException.class, action); + + assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage()); + } + @Test public void testRegisterPacket() { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - verify(aggregatorSpy).PacketRegistered(srcNetwork, contractAddress, srcSn); + verify(aggregatorSpy).PacketRegistered(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); + } + + @Test + public void testRegisterPacket_nullArg() { + String srcNetwork = null; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; + + Executable action = () -> aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, + srcHeight, dstNetwork, data); + + UserRevertedException e = assertThrows(UserRevertedException.class, action); + + assertEquals("Reverted(0): srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, and data cannot be null", + e.getMessage()); } @Test @@ -98,81 +204,121 @@ public void testRegisterPacket_duplicate() { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); Executable action = () -> aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, - dstNetwork, data); + srcHeight, dstNetwork, data); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Packet already exists", e.getMessage()); } @Test - public void testSubmitSignature() throws Exception { + public void testAcknowledgePacket() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, sign); + aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, sign); String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); verify(aggregatorSpy).setSignature(pktID, relayerOneAc.getAddress(), sign); } @Test - public void testSubmitSignature_unauthorized() throws Exception { + public void testAcknowledgePacket_thresholdReached() throws Exception { + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; + + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); + + byte[] dataHash = Context.hash("sha-256", data); + + byte[] signOne = relayerOne.sign(dataHash); + aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, signOne); + + byte[] signTwo = relayerTwo.sign(dataHash); + aggregator.invoke(relayerTwoAc, "acknowledgePacket", srcNetwork, + contractAddress, srcSn, signTwo); + + byte[][] sigs = new byte[2][]; + sigs[0] = signOne; + sigs[1] = signTwo; + + byte[] encodedSigs = RelayAggregator.serializeSignatures(sigs); + byte[][] decodedSigs = RelayAggregator.deserializeSignatures(encodedSigs); + + assertTrue(Arrays.areEqual(signOne, decodedSigs[0])); + assertTrue(Arrays.areEqual(signTwo, decodedSigs[1])); + + verify(aggregatorSpy).PacketAcknowledged(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + encodedSigs); + } + + @Test + public void testAcknowledgePacket_unauthorized() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); byte[] dataHash = Context.hash("sha-256", data); - byte[] sign = relayerThree.sign(dataHash); + byte[] sign = relayerFour.sign(dataHash); - Executable action = () -> aggregator.invoke(relayerThreeAc, "submitSignature", srcNetwork, contractAddress, + Executable action = () -> aggregator.invoke(relayerFourAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, sign); + UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Unauthorized: caller is not a registered relayer", e.getMessage()); } @Test - public void testSubmitSignature_duplicate() throws Exception { + public void testAcknowledgePacket_duplicate() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, dstNetwork, data); + aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, sign); + aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, sign); - Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, + Executable action = () -> aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, + srcSn, sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Signature already exists", e.getMessage()); } @Test - public void testSubmitSignature_packetUnregistered() throws Exception { + public void testAcknowledgePacket_packetUnregistered() throws Exception { String srcNetwork = "0x2.icon"; BigInteger srcSn = BigInteger.ONE; String contractAddress = "hxjuiod"; @@ -181,10 +327,10 @@ public void testSubmitSignature_packetUnregistered() throws Exception { byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - Executable action = () -> aggregator.invoke(relayerOneAc, "submitSignature", srcNetwork, contractAddress, srcSn, + Executable action = () -> aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, + srcSn, sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Packet not registered", e.getMessage()); } - } From 34b8f2219660a7062dac7f57e2246aa74a6c0360 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Fri, 27 Sep 2024 15:02:51 +0545 Subject: [PATCH 09/18] fix: remove unwanted deps --- contracts/javascore/aggregator/build.gradle | 5 ----- .../src/test/java/relay/aggregator/RelayAggregatorTest.java | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/contracts/javascore/aggregator/build.gradle b/contracts/javascore/aggregator/build.gradle index 0704d410..f8881dcc 100644 --- a/contracts/javascore/aggregator/build.gradle +++ b/contracts/javascore/aggregator/build.gradle @@ -1,11 +1,6 @@ version = '0.1.0' -repositories { - mavenCentral() -} - dependencies { - implementation 'org.bouncycastle:bcprov-jdk15on:1.70' testImplementation 'foundation.icon:javaee-unittest:0.11.1' testImplementation project(':test-lib') } diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index b01113d0..8e8d6ee1 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -2,7 +2,6 @@ import java.math.BigInteger; -import org.bouncycastle.util.Arrays; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; @@ -18,6 +17,7 @@ import foundation.icon.icx.KeyWallet; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -264,8 +264,8 @@ public void testAcknowledgePacket_thresholdReached() throws Exception { byte[] encodedSigs = RelayAggregator.serializeSignatures(sigs); byte[][] decodedSigs = RelayAggregator.deserializeSignatures(encodedSigs); - assertTrue(Arrays.areEqual(signOne, decodedSigs[0])); - assertTrue(Arrays.areEqual(signTwo, decodedSigs[1])); + assertArrayEquals(signOne, decodedSigs[0]); + assertArrayEquals(signTwo, decodedSigs[1]); verify(aggregatorSpy).PacketAcknowledged(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, encodedSigs); From ef5d937ec9c01b0f77c718d0821b3c9f264c4e5f Mon Sep 17 00:00:00 2001 From: sherpalden Date: Mon, 30 Sep 2024 14:22:06 +0545 Subject: [PATCH 10/18] fix: use primitive int for external methods --- contracts/javascore/aggregator/build.gradle | 3 +-- .../src/main/java/relay/aggregator/RelayAggregator.java | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/contracts/javascore/aggregator/build.gradle b/contracts/javascore/aggregator/build.gradle index f8881dcc..b4c26462 100644 --- a/contracts/javascore/aggregator/build.gradle +++ b/contracts/javascore/aggregator/build.gradle @@ -37,7 +37,6 @@ deployJar { password = rootProject.hasProperty('keystorePass') ? "$keystorePass" : '' parameters { arg('_admin', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fd") - arg('_relayers', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fd") - arg('_relayers', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fe") + arg('_relayers', "[]") } } \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index a4e5ed15..c0bd8801 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -73,7 +73,7 @@ public void setSignatureThreshold(int threshold) { } @External(readonly = true) - public Integer getSignatureThreshold() { + public int getSignatureThreshold() { return signatureThreshold.get(); } @@ -272,7 +272,7 @@ private Boolean signatureThresholdReached(String pktID) { noOfSignatures++; } } - return noOfSignatures >= 2; + return noOfSignatures >= signatureThreshold.get(); } private void removePacket(String pktID) { From 29e17dd56d406c559498d7a1b14d3b8b4dc94381 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Mon, 30 Sep 2024 15:02:09 +0545 Subject: [PATCH 11/18] fix: make relayers list configurable from setter function --- contracts/javascore/aggregator/build.gradle | 1 - .../src/main/java/relay/aggregator/RelayAggregator.java | 5 +---- .../src/test/java/relay/aggregator/RelayAggregatorTest.java | 4 +++- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/contracts/javascore/aggregator/build.gradle b/contracts/javascore/aggregator/build.gradle index b4c26462..426026ca 100644 --- a/contracts/javascore/aggregator/build.gradle +++ b/contracts/javascore/aggregator/build.gradle @@ -37,6 +37,5 @@ deployJar { password = rootProject.hasProperty('keystorePass') ? "$keystorePass" : '' parameters { arg('_admin', "hxb6b5791be0b5ef67063b3c10b840fb81514db2fd") - arg('_relayers', "[]") } } \ No newline at end of file diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index c0bd8801..784bd9c4 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -45,12 +45,9 @@ public class RelayAggregator { private final BranchDB> signatures = Context.newBranchDB("signatures", byte[].class); - public RelayAggregator(Address _admin, Address[] _relayers) { + public RelayAggregator(Address _admin) { if (admin.get() == null) { admin.set(_admin); - for (Address relayer : _relayers) { - relayers.add(relayer); - } signatureThreshold.set(DEFAULT_SIGNATURE_THRESHOLD); } } diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index 8e8d6ee1..1a2e798b 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -62,9 +62,11 @@ void setup() throws Exception { relayerFour = KeyWallet.create(); relayerFourAc = sm.getAccount(Address.fromString(relayerFour.getAddress().toString())); + aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress()); + Address[] relayers = new Address[] { relayerOneAc.getAddress(), relayerTwoAc.getAddress(), relayerThreeAc.getAddress() }; - aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress(), relayers); + aggregator.invoke(adminAc, "addRelayers", (Object) relayers); aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance()); aggregator.setInstance(aggregatorSpy); From 8e8c581c17a42658b6d7a26b48c85bb5bff6d816 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Wed, 2 Oct 2024 21:30:59 +0545 Subject: [PATCH 12/18] refactor: add lookup for relayers --- .../relay/aggregator/RelayAggregator.java | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 784bd9c4..416786d3 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -39,6 +39,7 @@ public class RelayAggregator { private final VarDB
admin = Context.newVarDB("admin", Address.class); private final ArrayDB
relayers = Context.newArrayDB("relayers", Address.class); + private final DictDB relayersLookup = Context.newDictDB("relayersLookup", Boolean.class); private final DictDB packets = Context.newDictDB("packets", Packet.class); @@ -89,16 +90,11 @@ public void addRelayers(Address[] newRelayers) { Context.require(newRelayers != null && newRelayers.length != 0, "new relayers cannot be empty"); - HashMap existingRelayers = new HashMap(); - for (int i = 0; i < relayers.size(); i++) { - Address relayer = relayers.get(i); - existingRelayers.put(relayer, true); - } - for (Address newRelayer : newRelayers) { - if (!existingRelayers.containsKey(newRelayer)) { + Boolean exits = relayersLookup.get(newRelayer); + if (exits == null) { relayers.add(newRelayer); - existingRelayers.put(newRelayer, true); + relayersLookup.set(newRelayer, true); } } } @@ -118,6 +114,7 @@ public void removeRelayers(Address[] relayersToBeRemoved) { for (Address relayerToBeRemoved : relayersToBeRemoved) { if (existingRelayers.containsKey(relayerToBeRemoved)) { + relayersLookup.set(relayerToBeRemoved, null); Address top = relayers.pop(); if (!top.equals(relayerToBeRemoved)) { Integer pos = existingRelayers.get(relayerToBeRemoved); @@ -249,15 +246,8 @@ private void adminOnly() { private void relayersOnly() { Address caller = Context.getCaller(); - Boolean isRelayer = false; - for (int i = 0; i < relayers.size(); i++) { - Address relayer = relayers.get(i); - if (relayer.equals(caller)) { - isRelayer = true; - break; - } - } - Context.require(isRelayer, "Unauthorized: caller is not a registered relayer"); + Boolean isRelayer = relayersLookup.get(caller); + Context.require(isRelayer != null && isRelayer, "Unauthorized: caller is not a registered relayer"); } private Boolean signatureThresholdReached(String pktID) { From 555f5eb2bd46087753762faf84838c8a209c9e55 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Fri, 4 Oct 2024 12:55:47 +0545 Subject: [PATCH 13/18] fix: handle packet registration and signature submission in a single method --- .../relay/aggregator/RelayAggregator.java | 44 +++--- .../relay/aggregator/RelayAggregatorTest.java | 126 ++++-------------- 2 files changed, 43 insertions(+), 127 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 416786d3..fd3a8ccd 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -126,44 +126,30 @@ public void removeRelayers(Address[] relayersToBeRemoved) { } @External - public void registerPacket( + public void submitPacket( String srcNetwork, String contractAddress, BigInteger srcSn, BigInteger srcHeight, String dstNetwork, - byte[] data) { - - adminOnly(); - - Packet pkt = new Packet(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - String id = pkt.getId(); - - Context.require(packets.get(id) == null, "Packet already exists"); - - packets.set(id, pkt); - - PacketRegistered( - pkt.getSrcNetwork(), - pkt.getContractAddress(), - pkt.getSrcSn(), - pkt.getSrcHeight(), - pkt.getDstNetwork(), - pkt.getData()); - } - - @External - public void acknowledgePacket( - String srcNetwork, - String contractAddress, - BigInteger srcSn, + byte[] data, byte[] signature) { relayersOnly(); - String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); - Packet pkt = packets.get(pktID); - Context.require(pkt != null, "Packet not registered"); + Packet pkt = new Packet(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); + String pktID = pkt.getId(); + + if (packets.get(pktID) == null) { + packets.set(pktID, pkt); + PacketRegistered( + pkt.getSrcNetwork(), + pkt.getContractAddress(), + pkt.getSrcSn(), + pkt.getSrcHeight(), + pkt.getDstNetwork(), + pkt.getData()); + } byte[] existingSign = signatures.at(pktID).get(Context.getCaller()); Context.require(existingSign == null, "Signature already exists"); diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index 1a2e798b..d043ea0d 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -64,8 +64,9 @@ void setup() throws Exception { aggregator = sm.deploy(adminAc, RelayAggregator.class, adminAc.getAddress()); - Address[] relayers = new Address[] { relayerOneAc.getAddress(), relayerTwoAc.getAddress(), + Address[] relayers = new Address[] { adminAc.getAddress(), relayerOneAc.getAddress(), relayerTwoAc.getAddress(), relayerThreeAc.getAddress() }; + aggregator.invoke(adminAc, "addRelayers", (Object) relayers); aggregatorSpy = (RelayAggregator) spy(aggregator.getInstance()); @@ -153,7 +154,7 @@ public void testRemoveRelayers() { } assertTrue(removed); - assertEquals(updatedRelayers[0], relayerThreeAc.getAddress()); + assertEquals(updatedRelayers[1], relayerThreeAc.getAddress()); } @Test @@ -170,57 +171,7 @@ public void testRemoveRelayers_unauthorised() { } @Test - public void testRegisterPacket() { - String srcNetwork = "0x2.icon"; - String dstNetwork = "sui"; - BigInteger srcSn = BigInteger.ONE; - BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; - byte[] data = new byte[] { 0x01, 0x02 }; - - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - - verify(aggregatorSpy).PacketRegistered(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - } - - @Test - public void testRegisterPacket_nullArg() { - String srcNetwork = null; - String dstNetwork = "sui"; - BigInteger srcSn = BigInteger.ONE; - BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; - byte[] data = new byte[] { 0x01, 0x02 }; - - Executable action = () -> aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, - srcHeight, dstNetwork, data); - - UserRevertedException e = assertThrows(UserRevertedException.class, action); - - assertEquals("Reverted(0): srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, and data cannot be null", - e.getMessage()); - } - - @Test - public void testRegisterPacket_duplicate() { - String srcNetwork = "0x2.icon"; - String dstNetwork = "sui"; - BigInteger srcSn = BigInteger.ONE; - BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; - byte[] data = new byte[] { 0x01, 0x02 }; - - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - - Executable action = () -> aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, - srcHeight, dstNetwork, data); - UserRevertedException e = assertThrows(UserRevertedException.class, action); - - assertEquals("Reverted(0): Packet already exists", e.getMessage()); - } - - @Test - public void testAcknowledgePacket() throws Exception { + public void testSubmitPacket() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; @@ -228,19 +179,19 @@ public void testAcknowledgePacket() throws Exception { String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, sign); + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + sign); String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + verify(aggregatorSpy).PacketRegistered(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); verify(aggregatorSpy).setSignature(pktID, relayerOneAc.getAddress(), sign); } @Test - public void testAcknowledgePacket_thresholdReached() throws Exception { + public void testSubmitPacket_thresholdReached() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; @@ -248,33 +199,32 @@ public void testAcknowledgePacket_thresholdReached() throws Exception { String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - byte[] dataHash = Context.hash("sha-256", data); - byte[] signOne = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, signOne); + byte[] signAdmin = admin.sign(dataHash); + aggregator.invoke(adminAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + signAdmin); - byte[] signTwo = relayerTwo.sign(dataHash); - aggregator.invoke(relayerTwoAc, "acknowledgePacket", srcNetwork, - contractAddress, srcSn, signTwo); + byte[] signOne = relayerOne.sign(dataHash); + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + signOne); byte[][] sigs = new byte[2][]; - sigs[0] = signOne; - sigs[1] = signTwo; + sigs[0] = signAdmin; + sigs[1] = signOne; byte[] encodedSigs = RelayAggregator.serializeSignatures(sigs); byte[][] decodedSigs = RelayAggregator.deserializeSignatures(encodedSigs); - assertArrayEquals(signOne, decodedSigs[0]); - assertArrayEquals(signTwo, decodedSigs[1]); + assertArrayEquals(signAdmin, decodedSigs[0]); + assertArrayEquals(signOne, decodedSigs[1]); verify(aggregatorSpy).PacketAcknowledged(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, encodedSigs); } @Test - public void testAcknowledgePacket_unauthorized() throws Exception { + public void testSubmitPacket_unauthorized() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; @@ -282,14 +232,11 @@ public void testAcknowledgePacket_unauthorized() throws Exception { String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerFour.sign(dataHash); - Executable action = () -> aggregator.invoke(relayerFourAc, "acknowledgePacket", srcNetwork, contractAddress, - srcSn, - sign); + Executable action = () -> aggregator.invoke(relayerFourAc, "submitPacket", srcNetwork, contractAddress, srcSn, + srcHeight, dstNetwork, data, sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Unauthorized: caller is not a registered relayer", @@ -297,7 +244,7 @@ public void testAcknowledgePacket_unauthorized() throws Exception { } @Test - public void testAcknowledgePacket_duplicate() throws Exception { + public void testSubmitPacket_duplicate() throws Exception { String srcNetwork = "0x2.icon"; String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; @@ -305,34 +252,17 @@ public void testAcknowledgePacket_duplicate() throws Exception { String contractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; - aggregator.invoke(adminAc, "registerPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); - byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, srcSn, sign); + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, + data, sign); - Executable action = () -> aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, - srcSn, - sign); + Executable action = () -> aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, + srcHeight, dstNetwork, + data, sign); + ; UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Signature already exists", e.getMessage()); } - - @Test - public void testAcknowledgePacket_packetUnregistered() throws Exception { - String srcNetwork = "0x2.icon"; - BigInteger srcSn = BigInteger.ONE; - String contractAddress = "hxjuiod"; - byte[] data = new byte[] { 0x01, 0x02 }; - - byte[] dataHash = Context.hash("sha-256", data); - byte[] sign = relayerOne.sign(dataHash); - - Executable action = () -> aggregator.invoke(relayerOneAc, "acknowledgePacket", srcNetwork, contractAddress, - srcSn, - sign); - UserRevertedException e = assertThrows(UserRevertedException.class, action); - assertEquals("Reverted(0): Packet not registered", e.getMessage()); - } } From 7291e2c6f12e64822403f1f4a869ec90e44a9b7e Mon Sep 17 00:00:00 2001 From: sherpalden Date: Fri, 4 Oct 2024 13:42:40 +0545 Subject: [PATCH 14/18] fix: add external method to check if packet is already submitted --- .../relay/aggregator/RelayAggregator.java | 11 +++++++ .../relay/aggregator/RelayAggregatorTest.java | 31 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index fd3a8ccd..3e2039fc 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -125,6 +125,17 @@ public void removeRelayers(Address[] relayersToBeRemoved) { } } + @External(readonly = true) + public boolean packetSubmitted( + Address relayer, + String srcNetwork, + String contractAddress, + BigInteger srcSn) { + String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + byte[] existingSign = signatures.at(pktID).get(relayer); + return existingSign != null; + } + @External public void submitPacket( String srcNetwork, diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index d043ea0d..ac8583a2 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -170,6 +170,37 @@ public void testRemoveRelayers_unauthorised() { assertEquals("Reverted(0): Unauthorized: caller is not the leader relayer", e.getMessage()); } + @Test + public void testPacketSubmitted_true() throws Exception { + String srcNetwork = "0x2.icon"; + String dstNetwork = "sui"; + BigInteger srcSn = BigInteger.ONE; + BigInteger srcHeight = BigInteger.ONE; + String contractAddress = "hxjuiod"; + byte[] data = new byte[] { 0x01, 0x02 }; + + byte[] dataHash = Context.hash("sha-256", data); + byte[] sign = relayerOne.sign(dataHash); + + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + sign); + + boolean submitted = (boolean) aggregator.call("packetSubmitted", relayerOneAc.getAddress(), srcNetwork, + contractAddress, srcSn); + assertEquals(submitted, true); + } + + @Test + public void testPacketSubmitted_false() throws Exception { + String srcNetwork = "0x2.icon"; + BigInteger srcSn = BigInteger.ONE; + String contractAddress = "hxjuiod"; + + boolean submitted = (boolean) aggregator.call("packetSubmitted", relayerOneAc.getAddress(), srcNetwork, + contractAddress, srcSn); + assertEquals(submitted, false); + } + @Test public void testSubmitPacket() throws Exception { String srcNetwork = "0x2.icon"; From c11c34a41aa0d3462aa5709ac65e926f8c1549e9 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Fri, 4 Oct 2024 15:49:29 +0545 Subject: [PATCH 15/18] fix: add dstContractAddress info in packet --- .../main/java/relay/aggregator/Packet.java | 46 +++++++++++----- .../relay/aggregator/RelayAggregator.java | 27 +++++---- .../relay/aggregator/RelayAggregatorTest.java | 55 ++++++++++++------- 3 files changed, 83 insertions(+), 45 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java index e481a235..0568de37 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/Packet.java @@ -16,7 +16,7 @@ public class Packet { /** * The contract address on the source network (chain). */ - private final String contractAddress; + private final String srcContractAddress; /** * The sequence number of the packet in the source network (chain). @@ -33,6 +33,11 @@ public class Packet { */ private final String dstNetwork; + /** + * The contract address on the destination network (chain). + */ + private final String dstContractAddress; + /** * The payload data associated with this packet. */ @@ -47,29 +52,33 @@ public class Packet { * @param dstNetwork the ID of the destination network (chain). * @param data the payload data for this packet. * @throws IllegalArgumentException if {@code srcNetwork}, - * {@code contractAddress}, {@code srcSn}, + * {@code srcContractAddress}, {@code srcSn}, * {@code srcHeight}, - * {@code dstNetwork}, or {@code data} is + * {@code dstNetwork}, + * {@code dstContractAddress}, or {@code data} + * is * {@code null}. */ - public Packet(String srcNetwork, String contractAddress, BigInteger srcSn, BigInteger srcHeight, String dstNetwork, + public Packet(String srcNetwork, String srcContractAddress, BigInteger srcSn, BigInteger srcHeight, + String dstNetwork, String dstContractAddress, byte[] data) { - Boolean isIllegalArg = srcNetwork == null || contractAddress == null || contractAddress == null || srcSn == null - || srcHeight == null || dstNetwork == null || data == null; + Boolean isIllegalArg = srcNetwork == null || srcContractAddress == null || srcSn == null + || 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.contractAddress = contractAddress; + this.srcContractAddress = srcContractAddress; this.srcSn = srcSn; this.srcHeight = srcHeight; this.dstNetwork = dstNetwork; + this.dstContractAddress = dstContractAddress; this.data = data; } public String getId() { - return createId(this.srcNetwork, this.contractAddress, this.srcSn); + return createId(this.srcNetwork, this.srcContractAddress, this.srcSn); } public static String createId(String srcNetwork, String contractAddress, BigInteger srcSn) { @@ -88,10 +97,10 @@ public String getSrcNetwork() { /** * Returns the contract address on the source network (chain). * - * @return the contract address. + * @return the source contract address. */ - public String getContractAddress() { - return contractAddress; + public String getSrcContractAddress() { + return srcContractAddress; } /** @@ -121,6 +130,15 @@ public String getDstNetwork() { return dstNetwork; } + /** + * Returns the contract address on the destination network (chain). + * + * @return the destination contract address. + */ + public String getDstContractAddress() { + return dstContractAddress; + } + /** * Returns a copy of the data associated with this packet. * @@ -131,12 +149,13 @@ public byte[] getData() { } public static void writeObject(ObjectWriter w, Packet p) { - w.beginList(6); + w.beginList(7); w.write(p.srcNetwork); - w.write(p.contractAddress); + w.write(p.srcContractAddress); w.write(p.srcSn); w.write(p.srcHeight); w.write(p.dstNetwork); + w.write(p.dstContractAddress); w.writeNullable(p.data); w.end(); } @@ -149,6 +168,7 @@ public static Packet readObject(ObjectReader r) { r.readBigInteger(), r.readBigInteger(), r.readString(), + r.readString(), r.readNullable(byte[].class)); r.end(); return p; diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 3e2039fc..18dca0df 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -129,9 +129,9 @@ public void removeRelayers(Address[] relayersToBeRemoved) { public boolean packetSubmitted( Address relayer, String srcNetwork, - String contractAddress, + String srcContractAddress, BigInteger srcSn) { - String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + String pktID = Packet.createId(srcNetwork, srcContractAddress, srcSn); byte[] existingSign = signatures.at(pktID).get(relayer); return existingSign != null; } @@ -139,26 +139,28 @@ public boolean packetSubmitted( @External public void submitPacket( String srcNetwork, - String contractAddress, + String srcContractAddress, BigInteger srcSn, BigInteger srcHeight, String dstNetwork, + String dstContractAddress, byte[] data, byte[] signature) { relayersOnly(); - Packet pkt = new Packet(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); + Packet pkt = new Packet(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, dstContractAddress, data); String pktID = pkt.getId(); if (packets.get(pktID) == null) { packets.set(pktID, pkt); PacketRegistered( pkt.getSrcNetwork(), - pkt.getContractAddress(), + pkt.getSrcContractAddress(), pkt.getSrcSn(), pkt.getSrcHeight(), pkt.getDstNetwork(), + pkt.getDstContractAddress(), pkt.getData()); } @@ -168,22 +170,23 @@ public void submitPacket( setSignature(pktID, Context.getCaller(), signature); if (signatureThresholdReached(pktID)) { - byte[][] sigs = getSignatures(srcNetwork, contractAddress, srcSn); + byte[][] sigs = getSignatures(srcNetwork, srcContractAddress, srcSn); byte[] encodedSigs = serializeSignatures(sigs); PacketAcknowledged( pkt.getSrcNetwork(), - pkt.getContractAddress(), + pkt.getSrcContractAddress(), pkt.getSrcSn(), pkt.getSrcHeight(), pkt.getDstNetwork(), + pkt.getDstContractAddress(), pkt.getData(), encodedSigs); removePacket(pktID); } } - private byte[][] getSignatures(String srcNetwork, String contractAddress, BigInteger srcSn) { - String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); + private byte[][] getSignatures(String srcNetwork, String srcContractAddress, BigInteger srcSn) { + String pktID = Packet.createId(srcNetwork, srcContractAddress, srcSn); DictDB signDict = signatures.at(pktID); ArrayList signatureList = new ArrayList(); @@ -272,20 +275,22 @@ private void removePacket(String pktID) { @EventLog(indexed = 2) public void PacketRegistered( String srcNetwork, - String contractAddress, + String srcContractAddress, BigInteger srcSn, BigInteger srcHeight, String dstNetwork, + String dstContractAddress, byte[] data) { } @EventLog(indexed = 2) public void PacketAcknowledged( String srcNetwork, - String contractAddress, + String srcContractAddress, BigInteger srcSn, BigInteger srcHeight, String dstNetwork, + String dstContractAddress, byte[] data, byte[] signatures) { } diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index ac8583a2..f06fae8d 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -176,17 +176,19 @@ public void testPacketSubmitted_true() throws Exception { String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; + String srcContractAddress = "hxjuiod"; + String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, data, sign); boolean submitted = (boolean) aggregator.call("packetSubmitted", relayerOneAc.getAddress(), srcNetwork, - contractAddress, srcSn); + srcContractAddress, srcSn); assertEquals(submitted, true); } @@ -194,10 +196,10 @@ public void testPacketSubmitted_true() throws Exception { public void testPacketSubmitted_false() throws Exception { String srcNetwork = "0x2.icon"; BigInteger srcSn = BigInteger.ONE; - String contractAddress = "hxjuiod"; + String srcContractAddress = "hxjuiod"; boolean submitted = (boolean) aggregator.call("packetSubmitted", relayerOneAc.getAddress(), srcNetwork, - contractAddress, srcSn); + srcContractAddress, srcSn); assertEquals(submitted, false); } @@ -207,17 +209,20 @@ public void testSubmitPacket() throws Exception { String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; + String srcContractAddress = "hxjuiod"; + String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, data, sign); - String pktID = Packet.createId(srcNetwork, contractAddress, srcSn); - verify(aggregatorSpy).PacketRegistered(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data); + String pktID = Packet.createId(srcNetwork, srcContractAddress, srcSn); + verify(aggregatorSpy).PacketRegistered(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, data); verify(aggregatorSpy).setSignature(pktID, relayerOneAc.getAddress(), sign); } @@ -227,17 +232,21 @@ public void testSubmitPacket_thresholdReached() throws Exception { String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; + String srcContractAddress = "hxjuiod"; + String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; byte[] dataHash = Context.hash("sha-256", data); byte[] signAdmin = admin.sign(dataHash); - aggregator.invoke(adminAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + aggregator.invoke(adminAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, data, signAdmin); byte[] signOne = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, + data, signOne); byte[][] sigs = new byte[2][]; @@ -250,7 +259,8 @@ public void testSubmitPacket_thresholdReached() throws Exception { assertArrayEquals(signAdmin, decodedSigs[0]); assertArrayEquals(signOne, decodedSigs[1]); - verify(aggregatorSpy).PacketAcknowledged(srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, data, + verify(aggregatorSpy).PacketAcknowledged(srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, data, encodedSigs); } @@ -260,14 +270,16 @@ public void testSubmitPacket_unauthorized() throws Exception { String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; + String srcContractAddress = "hxjuiod"; + String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerFour.sign(dataHash); - Executable action = () -> aggregator.invoke(relayerFourAc, "submitPacket", srcNetwork, contractAddress, srcSn, - srcHeight, dstNetwork, data, sign); + Executable action = () -> aggregator.invoke(relayerFourAc, "submitPacket", srcNetwork, srcContractAddress, + srcSn, + srcHeight, dstNetwork, dstContractAddress, data, sign); UserRevertedException e = assertThrows(UserRevertedException.class, action); assertEquals("Reverted(0): Unauthorized: caller is not a registered relayer", @@ -280,17 +292,18 @@ public void testSubmitPacket_duplicate() throws Exception { String dstNetwork = "sui"; BigInteger srcSn = BigInteger.ONE; BigInteger srcHeight = BigInteger.ONE; - String contractAddress = "hxjuiod"; + String srcContractAddress = "hxjuiod"; + String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); - aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, srcHeight, dstNetwork, - data, sign); + aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, srcHeight, dstNetwork, + dstContractAddress, data, sign); - Executable action = () -> aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, contractAddress, srcSn, - srcHeight, dstNetwork, + Executable action = () -> aggregator.invoke(relayerOneAc, "submitPacket", srcNetwork, srcContractAddress, srcSn, + srcHeight, dstNetwork, dstContractAddress, data, sign); ; UserRevertedException e = assertThrows(UserRevertedException.class, action); From 7d7031aa05182e145be3f00ee9531d47baf44750 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Mon, 7 Oct 2024 12:19:35 +0545 Subject: [PATCH 16/18] fix: for a single relayer cluster do not emit PacketRegistered event --- .../relay/aggregator/RelayAggregator.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 18dca0df..12203c70 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -67,6 +67,7 @@ public Address getAdmin() { @External public void setSignatureThreshold(int threshold) { adminOnly(); + Context.require(threshold >= 1, "invalid threshold value: should be >= 1"); signatureThreshold.set(threshold); } @@ -154,14 +155,17 @@ public void submitPacket( if (packets.get(pktID) == null) { packets.set(pktID, pkt); - PacketRegistered( - pkt.getSrcNetwork(), - pkt.getSrcContractAddress(), - pkt.getSrcSn(), - pkt.getSrcHeight(), - pkt.getDstNetwork(), - pkt.getDstContractAddress(), - pkt.getData()); + if (signatureThreshold.get() > 1) { + PacketRegistered( + pkt.getSrcNetwork(), + pkt.getSrcContractAddress(), + pkt.getSrcSn(), + pkt.getSrcHeight(), + pkt.getDstNetwork(), + pkt.getDstContractAddress(), + pkt.getData()); + } + } byte[] existingSign = signatures.at(pktID).get(Context.getCaller()); From 08508f562d07c76316a986334b7403e9a49851c0 Mon Sep 17 00:00:00 2001 From: sherpalden Date: Mon, 7 Oct 2024 17:46:27 +0545 Subject: [PATCH 17/18] fix: set default threshold to 1 && rf: relayers add and remove --- .../relay/aggregator/RelayAggregator.java | 47 +++++++++++-------- .../relay/aggregator/RelayAggregatorTest.java | 24 +++++++++- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index 12203c70..f4f41542 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -29,10 +29,9 @@ import score.annotation.External; import score.ObjectReader; import scorex.util.ArrayList; -import scorex.util.HashMap; public class RelayAggregator { - private final Integer DEFAULT_SIGNATURE_THRESHOLD = 2; + private final Integer DEFAULT_SIGNATURE_THRESHOLD = 1; private final VarDB signatureThreshold = Context.newVarDB("signatureThreshold", Integer.class); @@ -50,12 +49,20 @@ public RelayAggregator(Address _admin) { if (admin.get() == null) { admin.set(_admin); signatureThreshold.set(DEFAULT_SIGNATURE_THRESHOLD); + addRelayer(_admin); } } @External public void setAdmin(Address _admin) { adminOnly(); + + // add new admin as relayer + addRelayer(_admin); + + // remove old admin from relayer list + removeRelayer(admin.get()); + admin.set(_admin); } @@ -94,8 +101,7 @@ public void addRelayers(Address[] newRelayers) { for (Address newRelayer : newRelayers) { Boolean exits = relayersLookup.get(newRelayer); if (exits == null) { - relayers.add(newRelayer); - relayersLookup.set(newRelayer, true); + addRelayer(newRelayer); } } } @@ -107,22 +113,9 @@ public void removeRelayers(Address[] relayersToBeRemoved) { Context.require(relayersToBeRemoved != null && relayersToBeRemoved.length != 0, "relayers to be removed cannot be empty"); - HashMap existingRelayers = new HashMap(); - for (int i = 0; i < relayers.size(); i++) { - Address relayer = relayers.get(i); - existingRelayers.put(relayer, i); - } - for (Address relayerToBeRemoved : relayersToBeRemoved) { - if (existingRelayers.containsKey(relayerToBeRemoved)) { - relayersLookup.set(relayerToBeRemoved, null); - Address top = relayers.pop(); - if (!top.equals(relayerToBeRemoved)) { - Integer pos = existingRelayers.get(relayerToBeRemoved); - relayers.set(pos, top); - } - existingRelayers.remove(relayerToBeRemoved); - } + Context.require(relayerToBeRemoved != admin.get(), "admin cannot be removed from relayers list"); + removeRelayer(relayerToBeRemoved); } } @@ -254,6 +247,22 @@ private void relayersOnly() { Context.require(isRelayer != null && isRelayer, "Unauthorized: caller is not a registered relayer"); } + private void addRelayer(Address newRelayer) { + 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; + } + } + } + private Boolean signatureThresholdReached(String pktID) { int noOfSignatures = 0; for (int i = 0; i < relayers.size(); i++) { diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index f06fae8d..d44e84a2 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -1,6 +1,7 @@ package relay.aggregator; import java.math.BigInteger; +import java.util.Arrays; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -19,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.spy; @@ -75,11 +77,21 @@ void setup() throws Exception { @Test public void testSetAdmin() { + Address oldAdmin = (Address) aggregator.call("getAdmin"); + Account newAdminAc = sm.createAccount(); aggregator.invoke(adminAc, "setAdmin", newAdminAc.getAddress()); - Address result = (Address) aggregator.call("getAdmin"); - assertEquals(newAdminAc.getAddress(), result); + Address newAdmin = (Address) aggregator.call("getAdmin"); + assertEquals(newAdminAc.getAddress(), newAdmin); + + Address[] relayers = (Address[]) aggregator.call("getRelayers"); + + boolean containsNewAdmin = Arrays.asList(relayers).contains(newAdmin); + boolean containsOldAdmin = Arrays.asList(relayers).contains(oldAdmin); + + assertTrue(containsNewAdmin); + assertFalse(containsOldAdmin); } @Test @@ -180,6 +192,8 @@ public void testPacketSubmitted_true() throws Exception { String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; + aggregator.invoke(adminAc, "setSignatureThreshold", 2); + byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); @@ -213,6 +227,8 @@ public void testSubmitPacket() throws Exception { String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; + aggregator.invoke(adminAc, "setSignatureThreshold", 2); + byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); @@ -236,6 +252,8 @@ public void testSubmitPacket_thresholdReached() throws Exception { String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; + aggregator.invoke(adminAc, "setSignatureThreshold", 2); + byte[] dataHash = Context.hash("sha-256", data); byte[] signAdmin = admin.sign(dataHash); @@ -296,6 +314,8 @@ public void testSubmitPacket_duplicate() throws Exception { String dstContractAddress = "hxjuiod"; byte[] data = new byte[] { 0x01, 0x02 }; + aggregator.invoke(adminAc, "setSignatureThreshold", 2); + byte[] dataHash = Context.hash("sha-256", data); byte[] sign = relayerOne.sign(dataHash); From 7034223ef5e2295a118efbd346cdfd97c87f191c Mon Sep 17 00:00:00 2001 From: sherpalden Date: Fri, 25 Oct 2024 14:05:53 +0545 Subject: [PATCH 18/18] fix: add setRelayers method --- .../relay/aggregator/RelayAggregator.java | 59 +++++---- .../relay/aggregator/RelayAggregatorTest.java | 125 +++++++++++------- 2 files changed, 113 insertions(+), 71 deletions(-) diff --git a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java index f4f41542..ec475a99 100644 --- a/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java +++ b/contracts/javascore/aggregator/src/main/java/relay/aggregator/RelayAggregator.java @@ -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; @@ -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); @@ -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); } @@ -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 newRelayersMap = new HashMap(); + 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) @@ -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; + } } } } diff --git a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java index d44e84a2..0da3391f 100644 --- a/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java +++ b/contracts/javascore/aggregator/src/test/java/relay/aggregator/RelayAggregatorTest.java @@ -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; @@ -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); @@ -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
updatedRelayersSet = new HashSet
(); + for (Address rlr : updatedRelayers) { + updatedRelayersSet.add(rlr); + } + + HashSet
expectedRelayersSet = new HashSet
(); + 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 @@ -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); } @@ -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); } @@ -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); } @@ -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); @@ -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); } @@ -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); @@ -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); ;