Skip to content

Commit

Permalink
Merge pull request #14 from rsksmart/remove-orphans
Browse files Browse the repository at this point in the history 
No longer accepting orphan block headers
  • Loading branch information
@josedahlquist
josedahlquist authored May 30, 2019
2 parents be97085 + 1306558 commit c3195e9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 134 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>co.rsk.bitcoinj</groupId>
<version>0.14.4-rsk-7</version>
<version>0.14.4-rsk-8</version>

<artifactId>bitcoinj-thin</artifactId>

Expand Down
140 changes: 7 additions & 133 deletions src/main/java/co/rsk/bitcoinj/core/BtcAbstractBlockChain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import co.rsk.bitcoinj.store.BlockStoreException;
import co.rsk.bitcoinj.store.BtcBlockStore;
import co.rsk.bitcoinj.wallet.Wallet;
import com.google.common.base.Preconditions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -97,29 +96,6 @@ public abstract class BtcAbstractBlockChain {

protected final NetworkParameters params;

// Holds a block header and, optionally, a list of tx hashes or block's transactions
class OrphanBlock {
final BtcBlock block;
final FilteredBlock filteredBlock;
final List<Sha256Hash> filteredTxHashes;
final Map<Sha256Hash, BtcTransaction> filteredTxn;
OrphanBlock(BtcBlock block, @Nullable List<Sha256Hash> filteredTxHashes, @Nullable Map<Sha256Hash, BtcTransaction> filteredTxn, FilteredBlock filteredBlock) {
final boolean filtered = filteredTxHashes != null && filteredTxn != null;
Preconditions.checkArgument((block.transactions == null && filtered)
|| (block.transactions != null && !filtered));
this.block = block;
this.filteredTxHashes = filteredTxHashes;
this.filteredTxn = filteredTxn;
this.filteredBlock = filteredBlock;
}
public Boolean hasFilteredBlock() {
return filteredBlock != null;
}
}
// Holds blocks that we have received but can't plug into the chain yet, eg because they were created whilst we
// were downloading the block chain.
private final LinkedHashMap<Sha256Hash, OrphanBlock> orphanBlocks = new LinkedHashMap<Sha256Hash, OrphanBlock>();

/** False positive estimation uses a double exponential moving average. */
public static final double FP_ESTIMATOR_ALPHA = 0.0001;
/** False positive estimation uses a double exponential moving average. */
Expand Down Expand Up @@ -216,21 +192,21 @@ public boolean add(FilteredBlock block) throws VerificationException {
* of blocks added during the execution of the add process.
*/
public BlockchainAddResult addBlock(BtcBlock block) throws VerificationException {
return runAddProcces(block, true, null, null, null);
return runAddProcces(block, null, null, null);
}

/**
* Same as add(FilteredBlock block) method, but returns an BlockchainAddResult that informs the global result plus a list
* of blocks added during the execution of the add process.
*/
public BlockchainAddResult addBlock(FilteredBlock block) throws VerificationException {
return runAddProcces(block.getBlockHeader(), true, block.getTransactionHashes(), block.getAssociatedTransactions(), block);
return runAddProcces(block.getBlockHeader(), block.getTransactionHashes(), block.getAssociatedTransactions(), block);
}

/**
* This code was duplicated on add(Block) and in add(FilteredBlock), as the original comment says. The way to handle exceptions should be improved
*/
private BlockchainAddResult runAddProcces(BtcBlock block, boolean tryConnecting,
private BlockchainAddResult runAddProcces(BtcBlock block,
@Nullable List<Sha256Hash> filteredTxHashList, @Nullable Map<Sha256Hash, BtcTransaction> filteredTxn, FilteredBlock filteredBlock) throws VerificationException{
try {
// The block has a list of hashes of transactions that matched the Bloom filter, and a list of associated
Expand All @@ -240,7 +216,7 @@ private BlockchainAddResult runAddProcces(BtcBlock block, boolean tryConnecting,
// a false positive, as expected in any Bloom filtering scheme). The filteredTxn list here will usually
// only be full of data when we are catching up to the head of the chain and thus haven't witnessed any
// of the transactions.
return add(block, tryConnecting, filteredTxHashList, filteredTxn, filteredBlock);
return add(block, filteredTxHashList, filteredTxn, filteredBlock);
} catch (BlockStoreException e) {
// TODO: Figure out a better way to propagate this exception to the user.
throw new RuntimeException(e);
Expand All @@ -263,22 +239,17 @@ private BlockchainAddResult runAddProcces(BtcBlock block, boolean tryConnecting,


// filteredTxHashList contains all transactions, filteredTxn just a subset
private BlockchainAddResult add(BtcBlock block, boolean tryConnecting,
private BlockchainAddResult add(BtcBlock block,
@Nullable List<Sha256Hash> filteredTxHashList, @Nullable Map<Sha256Hash, BtcTransaction> filteredTxn, FilteredBlock filteredBlock)
throws BlockStoreException, VerificationException {
// TODO: Use read/write locks to ensure that during chain download properties are still low latency.
BlockchainAddResult result = new BlockchainAddResult();
try {
// Quick check for duplicates to avoid an expensive check further down (in findSplit). This can happen a lot
// when connecting orphan transactions due to the dumb brute force algorithm we use.
// Quick check for duplicates to avoid an expensive check further down (in findSplit).
if (block.equals(getChainHead().getHeader())) {
result.setSuccess(Boolean.TRUE);
return result;
}
if (tryConnecting && orphanBlocks.containsKey(block.getHash())) {
result.setSuccess(Boolean.FALSE);
return result;
}

// If we want to verify transactions (ie we are running with full blocks), verify that block has transactions
if (shouldVerifyTransactions() && block.transactions == null)
Expand Down Expand Up @@ -308,48 +279,21 @@ private BlockchainAddResult add(BtcBlock block, boolean tryConnecting,
// Try linking it to a place in the currently known blocks.

if (storedPrev == null) {
// We can't find the previous block. Probably we are still in the process of downloading the chain and a
// block was solved whilst we were doing it. We put it to one side and try to connect it later when we
// have more blocks.
checkState(tryConnecting, "bug in tryConnectingOrphans");
log.warn("Block does not connect: {} prev {}", block.getHashAsString(), block.getPrevBlockHash());
orphanBlocks.put(block.getHash(), new OrphanBlock(block, filteredTxHashList, filteredTxn, filteredBlock));
result.setSuccess(Boolean.FALSE);
return result;
} else {
// It connects to somewhere on the chain. Not necessarily the top of the best known chain.
params.checkDifficultyTransitions(storedPrev, block, blockStore);
connectBlock(block, storedPrev, shouldVerifyTransactions(), filteredTxHashList, filteredTxn);
}

if (tryConnecting) {
List<OrphanBlock> orphans = tryConnectingOrphans();
for(OrphanBlock ob : orphans) {
result.addConnectedOrphan(ob.block);
if(ob.hasFilteredBlock())
result.addConnectedFilteredOrphan(ob.filteredBlock);
}
}

result.setSuccess(Boolean.TRUE);
return result;
} finally {
}
}

/**
* Returns the hashes of the currently stored orphan blocks and then deletes them from this objects storage.
* Used by Peer when a filter exhaustion event has occurred and thus any orphan blocks that have been downloaded
* might be inaccurate/incomplete.
*/
public Set<Sha256Hash> drainOrphanBlocks() {
try {
Set<Sha256Hash> hashes = new HashSet<Sha256Hash>(orphanBlocks.keySet());
orphanBlocks.clear();
return hashes;
} finally {
}
}

// expensiveChecks enables checks that require looking at blocks further back in the chain
// than the previous one when connecting (eg median timestamp check)
// It could be exposed, but for now we just set it to shouldVerifyTransactions()
Expand Down Expand Up @@ -529,45 +473,6 @@ protected void setChainHead(StoredBlock chainHead) throws BlockStoreException {
}
}

/**
* For each block in orphanBlocks, see if we can now fit it on top of the chain and if so, do so.
*/
private List<OrphanBlock> tryConnectingOrphans() throws VerificationException, BlockStoreException {
// For each block in our orphan list, try and fit it onto the head of the chain. If we succeed remove it
// from the list and keep going. If we changed the head of the list at the end of the round try again until
// we can't fit anything else on the top.
//
// This algorithm is kind of crappy, we should do a topo-sort then just connect them in order, but for small
// numbers of orphan blocks it does OK.
List<OrphanBlock> orphansAdded = new ArrayList<OrphanBlock>();
int blocksConnectedThisRound;
do {
blocksConnectedThisRound = 0;
Iterator<OrphanBlock> iter = orphanBlocks.values().iterator();
while (iter.hasNext()) {
OrphanBlock orphanBlock = iter.next();
// Look up the blocks previous.
StoredBlock prev = getStoredBlockInCurrentScope(orphanBlock.block.getPrevBlockHash());
if (prev == null) {
// This is still an unconnected/orphan block.
log.debug("Orphan block {} is not connectable right now", orphanBlock.block.getHash());
continue;
}
// Otherwise we can connect it now.
// False here ensures we don't recurse infinitely downwards when connecting huge chains.
log.info("Connected orphan {}", orphanBlock.block.getHash());
add(orphanBlock.block, false, orphanBlock.filteredTxHashes, orphanBlock.filteredTxn, orphanBlock.filteredBlock);
orphansAdded.add(orphanBlock);
iter.remove();
blocksConnectedThisRound++;
}
if (blocksConnectedThisRound > 0) {
log.info("Connected {} orphan blocks.", blocksConnectedThisRound);
}
} while (blocksConnectedThisRound > 0);
return orphansAdded;
}

/**
* Returns the block at the head of the current best chain. This is the block which represents the greatest
* amount of cumulative work done.
Expand All @@ -578,37 +483,6 @@ public StoredBlock getChainHead() {
}
}

/**
* An orphan block is one that does not connect to the chain anywhere (ie we can't find its parent, therefore
* it's an orphan). Typically this occurs when we are downloading the chain and didn't reach the head yet, and/or
* if a block is solved whilst we are downloading. It's possible that we see a small amount of orphan blocks which
* chain together, this method tries walking backwards through the known orphan blocks to find the bottom-most.
*
* @return from or one of froms parents, or null if "from" does not identify an orphan block
*/
@Nullable
public BtcBlock getOrphanRoot(Sha256Hash from) {
try {
OrphanBlock cursor = orphanBlocks.get(from);
if (cursor == null)
return null;
OrphanBlock tmp;
while ((tmp = orphanBlocks.get(cursor.block.getPrevBlockHash())) != null) {
cursor = tmp;
}
return cursor.block;
} finally {
}
}

/** Returns true if the given block is currently in the orphan blocks list. */
public boolean isOrphan(Sha256Hash block) {
try {
return orphanBlocks.containsKey(block);
} finally {
}
}

/**
* Returns an estimate of when the given block will be reached, assuming a perfect 10 minute average for each
* block. This is useful for turning transaction lock times into human readable times. Note that a height in
Expand Down
33 changes: 33 additions & 0 deletions src/test/java/co/rsk/bitcoinj/core/BtcBlockChainTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package co.rsk.bitcoinj.core;

import co.rsk.bitcoinj.store.BtcBlockStore;
import co.rsk.bitcoinj.store.BtcMemoryBlockStore;
import org.junit.Before;
import org.junit.Test;

import java.util.ArrayList;

import static co.rsk.bitcoinj.core.TransactionOutputTest.PARAMS;
import static co.rsk.bitcoinj.core.Utils.HEX;
import static org.junit.Assert.assertFalse;

public class BtcBlockChainTest {

BtcAbstractBlockChain blockchain;

@Before
public void setUp() throws Exception {
final BtcBlockStore blockStore = new BtcMemoryBlockStore(PARAMS);
blockchain = new BtcBlockChain(new Context(PARAMS), blockStore);
}

@Test
public void orphanBlockNotStored() {
BtcBlock block = new BtcBlock(
PARAMS, 2l, Sha256Hash.of(HEX.decode("0011")),
Sha256Hash.ZERO_HASH, PARAMS.genesisBlock.getTime().getTime() + 1, PARAMS.genesisBlock.getDifficultyTarget(),
0, new ArrayList<BtcTransaction>());
assertFalse(blockchain.add(block.cloneAsHeader()));
}

}

0 comments on commit c3195e9

Please sign in to comment.