Merge pull request #2931 from rsksmart/coinbase-parsing-integration-r…

…ebased

Ignore non-standard outputs when searching for the witness commitment hash

rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java

@@ -2465,34 +2465,35 @@ public void registerBtcCoinbaseTransaction(
         Sha256Hash witnessMerkleRoot,
         byte[] witnessReservedValue
     ) throws VMException {
+        final String LOG_PREFIX = "[registerBtcCoinbaseTransaction]";
         Context.propagate(btcContext);
         try{
             this.ensureBtcBlockStore();
         }catch (BlockStoreException | IOException e) {
             String message = String.format("Exception in registerBtcCoinbaseTransaction. %s", e.getMessage());
-            logger.warn("[registerBtcCoinbaseTransaction] {}", message);
+            logger.warn("{} {}", LOG_PREFIX, message);
             throw new VMException(message, e);
         }
 
         Sha256Hash btcTxHash = BtcTransactionFormatUtils.calculateBtcTxHash(btcTxSerialized);
-        logger.debug("[registerBtcCoinbaseTransaction] Going to register coinbase information for btcTx: {}", btcTxHash);
+        logger.debug("{} Going to register coinbase information for btcTx: {}", LOG_PREFIX, btcTxHash);
 
         if (witnessReservedValue.length != 32) {
             String message = String.format(
                 "Witness reserved value length can't be different than 32 bytes. Value received: %s",
                 Bytes.of(witnessReservedValue)
             );
-            logger.warn("[registerBtcCoinbaseTransaction] {}", message);
+            logger.warn("{} {}", LOG_PREFIX, message);
             throw new BridgeIllegalArgumentException(message);
         }
-        logger.trace("[registerBtcCoinbaseTransaction] Witness reserved value: {}", Bytes.of(witnessReservedValue));
+        logger.trace("{} Witness reserved value: {}", LOG_PREFIX, Bytes.of(witnessReservedValue));
 
         if (!PartialMerkleTreeFormatUtils.hasExpectedSize(pmtSerialized)) {
             String message = String.format(
                 "PartialMerkleTree doesn't have expected size. Value received: %s",
                 Bytes.of(pmtSerialized)
             );
-            logger.warn("[registerBtcCoinbaseTransaction] {}", message);
+            logger.warn("{} {}", LOG_PREFIX, message);
             throw new BridgeIllegalArgumentException(message);
         }
 
@@ -2503,18 +2504,19 @@ public void registerBtcCoinbaseTransaction(
             merkleRoot = pmt.getTxnHashAndMerkleRoot(hashesInPmt);
             if (!hashesInPmt.contains(btcTxHash)) {
                 logger.warn(
-                    "[registerBtcCoinbaseTransaction] Supplied btc tx {} is not in the supplied partial merkle tree {}",
+                    "{} Supplied btc tx {} is not in the supplied partial merkle tree {}",
+                    LOG_PREFIX,
                     btcTxHash,
                     pmt
                 );
                 return;
             }
         } catch (VerificationException e) {
             String message = String.format("Partial merkle tree could not be parsed. %s", Bytes.of(pmtSerialized));
-            logger.warn("[registerBtcCoinbaseTransaction] {}", message);
+            logger.warn("{} {}", LOG_PREFIX, message);
             throw new BridgeIllegalArgumentException(message, e);
         }
-        logger.trace("[registerBtcCoinbaseTransaction] Merkle root: {}", merkleRoot);
+        logger.trace("{} Merkle root: {}", LOG_PREFIX, merkleRoot);
 
         // Check merkle root equals btc block merkle root at the specified height in the btc best chain
         // Btc blockstore is available since we've already queried the best chain height
@@ -2523,19 +2525,21 @@ public void registerBtcCoinbaseTransaction(
             storedBlock = btcBlockStore.get(blockHash);
         } catch (BlockStoreException e) {
             logger.error(
-                "[registerBtcCoinbaseTransaction] Error gettin block {} from block store. {}",
+                "{} Error gettin block {} from block store. {}",
+                LOG_PREFIX,
                 blockHash,
                 e.getMessage()
             );
         }
 
         if (storedBlock == null) {
             String message = String.format("Block %s not yet registered", blockHash);
-            logger.warn("[registerBtcCoinbaseTransaction] {}", message);
+            logger.warn("{} {}", LOG_PREFIX, message);
             throw new BridgeIllegalArgumentException(message);
         }
         logger.trace(
-            "[registerBtcCoinbaseTransaction] Found block with hash {} at height {}",
+            "{} Found block with hash {} at height {}",
+            LOG_PREFIX,
             blockHash,
             storedBlock.getHeight()
         );
@@ -2548,8 +2552,7 @@ public void registerBtcCoinbaseTransaction(
                 merkleRoot,
                 blockHeader.getMerkleRoot()
             );
-            logger.warn("[registerBtcCoinbaseTransaction] {}", panicMessage);
-            panicProcessor.panic("btclock", panicMessage);
+            logger.warn("{} {}", LOG_PREFIX, panicMessage);
             return;
         }
 
@@ -2561,15 +2564,15 @@ public void registerBtcCoinbaseTransaction(
         CoinbaseInformation coinbaseInformation = new CoinbaseInformation(witnessMerkleRoot);
         provider.setCoinbaseInformation(blockHeader.getHash(), coinbaseInformation);
 
-        logger.debug("[registerBtcCoinbaseTransaction] Registered coinbase information for btc tx {}", btcTxHash);
+        logger.debug("{} Registered coinbase information for btc tx {}", LOG_PREFIX, btcTxHash);
     }
 
     private void validateWitnessInformation(
         BtcTransaction coinbaseTransaction,
         Sha256Hash witnessMerkleRoot,
         byte[] witnessReservedValue
     ) throws BridgeIllegalArgumentException {
-        Optional<Sha256Hash> expectedWitnessCommitment = findWitnessCommitment(coinbaseTransaction);
+        Optional<Sha256Hash> expectedWitnessCommitment = findWitnessCommitment(coinbaseTransaction, activations);
         Sha256Hash calculatedWitnessCommitment = Sha256Hash.twiceOf(witnessMerkleRoot.getReversedBytes(), witnessReservedValue);
 
         if (expectedWitnessCommitment.isEmpty() || !expectedWitnessCommitment.get().equals(calculatedWitnessCommitment)) {
@@ -3188,7 +3191,6 @@ protected boolean validationsForRegisterBtcTransaction(Sha256Hash btcTxHash, int
         } catch (Exception e) {
             String panicMessage = String.format("[validationsForRegisterBtcTransaction] Btc Tx %s Supplied Height is %d but should be greater than 0", btcTxHash, height);
             logger.warn(panicMessage);
-            panicProcessor.panic("btclock", panicMessage);
             return false;
         }
 
@@ -3227,7 +3229,6 @@ protected boolean validationsForRegisterBtcTransaction(Sha256Hash btcTxHash, int
                 blockHeader.getMerkleRoot()
             );
             logger.warn(panicMessage);
-            panicProcessor.panic("btclock", panicMessage);
             return false;
         }
 

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

@@ -8,13 +8,15 @@
 import com.google.common.collect.Lists;
 import java.util.*;
 import org.bouncycastle.util.encoders.Hex;
+import org.ethereum.config.blockchain.upgrades.ActivationConfig;
+import org.ethereum.config.blockchain.upgrades.ConsensusRule;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class BitcoinUtils {
     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 int MINIMUM_WITNESS_COMMITMENT_SIZE = WITNESS_COMMITMENT_LENGTH + 2; // 1 extra byte 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;
 
@@ -110,25 +112,35 @@ public static Sha256Hash generateSigHashForP2SHTransactionInput(BtcTransaction b
             .orElseThrow(() -> new IllegalArgumentException("Couldn't extract redeem script from p2sh input"));
     }
 
-    public static Optional<Sha256Hash> findWitnessCommitment(BtcTransaction tx) {
+    public static Optional<Sha256Hash> findWitnessCommitment(BtcTransaction tx, ActivationConfig.ForBlock activations) {
         Preconditions.checkState(tx.isCoinBase());
         // If more than one witness commitment, take the last one as the valid one
         List<TransactionOutput> outputsReversed = Lists.reverse(tx.getOutputs());
 
         for (TransactionOutput output : outputsReversed) {
-            Script scriptPubKey = output.getScriptPubKey();
-            if (isWitnessCommitment(scriptPubKey)) {
-                Sha256Hash witnessCommitment = extractWitnessCommitmentHash(scriptPubKey);
+            byte[] scriptPubKeyBytes = getOutputScriptPubKeyBytes(activations, output);
+
+            if (isWitnessCommitment(scriptPubKeyBytes)) {
+                Sha256Hash witnessCommitment = extractWitnessCommitmentHash(scriptPubKeyBytes);
                 return Optional.of(witnessCommitment);
             }
         }
 
         return Optional.empty();
     }
 
-    private static boolean isWitnessCommitment(Script scriptPubKey) {
-        byte[] scriptPubKeyProgram = scriptPubKey.getProgram();
+    private static byte[] getOutputScriptPubKeyBytes(ActivationConfig.ForBlock activations, TransactionOutput output) {
+        if (!activations.isActive(ConsensusRule.RSKIP460)) {
+            /*
+                Calling `getScriptPubKey` pre RSKIP460 keeps consensus by throwing a ScripException
+                when the output has a non-standard format that bitcoinj-thin is not able to parse
+            */
+            return output.getScriptPubKey().getProgram();
+        }
+        return output.getScriptBytes();
+    }
 
+    private static boolean isWitnessCommitment(byte[] scriptPubKeyProgram) {
         return scriptPubKeyProgram.length >= MINIMUM_WITNESS_COMMITMENT_SIZE
             && hasCommitmentStructure(scriptPubKeyProgram);
     }
@@ -146,8 +158,7 @@ private static boolean hasWitnessCommitmentHeader(byte[] 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();
+    private static Sha256Hash extractWitnessCommitmentHash(byte[] scriptPubKeyProgram) {
         Preconditions.checkState(scriptPubKeyProgram.length >= MINIMUM_WITNESS_COMMITMENT_SIZE);
 
         final int WITNESS_COMMITMENT_HASH_START = 6; // 4 bytes for header + OP_RETURN + data length

rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java

@@ -105,6 +105,7 @@ public enum ConsensusRule {
     RSKIP453("rskip453"),
     RSKIP454("rskip454"),
     RSKIP459("rskip459"),
+    RSKIP460("rskip460")
     ;
 
     private final String configKey;

rskj-core/src/main/resources/expected.conf

@@ -106,6 +106,7 @@ blockchain = {
             rskip453 = <hardforkName>
             rskip454 = <hardforkName>
             rskip459 = <hardforkName>
+            rskip460 = <hardforkName>
         }
     }
     gc = {

rskj-core/src/main/resources/reference.conf

@@ -91,6 +91,7 @@ blockchain = {
             rskip453 = lovell700
             rskip454 = lovell700
             rskip459 = lovell700
+            rskip460 = lovell700
         }
     }
     gc = {