Get retiring federator public key optional

rskj-core/src/main/java/co/rsk/peg/Bridge.java

@@ -19,6 +19,7 @@
 
 import static co.rsk.peg.BridgeSerializationUtils.deserializeRskTxHash;
 import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP417;
+import static org.ethereum.util.ByteUtil.EMPTY_BYTE_ARRAY;
 
 import co.rsk.bitcoinj.core.*;
 import co.rsk.bitcoinj.script.Script;
@@ -479,7 +480,7 @@ private byte[] calculateVoidReturnValue() {
         if (shouldReturnNullOnVoidMethods()) {
             return null;
         }
-        return new byte[]{};
+        return EMPTY_BYTE_ARRAY;
     }
 
     private boolean shouldReturnNullOnVoidMethods() {
@@ -887,14 +888,8 @@ public byte[] getRetiringFederatorPublicKey(Object[] args) {
         logger.trace("getRetiringFederatorPublicKey");
 
         int index = ((BigInteger) args[0]).intValue();
-        byte[] publicKey = bridgeSupport.getRetiringFederatorBtcPublicKey(index);
-
-        if (publicKey == null) {
-            // Empty array is returned when public key is not found or there's no retiring federation
-            return new byte[]{};
-        }
-
-        return publicKey;
+        Optional<BtcECKey> federatorBtcPublicKey = bridgeSupport.getRetiringFederatorBtcPublicKey(index);
+        return federatorBtcPublicKey.map(BtcECKey::getPubKey).orElse(EMPTY_BYTE_ARRAY);
     }
 
     public byte[] getRetiringFederatorPublicKeyOfType(Object[] args) throws VMException {
@@ -914,7 +909,7 @@ public byte[] getRetiringFederatorPublicKeyOfType(Object[] args) throws VMExcept
 
         if (publicKey == null) {
             // Empty array is returned when public key is not found or there's no retiring federation
-            return new byte[]{};
+            return EMPTY_BYTE_ARRAY;
         }
 
         return publicKey;
@@ -1019,7 +1014,7 @@ public byte[] getPendingFederationHashSerialized(Object[] args) {
 
         if (hash == null) {
             // Empty array is returned when pending federation is not present
-            return new byte[]{};
+            return EMPTY_BYTE_ARRAY;
         }
 
         return hash.getBytes();
@@ -1039,7 +1034,7 @@ public byte[] getPendingFederatorPublicKey(Object[] args) {
 
         if (publicKey == null) {
             // Empty array is returned when public key is not found
-            return new byte[]{};
+            return EMPTY_BYTE_ARRAY;
         }
 
         return publicKey;
@@ -1062,7 +1057,7 @@ public byte[] getPendingFederatorPublicKeyOfType(Object[] args) throws VMExcepti
 
         if (publicKey == null) {
             // Empty array is returned when public key is not found
-            return new byte[]{};
+            return EMPTY_BYTE_ARRAY;
         }
 
         return publicKey;
@@ -1194,7 +1189,7 @@ public byte[] getProposedFederatorPublicKeyOfType(Object[] args) throws VMExcept
         }
 
         return publicKey
-            .orElse(new byte[]{});
+            .orElse(EMPTY_BYTE_ARRAY);
     }
 
     public Integer getLockWhitelistSize(Object[] args) {
@@ -1324,7 +1319,7 @@ public byte[] getActivePowpegRedeemScript(Object[] args) {
         try {
             Optional<Script> redeemScript = bridgeSupport.getActiveFederationRedeemScript();
             logger.debug("[getActivePowpegRedeemScript] finished");
-            return redeemScript.orElse(new Script(new byte[]{})).getProgram();
+            return redeemScript.orElse(new Script(EMPTY_BYTE_ARRAY)).getProgram();
         } catch (Exception ex) {
             logger.warn("[getActivePowpegRedeemScript] something failed", ex);
             throw ex;

rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java

@@ -2346,7 +2346,7 @@ public Optional<Integer> getRetiringFederationThreshold() {
         return federationSupport.getRetiringFederationThreshold();
     }
 
-    public byte[] getRetiringFederatorBtcPublicKey(int index) {
+    public Optional<BtcECKey> getRetiringFederatorBtcPublicKey(int index) {
         return federationSupport.getRetiringFederatorBtcPublicKey(index);
     }
 

rskj-core/src/main/java/co/rsk/peg/federation/FederationSupport.java

@@ -1,6 +1,7 @@
 package co.rsk.peg.federation;
 
 import co.rsk.bitcoinj.core.Address;
+import co.rsk.bitcoinj.core.BtcECKey;
 import co.rsk.bitcoinj.core.UTXO;
 import co.rsk.bitcoinj.script.Script;
 import co.rsk.crypto.Keccak256;
@@ -122,10 +123,12 @@ public interface FederationSupport {
 
     /**
      * Returns the public key of the retiring federation's federator at the given index
+     *
      * @param index the retiring federator's index (zero-based)
-     * @return the retiring federator's public key, null if no retiring federation exists
+     * @return An {@link Optional} containing the retiring federator's public key,
+     * or an empty {@link Optional} if no retiring federation exists.
      */
-    byte[] getRetiringFederatorBtcPublicKey(int index);
+    Optional<BtcECKey> getRetiringFederatorBtcPublicKey(int index);
 
     /**
      * Returns the public key of the given type of the retiring federation's federator at the given index

rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java

@@ -245,10 +245,10 @@ public Optional<Long> getRetiringFederationCreationBlockNumber() {
     }
 
     @Override
-    public byte[] getRetiringFederatorBtcPublicKey(int index) {
+    public Optional<BtcECKey> getRetiringFederatorBtcPublicKey(int index) {
         Optional<Federation> retiringFederation = getRetiringFederation();
         if (retiringFederation.isEmpty()) {
-            return null;
+            return Optional.empty();
         }
 
         List<BtcECKey> publicKeys = retiringFederation.get().getBtcPublicKeys();
@@ -257,7 +257,8 @@ public byte[] getRetiringFederatorBtcPublicKey(int index) {
             throw new IndexOutOfBoundsException(String.format("Retiring federator index must be between 0 and %d", publicKeys.size() - 1));
         }
 
-        return publicKeys.get(index).getPubKey();
+        BtcECKey retiringFederatorBtcPublicKey = publicKeys.get(index);
+        return Optional.of(retiringFederatorBtcPublicKey);
     }
 
     @Override

rskj-core/src/test/java/co/rsk/peg/BridgeIT.java

@@ -21,6 +21,7 @@
 import static co.rsk.peg.federation.FederationStorageIndexKey.*;
 import static co.rsk.peg.federation.FederationTestUtils.REGTEST_FEDERATION_PRIVATE_KEYS;
 import static org.ethereum.config.blockchain.upgrades.ConsensusRule.*;
+import static org.ethereum.util.ByteUtil.EMPTY_BYTE_ARRAY;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.*;
@@ -42,6 +43,7 @@
 import co.rsk.peg.vote.ABICallSpec;
 import co.rsk.peg.federation.*;
 import co.rsk.test.builders.BlockChainBuilder;
+import co.rsk.test.builders.BridgeBuilder;
 import org.bouncycastle.util.encoders.Hex;
 import org.ethereum.TestUtils;
 import org.ethereum.config.Constants;
@@ -164,6 +166,7 @@ class BridgeIT {
     private static final String DATA = "80af2871";
     private static final String ERR_NOT_FROM_ACTIVE_OR_RETIRING_FED = "The sender is not a member of the active or retiring federations";
     private static final String ERR_NOT_FROM_ACTIVE_RETIRING_OR_PROPOSED_FED = "The sender is not a member of the active, retiring, or proposed federations";
+    private static final BridgeBuilder bridgeBuilder = new BridgeBuilder();
 
     private TestSystemProperties config = new TestSystemProperties();
     private Constants constants;
@@ -173,6 +176,8 @@ class BridgeIT {
     private SignatureCache signatureCache;
     private Bridge bridge;
     private Repository track;
+    private BridgeSupport bridgeSupport;
+
 
     @BeforeEach
     void resetConfigToRegTest() {
@@ -184,6 +189,7 @@ void resetConfigToRegTest() {
         blockFactory = new BlockFactory(activationConfig);
         activationConfigAll = ActivationConfigsForTest.all().forBlock(0);
         signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache());
+        bridgeSupport = mock(BridgeSupport.class);
     }
 
     @Test
@@ -1968,39 +1974,62 @@ void getRetiringFederationCreationBlockNumber() {
         MatcherAssert.assertThat(bridge.getRetiringFederationCreationBlockNumber(new Object[]{}), is(retiringFederationCreationBlockNumber));
     }
 
-    @Test
-    void getRetiringFederatorPublicKey_beforeMultikey() throws Exception {
+    @Test()
+    void getRetiringFederatorPublicKey_beforeMultikey_withARetiringFederation_shouldReturnTheCorrectRetiringFederatorPublicKeyBytes() throws Exception {
+        // Arrange
         doReturn(false).when(activationConfig).isActive(eq(RSKIP123), anyLong());
-        BridgeSupportFactory bridgeSupportFactoryMock = mock(BridgeSupportFactory.class);
 
-        Bridge bridge = new Bridge(BRIDGE_ADDRESS, constants, activationConfig,
-                bridgeSupportFactoryMock, signatureCache);
-        bridge.init(mock(Transaction.class), getGenesisBlock(), createRepository().startTracking(), null, null, null);
+        bridge = bridgeBuilder
+            .bridgeSupport(bridgeSupport)
+            .activationConfig(activationConfig)
+            .contractAddress(BRIDGE_ADDRESS)
+            .constants(constants)
+            .signatureCache(signatureCache)
+            .build();
 
-        BridgeSupport bridgeSupportMock = mock(BridgeSupport.class);
-        when(bridgeSupportFactoryMock.newInstance(any(), any(), any(), any())).thenReturn(bridgeSupportMock);
-        when(bridgeSupportMock.getRetiringFederatorBtcPublicKey(any(int.class))).then((InvocationOnMock invocation) ->
-                BigInteger.valueOf(invocation.<Integer>getArgument(0)).toByteArray());
-        bridge.init(mock(Transaction.class), getGenesisBlock(), createRepository().startTracking(), null, null, null);
+        int retiringFederatorBtcPublicKeyIndex = 0;
+        BtcECKey retiringFederatorBtcPublicKey = RETIRING_FEDERATION_KEYS.get(retiringFederatorBtcPublicKeyIndex);
+        when(bridgeSupport.getRetiringFederatorBtcPublicKey(retiringFederatorBtcPublicKeyIndex)).thenReturn(Optional.of(retiringFederatorBtcPublicKey));
 
+        // Act
+        byte[] executionResult = executeGetRetiringFederatorPublicKeyFunction(bridge, retiringFederatorBtcPublicKeyIndex);
 
-        assertTrue(Arrays.equals(new byte[]{10},
-                (byte[]) BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().decodeResult(
-                        bridge.execute(BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().encode(new Object[]{BigInteger.valueOf(10)}))
-                )[0]
-        ));
+        // Assert
+        assertGetRetiringFederatorPublicKeyExecution(retiringFederatorBtcPublicKey.getPubKey(), executionResult);
+    }
 
-        assertTrue(Arrays.equals(new byte[]{20},
-                (byte[]) BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().decodeResult(
-                        bridge.execute(BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().encode(new Object[]{BigInteger.valueOf(20)}))
-                )[0]
-        ));
+    @Test()
+    void getRetiringFederatorPublicKey_beforeMultikey_withoutARetiringFederation_shouldReturnAnEmptyByteArray() throws Exception {
+        // Arrange
+        doReturn(false).when(activationConfig).isActive(eq(RSKIP123), anyLong());
 
-        assertTrue(Arrays.equals(new byte[]{1, 0},
-                (byte[]) BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().decodeResult(
-                        bridge.execute(BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().encode(new Object[]{BigInteger.valueOf(256)}))
-                )[0]
-        ));
+        bridge = bridgeBuilder
+            .bridgeSupport(bridgeSupport)
+            .activationConfig(activationConfig)
+            .contractAddress(BRIDGE_ADDRESS)
+            .constants(constants)
+            .signatureCache(signatureCache)
+            .build();
+
+        // Act
+        int retiringFederatorBtcPublicKeyIndex = 0;
+        byte[] executionResult = executeGetRetiringFederatorPublicKeyFunction(bridge, retiringFederatorBtcPublicKeyIndex);
+
+        // Assert
+        assertGetRetiringFederatorPublicKeyExecution(EMPTY_BYTE_ARRAY, executionResult);
+    }
+
+    private void assertGetRetiringFederatorPublicKeyExecution(byte[] expectedExecutionResult, byte[] executionResult) {
+        CallTransaction.Function getRetiringFederatorPublicKeyFunction = BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction();
+        byte[] actualResult = (byte[]) getRetiringFederatorPublicKeyFunction.decodeResult(executionResult)[0];
+        assertArrayEquals(expectedExecutionResult, actualResult);
+    }
+
+    private byte[] executeGetRetiringFederatorPublicKeyFunction(Bridge bridge, int federatorIndex) throws VMException {
+        CallTransaction.Function getRetiringFederatorPublicKeyFunction = BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction();
+        BigInteger indexArgument = BigInteger.valueOf(federatorIndex);
+        byte[] getRetiringFederatorPublicKeyCallEncoded = getRetiringFederatorPublicKeyFunction.encode(indexArgument);
+        return bridge.execute(getRetiringFederatorPublicKeyCallEncoded);
     }
 
     @Test
@@ -2014,7 +2043,7 @@ void getRetiringFederatorPublicKey_afterMultikey() throws Exception {
         bridge.init(mock(Transaction.class), getGenesisBlock(), createRepository().startTracking(), null, null, null);
         BridgeSupport bridgeSupportMock = mock(BridgeSupport.class);
 
-        Assertions.assertNull(bridge.execute(BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().encode(new Object[]{BigInteger.valueOf(10)})));
+        Assertions.assertNull(bridge.execute(BridgeMethods.GET_RETIRING_FEDERATOR_PUBLIC_KEY.getFunction().encode(BigInteger.valueOf(10))));
         verify(bridgeSupportMock, never()).getRetiringFederatorBtcPublicKey(any(int.class));
     }
 

rskj-core/src/test/java/co/rsk/peg/BridgeSupportIT.java

@@ -2392,7 +2392,7 @@ void getRetiringFederationMethods_none() {
 
         assertTrue(bridgeSupport.getRetiringFederationSize().isEmpty());
         assertTrue(bridgeSupport.getRetiringFederationThreshold().isEmpty());
-        assertNull(bridgeSupport.getRetiringFederatorBtcPublicKey(0));
+        assertTrue(bridgeSupport.getRetiringFederatorBtcPublicKey(0).isEmpty());
         assertNull(bridgeSupport.getRetiringFederatorPublicKeyOfType(0, FederationMember.KeyType.BTC));
         assertNull(bridgeSupport.getRetiringFederatorPublicKeyOfType(0, FederationMember.KeyType.RSK));
         assertNull(bridgeSupport.getRetiringFederatorPublicKeyOfType(0, FederationMember.KeyType.MST));
@@ -2435,7 +2435,7 @@ void getRetiringFederationMethods_presentNewInactive() {
 
         assertTrue(bridgeSupport.getRetiringFederationSize().isEmpty());
         assertTrue(bridgeSupport.getRetiringFederationThreshold().isEmpty());
-        assertNull(bridgeSupport.getRetiringFederatorBtcPublicKey(0));
+        assertTrue(bridgeSupport.getRetiringFederatorBtcPublicKey(0).isEmpty());
         assertNull(bridgeSupport.getRetiringFederatorPublicKeyOfType(0, FederationMember.KeyType.BTC));
         assertNull(bridgeSupport.getRetiringFederatorPublicKeyOfType(0, FederationMember.KeyType.RSK));
         assertNull(bridgeSupport.getRetiringFederatorPublicKeyOfType(0, FederationMember.KeyType.MST));
@@ -2492,9 +2492,11 @@ void getRetiringFederationMethods_presentNewActive() {
         assertEquals(mockedOldFederation.getAddress().toString(), bridgeSupport.getRetiringFederationAddress().toString());
         List<FederationMember> members = FederationTestUtils.getFederationMembers(4);
         for (int i = 0; i < 4; i++) {
-            assertArrayEquals(
-                members.get(i).getBtcPublicKey().getPubKey(),
-                bridgeSupport.getRetiringFederatorBtcPublicKey(i)
+            Optional<BtcECKey> retiringFederatorBtcPublicKey = bridgeSupport.getRetiringFederatorBtcPublicKey(i);
+            assertTrue(retiringFederatorBtcPublicKey.isPresent());
+            assertEquals(
+                members.get(i).getBtcPublicKey(),
+                retiringFederatorBtcPublicKey.get()
                 );
             assertArrayEquals(
                 members.get(i).getBtcPublicKey().getPubKey(),

rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java

@@ -515,9 +515,11 @@ void getRetiringFederationCreationBlockNumber() {
         @Test
         void getRetiringFederatorBtcPublicKey() {
             BtcECKey publicKey = federation.getBtcPublicKeys().get(0);
+            when(federationSupport.getRetiringFederatorBtcPublicKey(0)).thenReturn(Optional.of(publicKey));
 
-            when(federationSupport.getRetiringFederatorBtcPublicKey(0)).thenReturn(publicKey.getPubKey());
-            assertThat(bridgeSupport.getRetiringFederatorBtcPublicKey(0), is(publicKey.getPubKey()));
+            Optional<BtcECKey> retiringFederatorBtcPublicKey = bridgeSupport.getRetiringFederatorBtcPublicKey(0);
+            assertTrue(retiringFederatorBtcPublicKey.isPresent());
+            assertEquals(retiringFederatorBtcPublicKey.get(), publicKey);
         }
 
         @Test

rskj-core/src/test/java/co/rsk/peg/federation/FederationSupportImplTest.java

@@ -1312,9 +1312,9 @@ void getRetiringFederationCreationBlockNumber_returnsEmpty() {
 
         @Test
         @Tag("getRetiringFederatorBtcPublicKey")
-        void getRetiringFederatorBtcPublicKey_returnsNull() {
-            byte[] retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
-            assertThat(retiringFederatorBtcPublicKey, is(nullValue()));
+        void getRetiringFederatorBtcPublicKey_returnsEmpty() {
+            Optional<BtcECKey> retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
+            assertTrue(retiringFederatorBtcPublicKey.isEmpty());
         }
 
         @Test
@@ -1401,9 +1401,9 @@ void getRetiringFederationCreationBlockNumber_returnsEmpty() {
 
         @Test
         @Tag("getRetiringFederatorBtcPublicKey")
-        void getRetiringFederatorBtcPublicKey_returnsNull() {
-            byte[] retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
-            assertThat(retiringFederatorBtcPublicKey, is(nullValue()));
+        void getRetiringFederatorBtcPublicKey_returnsEmpty() {
+            Optional<BtcECKey> retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
+            assertTrue(retiringFederatorBtcPublicKey.isEmpty());
         }
 
         @Test
@@ -1732,7 +1732,7 @@ void getRetiringFederationCreationBlockNumber_withNewFederationActive_returnsOld
         @ParameterizedTest
         @Tag("getRetiringFederatorBtcPublicKey")
         @MethodSource("newFederationNotActiveActivationArgs")
-        void getRetiringFederatorBtcPublicKey_withNewFederationNotActive_returnsNull(
+        void getRetiringFederatorBtcPublicKey_withNewFederationNotActive_returnsEmpty(
             long currentBlock,
             ActivationConfig.ForBlock activations) {
 
@@ -1746,8 +1746,8 @@ void getRetiringFederatorBtcPublicKey_withNewFederationNotActive_returnsNull(
                 .withActivations(activations)
                 .build();
 
-            byte[] retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
-            assertThat(retiringFederatorBtcPublicKey, is(nullValue()));
+            Optional<BtcECKey> retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
+            assertTrue(retiringFederatorBtcPublicKey.isEmpty());
         }
 
         @ParameterizedTest
@@ -1767,8 +1767,10 @@ void getRetiringFederatorBtcPublicKey_withNewFederationActive_returnsFederatorFr
                 .withActivations(activations)
                 .build();
 
-            byte[] retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
-            assertThat(retiringFederatorBtcPublicKey, is(oldFederation.getBtcPublicKeys().get(0).getPubKey()));
+            Optional<BtcECKey> retiringFederatorBtcPublicKey = federationSupport.getRetiringFederatorBtcPublicKey(0);
+            assertTrue(retiringFederatorBtcPublicKey.isPresent());
+            List<BtcECKey> oldFederationBtcPublicKeys = oldFederation.getBtcPublicKeys();
+            assertEquals(retiringFederatorBtcPublicKey.get(), oldFederationBtcPublicKeys.get(0));
         }
 
         @ParameterizedTest