Merge pull request #360 from rsksmart/coinbase-refactor

Coinbase refactor

Author: aeidelman

src/main/java/co/rsk/federate/BtcToRskClient.java

@@ -3,50 +3,33 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import co.rsk.bitcoinj.core.BtcTransaction;
-import co.rsk.federate.config.PowpegNodeSystemProperties;
-import co.rsk.peg.constants.BridgeConstants;
 import co.rsk.federate.adapter.ThinConverter;
-import co.rsk.federate.bitcoin.BitcoinWrapper;
-import co.rsk.federate.bitcoin.BlockListener;
-import co.rsk.federate.bitcoin.TransactionListener;
-import co.rsk.federate.io.BtcToRskClientFileData;
-import co.rsk.federate.io.BtcToRskClientFileReadResult;
-import co.rsk.federate.io.BtcToRskClientFileStorage;
+import co.rsk.federate.bitcoin.*;
+import co.rsk.federate.config.PowpegNodeSystemProperties;
+import co.rsk.federate.io.*;
 import co.rsk.federate.timing.TurnScheduler;
 import co.rsk.net.NodeBlockProcessor;
 import co.rsk.panic.PanicProcessor;
 import co.rsk.peg.BridgeUtils;
 import co.rsk.peg.PegUtilsLegacy;
-import co.rsk.peg.federation.Federation;
-import co.rsk.peg.federation.FederationMember;
 import co.rsk.peg.PeginInformation;
+import co.rsk.peg.bitcoin.BitcoinUtils;
 import co.rsk.peg.btcLockSender.BtcLockSender.TxSenderAddressType;
 import co.rsk.peg.btcLockSender.BtcLockSenderProvider;
+import co.rsk.peg.constants.BridgeConstants;
+import co.rsk.peg.federation.Federation;
+import co.rsk.peg.federation.FederationMember;
 import co.rsk.peg.pegininstructions.PeginInstructionsException;
 import co.rsk.peg.pegininstructions.PeginInstructionsProvider;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import java.io.IOException;
 import java.time.Clock;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
+import java.util.*;
+import java.util.concurrent.*;
 import java.util.stream.IntStream;
 import javax.annotation.PreDestroy;
-import org.bitcoinj.core.Block;
-import org.bitcoinj.core.NetworkParameters;
-import org.bitcoinj.core.PartialMerkleTree;
-import org.bitcoinj.core.Sha256Hash;
-import org.bitcoinj.core.StoredBlock;
-import org.bitcoinj.core.Transaction;
-import org.bitcoinj.core.Utils;
+import org.bitcoinj.core.*;
 import org.bitcoinj.store.BlockStoreException;
 import org.ethereum.config.blockchain.upgrades.ActivationConfig;
 import org.ethereum.config.blockchain.upgrades.ConsensusRule;
@@ -243,13 +226,12 @@ public void updateBridge() {
                 panicProcessor.panic("btclock", e.getMessage());
             }
         }
-
     }
 
     @Override
     public void onBlock(Block block) {
         synchronized (this) {
-            logger.debug("onBlock {}", block.getHash());
+            logger.debug("[onBlock] {}", block.getHash());
             PartialMerkleTree tree;
             Transaction coinbase = null;
             boolean dataToWrite = false;
@@ -258,6 +240,7 @@ public void onBlock(Block block) {
 
                 if (tx.isCoinBase()) {
                     // safe keep the coinbase and move on
+                    logger.debug("[onBlock] Transaction {} is the coinbase", tx.getTxId());
                     coinbase = tx;
                     continue;
                 }
@@ -266,57 +249,51 @@ public void onBlock(Block block) {
                     // this tx is not important move on
                     continue;
                 }
+                logger.debug("[onBlock] Found transaction {} that needs to be registered in the Bridge", tx.getTxId());
 
                 List<Proof> proofs = fileData.getTransactionProofs().get(tx.getWTxId());
                 boolean blockInProofs = proofs.stream().anyMatch(p -> p.getBlockHash().equals(block.getHash()));
                 if (blockInProofs) {
-                    logger.info("Proof for tx {} in block {} already stored", tx, block.getHash());
+                    logger.info("[onBlock] Proof for tx {} in block {} already stored", tx.getTxId(), block.getHash());
                     continue;
                 }
 
                 // Always use the wtxid for the lock transactions
                 tree = generatePMT(block, tx, tx.hasWitnesses());
                 // If the transaction has a witness, then we need to store the coinbase information to inform it
                 if (tx.hasWitnesses() && !coinbaseRegistered) {
+                    logger.debug("[onBlock] Transaction {} has witness, will need to store the coinbase information to inform it", tx.getTxId());
                     // We don't want to generate the PMT with the wtxid for the coinbase
                     // as it doesn't have a corresponding hash in the witness root
                     PartialMerkleTree coinbasePmt = generatePMT(block, coinbase, false);
                     try {
                         Sha256Hash witnessMerkleRoot = tree.getTxnHashAndMerkleRoot(new ArrayList<>());
                         CoinbaseInformation coinbaseInformation = new CoinbaseInformation(
-                                coinbase,
-                                witnessMerkleRoot,
-                                block.getHash(),
-                                coinbasePmt
+                            coinbase,
+                            witnessMerkleRoot,
+                            block.getHash(),
+                            coinbasePmt
                         );
-                        // Validate information
-                        byte[] witnessReservedValue = coinbaseInformation.getCoinbaseWitnessReservedValue();
-                        if (witnessReservedValue == null) {
-                            logger.error("block {} with lock segwit tx {} has coinbase with no witness reserved value. Aborting block processing.", block.getHash(), tx.getWTxId());
-                            // Can't register this transaction, it would be rejected by the Bridge
-                            return;
-                        }
-                        Sha256Hash calculatedWitnessCommitment = Sha256Hash.twiceOf(witnessMerkleRoot.getReversedBytes(), witnessReservedValue);
-                        Sha256Hash witnessCommitment = coinbase.findWitnessCommitment();
-                        if (!witnessCommitment.equals(calculatedWitnessCommitment)) {
-                            logger.error("block {} with lock segwit tx {} generated an invalid witness merkle root", tx.getWTxId(), block.getHash());
-                            // Can't register this transaction, it would be rejected by the Bridge
-                            return;
-                        }
+                        validateCoinbaseInformation(coinbaseInformation);
+
                         // store the coinbase
-                        fileData.getCoinbaseInformationMap().put(coinbaseInformation.getBlockHash(), coinbaseInformation);
+                        fileData.getCoinbaseInformationMap().put(
+                            coinbaseInformation.getBlockHash(),
+                            coinbaseInformation
+                        );
+                        logger.debug("[onBlock] Coinbase information for transaction {} successully stored", tx.getTxId());
 
                         // Register the coinbase just once per block
                         coinbaseRegistered = true;
                     } catch (Exception e) {
-                        logger.error(e.getMessage(), e);
+                        logger.error("[onBlock] {}", e.getMessage());
                         // Without a coinbase related to this transaction the Bridge would reject the transaction
                         return;
                     }
                 }
 
                 proofs.add(new Proof(block.getHash(), tree));
-                logger.info("New proof for tx {} in block {}", tx, block.getHash());
+                logger.info("[onBlock] New proof for tx {} in block {}", tx.getTxId(), block.getHash());
                 dataToWrite = true;
             }
 
@@ -326,17 +303,58 @@ public void onBlock(Block block) {
 
             try {
                 this.btcToRskClientFileStorage.write(fileData);
-                logger.info("Stored proofs for block {}", block.getHash());
+                logger.info("[onBlock] Stored proofs for block {}", block.getHash());
             } catch (IOException e) {
-                logger.error(e.getMessage(), e);
+                logger.error("[onBlock] {}", e.getMessage());
                 panicProcessor.panic("btclock", e.getMessage());
             }
         }
     }
 
+    private void validateCoinbaseInformation(CoinbaseInformation coinbaseInformation) {
+        Sha256Hash witnessMerkleRoot = coinbaseInformation.getWitnessRoot();
+        byte[] witnessReservedValue = coinbaseInformation.getCoinbaseWitnessReservedValue();
+        BtcTransaction coinbaseTransaction = ThinConverter.toThinInstance(
+            bridgeConstants.getBtcParams(),
+            coinbaseInformation.getCoinbaseTransaction()
+        );
+
+        if (witnessReservedValue == null) {
+            String message = String.format(
+                "Block %s with segwit peg-in tx %s has coinbase with no witness reserved value. Aborting block processing.",
+                coinbaseInformation.getBlockHash(),
+                coinbaseTransaction.getHash()
+            );
+            logger.error("[validateCoinbaseInformation] {}", message);
+            throw new IllegalArgumentException(message);
+        }
+
+        co.rsk.bitcoinj.core.Sha256Hash calculatedWitnessCommitment = co.rsk.bitcoinj.core.Sha256Hash.twiceOf(
+            witnessMerkleRoot.getReversedBytes(),
+            witnessReservedValue
+        );
+
+        BitcoinUtils.findWitnessCommitment(coinbaseTransaction)
+            .filter(commitment -> commitment.equals(calculatedWitnessCommitment))
+            .orElseThrow(() -> {
+                String message = String.format(
+                    "Block %s with segwit peg-in tx %s generated an invalid witness merkle root",
+                    coinbaseInformation.getBlockHash(),
+                    coinbaseTransaction.getHash()
+                );
+                logger.error("[validateCoinbaseInformation] {}", message);
+                return new IllegalArgumentException(message);
+            });
+        logger.debug(
+            "[validateCoinbaseInformation] Block {} with segwit peg-in tx {} has a valid witness merkle root",
+            coinbaseInformation.getBlockHash(),
+            coinbaseTransaction.getHash()
+        );
+    }
+
     @Override
     public void onTransaction(Transaction tx) {
-        logger.debug("onTransaction {}", tx.getWTxId());
+        logger.debug("[onTransaction] {} (wtxid:{})", tx.getTxId(), tx.getWTxId());
         synchronized (this) {
             this.fileData.getTransactionProofs().put(tx.getWTxId(), new ArrayList<>());
             try {
@@ -425,12 +443,17 @@ protected void markCoinbasesAsReadyToBeInformed(List<Block> informedBlocks) thro
             return;
         }
         boolean modified = false;
-        for (Block informedBlock:informedBlocks) {
+        for (Block informedBlock : informedBlocks) {
             if (coinbaseInformationMap.containsKey(informedBlock.getHash())) {
                 CoinbaseInformation coinbaseInformation = coinbaseInformationMap.get(informedBlock.getHash());
                 coinbaseInformation.setReadyToInform(true);
                 this.fileData.getCoinbaseInformationMap().put(informedBlock.getHash(), coinbaseInformation);
                 modified = true;
+                logger.debug(
+                    "[markCoinbasesAsReadyToBeInformed] Marked coinbase {} for block {} as ready to be informed",
+                    coinbaseInformation.getCoinbaseTransaction().getTxId(),
+                    informedBlock.getHash()
+                );
             }
         }
         if (!modified) {