Modify public method to remove signatures for just returning the hash…

… without them.
rsksmart · Nov 19, 2024 · 3513c98 · 3513c98
1 parent 3b02692
commit 3513c98
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 17 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
@@ -57,7 +57,19 @@ public static Optional<Script> extractRedeemScriptFromInput(TransactionInput txI
         }
     }
 
-    public static void removeSignaturesFromTransactionWithP2shMultiSigInputs(BtcTransaction transaction) {
+    public static Sha256Hash getMultiSigTransactionHashWithoutSignatures(NetworkParameters networkParameters, BtcTransaction transaction) {
+        Sha256Hash transactionHash = transaction.getHash();
+
+        if (!transaction.hasWitness()) {
+            BtcTransaction transactionCopyWithoutSignatures = new BtcTransaction(networkParameters, transaction.bitcoinSerialize()); // this is needed to not remove signatures from the actual tx
+            BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transactionCopyWithoutSignatures);
+            transactionHash = transactionCopyWithoutSignatures.getHash();
+        }
+
+        return transactionHash;
+    }
+
+    private static void removeSignaturesFromTransactionWithP2shMultiSigInputs(BtcTransaction transaction) {
         if (transaction.hasWitness()) {
             String message = "Removing signatures from SegWit transactions is not allowed.";
             logger.error("[removeSignaturesFromTransactionWithP2shMultiSigInputs] {}", message);

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
@@ -303,35 +303,36 @@ void test_extractRedeemScriptFromInput_migration_multiple_inputs() {
     }
 
     @Test
-    void removeSignaturesFromTransaction_whenTransactionDoesNotHaveInputs_shouldReturnSameTransaction() {
+    void getTransactionHashWithoutSignatures_whenTransactionDoesNotHaveInputs_shouldReturnSameTransaction() {
         // arrange
         BtcTransaction transaction = new BtcTransaction(btcMainnetParams);
         Sha256Hash transactionHashBeforeRemovingSignatures = transaction.getHash();
 
         // act
-        BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transaction);
+        Sha256Hash transactionHashWithoutSignatures = BitcoinUtils.getMultiSigTransactionHashWithoutSignatures(btcMainnetParams, transaction);
 
         // assert
-        assertEquals(transactionHashBeforeRemovingSignatures, transaction.getHash());
+        assertEquals(transactionHashBeforeRemovingSignatures, transactionHashWithoutSignatures);
     }
 
     @Test
-    void removeSignaturesFromTransaction_whenTransactionIsSegwit_shouldThrowIllegalArgumentException() {
+    void getTransactionHashWithoutSignatures_whenTransactionIsSegwit_shouldReturnExpectedTxHash() {
         // arrange
         BtcTransaction transaction = new BtcTransaction(btcMainnetParams);
         transaction.addInput(BitcoinTestUtils.createHash(1), 0, new Script(new byte[]{}));
         TransactionWitness transactionWitness = new TransactionWitness(1);
         transactionWitness.setPush(0, new byte[]{});
         transaction.setWitness(0, transactionWitness);
 
-        // act & assert
-        IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class,
-            () -> BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transaction));
-        assertEquals("Removing signatures from SegWit transactions is not allowed.", exception.getMessage());
+        // act
+        Sha256Hash transactionHashWithoutSignatures = BitcoinUtils.getMultiSigTransactionHashWithoutSignatures(btcMainnetParams, transaction);
+
+        // assert
+        assertEquals(transaction.getHash(), transactionHashWithoutSignatures);
     }
 
     @Test
-    void removeSignaturesFromTransaction_whenTransactionInputsDoNotHaveP2shMultiSigInputScript_shouldThrowIllegalArgumentException() {
+    void getTransactionHashWithoutSignatures_whenTxIsNotSegwitAndTransactionInputsDoNotHaveP2shMultiSigInputScript_shouldThrowIllegalArgumentException() {
         // arrange
         BtcTransaction transaction = new BtcTransaction(btcMainnetParams);
         BtcECKey pubKey = BitcoinTestUtils.getBtcEcKeyFromSeed("abc");
@@ -340,12 +341,12 @@ void removeSignaturesFromTransaction_whenTransactionInputsDoNotHaveP2shMultiSigI
 
         // act & assert
         IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class,
-            () -> BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transaction));
+            () -> BitcoinUtils.getMultiSigTransactionHashWithoutSignatures(btcMainnetParams, transaction));
         assertEquals("Cannot remove signatures from transaction inputs that do not have p2sh multisig input script.", exception.getMessage());
     }
 
     @Test
-    void removeSignaturesFromTransaction_whenNotAllTransactionInputsHaveP2shMultiSigInputScript_shouldThrowIllegalArgumentException() {
+    void getTransactionHashWithoutSignatures_whenTxIsNotSegwitButNotAllTransactionInputsHaveP2shMultiSigInputScript_shouldThrowIllegalArgumentException() {
         // arrange
         Federation federation = P2shErpFederationBuilder.builder().build();
         Script p2shMultiSigScriptSig = federation.getP2SHScript().createEmptyInputScript(null, federation.getRedeemScript());
@@ -358,12 +359,12 @@ void removeSignaturesFromTransaction_whenNotAllTransactionInputsHaveP2shMultiSig
 
         // act & assert
         IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class,
-            () -> BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transaction));
+            () -> BitcoinUtils.getMultiSigTransactionHashWithoutSignatures(btcMainnetParams, transaction));
         assertEquals("Cannot remove signatures from transaction inputs that do not have p2sh multisig input script.", exception.getMessage());
     }
 
     @Test
-    void removeSignaturesFromTransaction_whenTransactionIsLegacyAndInputsHaveP2shMultiSigInputScript_shouldRemoveSignatures() {
+    void getTransactionHashWithoutSignatures_whenTransactionIsLegacyAndInputsHaveP2shMultiSigInputScript_shouldReturnExpectedTxHash() {
         // arrange
         Federation federation = P2shErpFederationBuilder.builder().build();
         Script scriptSig = federation.getP2SHScript().createEmptyInputScript(null, federation.getRedeemScript());
@@ -372,7 +373,7 @@ void removeSignaturesFromTransaction_whenTransactionIsLegacyAndInputsHaveP2shMul
         transaction.addInput(BitcoinTestUtils.createHash(1), 0, scriptSig);
         transaction.addInput(BitcoinTestUtils.createHash(2), 0, scriptSig);
         transaction.addOutput(Coin.COIN, destinationAddress);
-        Sha256Hash transactionWithoutSignaturesHash = transaction.getHash();
+        Sha256Hash transactionHashBeforeSigning = transaction.getHash();
 
         List<BtcECKey> keysToSign = BitcoinTestUtils.getBtcEcKeysFromSeeds(new String[]{
             "member01",
@@ -391,10 +392,10 @@ void removeSignaturesFromTransaction_whenTransactionIsLegacyAndInputsHaveP2shMul
         }
 
         // act
-        BitcoinUtils.removeSignaturesFromTransactionWithP2shMultiSigInputs(transaction);
+        Sha256Hash transactionHashWithoutSignatures = BitcoinUtils.getMultiSigTransactionHashWithoutSignatures(btcMainnetParams, transaction);
 
         // assert
-        assertEquals(transactionWithoutSignaturesHash, transaction.getHash());
+        assertEquals(transactionHashBeforeSigning, transactionHashWithoutSignatures);
     }
 
     @Test