Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Skip updateBridge if there is no federation in BtcToRskClient #137

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Skip updateBridge if there is no federation in BtcToRskClient #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c5ab85
Skip updateBridge if there is no federation in BtcToRskClient
marcos-iov Feb 7, 2023
d2dd1bb
Put method name in log messages in BtcToRskClient class
marcos-iov Feb 7, 2023
d93eef7
Refactor updateBridge validations
marcos-iov Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 66 additions & 48 deletions src/main/java/co/rsk/federate/BtcToRskClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,54 @@
package co.rsk.federate;

import static com.google.common.base.Preconditions.checkNotNull;

import co.rsk.bitcoinj.core.BtcTransaction;
import co.rsk.config.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.*;
import co.rsk.federate.io.BtcToRskClientFileData;
import co.rsk.federate.io.BtcToRskClientFileReadResult;
import co.rsk.federate.io.BtcToRskClientFileStorage;
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.Federation;
import co.rsk.peg.PeginInformation;
import co.rsk.peg.btcLockSender.BtcLockSender.TxSenderAddressType;
import co.rsk.peg.btcLockSender.BtcLockSenderProvider;
import co.rsk.peg.pegininstructions.PeginInstructionsException;
import co.rsk.peg.pegininstructions.PeginInstructionsProvider;
import com.google.common.annotations.VisibleForTesting;
import co.rsk.peg.btcLockSender.BtcLockSenderProvider;
import com.google.common.collect.Lists;
import org.bitcoinj.core.*;
import org.bitcoinj.store.BlockStoreException;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.PreDestroy;
import java.io.IOException;
import java.time.Clock;
import java.util.*;
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.stream.IntStream;

import static com.google.common.base.Preconditions.checkNotNull;
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.store.BlockStoreException;
import org.ethereum.config.blockchain.upgrades.ActivationConfig;
import org.ethereum.config.blockchain.upgrades.ConsensusRule;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Manages the process of informing the RSK bridge news about the bitcoin blockchain
Expand Down Expand Up @@ -115,7 +128,7 @@ public synchronized void setup(
}

public void start(Federation federation) {
logger.info("Starting for Federation {}", federation.getAddress().toString());
logger.info("[start] Starting for Federation {}", federation.getAddress());
marcos-iov marked this conversation as resolved.
Show resolved Hide resolved
this.federation = federation;
if (federation.isMember(federatorSupport.getFederationMember())) {
logger.info("Watching federation {} since I belong to it", federation.getAddress().toString());
Expand All @@ -138,7 +151,7 @@ public void start(Federation federation) {
}

public void stop() {
logger.info("Stopping");
logger.info("[stop] Stopping");

federation = null;

Expand All @@ -153,8 +166,12 @@ public synchronized Map<Sha256Hash, List<Proof>> getTransactionsToSendToRsk() {
}

public void updateBridge() {
if (federation == null) {
logger.warn("[updateBridge] updateBridge skipped because no Federation is associated to this BtcToRskClient");
if (federation == null || nodeBlockProcessor.hasBetterBlockToSync()) {
String reason = federation == null ?
"no Federation is associated to this BtcToRskClient" :
"the node is syncing blocks";
logger.warn("[updateBridge] updateBridge skipped because {}", reason);
return;
}
if (nodeBlockProcessor.hasBetterBlockToSync()) {
logger.warn("[updateBridge] updateBridge skipped because the node is syncing blocks");
Expand Down Expand Up @@ -194,15 +211,15 @@ public void updateBridge() {
logger.debug("[updateBridge] Sending updateCollections");
federatorSupport.sendUpdateCollections();
} catch (Exception e) {
logger.error(e.getMessage(), e);
logger.error("[updateBridge] Error while updating Bridge. {}", e.getMessage());
panicProcessor.panic("btclock", e.getMessage());
}
}

@Override
public void onBlock(Block block) {
synchronized (this) {
logger.debug("onBlock {}", block.getHash());
logger.debug("[onBlock] onBlock {}", block.getHash());
PartialMerkleTree tree;
Transaction coinbase = null;
boolean dataToWrite = false;
Expand All @@ -223,7 +240,7 @@ public void onBlock(Block block) {
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, block.getHash());
continue;
}

Expand All @@ -237,22 +254,22 @@ public void onBlock(Block block) {
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());
logger.error("[onBlock] 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());
logger.error("[onBlock] 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;
}
Expand All @@ -262,14 +279,14 @@ public void onBlock(Block block) {
// 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, block.getHash());
dataToWrite = true;
}

Expand All @@ -279,23 +296,23 @@ 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());
}
}
}

@Override
public void onTransaction(Transaction tx) {
logger.debug("onTransaction {}", tx.getWTxId());
logger.debug("[onTransaction] onTransaction {}", tx.getWTxId());
synchronized (this) {
this.fileData.getTransactionProofs().put(tx.getWTxId(), new ArrayList<>());
try {
this.btcToRskClientFileStorage.write(this.fileData);
} catch (IOException e) {
logger.error(e.getMessage(), e);
logger.error("[onTransaction] {}", e.getMessage());
panicProcessor.panic("btclock", e.getMessage());
}
}
Expand All @@ -309,8 +326,8 @@ protected int updateBridgeBtcBlockchain() throws BlockStoreException, IOExceptio
int federatorBtcBlockchainBestChainHeight = bitcoinWrapper.getBestChainHeight();
if (federatorBtcBlockchainBestChainHeight > bridgeBtcBlockchainBestChainHeight) {
logger.debug(
"[updateBridgeBtcBlockchain] BTC blockchain height - Federator : {}, Bridge : {}.",
bitcoinWrapper.getBestChainHeight(),
"[updateBridgeBtcBlockchain] BTC blockchain height - Federator : {}, Bridge : {}",
federatorBtcBlockchainBestChainHeight,
bridgeBtcBlockchainBestChainHeight
);
// Federator's blockchain has more blocks than bridge's blockchain - go and try to
Expand All @@ -319,13 +336,9 @@ protected int updateBridgeBtcBlockchain() throws BlockStoreException, IOExceptio
// First, find the common ancestor that is in the federator's bestchain
// using either the old method -- block locator
// or the new one -- block depth incremental search
StoredBlock commonAncestor = null;
if (useBlockDepth) {
commonAncestor = findBridgeBtcBlockchainMatchingAncestor(bridgeBtcBlockchainBestChainHeight);
} else {
commonAncestor = findBridgeBtcBlockchainMatchingAncestorUsingBlockLocator();
}

StoredBlock commonAncestor = useBlockDepth ?
findBridgeBtcBlockchainMatchingAncestor(bridgeBtcBlockchainBestChainHeight) :
findBridgeBtcBlockchainMatchingAncestorUsingBlockLocator();
checkNotNull(commonAncestor, "No best chain block found");

logger.debug(
Expand Down Expand Up @@ -448,7 +461,11 @@ private StoredBlock findBridgeBtcBlockchainMatchingAncestorUsingBlockLocator() t
// Find the last best chain block the bridge has with respect
// to the federate node's best chain.
Object[] blockLocatorArray = federatorSupport.getBtcBlockchainBlockLocator();
logger.debug("Block locator size {}, first {}, last {}.", blockLocatorArray.length, blockLocatorArray[0], blockLocatorArray[blockLocatorArray.length - 1]);
logger.debug(
"[findBridgeBtcBlockchainMatchingAncestorUsingBlockLocator] Block locator size {}, first {}, last {}.",
blockLocatorArray.length, blockLocatorArray[0],
blockLocatorArray[blockLocatorArray.length - 1]
);

StoredBlock matchedBlock = null;
for (Object o : blockLocatorArray) {
Expand Down Expand Up @@ -630,7 +647,8 @@ protected void updateBridgeBtcTransactions() {
* Gets the first ready to be informed coinbase transaction and informs it
*/
protected void updateBridgeBtcCoinbaseTransactions() {
Optional<CoinbaseInformation> coinbaseInformationReadyToInform = fileData.getCoinbaseInformationMap()
Optional<CoinbaseInformation> coinbaseInformationReadyToInform = fileData
.getCoinbaseInformationMap()
.values()
.stream()
.filter(CoinbaseInformation::isReadyToInform)
Expand Down Expand Up @@ -701,7 +719,7 @@ private StoredBlock findBestChainStoredBlockFor(Transaction tx) throws IllegalSt

@PreDestroy
public void tearDown() throws IOException {
logger.info("BtcToRskClient tearDown starting...");
logger.info("[tearDown] BtcToRskClient tearDown starting...");

if (federation != null) {
bitcoinWrapper.removeFederationListener(federation, this);
Expand All @@ -713,7 +731,7 @@ public void tearDown() throws IOException {
this.btcToRskClientFileStorage.write(this.fileData);
}

logger.info("BtcToRskClient tearDown finished.");
logger.info("[tearDown] BtcToRskClient tearDown finished.");
}

private void restoreFileData() throws Exception {
Expand All @@ -725,12 +743,12 @@ private void restoreFileData() throws Exception {
this.fileData = result.getData();
} else {
String errorMessage = "Can't operate without a valid storage file";
logger.error(errorMessage);
panicProcessor.panic("fed-storage",errorMessage);
logger.error("[restoreFileData] {}", errorMessage);
panicProcessor.panic("fed-storage", errorMessage);
throw new Exception(errorMessage);
}
} catch (IOException e) {
logger.error("Failed to read data from BtcToRskClient file: {}", e.getMessage(), e);
logger.error("[restoreFileData] Failed to read data from BtcToRskClient file: {}", e.getMessage(), e);
panicProcessor.panic("fed-storage",e.getMessage());
throw new Exception("Failed to read data from BtcToRskClient file", e);
}
Expand Down
24 changes: 21 additions & 3 deletions src/test/java/co/rsk/federate/BtcToRskClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2233,8 +2233,7 @@ public void updateBridgeBtcCoinbaseTransactions_when_coinbase_map_has_readyToBeI
}

@Test
public void updateBridgeBtcCoinbaseTransactions_not_removing_from_storage_until_confirmation()
throws Exception {
public void updateBridgeBtcCoinbaseTransactions_not_removing_from_storage_until_confirmation() throws Exception {
Sha256Hash blockHash = Sha256Hash.ZERO_HASH;

ActivationConfig activations = mock(ActivationConfig.class);
Expand Down Expand Up @@ -2282,7 +2281,26 @@ public void updateBridge_when_hasBetterBlockToSync_does_not_update_headers() thr
NodeBlockProcessor nodeBlockProcessor = mock(NodeBlockProcessor.class);
when(nodeBlockProcessor.hasBetterBlockToSync()).thenReturn(true);

BtcToRskClient btcToRskClient = spy(buildWithFactory(mock(FederatorSupport.class), nodeBlockProcessor));
BtcToRskClient btcToRskClient = spy(buildWithFactory(
mock(FederatorSupport.class),
nodeBlockProcessor
));
btcToRskClient.start(mock(Federation.class)); // Ensure the federation is not null

btcToRskClient.updateBridge();

verify(btcToRskClient, never()).updateBridgeBtcBlockchain();
}

@Test
public void updateBridge_when_federation_is_null_does_not_update_headers() throws IOException, BlockStoreException {
NodeBlockProcessor nodeBlockProcessor = mock(NodeBlockProcessor.class);
when(nodeBlockProcessor.hasBetterBlockToSync()).thenReturn(false);

BtcToRskClient btcToRskClient = spy(buildWithFactory(
mock(FederatorSupport.class),
nodeBlockProcessor
));

btcToRskClient.updateBridge();

Expand Down