- Use new switch with arrow labels

- Make logic handling rejected pegin functional using the Optional API - Fix formatting
rsksmart · Jan 13, 2025 · 18fd560 · 18fd560
1 parent 4037ab7
commit 18fd560
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 54 deletions.
diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java
@@ -504,40 +504,45 @@ protected void registerPegIn(
 
         PeginProcessAction peginProcessAction = peginEvaluationResult.getPeginProcessAction();
 
-        if (peginProcessAction == PeginProcessAction.CAN_BE_REGISTERED){
+        if (peginProcessAction == PeginProcessAction.CAN_BE_REGISTERED) {
             logger.debug("[{}] Peg-in is valid, going to register", METHOD_NAME);
             executePegIn(btcTx, peginInformation, totalAmount);
             return;
         }
 
         // If the peg-in cannot be registered means it should be rejected
-        Optional<RejectedPeginReason> rejectedPeginReasonOptional = peginEvaluationResult.getRejectedPeginReason();
-        if (rejectedPeginReasonOptional.isEmpty()) {
-            // This flow should never be reached. There should always be a rejected pegin reason.
-            String message = "Invalid state. No rejected reason was returned for an invalid pegin.";
-            logger.error("[{}] {}", METHOD_NAME, message);
-            throw new IllegalStateException(message);
-        }
-        RejectedPeginReason rejectedPeginReason = rejectedPeginReasonOptional.get();
-        logger.debug("[{}] Rejected peg-in, reason {}", METHOD_NAME, rejectedPeginReason);
-        eventLogger.logRejectedPegin(btcTx, rejectedPeginReason);
+        RejectedPeginReason rejectedPeginReason = peginEvaluationResult.getRejectedPeginReason()
+            .map(reason -> {
+                logger.debug("[{}] Rejected peg-in, reason {}", METHOD_NAME, reason);
+                eventLogger.logRejectedPegin(btcTx, reason);
+                return reason;
+            }).orElseThrow(() -> {
+                // This flow should never be reached. There should always be a rejected pegin reason.
+                String message = "Invalid state. No rejected reason was returned for an invalid pegin.";
+                logger.error("[{}] {}", METHOD_NAME, message);
+                return new IllegalStateException(message);
+            });
 
         switch (peginProcessAction) {
-            case CAN_BE_REFUNDED:
+            case CAN_BE_REFUNDED -> {
                 logger.debug("[{}] Refunding to address {} ", METHOD_NAME,
                     peginInformation.getBtcRefundAddress());
                 generateRejectionRelease(btcTx, peginInformation.getBtcRefundAddress(), rskTxHash,
                     totalAmount);
                 markTxAsProcessed(btcTx);
-                break;
-            case CANNOT_BE_REFUNDED:
+            }
+            case CANNOT_BE_REFUNDED -> {
                 logger.debug("[{}] Nonrefundable transaction {}.", METHOD_NAME, btcTx.getHash());
-                handleNonRefundablePegin(btcTx, peginInformation.getProtocolVersion(), rejectedPeginReason);
-                // Since RSKIP459, rejected peg-ins should be marked as processed
-                if (activations.isActive(RSKIP459)) {
-                    markTxAsProcessed(btcTx);
+                handleNonRefundablePegin(btcTx, peginInformation.getProtocolVersion(),
+                    rejectedPeginReason);
+
+                if (!activations.isActive(RSKIP459)) {
+                    return;
                 }
-                break;
+
+                // Since RSKIP459, rejected peg-ins should be marked as processed
+                markTxAsProcessed(btcTx);
+            }
         }
     }
 
@@ -555,7 +560,8 @@ private void handleNonRefundablePegin(
                 UnrefundablePeginReason.LEGACY_PEGIN_UNDETERMINED_SENDER;
         }
 
-        logger.debug("[handleUnprocessableBtcTx] Unprocessable tx {}. Reason {}", btcTx.getHash(), unrefundablePeginReason);
+        logger.debug("[handleNonRefundablePegin] Nonrefundable tx {}. Reason {}", btcTx.getHash(),
+            unrefundablePeginReason);
         eventLogger.logUnrefundablePegin(btcTx, unrefundablePeginReason);
     }
 

diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRegisterBtcTransactionTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRegisterBtcTransactionTest.java
@@ -1555,7 +1555,7 @@ void pegin_legacy_from_segwit_to_active_fed_cannot_be_processed(
         );
 
         // assert
-        if (activations == fingerrootActivations){
+        if (activations == fingerrootActivations) {
             // BEFORE RSKIP379 REJECTED PEGIN WERE MARKED AS PROCESSED.
             assertLegacyUndeterminedSenderPeginIsRejectedAsPeginV1InvalidPayloadBeforeRSKIP379(btcTransaction);
         } else {

diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRejectedPeginTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportRejectedPeginTest.java
@@ -1,5 +1,6 @@
 package co.rsk.peg;
 
+import static co.rsk.peg.BridgeSupportTestUtil.createValidPmtForTransactions;
 import static co.rsk.peg.BridgeSupportTestUtil.mockChainOfStoredBlocks;
 import static co.rsk.peg.pegin.RejectedPeginReason.INVALID_AMOUNT;
 import static co.rsk.peg.pegin.RejectedPeginReason.PEGIN_V1_INVALID_PAYLOAD;
@@ -40,7 +41,6 @@
 import co.rsk.peg.federation.FederationTestUtils;
 import co.rsk.peg.federation.constants.FederationConstants;
 import co.rsk.peg.lockingcap.LockingCapSupport;
-import co.rsk.peg.pegin.PeginProcessAction;
 import co.rsk.peg.pegin.RejectedPeginReason;
 import co.rsk.peg.pegininstructions.PeginInstructionsProvider;
 import co.rsk.peg.utils.BridgeEventLogger;
@@ -64,7 +64,6 @@
 import org.ethereum.vm.PrecompiledContracts;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -74,9 +73,10 @@ class BridgeSupportRejectedPeginTest {
     private static final BridgeConstants bridgeMainnetConstants = BridgeMainNetConstants.getInstance();
     private static final FederationConstants federationMainnetConstants = bridgeMainnetConstants.getFederationConstants();
     private static final NetworkParameters btcMainnetParams = bridgeMainnetConstants.getBtcParams();
-    private static final ActivationConfig.ForBlock arrowHeadActivations = ActivationConfigsForTest.arrowhead600().forBlock(0);
-    private static final ActivationConfig.ForBlock lovellActivations = ActivationConfigsForTest.lovell700().forBlock(0);
-    /*private static final ActivationConfig.ForBlock activations = ActivationConfigsForTest.all().forBlock(0);*/
+    private static final ActivationConfig.ForBlock arrowHeadActivations = ActivationConfigsForTest.arrowhead600()
+        .forBlock(0);
+    private static final ActivationConfig.ForBlock lovellActivations = ActivationConfigsForTest.lovell700()
+        .forBlock(0);
 
     private static final Coin minimumPeginTxValue = bridgeMainnetConstants.getMinimumPeginTxValue(
         ActivationConfigsForTest.all().forBlock(0));
@@ -122,21 +122,23 @@ void init() throws IOException {
         userAddress = BitcoinTestUtils.createP2PKHAddress(btcMainnetParams, "userAddress");
         NetworkParameters btcParams = bridgeMainnetConstants.getBtcParams();
 
-
         List<BtcECKey> erpPubKeys = federationMainnetConstants.getErpFedPubKeysList();
         long activationDelay = federationMainnetConstants.getErpFedActivationDelay();
 
         List<BtcECKey> retiringFedSigners = BitcoinTestUtils.getBtcEcKeysFromSeeds(
             new String[]{"fa04", "fa05", "fa06"}, true
         );
         retiringFedSigners.sort(BtcECKey.PUBKEY_COMPARATOR);
-        List<FederationMember> retiringFedMembers = FederationTestUtils.getFederationMembersWithBtcKeys(retiringFedSigners);
+        List<FederationMember> retiringFedMembers = FederationTestUtils.getFederationMembersWithBtcKeys(
+            retiringFedSigners);
         Instant retiringCreationTime = Instant.ofEpochMilli(1000L);
         long retiringFedCreationBlockNumber = 1;
 
         FederationArgs retiringFedArgs =
-            new FederationArgs(retiringFedMembers, retiringCreationTime, retiringFedCreationBlockNumber, btcParams);
-        retiringFederation = FederationFactory.buildP2shErpFederation(retiringFedArgs, erpPubKeys, activationDelay);
+            new FederationArgs(retiringFedMembers, retiringCreationTime,
+                retiringFedCreationBlockNumber, btcParams);
+        retiringFederation = FederationFactory.buildP2shErpFederation(retiringFedArgs, erpPubKeys,
+            activationDelay);
 
         List<BtcECKey> activeFedSigners = BitcoinTestUtils.getBtcEcKeysFromSeeds(
             new String[]{"fa07", "fa08", "fa09", "fa10", "fa11"}, true
@@ -203,10 +205,11 @@ void init() throws IOException {
 
     private PartialMerkleTree createPmtAndMockBlockStore(BtcTransaction btcTransaction)
         throws BlockStoreException {
-        PartialMerkleTree pmt = new PartialMerkleTree(btcMainnetParams, new byte[]{0x3f},
-            Collections.singletonList(btcTransaction.getHash()), 1);
-        Sha256Hash blockMerkleRoot = pmt.getTxnHashAndMerkleRoot(new ArrayList<>());
 
+        PartialMerkleTree pmt = createValidPmtForTransactions(
+            Collections.singletonList(btcTransaction.getHash()), btcMainnetParams);
+
+        Sha256Hash blockMerkleRoot = pmt.getTxnHashAndMerkleRoot(new ArrayList<>());
         registerHeader = new co.rsk.bitcoinj.core.BtcBlock(
             btcMainnetParams,
             1,
@@ -265,11 +268,13 @@ private PartialMerkleTree createPmtAndMockBlockStore(BtcTransaction btcTransacti
 
     @ParameterizedTest
     @MethodSource("activationsProvider")
-    void registerBtcTransaction_whenBelowTheMinimum_shouldRejectPegin(ActivationConfig.ForBlock activations)
+    void registerBtcTransaction_whenBelowTheMinimum_shouldRejectPegin(
+        ActivationConfig.ForBlock activations)
         throws BlockStoreException, BridgeIllegalArgumentException, IOException {
         // arrange
         BtcTransaction btcTransaction = new BtcTransaction(btcMainnetParams);
-        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX, new Script(new byte[]{}));
+        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX,
+            new Script(new byte[]{}));
         btcTransaction.addOutput(belowMinimumPeginTxValue, activeFederation.getAddress());
 
         FederationSupport federationSupport = FederationSupportBuilder.builder()
@@ -303,8 +308,9 @@ void registerBtcTransaction_whenBelowTheMinimum_shouldRejectPegin(ActivationConf
         // assert
 
         // tx should be marked as processed since RSKIP459 is active
-        var shouldMarkTxAsProcessed = activations == lovellActivations? times(1) : never();
-        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(), anyLong());
+        var shouldMarkTxAsProcessed = activations == lovellActivations ? times(1) : never();
+        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(),
+            anyLong());
 
         verify(bridgeEventLogger, times(1)).logRejectedPegin(btcTransaction, INVALID_AMOUNT);
         verify(bridgeEventLogger, times(1)).logUnrefundablePegin(btcTransaction,
@@ -316,7 +322,8 @@ void registerBtcTransaction_whenBelowTheMinimum_shouldRejectPegin(ActivationConf
 
     @ParameterizedTest
     @MethodSource("activationsProvider")
-    void registerBtcTransaction_whenUndeterminedSender_shouldRejectPegin(ActivationConfig.ForBlock activations)
+    void registerBtcTransaction_whenUndeterminedSender_shouldRejectPegin(
+        ActivationConfig.ForBlock activations)
         throws BlockStoreException, BridgeIllegalArgumentException, IOException {
         // arrange
         btcLockSenderProvider = mock(BtcLockSenderProvider.class);
@@ -373,8 +380,9 @@ void registerBtcTransaction_whenUndeterminedSender_shouldRejectPegin(ActivationC
         verify(bridgeEventLogger, never()).logReleaseBtcRequested(any(), any(), any());
 
         // tx should be marked as processed since RSKIP459 is active
-        var shouldMarkTxAsProcessed = activations == lovellActivations? times(1) : never();
-        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(), anyLong());
+        var shouldMarkTxAsProcessed = activations == lovellActivations ? times(1) : never();
+        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(),
+            anyLong());
 
         Assertions.assertTrue(activeFederationUtxos.isEmpty());
         Assertions.assertTrue(retiringFederationUtxos.isEmpty());
@@ -383,7 +391,8 @@ void registerBtcTransaction_whenUndeterminedSender_shouldRejectPegin(ActivationC
 
     @ParameterizedTest
     @MethodSource("activationsProvider")
-    void registerBtcTransaction_whenPeginV1WithInvalidPayloadAndUnderminedSender_shouldRejectPegin(ActivationConfig.ForBlock activations)
+    void registerBtcTransaction_whenPeginV1WithInvalidPayloadAndUnderminedSender_shouldRejectPegin(
+        ActivationConfig.ForBlock activations)
         throws BlockStoreException, BridgeIllegalArgumentException, IOException {
         // arrange
         btcLockSenderProvider = mock(BtcLockSenderProvider.class);
@@ -397,7 +406,8 @@ void registerBtcTransaction_whenPeginV1WithInvalidPayloadAndUnderminedSender_sho
             new Script(new byte[]{})
         );
         btcTransaction.addOutput(Coin.COIN, activeFederation.getAddress());
-        btcTransaction.addOutput(Coin.ZERO, PegTestUtils.createOpReturnScriptForRskWithCustomPayload(1, new byte[]{}));
+        btcTransaction.addOutput(Coin.ZERO,
+            PegTestUtils.createOpReturnScriptForRskWithCustomPayload(1, new byte[]{}));
 
         FederationSupport federationSupport = FederationSupportBuilder.builder()
             .withFederationConstants(federationMainnetConstants)
@@ -430,8 +440,9 @@ void registerBtcTransaction_whenPeginV1WithInvalidPayloadAndUnderminedSender_sho
         // assert
 
         // tx should be marked as processed since RSKIP459 is active
-        var shouldMarkTxAsProcessed = activations == lovellActivations? times(1) : never();
-        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(), anyLong());
+        var shouldMarkTxAsProcessed = activations == lovellActivations ? times(1) : never();
+        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(),
+            anyLong());
 
         verify(bridgeEventLogger, times(1)).logRejectedPegin(
             btcTransaction, PEGIN_V1_INVALID_PAYLOAD
@@ -457,11 +468,13 @@ void registerBtcTransaction_whenUtxoToActiveFedBelowMinimumAndUtxoToRetiringFedA
     ) throws BlockStoreException, BridgeIllegalArgumentException, IOException {
         // arrange
         BtcTransaction btcTransaction = new BtcTransaction(btcMainnetParams);
-        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX, new Script(new byte[]{}));
+        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX,
+            new Script(new byte[]{}));
         btcTransaction.addOutput(belowMinimumPeginTxValue, activeFederation.getAddress());
         btcTransaction.addOutput(minimumPeginTxValue, retiringFederation.getAddress());
 
-        when(federationStorageProvider.getOldFederation(federationMainnetConstants, activations)).thenReturn(retiringFederation);
+        when(federationStorageProvider.getOldFederation(federationMainnetConstants,
+            activations)).thenReturn(retiringFederation);
         FederationSupport federationSupport = FederationSupportBuilder.builder()
             .withFederationConstants(federationMainnetConstants)
             .withFederationStorageProvider(federationStorageProvider)
@@ -491,10 +504,12 @@ void registerBtcTransaction_whenUtxoToActiveFedBelowMinimumAndUtxoToRetiringFedA
         );
 
         // assert
-        var shouldMarkTxAsProcessed = activations == lovellActivations? times(1) : never();
-        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(), anyLong());
+        var shouldMarkTxAsProcessed = activations == lovellActivations ? times(1) : never();
+        verify(provider, shouldMarkTxAsProcessed).setHeightBtcTxhashAlreadyProcessed(any(),
+            anyLong());
 
-        verify(bridgeEventLogger, times(1)).logRejectedPegin(btcTransaction, RejectedPeginReason.LEGACY_PEGIN_UNDETERMINED_SENDER);
+        verify(bridgeEventLogger, times(1)).logRejectedPegin(btcTransaction,
+            RejectedPeginReason.LEGACY_PEGIN_UNDETERMINED_SENDER);
         verify(bridgeEventLogger, times(1)).logUnrefundablePegin(btcTransaction,
             UnrefundablePeginReason.LEGACY_PEGIN_UNDETERMINED_SENDER);
         verify(bridgeEventLogger, never()).logPeginBtc(any(), any(), any(), anyInt());
@@ -505,7 +520,8 @@ void registerBtcTransaction_whenUtxoToActiveFedBelowMinimumAndUtxoToRetiringFedA
     // flyover pegin
     @ParameterizedTest
     @MethodSource("activationsProvider")
-    void registerBtcTransaction_whenAttemptToRegisterFlyoverPegin_shouldIgnorePegin(ActivationConfig.ForBlock activations)
+    void registerBtcTransaction_whenAttemptToRegisterFlyoverPegin_shouldIgnorePegin(
+        ActivationConfig.ForBlock activations)
         throws BlockStoreException, BridgeIllegalArgumentException, IOException {
         // arrange
         Address userRefundBtcAddress = BitcoinTestUtils.createP2PKHAddress(btcMainnetParams,
@@ -549,7 +565,8 @@ void registerBtcTransaction_whenAttemptToRegisterFlyoverPegin_shouldIgnorePegin(
         );
 
         BtcTransaction btcTransaction = new BtcTransaction(bridgeMainnetConstants.getBtcParams());
-        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX, new Script(new byte[]{}));
+        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX,
+            new Script(new byte[]{}));
         btcTransaction.addOutput(minimumPeginTxValue, flyoverFederationAddress);
 
         // act
@@ -574,11 +591,13 @@ private void assertUnknownTxIsIgnored() throws IOException {
 
     @ParameterizedTest
     @MethodSource("activationsProvider")
-    void registerBtcTransaction_whenNoUtxoToFed_shouldIgnorePegin(ActivationConfig.ForBlock activations)
+    void registerBtcTransaction_whenNoUtxoToFed_shouldIgnorePegin(
+        ActivationConfig.ForBlock activations)
         throws BlockStoreException, BridgeIllegalArgumentException, IOException {
         // arrange
         BtcTransaction btcTransaction = new BtcTransaction(btcMainnetParams);
-        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX, new Script(new byte[]{}));
+        btcTransaction.addInput(BitcoinTestUtils.createHash(1), FIRST_OUTPUT_INDEX,
+            new Script(new byte[]{}));
         btcTransaction.addOutput(minimumPeginTxValue, userAddress);
 
         FederationSupport federationSupport = FederationSupportBuilder.builder()

diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java
@@ -1052,7 +1052,7 @@ void registerBtcTransaction_whenIsNotTheSpendTransaction_shouldNotProcessSpendTx
         /*
          This is a hypothetical case, which is not realistic with the implementation of the svp.
          A btc tx hash that is not saved as a spend tx, is identified as a pegin, and will be
-         rejected due to invalid amount. This is because the pegin amount is below the minimum.
+         rejected due to invalid amount, since the pegin amount is below the minimum.
          Therefore, this tx should be rejected as pegin and mark as processed
          */
         @Test