Merge branch 'master' into add_valid_range_to_fsmgp

gradle/verification-metadata.xml

@@ -54,12 +54,12 @@
             <sha256 value="c717c468bfc91536f1dbc0d98d6116a8e7c49fbaff643ef8710e7505cc450878" origin="Generated by Gradle"/>
          </artifact>
       </component>
-      <component group="co.rsk.bitcoinj" name="bitcoinj-thin" version="0.14.4-rsk-16">
-         <artifact name="bitcoinj-thin-0.14.4-rsk-16.jar">
-            <sha256 value="53957d6941a882f7aa33188da3dadcf96f15fa2ffd62e9130937eb84e98f11a4" origin="Generated by Gradle"/>
+      <component group="co.rsk.bitcoinj" name="bitcoinj-thin" version="0.14.4-rsk-17">
+         <artifact name="bitcoinj-thin-0.14.4-rsk-17.jar">
+            <sha256 value="beff6002e72162984daa3e7016b1a6fb6d372e5cfb14101a50d150c0adfbda71" origin="Generated by Gradle"/>
          </artifact>
-         <artifact name="bitcoinj-thin-0.14.4-rsk-16.pom">
-            <sha256 value="ea7726838385d7694f63622398d871b92a247b83418fe4d6cb9c6f3d5991fd3a" origin="Generated by Gradle"/>
+         <artifact name="bitcoinj-thin-0.14.4-rsk-17.pom">
+            <sha256 value="32fca4aac555f60d824cda58f69e64a7641f60dbbf5ec733966dbe2ded3214d7" origin="Generated by Gradle"/>
          </artifact>
       </component>
       <component group="com.diffplug.durian" name="durian-collect" version="1.2.0">

rskj-core/build.gradle

@@ -273,7 +273,7 @@ ext {
             jaxwsRtVer             : '2.3.5',
             picocliVer             : '4.6.3',
 
-            bitcoinjThinVer: '0.14.4-rsk-17-SNAPSHOT',
+            bitcoinjThinVer: '0.14.4-rsk-17',
             rskjNativeVer: '1.3.0',
     ]
 

rskj-core/src/main/java/co/rsk/peg/bitcoin/BitcoinUtils.java

@@ -12,9 +12,9 @@
 import org.slf4j.LoggerFactory;
 
 public class BitcoinUtils {
-    public static final byte WITNESS_COMMITMENT_LENGTH = 36; // 4 bytes for header, 32 for hash
-    public static final Sha256Hash WITNESS_RESERVED_VALUE = Sha256Hash.ZERO_HASH;
     protected static final byte[]  WITNESS_COMMITMENT_HEADER = Hex.decode("aa21a9ed");
+    protected static final int WITNESS_COMMITMENT_LENGTH = WITNESS_COMMITMENT_HEADER.length + Sha256Hash.LENGTH;
+    private static final int MINIMUM_WITNESS_COMMITMENT_SIZE = WITNESS_COMMITMENT_LENGTH + 2; // 1 extra by for OP_RETURN and another one for data length
     private static final Logger logger = LoggerFactory.getLogger(BitcoinUtils.class);
     private static final int FIRST_INPUT_INDEX = 0;
 
@@ -90,7 +90,7 @@ public static Optional<Sha256Hash> findWitnessCommitment(BtcTransaction tx) {
         for (TransactionOutput output : outputsReversed) {
             Script scriptPubKey = output.getScriptPubKey();
             if (isWitnessCommitment(scriptPubKey)) {
-                Sha256Hash witnessCommitment = ScriptPattern.extractWitnessCommitmentHash(scriptPubKey);
+                Sha256Hash witnessCommitment = extractWitnessCommitmentHash(scriptPubKey);
                 return Optional.of(witnessCommitment);
             }
         }
@@ -99,7 +99,6 @@ public static Optional<Sha256Hash> findWitnessCommitment(BtcTransaction tx) {
     }
 
     private static boolean isWitnessCommitment(Script scriptPubKey) {
-        final int MINIMUM_WITNESS_COMMITMENT_SIZE = 38;
         byte[] scriptPubKeyProgram = scriptPubKey.getProgram();
 
         return scriptPubKeyProgram.length >= MINIMUM_WITNESS_COMMITMENT_SIZE
@@ -115,4 +114,21 @@ private static boolean hasCommitmentStructure(byte[] scriptPubKeyProgram) {
     private static boolean hasWitnessCommitmentHeader(byte[] header) {
         return Arrays.equals(header, WITNESS_COMMITMENT_HEADER);
     }
+
+    /**
+     * Retrieves the hash from a segwit commitment (in an output of the coinbase transaction).
+     */
+    private static Sha256Hash extractWitnessCommitmentHash(Script scriptPubKey) {
+        byte[] scriptPubKeyProgram = scriptPubKey.getProgram();
+        Preconditions.checkState(scriptPubKeyProgram.length >= MINIMUM_WITNESS_COMMITMENT_SIZE);
+
+        final int WITNESS_COMMITMENT_HASH_START = 6; // 4 bytes for header + OP_RETURN + data length
+        byte[] witnessCommitmentHash = Arrays.copyOfRange(
+            scriptPubKeyProgram,
+            WITNESS_COMMITMENT_HASH_START,
+            MINIMUM_WITNESS_COMMITMENT_SIZE
+        );
+
+        return Sha256Hash.wrap(witnessCommitmentHash);
+    }
 }

rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinTestUtils.java

@@ -14,6 +14,7 @@
 import org.ethereum.util.ByteUtil;
 
 public class BitcoinTestUtils {
+    public static final Sha256Hash WITNESS_RESERVED_VALUE = Sha256Hash.ZERO_HASH;
 
     public static BtcECKey getBtcEcKeyFromSeed(String seed) {
         byte[] serializedSeed = HashUtil.keccak256(seed.getBytes(StandardCharsets.UTF_8));
@@ -215,7 +216,7 @@ public static BtcTransaction createCoinbaseTransactionWithWrongWitnessCommitment
         byte[] wrongWitnessCommitmentWithHeader = ByteUtil.merge(
             new byte[]{ScriptOpCodes.OP_RETURN},
             new byte[]{ScriptOpCodes.OP_PUSHDATA1},
-            new byte[]{BitcoinUtils.WITNESS_COMMITMENT_LENGTH},
+            new byte[]{(byte) BitcoinUtils.WITNESS_COMMITMENT_LENGTH},
             BitcoinUtils.WITNESS_COMMITMENT_HEADER,
             witnessCommitment.getBytes()
         );
@@ -230,7 +231,7 @@ private static BtcTransaction createCoinbaseTxWithWitnessReservedValue(NetworkPa
         BtcTransaction coinbaseTx = createCoinbaseTransaction(networkParameters);
 
         TransactionWitness txWitness = new TransactionWitness(1);
-        txWitness.setPush(0, BitcoinUtils.WITNESS_RESERVED_VALUE.getBytes());
+        txWitness.setPush(0, WITNESS_RESERVED_VALUE.getBytes());
         coinbaseTx.setWitness(0, txWitness);
 
         return coinbaseTx;

rskj-core/src/test/java/co/rsk/peg/bitcoin/BitcoinUtilsTest.java

@@ -33,7 +33,7 @@ private boolean isSigHashValid(Sha256Hash sigHash, List<BtcECKey> pubKeys, List<
         LinkedList<BtcECKey> keys = new LinkedList<>(pubKeys);
         LinkedList<BtcECKey.ECDSASignature> sigs = new LinkedList<>(signatures);
 
-        while (sigs.size() > 0){
+        while (!sigs.isEmpty()){
             BtcECKey pubKey = keys.pollFirst();
             BtcECKey.ECDSASignature signature = sigs.getFirst();
             if(pubKey.verify(sigHash, signature)){
@@ -272,7 +272,7 @@ void extractRedeemScriptFromInput_p2sh_pegin_v1() {
         assertFalse(btcTx.getInputs().isEmpty());
 
         List<ScriptChunk> scriptSigChunks = scriptSig.getChunks();
-        Script expectedRedeemScript = new Script( scriptSigChunks.get(scriptSigChunks.size()- 1).data);
+        Script expectedRedeemScript = new Script(scriptSigChunks.get(scriptSigChunks.size()- 1).data);
 
         // Act
         Optional<Script> redeemScript = BitcoinUtils.extractRedeemScriptFromInput(btcTx.getInputs().get(FIRST_INPUT_INDEX));
@@ -398,7 +398,6 @@ void findWitnessCommitment_whenTxHasNoOutputs_shouldThrowException() {
 
             // Act
             assertThrows(IllegalStateException.class, () -> BitcoinUtils.findWitnessCommitment(btcTx));
-            assertThrows(IllegalStateException.class, btcTx::findWitnessCommitment);
         }
 
         @Test
@@ -417,7 +416,6 @@ void findWitnessCommitment_whenTxIsNotCoinbase_shouldThrowException() {
 
             // Act
             assertThrows(IllegalStateException.class, () -> BitcoinUtils.findWitnessCommitment(btcTx));
-            assertThrows(IllegalStateException.class, btcTx::findWitnessCommitment);
         }
 
         @Test
@@ -430,7 +428,6 @@ void findWitnessCommitment_whenNoWitnessCommitment_shouldReturnEmpty() {
 
             // Assert
             assertFalse(witnessCommitment.isPresent());
-            assertNull(btcTx.findWitnessCommitment());
         }
 
         @Test
@@ -448,7 +445,6 @@ void findWitnessCommitment_withWitnessCommitment_shouldReturnExpectedValue() {
             // Assert
             assertTrue(witnessCommitmentFound.isPresent());
             assertEquals(witnessCommitment, witnessCommitmentFound.get());
-            assertEquals(witnessCommitment, btcTx.findWitnessCommitment());
         }
 
         @Test
@@ -468,7 +464,6 @@ void findWitnessCommitment_withMultipleWitnessCommitments_shouldReturnLastOne()
             // Assert
             assertTrue(witnessCommitmentFound.isPresent());
             assertEquals(witnessCommitment3, witnessCommitmentFound.get());
-            assertEquals(witnessCommitment3, btcTx.findWitnessCommitment());
         }
 
         @Test
@@ -485,7 +480,6 @@ void findWitnessCommitment_withWrongWitnessCommitment_shouldReturnEmpty() {
 
             // Assert
             assertFalse(witnessCommitment.isPresent());
-            assertEquals(fakeWitnessCommitment, btcTx.findWitnessCommitment()); // bitcoinj implementation detects it as a valid witness commitment
         }
 
         @Test
@@ -502,7 +496,6 @@ void findWitnessCommitment_withRealTransaction_shouldReturnExpectedValue() {
             // Assert
             assertTrue(witnessCommitment.isPresent());
             assertEquals(expectedWitnessCommitment, witnessCommitment.get());
-            assertEquals(expectedWitnessCommitment, btcTx.findWitnessCommitment());
         }
     }
 
@@ -512,7 +505,7 @@ void findWitnessCommitment_withDataLargenThanExpected_shouldReturnEmpty() {
         BtcTransaction btcTx = BitcoinTestUtils.createCoinbaseTransaction(btcMainnetParams);
 
         TransactionWitness txWitness = new TransactionWitness(1);
-        txWitness.setPush(0, BitcoinUtils.WITNESS_RESERVED_VALUE.getBytes());
+        txWitness.setPush(0, BitcoinTestUtils.WITNESS_RESERVED_VALUE.getBytes());
         btcTx.setWitness(0, txWitness);
 
         Sha256Hash witnessCommitment = BitcoinTestUtils.createHash(100);
@@ -530,6 +523,5 @@ void findWitnessCommitment_withDataLargenThanExpected_shouldReturnEmpty() {
 
         // Assert, should not find the commitment since the data length != 36 bytes
         assertFalse(obtainedWitnessCommitment.isPresent());
-        assertNull(btcTx.findWitnessCommitment());
     }
 }