From 3ac5e264e32b5dd2055fcd2e40d45fe21d715b94 Mon Sep 17 00:00:00 2001
From: julia-zack <julia.zack@iovlabs.org>
Date: Tue, 10 Sep 2024 15:34:59 -0300
Subject: [PATCH] Add logs when throwing errors. Use getScriptSigWithSignature
 instead of updateScriptWithSignature

---
 .../java/co/rsk/peg/bitcoin/BitcoinUtils.java | 22 +++++++++++++------
 .../co/rsk/peg/bitcoin/BitcoinTestUtils.java  | 15 +++++++------
 .../co/rsk/peg/bitcoin/BitcoinUtilsTest.java  |  2 +-
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/rskj-core/src/main/java/co/rsk/peg/bitcoin/BitcoinUtils.java b/rskj-core/src/main/java/co/rsk/peg/bitcoin/BitcoinUtils.java
index f0cbe4d1050..c8e365e2746 100644
--- a/rskj-core/src/main/java/co/rsk/peg/bitcoin/BitcoinUtils.java
+++ b/rskj-core/src/main/java/co/rsk/peg/bitcoin/BitcoinUtils.java
@@ -65,21 +65,29 @@ public static Optional<Script> extractRedeemScriptFromInput(TransactionInput txI
 
     public static void removeSignaturesFromTransactionWithInputsWithP2shMultiSigInputScript(BtcTransaction transaction) {
         if (transaction.hasWitness()) {
-            throw new IllegalArgumentException("Removing signatures from SegWit transactions is not allowed.");
+            String message = "Removing signatures from SegWit transactions is not allowed.";
+            logger.error(message);
+            throw new IllegalArgumentException(message);
         }
 
         if (transaction.getInputs().isEmpty()) {
-            throw new IllegalArgumentException("Cannot remove signatures from transaction inputs of a transaction without inputs.");
+            String message = "Cannot remove signatures from transaction inputs of a transaction without inputs.";
+            logger.error(message);
+            throw new IllegalArgumentException(message);
         }
 
         for (TransactionInput input : transaction.getInputs()) {
             Optional<Script> inputRedeemScriptOpt = extractRedeemScriptFromInput(input);
 
-            if (!inputRedeemScriptOpt.isPresent()) {
-                throw new IllegalArgumentException("Cannot remove signatures from transaction inputs that does not have p2sh multisig input script.");
-            }
-
-            Script inputRedeemScript = inputRedeemScriptOpt.get();
+            Script inputRedeemScript = Optional.of(inputRedeemScriptOpt)
+                .get()
+                .orElseThrow(
+                    () -> {
+                        String message = "Cannot remove signatures from transaction inputs that do not have p2sh multisig input script.";
+                        logger.error(message);
+                        return new IllegalArgumentException(message);
+                    }
+                );
             Script p2shScript = ScriptBuilder.createP2SHOutputScript(inputRedeemScript);
             Script emptyInputScript = p2shScript.createEmptyInputScript(null, inputRedeemScript);
             input.setScriptSig(emptyInputScript);
diff --git a/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinTestUtils.java b/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinTestUtils.java
index 0a5cd86ce99..c3e763843ae 100644
--- a/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinTestUtils.java
+++ b/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinTestUtils.java
@@ -14,6 +14,8 @@
 import org.bouncycastle.util.encoders.Hex;
 import org.ethereum.crypto.HashUtil;
 
+import static co.rsk.bitcoinj.script.ScriptBuilder.createP2SHOutputScript;
+
 public class BitcoinTestUtils {
 
     public static List<BtcECKey> getBtcEcKeysFromSeeds(String[] seeds, boolean sorted) {
@@ -38,7 +40,7 @@ public static Address createP2PKHAddress(NetworkParameters networkParameters, St
 
     public static Address createP2SHMultisigAddress(NetworkParameters networkParameters, List<BtcECKey> keys) {
         Script redeemScript = ScriptBuilder.createRedeemScript((keys.size() / 2) + 1, keys);
-        Script outputScript = ScriptBuilder.createP2SHOutputScript(redeemScript);
+        Script outputScript = createP2SHOutputScript(redeemScript);
 
         return Address.fromP2SHScript(networkParameters, outputScript);
     }
@@ -137,11 +139,10 @@ private static void signLegacyTransactionInputWithP2shMultiSigInputScript(BtcTra
         TransactionInput input = transaction.getInput(inputIndex);
 
         Optional<Script> inputRedeemScriptOpt = BitcoinUtils.extractRedeemScriptFromInput(input);
-        if (!inputRedeemScriptOpt.isPresent()) {
-            throw new IllegalArgumentException("Cannot sign inputs that are not from a multisig");
-        }
+        Script inputRedeemScript = Optional.of(inputRedeemScriptOpt)
+            .get()
+            .orElseThrow(() -> new IllegalArgumentException("Cannot sign inputs that are not from a multisig"));
 
-        Script inputRedeemScript = inputRedeemScriptOpt.get();
         Sha256Hash sigHash = transaction.hashForSignature(inputIndex, inputRedeemScript, BtcTransaction.SigHash.ALL, false);
         Script inputScriptSig = input.getScriptSig();
         for (BtcECKey key : keys) {
@@ -149,8 +150,8 @@ private static void signLegacyTransactionInputWithP2shMultiSigInputScript(BtcTra
             TransactionSignature txSig = new TransactionSignature(sig, BtcTransaction.SigHash.ALL, false);
             byte[] txSigEncoded = txSig.encodeToBitcoin();
 
-            inputScriptSig =
-                ScriptBuilder.updateScriptWithSignature(inputScriptSig, txSigEncoded, keys.indexOf(key), 1, 1);
+            Script outputScript = createP2SHOutputScript(inputRedeemScript);
+            inputScriptSig = outputScript.getScriptSigWithSignature(inputScriptSig, txSigEncoded, keys.indexOf(key));
             input.setScriptSig(inputScriptSig);
         }
     }
diff --git a/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinUtilsTest.java b/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinUtilsTest.java
index fa16cc241d3..016c5d5f21a 100644
--- a/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinUtilsTest.java
+++ b/rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinUtilsTest.java
@@ -341,7 +341,7 @@ void removeSignaturesFromTransaction_whenTransactionInputsDoNotHaveP2shMultiSigI
         // act & assert
         IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class,
             () -> BitcoinUtils.removeSignaturesFromTransactionWithInputsWithP2shMultiSigInputScript(transaction));
-        Assertions.assertEquals("Cannot remove signatures from transaction inputs that does not have p2sh multisig input script.", exception.getMessage());
+        Assertions.assertEquals("Cannot remove signatures from transaction inputs that do not have p2sh multisig input script.", exception.getMessage());
     }
 
     @Test