Handle exception when removing signatures from a p2pkh sender

rsksmart · Feb 5, 2025 · 2c02da9 · 2c02da9
1 parent 9a96134
commit 2c02da9
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 13 deletions.
diff --git a/rskj-core/src/main/java/co/rsk/peg/PegUtils.java b/rskj-core/src/main/java/co/rsk/peg/PegUtils.java
@@ -150,23 +150,33 @@ private static boolean isTheSvpFundTransaction(
         BridgeStorageProvider provider,
         BtcTransaction transaction
     ) {
-        return provider.getSvpFundTxHashUnsigned()
-            .filter(svpFundTransactionHashUnsigned ->
-                getMultiSigTransactionHashWithoutSignatures(networkParameters, transaction).equals(svpFundTransactionHashUnsigned)
-            )
-            .isPresent();
+        Sha256Hash txHashWithoutSignatures;
+        try {
+            txHashWithoutSignatures = getMultiSigTransactionHashWithoutSignatures(networkParameters, transaction);
+            return provider.getSvpFundTxHashUnsigned()
+                .filter(txHashWithoutSignatures::equals)
+                .isPresent();
+        } catch (IllegalArgumentException e) {
+            // if an IllegalArgumentException is thrown, the tx is not from a p2sh-multisig. So it's not the fund tx
+            return false;
+        }
     }
 
     private static boolean isTheSvpSpendTransaction(
         NetworkParameters networkParameters,
         BridgeStorageProvider provider,
         BtcTransaction transaction
     ) {
-        return provider.getSvpSpendTxHashUnsigned()
-            .filter(svpSpendTransactionHashUnsigned ->
-                getMultiSigTransactionHashWithoutSignatures(networkParameters, transaction).equals(svpSpendTransactionHashUnsigned)
-            )
-            .isPresent();
+        Sha256Hash txHashWithoutSignatures;
+        try {
+            txHashWithoutSignatures = getMultiSigTransactionHashWithoutSignatures(networkParameters, transaction);
+            return provider.getSvpSpendTxHashUnsigned()
+                .filter(txHashWithoutSignatures::equals)
+                .isPresent();
+        } catch (IllegalArgumentException e) {
+            // if an IllegalArgumentException is thrown, the tx is not from a p2sh-multisig. So it's not the spend tx
+            return false;
+        }
     }
 
     static PeginEvaluationResult evaluatePegin(
@@ -176,11 +186,11 @@ static PeginEvaluationResult evaluatePegin(
         Wallet fedWallet,
         ActivationConfig.ForBlock activations
     ) {
-        if(!activations.isActive(ConsensusRule.RSKIP379)) {
+        if (!activations.isActive(ConsensusRule.RSKIP379)) {
             throw new IllegalStateException("Can't call this method before RSKIP379 activation");
         }
 
-        if(!allUTXOsToFedAreAboveMinimumPeginValue(btcTx, fedWallet, minimumPeginTxValue, activations)) {
+        if (!allUTXOsToFedAreAboveMinimumPeginValue(btcTx, fedWallet, minimumPeginTxValue, activations)) {
             logger.debug("[evaluatePegin] Peg-in contains at least one utxo below the minimum value");
             return new PeginEvaluationResult(PeginProcessAction.NO_REFUND, INVALID_AMOUNT);
         }

diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java
@@ -22,6 +22,7 @@
 import co.rsk.core.RskAddress;
 import co.rsk.crypto.Keccak256;
 import co.rsk.peg.bitcoin.*;
+import co.rsk.peg.btcLockSender.BtcLockSenderProvider;
 import co.rsk.peg.constants.BridgeConstants;
 import co.rsk.peg.constants.BridgeMainNetConstants;
 import co.rsk.peg.federation.*;
@@ -1015,7 +1016,7 @@ private void assertFederatorSignedInputs(List<TransactionInput> inputs, List<Sha
     @Tag("Spend transaction registration tests")
     class SpendTxRegistrationTests {
         @Test
-        void registerBtcTransaction_whenIsNotTheSpendTransaction_shouldNotProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
+        void registerBtcTransaction_forPegout_whenWaitingForSvpSpendTx_shouldProcessPegoutButNotProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
             // arrange
             arrangeSvpSpendTransaction();
             setUpForTransactionRegistration(svpSpendTransaction);
@@ -1049,6 +1050,57 @@ void registerBtcTransaction_whenIsNotTheSpendTransaction_shouldNotProcessSpendTx
             assertProposedFederationExists();
         }
 
+        @Test
+        void registerBtcTransaction_forLegacyPegin_whenWaitingForSvpSpendTx_shouldProcessPeginButNotProcessSpendTx() throws BlockStoreException, BridgeIllegalArgumentException, IOException {
+            // arrange
+            arrangeSvpSpendTransaction();
+            setUpForTransactionRegistration(svpSpendTransaction);
+
+            BtcTransaction pegin = new BtcTransaction(btcMainnetParams);
+            BtcECKey senderPubKey = getBtcEcKeyFromSeed("legacy_pegin_sender");
+            Script scriptSig = ScriptBuilder.createInputScript(null, senderPubKey);
+            pegin.addInput(BitcoinTestUtils.createHash(1), 0, scriptSig);
+            Coin amountToSend = Coin.COIN;
+            pegin.addOutput(amountToSend, activeFederation.getAddress());
+            setUpForTransactionRegistration(pegin);
+
+            // recreate bridge support with a real btcLockSenderProvider instead of a mock to be able to parse pegin
+            BtcLockSenderProvider btcLockSenderProvider = new BtcLockSenderProvider();
+            bridgeSupport = bridgeSupportBuilder
+                .withBridgeConstants(bridgeMainNetConstants)
+                .withProvider(bridgeStorageProvider)
+                .withEventLogger(bridgeEventLogger)
+                .withActivations(allActivations)
+                .withFederationSupport(federationSupport)
+                .withFeePerKbSupport(feePerKbSupport)
+                .withExecutionBlock(rskExecutionBlock)
+                .withBtcLockSenderProvider(btcLockSenderProvider)
+                .build();
+
+            int activeFederationUtxosSizeBeforeRegisteringTx = federationSupport.getActiveFederationBtcUTXOs().size();
+
+            // act
+            bridgeSupport.registerBtcTransaction(
+                rskTx,
+                pegin.bitcoinSerialize(),
+                btcBlockWithPmtHeight,
+                pmtWithTransactions.bitcoinSerialize()
+            );
+            bridgeStorageProvider.save();
+
+            // assert
+            // pegin was registered
+            assertActiveFederationUtxosSize(activeFederationUtxosSizeBeforeRegisteringTx + 1);
+            assertTransactionWasProcessed(pegin.getHash());
+            // but spend tx was not
+            assertTransactionWasNotProcessed(svpSpendTransaction.getHash());
+
+            // svp success was not processed
+            assertSvpSpendTxHashUnsignedIsInStorage();
+            assertNoHandoverToNewFederation();
+            assertProposedFederationExists();
+        }
+
         /*
          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